Merge pull request #823 from vladak/tag_fix

reassign tags to changesets when performing merging of old and new histo...

Author: Vladimir Kotal

src/org/opensolaris/opengrok/history/FileHistoryCache.java

@@ -124,7 +124,7 @@ private void doFileHistory(Map.Entry<String, List<HistoryEntry>> map_entry,
 
         File file = new File(root, map_entry.getKey());
         if (!file.isDirectory()) {
-            storeFile(hist, file);
+            storeFile(hist, file, repository);
         }
     }
 
@@ -213,9 +213,10 @@ private static History readCache(File file) throws IOException {
      *
      * @param history history object to store
      * @param file file to store the history object into
+     * @param repo repository for the file
      * @throws HistoryException
      */
-    private void storeFile(History histNew, File file) throws HistoryException {
+    private void storeFile(History histNew, File file, Repository repo) throws HistoryException {
 
         File cache = getCachedFile(file);
         History history = histNew;
@@ -233,12 +234,26 @@ private void storeFile(History histNew, File file) throws HistoryException {
             // Merge old history with the new history.
             List<HistoryEntry> listOld = histOld.getHistoryEntries();
             if (!listOld.isEmpty()) {
+                RuntimeEnvironment env = RuntimeEnvironment.getInstance();
                 List<HistoryEntry> listNew = histNew.getHistoryEntries();
                 ListIterator li = listNew.listIterator(listNew.size());
                 while (li.hasPrevious()) {
                     listOld.add(0, (HistoryEntry) li.previous());
                 }
                 history = new History(listOld);
+
+                // Retag the last changesets in case there have been some new
+                // tags added to the repository. Technically we should just
+                // retag the last revision from the listOld however this
+                // does not solve the problem when listNew contains new tags
+                // retroactively tagging changesets from listOld so we resort
+                // to this somewhat crude solution.
+                if (env.isTagsEnabled() && repo.hasFileBasedTags()) {
+                    for (HistoryEntry ent : history.getHistoryEntries()) {
+                        ent.setTags(null);
+                    }
+                    repo.assignTagsInHistory(history);
+                }
             }
         } catch (IOException ex) {
             // Ideally we would want to catch the case when incremental update
@@ -491,7 +506,7 @@ public History get(File file, Repository repository, boolean withFiles)
                         (cache.exists() ||
                              (time > env.getHistoryReaderTimeLimit()))) {
                 // retrieving the history takes too long, cache it!
-                storeFile(history, file);
+                storeFile(history, file, repository);
             }
         }
         return history;
@@ -573,13 +588,13 @@ private void storeLatestCachedRevision(Repository repository, String rev) {
                   new FileOutputStream(getRepositoryCachedRevPath(repository))));
             writer.write(rev);
         } catch (IOException ex) {
-            logger.log(Level.WARNING, "cannot write latest cached revision to file: {0}",
+            logger.log(Level.WARNING, "cannot write latest cached revision to file: " +
                 ex.getCause());
         } finally {
            try {
                writer.close();
            } catch (IOException ex) {
-               logger.log(Level.INFO, "cannot close file: {0}", ex);
+               logger.log(Level.INFO, "cannot close file: " + ex);
            }
         }
     }

src/org/opensolaris/opengrok/history/HistoryGuru.java

@@ -43,7 +43,6 @@
 import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
 import org.opensolaris.opengrok.index.IgnoredNames;
 import org.opensolaris.opengrok.util.Statistics;
