You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/08/01 14:28:58 UTC
[GitHub] [dubbo] plusmancn opened a new issue #8387: [3.0 Code Review] updating continuously......
plusmancn opened a new issue #8387:
URL: https://github.com/apache/dubbo/issues/8387
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn edited a comment on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn edited a comment on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895104012
### 3) Redundant method call
Furthermore, the findApi method is invoked only once in the same class, is this method really necessary?
### 4) Variables a1 and a2 will never be null, and the defaultValue of OrderInfo#order is 0.
```java
OrderInfo a1 = parseOrder(clazz1);
OrderInfo a2 = parseOrder(clazz2);
int n1 = a1 == null ? 0 : a1.order;
int n2 = a2 == null ? 0 : a2.order;
// never return 0 even if n1 equals n2, otherwise, o1 and o2 will override each other in collection like HashSet
return n1 > n2 ? 1 : -1;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn edited a comment on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn edited a comment on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895104012
### 3) Redundant method call
Furthermore, the findApi method is invoked only once in the same class, is this method really necessary?
### 4) a1 and a2 will never be null, and the defaultValue of OrderInfo#order is 0.
```java
OrderInfo a1 = parseOrder(clazz1);
OrderInfo a2 = parseOrder(clazz2);
int n1 = a1 == null ? 0 : a1.order;
int n2 = a2 == null ? 0 : a2.order;
// never return 0 even if n1 equals n2, otherwise, o1 and o2 will override each other in collection like HashSet
return n1 > n2 ? 1 : -1;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn commented on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn commented on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895139370
### 5) The cachedWrapperClasses may conflict with initExtension method.
```java
// => org.apache.dubbo.common.extension.ExtensionLoader#createExtension
private T createExtension(String name, boolean wrap) {
// ......
if (wrap) {
List<Class<?>> wrapperClassesList = new ArrayList<>();
if (cachedWrapperClasses != null) {
wrapperClassesList.addAll(cachedWrapperClasses);
// 对包装类进行排序，依赖 @Activate 注解的 order 值，如果缺失则为 0
// 值越大，在洋葱模型的越内层.
wrapperClassesList.sort(WrapperComparator.COMPARATOR);
Collections.reverse(wrapperClassesList);
}
if (CollectionUtils.isNotEmpty(wrapperClassesList)) {
for (Class<?> wrapperClass : wrapperClassesList) {
// @Wrapper 注解表明，使用该包装器需要什么条件
Wrapper wrapper = wrapperClass.getAnnotation(Wrapper.class);
if (wrapper == null
|| (ArrayUtils.contains(wrapper.matches(), name) && !ArrayUtils.contains(wrapper.mismatches(), name))) {
// (T) 向上转型为原始接口类，例如 Protocol
// 并对包装类再次执行依赖注入
instance = injectExtension((T) wrapperClass.getConstructor(type).newInstance(instance));
}
}
}
}
// 现在我们再看 initExtension 方法，当实例被包装过后，有可能不再是 Lifecycle 实例
// 从严谨的角度来看，职责链和此处的 Hook 方法是冲突的，容易给用户造成不必要的困惑。
initExtension(instance);
// ......
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn commented on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn commented on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895104012
### Redundant method call
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn edited a comment on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn edited a comment on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895104012
### 3) Redundant method call
Furthermore, the findApi method is invoked only once in the same class, is this method really necessary?
### 4)
a1 and a2 will never be null, and the defaultValue of OrderInfo#order is 0.
```java
OrderInfo a1 = parseOrder(clazz1);
OrderInfo a2 = parseOrder(clazz2);
int n1 = a1 == null ? 0 : a1.order;
int n2 = a2 == null ? 0 : a2.order;
// never return 0 even if n1 equals n2, otherwise, o1 and o2 will override each other in collection like HashSet
return n1 > n2 ? 1 : -1;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn removed a comment on issue #8387: [3.0 Code Review] Service export & refer
Posted by GitBox <gi...@apache.org>.
plusmancn removed a comment on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-896107393
### 6) DEFAULT_DUBBO_PROTOCOL_VERSION mismatches the limitation of version which should be between 2.0.10 ~ 2.6.2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn edited a comment on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn edited a comment on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895139370
### 5) The cachedWrapperClasses may conflict with initExtension method.
```java
// => org.apache.dubbo.common.extension.ExtensionLoader#createExtension
private T createExtension(String name, boolean wrap) {
// ......
if (wrap) {
List<Class<?>> wrapperClassesList = new ArrayList<>();
if (cachedWrapperClasses != null) {
wrapperClassesList.addAll(cachedWrapperClasses);
// 对包装类进行排序，依赖 @Activate 注解的 order 值，如果缺失则为 0
// 值越大，流程越靠前
wrapperClassesList.sort(WrapperComparator.COMPARATOR);
Collections.reverse(wrapperClassesList);
}
if (CollectionUtils.isNotEmpty(wrapperClassesList)) {
for (Class<?> wrapperClass : wrapperClassesList) {
// @Wrapper 注解表明，使用该包装器需要什么条件
Wrapper wrapper = wrapperClass.getAnnotation(Wrapper.class);
if (wrapper == null
|| (ArrayUtils.contains(wrapper.matches(), name) && !ArrayUtils.contains(wrapper.mismatches(), name))) {
// (T) 向上转型为原始接口类，例如 Protocol
// 并对包装类再次执行依赖注入
instance = injectExtension((T) wrapperClass.getConstructor(type).newInstance(instance));
}
}
}
}
// 现在我们再看 initExtension 方法，当实例被包装过后，有可能不再是 Lifecycle 实例
// 从严谨的角度来看，职责链和此处的 Hook 方法是冲突的，容易给用户造成不必要的困惑。
initExtension(instance);
// ......
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn edited a comment on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn edited a comment on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895104012
### 3) Redundant method call
Furthermore, the findApi method is invoked only once in the same class, is this method really necessary?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] changfubai commented on issue #8387: [3.0 Code Review] Service export & refer
Posted by GitBox <gi...@apache.org>.
changfubai commented on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-907072687
some bad smell also exist on master branch, i'll correct it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ closed issue #8387: [3.0 Code Review] Service export & refer
Posted by GitBox <gi...@apache.org>.
AlbumenJ closed issue #8387:
URL: https://github.com/apache/dubbo/issues/8387
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn commented on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn commented on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-896107393
### 6) DEFAULT_DUBBO_PROTOCOL_VERSION mismatches the limitation of version which should be between 2.0.10 ~ 2.6.2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] plusmancn edited a comment on issue #8387: [3.0 Code Review] updating continuously......
Posted by GitBox <gi...@apache.org>.
plusmancn edited a comment on issue #8387:
URL: https://github.com/apache/dubbo/issues/8387#issuecomment-895104012
### 3) Redundant method call
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org