Feature/352 tag parsing in xml documents

api/src/main/java/org/itsallcode/openfasttrace/api/importer/ImporterFactory.java

@@ -20,6 +20,18 @@ protected ImporterFactory()
         // empty by intention
     }
 
+    /**
+     * Get the priority of this {@link ImporterFactory}.
+     * <p>
+     * The priority is used to determine the order in which importer factories
+     * are tried when importing a file. Lower priority values indicate higher
+     * precedence.
+     * </p>
+     * 
+     * @return priority of this importer factory
+     */
+    public abstract int getPriority();
+
     /**
      * Returns {@code true} if this {@link ImporterFactory} supports
      * importing the given file based on its file extension.

core/src/main/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoader.java

@@ -1,6 +1,7 @@
 package org.itsallcode.openfasttrace.core.importer;
 
 import java.nio.file.Path;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
 import java.util.logging.Logger;
@@ -17,7 +18,6 @@
 public class ImporterFactoryLoader
 {
     private static final Logger LOG = Logger.getLogger(ImporterFactoryLoader.class.getName());
-
     private final Loader<ImporterFactory> serviceLoader;
 
     /**
@@ -45,29 +45,27 @@ public ImporterFactoryLoader(final ImporterContext context)
 
     /**
      * Finds a matching {@link ImporterFactory} that can handle the given
-     * {@link Path}. If no or more than one {@link ImporterFactory} is found,
-     * this throws an {@link ImporterException}.
+     * {@link Path}. If no {@link ImporterFactory} is found, this throws an
+     * {@link ImporterException}.
      *
      * @param file
      *            the file for which to get a {@link ImporterFactory}.
      * @return a matching {@link ImporterFactory} that can handle the given
      *         {@link Path}
      * @throws ImporterException
-     *             when no or more than one {@link ImporterFactory} is found.
+     *             when no {@link ImporterFactory} is found.
      */
     public Optional<ImporterFactory> getImporterFactory(final InputFile file)
     {
-        final List<ImporterFactory> matchingImporters = getMatchingFactories(file);
-        switch (matchingImporters.size())
+        final List<ImporterFactory> matchingImporters = getMatchingFactoriesInOrderOfPriority(file);
+        if (matchingImporters.isEmpty())
         {
-        case 0:
             LOG.info(() -> "Found no matching importer for file '" + file + "'");
             return Optional.empty();
-        case 1:
+        }
+        else
+        {
             return Optional.of(matchingImporters.get(0));
-        default:
-            LOG.info(() -> "Found more than one matching importer for file '" + file + "'");
-            return Optional.empty();
         }
     }
 
@@ -81,12 +79,12 @@ public Optional<ImporterFactory> getImporterFactory(final InputFile file)
      */
     public boolean supportsFile(final InputFile file)
     {
-        final boolean supported = !getMatchingFactories(file).isEmpty();
+        final boolean supported = !getMatchingFactoriesInOrderOfPriority(file).isEmpty();
         LOG.finest(() -> "File " + file + " is supported = " + supported);
         return supported;
     }
 
-    private List<ImporterFactory> getMatchingFactories(final InputFile file)
+    private List<ImporterFactory> getMatchingFactoriesInOrderOfPriority(final InputFile file)
     {
         final List<ImporterFactory> allFactories = this.serviceLoader.load().toList();
         if (allFactories.isEmpty())
@@ -96,6 +94,7 @@ private List<ImporterFactory> getMatchingFactories(final InputFile file)
         }
         return allFactories.stream()
                 .filter(f -> f.supportsFile(file))
+                .sorted(Comparator.comparingInt(ImporterFactory::getPriority))
                 .toList();
     }
 }

core/src/test/java/org/itsallcode/openfasttrace/core/importer/TestImporterFactoryLoader.java

@@ -1,12 +1,11 @@
 package org.itsallcode.openfasttrace.core.importer;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.same;
-import static org.mockito.Mockito.lenient;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 import java.nio.file.Paths;
 import java.util.Arrays;
@@ -26,8 +25,7 @@
  * Test for {@link ImporterFactoryLoader}
  */
 @ExtendWith(MockitoExtension.class)
