From 91e1cd9731108b90855b8922c4895b43363d12cf Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Thu, 3 Sep 2026 16:24:53 -0400 Subject: [PATCH 1/4] fix: ignore malformed published check versions --- .../controller/EligibilityCheckResource.java | 22 +++++++++++-------- .../EligibilityCheckResourceTest.java | 17 ++++++++++++++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java index f57bc902..440b2006 100644 --- a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java +++ b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java @@ -25,7 +25,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; @Path("/api/custom-checks") @@ -454,8 +453,8 @@ public Response getPublishedVersionsOfWorkingCheck(@Context SecurityIdentity ide String versionForPublish(String workingVersion, List publishedChecks) { return publishedChecks.stream() .map(EligibilityCheck::getVersion) - .filter(Objects::nonNull) - .map(this::normalize) + .filter(version -> version != null) + .flatMap(version -> normalize(version).stream()) .max(VERSION_ORDER) .map(this::incrementMajorVersion) .orElse(workingVersion); @@ -465,13 +464,18 @@ private String incrementMajorVersion(int[] version) { return (version[0] + 1) + ".0.0"; // increment major, reset minor and patch } - private int[] normalize(String version) { - String[] parts = version.split("\\."); - int[] nums = new int[]{0, 0, 0}; + private Optional normalize(String version) { + try { + String[] parts = version.split("\\."); + int[] nums = new int[]{0, 0, 0}; - for (int i = 0; i < parts.length && i < 3; i++) { - nums[i] = Integer.parseInt(parts[i]); + for (int i = 0; i < parts.length && i < 3; i++) { + nums[i] = Integer.parseInt(parts[i]); + } + return Optional.of(nums); + } catch (NumberFormatException e) { + Log.warn("Ignoring malformed published check version: " + version); + return Optional.empty(); } - return nums; } } diff --git a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java index ef9f8c82..5560c7b1 100644 --- a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java +++ b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java @@ -218,6 +218,23 @@ void staleWorkingVersionDoesNotReusePublishedVersion() throws Exception { assertEquals("3.0.0", capturedPublishedVersion()); } + @Test + void malformedPublishedVersionsAreIgnored() throws Exception { + workingCheck.setVersion("2.0.0"); + when(repository.getPublishedCheckVersions(workingCheck)) + .thenReturn(List.of( + publishedVersion(""), + publishedVersion("not-a-version"), + publishedVersion("1.invalid.0"), + publishedVersion("2.0.0") + )); + + Response response = resource.publishCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals("3.0.0", capturedPublishedVersion()); + } + // An unreadable version list must not be mistaken for "never published" @Test void publishFailsWhenPublishedVersionsCannotBeRead() throws Exception { From 2646f48de4606eb304fe97ab26ce03fa55ce0bac Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Thu, 3 Sep 2026 17:22:52 -0400 Subject: [PATCH 2/4] fix: keep a malformed version from breaking the check list The published check list parsed stored versions with a bare Integer.parseInt, so a single corrupt version field made GET /api/custom-checks fail with a 500 for every check the user owns, with no way to recover through the API. Move version parsing into CheckVersion, shared by the publish path and the read path, and order unreadable versions below readable ones instead of throwing out of the comparison. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019VpGMR8VeD27SgrfNeWiUr --- .../controller/EligibilityCheckResource.java | 27 +++------- .../org/acme/model/domain/CheckVersion.java | 48 +++++++++++++++++ .../impl/EligibilityCheckRepositoryImpl.java | 25 +-------- .../acme/model/domain/CheckVersionTest.java | 53 +++++++++++++++++++ 4 files changed, 110 insertions(+), 43 deletions(-) create mode 100644 builder-api/src/main/java/org/acme/model/domain/CheckVersion.java create mode 100644 builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java diff --git a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java index 440b2006..2741949f 100644 --- a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java +++ b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java @@ -11,6 +11,7 @@ import com.fasterxml.jackson.databind.JsonNode; import org.acme.auth.AuthUtils; import org.acme.constants.CheckStatus; +import org.acme.model.domain.CheckVersion; import org.acme.model.domain.EligibilityCheck; import org.acme.model.dto.EligibilityCheck.CheckDmnRequest; import org.acme.model.dto.EligibilityCheck.CreateCheckRequest; @@ -21,7 +22,6 @@ import org.acme.service.CustomCheckDmnTemplate; import org.acme.service.DmnService; -import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -443,19 +443,13 @@ public Response getPublishedVersionsOfWorkingCheck(@Context SecurityIdentity ide // ========== Private Helper Methods ========== - private static final Comparator VERSION_ORDER = Comparator - .comparingInt(v -> v[0]) - .thenComparingInt(v -> v[1]) - .thenComparingInt(v -> v[2]); - /* The first publish keeps the working version; later ones increment past the highest published version, so a working version that lags what is already published cannot produce a duplicate published id. */ String versionForPublish(String workingVersion, List publishedChecks) { return publishedChecks.stream() .map(EligibilityCheck::getVersion) - .filter(version -> version != null) - .flatMap(version -> normalize(version).stream()) - .max(VERSION_ORDER) + .flatMap(version -> parsePublishedVersion(version).stream()) + .max(CheckVersion.ORDER) .map(this::incrementMajorVersion) .orElse(workingVersion); } @@ -464,18 +458,11 @@ private String incrementMajorVersion(int[] version) { return (version[0] + 1) + ".0.0"; // increment major, reset minor and patch } - private Optional normalize(String version) { - try { - String[] parts = version.split("\\."); - int[] nums = new int[]{0, 0, 0}; - - for (int i = 0; i < parts.length && i < 3; i++) { - nums[i] = Integer.parseInt(parts[i]); - } - return Optional.of(nums); - } catch (NumberFormatException e) { + private Optional parsePublishedVersion(String version) { + Optional parsed = CheckVersion.parse(version); + if (parsed.isEmpty()) { Log.warn("Ignoring malformed published check version: " + version); - return Optional.empty(); } + return parsed; } } diff --git a/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java b/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java new file mode 100644 index 00000000..edc92873 --- /dev/null +++ b/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java @@ -0,0 +1,48 @@ +package org.acme.model.domain; + +import java.util.Comparator; +import java.util.Optional; + +/* Check versions are stored as free-form strings, so a stored version can be anything at all. + Parsing yields an empty result for values that are not versions, which lets callers ignore + corrupt data instead of failing every later read and publish of the check. */ +public final class CheckVersion { + + private static final int PART_COUNT = 3; + + public static final Comparator ORDER = Comparator + .comparingInt(version -> version[0]) + .thenComparingInt(version -> version[1]) + .thenComparingInt(version -> version[2]); + + private CheckVersion() { + } + + /* A version is dot-separated numbers; missing parts count as zero. */ + public static Optional parse(String version) { + if (version == null) { + return Optional.empty(); + } + + String[] parts = version.split("\\."); + int[] numbers = new int[PART_COUNT]; + for (int i = 0; i < parts.length && i < PART_COUNT; i++) { + try { + numbers[i] = Integer.parseInt(parts[i]); + } catch (NumberFormatException e) { + return Optional.empty(); + } + } + return Optional.of(numbers); + } + + /* Orders stored versions, treating one that cannot be read as older than one that can. */ + public static int compare(String version1, String version2) { + Optional parsed1 = parse(version1); + Optional parsed2 = parse(version2); + if (parsed1.isPresent() && parsed2.isPresent()) { + return ORDER.compare(parsed1.get(), parsed2.get()); + } + return Boolean.compare(parsed1.isPresent(), parsed2.isPresent()); + } +} diff --git a/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java b/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java index 94c7d754..b030511f 100644 --- a/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java +++ b/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java @@ -10,6 +10,7 @@ import org.acme.constants.CollectionNames; import org.acme.constants.FieldNames; import org.acme.model.domain.Benefit; +import org.acme.model.domain.CheckVersion; import org.acme.model.domain.EligibilityCheck; import org.acme.persistence.EligibilityCheckRepository; import org.acme.persistence.FirestoreUtils; @@ -55,33 +56,11 @@ public List getLatestVersionPublishedCustomChecks(String userI .collect(java.util.stream.Collectors.toMap( check -> getPublishedPrefix(check), check -> check, - (check1, check2) -> compareVersions(check1.getVersion(), check2.getVersion()) > 0 ? check1 : check2 + (check1, check2) -> CheckVersion.compare(check1.getVersion(), check2.getVersion()) > 0 ? check1 : check2 )); return new ArrayList<>(latestVersionMap.values()); } - private static int compareVersions(String v1, String v2) { - int[] a = normalize(v1); - int[] b = normalize(v2); - - for (int i = 0; i < 3; i++) { - if (a[i] != b[i]) { - return a[i] - b[i]; - } - } - return 0; - } - - private static int[] normalize(String version) { - String[] parts = version.split("\\."); - int[] nums = new int[] {0, 0, 0}; - - for (int i = 0; i < parts.length && i < 3; i++) { - nums[i] = Integer.parseInt(parts[i]); - } - return nums; - } - public List getPublishedCheckVersions(EligibilityCheck workingCustomCheck) throws Exception { Map fieldValues = Map.of( "ownerId", workingCustomCheck.getOwnerId(), diff --git a/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java b/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java new file mode 100644 index 00000000..466fb00f --- /dev/null +++ b/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java @@ -0,0 +1,53 @@ +package org.acme.model.domain; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class CheckVersionTest { + + @Test + void parsesADottedVersion() { + assertArrayEquals(new int[]{2, 3, 4}, CheckVersion.parse("2.3.4").orElseThrow()); + } + + @Test + void treatsMissingPartsAsZero() { + assertArrayEquals(new int[]{2, 0, 0}, CheckVersion.parse("2").orElseThrow()); + assertArrayEquals(new int[]{2, 3, 0}, CheckVersion.parse("2.3").orElseThrow()); + } + + /* Stored versions are free-form strings, so anything at all can come back from Firestore */ + @ParameterizedTest + @ValueSource(strings = {"", " ", ".1", "not-a-version", "v2.0.0", "1.invalid.0", "1.0.0-rc1", "99999999999999"}) + void rejectsAnythingThatIsNotAVersion(String version) { + assertTrue(CheckVersion.parse(version).isEmpty(), "expected \"" + version + "\" to be rejected"); + } + + @Test + void rejectsAMissingVersion() { + assertEquals(Optional.empty(), CheckVersion.parse(null)); + } + + @Test + void ordersVersionsByEachPart() { + assertTrue(CheckVersion.compare("2.0.0", "1.9.9") > 0); + assertTrue(CheckVersion.compare("1.2.3", "1.3.0") < 0); + assertEquals(0, CheckVersion.compare("1.2.3", "1.2.3")); + } + + /* An unreadable version must not crash the comparison it takes part in */ + @Test + void ordersUnreadableVersionsBelowReadableOnes() { + assertTrue(CheckVersion.compare("1.0.0", "v2.0.0") > 0); + assertTrue(CheckVersion.compare("v2.0.0", "1.0.0") < 0); + assertEquals(0, CheckVersion.compare("v2.0.0", "also-bad")); + assertEquals(0, CheckVersion.compare(null, null)); + } +} From f80b8355d44772ad7b2cf142775ca52337eec83d Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Thu, 3 Sep 2026 17:23:26 -0400 Subject: [PATCH 3/4] fix: reject stored versions that only look like versions Java drops trailing empty parts when splitting, so "." and "..." split into no parts at all: the parse loop never ran and the value was accepted as 0.0.0 rather than ignored as corrupt data. Require one to three non-negative numeric parts, which also rejects negative and over-long versions instead of silently reading the first three parts. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019VpGMR8VeD27SgrfNeWiUr --- .../java/org/acme/model/domain/CheckVersion.java | 12 ++++++++++-- .../controller/EligibilityCheckResourceTest.java | 13 +++++++++++++ .../org/acme/model/domain/CheckVersionTest.java | 3 ++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java b/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java index edc92873..a3dd2a0a 100644 --- a/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java +++ b/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java @@ -18,20 +18,28 @@ public final class CheckVersion { private CheckVersion() { } - /* A version is dot-separated numbers; missing parts count as zero. */ + /* A version is one to three dot-separated non-negative numbers; missing parts count as zero. */ public static Optional parse(String version) { if (version == null) { return Optional.empty(); } + // Java drops trailing empty parts, so dot-only strings such as "." split into no parts at all String[] parts = version.split("\\."); + if (parts.length == 0 || parts.length > PART_COUNT) { + return Optional.empty(); + } + int[] numbers = new int[PART_COUNT]; - for (int i = 0; i < parts.length && i < PART_COUNT; i++) { + for (int i = 0; i < parts.length; i++) { try { numbers[i] = Integer.parseInt(parts[i]); } catch (NumberFormatException e) { return Optional.empty(); } + if (numbers[i] < 0) { + return Optional.empty(); + } } return Optional.of(numbers); } diff --git a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java index 5560c7b1..07b69ba7 100644 --- a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java +++ b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java @@ -235,6 +235,19 @@ void malformedPublishedVersionsAreIgnored() throws Exception { assertEquals("3.0.0", capturedPublishedVersion()); } + // Dot-only strings split into no parts at all, so they must not be read as version 0.0.0 + @Test + void dotOnlyPublishedVersionsAreIgnored() throws Exception { + workingCheck.setVersion("2.0.0"); + when(repository.getPublishedCheckVersions(workingCheck)) + .thenReturn(List.of(publishedVersion("."), publishedVersion("..."))); + + Response response = resource.publishCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals("2.0.0", capturedPublishedVersion()); + } + // An unreadable version list must not be mistaken for "never published" @Test void publishFailsWhenPublishedVersionsCannotBeRead() throws Exception { diff --git a/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java b/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java index 466fb00f..2aa1c9e1 100644 --- a/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java +++ b/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java @@ -25,7 +25,8 @@ void treatsMissingPartsAsZero() { /* Stored versions are free-form strings, so anything at all can come back from Firestore */ @ParameterizedTest - @ValueSource(strings = {"", " ", ".1", "not-a-version", "v2.0.0", "1.invalid.0", "1.0.0-rc1", "99999999999999"}) + @ValueSource(strings = {"", " ", ".", "..", "...", ".1", "not-a-version", "v2.0.0", "1.invalid.0", + "1.2.3.4", "-1.0.0", "1.0.0-rc1", "99999999999999"}) void rejectsAnythingThatIsNotAVersion(String version) { assertTrue(CheckVersion.parse(version).isEmpty(), "expected \"" + version + "\" to be rejected"); } From 9d9f01a889a44ed191211831b67fb8afcc9d2a74 Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Thu, 3 Sep 2026 17:23:52 -0400 Subject: [PATCH 4/4] fix: skip published versions whose document id is taken A published document id is derived from the version, and Firestore refuses to create an id that already exists. Ignoring a corrupt version field therefore left publishing broken for good: the corrupt version dropped out of the maximum, the next version landed back on an id an existing document already held, and every retry failed the same way. Compare each candidate against the ids the published documents actually carry, which is the only remaining record of a version whose field was corrupted later, and increment past the ones already taken. A corrupt working version now starts at the initial version instead of being published as-is and carrying the corruption into a new id. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019VpGMR8VeD27SgrfNeWiUr --- .../controller/EligibilityCheckResource.java | 34 ++++++++---- .../org/acme/model/domain/CheckVersion.java | 12 +++++ .../EligibilityCheckRepository.java | 3 ++ .../impl/EligibilityCheckRepositoryImpl.java | 6 ++- .../EligibilityCheckResourceTest.java | 54 +++++++++++++++++++ .../acme/model/domain/CheckVersionTest.java | 13 +++++ 6 files changed, 110 insertions(+), 12 deletions(-) diff --git a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java index 2741949f..0b1cf342 100644 --- a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java +++ b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java @@ -26,6 +26,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; @Path("/api/custom-checks") public class EligibilityCheckResource { @@ -343,7 +345,7 @@ public Response publishCustomCheck(@Context SecurityIdentity identity, @PathPara .entity(Map.of("error", "could not read published versions of Check, published check version was not created")) .build(); } - check.setVersion(versionForPublish(check.getVersion(), publishedChecks)); + check.setVersion(versionForPublish(check, publishedChecks)); // Update the working check so the extracted input definition and current version are saved. try { @@ -443,19 +445,29 @@ public Response getPublishedVersionsOfWorkingCheck(@Context SecurityIdentity ide // ========== Private Helper Methods ========== - /* The first publish keeps the working version; later ones increment past the highest published version, - so a working version that lags what is already published cannot produce a duplicate published id. */ - String versionForPublish(String workingVersion, List publishedChecks) { - return publishedChecks.stream() + /* The first publish keeps the working version; later ones increment past the highest published + version, so a working version that lags what is already published cannot produce a duplicate + published id. */ + String versionForPublish(EligibilityCheck check, List publishedChecks) { + Set publishedIds = publishedChecks.stream() + .map(EligibilityCheck::getId) + .filter(id -> id != null) + .collect(Collectors.toSet()); + + int[] version = publishedChecks.stream() .map(EligibilityCheck::getVersion) - .flatMap(version -> parsePublishedVersion(version).stream()) + .flatMap(publishedVersion -> parsePublishedVersion(publishedVersion).stream()) .max(CheckVersion.ORDER) - .map(this::incrementMajorVersion) - .orElse(workingVersion); - } + .map(CheckVersion::nextMajor) + .orElseGet(() -> CheckVersion.parse(check.getVersion()).orElseGet(CheckVersion::initial)); - private String incrementMajorVersion(int[] version) { - return (version[0] + 1) + ".0.0"; // increment major, reset minor and patch + /* A published id is derived from the version and can never be written twice, so skip past + versions that are already taken. A version ignored above still holds its id, which is the + only remaining record that the version was used. */ + while (publishedIds.contains(eligibilityCheckRepository.getPublishedId(check, CheckVersion.format(version)))) { + version = CheckVersion.nextMajor(version); + } + return CheckVersion.format(version); } private Optional parsePublishedVersion(String version) { diff --git a/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java b/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java index a3dd2a0a..4d45b5a0 100644 --- a/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java +++ b/builder-api/src/main/java/org/acme/model/domain/CheckVersion.java @@ -53,4 +53,16 @@ public static int compare(String version1, String version2) { } return Boolean.compare(parsed1.isPresent(), parsed2.isPresent()); } + + public static int[] initial() { + return new int[]{1, 0, 0}; + } + + public static int[] nextMajor(int[] version) { + return new int[]{version[0] + 1, 0, 0}; // increment major, reset minor and patch + } + + public static String format(int[] version) { + return version[0] + "." + version[1] + "." + version[2]; + } } diff --git a/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java b/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java index 7329f02a..b15ef1cd 100644 --- a/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java +++ b/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java @@ -33,4 +33,7 @@ public interface EligibilityCheckRepository { void deleteWorkingCustomCheck(String checkId) throws Exception; void updatePublishedCustomCheck(EligibilityCheck check) throws Exception; + + /* The published document id a check would get at the given version. */ + String getPublishedId(EligibilityCheck check, String version); } diff --git a/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java b/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java index b030511f..fb65769d 100644 --- a/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java +++ b/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java @@ -177,6 +177,10 @@ public String getPublishedPrefix(EligibilityCheck check) { } public String getPublishedId(EligibilityCheck check) { - return getPublishedPrefix(check) + "-" + check.getVersion().toString(); + return getPublishedId(check, check.getVersion()); + } + + public String getPublishedId(EligibilityCheck check, String version) { + return getPublishedPrefix(check) + "-" + version; } } diff --git a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java index 07b69ba7..b4e64e7c 100644 --- a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java +++ b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java @@ -32,6 +32,7 @@ class EligibilityCheckResourceTest { private static final String USER_ID = "owner-1"; private static final String CHECK_ID = "check-1"; + private static final String PUBLISHED_PREFIX = "P-" + USER_ID + "-my-module-my-check"; private final EligibilityCheckResource resource = new EligibilityCheckResource(); private final EligibilityCheckRepository repository = mock(EligibilityCheckRepository.class); @@ -66,6 +67,8 @@ void setUp() throws Exception { EligibilityCheck check = invocation.getArgument(0); return "W-" + check.getOwnerId() + "-" + check.getModule() + "-" + check.getName(); }); + when(repository.getPublishedId(any(), anyString())) + .thenAnswer(invocation -> PUBLISHED_PREFIX + "-" + invocation.getArgument(1)); } @Test @@ -248,6 +251,51 @@ void dotOnlyPublishedVersionsAreIgnored() throws Exception { assertEquals("2.0.0", capturedPublishedVersion()); } + /* Ignoring a corrupt version must not hand back a version whose published id is already taken: + that document could never be written, so publishing would fail for good. */ + @Test + void publishSkipsVersionsWhosePublishedIdAlreadyExists() throws Exception { + workingCheck.setVersion("2.0.0"); + when(repository.getPublishedCheckVersions(workingCheck)) + .thenReturn(List.of( + publishedVersion("1.0.0"), + publishedVersion("v2.0.0", PUBLISHED_PREFIX + "-2.0.0") + )); + + Response response = resource.publishCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals("3.0.0", capturedPublishedVersion()); + } + + @Test + void publishKeepsSkippingUntilAnUnusedVersionIsFound() throws Exception { + workingCheck.setVersion("1.0.0"); + when(repository.getPublishedCheckVersions(workingCheck)) + .thenReturn(List.of( + publishedVersion("1.0.0"), + publishedVersion("v2.0.0", PUBLISHED_PREFIX + "-2.0.0"), + publishedVersion("v3.0.0", PUBLISHED_PREFIX + "-3.0.0") + )); + + Response response = resource.publishCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals("4.0.0", capturedPublishedVersion()); + } + + // A corrupt working version cannot be published as-is, or the next publish inherits the corruption + @Test + void firstPublishOfACorruptWorkingVersionStartsAtTheInitialVersion() throws Exception { + workingCheck.setVersion("not-a-version"); + when(repository.getPublishedCheckVersions(workingCheck)).thenReturn(List.of()); + + Response response = resource.publishCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals("1.0.0", capturedPublishedVersion()); + } + // An unreadable version list must not be mistaken for "never published" @Test void publishFailsWhenPublishedVersionsCannotBeRead() throws Exception { @@ -262,8 +310,14 @@ void publishFailsWhenPublishedVersionsCannotBeRead() throws Exception { } private EligibilityCheck publishedVersion(String version) { + return publishedVersion(version, PUBLISHED_PREFIX + "-" + version); + } + + /* The id is fixed when a version is published, so a version field corrupted later no longer matches it */ + private EligibilityCheck publishedVersion(String version, String id) { EligibilityCheck published = new EligibilityCheck("my-check", "my-module", "a check", List.of(), USER_ID); published.setVersion(version); + published.setId(id); return published; } diff --git a/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java b/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java index 2aa1c9e1..e65f9663 100644 --- a/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java +++ b/builder-api/src/test/java/org/acme/model/domain/CheckVersionTest.java @@ -4,6 +4,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import java.util.List; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -51,4 +52,16 @@ void ordersUnreadableVersionsBelowReadableOnes() { assertEquals(0, CheckVersion.compare("v2.0.0", "also-bad")); assertEquals(0, CheckVersion.compare(null, null)); } + + @Test + void incrementsTheMajorPartAndResetsTheRest() { + assertArrayEquals(new int[]{3, 0, 0}, CheckVersion.nextMajor(new int[]{2, 4, 6})); + assertEquals("3.0.0", CheckVersion.format(CheckVersion.nextMajor(new int[]{2, 4, 6}))); + } + + @Test + void startsAtTheVersionNewChecksAreCreatedWith() { + EligibilityCheck newCheck = new EligibilityCheck("a-check", "a-module", "", List.of(), "owner-1"); + assertEquals(newCheck.getVersion(), CheckVersion.format(CheckVersion.initial())); + } }