Merge pull request #37 from Tinder/issue_34

Fix digest reset issues when using rule inputs

Author: tinder-maxwellelliott

integration/integration_test.sh

@@ -5,7 +5,7 @@ bazel_path=$(which bazelisk)
 
 previous_revision="HEAD^"
 final_revision="HEAD"
-output_dir=$(mktemp -d)
+output_dir="/tmp"
 modified_filepaths_output="$PWD/integration/modified_filepaths.txt"
 starting_hashes_json="$output_dir/starting_hashes.json"
 final_hashes_json="$output_dir/final_hashes_json.json"

src/main/java/com/bazel_diff/TargetHashingClient.java

@@ -24,7 +24,7 @@ public Map<String, String> hashAllBazelTargets(Set<Path> modifiedFilepaths) thro
         Set<BazelSourceFileTarget> bazelSourcefileTargets = bazelClient.convertFilepathsToSourceTargets(modifiedFilepaths);
         List<BazelTarget> allTargets = bazelClient.queryAllTargets();
         Map<String, String> targetHashes = new HashMap<>();
-        Map<String, MessageDigest> ruleHashes = new HashMap<>();
+        Map<String, byte[]> ruleHashes = new HashMap<>();
         Map<String, BazelRule> allRulesMap = new HashMap<>();
         for (BazelTarget target : allTargets) {
             String targetName = getNameForTarget(target);
@@ -38,14 +38,14 @@ public Map<String, String> hashAllBazelTargets(Set<Path> modifiedFilepaths) thro
             if (targetName == null) {
                 continue;
             }
-            MessageDigest targetDigest = createDigestForTarget(
+            byte[] targetDigest = createDigestForTarget(
                     target,
                     allRulesMap,
                     bazelSourcefileTargets,
                     ruleHashes
             );
             if (targetDigest != null) {
-                targetHashes.put(targetName, digestToString(targetDigest));
+                targetHashes.put(targetName, convertByteArrayToString(targetDigest));
             }
         }
         return targetHashes;
@@ -67,11 +67,11 @@ public Set<String> getImpactedTargets(
         return bazelClient.queryForImpactedTargets(impactedTargets, avoidQuery);
     }
 
-    private MessageDigest createDigestForTarget(
+    private byte[] createDigestForTarget(
             BazelTarget target,
             Map<String, BazelRule> allRulesMap,
             Set<BazelSourceFileTarget> bazelSourcefileTargets,
-            Map<String, MessageDigest> ruleHashes
+            Map<String, byte[]> ruleHashes
     ) throws NoSuchAlgorithmException {
         BazelRule targetRule = target.getRule();
         if (target.hasSourceFile()) {
@@ -80,23 +80,23 @@ private MessageDigest createDigestForTarget(
                 MessageDigest digest = MessageDigest.getInstance("SHA-256");
                 byte[] sourceTargetDigestBytes = getDigestForSourceTargetName(sourceFileName, bazelSourcefileTargets);
                 if (sourceTargetDigestBytes != null) {
-                    digest.update(getDigestForSourceTargetName(sourceFileName, bazelSourcefileTargets));
+                    digest.update(sourceTargetDigestBytes);
                 }
-                return digest;
+                return digest.digest().clone();
             }
         }
-        return createHashForRule(targetRule, allRulesMap, ruleHashes, bazelSourcefileTargets);
+        return createDigestForRule(targetRule, allRulesMap, ruleHashes, bazelSourcefileTargets);
     }
 
-    private MessageDigest createHashForRule(
+    private byte[] createDigestForRule(
             BazelRule rule,
             Map<String, BazelRule> allRulesMap,
-            Map<String, MessageDigest> ruleHashes,
+            Map<String, byte[]> ruleHashes,
             Set<BazelSourceFileTarget> bazelSourcefileTargets
     ) throws NoSuchAlgorithmException {
-        MessageDigest existingMessage = ruleHashes.get(rule.getName());
-        if (existingMessage != null) {
-            return existingMessage;
+        byte[] existingByteArray = ruleHashes.get(rule.getName());
+        if (existingByteArray != null) {
+            return existingByteArray;
         }
         MessageDigest digest = MessageDigest.getInstance("SHA-256");
         digest.update(rule.getDigest());
@@ -105,21 +105,22 @@ private MessageDigest createHashForRule(
             BazelRule inputRule = allRulesMap.get(ruleInput);
             byte[] sourceFileDigest = getDigestForSourceTargetName(ruleInput, bazelSourcefileTargets);
             if (inputRule != null) {
-                MessageDigest ruleInputDigest = createHashForRule(
+                byte[] ruleInputHash = createDigestForRule(
                         inputRule,
                         allRulesMap,
                         ruleHashes,
                         bazelSourcefileTargets
                 );
-                if (ruleInputDigest != null) {
-                    digest.update(ruleInputDigest.digest());
+                if (ruleInputHash != null) {
+                    digest.update(ruleInputHash);
                 }
             } else if (sourceFileDigest != null) {
                 digest.update(sourceFileDigest);
             }
         }
-        ruleHashes.put(rule.getName(), digest);
-        return digest;
+        byte[] finalHashValue = digest.digest().clone();
+        ruleHashes.put(rule.getName(), finalHashValue);
+        return finalHashValue;
     }
 
     private byte[] getDigestForSourceTargetName(
@@ -134,9 +135,9 @@ private byte[] getDigestForSourceTargetName(
         return null;
     }
 
-    private String digestToString(MessageDigest digest) {
+    private String convertByteArrayToString(byte[] bytes) {
         StringBuilder result = new StringBuilder();
-        for (byte aByte : digest.digest()) {
+        for (byte aByte : bytes) {
             result.append(String.format("%02x", aByte));
         }
         return result.toString();

test/java/com/bazel_diff/TargetHashingClientImplTests.java

@@ -64,14 +64,17 @@ public void hashAllBazelTargets_ruleTargets_ruleInputs() throws IOException, NoS
         ruleInputs.add("rule1");
         BazelTarget rule3 = createRuleTarget("rule3", ruleInputs, "digest");
         defaultTargets.add(rule3);
+        BazelTarget rule4 = createRuleTarget("rule4", ruleInputs, "digest2");
+        defaultTargets.add(rule4);
         when(bazelClientMock.queryAllTargets()).thenReturn(defaultTargets);
         TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock);
         try {
             Map<String, String> hash = client.hashAllBazelTargets(new HashSet<>());
-            assertEquals(3, hash.size());
+            assertEquals(4, hash.size());
             assertEquals("2c963f7c06bc1cead7e3b4759e1472383d4469fc3238dc42f8848190887b4775", hash.get("rule1"));
             assertEquals("bdc1abd0a07103cea34199a9c0d1020619136ff90fb88dcc3a8f873c811c1fe9", hash.get("rule2"));
-            assertEquals("0f34d9fff9902c8e5915d82ded7a42f714c80bc55dfc1558f743fe01c5821923", hash.get("rule3"));
+            assertEquals("87dd050f1ca0f684f37970092ff6a02677d995718b5a05461706c0f41ffd4915", hash.get("rule3"));
+            assertEquals("a7bc5d23cd98c4942dc879c649eb9646e38eddd773f9c7996fa0d96048cf63dc", hash.get("rule4"));
         } catch (IOException | NoSuchAlgorithmException e) {
             fail(e.getMessage());
         }