fix: handle null or blank rpcExt(#6650)

[juicewcode]

Fixes #6650

Summary

• Handle null or blank rpcExt values in GrpcPlugin.
• Fall back to a default GrpcExtInfo when Gson deserialization returns null.

Test

• Added coverage for metadata with a null rpcExt.
• Added coverage for metadata with a blank rpcExt.
• Verified that both cases use the default UNARY method type without throwing an NPE.
• Verified that an explicitly configured SERVER_STREAMING method type is preserved.

Make sure that:

• You have read the contribution guidelines.
• You submit test cases (unit or integration tests) that back your changes.
• Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

[utafrali]

The null-rpcExt fix and the default UNARY method type are correct improvements, but the blank-string handling relies on Gson returning null for whitespace input rather than throwing, which is not guaranteed across Gson versions and will silently regress on a Gson upgrade to 2.9+.

[utafrali]

The Optional.ofNullable wrapper handles the case where Gson returns null (which covers a null input), but it does not protect against JsonSyntaxException, which Gson throws for blank/whitespace-only strings in Gson 2.9+. On those versions, fromJson(" ", GrpcExtInfo.class) throws rather than returns null, so this fix silently breaks the blank-string case on a Gson upgrade.

A more robust approach is to guard with a blank check before calling fromJson:

String rpcExt = metaData.getRpcExt();
GrpcExtInfo extInfo = StringUtils.isBlank(rpcExt)
    ? new GrpcExtInfo()
    : Optional.ofNullable(GsonUtils.getGson().fromJson(rpcExt, GrpcExtInfo.class))
            .orElseGet(GrpcExtInfo::new);

This is explicit about intent and does not rely on Gson's version-specific behavior for whitespace input.

[utafrali]

The blank test only covers a single-space string " ". An empty string "" is a distinct boundary case — some Gson versions return null for it while others throw JsonSyntaxException. Adding a testDoExecuteWithEmptyRpcExt alongside this one (with metaData.setRpcExt("")) would make it clear that both are handled and would catch a regression if Gson behavior changes.

[juicewcode]

Thanks for your review. I have added a separate test for an empty rpcExt value and moved the blank check before Gson deserialization. Null, empty, and whitespace-only values are now handled explicitly without relying on Gson's behavior.

[Aias00]

Suggestion (non-blocking): the guard covers the case where Gson returns a null object, but not the case where individual fields end up null. If rpcExt contains "methodType": null, or an enum name Gson cannot resolve, the field is left null and ShenyuGrpcClient#invoke still NPEs on switch (methodType) (same for JsonMessage#getMethodType).

A small normalisation after parsing would close the same class of bug completely - e.g. after building extInfo, if (Objects.isNull(extInfo.getMethodType())) { extInfo.setMethodType(MethodDescriptor.MethodType.UNARY); } with a warn log.

[Aias00]

Approving - this is a real crash fix, not just defensive plumbing.

I traced the call chain to confirm the impact: GrpcPlugin#doExecute passes extInfo.methodType into ShenyuGrpcClient#call(...), whose invoke(...) does switch (methodType), and JsonMessage#getMethodType(...) switches on it too. Both throw NullPointerException for a null methodType. Gson's fromJson(String, Class) returns null for both a null and an empty rpcExt, so before this change any grpc metadata registered without a rpcExt (or with "methodType" missing) blew up inside the plugin chain - exactly #6650.

So the combination here is right: skip the parse when rpcExt is blank, fall back to a default instance when Gson still returns null (literal null), and give methodType a non-null default instead of relying on the missing key. The three added tests (null / "" / " ") cover precisely what was previously untested, and switching the fixture to strict JSON {"timeout":5000,"methodType":"SERVER_STREAMING"} with an assertion on the parsed method type makes executeRequest actually assert something now.

Two follow-ups, neither blocking:

1. The guard handles a null object, not null fields. If rpcExt parses but leaves methodType null - an unresolvable enum name, or an explicit "methodType": null - the field ends up null and invoke(...) still NPEs. A normalisation right after parsing would close that residual path completely (see inline comment).

2. Defaulting to UNARY beats crashing, but it now silently assumes unary semantics for metadata that simply forgot to declare the method type, so a mis-registered streaming method produces a confusing downstream result instead of an immediate NPE. Consider a LOG.warn when the defaults are used. Related nit: a {"timeout":0} payload yields a 0 ms deadline; keeping the 5000 default when the parsed value is null or <= 0 would be safer.

CI is fully green (build on 17 and 21, all integrated tests, all e2e cases including the storage ones), so nothing blocks merge.