convert Mercurial to RepositoryWithHistoryTraversal

vladak · vladak · commit 794d0b760105 · 2022-05-24T18:21:36.000+02:00
diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java
@@ -475,27 +475,6 @@ public History getHistory(File file, String sinceRevision, String tillRevision)
         return getHistory(file, sinceRevision, tillRevision, null);
     }
 
-    public History getHistory(File file, String sinceRevision, String tillRevision,
-                              Integer numCommits) throws HistoryException {
-
-        if (numCommits != null && numCommits <= 0) {
-            return null;
-        }
-
-        HistoryCollector historyCollector = new HistoryCollector(isMergeCommitsEnabled());
-        traverseHistory(file, sinceRevision, tillRevision, numCommits, List.of(historyCollector));
-        History history = new History(historyCollector.entries, historyCollector.renamedFiles);
-
-        // Assign tags to changesets they represent
-        // We don't need to check if this repository supports tags,
-        // because we know it :-)
-        if (RuntimeEnvironment.getInstance().isTagsEnabled()) {
-            assignTagsInHistory(history);
-        }
-
-        return history;
-    }
-
     public void traverseHistory(File file, String sinceRevision, String tillRevision,
                               Integer numCommits, List<ChangesetVisitor> visitors) throws HistoryException {
 
diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryCollector.java
@@ -39,8 +39,17 @@ class HistoryCollector extends ChangesetVisitor {
 
     public void accept(RepositoryWithHistoryTraversal.ChangesetInfo changesetInfo) {
         RepositoryWithHistoryTraversal.CommitInfo commit = changesetInfo.commit;
+
+        // TODO: add a test for this
+        String author;
+        if (commit.authorEmail != null) {
+            author = commit.authorName + " <" + commit.authorEmail + ">";
+        } else {
+            author = commit.authorName;
+        }
+
         HistoryEntry historyEntry = new HistoryEntry(commit.revision,
-                commit.date, commit.authorName + " <" + commit.authorEmail + ">",
+                commit.date, author,
                 commit.message, true);
 
         if (changesetInfo.renamedFiles != null) {
diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialHistoryParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialHistoryParser.java
@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2006, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2017, Chris Fraire <cfraire@me.com>.
  */
 package org.opengrok.indexer.history;
@@ -33,10 +33,8 @@
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Date;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.opengrok.indexer.configuration.RuntimeEnvironment;
@@ -54,67 +52,94 @@ class MercurialHistoryParser implements Executor.StreamHandler {
     /** Prefix which identifies lines with the description of a commit. */
     private static final String DESC_PREFIX = "description: ";
 
-    private List<HistoryEntry> entries = new ArrayList<>();
+    private List<RepositoryWithHistoryTraversal.ChangesetInfo> entries = new ArrayList<>();
     private final MercurialRepository repository;
     private final String mydir;
     private boolean isDir;
-    private final Set<String> renamedFiles = new HashSet<>();
+    private final List<ChangesetVisitor> visitors;
 
-    MercurialHistoryParser(MercurialRepository repository) {
+    MercurialHistoryParser(MercurialRepository repository, List<ChangesetVisitor> visitors) {
         this.repository = repository;
+        this.visitors = visitors;
         mydir = repository.getDirectoryName() + File.separator;
     }
 
     /**
      * Parse the history for the specified file or directory. If a changeset is
-     * specified, only return the history from the changeset right after the
-     * specified one.
+     * specified, only return the history from the changeset right after the specified one.
      *
      * @param file the file or directory to get history for
      * @param sinceRevision the changeset right before the first one to fetch, or
      * {@code null} if all changesets should be fetched
      * @param tillRevision end revision or {@code null}
      * @param numCommits number of revisions to get
-     * @return history for the specified file or directory
      * @throws HistoryException if an error happens when parsing the history
      */
-    History parse(File file, String sinceRevision, String tillRevision, Integer numCommits) throws HistoryException {
+    void parse(File file, String sinceRevision, String tillRevision, Integer numCommits) throws HistoryException {
         isDir = file.isDirectory();
         try {
             Executor executor = repository.getHistoryLogExecutor(file, sinceRevision, tillRevision, false,
                     numCommits);
             int status = executor.exec(true, this);
 
             if (status != 0) {
-                throw new HistoryException("Failed to get history for: \"" +
-                                           file.getAbsolutePath() +
+                throw new HistoryException("Failed to get history for: \"" + file.getAbsolutePath() +
                                            "\" Exit code: " + status);
             }
         } catch (IOException e) {
-            throw new HistoryException("Failed to get history for: \"" +
-                                       file.getAbsolutePath() + "\"", e);
+            throw new HistoryException("Failed to get history for: \"" + file.getAbsolutePath() + "\"", e);
         }
 
-        // If a changeset to start from is specified, remove that changeset
-        // from the list, since only the ones following it should be returned.
-        // Also check that the specified changeset was found, otherwise throw
-        // an exception.
+        // If a changeset to start from is specified, remove that changeset from the list,
+        // since only the ones following it should be returned.
+        // Also check that the specified changeset was found, otherwise throw an exception.
         if (sinceRevision != null) {
-            repository.removeAndVerifyOldestChangeset(entries, sinceRevision);
+            removeAndVerifyOldestChangeset(entries, sinceRevision);
         }
 
         // See getHistoryLogExecutor() for explanation.
         if (repository.isHandleRenamedFiles() && file.isFile() && tillRevision != null) {
             removeChangesets(entries, tillRevision);
         }
 
-        return new History(entries, renamedFiles);
+        // The visitors are fed with the ChangesetInfo instances here (as opposed to in parse()),
+        // because of the above manipulations with the entries.
+        for (RepositoryWithHistoryTraversal.ChangesetInfo info : entries) {
+            for (ChangesetVisitor visitor : visitors) {
+                visitor.accept(info);
+            }
+        }
+    }
+
+    /**
+     * Remove the oldest changeset from a list (assuming sorted with most recent
+     * changeset first) and verify that it is the changeset expected to find there.
+     *
+     * @param entries a list of {@code HistoryEntry} objects
+     * @param revision the revision we expect the oldest entry to have
+     * @throws HistoryException if the oldest entry was not the one we expected
+     */
+    private void removeAndVerifyOldestChangeset(List<RepositoryWithHistoryTraversal.ChangesetInfo> entries, String revision)
+            throws HistoryException {
+
+        RepositoryWithHistoryTraversal.ChangesetInfo entry = entries.isEmpty() ? null : entries.remove(entries.size() - 1);
+
+        // TODO We should check more thoroughly that the changeset is the one
+        // we expected it to be, since some SCMs may change the revision
+        // numbers so that identical revision numbers does not always mean
+        // identical changesets. We could for example get the cached changeset
+        // and compare more fields, like author and date.
+        if (entry == null || !revision.equals(entry.commit.revision)) {
+            throw new HistoryException("Cached revision '" + revision
+                    + "' not found in the repository "
+                    + repository.getDirectoryName());
+        }
     }
 
-    private void removeChangesets(List<HistoryEntry> entries, String tillRevision) {
-        for (Iterator<HistoryEntry> iter = entries.listIterator(); iter.hasNext(); ) {
-            HistoryEntry entry = iter.next();
-            if (entry.getRevision().equals(tillRevision)) {
+    private void removeChangesets(List<RepositoryWithHistoryTraversal.ChangesetInfo> entries, String tillRevision) {
+        for (Iterator<RepositoryWithHistoryTraversal.ChangesetInfo> iter = entries.listIterator(); iter.hasNext(); ) {
+            RepositoryWithHistoryTraversal.ChangesetInfo entry = iter.next();
+            if (entry.commit.revision.equals(tillRevision)) {
                 break;
             }
             iter.remove();
@@ -123,7 +148,7 @@ private void removeChangesets(List<HistoryEntry> entries, String tillRevision) {
 
     /**
      * Process the output from the {@code hg log} command and collect
-     * {@link HistoryEntry} elements.
+     * {@link org.opengrok.indexer.history.RepositoryWithHistoryTraversal.ChangesetInfo} elements.
      *
      * @param input The output from the process
      * @throws java.io.IOException If an error occurs while reading the stream
@@ -134,15 +159,14 @@ public void processStream(InputStream input) throws IOException {
         BufferedReader in = new BufferedReader(new InputStreamReader(input));
         entries = new ArrayList<>();
         String s;
-        HistoryEntry entry = null;
+        RepositoryWithHistoryTraversal.ChangesetInfo entry = null;
         while ((s = in.readLine()) != null) {
             if (s.startsWith(MercurialRepository.CHANGESET)) {
-                entry = new HistoryEntry();
+                entry = new RepositoryWithHistoryTraversal.ChangesetInfo(new RepositoryWithHistoryTraversal.CommitInfo());
                 entries.add(entry);
-                entry.setActive(true);
-                entry.setRevision(s.substring(MercurialRepository.CHANGESET.length()).trim());
+                entry.commit.revision = s.substring(MercurialRepository.CHANGESET.length()).trim();
             } else if (s.startsWith(MercurialRepository.USER) && entry != null) {
-                entry.setAuthor(s.substring(MercurialRepository.USER.length()).trim());
+                entry.commit.authorName = s.substring(MercurialRepository.USER.length()).trim();
             } else if (s.startsWith(MercurialRepository.DATE) && entry != null) {
                 Date date;
                 try {
@@ -154,15 +178,15 @@ public void processStream(InputStream input) throws IOException {
                     //
                     throw new IOException("Could not parse date: " + s, pe);
                 }
-                entry.setDate(date);
+                entry.commit.date = date;
             } else if (s.startsWith(MercurialRepository.FILES) && entry != null) {
                 String[] strings = s.split(" ");
                 for (int ii = 1; ii < strings.length; ++ii) {
                     if (strings[ii].length() > 0) {
                         File f = new File(mydir, strings[ii]);
                         try {
                             String path = env.getPathRelativeToSourceRoot(f);
-                            entry.addFile(path.intern());
+                            entry.files.add(path.intern());
                         } catch (ForbiddenSymlinkException e) {
                             LOGGER.log(Level.FINER, e.getMessage());
                             // ignore
@@ -189,11 +213,11 @@ public void processStream(InputStream input) throws IOException {
                      String[] move = part.split(" \\(");
                      File f = new File(mydir + move[0]);
                      if (!move[0].isEmpty() && f.exists()) {
-                             renamedFiles.add(repository.getDirectoryNameRelative() + File.separator + move[0]);
+                             entry.renamedFiles.add(repository.getDirectoryNameRelative() + File.separator + move[0]);
                      }
                 }
             } else if (s.startsWith(DESC_PREFIX) && entry != null) {
-                entry.setMessage(decodeDescription(s));
+                entry.commit.message = decodeDescription(s);
             } else if (s.equals(MercurialRepository.END_OF_ENTRY)
                 && entry != null) {
                     entry = null;
diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java
@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2006, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2017, 2019, Chris Fraire <cfraire@me.com>.
  */
 package org.opengrok.indexer.history;
@@ -52,7 +52,7 @@
  * Access to a Mercurial repository.
  *
  */
-public class MercurialRepository extends RepositoryWithPerPartesHistory {
+public class MercurialRepository extends RepositoryWithHistoryTraversal {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(MercurialRepository.class);
 
@@ -584,31 +584,12 @@ History getHistory(File file, String sinceRevision, String tillRevision) throws
         return getHistory(file, sinceRevision, tillRevision, null);
     }
 
-    History getHistory(File file, String sinceRevision, String tillRevision,
-                       Integer numCommits) throws HistoryException {
+    // TODO: add a test for this
+    public void traverseHistory(File file, String sinceRevision, String tillRevision,
+                                Integer numCommits, List<ChangesetVisitor> visitors) throws HistoryException {
 
-        if (numCommits != null && numCommits <= 0) {
-            return null;
-        }
-
-        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
-        // Note that the filtering of revisions based on sinceRevision is done
-        // in the history log executor by passing appropriate options to
-        // the 'hg' executable.
-        // This is done only for directories since if getHistory() is used
-        // for file, the file is renamed and its complete history is fetched
-        // so no sinceRevision filter is needed.
-        // See findOriginalName() code for more details.
-        History result = new MercurialHistoryParser(this).
+        new MercurialHistoryParser(this, visitors).
                 parse(file, sinceRevision, tillRevision, numCommits);
-
-        // Assign tags to changesets they represent.
-        // We don't need to check if this repository supports tags,
-        // because we know it :-)
-        if (env.isTagsEnabled()) {
-            assignTagsInHistory(result);
-        }
-        return result;
     }
 
     /**
diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java
@@ -301,11 +301,9 @@ public InputStream getHistoryGet(String parent, String basename, String rev) {
 
     /**
      * Returns if this repository tags only files changed in last commit, i.e.
-     * if we need to prepare list of repository-wide tags prior to creation of
-     * file history entries.
+     * if we need to prepare list of repository-wide tags prior to creation of file history entries.
      *
-     * @return True if we need tag list creation prior to file parsing, false by
-     * default.
+     * @return True if we need tag list creation prior to file parsing, false by default.
      */
     boolean hasFileBasedTags() {
         return false;
diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryWithHistoryTraversal.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryWithHistoryTraversal.java
@@ -31,9 +31,11 @@
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.SortedSet;
+import java.util.TreeSet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -50,6 +52,9 @@ public static class CommitInfo {
         String authorEmail;
         String message;
 
+        CommitInfo() {
+        }
+
         CommitInfo(String revision, Date date, String authorName, String authorEmail, String message) {
             this.revision = revision;
             this.date = date;
@@ -67,6 +72,9 @@ public static class ChangesetInfo {
 
         ChangesetInfo(CommitInfo commit) {
             this.commit = commit;
+            this.files = new TreeSet<>();
+            this.renamedFiles = new HashSet<>();
+            this.deletedFiles = new HashSet<>();
         }
 
         ChangesetInfo(CommitInfo commit, SortedSet<String> files, Set<String> renamedFiles, Set<String> deletedFiles) {
@@ -89,6 +97,25 @@ public static class ChangesetInfo {
     public abstract void traverseHistory(File file, String sinceRevision, @Nullable String tillRevision,
                          Integer numCommits, List<ChangesetVisitor> visitors) throws HistoryException;
 
+    public History getHistory(File file, String sinceRevision, String tillRevision,
+                              Integer numCommits) throws HistoryException {
+
+        if (numCommits != null && numCommits <= 0) {
+            return null;
+        }
+
+        HistoryCollector historyCollector = new HistoryCollector(isMergeCommitsEnabled());
+        traverseHistory(file, sinceRevision, tillRevision, numCommits, List.of(historyCollector));
+        History history = new History(historyCollector.entries, historyCollector.renamedFiles);
+
+        // Assign tags to changesets they represent.
+        if (RuntimeEnvironment.getInstance().isTagsEnabled() && hasFileBasedTags()) {
+            assignTagsInHistory(history);
+        }
+
+        return history;
+    }
+
     @Override
     protected void doCreateCache(HistoryCache cache, String sinceRevision, File directory) throws HistoryException {
         RuntimeEnvironment env = RuntimeEnvironment.getInstance();
@@ -115,8 +142,7 @@ protected void doCreateCache(HistoryCache cache, String sinceRevision, File dire
             History history = new History(historyCollector.entries, historyCollector.renamedFiles);
 
             // Assign tags to changesets they represent.
-            // We don't need to check if this repository supports tags, because we know it :-)
-            if (env.isTagsEnabled()) {
+            if (env.isTagsEnabled() && hasFileBasedTags()) {
                 assignTagsInHistory(history);
             }
 
@@ -150,8 +176,7 @@ protected void doCreateCache(HistoryCache cache, String sinceRevision, File dire
             History history = new History(historyCollector.entries, historyCollector.renamedFiles);
 
             // Assign tags to changesets they represent.
-            // We don't need to check if this repository supports tags, because we know it :-)
-            if (env.isTagsEnabled()) {
+            if (env.isTagsEnabled() && hasFileBasedTags()) {
                 assignTagsInHistory(history);
             }
 
diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java
