#366: Allow all unicode characters in names of specification ID

api/src/main/java/org/itsallcode/openfasttrace/api/core/SpecificationItemId.java

@@ -21,15 +21,15 @@ public class SpecificationItemId implements Comparable<SpecificationItemId>
     public static final String UNKNOWN_ARTIFACT_TYPE = "unknown";
     private static final String ITEM_REVISION_PATTERN = "(\\d+)";
     /** Regexp pattern for item names. */
-    public static final String ITEM_NAME_PATTERN = "(\\p{Alpha}[\\w-]*(?:\\.\\p{Alpha}[\\w-]*)*+)";
+    public static final String ITEM_NAME_PATTERN = "(?U)(\\p{Alpha}[\\w-]*(?:\\.\\p{Alpha}[\\w-]*)*+)";
     private static final String LEGACY_ID_NAME = "(\\p{Alpha}+)(?:~\\p{Alpha}+)?:"
             + ITEM_NAME_PATTERN;
     /** Separator between artifact type and name in an item ID. */
     public static final String ARTIFACT_TYPE_SEPARATOR = "~";
     /** Separator between name and revision in an item ID. */
     public static final String REVISION_SEPARATOR = "~";
     static final int REVISION_WILDCARD = Integer.MIN_VALUE;
-    // [impl->dsn~md.specification-item-id-format~2]
+    // [impl->dsn~md.specification-item-id-format~3]
     private static final String ID = "(\\p{Alpha}+)" //
             + ARTIFACT_TYPE_SEPARATOR //
             + ITEM_NAME_PATTERN //
@@ -213,7 +213,7 @@ public static SpecificationItemId createId(final String artifactType, final Stri
      */
     public static class Builder
     {
-        // [impl->dsn~md.specification-item-id-format~2]
+        // [impl->dsn~md.specification-item-id-format~3]
         private final String id;
         private String artifactType;
         private String name;

api/src/test/java/org/itsallcode/openfasttrace/api/core/SpecificationItemIdTest.java

@@ -22,7 +22,8 @@ class SpecificationItemIdTest
             "type~name.with.dot~42, type, name.with.dot, 42",
             "type~name-trailing-~42, type, name-trailing-, 42",
     })
-    void parsingValidIdsSucceeds(String id, String expectedType, String expectedName, int expectedRevision)
+    void parsingValidIdsSucceeds(final String id, final String expectedType, final String expectedName,
+            final int expectedRevision)
     {
         final SpecificationItemId parsedId = parseId(id);
         assertAll(
@@ -41,13 +42,12 @@ void parsingValidIdsSucceeds(String id, String expectedType, String expectedName
             "type~name-with-trailing-dot.~42",
             "type~name~rev",
             "typeÄ~name~42",
-            "type~nameÄ~42",
             "type~na.-me~42",
             "~name~42",
             "type~~42",
             "type~name~"
     })
-    void parsingIllegalIdsFails(String id)
+    void parsingIllegalIdsFails(final String id)
     {
         final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> parseId(id));
         assertThat(exception.getMessage(),
@@ -74,7 +74,7 @@ void toRevisionWildcard()
         assertThat(id.toString(), equalTo("type~name~" + SpecificationItemId.REVISION_WILDCARD));
     }
 
-    private SpecificationItemId parseId(String id)
+    private SpecificationItemId parseId(final String id)
     {
         return new SpecificationItemId.Builder(id).build();
     }

api/src/test/java/org/itsallcode/openfasttrace/api/core/TestSpecificationItemId.java

@@ -1,20 +1,20 @@
 package org.itsallcode.openfasttrace.api.core;
 
 import static org.hamcrest.MatcherAssert.assertThat;
-
 import static org.hamcrest.Matchers.equalTo;
 import static org.itsallcode.openfasttrace.api.core.SpecificationItemId.createId;
 import static org.itsallcode.openfasttrace.api.core.SpecificationItemId.parseId;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.fail;
 
 import org.itsallcode.openfasttrace.api.core.SpecificationItemId.Builder;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 import nl.jqno.equalsverifier.EqualsVerifier;
 
 /**
- * [utest->dsn~md.specification-item-id-format~2]
+ * [utest->dsn~md.specification-item-id-format~3]
  */
 // [utest->dsn~specification-item-id~1]
 class TestSpecificationItemId
@@ -48,6 +48,7 @@ void testParseId_singleDigitRevision()
         assertThat(id.getRevision(), equalTo(1));
     }
 
