Update diff only if changed / Rename createDiff to createOrUpdateDiff

common/repo/tests/GitRepoTest.java

@@ -174,7 +174,6 @@ public void testAddFile() {
     fileUtils.writeStringUnchecked(
         TEST_FILE_CONTENTS, fileUtils.joinToAbsolutePath(repoFolder, TEST_FILE));
     gitRepo.addFile(TEST_FILE);
-    List<File> uncom = repo.getUncommittedFiles();
     assertEquals(1, repo.getUncommittedFiles().size());
   }
 

tools/reviewer/aa/commands/DiffCommand.java

@@ -33,6 +33,7 @@
 import com.google.startupos.tools.reviewer.local_server.service.Protos.Reviewer;
 import io.grpc.ManagedChannel;
 import io.grpc.ManagedChannelBuilder;
+import java.io.IOException;
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -116,11 +117,7 @@ private Diff createDiff() {
 
     Map<GitRepo, String> repoToInitialBranch = new HashMap<>();
     try {
-      fileUtils
-          .listContents(workspacePath)
-          .stream()
-          .map(path -> fileUtils.joinToAbsolutePath(workspacePath, path))
-          .filter(fileUtils::folderExists)
+      getAbsRepoPaths()
           .forEach(
               path -> {
                 String repoName = Paths.get(path).getFileName().toString();
@@ -177,48 +174,68 @@ public boolean run(String[] args) {
 
     Diff diff = (diffNumber == -1) ? createDiff() : updateDiff(diffNumber);
     CreateDiffRequest request = CreateDiffRequest.newBuilder().setDiff(diff).build();
-    // TODO: Check if we changed diff, update only if changed. Write to output if no change.
-    // TODO: Rename createDiff to createOrUpdateDiff
-    codeReviewBlockingStub.createDiff(request);
+    if (isChangesExist("D" + diff.getId())) {
+      codeReviewBlockingStub.createOrUpdateDiff(request);
+    } else {
+      System.out.println(String.format("%s diff has no changes. Nothing to update.", diff.getId()));
+    }
     return true;
   }
 
+  private boolean isChangesExist(String branchName) {
+    for (String path : getAbsRepoPaths()) {
+      GitRepo repo = this.gitRepoFactory.create(path);
+      if (repo.hasChanges(branchName)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   private void addGithubRepos(Diff.Builder diffBuilder) {
     List<String> existingGithubRepoNames =
         diffBuilder.getGithubPrList().stream().map(GithubPr::getRepo).collect(Collectors.toList());
-    try {
-      fileUtils
-          .listContents(workspacePath)
-          .stream()
-          .map(path -> fileUtils.joinToAbsolutePath(workspacePath, path))
-          .filter(fileUtils::folderExists)
-          .forEach(
-              path -> {
-                GitRepo repo = this.gitRepoFactory.create(path);
-                if (repo.hasChanges(repo.currentBranch())) {
-                  // Example of repoUrl: https://github.com/google/startup-os.git
-                  String repoUrl = repo.getRemoteUrl();
-                  String repoOwner = repoUrl.split("/")[3];
-                  String repoName = repoUrl.split("/")[4].replace(".git", "").trim();
-
-                  String folderName = Paths.get(path).getFileName().toString();
-                  if (!repoName.equals(folderName)) {
-                    System.out.println(
-                        String.format(
-                            "Repository name from the URL(%s) and folder "
-                                + "name from workspace(%s) aren't the same.",
-                            repoName, folderName));
-                  }
-
-                  if (!existingGithubRepoNames.contains(repoName)) {
-                    diffBuilder.addGithubPr(
-                        GithubPr.newBuilder().setRepo(repoName).setOwner(repoOwner).build());
-                  }
+    getAbsRepoPaths()
+        .forEach(
+            path -> {
+              GitRepo repo = this.gitRepoFactory.create(path);
+              if (repo.hasChanges(repo.currentBranch())) {
+                // Example of repoUrl: https://github.com/google/startup-os.git
+                String repoUrl = repo.getRemoteUrl();
+                String repoOwner = repoUrl.split("/")[3];
+                String repoName = repoUrl.split("/")[4].replace(".git", "").trim();
+
+                String folderName = Paths.get(path).getFileName().toString();
+                if (!repoName.equals(folderName)) {
+                  System.out.println(
+                      String.format(
+                          "Repository name from the URL(%s) and folder "
+                              + "name from workspace(%s) aren't the same.",
+                          repoName, folderName));
                 }
-              });
-    } catch (Exception e) {
+
+                if (!existingGithubRepoNames.contains(repoName)) {
+                  diffBuilder.addGithubPr(
+                      GithubPr.newBuilder().setRepo(repoName).setOwner(repoOwner).build());
+                }
+              }
+            });
+  }
+
+  private ImmutableList<String> getAbsRepoPaths() {
+    ImmutableList.Builder<String> result = ImmutableList.builder();
+    try {
+      result.addAll(
+          fileUtils
+              .listContents(workspacePath)
+              .stream()
+              .map(path -> fileUtils.joinToAbsolutePath(workspacePath, path))
+              .filter(fileUtils::folderExists)
+              .collect(Collectors.toList()));
+    } catch (IOException e) {
       e.printStackTrace();
     }
+    return result.build();
   }
 }
 

tools/reviewer/aa/commands/ReviewCommand.java

@@ -102,7 +102,7 @@ public boolean run(String[] args) {
       e.printStackTrace();
     }
 
-    codeReviewBlockingStub.createDiff(
+    codeReviewBlockingStub.createOrUpdateDiff(
         CreateDiffRequest.newBuilder().setDiff(diffBuilder.build()).build());
 
     return true;

tools/reviewer/aa/commands/SubmitCommand.java

@@ -81,7 +81,7 @@ public boolean run(String[] args) {
     }
 
     System.out.println("Updating diff status: SUBMITTING");
-    codeReviewBlockingStub.createDiff(
+    codeReviewBlockingStub.createOrUpdateDiff(
         CreateDiffRequest.newBuilder().setDiff(diffBuilder.setStatus(Status.SUBMITTING)).build());
 
     final String diffBranchName = String.format("D%s", diffBuilder.getId());
@@ -122,7 +122,7 @@ public boolean run(String[] args) {
     }
 
     System.out.println("Updating diff status: SUBMITTED");
-    codeReviewBlockingStub.createDiff(
+    codeReviewBlockingStub.createOrUpdateDiff(
         CreateDiffRequest.newBuilder().setDiff(diffBuilder.setStatus(Status.SUBMITTED)).build());
     return true;
   }

tools/reviewer/job/sync/ReviewerClient.java

@@ -216,7 +216,7 @@ private static int getCommentIndex(List<Comment> comments, String id) {
   }
 
   private void updateDiff(Diff diff) {
-    codeReviewBlockingStub.createDiff(CreateDiffRequest.newBuilder().setDiff(diff).build());
+    codeReviewBlockingStub.createOrUpdateDiff(CreateDiffRequest.newBuilder().setDiff(diff).build());
   }
 
   private Comment.Builder getCommentBuilder(

tools/reviewer/local_server/service/CodeReviewService.java

@@ -178,7 +178,7 @@ public void getFile(FileRequest req, StreamObserver<FileResponse> responseObserv
   }
 
   @Override
-  public void createDiff(CreateDiffRequest req, StreamObserver<Empty> responseObserver) {
+  public void createOrUpdateDiff(CreateDiffRequest req, StreamObserver<Empty> responseObserver) {
     checkAuth();
     FirestoreProtoClient client =
         new FirestoreProtoClient(authService.getProjectId(), authService.getToken());

tools/reviewer/local_server/service/code_review.proto

@@ -12,7 +12,7 @@ import "tools/reviewer/reviewer.proto";
 service CodeReviewService {
   // Get a text file's contents
   rpc getFile(FileRequest) returns (FileResponse);
-  rpc createDiff(CreateDiffRequest) returns (Empty);
+  rpc createOrUpdateDiff(CreateDiffRequest) returns (Empty);
   rpc getTextDiff(TextDiffRequest) returns (TextDiffResponse);
   // returns available diff number and increments stored value by one
   rpc getAvailableDiffNumber(Empty) returns (DiffNumberResponse);

tools/reviewer/local_server/service/tests/TestTool.java

@@ -77,7 +77,7 @@ private String getFile(String name) {
   private void createDiff(Diff diff) {
     final CreateDiffRequest request = CreateDiffRequest.newBuilder().setDiff(diff).build();
     try {
-      blockingStub.createDiff(request);
+      blockingStub.createOrUpdateDiff(request);
     } catch (StatusRuntimeException e) {
       e.printStackTrace();
     }