You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/04/30 06:45:43 UTC
[GitHub] [rocketmq-mqtt] YxAc opened a new pull request, #89: [ISSUE #22] Improving codeCov for mqtt.common
YxAc opened a new pull request, #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89
codeCov for mqtt.common to improving #22
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] tianliuliu commented on pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
tianliuliu commented on PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#issuecomment-1118117579
> Wonder under what conditions the codecov-commenter will be triggered？ @tianliuliu , thanks :-)
it can be triggered always, sometimes it dosen't work, maybe the bug itself.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] YxAc commented on a diff in pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
YxAc commented on code in PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#discussion_r862317870
##########
mqtt-common/src/main/java/org/apache/rocketmq/mqtt/common/util/StatUtil.java:
##########
@@ -227,40 +227,7 @@ public static void addInvoke(String key, int num, long rt, boolean success) {
}
public static void addInvoke(String key, long rt, boolean success) {
- if (invokeCache.size() > MAX_KEY_NUM || secondInvokeCache.size() > MAX_KEY_NUM) {
- return;
- }
- Invoke invoke = getAndSetInvoke(key);
- if (invoke == null) {
- return;
- }
-
- invoke.totalPv.getAndIncrement();
- if (!success) {
- invoke.failPv.getAndIncrement();
- }
- long now = nowSecond();
- AtomicLong oldSecond = invoke.second;
- if (oldSecond.get() == now) {
- invoke.secondPv.getAndIncrement();
- } else {
- if (oldSecond.compareAndSet(oldSecond.get(), now)) {
- if (invoke.secondPv.get() > invoke.topSecondPv.get()) {
- invoke.topSecondPv.set(invoke.secondPv.get());
- }
- invoke.secondPv.set(1);
- } else {
- invoke.secondPv.getAndIncrement();
- }
- }
-
- invoke.sumRt.addAndGet(rt);
- if (invoke.maxRt.get() < rt) {
- invoke.maxRt.set(rt);
- }
- if (invoke.minRt.get() > rt) {
- invoke.minRt.set(rt);
- }
+ addInvoke(key, 1, rt, success);
Review Comment:
reusing 'addInvoke' method
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] YxAc commented on a diff in pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
YxAc commented on code in PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#discussion_r865833649
##########
mqtt-common/src/main/java/org/apache/rocketmq/mqtt/common/util/TopicUtils.java:
##########
@@ -31,7 +31,7 @@ public class TopicUtils {
* @return
*/
public static String normalizeTopic(String topic) {
- if (topic == null) {
+ if (topic == null || topic.isEmpty()) {
Review Comment:
> may be use StringUtils.isBlank(topic) will be better
done
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] tianliuliu commented on a diff in pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
tianliuliu commented on code in PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#discussion_r865525539
##########
mqtt-common/src/main/java/org/apache/rocketmq/mqtt/common/util/TopicUtils.java:
##########
@@ -31,7 +31,7 @@ public class TopicUtils {
* @return
*/
public static String normalizeTopic(String topic) {
- if (topic == null) {
+ if (topic == null || topic.isEmpty()) {
Review Comment:
may be use StringUtils.isBlank(topic) will be better
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] tianliuliu merged pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
tianliuliu merged PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] YxAc commented on a diff in pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
YxAc commented on code in PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#discussion_r862317935
##########
mqtt-common/src/main/java/org/apache/rocketmq/mqtt/common/util/TopicUtils.java:
##########
@@ -31,7 +31,7 @@ public class TopicUtils {
* @return
*/
public static String normalizeTopic(String topic) {
- if (topic == null) {
+ if (topic == null || topic.isEmpty()) {
Review Comment:
bug of 'empty str'
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] YxAc commented on pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
YxAc commented on PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#issuecomment-1114206634
Wonder under what conditions the codecov-commenter will be triggered？ @tianliuliu , thanks :-)
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] YxAc commented on a diff in pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
YxAc commented on code in PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#discussion_r862317815
##########
mqtt-common/src/main/java/org/apache/rocketmq/mqtt/common/util/NamespaceUtil.java:
##########
@@ -24,45 +24,35 @@ public class NamespaceUtil {
private static final int RESOURCE_LENGTH = 2;
public static final String MQ_DEFAULT_NAMESPACE_NAME = "DEFAULT_INSTANCE";
- public NamespaceUtil() {
- }
-
public static String encodeToNamespaceResource(String namespace, String resource) {
- return resource != null && namespace != null ? StringUtils.join(new String[]{namespace, "%", resource}) : resource;
+ return resource != null && namespace != null ? StringUtils.join(namespace, NAMESPACE_SPLITER, resource) : resource;
}
public static String decodeOriginResource(String resource) {
- if (resource != null && resource.contains("%")) {
- int firstIndex = resource.indexOf("%");
+ if (resource != null && resource.contains(NAMESPACE_SPLITER)) {
+ int firstIndex = resource.indexOf(NAMESPACE_SPLITER);
return resource.substring(firstIndex + 1);
- } else {
- return resource;
}
+ return resource;
}
public static String decodeMqttNamespaceIdFromKey(String key) {
return decodeMqttNamespaceIdFromClientId(key);
}
public static String decodeMqttNamespaceIdFromClientId(String clientId) {
- if (clientId != null && clientId.contains("%")) {
- String mqttNamespaceId = clientId.split("%")[0];
- return mqttNamespaceId;
- } else {
- return null;
- }
+ return splitNamespaceStr(clientId);
}
public static String decodeStoreNamespaceIdFromTopic(String topic) {
- if (topic != null && topic.contains("%")) {
- String storeNamespaceId = topic.split("%")[0];
- return storeNamespaceId;
- } else {
- return null;
- }
+ return splitNamespaceStr(topic);
Review Comment:
encapsulate duplicate code block
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] YxAc commented on a diff in pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
YxAc commented on code in PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#discussion_r862317524
##########
mqtt-common/src/main/java/org/apache/rocketmq/mqtt/common/util/NamespaceUtil.java:
##########
@@ -24,45 +24,35 @@ public class NamespaceUtil {
private static final int RESOURCE_LENGTH = 2;
public static final String MQ_DEFAULT_NAMESPACE_NAME = "DEFAULT_INSTANCE";
- public NamespaceUtil() {
- }
-
public static String encodeToNamespaceResource(String namespace, String resource) {
- return resource != null && namespace != null ? StringUtils.join(new String[]{namespace, "%", resource}) : resource;
+ return resource != null && namespace != null ? StringUtils.join(namespace, NAMESPACE_SPLITER, resource) : resource;
}
public static String decodeOriginResource(String resource) {
- if (resource != null && resource.contains("%")) {
- int firstIndex = resource.indexOf("%");
+ if (resource != null && resource.contains(NAMESPACE_SPLITER)) {
+ int firstIndex = resource.indexOf(NAMESPACE_SPLITER);
Review Comment:
using defined constant
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-mqtt] YxAc commented on a diff in pull request #89: [ISSUE #22] Improving codeCov for mqtt.common
Posted by GitBox <gi...@apache.org>.
YxAc commented on code in PR #89:
URL: https://github.com/apache/rocketmq-mqtt/pull/89#discussion_r862318014
##########
mqtt-common/src/main/java/org/apache/rocketmq/mqtt/common/util/TopicUtils.java:
##########
@@ -93,10 +93,10 @@ public static boolean isP2pTopic(String topic) {
}
public static String getP2Peer(MqttTopic mqttTopic, String namespace) {
- if (!isP2P(mqttTopic.getSecondTopic())) {
+ if (mqttTopic.getSecondTopic() == null || mqttTopic.getFirstTopic() == null) {
return null;
}
- if (mqttTopic.getSecondTopic() == null || mqttTopic.getFirstTopic() == null) {
+ if (!isP2P(mqttTopic.getSecondTopic())) {
Review Comment:
reserve the order of 'null judgment' and 'p2p judgment'
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org