+
     @Test
     void testParseId_multipleFragmentName()
     {
@@ -57,6 +58,15 @@ void testParseId_multipleFragmentName()
         assertThat(id.getRevision(), equalTo(1));
     }
 
+    @Test
+    void testParseId_umlautName()
+    {
+        final SpecificationItemId id = parseId("feat~änderung~1");
+        assertThat(id.getArtifactType(), equalTo(ARTIFACT_TYPE_FEATURE));
+        assertThat(id.getName(), equalTo("änderung"));
+        assertThat(id.getRevision(), equalTo(1));
+    }
+
     @Test
     void testParseId_multipleDigitRevision()
     {
@@ -69,42 +79,26 @@ void testParseId_multipleDigitRevision()
     @Test
     void testParseId_IllegalNumberFormat()
     {
-        assertThrows(IllegalArgumentException.class,
+        final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class,
                 () -> parseId("feat~foo~999999999999999999999999999999999999999"));
+        assertThat(exception.getMessage(), equalTo(
+                "Error parsing version number from specification item ID: \"feat~foo~999999999999999999999999999999999999999\""));
     }
 
-    @Test
-    void testParseIdFailsForWildcardRevision()
+    @ParameterizedTest
+    @CsvSource(
+    { "feat.foo~1", "foo~1", "req~foo", "req1~foo~1", "req.r~foo~1", "req~1foo~1", "req~.foo~1", "req~foo~-1",
+            // Wildcard revision:
+            "feat~foo~-2147483648" })
+    void testParseId_mustFailForIllegalIds(final String illegalId)
     {
-        assertThrows(IllegalStateException.class,
-                () -> parseId("feat~foo~" + SpecificationItemId.REVISION_WILDCARD));
-    }
 
-    @Test
-    void testParseId_mustFailForIllegalIds()
-    {
-        final String[] negatives = { "feat.foo~1", "foo~1", "req~foo", "req1~foo~1", "req.r~foo~1",
-                "req~1foo~1", "req~.foo~1", "req~foo~-1" };
-
-        for (final String sample : negatives)
-        {
-            assertParsingExceptionOnIllegalSpecificationItemId(sample);
-        }
+        final IllegalStateException exception = assertThrows(IllegalStateException.class,
+                () -> parseId(illegalId));
+        assertThat(exception.getMessage(),
+                equalTo("String \"" + illegalId + "\" cannot be parsed to a specification item ID"));
     }
 
-    private void assertParsingExceptionOnIllegalSpecificationItemId(final String sample)
-    {
-        try
-        {
-            parseId(sample);
-            fail("Expected exception trying to parse \"" + sample
-                    + "\" into a specification item ID");
-        }
-        catch (final IllegalStateException exception)
-        {
-            // this block intentionally left empty
-        }
-    }
 
     @Test
     void testToRevisionWildcard()

doc/changes/changes_3.7.2.md

@@ -9,3 +9,4 @@ This release fixes parsing of `Needs` and `Tags` entries in Markdown. OFT now ig
 ## Bugfixes
 
 * #373: Ignore spaces after items in "Needs:" and "Tags:" lists (thanks to [@sambishop](https://github.com/sambishop) for his contribution!)
+* #366: Allow all unicode characters in names of specification ID names (thanks to [@sebastianohl](https://github.com/sebastianohl) for the bug report!)

doc/spec/design.md

@@ -504,7 +504,7 @@ Needs: impl, utest
 ### Markdown-style Structures
 
 #### Markdown Specification Item ID Format
-`dsn~md.specification-item-id-format~2`
+`dsn~md.specification-item-id-format~3`
 
 A requirement ID has the following format
 
@@ -514,14 +514,15 @@ A requirement ID has the following format
 
     id = id-fragment *("." id-fragment)
 
-    id-fragment = ALPHA *(ALPHA / DIGIT / "_" / "-")
+    id-fragment = UNICODE_ALPHA *(UNICODE_ALPHA / DIGIT / "_" / "-")
 
     revision = 1*DIGIT
 
 Rationale:
 
-The ID must only contain characters that can be used in URIs without quoting. This makes linking in formats like Markdown or HTML clean and easy.
-Requirement type and revision must be immediately recognizable from the requirement ID. The built-in revision number makes links break if a requirement is updated - a desired behavior.
+* The ID may contain unicode letters to allow naming requirements using non-ASCII characters. This makes linking in formats like Markdown or HTML clean and easy.
+* Requirement type and revision must be immediately recognizable from the requirement ID.
+* The built-in revision number makes links break if a requirement is updated - a desired behavior.
 
 Comment:
 
@@ -687,15 +688,15 @@ Alternatively a Markdown requirement ID can have the following format
 
     requirement-id = *1(type~)type ":" id "," *WSP "v" revision
 
-See `dsn~md.specification-item-id-format~2` for definitions of the ABNF sub-rules referred to here.
+See [`dsn~md.specification-item-id-format~3`](#markdown-specification-item-id-format) for definitions of the ABNF sub-rules referred to here.
 
 Rationale:
 
 This ID format is supported for backwards compatibility with Elektrobit's legacy requirement-enhanced Markdown format.
 
 Comment:
 
-This format is deprecated. Please use the one specified in `dsn~md.specification-item-id-format~2` for new documents.
+This format is deprecated. Please use the one specified in [`dsn~md.specification-item-id-format~3`](#markdown-specification-item-id-format) for new documents.
 
 Covers:
 

doc/user_guide.md

@@ -92,7 +92,7 @@ Examples:
     dsn~html5-exporter~1
     utest~html5-exporter~4
 
-The name part of the ID must be a character string consisting of ASCII letters and or numbers separated by underscore ("_"), hyphen ("-") or dot ("."). Whitespaces are not allowed.
+The name part of the ID must be a character string consisting of Unicode letters and or numbers separated by underscore ("_"), hyphen ("-") or dot ("."). Whitespaces are not allowed.
 
 The revision number is a positive integer number that can be started at zero but out of convention usually is started at one.
 

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

@@ -22,7 +22,6 @@
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 
-
 class ITMarkdownImporter
 {
     private static final String NL = System.lineSeparator();
@@ -240,4 +239,20 @@ static Stream<Arguments> tags()
                 Arguments.of("Tags:\n* req \n\t* dsn ", List.of("req", "dsn")),
                 Arguments.of("Tags:\n* req\n* dsn", List.of("req", "dsn")));
     }
+
+    @Test
+    void testItemIdSupportsUmlauts()
+    {
+        final List<SpecificationItem> items = runImporterOnText(
+                "`### Die Implementierung muss den Zustand einzelner Zellen ändern.\n"
+                        + "`req~zellzustandsänderung~1`\n"
+                        + "Diese Anforderung ermöglicht die Aktualisierung des Zustands von lebenden und toten Zellen in jeder Generation.\n"
+                        + "Needs: arch");
+        assertThat(items, AutoMatcher.contains(SpecificationItem.builder()
+                .id(SpecificationItemId.createId("req", "zellzustandsänderung", 1))
+                .description(
+                        "Diese Anforderung ermöglicht die Aktualisierung des Zustands von lebenden und toten Zellen in jeder Generation.")
+                .location("file name", 2).addNeedsArtifactType("arch")
+                .build()));
+    }
 }

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

@@ -3,7 +3,6 @@
 import static org.itsallcode.openfasttrace.importer.markdown.MarkdownAsserts.assertMatch;
 import static org.itsallcode.openfasttrace.importer.markdown.MarkdownAsserts.assertMismatch;
 import static org.itsallcode.openfasttrace.importer.markdown.MarkdownTestConstants.*;
-import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.mockito.Mockito.*;
 
 import java.io.*;
@@ -17,6 +16,8 @@
 import org.itsallcode.openfasttrace.testutil.importer.input.StreamInput;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -34,37 +35,77 @@ class TestMarkdownImporter
     @Mock
     Reader readerMock;
 
-    // [utest~md.specification_item_id_format~1]
-    @Test
-    void testIdentifyId()
+    // [utest->dsn~md.specification-item-id-format~3]
+    @ParameterizedTest
+    @CsvSource(
+    { "req~foo~1<a id=\"req~foo~1\"></a>", "a~b~0", "req~test~1",
+            "req~test~999", "req~test.requirement~1", "req~test_underscore~1",
+            "`req~test1~1`arbitrary text",
+            // See https://github.com/itsallcode/openfasttrace/issues/366
+            "req~zellzustandsänderung~1", "req~öäüßÖÄ~1"
+    })
+    void testIdentifyId(final String text)
     {
-        assertAll(
-                () -> assertMatch(MdPattern.ID, "req~foo~1<a id=\"req~foo~1\"></a>", "a~b~0", "req~test~1",
-                        "req~test~999", "req~test.requirement~1", "req~test_underscore~1",
-                        "`req~test1~1`arbitrary text"),
-                () -> assertMismatch(MdPattern.ID, "test~1", "req-test~1", "req~4test~1"));
+        assertMatch(MdPattern.ID, text);
+    }
+
+    // [utest->dsn~md.specification-item-id-format~3]
+    @ParameterizedTest
+    @CsvSource(
+    { "test~1", "req-test~1", "req~4test~1", "räq~test~1" })
+    void testIdentifyNonId(final String text)
+    {
+        assertMismatch(MdPattern.ID, text);
     }
 
     // [utest->dsn~md.specification-item-title~1]
-    @Test
-    void testIdentifyTitle()
+    @ParameterizedTest
+    @CsvSource(
+    { "#Title", "# Title", "###### Title", "#   Title", "# Änderung" })
+    void testIdentifyTitle(final String text)
     {
-        assertAll(() -> assertMatch(MdPattern.TITLE, "#Title", "# Title", "###### Title", "#   Title"),
-                () -> assertMismatch(MdPattern.TITLE, "Title", "Title #", " # Title"));
+        assertMatch(MdPattern.TITLE, text);
     }
 
-    @Test
-    void testIdentifyNeeds()
+    // [utest->dsn~md.specification-item-title~1]
+    @ParameterizedTest
+    @CsvSource(
+    { "Title", "Title #", "' # Title'" })
+    void testIdentifyNonTitle(final String text)
     {
-        assertAll(() -> assertMatch(MdPattern.NEEDS_INT, "Needs: req, dsn", "Needs:req,dsn", "Needs:  \treq , dsn "),
-                () -> assertMismatch(MdPattern.NEEDS_INT, "Needs:"));
+        assertMismatch(MdPattern.TITLE, text);
     }
 
-    @Test
-    void testIdentifyTags()
+    @ParameterizedTest
+    @CsvSource(
+    { "Needs: req, dsn", "Needs:req,dsn", "'Needs:  \treq , dsn '" })
+    void testIdentifyNeeds(final String text)
+    {
+        assertMatch(MdPattern.NEEDS_INT, text);
+    }
+
+    @ParameterizedTest
+    @CsvSource(
+    { "Needs:", "#Needs: abc", "' Needs: abc'", "Needs: önderung" })
+    void testIdentifyNonNeeds(final String text)
+    {
+        assertMismatch(MdPattern.NEEDS_INT, text);
+    }
+
+    @ParameterizedTest
+    @CsvSource(
+    { "Tags: req, dsn", "Tags:req,dsn", "'Tags:  \treq , dsn '" })
+    void testIdentifyTags(final String text)
+    {
+        assertMatch(MdPattern.TAGS_INT, text);
+    }
+
+    @ParameterizedTest
+    @CsvSource(
+    { "Tags:", "#Needs: abc", "' Needs: abc'", "Needs: änderung" })
+    void testIdentifyNonTags(final String text)
     {
-        assertAll(() -> assertMatch(MdPattern.TAGS_INT, "Tags: req, dsn", "Tags:req,dsn", "Tags:  \treq , dsn "),
-                () -> assertMismatch(MdPattern.TAGS_INT, "Tags:"));
+        assertMismatch(MdPattern.TAGS_INT, text);
     }
 
     @Test