Merge "Revert "Reject inline comments on files that do not exist in the patch set""

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -20,7 +20,6 @@ import com.google.common.base.Objects;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
 import com.google.gerrit.common.ChangeHooks;
@@ -52,7 +51,6 @@ import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.index.ChangeIndexer;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.util.LabelVote;
 import com.google.gerrit.server.util.TimeUtil;
 import com.google.gwtorm.server.OrmException;
@@ -68,7 +66,6 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 public class PostReview implements RestModifyView<RevisionResource, ReviewInput> {
   private static final Logger log = LoggerFactory.getLogger(PostReview.class);
@@ -79,7 +76,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
 
   private final Provider<ReviewDb> db;
   private final ChangesCollection changes;
-  private final ChangeData.Factory changeDataFactory;
   private final ChangeUpdate.Factory updateFactory;
   private final ApprovalsUtil approvalsUtil;
   private final ChangeIndexer indexer;
@@ -97,7 +93,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
   @Inject
   PostReview(Provider<ReviewDb> db,
       ChangesCollection changes,
-      ChangeData.Factory changeDataFactory,
       ChangeUpdate.Factory updateFactory,
       ApprovalsUtil approvalsUtil,
       ChangeIndexer indexer,
@@ -106,7 +101,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
       ChangeHooks hooks) {
     this.db = db;
     this.changes = changes;
-    this.changeDataFactory = changeDataFactory;
     this.updateFactory = updateFactory;
     this.approvalsUtil = approvalsUtil;
     this.indexer = indexer;
@@ -126,7 +120,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
       checkLabels(revision, input.strictLabels, input.labels);
     }
     if (input.comments != null) {
-      checkComments(revision, input.comments);
+      checkComments(input.comments);
     }
     if (input.notify == null) {
       log.warn("notify = null; assuming notify = NONE");
@@ -272,23 +266,13 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
     }
   }
 
-  private void checkComments(RevisionResource revision, Map<String, List<Comment>> in)
+  private void checkComments(Map<String, List<Comment>> in)
-      throws BadRequestException, OrmException {
+      throws BadRequestException {
     Iterator<Map.Entry<String, List<Comment>>> mapItr =
         in.entrySet().iterator();
-    Set<String> filePaths =
-        Sets.newHashSet(changeDataFactory.create(
-            db.get(), revision.getChange()).filePaths(
-                revision.getPatchSet()));
     while (mapItr.hasNext()) {
       Map.Entry<String, List<Comment>> ent = mapItr.next();
       String path = ent.getKey();
-      if (!filePaths.contains(path) && !Patch.COMMIT_MSG.equals(path)) {
-        throw new BadRequestException(String.format(
-            "file %s not found in revision %s",
-            path, change.currentPatchSetId()));
-      }
-
       List<Comment> list = ent.getValue();
       if (list == null) {
         mapItr.remove();

gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java

@@ -62,8 +62,7 @@ public class CommentSender extends ReplyToChangeSender {
     this.notify = notify;
   }
 
-  public void setPatchLineComments(final List<PatchLineComment> plc)
+  public void setPatchLineComments(final List<PatchLineComment> plc) {
-      throws OrmException {
     inlineComments = plc;
 
     Set<String> paths = new HashSet<>();

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java

@@ -62,7 +62,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -173,7 +172,7 @@ public class ChangeData {
   private Collection<PatchSet> patches;
   private ListMultimap<PatchSet.Id, PatchSetApproval> allApprovals;
   private List<PatchSetApproval> currentApprovals;
-  private Map<Integer, List<String>> files = new HashMap<>();
+  private List<String> currentFiles;
   private Collection<PatchLineComment> comments;
   private CurrentUser visibleTo;
   private ChangeControl changeControl;
@@ -259,35 +258,27 @@ public class ChangeData {
     returnedBySource = s;
   }
 
-  public void setCurrentFilePaths(List<String> filePaths) throws OrmException {
+  public void setCurrentFilePaths(List<String> filePaths) {
-    PatchSet ps = currentPatchSet();
+    currentFiles = ImmutableList.copyOf(filePaths);
-    if (ps != null) {
-      files.put(ps.getPatchSetId(), ImmutableList.copyOf(filePaths));
-    }
   }
 
   public List<String> currentFilePaths() throws OrmException {
-    PatchSet ps = currentPatchSet();
+    if (currentFiles == null) {
-    if (ps == null) {
-      return null;
-    }
-    return filePaths(currentPatchSet);
-  }
-
-  public List<String> filePaths(PatchSet ps) throws OrmException {
-    if (!files.containsKey(ps.getPatchSetId())) {
       Change c = change();
       if (c == null) {
         return null;
       }
+      PatchSet ps = currentPatchSet();
+      if (ps == null) {
+        return null;
+      }
 
       PatchList p;
       try {
         p = patchListCache.get(c, ps);
       } catch (PatchListNotAvailableException e) {
-        List<String> emptyFileList = Collections.emptyList();
+        currentFiles = Collections.emptyList();
-        files.put(ps.getPatchSetId(), emptyFileList);
+        return currentFiles;
-        return emptyFileList;
       }
 
       List<String> r = new ArrayList<>(p.getPatches().size());
@@ -311,9 +302,9 @@ public class ChangeData {
         }
       }
       Collections.sort(r);
-      files.put(ps.getPatchSetId(), Collections.unmodifiableList(r));
+      currentFiles = Collections.unmodifiableList(r);
     }
-    return files.get(ps.getPatchSetId());
+    return currentFiles;
   }
 
   public ChangedLines changedLines() throws OrmException {