add tests for Mercurial tag parsing

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

@@ -635,6 +635,11 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) {
         ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK);
         argv.add(RepoCommand);
         argv.add("tags");
+        argv.add("--template");
+        // Use '|' as a revision separator rather than ':' to avoid collision with the commonly used
+        // separator within the revision string (which is not used in the 'hg tags' output but better
+        // safe than sorry).
+        argv.add("{rev}|{tag}\\n");
 
         Executor executor = new Executor(argv, directory,
                 RuntimeEnvironment.getInstance().getCommandTimeout(cmdType));

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

@@ -18,14 +18,15 @@
  */
 
 /*
- * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  */
 package org.opengrok.indexer.history;
 
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.util.NavigableSet;
 import java.util.TreeSet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -49,55 +50,39 @@ public class MercurialTagParser implements Executor.StreamHandler {
      *
      * @return entries a set of tag entries
      */
-    public TreeSet<TagEntry> getEntries() {
+    public NavigableSet<TagEntry> getEntries() {
         return entries;
     }
 
     @Override
     public void processStream(InputStream input) throws IOException {
         try {
-            try (BufferedReader in = new BufferedReader(
-                    new InputStreamReader(input))) {
+            try (BufferedReader in = new BufferedReader(new InputStreamReader(input))) {
                 String line;
                 while ((line = in.readLine()) != null) {
-                    String[] parts = line.split("  *");
+                    String[] parts = line.split("\\|");
                     if (parts.length < 2) {
                         LOGGER.log(Level.WARNING,
                                 "Failed to parse tag list: {0}",
                                 "Tag line contains more than 2 columns: " + line);
                         entries = null;
                         break;
                     }
-                    // Grrr, how to parse tags with spaces inside?
-                    // This solution will lose multiple spaces ;-/
-                    String tag = parts[0];
-                    for (int i = 1; i < parts.length - 1; ++i) {
-                        tag = tag.concat(" ");
-                        tag = tag.concat(parts[i]);
-                    }
+                    String rev = parts[0];
+                    String tag = parts[1];
+
                     // 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) {
-                        LOGGER.log(Level.WARNING,
-                                "Failed to parse tag list: {0}",
-                                "Mercurial revision parsing error: "
-                                        + parts[parts.length - 1]);
-                        entries = null;
-                        break;
-                    }
-                    TagEntry tagEntry
-                            = new MercurialTagEntry(Integer.parseInt(revParts[0]),
-                                    tag);
-                    // Reverse the order of the list
+
+                    TagEntry tagEntry = new MercurialTagEntry(Integer.parseInt(rev), tag);
+                    // Reverse the order of the list.
                     entries.add(tagEntry);
                 }
             }
         } catch (IOException e) {
-            LOGGER.log(Level.WARNING,
-                    "Failed to read tag list: {0}", e.getMessage());
+            LOGGER.log(Level.WARNING, "Failed to read tag list: {0}", e.getMessage());
             entries = null;
         }
     }

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

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2008, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2017, 2020, Chris Fraire <cfraire@me.com>.
  */
 package org.opengrok.indexer.history;
@@ -283,7 +283,7 @@ boolean hasFileBasedTags() {
         return false;
     }
 
-    NavigableSet<TagEntry> getTagList() {
+    @Nullable NavigableSet<TagEntry> getTagList() {
         return this.tagList;
     }
 

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

@@ -83,6 +83,10 @@ protected TagEntry(Date date, String tags) {
         this.tags = tags;
     }
 
+    public int getRevision() {
+        return revision;
+    }
+
     public String getTags() {
         return this.tags;
     }

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

@@ -31,19 +31,26 @@
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.opengrok.indexer.condition.EnabledForRepository;
+import org.opengrok.indexer.configuration.CommandTimeoutType;
 import org.opengrok.indexer.configuration.RuntimeEnvironment;
 import org.opengrok.indexer.util.Executor;
+import org.opengrok.indexer.util.IOUtils;
 import org.opengrok.indexer.util.TestRepository;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -97,7 +104,7 @@ public class MercurialRepositoryTest {
      */
     private void setUpTestRepository() throws IOException, URISyntaxException {
         repository = new TestRepository();
-        repository.create(getClass().getResource("/repositories"));
+        repository.create(Objects.requireNonNull(getClass().getResource("/repositories")));
         repositoryRoot = new File(repository.getSourceRoot(), "mercurial");
     }
 
@@ -441,4 +448,67 @@ void testAnnotationPositive(Pair<String, List<String>> pair) throws Exception {
         List<String> revisions = new ArrayList<>(annotation.getRevisions());
         assertEquals(pair.getValue(), revisions);
     }
+
+    /**
+     * Test special case of repository with no tags, in this case empty repository.
+     */
+    @Test
+    void testBuildTagListEmpty() throws Exception {
+        Path repositoryRootPath = Files.createDirectory(Path.of(RuntimeEnvironment.getInstance().getSourceRootPath(),
+                "emptyTagsTest"));
+        File repositoryRoot = repositoryRootPath.toFile();
+        assertTrue(repositoryRoot.isDirectory());
+        runHgCommand(repositoryRoot, "init");
+        MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot);
+        assertNotNull(hgRepo);
+        hgRepo.buildTagList(new File(hgRepo.getDirectoryName()), CommandTimeoutType.INDEXER);
+        assertEquals(0, hgRepo.getTagList().size());
+        IOUtils.removeRecursive(repositoryRootPath);
+    }
+
+    /**
+     * Extract the tags from the original repository. It already contains one tag.
+     */
+    @Test
+    void testBuildTagListInitial() throws Exception {
+        MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot);
+        assertNotNull(hgRepo);
+        hgRepo.buildTagList(new File(hgRepo.getDirectoryName()), CommandTimeoutType.INDEXER);
+        var tags = hgRepo.getTagList();
+        assertEquals(1, tags.size());
+        Set<TagEntry> expectedTags = new TreeSet<>();
+        TagEntry tagEntry = new MercurialTagEntry(7, "start_of_novel");
+        expectedTags.add(tagEntry);
+        assertEquals(expectedTags, tags);
+    }
+
+    /**
+     * Clone the original repository, add new tag, check that the extracted tags contain the pre-existing
+     * and new one.
+     */
+    @Test
+    void testBuildTagListOneMore() throws Exception {
+        Path repositoryRootPath = Files.createDirectory(Path.of(RuntimeEnvironment.getInstance().getSourceRootPath(),
+                "addedTagTest"));
+        File repositoryRoot = repositoryRootPath.toFile();
+        // Clone the internal repository because it will be modified.
+        // This avoids interference with other tests in this class.
+        runHgCommand(this.repositoryRoot, "clone", this.repositoryRoot.toString(), repositoryRootPath.toString());
+        MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot);
+        assertNotNull(hgRepo);
+        // Using double space on purpose to test the parsing of tags.
+        final String newTagName = "foo  bar";
+        runHgCommand(repositoryRoot, "tag", newTagName);
+        hgRepo.buildTagList(new File(hgRepo.getDirectoryName()), CommandTimeoutType.INDEXER);
+        var tags = hgRepo.getTagList();
+        assertNotNull(tags);
+        assertEquals(2, tags.size());
+        // TagEntry has special semantics for comparing/equality which does not compare the tags at all,
+        // so using assertEquals() on two sets of TagEntry objects would not help.
+        // Instead, check the tags separately.
+        assertEquals(List.of(7, 9), tags.stream().map(TagEntry::getRevision).collect(Collectors.toList()));
+        List<String> expectedTags = List.of("start_of_novel", newTagName);
+        assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).collect(Collectors.toList()));
+        IOUtils.removeRecursive(repositoryRootPath);
+    }
 }