Copilot commented on code in PR #15633:
URL: https://github.com/apache/dubbo/pull/15633#discussion_r2278143954
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/DefaultServiceInstance.java:
##########
@@ -305,21 +308,43 @@ public boolean equals(Object o) {
return false;
}
DefaultServiceInstance that = (DefaultServiceInstance) o;
- boolean equals = Objects.equals(getServiceName(),
that.getServiceName())
- && Objects.equals(getHost(), that.getHost())
- && Objects.equals(getPort(), that.getPort());
+ if (!Objects.equals(getServiceName(), that.getServiceName())
+ || !Objects.equals(getHost(), that.getHost())
+ || !Objects.equals(getPort(), that.getPort())) {
+ return false;
+ }
+
+ Set<String> checkedKeys = new HashSet<>(this.getMetadata().size());
for (Map.Entry<String, String> entry : this.getMetadata().entrySet()) {
- if
(entry.getKey().equals(EXPORTED_SERVICES_REVISION_PROPERTY_NAME)) {
+ String key = entry.getKey();
+ checkedKeys.add(key);
+ if (key.equals(EXPORTED_SERVICES_REVISION_PROPERTY_NAME) ||
key.equals(TIMESTAMP_KEY)) {
continue;
}
if (entry.getValue() == null) {
- equals = equals && (entry.getValue() ==
that.getMetadata().get(entry.getKey()));
+ if (null != that.getMetadata().get(key)) {
+ return false;
+ }
} else {
- equals = equals &&
entry.getValue().equals(that.getMetadata().get(entry.getKey()));
+ if (!entry.getValue().equals(that.getMetadata().get(key))) {
+ return false;
+ }
+ }
+ }
+
+ for (Map.Entry<String, String> thatEntry :
that.getMetadata().entrySet()) {
+ String thatKey = thatEntry.getKey();
+ if (checkedKeys.contains(thatKey)
+ || thatKey.equals(EXPORTED_SERVICES_REVISION_PROPERTY_NAME)
+ || thatKey.equals(TIMESTAMP_KEY)) {
+ continue;
+ }
+ if (null != thatEntry.getValue()) {
Review Comment:
The logic appears incorrect. This condition returns false when the other
instance has a non-null value for a key that wasn't checked, but it should also
check if this instance has a null value for the same key. The comparison should
be symmetric.
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/DefaultServiceInstance.java:
##########
@@ -305,21 +308,43 @@ public boolean equals(Object o) {
return false;
}
DefaultServiceInstance that = (DefaultServiceInstance) o;
- boolean equals = Objects.equals(getServiceName(),
that.getServiceName())
- && Objects.equals(getHost(), that.getHost())
- && Objects.equals(getPort(), that.getPort());
+ if (!Objects.equals(getServiceName(), that.getServiceName())
+ || !Objects.equals(getHost(), that.getHost())
+ || !Objects.equals(getPort(), that.getPort())) {
+ return false;
+ }
+
+ Set<String> checkedKeys = new HashSet<>(this.getMetadata().size());
Review Comment:
The HashSet is initialized with the size of metadata, but only keys that are
not skipped will be added. Consider initializing without a specific size or
using a more accurate estimate to avoid potential memory waste.
```suggestion
Set<String> checkedKeys = new HashSet<>();
```
--
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-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
