fix/remove TODOs, add tests

Author: Vladimir Kotal

opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java

@@ -606,8 +606,6 @@ History getHistory(File file, String sinceRevision) throws HistoryException {
         try (org.eclipse.jgit.lib.Repository repository = getJGitRepository(getDirectoryName());
              RevWalk walk = new RevWalk(repository)) {
 
-            // TODO: does the walk take current branch into account ?
-
             if (sinceRevision != null) {
                 walk.markUninteresting(walk.lookupCommit(repository.resolve(sinceRevision)));
             }
@@ -622,16 +620,7 @@ History getHistory(File file, String sinceRevision) throws HistoryException {
                     FollowFilter followFilter = FollowFilter.create(getRepoRelativePath(file), dc);
                     walk.setTreeFilter(followFilter);
                 } else {
-                    // TODO: untested, should be simpler (taken from LogCommand)
-                    List<PathFilter> pathFilters = new ArrayList<>();
-                    pathFilters.add(PathFilter.create(getRepoRelativePath(file)));
-                    List<TreeFilter> filters = new ArrayList<>();
-                    filters.add(AndTreeFilter.create(PathFilterGroup.create(pathFilters), TreeFilter.ANY_DIFF));
-                    if (filters.size() == 1) {
-                        filters.add(TreeFilter.ANY_DIFF);
-                    }
-
-                    walk.setTreeFilter(AndTreeFilter.create(filters));
+                    walk.setTreeFilter(PathFilter.create(getRepoRelativePath(file)));
                 }
             }
 
@@ -650,7 +639,7 @@ History getHistory(File file, String sinceRevision) throws HistoryException {
                 SortedSet<String> files = new TreeSet<>();
                 if (numParents == 1) {
                     getFiles(repository, commit.getParent(0), commit, files, renamedFiles);
-                } else if (numParents == 0) { // first commit (TODO: could be dangling ?)
+                } else if (numParents == 0) { // first commit
                     try (TreeWalk treeWalk = new TreeWalk(repository)) {
                         treeWalk.addTree(commit.getTree());
                         treeWalk.setRecursive(true);
@@ -699,7 +688,7 @@ private void getFiles(org.eclipse.jgit.lib.Repository repository,
                 if (diff.getChangeType() != DiffEntry.ChangeType.DELETE) {
                     files.add(getDirectoryNameRelative() + "/" + diff.getNewPath());
                 }
-                if (diff.getChangeType() == DiffEntry.ChangeType.RENAME && isHandleRenamedFiles()) {  // TODO: add COPY ?
+                if (diff.getChangeType() == DiffEntry.ChangeType.RENAME && isHandleRenamedFiles()) {
                     renamedFiles.add(diff.getNewPath());
                 }
             }

opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryTest.java

@@ -27,6 +27,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.InvocationTargetException;
 import java.nio.file.Paths;
 import java.text.ParseException;
 import java.util.Arrays;
@@ -54,6 +55,7 @@
 import org.opengrok.indexer.configuration.CommandTimeoutType;
 import org.opengrok.indexer.configuration.RuntimeEnvironment;
 import org.opengrok.indexer.util.FileUtilities;
+import org.opengrok.indexer.util.ForbiddenSymlinkException;
 import org.opengrok.indexer.util.TestRepository;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -209,6 +211,43 @@ public void testDetermineBranchAfterChange() throws Exception {
         }
     }
 
+    @Test
+    public void testGetHistoryInBranch() throws Exception {
+        // Clone the test repository and create new branch there.
+        // Clone under source root to avoid problems with prohibited symlinks.
+        File root = new File(repository.getSourceRoot(), "git");
+        File localPath = new File(repository.getSourceRoot(), "gitCloneTestDetermineBranch");
+        GitRepository gitrepo = (GitRepository) RepositoryFactory.getRepository(root);
+        String branch;
+        String cloneUrl = root.toURI().toString();
+        try (Git gitClone = Git.cloneRepository()
+                .setURI(cloneUrl)
+                .setDirectory(localPath)
+                .call()) {
+
+            Ref ref = gitClone.checkout().setCreateBranch(true).setName("foo").call();
+            assertNotNull(ref);
+
+            File cloneRoot = gitClone.getRepository().getWorkTree();
+            gitrepo = (GitRepository) RepositoryFactory.getRepository(cloneRoot);
+
+            History history = gitrepo.getHistory(cloneRoot);
+            assertNotNull(history);
+            int numEntries = history.getHistoryEntries().size();
+            assertTrue(numEntries > 0);
+
+            RevCommit commit = gitClone.commit().
+                    setAuthor("Snufkin", "[email protected]").
+                    setMessage("fresh commit on a new branch").setAllowEmpty(true).call();
+            assertNotNull(commit);
+
+            history = gitrepo.getHistory(cloneRoot);
+            assertEquals(numEntries + 1, history.getHistoryEntries().size());
+
+            FileUtilities.removeDirs(cloneRoot);
+        }
+    }
+
     @Test
     public void testDetermineParentEmpty() throws Exception {
         File root = new File(repository.getSourceRoot(), "git");
@@ -579,6 +618,19 @@ public void testHistory(boolean renamedHandling) throws Exception {
         assertEquals(expectedHistory, history);
     }
 
+    @Test
+    public void testSingleHistory() throws Exception {
+        RuntimeEnvironment.getInstance().setHandleHistoryOfRenamedFiles(false);
+        File root = new File(repository.getSourceRoot(), "git");
+        GitRepository gitrepo = (GitRepository) RepositoryFactory.getRepository(root);
+
+        History history = gitrepo.getHistory(new File(root.getAbsolutePath(), "moved2/renamed2.c"));
+        assertNotNull(history);
+        assertNotNull(history.getHistoryEntries());
+        assertEquals(1, history.getHistoryEntries().size());
+        assertEquals("84599b3c", history.getHistoryEntries().get(0).getRevision());
+    }
+
     @Test
     public void testRenamedSingleHistory() throws Exception {
         RuntimeEnvironment.getInstance().setHandleHistoryOfRenamedFiles(true);