You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "beiwei30 (GitHub)" <gi...@apache.org> on 2018/12/19 06:22:05 UTC
[GitHub] [incubator-dubbo] beiwei30 opened pull request #3013:
[DUBBO-2988] Unrecognized the other provider
## What is the purpose of the change
fix #2988
## Brief changelog
1. start to honor both 'group' and 'version'
2. git rid of relying on Dubbo protocol's exporter, instead, use injvm invoker and application model.
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
dubbo-demo/dubbo-demo-provider/src/main/java/org/apache/dubbo/demo/provider/ServiceA.java
dubbo-demo/dubbo-demo-provider/src/main/java/org/apache/dubbo/demo/provider/ServiceB.java
dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/command/impl/Ls.java
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/ProviderConsumerRegTable.java
dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/model/ConsumerModel.java
dubbo-rpc/dubbo-rpc-dubbo/pom.xml
dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java
dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/ListTelnetHandler.java
dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/ImplicitCallBackTest.java
## Verifying this change
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Make sure there is a [GITHUB_issue](https://github.com/apache/incubator-dubbo/issues) field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [ ] Format the pull request title like `[Dubbo-XXX] Fix UnknownException when host config not exist #XXX`. Each commit in the pull request should have a meaningful subject line and body.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in [test module](https://github.com/apache/incubator-dubbo/tree/master/dubbo-test).
- [ ] Run `mvn clean install -DskipTests=false` & `mvn clean test-compile failsafe:integration-test` to make sure unit-test and integration-test pass.
- [ ] If this contribution is large, please follow the [Software Donation Guide](https://github.com/apache/incubator-dubbo/wiki/Software-donation-guide).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@carryxyh I have created an issue [3059](https://github.com/apache/incubator-dubbo/issues/3059) for the same which we can use as a reffrence to work on.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
`service.equals(exporter.getInvoker().getUrl().getPath())`
This condition will lost in the new implementation while the Model concept used only takes `group/service:version` into account. I am a little confused about the relation between `path` and `group/service:version` at this moment, but I think we should be careful before we can think of them clearly enough.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@carryxyh . Got it. Thanks for expplaing it to me. What you saying is make sense and I think we allow to use the actual call path instead of by pass.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Can be change to
`if(StringUtils.isEmpty(service))`
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
let leave it as is :)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Can be change to
`if(!StringUtils.isEmpty(message))`
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Can be simplified to
`private void printAllProvidedServices(StringBuilder buf, boolean detail) {
if(!ApplicationModel.allProviderModels().isEmpty()) {
buf.append("PROVIDER:\r\n");
}
for (ProviderModel provider : ApplicationModel.allProviderModels()) {
buf.append(provider.getServiceName());
if (detail) {
buf.append(" -> ");
buf.append(" published: ");
buf.append(isRegistered(provider.getServiceName()) ? "Y" : "N");
}
buf.append("\r\n");
}
}`
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
ditto
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
I am afraid I don't fully understand, could you pls. rephrase your concern?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
same as above.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
can be rewritten as
return providerInvokerWrapperSet.stream().anyMatch(ProviderInvokerWrapper::isReg);
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
let's leave it as is.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
It is my bad I failed to explain it.
The constructor **ConsumerModel** takes 5 parameter where **methods** and **attributes are** are getting used if and only if **proxyObject ** is not null. So if **proxyObject ** is null then rest of the two methods and attributes are not getting used. So to keep it simple I was saying we can have two constructor
1. `public ConsumerModel(String serviceName, Class<?> serviceInterfaceClass)`
2.` public ConsumerModel(String serviceName, Class<?> serviceInterfaceClass, Object proxyObject, Method[] methods, Map<String, Object> attributes)`
So using constructor definition it will be easy to understand by the user that object created using constructor **1** does not deal with proxy object, methods and attributes, object populated with these 3 parameter will be null or empty. Where if someone populate object using constructor **2** will be dealing with rest of the parameters and explicitly throw exception of **proxyObject** is null.
Do let me know if I am able to explain the scenario or whether it make sense or not to consider this.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
It is my bad I failed to explain it.
The constructor **ConsumerModel** takes 5 parameter where **methods** and **attributes are** are getting used if and only if **proxyObject ** is not null. So if **proxyObject ** is null then rest of the two methods and attributes are not getting used. So to keep it simple I was saying we can have two constructor
1. public ConsumerModel(String serviceName, Class<?> serviceInterfaceClass)
2. public ConsumerModel(String serviceName, Class<?> serviceInterfaceClass, Object proxyObject, Method[] methods, Map<String, Object> attributes)
So using constructor definition it will be easy to understand by the user that object created using constructor **1** does not deal with proxy object, methods and attributes, object populated with these 3 parameter will be null or empty. Where if someone populate object using constructor **2** will be dealing with rest of the parameters and explicitly throw exception of **proxyObject** is null.
Do let me know if I am able to explain the scenario or whether it make sense or not to consider this.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
It is my bad I failed to explain it.
The constructor **ConsumerModel** takes 5 parameter where **methods** and **attributes are** are getting used if and only if **proxyObject ** is not null. So if **proxyObject ** is null then rest of the two methods and attributes are not getting used. So to keep it simple I was saying we can have two constructor
1. `public ConsumerModel(String serviceName, Class<?> serviceInterfaceClass)`
2. ` public ConsumerModel(String serviceName, Class<?> serviceInterfaceClass, Object proxyObject, Method[] methods, Map<String, Object> attributes)`
So using constructor definition it will be easy to understand by the user that object created using constructor **1** does not deal with proxy object, methods and attributes, object populated with these 3 parameter will be null or empty. Where if someone populate object using constructor **2** will be dealing with rest of the parameters and explicitly throw exception of **proxyObject** is null.
Do let me know if I am able to explain the scenario or whether it make sense or not to consider this.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 closed pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
[ pull request closed by beiwei30 ]
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
I beleive providedServices and consumedServices would be never null here?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
let's revisit this when we enhance unit test in config-api. In my opinion, config-api's unit test needs to clean up:
1. as you mentioned, not only 'mock protocol' but also other constants are needed to defined as constants.
2. there are some test cases depends on other module which may lead to cycle dependency in the future, for example: 'dubbo-registry-multicast' should not be used by 'dubbo-config-api'.
In this pull request, I solved some of cycle dependencies when I start to use 'dubbo-config-api' in 'dubbo-rpc-dubbo'. We should address others in another delegated change.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on issue #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
since there's no further comments, I will merge this pull request. @khanimteyaz you can continue your work based on it.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
can be replace by
```
public static int getConsumerAddressNum(String serviceUniqueName) {
Set<ConsumerInvokerWrapper> providerInvokerWrapperSet = ProviderConsumerRegTable.getConsumerInvoker(serviceUniqueName);
return providerInvokerWrapperSet.stream()
.map(piw->piw.getRegistryDirectory().getUrlInvokerMap())
.filter(Objects::nonNull)
.mapToInt(Map::size).sum();
}
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Can be simplified to
`private void printAllProvidedServices(StringBuilder buf, boolean detail) {
if(!ApplicationModel.allProviderModels().isEmpty()) {
buf.append("PROVIDER:\r\n");
}
for (ProviderModel provider : ApplicationModel.allProviderModels()) {
buf.append(provider.getServiceName());
if (detail) {
buf.append(" -> ");
buf.append(" published: ");
buf.append(isRegistered(provider.getServiceName()) ? "Y" : "N");
}
buf.append("\r\n");
}
}`
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
can be change to same above one of the example
`if(isServiceMatch(service,provider)) {
buf.append(provider.getServiceName()).append(" (as provider):\r\n");
for (ProviderMethodModel method : provider.getAllMethods()) {
printMethod(method.getMethod(), buf, detail);
}
}`
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Should we refactor this to a another method something like
```
if (isServiceMatch(provider,service) {
-- existing code ---
}
public static boolean isServiceMatch( ProviderModel provider,String service) {
return StringUtils.isEmpty(service)
||provider.getServiceName().equalsIgnoreCase(service)
||provider.getServiceName().equalsIgnoreCase(service)
||provider.getServiceInterfaceClass().getSimpleName().equalsIgnoreCase(service)
||provider.getServiceInterfaceClass().getName().equalsIgnoreCase(service)
}
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
can be done same as above approach
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
I doubt that invoking the underlying service instance directly bypassing Dubbo's proxy will make this `invoke` command meaningless. For a reading service, the result may be qualified as a reflection of that it will behave in production; for a writing service, the side effects may be not predictable
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
fixed
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Just one thought, as you are indicating it is only for unit test, should we mark access specifier as protected. Which could help to use it more knowingly. This is just an thought.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
can be replace by
return providerInvokerWrapperSet.stream()
.map(piw->piw.getRegistryDirectory().getUrlInvokerMap())
.filter(Objects::nonNull)
.mapToInt(Map::size).sum();
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
ok.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=h1) Report
> Merging [#3013](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/859efeff747587c10cae4fdbfbf38719ec44e009?src=pr&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `69.64%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3013 +/- ##
==========================================
+ Coverage 64.28% 64.34% +0.05%
==========================================
Files 584 584
Lines 26056 26088 +32
Branches 4562 4561 -1
==========================================
+ Hits 16751 16786 +35
+ Misses 7123 7122 -1
+ Partials 2182 2180 -2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ain/java/org/apache/dubbo/qos/command/impl/Ls.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL2NvbW1hbmQvaW1wbC9Mcy5qYXZh) | `96% <0%> (+11%)` | :arrow_up: |
| [.../java/org/apache/dubbo/config/ReferenceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9SZWZlcmVuY2VDb25maWcuamF2YQ==) | `56.01% <100%> (-0.38%)` | :arrow_down: |
| [...a/org/apache/dubbo/rpc/model/ApplicationModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9BcHBsaWNhdGlvbk1vZGVsLmphdmE=) | `94.11% <100%> (+1.26%)` | :arrow_up: |
| [...o/rpc/protocol/dubbo/telnet/ListTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9MaXN0VGVsbmV0SGFuZGxlci5qYXZh) | `65.38% <62.71%> (-14.17%)` | :arrow_down: |
| [...java/org/apache/dubbo/rpc/model/ConsumerModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9Db25zdW1lck1vZGVsLmphdmE=) | `62.5% <66.66%> (-1.79%)` | :arrow_down: |
| [...rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9JbnZva2VUZWxuZXRIYW5kbGVyLmphdmE=) | `72.64% <75.67%> (+2.47%)` | :arrow_up: |
| [...bbo/registry/support/ProviderConsumerRegTable.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L1Byb3ZpZGVyQ29uc3VtZXJSZWdUYWJsZS5qYXZh) | `84.44% <87.5%> (+9.44%)` | :arrow_up: |
| [...ache/dubbo/remoting/p2p/support/AbstractGroup.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctcDJwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9wMnAvc3VwcG9ydC9BYnN0cmFjdEdyb3VwLmphdmE=) | `45.45% <0%> (-11.37%)` | :arrow_down: |
| [...e/dubbo/remoting/transport/netty/NettyChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNoYW5uZWwuamF2YQ==) | `57.64% <0%> (-4.71%)` | :arrow_down: |
| [...c/main/java/org/apache/dubbo/rpc/RpcException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNFeGNlcHRpb24uamF2YQ==) | `85.71% <0%> (-3.58%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=footer). Last update [859efef...8ffbc9b](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Thanks.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Thanks.Appreciate your care.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
yes agree.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=h1) Report
> Merging [#3013](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/859efeff747587c10cae4fdbfbf38719ec44e009?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `70%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3013 +/- ##
==========================================
+ Coverage 64.28% 64.29% +0.01%
==========================================
Files 584 584
Lines 26056 26087 +31
Branches 4562 4561 -1
==========================================
+ Hits 16749 16772 +23
- Misses 7124 7134 +10
+ Partials 2183 2181 -2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ain/java/org/apache/dubbo/qos/command/impl/Ls.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL2NvbW1hbmQvaW1wbC9Mcy5qYXZh) | `96% <0%> (+11%)` | :arrow_up: |
| [.../java/org/apache/dubbo/config/ReferenceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9SZWZlcmVuY2VDb25maWcuamF2YQ==) | `56.01% <100%> (-0.38%)` | :arrow_down: |
| [...a/org/apache/dubbo/rpc/model/ApplicationModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9BcHBsaWNhdGlvbk1vZGVsLmphdmE=) | `94.11% <100%> (+1.26%)` | :arrow_up: |
| [...o/rpc/protocol/dubbo/telnet/ListTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9MaXN0VGVsbmV0SGFuZGxlci5qYXZh) | `65.38% <62.71%> (-14.17%)` | :arrow_down: |
| [...java/org/apache/dubbo/rpc/model/ConsumerModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9Db25zdW1lck1vZGVsLmphdmE=) | `62.5% <66.66%> (-1.79%)` | :arrow_down: |
| [...rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9JbnZva2VUZWxuZXRIYW5kbGVyLmphdmE=) | `70.68% <77.14%> (+0.51%)` | :arrow_up: |
| [...bbo/registry/support/ProviderConsumerRegTable.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L1Byb3ZpZGVyQ29uc3VtZXJSZWdUYWJsZS5qYXZh) | `84.44% <87.5%> (+9.44%)` | :arrow_up: |
| [...ache/dubbo/remoting/p2p/support/AbstractGroup.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctcDJwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9wMnAvc3VwcG9ydC9BYnN0cmFjdEdyb3VwLmphdmE=) | `45.45% <0%> (-11.37%)` | :arrow_down: |
| [.../apache/dubbo/qos/protocol/QosProtocolWrapper.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL3Byb3RvY29sL1Fvc1Byb3RvY29sV3JhcHBlci5qYXZh) | `64.1% <0%> (-5.13%)` | :arrow_down: |
| [...c/main/java/org/apache/dubbo/rpc/RpcException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNFeGNlcHRpb24uamF2YQ==) | `85.71% <0%> (-3.58%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=footer). Last update [859efef...14de967](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
@beiwei30 @khanimteyaz
Please let me know your thoughts on my review comments above.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Should we create a test constant for this, as we might be using it many places. It will be easier to change in case in future if have to do.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
In fact, `if (proxyObject != null)` is a defensive programming. In Dubbo, proxyObject should not be null and will not be null.
I guess we should change it to `assert` or `throw IllegelArgumentException`, would you mind take care of this in other change?
This change contains too much already, in my opinion.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] jasonjoo2010 commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "jasonjoo2010 (GitHub)" <gi...@apache.org>.
I think we should avoid unnecessary code format to show the difference clearly.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Agree with you. May be once this PR is merged I could take this cleanup part.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Can be repalce with
```
if(!StringUtils.isEmpty(service)) {
buf.append("Use default service ").append(service).append(".\r\n");
}
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Sure.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=h1) Report
> Merging [#3013](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/859efeff747587c10cae4fdbfbf38719ec44e009?src=pr&el=desc) will **increase** coverage by `0.11%`.
> The diff coverage is `69.36%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3013 +/- ##
==========================================
+ Coverage 64.28% 64.39% +0.11%
==========================================
Files 584 584
Lines 26056 26084 +28
Branches 4562 4559 -3
==========================================
+ Hits 16751 16798 +47
+ Misses 7123 7108 -15
+ Partials 2182 2178 -4
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ain/java/org/apache/dubbo/qos/command/impl/Ls.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL2NvbW1hbmQvaW1wbC9Mcy5qYXZh) | `96% <0%> (+11%)` | :arrow_up: |
| [.../java/org/apache/dubbo/config/ReferenceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9SZWZlcmVuY2VDb25maWcuamF2YQ==) | `56.01% <100%> (-0.38%)` | :arrow_down: |
| [...a/org/apache/dubbo/rpc/model/ApplicationModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9BcHBsaWNhdGlvbk1vZGVsLmphdmE=) | `94.11% <100%> (+1.26%)` | :arrow_up: |
| [...o/rpc/protocol/dubbo/telnet/ListTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9MaXN0VGVsbmV0SGFuZGxlci5qYXZh) | `66.21% <61.4%> (-13.33%)` | :arrow_down: |
| [...java/org/apache/dubbo/rpc/model/ConsumerModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9Db25zdW1lck1vZGVsLmphdmE=) | `62.5% <66.66%> (-1.79%)` | :arrow_down: |
| [...rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9JbnZva2VUZWxuZXRIYW5kbGVyLmphdmE=) | `71.79% <76.31%> (+1.61%)` | :arrow_up: |
| [...bbo/registry/support/ProviderConsumerRegTable.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L1Byb3ZpZGVyQ29uc3VtZXJSZWdUYWJsZS5qYXZh) | `84.44% <87.5%> (+9.44%)` | :arrow_up: |
| [...ache/dubbo/remoting/p2p/support/AbstractGroup.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctcDJwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9wMnAvc3VwcG9ydC9BYnN0cmFjdEdyb3VwLmphdmE=) | `45.45% <0%> (-11.37%)` | :arrow_down: |
| [...org/apache/dubbo/rpc/protocol/AbstractInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9wcm90b2NvbC9BYnN0cmFjdEludm9rZXIuamF2YQ==) | `62.9% <0%> (-3.23%)` | :arrow_down: |
| [...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tbW9uaXRvci9kdWJiby1tb25pdG9yLWRlZmF1bHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL21vbml0b3IvZHViYm8vRHViYm9Nb25pdG9yLmphdmE=) | `87.85% <0%> (-1.87%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=footer). Last update [859efef...34959fb](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=h1) Report
> Merging [#3013](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/3e6ae66fde6db2edb139e59d3c5530c9196b4144?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `70%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3013 +/- ##
==========================================
+ Coverage 64.3% 64.32% +0.01%
==========================================
Files 584 584
Lines 26056 26084 +28
Branches 4562 4559 -3
==========================================
+ Hits 16755 16778 +23
- Misses 7118 7129 +11
+ Partials 2183 2177 -6
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ain/java/org/apache/dubbo/qos/command/impl/Ls.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL2NvbW1hbmQvaW1wbC9Mcy5qYXZh) | `96% <0%> (+11%)` | :arrow_up: |
| [.../java/org/apache/dubbo/config/ReferenceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9SZWZlcmVuY2VDb25maWcuamF2YQ==) | `56.01% <100%> (ø)` | :arrow_up: |
| [...a/org/apache/dubbo/rpc/model/ApplicationModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9BcHBsaWNhdGlvbk1vZGVsLmphdmE=) | `94.11% <100%> (+1.26%)` | :arrow_up: |
| [...o/rpc/protocol/dubbo/telnet/ListTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9MaXN0VGVsbmV0SGFuZGxlci5qYXZh) | `66.21% <62.5%> (-13.33%)` | :arrow_down: |
| [...java/org/apache/dubbo/rpc/model/ConsumerModel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9tb2RlbC9Db25zdW1lck1vZGVsLmphdmE=) | `62.5% <66.66%> (-1.79%)` | :arrow_down: |
| [...rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9JbnZva2VUZWxuZXRIYW5kbGVyLmphdmE=) | `73.5% <76.31%> (+3.32%)` | :arrow_up: |
| [...bbo/registry/support/ProviderConsumerRegTable.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L1Byb3ZpZGVyQ29uc3VtZXJSZWdUYWJsZS5qYXZh) | `84.44% <87.5%> (+9.44%)` | :arrow_up: |
| [.../apache/dubbo/qos/protocol/QosProtocolWrapper.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL3Byb3RvY29sL1Fvc1Byb3RvY29sV3JhcHBlci5qYXZh) | `64.1% <0%> (-17.95%)` | :arrow_down: |
| [...e/dubbo/remoting/transport/netty/NettyChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNoYW5uZWwuamF2YQ==) | `57.64% <0%> (-4.71%)` | :arrow_down: |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `58.69% <0%> (-4.35%)` | :arrow_down: |
| ... and [12 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3013/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=footer). Last update [3e6ae66...1e98e87](https://codecov.io/gh/apache/incubator-dubbo/pull/3013?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Can be changed to
`if(!StringUtils.isEmpty(service))`
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@chickenlj would be possible to explain this in a bit more details (you might have already explained it, but for the shake of mine understanding it would great if you can again)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
This can be defined as test constant
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Yes agree that for changes we should be thought full. In this case what approach we could take to make it better. If you could guide, we would make change to enhance it?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
can be rewritten as
```
public static boolean isRegistered(String serviceUniqueName) {
Set<ProviderInvokerWrapper> providerInvokerWrapperSet = ProviderConsumerRegTable.getProviderInvoker(serviceUniqueName);
return providerInvokerWrapperSet.stream().anyMatch(ProviderInvokerWrapper::isReg);
}
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
@khanimteyaz
Hi, I tried to explain it instead of him.
In dubbo, the Provider side exposes a Proxy object instead of the actual ServiceImpl object (such as UserServiceImpl).
If we try to call UserServiceImpl directly, then we actually bypass the proxy class of UserServiceImpl (this proxy class is produced by dubbo).
Telnet is to test the effect of a method. If we bypass the entire proxy and only test UserServiceImpl, then it is meaningless. The effect of this operation is equivalent to Unit Test.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
It cannot be protected, since it's used in telnet's unit test.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Sorry my bad, it is initialize in this class while it is created. Plz ignore this comment 😄
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3013:
[DUBBO-2988] Unrecognized the other provider
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Should we refactor this to a another method something like
if (isServiceMatch(provider,service) {
-- existing code ---
}
public static boolean isServiceMatch( ProviderModel provider,String service) {
return StringUtils.isEmpty(service)
||provider.getServiceName().equalsIgnoreCase(service)
||provider.getServiceName().equalsIgnoreCase(service)
||provider.getServiceInterfaceClass().getSimpleName().equalsIgnoreCase(service)
||provider.getServiceInterfaceClass().getName().equalsIgnoreCase(service)
}
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Should we refactor this to a another method something like
```
if (isServiceMatch(provider,service) {
-- existing code ---
}
public static boolean isServiceMatch( ProviderModel provider,String service) {
return StringUtils.isEmpty(service)
||provider.getServiceName().equalsIgnoreCase(service)
||provider.getServiceInterfaceClass().getSimpleName().equalsIgnoreCase(service)
||provider.getServiceInterfaceClass().getName().equalsIgnoreCase(service)
}
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
can be rewritten as
public static boolean isRegistered(String serviceUniqueName) {
Set<ProviderInvokerWrapper> providerInvokerWrapperSet = ProviderConsumerRegTable.getProviderInvoker(serviceUniqueName);
return providerInvokerWrapperSet.stream().anyMatch(ProviderInvokerWrapper::isReg);
}
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3013: [DUBBO-2988] Unrecognized the other provider
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Better to define a constant for "Y" and "N".
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3013 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org