feat: split rpcContext #10798

aamingaa · 2022-10-23T16:04:20Z

This is the pure version

codecov-commenter · 2022-10-23T16:40:53Z

Codecov Report

Merging #10798 (b4dad1d) into 3.2 (51fc01c) will decrease coverage by 0.05%.
The diff coverage is 56.34%.

@@             Coverage Diff              @@
##                3.2   #10798      +/-   ##
============================================
- Coverage     64.82%   64.76%   -0.06%     
+ Complexity      392      391       -1     
============================================
  Files          1355     1356       +1     
  Lines         57595    57699     +104     
  Branches       8491     8506      +15     
============================================
+ Hits          37336    37371      +35     
- Misses        16272    16326      +54     
- Partials       3987     4002      +15

Impacted Files	Coverage Δ
...ubbo/metadata/definition/model/TypeDefinition.java	`72.09% <ø> (ø)`
...rc/main/java/com/alibaba/dubbo/rpc/RpcContext.java	`6.72% <0.00%> (-0.36%)`	⬇️
...gistry/client/ReflectionBasedServiceDiscovery.java	`0.00% <0.00%> (ø)`
...g/apache/dubbo/rpc/RpcServerContextAttachment.java	`55.71% <55.71%> (ø)`
...ava/org/apache/dubbo/rpc/filter/ContextFilter.java	`82.08% <57.14%> (-7.20%)`	⬇️
...src/main/java/org/apache/dubbo/rpc/RpcContext.java	`47.14% <70.83%> (+0.75%)`	⬆️
.../cluster/filter/support/ConsumerContextFilter.java	`67.39% <75.00%> (+0.72%)`	⬆️
...egistry/kubernetes/KubernetesServiceDiscovery.java	`77.83% <100.00%> (-2.36%)`	⬇️
.../protocol/tri/call/AbstractServerCallListener.java	`75.00% <100.00%> (ø)`
...e/dubbo/rpc/protocol/tri/transport/WriteQueue.java	`72.72% <0.00%> (-12.13%)`	⬇️
... and 19 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

AlbumenJ

What is the difference between #10793

...cluster/src/main/java/org/apache/dubbo/rpc/cluster/filter/support/ConsumerContextFilter.java

dubbo-compatible/src/main/java/com/alibaba/dubbo/rpc/RpcContext.java

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/PenetrateAttachmentSelector.java

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcContext.java

AlbumenJ · 2022-10-26T02:32:40Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcContext.java

+        @Override
+        public RpcContextAttachment setObjectAttachment(String key, Object value) {
+            if (value == null) {
+                attachments.remove(key);
+            } else {
+                RpcContext.getServerResponseContext().setAttachment(key, value);
+                attachments.put(key, value);
+            }
+            return this;
+        }
+    };


override all methods in RpcContextAttachment, and proxy to serverResponse / clientResponse

AlbumenJ · 2022-10-27T03:30:53Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        public String getAttachment(String key) {
+            return RpcContext.getClientResponseContext().getAttachment(key);


Get from serverResponse first and then fall back to clientResponse

AlbumenJ · 2022-10-27T03:30:59Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        public Object getObjectAttachment(String key) {
+            return RpcContext.getClientResponseContext().getObjectAttachment(key);


Get from serverResponse first and then fall back to clientResponse

AlbumenJ · 2022-10-27T03:31:21Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        public RpcContextAttachment removeAttachment(String key) {
+            return RpcContext.getServerResponseContext().removeAttachment(key);


Remove in both serverResponse and clientResponse

AlbumenJ · 2022-10-27T03:32:13Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        public Map<String, String> getAttachments() {
+            return RpcContext.getServerResponseContext().getAttachments();


Create a new map, add all attachments from clientResponse and serverResponse. serverResponse should override clientResponse.

AlbumenJ · 2022-10-27T03:32:17Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        public Map<String, Object> getObjectAttachments() {
+            return RpcContext.getServerResponseContext().getObjectAttachments();


Create a new map, add all attachments from clientResponse and serverResponse. serverResponse should override clientResponse.

AlbumenJ · 2022-10-27T03:34:04Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        public Object get(String key) {
+            return RpcContext.getServerResponseContext().get(key);


Deprecated method. Proxy it to getAttachment

AlbumenJ · 2022-10-27T03:34:44Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        public RpcContextAttachment copyOf(boolean needCopy) {
+            return RpcContext.getServerResponseContext().copyOf(needCopy);


Internal method, should not be called. Throw exception directly. BTW, check the usage.

AlbumenJ · 2022-10-27T03:35:01Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+
+        @Override
+        protected boolean isValid() {
+            return RpcContext.getServerResponseContext().isValid();


Internal method, should not be called. Throw exception directly. BTW, check the usage.

AlbumenJ · 2022-10-27T03:35:45Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ContextFilter.java

+    public ContextFilter() {
+        ApplicationModel applicationModel = ApplicationModel.defaultModel();
+        ExtensionLoader<PenetrateAttachmentSelector> selectorExtensionLoader = applicationModel.getExtensionLoader(PenetrateAttachmentSelector.class);


Suggested change

public ContextFilter() {

ApplicationModel applicationModel = ApplicationModel.defaultModel();

ExtensionLoader<PenetrateAttachmentSelector> selectorExtensionLoader = applicationModel.getExtensionLoader(PenetrateAttachmentSelector.class);

public ContextFilter(ApplicationModel applicationModel) {

ExtensionLoader<PenetrateAttachmentSelector> selectorExtensionLoader = applicationModel.getExtensionLoader(PenetrateAttachmentSelector.class);

AlbumenJ · 2022-10-27T03:36:23Z

...ubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/support/PenetrateAttachmentSelectorMock.java

+    @Override
+    public Map<String, Object> selectReverse(Invocation invocation, RpcContextAttachment clientResponseContext, RpcContextAttachment serverResponseContext) {
+        return null;
+    }


Add some test cases for this

AlbumenJ

Please enhance the description in https://dubbo.apache.org/zh/docs3-v2/java-sdk/advanced-features-and-usage/service/attachment

AlbumenJ · 2022-11-07T23:58:13Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcServerContextAttachment.java

+            Map<String, String> attachmentMap = this;
+            attachmentMap.putAll(getAllAttachmentMap());


what is the purpose of these two lines

AlbumenJ · 2022-11-07T23:59:29Z

...-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/call/AbstractServerCallListener.java

+        RpcContext.getServerResponseContext().setRemoteAddress(remoteAddress);
        RpcContext.getServerContext().setRemoteAddress(remoteAddress);


Suggested change

RpcContext.getServerResponseContext().setRemoteAddress(remoteAddress);

RpcContext.getServerContext().setRemoteAddress(remoteAddress);

RpcContext.getServiceContext().setRemoteAddress(remoteAddress);

AlbumenJ · 2022-11-07T23:59:45Z

...-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/call/AbstractServerCallListener.java

+            RpcContext.getServerResponseContext().setRemoteApplicationName(remoteApp);
            RpcContext.getServerContext().setRemoteApplicationName(remoteApp);


Suggested change

RpcContext.getServerResponseContext().setRemoteApplicationName(remoteApp);

RpcContext.getServerContext().setRemoteApplicationName(remoteApp);

RpcContext.getServiceContext().setRemoteApplicationName(remoteApp);

AlbumenJ · 2022-11-08T00:00:15Z

...-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/call/AbstractServerCallListener.java

            RpcContext.getServerContext().setRemoteApplicationName(remoteApp);
-        }
+        };


Suggested change

};

}

…xt_2

AlbumenJ

LGTM

AlbumenJ requested changes Oct 24, 2022

View reviewed changes

feat: split rpcContext

6c98f8e

aamingaa force-pushed the feature/split_rpcContext_2 branch from 0332eae to 6c98f8e Compare October 25, 2022 17:49

feat: split rpc context v3

9922c2d

AlbumenJ requested changes Oct 26, 2022

View reviewed changes

aamingaa added 5 commits October 26, 2022 19:23

feat: move some code

e0987fa

feat: add param to penetrateattachment

b15d94e

feat: fix PenetrateAttachmentSelectorTest

bdd828a

feat: override method

eb12d9d

feat: override mothod in RpcServerContextAttachment

7dbd8b5

AlbumenJ requested changes Oct 27, 2022

View reviewed changes

AlbumenJ reviewed Oct 27, 2022

View reviewed changes

aamingaa added 5 commits October 27, 2022 20:55

feat: change some override method

b96934f

feat: override method in rpcservercontext attachment

0a489cd

feat: implement map interface

0ef3a20

feat: modify some structure

b39c3ee

feat: adjust some structure

33cb66a

AlbumenJ requested changes Nov 8, 2022

View reviewed changes

aamingaa and others added 11 commits November 8, 2022 10:11

feat: remove the init code

0f0c7a5

feat: remove the init code v1

c8b87e5

Update AbstractServerCallListener.java

4cf4aa4

enhance api

1663a4b

feat: merge before

728d90c

feat: merge

02cfd2d

Merge remote-tracking branch 'origin/3.2' into feature/split_rpcConte…

34d85cc

…xt_2

feat: add some test case

5985981

feat: add some test case v1

cff364b

feat: go pikq

b4dad1d

feat: add coverage

1840c1d

AlbumenJ approved these changes Nov 10, 2022

View reviewed changes

AlbumenJ merged commit ee9d01a into apache:3.2 Nov 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: split rpcContext #10798

feat: split rpcContext #10798

aamingaa commented Oct 23, 2022

codecov-commenter commented Oct 23, 2022 •

edited

Loading

AlbumenJ left a comment

AlbumenJ Oct 26, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ Oct 27, 2022

AlbumenJ left a comment

AlbumenJ Nov 7, 2022

AlbumenJ Nov 7, 2022

AlbumenJ Nov 7, 2022

AlbumenJ Nov 8, 2022

AlbumenJ left a comment

		Map<String, String> attachmentMap = this;
		attachmentMap.putAll(getAllAttachmentMap());

		RpcContext.getServerResponseContext().setRemoteAddress(remoteAddress);
		RpcContext.getServerContext().setRemoteAddress(remoteAddress);

	RpcContext.getServerResponseContext().setRemoteAddress(remoteAddress);
	RpcContext.getServerContext().setRemoteAddress(remoteAddress);
	RpcContext.getServiceContext().setRemoteAddress(remoteAddress);

feat: split rpcContext #10798

feat: split rpcContext #10798

Conversation

aamingaa commented Oct 23, 2022

codecov-commenter commented Oct 23, 2022 • edited Loading

Codecov Report

AlbumenJ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlbumenJ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlbumenJ left a comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 23, 2022 •

edited

Loading