-import org.opensolaris.opengrok.util.StringUtils;
 
 /**
  * The HistoryGuru is used to implement an transparent layer to the various

src/org/opensolaris/opengrok/history/MercurialRepository.java

@@ -589,6 +589,10 @@ protected void buildTagList(File directory) {
                         tag.concat(" ");
                         tag.concat(parts[i]);
                     }
+                    // The implicit 'tip' tag only causes confusion so ignore it. 
+                    if (tag.contentEquals("tip")) {
+                        continue;
+                    }
                     String revParts[] = parts[parts.length - 1].split(":");
                     if (revParts.length != 2) {
                         OpenGrokLogger.getLogger().log(Level.WARNING,

src/org/opensolaris/opengrok/history/Repository.java

@@ -260,7 +260,7 @@ protected String getRevisionForAnnotate(String history_revision) {
     }
 
     /**
-     * Create a history log cache for all of the files in this repository.
+     * Create a history log cache for all files in this repository.
      * {@code getHistory()} is used to fetch the history for the entire
      * repository. If {@code hasHistoryForDirectories()} returns {@code false},
      * this method is a no-op.
@@ -317,6 +317,12 @@ final void createCache(HistoryCache cache, String sinceRevision)
             cache.clear(this);
         }
 
+        // We need to refresh list of tags for incremental reindex.
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+        if (env.isTagsEnabled() && this.hasFileBasedTags()) {
+            this.buildTagList(new File(this.directoryName));
+        }
+
         if (history != null) {
             cache.store(history, this);
         }

src/org/opensolaris/opengrok/history/RepositoryFactory.java

@@ -88,7 +88,8 @@ public static Repository getRepository(File file) throws InstantiationException,
                 try {
                     res.setDirectoryName(file.getCanonicalPath());
                 } catch (IOException e) {
-                    OpenGrokLogger.getLogger().log(Level.SEVERE, "Failed to get canonical path name for " + file.getAbsolutePath(), e);
+                    OpenGrokLogger.getLogger().log(Level.SEVERE,
+                        "Failed to get canonical path name for " + file.getAbsolutePath(), e);
                 }
 
                 if (!res.isWorking()) {

test/org/opensolaris/opengrok/history/FileHistoryCacheTest.java

@@ -30,6 +30,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import junit.framework.TestCase;
+import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
 import org.opensolaris.opengrok.util.Executor;
 import org.opensolaris.opengrok.util.TestRepository;
 
@@ -86,6 +87,7 @@ private void importHgChangeset(File reposRoot, String changesetFile) {
      * Assert that two HistoryEntry objects are equal.
      * @param expected the expected entry
      * @param actual the actual entry
+     * @param isdir was the history generated for a directory
      * @throws AssertFailure if the two entries don't match
      */
     private void assertSameEntries(
@@ -102,6 +104,7 @@ private void assertSameEntries(
      * Assert that two lists of HistoryEntry objects are equal.
      * @param expected the expected list of entries
      * @param actual the actual list of entries
+     * @param isdir was the history generated for directory
      * @throws AssertFailure if the two lists don't match
      */
     private void assertSameEntry(HistoryEntry expected, HistoryEntry actual, boolean isdir) {
@@ -114,8 +117,9 @@ private void assertSameEntry(HistoryEntry expected, HistoryEntry actual, boolean
         } else {
             assertEquals(0, actual.getFiles().size());
         }
+        assertEquals(expected.getTags(), actual.getTags());
     }
-    
+
     /**
      * Basic tests for the {@code store()} method on cache with disabled
      * handling of renamed files.
@@ -138,6 +142,83 @@ public void testStoreAndGetNotRenamed() throws Exception {
         assertEquals("9:8b340409b3a8", cache.getLatestCachedRevision(repo));
     }
 
+    /**
+     * Test tagging by creating history cache for repository with one tag
+     * and then importing couple of changesets which add both file changes
+     * and tags. The last history entry before the import is important
+     * as it needs to be retagged when old history is merged with the new one.
+     */
+    public void testStoreAndGetIncrementalTags() throws Exception {
+        // Enable tagging of history entries.
+        RuntimeEnvironment.getInstance().setTagsEnabled(true);
+        
+        File reposRoot = new File(repositories.getSourceRoot(), "mercurial");
+        Repository repo = RepositoryFactory.getRepository(reposRoot);
+        History historyToStore = repo.getHistory(reposRoot);
+
+        // Store the history.
+        cache.store(historyToStore, repo);
+
+        // Add bunch of changesets with file based changes and tags.
+        importHgChangeset(
+            reposRoot, getClass().getResource("hg-export-tag.txt").getPath());
+
+        // Perform incremental reindex.
+        repo.createCache(cache, cache.getLatestCachedRevision(repo));
+
+        // Check that the changesets were indeed applied and indexed.
+        History updatedHistory = cache.get(reposRoot, repo, true);
+        assertEquals("Unexpected number of history entries",
+            15, updatedHistory.getHistoryEntries().size());
+
+        // Verify tags in fileHistory for main.c which is the most interesting
+        // file from the repository from the perspective of tags.
+        File main = new File(reposRoot, "main.c");
+        assertTrue(main.exists());
+        History retrievedHistoryMainC = cache.get(main, repo, true);
+        List<HistoryEntry> entries = retrievedHistoryMainC.getHistoryEntries();
+        assertEquals("Unexpected number of entries for main.c",
+            3, entries.size());
+        HistoryEntry e0 = entries.get(0);
+        assertEquals("Unexpected revision for entry 0", "13:3d386f6bd848",
+            e0.getRevision());
+        assertEquals("Invalid tag list for revision 13", "tag3", e0.getTags());
+        HistoryEntry e1 = entries.get(1);
+        assertEquals("Unexpected revision for entry 1", "2:585a1b3f2efb",
+            e1.getRevision());
+        assertEquals("Invalid tag list for revision 2",
+            "tag2, tag1, start_of_novel", e1.getTags());
+        HistoryEntry e2 = entries.get(2);
+        assertEquals("Unexpected revision for entry 2", "1:f24a5fd7a85d",
+            e2.getRevision());
+        assertEquals("Invalid tag list for revision 1", null, e2.getTags());
+        
+        // Reindex from scratch.
+        File dir = new File(cache.getRepositoryHistDataDirname(repo));
+        assertTrue(dir.isDirectory());
+        cache.clear(repo);
+        // We cannot call cache.get() here since it would read the history anew.
+        // Instead check that the data directory does not exist anymore.
+        assertFalse(dir.exists());
+        History freshHistory = repo.getHistory(reposRoot);
+        cache.store(freshHistory, repo);
+        History updatedHistoryFromScratch = cache.get(reposRoot, repo, true);
+        assertEquals("Unexpected number of history entries",
+            freshHistory.getHistoryEntries().size(),
+            updatedHistoryFromScratch.getHistoryEntries().size());
+        
+        // Verify that the result for the directory is the same as incremental
+        // reindex.
+        assertSameEntries(updatedHistory.getHistoryEntries(),
+            updatedHistoryFromScratch.getHistoryEntries(), true);
+        // Do the same for main.c.
+        History retrievedUpdatedHistoryMainC = cache.get(main, repo, true);
+        assertSameEntries(retrievedHistoryMainC.getHistoryEntries(),
+            retrievedUpdatedHistoryMainC.getHistoryEntries(), false);
+
+        RuntimeEnvironment.getInstance().setTagsEnabled(false);
+    }
+
     /**
      * Basic tests for the {@code store()} and {@code get()} methods.
      */

test/org/opensolaris/opengrok/history/JDBCHistoryCacheTest.java

@@ -496,7 +496,7 @@ public void testTags() throws Exception {
         List<HistoryEntry> dirHistory =
                 cache.get(reposRoot, repos, false).getHistoryEntries();
         assertEquals("Size of history", 10, dirHistory.size());
-        assertEquals("tip", dirHistory.get(0).getTags());
+        assertEquals(null, dirHistory.get(0).getTags());
         assertNull(dirHistory.get(1).getTags());
         assertEquals("start_of_novel", dirHistory.get(2).getTags());
         assertNull(dirHistory.get(3).getTags());
@@ -505,14 +505,14 @@ public void testTags() throws Exception {
                 cache.get(new File(reposRoot, "novel.txt"),
                           repos, false).getHistoryEntries();
         assertEquals("Size of history", 6, novelHistory.size());
-        assertEquals("tip", novelHistory.get(0).getTags());
+        assertEquals(null, novelHistory.get(0).getTags());
         assertEquals("start_of_novel", novelHistory.get(1).getTags());
 
         List<HistoryEntry> maincHistory =
                 cache.get(new File(reposRoot, "main.c"),
                           repos, false).getHistoryEntries();
         assertEquals("Size of history", 2, maincHistory.size());
-        assertEquals("tip, start_of_novel", maincHistory.get(0).getTags());
+        assertEquals("start_of_novel", maincHistory.get(0).getTags());
         assertNull(maincHistory.get(1).getTags());
     }
 }

test/org/opensolaris/opengrok/history/hg-export-tag.txt

@@ -0,0 +1,77 @@
+# HG changeset patch
+# User Vladimir Kotal <[email protected]>
+# Date 1398454830 -7200
+#      Fri Apr 25 21:40:30 2014 +0200
+# Node ID 9203f5519aca0213ab1fb1700082eeabeda82e60
+# Parent  8b340409b3a81618750aa660cb113c65440688be
+Added tag tag1 for changeset 8b340409b3a8
+
+diff -r 8b340409b3a8 -r 9203f5519aca .hgtags
+--- a/.hgtags	Sat Jul 13 18:27:46 2013 +0200
++++ b/.hgtags	Fri Apr 25 21:40:30 2014 +0200
+@@ -1,1 +1,2 @@
+ db1394c05268686f89fbe4ca5524ac4ed61ec71c start_of_novel
++8b340409b3a81618750aa660cb113c65440688be tag1
+# HG changeset patch
+# User Vladimir Kotal <[email protected]>
+# Date 1398454848 -7200
+#      Fri Apr 25 21:40:48 2014 +0200
+# Node ID c102ebbaff2bf9320277a374c287987ec7f99ab9
+# Parent  9203f5519aca0213ab1fb1700082eeabeda82e60
+more
+
+diff -r 9203f5519aca -r c102ebbaff2b bar.txt
+--- a/bar.txt	Fri Apr 25 21:40:30 2014 +0200
++++ b/bar.txt	Fri Apr 25 21:40:48 2014 +0200
+@@ -1,1 +1,1 @@
+-stuff
++stuff more
+# HG changeset patch
+# User Vladimir Kotal <[email protected]>
+# Date 1398454861 -7200
+#      Fri Apr 25 21:41:01 2014 +0200
+# Node ID de12e7efa8d8771de0614fe151cf6e8a16530da6
+# Parent  c102ebbaff2bf9320277a374c287987ec7f99ab9
+Added tag tag2 for changeset c102ebbaff2b
+
+diff -r c102ebbaff2b -r de12e7efa8d8 .hgtags
+--- a/.hgtags	Fri Apr 25 21:40:48 2014 +0200
++++ b/.hgtags	Fri Apr 25 21:41:01 2014 +0200
+@@ -1,2 +1,3 @@
+ db1394c05268686f89fbe4ca5524ac4ed61ec71c start_of_novel
+ 8b340409b3a81618750aa660cb113c65440688be tag1
++c102ebbaff2bf9320277a374c287987ec7f99ab9 tag2
+# HG changeset patch
+# User Vladimir Kotal <[email protected]>
+# Date 1398454896 -7200
+#      Fri Apr 25 21:41:36 2014 +0200
+# Node ID 3d386f6bd84815069dbd6e4e1bc14bd1b958bdc7
+# Parent  de12e7efa8d8771de0614fe151cf6e8a16530da6
+brackets
+
+diff -r de12e7efa8d8 -r 3d386f6bd848 main.c
+--- a/main.c	Fri Apr 25 21:41:01 2014 +0200
++++ b/main.c	Fri Apr 25 21:41:36 2014 +0200
+@@ -8,5 +8,5 @@
+    }
+    (void)printf("\n");
+ 
+-   return EXIT_SUCCESS;
++   return (EXIT_SUCCESS);
+ }
+# HG changeset patch
+# User Vladimir Kotal <[email protected]>
+# Date 1398454900 -7200
+#      Fri Apr 25 21:41:40 2014 +0200
+# Node ID 5c85cd3019d8c8cbe54bca382c8329622be42c7c
+# Parent  3d386f6bd84815069dbd6e4e1bc14bd1b958bdc7
+Added tag tag3 for changeset 3d386f6bd848
+
+diff -r 3d386f6bd848 -r 5c85cd3019d8 .hgtags
+--- a/.hgtags	Fri Apr 25 21:41:36 2014 +0200
++++ b/.hgtags	Fri Apr 25 21:41:40 2014 +0200
+@@ -1,3 +1,4 @@
+ db1394c05268686f89fbe4ca5524ac4ed61ec71c start_of_novel
+ 8b340409b3a81618750aa660cb113c65440688be tag1
+ c102ebbaff2bf9320277a374c287987ec7f99ab9 tag2
++3d386f6bd84815069dbd6e4e1bc14bd1b958bdc7 tag3