Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sending generic requests in JSON format #7637

Merged
merged 2 commits into from
May 19, 2021

Conversation

goodjava
Copy link
Contributor

Invoking a dubbo service via calling from api gateway or running dynamic scripts like groovy by simply passing string values instead of using maps or updating jar files.
zzz

像网关和groovy动态脚本这样的调用,可以直接通过传递字符串完成一次rpc调用(不必自己再组装Map之类,或者更新jar包).变的更简单.(golang 夸语言的泛化调用也会大幅度降低难度,毕竟字符串更好产出出来,相较于struct)

@@ -90,7 +94,9 @@ public Result invoke(Invoker<?> invoker, Invocation inv) throws RpcException {
generic = RpcContext.getContext().getAttachment(GENERIC_KEY);
}

if (StringUtils.isEmpty(generic)
if (ProtocolUtils.isGsonGenericSerialization(generic)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code clean: This case should after origin first if judgement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with horizonzy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get

@@ -174,6 +180,14 @@ public Result invoke(Invoker<?> invoker, Invocation inv) throws RpcException {
return invoker.invoke(inv);
}

private Object[] getArgs(final Object[] args, Type[] types) {
Gson gson = new Gson();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transfer to instance variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get

Copy link
Contributor

@xiaoheng1 xiaoheng1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to catch exceptions when converting json to the corresponding type

@horizonzy
Copy link
Member

I think it is better to catch exceptions when converting json to the corresponding type

agree, catch JsonSyntaxException expcetion and throw RpcException.

@@ -90,7 +94,9 @@ public Result invoke(Invoker<?> invoker, Invocation inv) throws RpcException {
generic = RpcContext.getContext().getAttachment(GENERIC_KEY);
}

if (StringUtils.isEmpty(generic)
if (ProtocolUtils.isGsonGenericSerialization(generic)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with horizonzy

private Object[] getArgs(final Object[] args, Type[] types) {
Gson gson = new Gson();
return IntStream.range(0, args.length).mapToObj(i -> {
String s = args[i].toString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable s is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get

@goodjava
Copy link
Contributor Author

goodjava commented Apr 28, 2021

I think it is better to catch exceptions when converting json to the corresponding type

After resolving
errir

org.apache.dubbo.rpc.RpcException: Generic serialization [gson] Json syntax exception thrown when parsing (message:abc type:int) error:java.lang.NumberFormatException: For input string: "abc"

Copy link
Contributor

@xiaoheng1 xiaoheng1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2021

Codecov Report

Merging #7637 (962416f) into master (cd5bb00) will decrease coverage by 0.16%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7637      +/-   ##
============================================
- Coverage     59.26%   59.10%   -0.17%     
  Complexity      529      529              
============================================
  Files          1076     1076              
  Lines         43529    43542      +13     
  Branches       6361     6364       +3     
============================================
- Hits          25796    25734      -62     
- Misses        14879    14935      +56     
- Partials       2854     2873      +19     
Impacted Files Coverage Δ Complexity Δ
...apache/dubbo/common/constants/CommonConstants.java 100.00% <ø> (ø) 0.00 <0.00> (ø)
...va/org/apache/dubbo/rpc/support/ProtocolUtils.java 61.53% <0.00%> (-8.03%) 0.00 <0.00> (ø)
...ava/org/apache/dubbo/rpc/filter/GenericFilter.java 37.70% <0.00%> (-3.37%) 0.00 <0.00> (ø)
...ng/transport/dispatcher/all/AllChannelHandler.java 75.86% <0.00%> (-20.69%) 0.00% <0.00%> (ø%)
...dubbo/common/status/support/LoadStatusChecker.java 46.15% <0.00%> (-15.39%) 0.00% <0.00%> (ø%)
...rg/apache/dubbo/remoting/utils/PayloadDropper.java 76.92% <0.00%> (-15.39%) 0.00% <0.00%> (ø%)
...ng/transport/dispatcher/WrappedChannelHandler.java 47.82% <0.00%> (-15.22%) 0.00% <0.00%> (ø%)
...bo/config/event/listener/LoggingEventListener.java 50.00% <0.00%> (-12.50%) 0.00% <0.00%> (ø%)
...dubbo/remoting/exchange/support/DefaultFuture.java 78.63% <0.00%> (-8.55%) 0.00% <0.00%> (ø%)
...he/dubbo/remoting/transport/netty/NettyHelper.java 29.03% <0.00%> (-6.46%) 2.00% <0.00%> (ø%)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd5bb00...962416f. Read the comment docs.

@goodjava
Copy link
Contributor Author

I think it is better to catch exceptions when converting json to the corresponding type

agree, catch JsonSyntaxException expcetion and throw RpcException.

done

@AlbumenJ
Copy link
Member

Please resolve confilcts

@goodjava goodjava force-pushed the gson_generic branch 3 times, most recently from 74046cd to 72ea841 Compare May 6, 2021 06:23
@AlbumenJ AlbumenJ merged commit a873a1d into apache:master May 19, 2021
AlbumenJ added a commit to AlbumenJ/dubbo that referenced this pull request May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants