diff --git a/broker/import/README.md b/broker/import/README.md index 26039d529..cbf603554 100644 --- a/broker/import/README.md +++ b/broker/import/README.md @@ -114,6 +114,8 @@ Important validation rules include: - An `illTransaction`, when supplied, must use the same requester symbol and requester request ID. `locatedSuppliers` require an `illTransaction`. - Item, notification, and located-supplier IDs must be unique within their respective arrays. At most one located supplier may have status `selected`. +When a borrowing request's `id` has the `PREFIX-N` form that the broker generates, the import raises the request-ID sequence for `PREFIX` past `N`, so new requests cannot reuse the ID. `PREFIX` must be ASCII letters and digits, or equal the value of `requesterSymbol` (`US-RS1` in `US-RS1-1001`), which the broker uses when Directory sets no `requestIdPrefix`. Other IDs, such as UUIDs, leave the sequences unchanged. Request creation for that prefix waits while an import is advancing its sequence, so imports and new requests can run at the same time. + Minimal borrowing example: ```ndjson diff --git a/broker/import/db/repo.go b/broker/import/db/repo.go index fc9df9fed..17a7af5f7 100644 --- a/broker/import/db/repo.go +++ b/broker/import/db/repo.go @@ -3,6 +3,8 @@ package importdb import ( "errors" "fmt" + "regexp" + "strconv" "strings" "github.com/indexdata/crosslink/broker/common" @@ -150,6 +152,11 @@ func (r *PgImportRepo) ImportPatronRequest(ctx common.ExtendedContext, bundle Pa return fmt.Errorf("insert patron request %q: %w", bundle.PatronRequest.ID, err) } } + if bundle.PatronRequest.Side == prservice.SideBorrowing { + if err := r.advanceHrid(ctx, tx, bundle.PatronRequest.ID, bundle.PatronRequest.RequesterSymbol.String); err != nil { + return err + } + } itemIDs := make([]string, 0, len(bundle.Items)) for _, item := range bundle.Items { @@ -194,6 +201,40 @@ func (r *PgImportRepo) ImportPatronRequest(ctx common.ExtendedContext, bundle Pa return result, err } +var ( + // hridPattern splits the PREFIX-NUMBER form that get_next_hrid generates. + hridPattern = regexp.MustCompile(`^(.+)-([0-9]+)$`) + // alphanumericPrefix matches prefixes configured in Directory. + alphanumericPrefix = regexp.MustCompile(`^[A-Za-z0-9]+$`) +) + +// advanceHrid keeps generated request IDs from colliding with an imported one. +// When id has the PREFIX-NUMBER form, the HRID sequence for PREFIX is raised so +// get_next_hrid only returns numbers above NUMBER. PREFIX must be alphanumeric, +// like a Directory requestIdPrefix, or equal the value of requesterSymbol, which +// RequestIdPrefix falls back to and which may contain hyphens ("US-RS1-1001"). +// Other IDs, including UUIDs whose last group is all digits, are left alone. +// advance_hrid locks the sequence against other advances and get_next_hrid +// until commit, so imports may run while the tenant creates requests. +func (r *PgImportRepo) advanceHrid(ctx common.ExtendedContext, tx DBTX, id string, requesterSymbol string) error { + match := hridPattern.FindStringSubmatch(id) + if match == nil { + return nil + } + _, symbolValue, _ := common.SplitSymbol(requesterSymbol) + if !alphanumericPrefix.MatchString(match[1]) && (symbolValue == "" || !strings.EqualFold(match[1], symbolValue)) { + return nil + } + used, err := strconv.ParseInt(match[2], 10, 64) + if err != nil { + return fmt.Errorf("parse HRID number of patron request %q: %w", id, err) + } + if err := r.queries.AdvanceImportedHrid(ctx, tx, AdvanceImportedHridParams{Prefix: match[1], Used: used}); err != nil { + return fmt.Errorf("advance HRID sequence for patron request %q: %w", id, err) + } + return nil +} + func createImportedPatronRequestParams(params pr_db.CreatePatronRequestParams) CreateImportedPatronRequestParams { return CreateImportedPatronRequestParams{ ID: params.ID, CreatedAt: params.CreatedAt, IllRequest: params.IllRequest, diff --git a/broker/import/test/export_hrid_preflight_test.go b/broker/import/test/export_hrid_preflight_test.go index 268aeb79d..0f6251b51 100644 --- a/broker/import/test/export_hrid_preflight_test.go +++ b/broker/import/test/export_hrid_preflight_test.go @@ -11,7 +11,9 @@ import ( "github.com/stretchr/testify/require" ) -func TestExportPreflightRejectsNonAlphanumericHrid(t *testing.T) { +const hridProblem = " must be ASCII letters and digits, optionally followed by a hyphen and digits" + +func TestExportPreflightRejectsInvalidHrid(t *testing.T) { // Execute the actual preflight check without requiring the full legacy schema. source, err := os.ReadFile("../../../migration/export-crosslink-open-patron-requests.sql") require.NoError(t, err) @@ -32,13 +34,16 @@ func TestExportPreflightRejectsNonAlphanumericHrid(t *testing.T) { {name: "lowercase", side: "borrowing", hrid: pgtype.Text{String: "abc123", Valid: true}}, {name: "digits only", side: "borrowing", hrid: pgtype.Text{String: "123", Valid: true}}, {name: "missing HRID keeps UUID", side: "borrowing"}, - {name: "lending is not checked", side: "lending", hrid: pgtype.Text{String: "ABC-1", Valid: true}}, - {name: "hyphen", side: "borrowing", hrid: pgtype.Text{String: "ABC-1", Valid: true}, - want: "request pr1: HRID ABC-1 must contain only ASCII letters and digits"}, + {name: "lending is not checked", side: "lending", hrid: pgtype.Text{String: "AB 1", Valid: true}}, + {name: "hyphen before number", side: "borrowing", hrid: pgtype.Text{String: "ABC-10008", Valid: true}}, + {name: "hyphen before letters", side: "borrowing", hrid: pgtype.Text{String: "ABC-X1", Valid: true}, + want: "request pr1: HRID ABC-X1" + hridProblem}, + {name: "two hyphens", side: "borrowing", hrid: pgtype.Text{String: "A-B-1", Valid: true}, + want: "request pr1: HRID A-B-1" + hridProblem}, {name: "space", side: "borrowing", hrid: pgtype.Text{String: "AB 1", Valid: true}, - want: "request pr1: HRID AB 1 must contain only ASCII letters and digits"}, + want: "request pr1: HRID AB 1" + hridProblem}, {name: "non-ASCII", side: "borrowing", hrid: pgtype.Text{String: "ÅB1", Valid: true}, - want: "request pr1: HRID ÅB1 must contain only ASCII letters and digits"}, + want: "request pr1: HRID ÅB1" + hridProblem}, } { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() diff --git a/broker/import/test/patron_request_test.go b/broker/import/test/patron_request_test.go index b65e0048a..528b40ff3 100644 --- a/broker/import/test/patron_request_test.go +++ b/broker/import/test/patron_request_test.go @@ -3,6 +3,7 @@ package test import ( "context" "os" + "strings" "testing" "time" @@ -573,3 +574,109 @@ func TestImportPatronRequestPersistsBibRecordID(t *testing.T) { assert.Equal(t, expected, actual) } } + +func TestImportBorrowingPatronRequestAdvancesHridSequence(t *testing.T) { + hridPrefix := "T" + strings.ReplaceAll(uuid.NewString(), "-", "")[:12] + importID := func(id string, lending bool) { + t.Helper() + prefix := uuid.NewString() + bundle := testPatronBundle(prefix, prefix+"-request") + if lending { + bundle = testLendingPatronBundle(prefix, prefix+"-request", prefix+"-SUP") + } + bundle.PatronRequest.ID = id + _, err := importTestRepo.ImportPatronRequest(importTestCtx, bundle, importdb.ConflictPolicyFail) + require.NoError(t, err) + } + nextHrid := func() string { + t.Helper() + var hrid string + require.NoError(t, importTestPool.QueryRow(context.Background(), "SELECT get_next_hrid($1)", hridPrefix).Scan(&hrid)) + return hrid + } + + importID(hridPrefix+"-41", false) + assert.Equal(t, strings.ToUpper(hridPrefix)+"-42", nextHrid()) + importID(hridPrefix+"-10", false) + assert.Equal(t, strings.ToUpper(hridPrefix)+"-43", nextHrid(), "a lower imported number does not move the sequence back") + importID(hridPrefix+"-500", true) + assert.Equal(t, strings.ToUpper(hridPrefix)+"-44", nextHrid(), "lending request IDs are not generated locally") +} + +func TestImportHridAdvanceWaitsForConcurrentSequenceUse(t *testing.T) { + // Counts waiters on this prefix's sequence lock; a bigint advisory key is + // stored as classid (high 32 bits) and objid (low 32 bits). + const waitingQuery = `WITH lock_key AS ( + SELECT hashtextextended('broker:hrid-seq:' || lower($1) || '_hrid_seq', 0) AS k + ) + SELECT count(*) FROM pg_locks, lock_key + WHERE locktype = 'advisory' AND NOT granted AND objsubid = 1 + AND classid::bigint = (k >> 32) & 4294967295 + AND objid::bigint = k & 4294967295` + for _, tc := range []struct { + name, blockerSQL, wantNext string + }{ + {name: "uncommitted advance", blockerSQL: "SELECT advance_hrid($1, 90)", wantNext: "-91"}, + {name: "uncommitted request creation", blockerSQL: "SELECT get_next_hrid($1)", wantNext: "-42"}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + hridPrefix := "T" + strings.ReplaceAll(uuid.NewString(), "-", "")[:12] + blocker, err := importTestPool.Begin(ctx) + require.NoError(t, err) + t.Cleanup(func() { _ = blocker.Rollback(ctx) }) + _, err = blocker.Exec(ctx, tc.blockerSQL, hridPrefix) + require.NoError(t, err) + + prefix := uuid.NewString() + bundle := testPatronBundle(prefix, prefix+"-request") + bundle.PatronRequest.ID = hridPrefix + "-41" + done := make(chan error, 1) + go func() { + _, err := importTestRepo.ImportPatronRequest(importTestCtx, bundle, importdb.ConflictPolicyFail) + done <- err + }() + require.Eventually(t, func() bool { + var waiting int + err := importTestPool.QueryRow(ctx, waitingQuery, hridPrefix).Scan(&waiting) + return err == nil && waiting > 0 + }, 5*time.Second, 10*time.Millisecond, "the import must wait for the sequence lock") + select { + case err := <-done: + t.Fatalf("import finished while the sequence lock was held: %v", err) + default: + } + + require.NoError(t, blocker.Commit(ctx)) + require.NoError(t, <-done) + var hrid string + require.NoError(t, importTestPool.QueryRow(ctx, "SELECT get_next_hrid($1)", hridPrefix).Scan(&hrid)) + assert.Equal(t, strings.ToUpper(hridPrefix)+tc.wantNext, hrid) + }) + } +} + +func TestImportBorrowingPatronRequestAdvancesSymbolHridSequence(t *testing.T) { + // Without a Directory prefix, IDs use the requester symbol value, which may + // contain hyphens. + prefix := uuid.NewString() + bundle := testPatronBundle(prefix, prefix+"-request") + symbolValue := "T-" + strings.ReplaceAll(uuid.NewString(), "-", "")[:12] + bundle.PatronRequest.RequesterSymbol = pgtype.Text{String: "ISIL:" + symbolValue, Valid: true} + bundle.PatronRequest.ID = symbolValue + "-41" + _, err := importTestRepo.ImportPatronRequest(importTestCtx, bundle, importdb.ConflictPolicyFail) + require.NoError(t, err) + var hrid string + require.NoError(t, importTestPool.QueryRow(context.Background(), "SELECT get_next_hrid($1)", symbolValue).Scan(&hrid)) + assert.Equal(t, strings.ToUpper(symbolValue)+"-42", hrid) + + // A UUID whose last group is all digits is not an HRID. + uuidPrefix := uuid.NewString()[:23] + bundle = testPatronBundle(uuid.NewString(), uuid.NewString()+"-request") + bundle.PatronRequest.ID = uuidPrefix + "-" + strings.Repeat("7", 12) + _, err = importTestRepo.ImportPatronRequest(importTestCtx, bundle, importdb.ConflictPolicyFail) + require.NoError(t, err) + var sequences int + require.NoError(t, importTestPool.QueryRow(context.Background(), "SELECT count(*) FROM pg_class WHERE relkind = 'S' AND relname = $1", uuidPrefix+"_hrid_seq").Scan(&sequences)) + assert.Zero(t, sequences) +} diff --git a/broker/migrations/080_advance_hrid.down.sql b/broker/migrations/080_advance_hrid.down.sql new file mode 100644 index 000000000..8b28f697c --- /dev/null +++ b/broker/migrations/080_advance_hrid.down.sql @@ -0,0 +1,8 @@ +DROP FUNCTION advance_hrid; + +CREATE OR REPLACE FUNCTION get_next_hrid(prefix text) RETURNS varchar AS $$ +BEGIN + EXECUTE format('CREATE SEQUENCE IF NOT EXISTS %I START 1', LOWER(prefix) || '_hrid_seq'); + RETURN UPPER(prefix) || '-' || nextval(LOWER(prefix) || '_hrid_seq')::TEXT; +END; +$$ LANGUAGE plpgsql; diff --git a/broker/migrations/080_advance_hrid.up.sql b/broker/migrations/080_advance_hrid.up.sql new file mode 100644 index 000000000..d5e26c720 --- /dev/null +++ b/broker/migrations/080_advance_hrid.up.sql @@ -0,0 +1,30 @@ +-- Raises the HRID sequence for prefix so get_next_hrid never returns an ID at +-- or below used, which an imported request already holds. The sequence never +-- moves backwards. setval is not transactional, so a read followed by setval +-- must not interleave with another advance or with nextval: a value read +-- earlier would overwrite a higher one. advance_hrid therefore holds an +-- exclusive advisory lock on the sequence until commit, and get_next_hrid holds +-- a shared one, so request creation only waits while an import advances the +-- same prefix. +CREATE OR REPLACE FUNCTION advance_hrid(prefix text, used bigint) RETURNS void AS $$ +DECLARE + seq text := LOWER(prefix) || '_hrid_seq'; + current_value bigint; + called boolean; +BEGIN + PERFORM pg_advisory_xact_lock(hashtextextended('broker:hrid-seq:' || seq, 0)); + EXECUTE format('CREATE SEQUENCE IF NOT EXISTS %I START 1', seq); + EXECUTE format('SELECT last_value, is_called FROM %I', seq) INTO current_value, called; + IF NOT called OR current_value < used THEN + PERFORM setval(quote_ident(seq)::regclass, GREATEST(current_value, used)); + END IF; +END; +$$ LANGUAGE plpgsql; + +CREATE OR REPLACE FUNCTION get_next_hrid(prefix text) RETURNS varchar AS $$ +BEGIN + PERFORM pg_advisory_xact_lock_shared(hashtextextended('broker:hrid-seq:' || LOWER(prefix) || '_hrid_seq', 0)); + EXECUTE format('CREATE SEQUENCE IF NOT EXISTS %I START 1', LOWER(prefix) || '_hrid_seq'); + RETURN UPPER(prefix) || '-' || nextval(LOWER(prefix) || '_hrid_seq')::TEXT; +END; +$$ LANGUAGE plpgsql; diff --git a/broker/sqlc/import_query.sql b/broker/sqlc/import_query.sql index e9b5f99a8..1357e5cd7 100644 --- a/broker/sqlc/import_query.sql +++ b/broker/sqlc/import_query.sql @@ -16,6 +16,9 @@ WHERE id = sqlc.arg(id) ORDER BY id FOR UPDATE; +-- name: AdvanceImportedHrid :exec +SELECT advance_hrid(sqlc.arg(prefix)::text, sqlc.arg(used)::bigint); + -- name: CreateImportedPatronRequest :exec INSERT INTO patron_request ( id, created_at, ill_request, state, side, patron, requester_symbol, diff --git a/broker/sqlc/pr_schema.sql b/broker/sqlc/pr_schema.sql index a4b85b52e..1d3113342 100644 --- a/broker/sqlc/pr_schema.sql +++ b/broker/sqlc/pr_schema.sql @@ -53,11 +53,27 @@ CREATE TABLE archived_resource ( CREATE OR REPLACE FUNCTION get_next_hrid(prefix VARCHAR) RETURNS VARCHAR AS $$ BEGIN + PERFORM pg_advisory_xact_lock_shared(hashtextextended('broker:hrid-seq:' || LOWER(prefix) || '_hrid_seq', 0)); EXECUTE format('CREATE SEQUENCE IF NOT EXISTS %I START 1', LOWER(prefix) || '_hrid_seq'); RETURN UPPER(prefix) || '-' || nextval(LOWER(prefix) || '_hrid_seq')::TEXT; END; $$ LANGUAGE plpgsql; +CREATE OR REPLACE FUNCTION advance_hrid(prefix text, used bigint) RETURNS void AS $$ +DECLARE + seq text := LOWER(prefix) || '_hrid_seq'; + current_value bigint; + called boolean; +BEGIN + PERFORM pg_advisory_xact_lock(hashtextextended('broker:hrid-seq:' || seq, 0)); + EXECUTE format('CREATE SEQUENCE IF NOT EXISTS %I START 1', seq); + EXECUTE format('SELECT last_value, is_called FROM %I', seq) INTO current_value, called; + IF NOT called OR current_value < used THEN + PERFORM setval(quote_ident(seq)::regclass, GREATEST(current_value, used)); + END IF; +END; +$$ LANGUAGE plpgsql; + CREATE TABLE item ( id VARCHAR PRIMARY KEY, diff --git a/directory/test/request_id_prefix_test.go b/directory/test/request_id_prefix_test.go index 9e72a3958..adf3144aa 100644 --- a/directory/test/request_id_prefix_test.go +++ b/directory/test/request_id_prefix_test.go @@ -14,6 +14,8 @@ import ( var longestRequestIdPrefix = strings.Repeat("A", 32) +const exportPrefixError = "request_id_prefix must be at most 32 ASCII letters and digits after removing punctuation and spaces: " + var invalidRequestIdPrefixes = []string{"ABC-", "A B", "A.B", "A_B", "ÅBC", " ABC", longestRequestIdPrefix + "A"} func TestRequestIdPrefixPersistence(t *testing.T) { @@ -143,10 +145,12 @@ func TestDirectoryExportRequestIdPrefix(t *testing.T) { {name: "local", setting: "ABC", local: true, want: "ABC"}, {name: "trimmed", setting: " ABC ", local: true, want: "ABC"}, {name: "longest", setting: longestRequestIdPrefix, local: true, want: longestRequestIdPrefix}, - {name: "too long", setting: longestRequestIdPrefix + "A", local: true, wantError: "request_id_prefix must be at most 32 ASCII letters and digits: " + longestRequestIdPrefix + "A"}, - {name: "trailing delimiter", setting: "ABC-", local: true, wantError: "request_id_prefix must be at most 32 ASCII letters and digits: ABC-"}, - {name: "inner space", setting: "A B", local: true, wantError: "request_id_prefix must be at most 32 ASCII letters and digits: A B"}, - {name: "invalid default value", setting: " ", fallback: "A.B", local: true, wantError: "request_id_prefix must be at most 32 ASCII letters and digits: A.B"}, + {name: "too long", setting: longestRequestIdPrefix + "-A", local: true, wantError: exportPrefixError + longestRequestIdPrefix + "-A"}, + {name: "trailing delimiter removed", setting: "ABC-", local: true, want: "ABC"}, + {name: "inner punctuation and space removed", setting: "A.B C", local: true, want: "ABC"}, + {name: "only punctuation", setting: "-", local: true, wantError: exportPrefixError + "-"}, + {name: "non-ASCII punctuation kept", setting: "ABC—", local: true, wantError: exportPrefixError + "ABC—"}, + {name: "invalid default value", setting: " ", fallback: "ÅB-", local: true, wantError: exportPrefixError + "ÅB-"}, {name: "default value", setting: " ", fallback: "DEF", local: true, want: "DEF"}, {name: "direct branch inherits", setting: "ABC", branch: true}, {name: "unset", local: true}, diff --git a/migration/export-crosslink-directory-field-mapping.md b/migration/export-crosslink-directory-field-mapping.md index c4cb98678..8558652d8 100644 --- a/migration/export-crosslink-directory-field-mapping.md +++ b/migration/export-crosslink-directory-field-mapping.md @@ -181,7 +181,7 @@ exported defaults. | Presence of the ISO18626 endpoint | `data.illConfig.iso18626Vendor` | Set to `ReShare`. | | `app_setting.max_requests` | `data.illConfig.maxRequestsPerPatron` | Cast to an integer for the selected local entry. Values must be from 0 through 2147483647. The value is `null` for branches, which inherit it, and for other non-local pickup entries. | | `app_setting.check_duplicate_time` | `data.illConfig.duplicateCheckWindowHours` | Cast to an integer number of hours for the selected local entry. Values must be from 0 through 2147483647. The value is `null` for branches, which inherit it, and for other non-local pickup entries. | -| `app_setting.request_id_prefix` | `data.illConfig.requestIdPrefix` | Copied with surrounding spaces trimmed for the selected local entry, using the nonblank `st_value` and falling back to the nonblank `st_default_value`. The value is `null` when unset, for branches, which inherit it, and for other non-local pickup entries. A value longer than 32 characters, or containing anything other than ASCII letters and digits, fails the export before any records are emitted. | +| `app_setting.request_id_prefix` | `data.illConfig.requestIdPrefix` | Copied with surrounding spaces trimmed for the selected local entry, using the nonblank `st_value` and falling back to the nonblank `st_default_value`. The value is `null` when unset, for branches, which inherit it, and for other non-local pickup entries. ASCII punctuation and spaces are removed, because CrossLink adds its own `-` separator: `ALV-` becomes `ALV`. A value that is then empty, longer than 32 characters, or contains anything other than ASCII letters and digits fails the export before any records are emitted. | | `app_setting.default_copyright_type` resolved through `copyrightType` refdata | `data.illConfig.defaultCopyrightType` | Exported only on the selected local owner as a single object: `rdv_value` becomes `code` as its ISO18626 CopyrightCompliance code, and `rdv_label` becomes `defaultDesc`, falling back to the code for a null or whitespace-only label. | | Unsupported mod-rs agency-information behavior | `data.illConfig.includeRequestingAgencyInfo` | Set to `false`; mod-rs does not populate requesting-agency information. | | Unsupported mod-rs agency-information behavior | `data.illConfig.includeSupplierInfo` | Set to `false`; mod-rs does not populate supplier information. | diff --git a/migration/export-crosslink-directory.sql b/migration/export-crosslink-directory.sql index c066886f6..d9de7f428 100644 --- a/migration/export-crosslink-directory.sql +++ b/migration/export-crosslink-directory.sql @@ -286,6 +286,17 @@ SELECT FROM settings; CREATE TEMP TABLE crosslink_tenant_settings ON COMMIT DROP AS +SELECT + settings.*, + -- mod-rs appends the HRID number directly to the prefix, so prefixes often + -- end in a separator such as "ALV-". CrossLink adds its own "-" separator, + -- so ASCII punctuation and whitespace are dropped: "ALV-" becomes "ALV". + -- The C collation keeps the character classes ASCII-only whatever the + -- database locale, so non-ASCII characters such as "—" stay and fail + -- validation. + regexp_replace(settings.request_id_prefix_source COLLATE "C", + '[[:punct:][:space:]]', '', 'g') AS request_id_prefix +FROM ( SELECT max(coalesce(nullif(btrim(st_value), ''), nullif(btrim(st_default_value), ''))) FILTER (WHERE st_key = 'default_request_symbol') AS default_request_symbol, @@ -324,10 +335,11 @@ SELECT max(coalesce(nullif(btrim(st_value), ''), nullif(btrim(st_default_value), ''))) FILTER (WHERE st_key = 'default_copyright_type') AS default_copyright_type, max(coalesce(nullif(btrim(st_value), ''), nullif(btrim(st_default_value), ''))) - FILTER (WHERE st_key = 'request_id_prefix') AS request_id_prefix, + FILTER (WHERE st_key = 'request_id_prefix') AS request_id_prefix_source, max(coalesce(nullif(btrim(st_value), ''), nullif(btrim(st_default_value), ''))) FILTER (WHERE st_key = 'last_resort_lenders') AS last_resort_lenders -FROM app_setting; +FROM app_setting +) AS settings; -- ISO18626 CopyrightCompliance codes. mod-rs copyrightType values are these -- codes lowercased, so they are matched case-insensitively. @@ -380,10 +392,10 @@ DO $$ DECLARE problem text; BEGIN - SELECT request_id_prefix INTO problem FROM crosslink_tenant_settings + SELECT request_id_prefix_source INTO problem FROM crosslink_tenant_settings WHERE request_id_prefix !~ '^[A-Za-z0-9]{1,32}$'; IF problem IS NOT NULL THEN - RAISE EXCEPTION 'request_id_prefix must be at most 32 ASCII letters and digits: %', problem; + RAISE EXCEPTION 'request_id_prefix must be at most 32 ASCII letters and digits after removing punctuation and spaces: %', problem; END IF; END $$; diff --git a/migration/export-crosslink-open-patron-requests.sql b/migration/export-crosslink-open-patron-requests.sql index 01f564273..c5696912d 100644 --- a/migration/export-crosslink-open-patron-requests.sql +++ b/migration/export-crosslink-open-patron-requests.sql @@ -74,9 +74,11 @@ -- -- The preflight block runs before any NDJSON is emitted and stops the export if -- an open request or rota entry has an unmapped state or is missing required --- import data, or if a borrowing request's HRID is not alphanumeric. Broker --- generated IDs have the form PREFIX-N, so an alphanumeric HRID cannot collide --- with one. Borrowing requests without an HRID keep their UUID. +-- import data, or if a borrowing request's HRID is neither alphanumeric nor +-- PREFIX-N (as when the mod-rs prefix ends in "-"). Broker generated IDs have +-- the form PREFIX-N, so an alphanumeric HRID cannot collide with one. For a +-- PREFIX-N HRID, the broker import raises PREFIX's HRID sequence past N so new +-- IDs cannot collide either. Borrowing requests without an HRID keep their UUID. \set ON_ERROR_STOP on @@ -742,10 +744,10 @@ BEGIN UNION ALL SELECT 'request ' || pr_id || ': HRID ' || pr_hrid - || ' must contain only ASCII letters and digits' + || ' must be ASCII letters and digits, optionally followed by a hyphen and digits' FROM crosslink_open_requests WHERE import_side = 'borrowing' - AND pr_hrid !~ '^[A-Za-z0-9]+$' + AND pr_hrid !~ '^[A-Za-z0-9]+(-[0-9]+)?$' ) AS validation; IF problems IS NOT NULL THEN diff --git a/migration/scripts.md b/migration/scripts.md index c4d0aa2f0..4c625fc5f 100644 --- a/migration/scripts.md +++ b/migration/scripts.md @@ -59,9 +59,17 @@ It also exports the selected local owner's `defaultCopyrightType`, resolving the `default_copyright_type` setting through the `copyrightType` refdata vocabulary. An unset setting is omitted; a configured value without a unique refdata match fails the export. The `request_id_prefix` setting is exported as `requestIdPrefix` -on the selected local owner; its branches inherit it. A value longer than 32 -characters, or containing anything other than ASCII letters and digits, fails the -export. +on the selected local owner; its branches inherit it. ASCII punctuation and +spaces are removed, because CrossLink adds its own `-` separator: `ALV-` is +exported as `ALV`. A value that is then empty, longer than 32 characters, or +contains anything other than ASCII letters and digits fails the export. + +The patron-request export keeps mod-rs HRIDs as borrowing request IDs. An HRID +must be ASCII letters and digits, or such a prefix followed by `-` and digits +(for example `ALV-10008`). When the broker imports a borrowing request with a +`PREFIX-N` ID, it raises the HRID sequence for `PREFIX` past `N`, so new +CrossLink IDs cannot repeat a migrated one, even when the tenant creates requests +during the import. See the [ILL configuration field mappings](export-crosslink-directory-field-mapping.md#ill-configuration) for source fields, setting and label fallbacks, ordering, and missing-list behavior.