diff --git a/.agents/skills/openfasttrace/SKILL.md b/.agents/skills/openfasttrace/SKILL.md index c90c9512..68debdbc 100644 --- a/.agents/skills/openfasttrace/SKILL.md +++ b/.agents/skills/openfasttrace/SKILL.md @@ -60,7 +60,13 @@ Coverage in a YMAL file (e.g., GitHub workflow) Require coverage: ```plantuml -# [req -> dsn~hash-sum-calculation~1 >> impl, utest] +' [req -> dsn~hash-sum-calculation~1 >> impl, utest] +``` + +Multiple coverage: + +```Java +// [dsn -> req~local-stability~1,arch~dimensional-input~1] ``` ## Tracing diff --git a/doc/changes/changes_4.6.0.md b/doc/changes/changes_4.6.0.md index f9f1b6bc..bf3125cc 100644 --- a/doc/changes/changes_4.6.0.md +++ b/doc/changes/changes_4.6.0.md @@ -18,3 +18,7 @@ We moved some GitHub action permissions from workflow-level to job-level. * #546: Replaced `OsDetector` with JUnit5's `EnabledOnOs` annotation. * #544: Replaced optional parameter with null check * #543: Made `CliException` a `RuntimeException` + +## Features + +* #553 Tag importer supports multiple covered ids diff --git a/doc/spec/design.md b/doc/spec/design.md index 237ab4e5..ad2b4767 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -889,6 +889,24 @@ Covers: Needs: impl, utest +#### Full Coverage Tag Format Allows Multiple Coverage +`dsn~import.full-coverage-tag-multiple-needed-coverage~1` + +OFT imports full coverage tags with multiple need coverage ids: + + full-tag-multiple-coverage-id = + "[" *WSP reference *WSP "->" *WSP requirement-id *WSP *("," *WSP requirement-id) *WSP "]" + +Rationale: + +An item can cover multiple IDs. This avoids creating multiple IDs for the same item solely to represent multiple coverage relations. It also reduces the number of IDs that related items must reference for complete coverage, improving readability and maintainability. + +Covers: + +* `req~import.full-coverage-tag-format~1` + +Needs: impl, utest + #### Full Coverage Tag Format Allows Specifying a Revision `dsn~import.full-coverage-tag-with-revision~1` diff --git a/doc/user_guide.md b/doc/user_guide.md index a8b35e47..97db8133 100644 --- a/doc/user_guide.md +++ b/doc/user_guide.md @@ -708,7 +708,7 @@ To avoid conflict with the formats actual contents, you embed these definitions Tags have the following format: ``` -[ -> ] +[ -> ] ``` Spaces above were only added for readability. They are optional. In fact usually people prefer a more compact form. @@ -737,7 +737,7 @@ Examples: // [impl~validate-password~2->dsn~validate-authentication-request~1] ``` -##### Forwarding Requirements +##### Needed Coverage When using UML models as design document files like UML models it is useful to add needed coverage as well. To do this, you can use the following format: diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java index 12ba0865..268d47d7 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java +++ b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java @@ -2,7 +2,9 @@ import static java.util.Collections.emptyList; -import java.util.*; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; import java.util.function.Predicate; import java.util.logging.Logger; import java.util.regex.Matcher; @@ -23,18 +25,21 @@ class LongTagImportingLineConsumer extends AbstractRegexLineConsumer private static final String OPTIONAL_WHITESPACE = "\\s*"; private static final String TAG_PREFIX = "\\["; private static final String TAG_SUFFIX = "\\]"; - private static final String NEEDS_COVERAGE = ">>" + OPTIONAL_WHITESPACE + "(\\p{Alpha}+(?:" - + OPTIONAL_WHITESPACE - + "," + OPTIONAL_WHITESPACE + "\\p{Alpha}+)*)"; + private static final String COVERED_IDS = SpecificationItemId.ID_PATTERN + "(?:" + + OPTIONAL_WHITESPACE + "," + OPTIONAL_WHITESPACE + SpecificationItemId.ID_PATTERN + + ")*"; + private static final String NEEDS_COVERAGE = ">>" + OPTIONAL_WHITESPACE + + "(?\\p{Alpha}+(?:" + OPTIONAL_WHITESPACE + "," + + OPTIONAL_WHITESPACE + "\\p{Alpha}+)*)"; private static final String TAG_REGEX = TAG_PREFIX + OPTIONAL_WHITESPACE// - + "(" + COVERING_ARTIFACT_TYPE_PATTERN + ")" + + "(?" + COVERING_ARTIFACT_TYPE_PATTERN + ")" + "(?:" + SpecificationItemId.ARTIFACT_TYPE_SEPARATOR // [impl->dsn~import.full-coverage-tag-with-name-and-revision~1] - + "(" + SpecificationItemId.ITEM_NAME_PATTERN + ")?" + + "(?" + SpecificationItemId.ITEM_NAME_PATTERN + ")?" + SpecificationItemId.REVISION_SEPARATOR - + SpecificationItemId.ITEM_REVISION_PATTERN + ")?" // + + "(?" + SpecificationItemId.ITEM_REVISION_PATTERN + "))?" // + OPTIONAL_WHITESPACE + "->" + OPTIONAL_WHITESPACE // - + "(" + SpecificationItemId.ID_PATTERN + ")" // + + "(?" + COVERED_IDS + ")" // + OPTIONAL_WHITESPACE + "(?:" + NEEDS_COVERAGE + OPTIONAL_WHITESPACE + ")?" // + TAG_SUFFIX; @@ -53,18 +58,32 @@ public void processMatch(final Matcher matcher, final int lineNumber, final int { this.listener.beginSpecificationItem(); this.listener.setLocation(this.file.getPath(), lineNumber); - final SpecificationItemId coveredId = SpecificationItemId.parseId(matcher.group(5)); - final List neededArtifactTypes = parseCommaSeparatedList(matcher.group(9)); - final SpecificationItemId generatedId = createItemId(matcher, lineNumber, lineMatchCount, coveredId, - neededArtifactTypes); - logItem(lineNumber, coveredId, neededArtifactTypes, generatedId); + final List coveredIds = parseCoveredIds(matcher.group("coveredIds")); + final List neededArtifactTypes = parseNeededArtifactTypes(matcher.group("neededArtifactTypes")); + final SpecificationItemId generatedId = createItemId(matcher, lineNumber, lineMatchCount, + coveredIds, neededArtifactTypes); + logItem(lineNumber, coveredIds, neededArtifactTypes, generatedId); this.listener.setId(generatedId); - this.listener.addCoveredId(coveredId); - neededArtifactTypes.forEach(listener::addNeededArtifactType); + coveredIds.forEach(this.listener::addCoveredId); + neededArtifactTypes.forEach(this.listener::addNeededArtifactType); this.listener.endSpecificationItem(); } - private static List parseCommaSeparatedList(final String input) + private static List parseCoveredIds(final String input) + { + if (input == null) + { + return emptyList(); + } + + return Arrays.stream(input.split(",")) + .map(String::trim) + .filter(Predicate.not(String::isEmpty)) + .map(SpecificationItemId::parseId) + .toList(); + } + + private static List parseNeededArtifactTypes(final String input) { if (input == null) { @@ -78,28 +97,28 @@ private static List parseCommaSeparatedList(final String input) } private SpecificationItemId createItemId(final Matcher matcher, final int lineNumber, final int lineMatchCount, - final SpecificationItemId coveredId, final List neededArtifactTypes) + final List coveredIds, final List neededArtifactTypes) { - final String artifactType = matcher.group(1); - final String customName = matcher.group(2); - final String revision = matcher.group(4); + final String artifactType = matcher.group("artifactType"); + final String customName = matcher.group("customName"); + final String revision = matcher.group("revision"); final String name = customName != null ? customName - : getItemName(lineNumber, lineMatchCount, coveredId, neededArtifactTypes); + : getItemName(lineNumber, lineMatchCount, coveredIds, neededArtifactTypes); return SpecificationItemId.createId(artifactType, name, parseRevision(revision)); } - private void logItem(final int lineNumber, final SpecificationItemId coveredId, + private void logItem(final int lineNumber, final List coveredIds, final List neededArtifactTypes, final SpecificationItemId generatedId) { if (neededArtifactTypes.isEmpty()) { LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + generatedId - + "' covering id '" + coveredId); + + "' covering ids " + coveredIds); } else { LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + generatedId - + "' covering id '" + coveredId + "', needs artifact types " + + "' covering ids " + coveredIds + ", needs artifact types " + neededArtifactTypes); } } @@ -112,21 +131,32 @@ private static int parseRevision(final String revision) } // [impl->dsn~import.full-coverage-tag-with-needed-coverage-readable-names~1] - private String getItemName(final int lineNumber, final int lineMatchCount, final SpecificationItemId coveredId, - final List neededArtifactTypes) + private String getItemName(final int lineNumber, final int lineMatchCount, + final List coveredIds, final List neededArtifactTypes) { if (neededArtifactTypes.isEmpty()) { - return generateUniqueName(coveredId, lineNumber, lineMatchCount); + return generateUniqueName(coveredIds, lineNumber, lineMatchCount); } - return coveredId.getName(); + return joinCoveredIdNamesWithHyphen(coveredIds); } - private String generateUniqueName(final SpecificationItemId coveredId, final int lineNumber, + private String generateUniqueName(final List coveredIds, final int lineNumber, final int counter) { - final String uniqueName = this.file.getPath() + lineNumber + counter + coveredId; + final String CoveredIdStringsJoinedWithHyphen = coveredIds.stream() + .map(SpecificationItemId::toString) + .collect(java.util.stream.Collectors.joining("-")); + final String uniqueName = this.file.getPath() + lineNumber + counter + + CoveredIdStringsJoinedWithHyphen; final String checksum = Long.toString(ChecksumCalculator.calculateCrc32(uniqueName)); - return coveredId.getName() + "-" + checksum; + return joinCoveredIdNamesWithHyphen(coveredIds) + "-" + checksum; + } + + private static String joinCoveredIdNamesWithHyphen(final List coveredIds) + { + return coveredIds.stream() + .map(SpecificationItemId::getName) + .collect(java.util.stream.Collectors.joining("-")); } } diff --git a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java b/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java index f7017f6c..061e93bf 100644 --- a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java +++ b/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java @@ -91,6 +91,29 @@ static Stream tagImporterTests() itemACoveringB("implA~name1-2943155783~0", "dsn~name1~2"), itemACoveringB("implB~name2-1099447527~0", "dsn~name2~3"), itemACoveringB("implC~name3-2846888323~0", "dsn~name3~4")), + parsedItem("[impl~combined~1->dsn~name1~2,dsn~name2~3]", + itemBuilder().id(SpecificationItemId.parseId("impl~combined~1")) + .addCoveredId(SpecificationItemId.parseId("dsn~name1~2")) + .addCoveredId(SpecificationItemId.parseId("dsn~name2~3"))), + parsedItem("[ impl~combined~1 -> dsn~name1~2 , dsn~name2~3 >> test ]", + itemBuilder().id(SpecificationItemId.parseId("impl~combined~1")) + .addCoveredId(SpecificationItemId.parseId("dsn~name1~2")) + .addCoveredId(SpecificationItemId.parseId("dsn~name2~3")) + .addNeedsArtifactType("test")), + parsedItem("[ impl~combined~1 -> dsn~name1~2 , dsn~name2~3 >> utest,itest ]", + itemBuilder().id(SpecificationItemId.parseId("impl~combined~1")) + .addCoveredId(SpecificationItemId.parseId("dsn~name1~2")) + .addCoveredId(SpecificationItemId.parseId("dsn~name2~3")) + .addNeedsArtifactType("utest") + .addNeedsArtifactType("itest")), + parsedItem("[ impl~~1 -> dsn~name1~2 , dsn~name2~3 ]", + itemBuilder().id(SpecificationItemId.parseId("impl~name1-name2-506723840~1")) + .addCoveredId(SpecificationItemId.parseId("dsn~name1~2")) + .addCoveredId(SpecificationItemId.parseId("dsn~name2~3"))), + parsedItem("[ impl -> dsn~name1~2 , dsn~name2~3 ]", + itemBuilder().id(SpecificationItemId.parseId("impl~name1-name2-506723840~0")) + .addCoveredId(SpecificationItemId.parseId("dsn~name1~2")) + .addCoveredId(SpecificationItemId.parseId("dsn~name2~3"))), parsedItems("[implA->dsn~name1~2" + "]" + UNIX_NEWLINE + "[implB->dsn~name2~3" + "]", itemACoveringB("implA~name1-2943155783~0", "dsn~name1~2"), @@ -163,6 +186,7 @@ static Stream tagImporterTests() noItemDetected("[impl~missing-revision~->dsn~name2~2]"), noItemDetected("[impl~illegal?char~1->dsn~name2~2]"), noItemDetected("[impl~negative-revision~-1->dsn~name2~2]"), + noItemDetected("[impl~missing-covered-id~1->dsn~name2~2,]"), noItemDetected("[impl~missing-forward~1->dsn~name2~2>>]"), noItemDetected("[impl~trailing-comma~1->dsn~name2~2>>test,]"), noItemDetected("[impl~duplicate-comma~1->dsn~name2~2>>test,,other]"),