keith-turner commented on code in PR #77:
URL:
https://github.com/apache/accumulo-classloaders/pull/77#discussion_r2835567166
##########
modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/manifest/ManifestTest.java:
##########
@@ -55,8 +60,13 @@ private static String mockJson(boolean withComment, boolean
withMonitorInterval,
if (i > 0) {
json.append(COMMA);
}
- json.append("{'location': 'file:/home/user/ClassLoaderTestA/" + i +
".jar'").append(COMMA);
- json.append("'algorithm': 'MOCK',").append("'checksum': '" + i + "'}");
+ var n = i;
+ if (withResourceCount == 10) {
Review Comment:
When reading the new test `testDeserializationWithDeduplication()` it was
confusing at first how it was duplicating. Then saw that 10 was a special
value in this code.
##########
modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Manifest.java:
##########
@@ -121,23 +120,15 @@ public Set<Resource> getResources() {
@Override
public int hashCode() {
- return hash(monitorIntervalSeconds, resources);
+ return toJson().hashCode();
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
- if (obj == null) {
- return false;
- }
- if (getClass() != obj.getClass()) {
- return false;
- }
- Manifest other = (Manifest) obj;
- return monitorIntervalSeconds == other.monitorIntervalSeconds
- && Objects.equals(resources, other.resources);
+ return obj instanceof Manifest ? Objects.equals(toJson(), ((Manifest)
obj).toJson()) : false;
Review Comment:
Its not a problem to call Object.equals here, but does not seem needed since
the json values will not be null.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
