From 56a1e57e0398e366adc342f88c43866387a9f848 Mon Sep 17 00:00:00 2001 From: Steven Grimm Date: Fri, 13 Dec 2024 10:54:20 -0800 Subject: [PATCH] SW-6348 Add numeric identifiers to IdentifierGenerator In preparation for adding organization-wide plot numbers to monitoring plots, make `IdentifierGenerator` support returning simple numeric sequence numbers in addition to its existing compound identifiers. --- .../backend/db/IdentifierGenerator.kt | 55 +++++++++++---- .../backend/nursery/db/BatchStore.kt | 2 +- .../backend/seedbank/db/AccessionStore.kt | 2 +- .../0300/V327__IdentifierSequencesCascade.sql | 4 ++ .../backend/db/IdentifierGeneratorTest.kt | 68 ++++++++++++++----- 5 files changed, 96 insertions(+), 35 deletions(-) create mode 100644 src/main/resources/db/migration/0300/V327__IdentifierSequencesCascade.sql diff --git a/src/main/kotlin/com/terraformation/backend/db/IdentifierGenerator.kt b/src/main/kotlin/com/terraformation/backend/db/IdentifierGenerator.kt index 94207d4a2389..becfd3445d0e 100644 --- a/src/main/kotlin/com/terraformation/backend/db/IdentifierGenerator.kt +++ b/src/main/kotlin/com/terraformation/backend/db/IdentifierGenerator.kt @@ -15,7 +15,11 @@ import org.jooq.DSLContext * identify a resource but it's not acceptable to display the underlying integer ID from the * database, e.g., accession numbers. * - * Identifiers are mostly-fixed-length numeric values of the form `YY-T-F-XXX` where: + * Identifiers come in two flavors: text and numeric. + * + * # Text identifiers + * + * Text identifiers are mostly-fixed-length values of the form `YY-T-F-XXX` where: * - `YY` is the two-digit year * - `T` is a digit indicating the type of identifier; see [IdentifierType] * - `F` is a facility number that starts at 1 for each facility type in an organization @@ -38,6 +42,15 @@ import org.jooq.DSLContext * organization ID, it is possible for the generated identifiers to collide with user-supplied * identifiers. To guard against that, [AccessionStore.create] will retry a few times if it gets an * identifier that's already in use. + * + * # Numeric identifiers + * + * Numeric identifiers are sequential [Long] values that start at 1 for a given organization and + * identifier type. Unlike text identifiers, numeric identifiers don't reset at the start of the + * year and don't include any facility information. + * + * The identifier types for numeric identifiers are distinct from the ones for text identifiers and + * are listed in [NumericIdentifierType]. */ @Named class IdentifierGenerator( @@ -45,11 +58,11 @@ class IdentifierGenerator( private val dslContext: DSLContext, ) { /** - * Generates a new identifier for an organization. + * Generates a new text identifier for an organization. * * @param timeZone Time zone to use to determine the current date. */ - fun generateIdentifier( + fun generateTextIdentifier( organizationId: OrganizationId, identifierType: IdentifierType, facilityNumber: Int, @@ -58,22 +71,30 @@ class IdentifierGenerator( val shortYear = LocalDate.ofInstant(clock.instant(), timeZone).year.rem(100) val prefix = "%02d-%c-".format(shortYear, identifierType.digit) - val sequenceValue = - with(IDENTIFIER_SEQUENCES) { - dslContext - .insertInto(IDENTIFIER_SEQUENCES) - .set(ORGANIZATION_ID, organizationId) - .set(PREFIX, prefix) - .set(NEXT_VALUE, 1) - .onDuplicateKeyUpdate() - .set(NEXT_VALUE, NEXT_VALUE.plus(1)) - .returning(NEXT_VALUE) - .fetchOne(NEXT_VALUE)!! - } + val sequenceValue = getNextValue(organizationId, prefix) return "%s%d-%03d".format(prefix, facilityNumber, sequenceValue) } + /** Generates a new numeric identifier for an organization. */ + fun generateNumericIdentifier(organizationId: OrganizationId, type: NumericIdentifierType): Long { + return getNextValue(organizationId, type.name) + } + + private fun getNextValue(organizationId: OrganizationId, prefix: String): Long { + return with(IDENTIFIER_SEQUENCES) { + dslContext + .insertInto(IDENTIFIER_SEQUENCES) + .set(ORGANIZATION_ID, organizationId) + .set(PREFIX, prefix) + .set(NEXT_VALUE, 1) + .onDuplicateKeyUpdate() + .set(NEXT_VALUE, NEXT_VALUE.plus(1)) + .returning(NEXT_VALUE) + .fetchOne(NEXT_VALUE)!! + } + } + /** * Replaces the facility number in an existing identifier with a new one. Returns null if the * existing identifier isn't in the correct format (e.g., because it was supplied by a user). @@ -89,3 +110,7 @@ enum class IdentifierType(val digit: Char) { ACCESSION('1'), BATCH('2') } + +enum class NumericIdentifierType { + PlotNumber +} diff --git a/src/main/kotlin/com/terraformation/backend/nursery/db/BatchStore.kt b/src/main/kotlin/com/terraformation/backend/nursery/db/BatchStore.kt index ddfdb85580c6..1fa4c8f2eedb 100644 --- a/src/main/kotlin/com/terraformation/backend/nursery/db/BatchStore.kt +++ b/src/main/kotlin/com/terraformation/backend/nursery/db/BatchStore.kt @@ -207,7 +207,7 @@ class BatchStore( .copy( batchNumber = newModel.batchNumber - ?: identifierGenerator.generateIdentifier( + ?: identifierGenerator.generateTextIdentifier( organizationId, IdentifierType.BATCH, facilityNumber), createdBy = userId, createdTime = now, diff --git a/src/main/kotlin/com/terraformation/backend/seedbank/db/AccessionStore.kt b/src/main/kotlin/com/terraformation/backend/seedbank/db/AccessionStore.kt index 5db9f09bafc0..75f63ca9daa1 100644 --- a/src/main/kotlin/com/terraformation/backend/seedbank/db/AccessionStore.kt +++ b/src/main/kotlin/com/terraformation/backend/seedbank/db/AccessionStore.kt @@ -256,7 +256,7 @@ class AccessionStore( while (attemptsRemaining-- > 0) { val accessionNumber = accession.accessionNumber - ?: identifierGenerator.generateIdentifier( + ?: identifierGenerator.generateTextIdentifier( organizationId, IdentifierType.ACCESSION, facility.facilityNumber!!) try { diff --git a/src/main/resources/db/migration/0300/V327__IdentifierSequencesCascade.sql b/src/main/resources/db/migration/0300/V327__IdentifierSequencesCascade.sql new file mode 100644 index 000000000000..90ce81f5c617 --- /dev/null +++ b/src/main/resources/db/migration/0300/V327__IdentifierSequencesCascade.sql @@ -0,0 +1,4 @@ +-- Identifier sequences weren't being deleted when their organizations were deleted. +ALTER TABLE identifier_sequences DROP CONSTRAINT identifier_sequences_organization_id_fkey; + +ALTER TABLE identifier_sequences ADD FOREIGN KEY (organization_id) REFERENCES organizations ON DELETE CASCADE; diff --git a/src/test/kotlin/com/terraformation/backend/db/IdentifierGeneratorTest.kt b/src/test/kotlin/com/terraformation/backend/db/IdentifierGeneratorTest.kt index ef124c2351cb..e11fd3b93cb7 100644 --- a/src/test/kotlin/com/terraformation/backend/db/IdentifierGeneratorTest.kt +++ b/src/test/kotlin/com/terraformation/backend/db/IdentifierGeneratorTest.kt @@ -25,18 +25,19 @@ internal class IdentifierGeneratorTest : DatabaseTest(), RunsAsUser { } @Test - fun `identifiers are allocated per organization and per type`() { + fun `text identifiers are allocated per organization and per type`() { clock.instant = Instant.parse("2022-01-01T00:00:00Z") val otherOrganizationId = insertOrganization() val org1AccessionIdentifier1 = - generator.generateIdentifier(organizationId, IdentifierType.ACCESSION, 1) + generator.generateTextIdentifier(organizationId, IdentifierType.ACCESSION, 1) val org1AccessionIdentifier2 = - generator.generateIdentifier(organizationId, IdentifierType.ACCESSION, 1) - val org1BatchIdentifier = generator.generateIdentifier(organizationId, IdentifierType.BATCH, 1) + generator.generateTextIdentifier(organizationId, IdentifierType.ACCESSION, 1) + val org1BatchIdentifier = + generator.generateTextIdentifier(organizationId, IdentifierType.BATCH, 1) val org2AccessionIdentifier = - generator.generateIdentifier(otherOrganizationId, IdentifierType.ACCESSION, 1) + generator.generateTextIdentifier(otherOrganizationId, IdentifierType.ACCESSION, 1) assertEquals( mapOf( @@ -52,15 +53,15 @@ internal class IdentifierGeneratorTest : DatabaseTest(), RunsAsUser { } @Test - fun `identifier numbers are shared across facilities`() { + fun `text identifier numbers are shared across facilities`() { clock.instant = Instant.parse("2022-01-01T00:00:00Z") val nursery1BatchIdentifier1 = - generator.generateIdentifier(organizationId, IdentifierType.BATCH, 1) + generator.generateTextIdentifier(organizationId, IdentifierType.BATCH, 1) val nursery1BatchIdentifier2 = - generator.generateIdentifier(organizationId, IdentifierType.BATCH, 1) + generator.generateTextIdentifier(organizationId, IdentifierType.BATCH, 1) val nursery2BatchIdentifier1 = - generator.generateIdentifier(organizationId, IdentifierType.BATCH, 2) + generator.generateTextIdentifier(organizationId, IdentifierType.BATCH, 2) assertEquals( mapOf( @@ -74,13 +75,14 @@ internal class IdentifierGeneratorTest : DatabaseTest(), RunsAsUser { } @Test - fun `generateIdentifier honors time zone`() { + fun `generateTextIdentifier honors time zone`() { clock.instant = Instant.parse("2019-12-31T23:59:59Z") val identifierInUtc = - generator.generateIdentifier(organizationId, IdentifierType.ACCESSION, 2, ZoneOffset.UTC) + generator.generateTextIdentifier( + organizationId, IdentifierType.ACCESSION, 2, ZoneOffset.UTC) val identifierInLaterZone = - generator.generateIdentifier( + generator.generateTextIdentifier( organizationId, IdentifierType.ACCESSION, 3, ZoneOffset.ofHours(1)) assertEquals("19-1-2-001", identifierInUtc, "Identifier in earlier time zone") @@ -88,30 +90,60 @@ internal class IdentifierGeneratorTest : DatabaseTest(), RunsAsUser { } @Test - fun `generateIdentifier restarts suffixes at 001 when the year changes`() { + fun `generateTextIdentifier restarts suffixes at 001 when the year changes`() { clock.instant = Instant.parse("2022-01-01T00:00:00Z") - generator.generateIdentifier(organizationId, IdentifierType.ACCESSION, 1) + generator.generateTextIdentifier(organizationId, IdentifierType.ACCESSION, 1) clock.instant = Instant.parse("2023-05-06T00:00:00Z") val nextYearIdentifier = - generator.generateIdentifier(organizationId, IdentifierType.ACCESSION, 8) + generator.generateTextIdentifier(organizationId, IdentifierType.ACCESSION, 8) assertEquals("23-1-8-001", nextYearIdentifier) } @Test - fun `generateIdentifier picks up where it left off after a century`() { + fun `generateTextIdentifier picks up where it left off after a century`() { clock.instant = Instant.parse("2022-01-01T00:00:00Z") - generator.generateIdentifier(organizationId, IdentifierType.ACCESSION, 1) + generator.generateTextIdentifier(organizationId, IdentifierType.ACCESSION, 1) clock.instant = Instant.parse("2122-01-01T00:00:00Z") - val identifier = generator.generateIdentifier(organizationId, IdentifierType.ACCESSION, 1) + val identifier = generator.generateTextIdentifier(organizationId, IdentifierType.ACCESSION, 1) assertEquals("22-1-1-002", identifier) } + @Test + fun `generateNumericIdentifier starts at 1 for each organization`() { + val otherOrganizationId = insertOrganization() + + assertEquals( + 1L, + generator.generateNumericIdentifier(organizationId, NumericIdentifierType.PlotNumber), + "Identifier for first organization") + assertEquals( + 1L, + generator.generateNumericIdentifier(otherOrganizationId, NumericIdentifierType.PlotNumber), + "Identifier for second organization") + } + + @Test + fun `generateNumericIdentifier return value increases by 1 on each call`() { + assertEquals( + 1L, + generator.generateNumericIdentifier(organizationId, NumericIdentifierType.PlotNumber), + "Initial identifier") + assertEquals( + 2L, + generator.generateNumericIdentifier(organizationId, NumericIdentifierType.PlotNumber), + "Second identifier") + assertEquals( + 3L, + generator.generateNumericIdentifier(organizationId, NumericIdentifierType.PlotNumber), + "Third identifier") + } + @Test fun `replaceFacilityNumber retains other parts of identifier`() { assertEquals("23-1-78-123456789", generator.replaceFacilityNumber("23-1-55-123456789", 78))