-class TestImporterFactoryLoader
-{
+class TestImporterFactoryLoader {
     @Mock
     private Loader<ImporterFactory> serviceLoaderMock;
     @Mock
@@ -41,8 +39,7 @@ class TestImporterFactoryLoader
     private InputFile file;
 
     @BeforeEach
-    void beforeEach()
-    {
+    void beforeEach() {
         this.loader = new ImporterFactoryLoader(this.serviceLoaderMock);
         this.file = RealFileInput.forPath(Paths.get("dir", "name"));
 
@@ -52,49 +49,49 @@ void beforeEach()
     }
 
     @Test
-    void testNoFactoryRegisteredThrowsException()
-    {
+    void testNoFactoryRegisteredThrowsException() {
         simulateFactories();
         assertThrows(ImporterException.class, () -> this.loader.getImporterFactory(this.file));
     }
 
     @Test
-    void testSupportsFileWhenNoFactoryRegisteredThrowsException()
-    {
+    void testSupportsFileWhenNoFactoryRegisteredThrowsException() {
         simulateFactories();
         assertThrows(ImporterException.class, () -> this.loader.supportsFile(this.file));
     }
 
     @Test
-    void testMatchingFactoryFoundOnlyOneAvailable()
-    {
+    void testMatchingFactoryFoundOnlyOneAvailable() {
         simulateFactories(this.supportedFactory1);
         assertFactoryFound(this.supportedFactory1);
     }
 
     @Test
-    void testMatchingFactoryFoundTwoAvailable()
-    {
+    void testMatchingFactoryFoundTwoAvailable() {
         simulateFactories(this.supportedFactory1, this.unsupportedFactory);
         assertFactoryFound(this.supportedFactory1);
     }
 
     @Test
-    void testMultipleMatchingFactoriesFoundReturnNoImporter()
-    {
-        simulateFactories(this.supportedFactory1, this.supportedFactory1);
-        assertTrue(this.loader.getImporterFactory(this.file).isEmpty());
+    void testMultipleMatchingFactoriesReturnsTopPriority(@Mock ImporterFactory priority1Factory,
+                                                         @Mock ImporterFactory priority2Factory,
+                                                         @Mock ImporterFactory priority3Factory) {
+        when(priority1Factory.supportsFile(same(this.file))).thenReturn(true);
+        when(priority2Factory.supportsFile(same(this.file))).thenReturn(true);
+        when(priority3Factory.supportsFile(same(this.file))).thenReturn(true);
+        when(priority1Factory.getPriority()).thenReturn(1);
+        when(priority2Factory.getPriority()).thenReturn(2);
+        when(priority3Factory.getPriority()).thenReturn(3);
+        simulateFactories(priority2Factory, priority1Factory, priority3Factory);
+        assertThat(this.loader.getImporterFactory(this.file).orElseThrow().getPriority(), equalTo(1));
     }
 
-    private void assertFactoryFound(final ImporterFactory expectedFactory)
-    {
-        assertThat(this.loader.getImporterFactory(this.file).orElseThrow(() ->
-                new AssertionError("Unable to find ImporterFactory.")
-        ), sameInstance(expectedFactory));
+    private void assertFactoryFound(final ImporterFactory expectedFactory) {
+        assertThat(this.loader.getImporterFactory(this.file).orElseThrow(
+                () -> new AssertionError("Unable to find ImporterFactory.")), sameInstance(expectedFactory));
     }
 
-    private void simulateFactories(final ImporterFactory... factories)
-    {
+    private void simulateFactories(final ImporterFactory... factories) {
         when(this.serviceLoaderMock.load()).thenReturn(Arrays.stream(factories));
     }
 }

doc/changes/changes_4.5.0.md

@@ -12,10 +12,13 @@ We also refactored the tests around the CLI starter to improve readability and m
 
 We added FXML to the list of supported file formats for the tag importer.
 
+File extensions can now be handled by multiple importers. Importers have a priority order, the first importer that can handle a file extension will be used. Since the XML importer has a peek function to detect SpecObject files, it can decide not to handle an XML file, in which case the tag importer can take over.
+
 When no importer factory is found for a given file format, the importer factory loader will now throw an exception that explains the likely cause. This is helpful in case OFT is used as a library with a custom classloader and thus does not have the right context.
 
 ## Features
 
+* #352: Tag parsing in XML documents
 * #503: Added `-h` / `--help` to the command line.
 * #506: Exception when no importer was found.
 * #524: Added version number to help text.

doc/spec/design.md

@@ -178,7 +178,51 @@ Depending on the source format, a variety of [importers](#importers) takes care
 
 The listener handles Common parts of the import like filtering out unnecessary items or attributes.
 
-A factory for importers decides which importer to use. Usually, by file extension.
+The following sequence diagram illustrates the interaction between `MultiFileImporter`, `ImporterFactoryLoader`, `ImporterFactory`, and `Importer` during the import process:
+
+```puml
+@startuml
+participant MultiFileImporterImpl as MFI
+participant ImporterFactoryLoader as IFL
+participant ImporterFactory as IF
+participant Importer as I
+participant SpecificationListBuilder as SLB
+
+[-> MFI : importFile(file)
+activate MFI
+MFI -> IFL : getImporterFactory(file)
+activate IFL
+IFL -> IF : supportsFile(file)
+activate IF
+return true
+IFL -> IF : getPriority()
+activate IF
+return priority
+note over IFL: Choose factory with\nlowest priority value
+return factory
+MFI -> IF : createImporter(file, specItemBuilder)
+activate IF
+create I
+IF -> I : new
+return importer
+MFI -> I : runImport()
+activate I
+I -> SLB : event(item)
+activate SLB
+return
+return
+return
+@enduml
+```
+
+A factory for importers decides which importer to use. When multiple importers support the same file, the one with the lowest priority value (highest precedence) is chosen. Usually, importers are selected based on file extension, but some importers may peek into the file content to determine compatibility.
+
+The default priorities for standard importers are:
+1. Markdown Importer: 1000
+2. reStructuredText Importer: 2000
+3. Specobject (ReqM2) Importer: 3000
+4. Tag Importer: 10000
+5. Zip Importer: 20000
 
 ### ReqM2 File Detection
 `dsn~import.reqm2-file-detection~1`
@@ -188,6 +232,8 @@ The `SpecobjectImporterFactory` detects ReqM2 files either
 1. via the file extension `.oreqm` or
 2. via the file extension `.xml` and the presence of the string `<specdocument` within the first 4096 bytes of the file.
 
+Since the Tag Importer also supports `.xml` files but has a higher priority value (10000 vs 3000), `.xml` files containing the `<specdocument` tag will be handled by the Specobject Importer. Only if the file does not contain the tag will it fall back to the Tag Importer.
+
 Covers:
 
 * `req~import.reqm2-file-detection~1`

doc/user_guide.md

@@ -794,6 +794,7 @@ recognized file types:
 
 * HTML (`.html`, `.htm`, `.xhtml`)
 * YAML (`.yaml`, `.yml`)
+* XML (`xml`)
 
 **Modeling languages**
 

importer/markdown/src/main/java/org/itsallcode/openfasttrace/importer/markdown/MarkdownImporterFactory.java

@@ -14,6 +14,12 @@ public MarkdownImporterFactory()
         super("(?i).*\\.markdown", "(?i).*\\.md");
     }
 
+    @Override
+    public int getPriority()
+    {
+        return 1000;
+    }
+
     @Override
     public Importer createImporter(final InputFile fileName, final ImportEventListener listener)
     {

importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporterFactory.java

@@ -17,6 +17,11 @@ protected MarkdownImporterFactory createFactory()
         return new MarkdownImporterFactory();
     }
 
+    @Override
+    protected int getExpectedPriority() {
+        return 1000;
+    }
+
     @Override
     protected List<String> getSupportedFilenames()
     {

importer/restructuredtext/src/main/java/org/itsallcode/openfasttrace/importer/restructuredtext/RestructuredTextImporterFactory.java

@@ -14,6 +14,11 @@ public RestructuredTextImporterFactory()
         super("(?i).*\\.rst");
     }
 
+    @Override
+    public int getPriority() {
+        return 2000;
+    }
+
     @Override
     public Importer createImporter(final InputFile fileName, final ImportEventListener listener)
     {

importer/restructuredtext/src/test/java/org/itsallcode/openfasttrace/importer/restructuredtext/TestRestructuredTextImporterFactory.java

@@ -17,6 +17,11 @@ protected RestructuredTextImporterFactory createFactory()
         return new RestructuredTextImporterFactory();
     }
 
+    @Override
+    protected int getExpectedPriority() {
+        return 2000;
+    }
+
     @Override
     protected List<String> getSupportedFilenames()
     {

importer/specobject/src/main/java/org/itsallcode/openfasttrace/importer/specobject/SpecobjectImporterFactory.java

@@ -27,6 +27,11 @@ public SpecobjectImporterFactory()
         this.xmlParserFactory = new XmlParserFactory();
     }
 
+    @Override
+    public int getPriority() {
+        return 3000;
+    }
+
     // [impl -> dsn~import.reqm2-file-detection~1]
     @Override
     public boolean supportsFile(final InputFile file)

importer/specobject/src/test/java/org/itsallcode/openfasttrace/importer/specobject/TestSpecobjectImporterFactory.java

@@ -23,13 +23,17 @@
 class TestSpecobjectImporterFactory
         extends ImporterFactoryTestBase<SpecobjectImporterFactory>
 {
-
     @Override
     protected SpecobjectImporterFactory createFactory()
     {
         return new SpecobjectImporterFactory();
     }
 
+    @Override
+    protected int getExpectedPriority() {
+        return 3000;
+    }
+
     /**
      * Only the {@code .oreqm} extension is always supported. That is why we
      * can't simply list {@code .xml} here. Whether {@code .xml} is supported

importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporterFactory.java

@@ -23,7 +23,7 @@ public class TagImporterFactory extends ImporterFactory
             "feature", // Gherkin feature files
             "go", // Go
             "groovy", // Groovy
-            "json", "htm", "html", "xhtml", "yaml", "yml", // markup languages
+            "json", "htm", "html", "xhtml", "xml", "yaml", "yml", // markup languages
             "fxml", "java", // Java
             "clj", "kt", "kts", "scala", // JVM languages
             "js", "mjs", "cjs", "ejs", // JavaScript
@@ -55,6 +55,12 @@ public TagImporterFactory()
         // empty by intention
     }
 
+    @Override
+    public int getPriority() {
+        return 10000;
+    }
+
+
     @Override
     public boolean supportsFile(final InputFile path)
     {