Remote: correctly implement equals and hashCode.

Googler · copybara-github · commit 5b786da75837 · 2021-01-19T01:21:44.000-08:00
PiperOrigin-RevId: 352516263
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
@@ -577,12 +577,15 @@ public boolean equals(Object o) {
       RemoteFileArtifactValue that = (RemoteFileArtifactValue) o;
       return Arrays.equals(digest, that.digest)
           && size == that.size
-          && locationIndex == that.locationIndex;
+          && locationIndex == that.locationIndex
+          && Objects.equals(actionId, that.actionId)
+          && dataIsShareable() == that.dataIsShareable();
     }
 
     @Override
     public int hashCode() {
-      return Objects.hash(Arrays.hashCode(digest), size, locationIndex, dataIsShareable());
+      return Objects.hash(
+          Arrays.hashCode(digest), size, locationIndex, actionId, dataIsShareable());
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionFileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionFileArtifactValue.java
@@ -14,6 +14,8 @@
 package com.google.devtools.build.lib.remote.common;
 
 import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
+import java.util.Arrays;
+import java.util.Objects;
 
 /**
  * A {@link RemoteFileArtifactValue} with additional data only available when using Remote Execution
@@ -32,4 +34,30 @@ public RemoteActionFileArtifactValue(
   public boolean isExecutable() {
     return isExecutable;
   }
+
+  @Override
+  public boolean equals(Object o) {
+    if (!(o instanceof RemoteActionFileArtifactValue)) {
+      return false;
+    }
+
+    RemoteActionFileArtifactValue that = (RemoteActionFileArtifactValue) o;
+    return Arrays.equals(getDigest(), that.getDigest())
+        && getSize() == that.getSize()
+        && getLocationIndex() == that.getLocationIndex()
+        && Objects.equals(getActionId(), that.getActionId())
+        && isExecutable == that.isExecutable
+        && dataIsShareable() == that.dataIsShareable();
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(
+        Arrays.hashCode(getDigest()),
+        getSize(),
+        getLocationIndex(),
+        getActionId(),
+        isExecutable,
+        dataIsShareable());
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java
@@ -108,7 +108,7 @@ public boolean isExecutable() {
 
     @Override
     public int hashCode() {
-      return Objects.hash(super.hashCode(), path, data, digest);
+      return Objects.hash(super.hashCode(), path, data, digest, isExecutable);
     }
 
     @Override
@@ -118,7 +118,8 @@ public boolean equals(Object o) {
         return super.equals(other)
             && Objects.equals(path, other.path)
             && Objects.equals(data, other.data)
-            && Objects.equals(digest, other.digest);
+            && Objects.equals(digest, other.digest)
+            && isExecutable == other.isExecutable;
       }
       return false;
     }

Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ public boolean isExecutable() {`
`108`	`108`
`109`	`109`	`@Override`
`110`	`110`	`public int hashCode() {`
`111`		`- return Objects.hash(super.hashCode(), path, data, digest);`
	`111`	`+ return Objects.hash(super.hashCode(), path, data, digest, isExecutable);`
`112`	`112`	`}`
`113`	`113`
`114`	`114`	`@Override`
`@@ -118,7 +118,8 @@ public boolean equals(Object o) {`
`118`	`118`	`return super.equals(other)`
`119`	`119`	`&& Objects.equals(path, other.path)`
`120`	`120`	`&& Objects.equals(data, other.data)`
`121`		`- && Objects.equals(digest, other.digest);`
	`121`	`+ && Objects.equals(digest, other.digest)`
	`122`	`+ && isExecutable == other.isExecutable;`
`122`	`123`	`}`
`123`	`124`	`return false;`
`124`	`125`	`}`