diff --git a/broker/Dockerfile.dockerignore b/broker/Dockerfile.dockerignore index 7a8ef0a78..c54502c7d 100644 --- a/broker/Dockerfile.dockerignore +++ b/broker/Dockerfile.dockerignore @@ -21,9 +21,6 @@ broker/**/*_test.go # Include OpenAPI specs !broker/oapi/*.yaml -# Include embedded built-in profiles -!broker/profiles/builtin/*.yaml - # Include statemodels !broker/patron_request/service/statemodels diff --git a/broker/Makefile b/broker/Makefile index 9278343fa..a70d0b882 100644 --- a/broker/Makefile +++ b/broker/Makefile @@ -68,17 +68,20 @@ IMPORT_OAPI_CFG = $(IMPORT_OAPI_DIR)/import-cfg.yaml IMPORT_OAPI_SPEC = $(IMPORT_OAPI_DIR)/open-api.yaml IMPORT_OAPI_GEN = import/oapi/import_openapi_gen.go -.PHONY: all docker generate generate-sqlc generate-api generate-commit-id check run fmt fmt-check clean clean-build-caches view-coverage deps-update tools-update lint vulncheck check-coverage +.PHONY: all docker generate generate-sqlc generate-api generate-commit-id directory-generate check run fmt fmt-check clean clean-build-caches view-coverage deps-update tools-update lint vulncheck check-coverage all: $(BINARY) archive docker: generate cd .. && $(DOCKER) build -t indexdata/$(MODULE):latest -f ./$(MODULE)/Dockerfile . -generate: generate-commit-id generate-sqlc generate-api $(STATE_MODELS_JSON) +generate: generate-commit-id generate-sqlc generate-api directory-generate $(STATE_MODELS_JSON) generate-commit-id: $(COMMIT_ID) +directory-generate: + $(MAKE) -C ../directory generate + generate-sqlc: $(SQL_GEN_OUT) generate-api: $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(IMPORT_OAPI_GEN) @@ -109,10 +112,10 @@ $(SQL_GEN_OUT): $(SQL_GEN_IN) $(SQLC_CONFIG) $(COMMIT_ID): $(GIT_COMMIT_DEPS) commit_id="$$( $(GIT) rev-parse --short HEAD )" && printf '%s' "$$commit_id" > $(COMMIT_ID) -$(BINARY): $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(IMPORT_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) +$(BINARY): $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(IMPORT_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) ../directory/api.yaml | directory-generate $(GO) build -v -o $(BINARY) ./$(MAIN_PACKAGE) -archive: $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(IMPORT_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) +archive: $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(IMPORT_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) ../directory/api.yaml | directory-generate $(GO) build -v -o archive ./cmd/archive check: generate diff --git a/broker/profiles/builtin.go b/broker/profiles/builtin.go index 5aa75063e..cee21f67a 100644 --- a/broker/profiles/builtin.go +++ b/broker/profiles/builtin.go @@ -2,67 +2,84 @@ package profiles import ( "bytes" - "embed" "encoding/json" "fmt" - "strings" + "maps" + "slices" + "sync" dirapi "github.com/indexdata/crosslink/directory/api" - "gopkg.in/yaml.v3" ) -//go:embed builtin/*.yaml -var builtinFiles embed.FS - type builtinProfile struct { LMS object `json:"lmsConfig"` Catalog object `json:"catalogConfig"` } -// Load once; merge copies nested maps before applying directory overrides. -var builtins = loadBuiltins() - -func loadBuiltins() map[string]builtinProfile { - files, err := builtinFiles.ReadDir("builtin") +// Cache definitions and errors alike: the bundled specification cannot change +// during this process. Resolve must never modify these shared defaults. +var loadBuiltins = sync.OnceValues(func() (map[string]builtinProfile, error) { + data, err := dirapi.GetSpecJSON() if err != nil { - panic(err) + return nil, fmt.Errorf("load bundled directory specification: %w", err) } - result := map[string]builtinProfile{} - for _, file := range files { - data, err := builtinFiles.ReadFile("builtin/" + file.Name()) - if err != nil { - panic(err) - } - profile, err := decodeBuiltin(data) - if err != nil { - panic(fmt.Errorf("built-in profile %s: %w", file.Name(), err)) - } - result[strings.TrimSuffix(file.Name(), ".yaml")] = profile - } - return result -} + return decodeBuiltins(data) +}) -func decodeBuiltin(data []byte) (builtinProfile, error) { - var raw object - if err := yaml.Unmarshal(data, &raw); err != nil { - return builtinProfile{}, err +func decodeBuiltins(data []byte) (map[string]builtinProfile, error) { + var schema struct { + Profiles map[string]map[string]json.RawMessage `json:"x-host-profiles"` + Components struct { + Schemas map[string]struct { + Enum []string `json:"enum"` + } `json:"schemas"` + } `json:"components"` } - data, err := json.Marshal(raw) - if err != nil { - return builtinProfile{}, err + if err := json.Unmarshal(data, &schema); err != nil { + return nil, fmt.Errorf("decode directory specification: %w", err) } - // Validate names and types against the directory API, including nested fields. - // Only host configuration sections belong in these profiles. - var config struct { - LMS *dirapi.LmsConfig `json:"lmsConfig"` - Catalog *dirapi.CatalogConfig `json:"catalogConfig"` + if len(schema.Profiles) == 0 { + return nil, fmt.Errorf("directory specification: missing or empty x-host-profiles") } - decoder := json.NewDecoder(bytes.NewReader(data)) - decoder.DisallowUnknownFields() - if err := decoder.Decode(&config); err != nil { - return builtinProfile{}, err + for _, name := range []string{"Generic", "Alma", "Sierra", "Koha", "FOLIO"} { + if schema.Profiles[name] == nil { + return nil, fmt.Errorf("host profile %s: missing definition", name) + } + } + vendors := schema.Components.Schemas["HostLmsVendor"].Enum + profiles := make(map[string]builtinProfile, len(schema.Profiles)) + for _, name := range slices.Sorted(maps.Keys(schema.Profiles)) { + if !slices.Contains(vendors, name) { + return nil, fmt.Errorf("host profile %s: not in HostLmsVendor enum", name) + } + raw := schema.Profiles[name] + // JSON struct decoding accepts case variants, but profile sections must + // use their canonical names, including when both spellings are present. + for _, section := range slices.Sorted(maps.Keys(raw)) { + if section != "lmsConfig" && section != "catalogConfig" { + return nil, fmt.Errorf("host profile %s: invalid section %q; allowed sections are lmsConfig and catalogConfig", name, section) + } + } + encoded, err := json.Marshal(raw) + if err != nil { + return nil, fmt.Errorf("host profile %s: encode definition: %w", name, err) + } + var config struct { + LMS *dirapi.LmsConfig `json:"lmsConfig"` + Catalog *dirapi.CatalogConfig `json:"catalogConfig"` + } + decoder := json.NewDecoder(bytes.NewReader(encoded)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&config); err != nil { + return nil, fmt.Errorf("host profile %s: validate definition: %w", name, err) + } + // Retain raw maps rather than re-encoding the typed configuration, which + // could materialize omitted fields or omit explicit zero values. + var profile builtinProfile + if err := json.Unmarshal(encoded, &profile); err != nil { + return nil, fmt.Errorf("host profile %s: decode definition: %w", name, err) + } + profiles[name] = profile } - var profile builtinProfile - err = json.Unmarshal(data, &profile) - return profile, err + return profiles, nil } diff --git a/broker/profiles/builtin/Alma.yaml b/broker/profiles/builtin/Alma.yaml deleted file mode 100644 index 1b6062716..000000000 --- a/broker/profiles/builtin/Alma.yaml +++ /dev/null @@ -1,12 +0,0 @@ -# yaml-language-server: $schema=./profile.schema.json ---- -# Overrides of Generic defaults. -lmsConfig: {} -catalogConfig: - holdingsFormat: - opac: - availabilityRule: availableNow - requireLocalLocation: true - includeItemId: true - includeItemLoanPolicy: true - allCirculations: true diff --git a/broker/profiles/builtin/FOLIO.yaml b/broker/profiles/builtin/FOLIO.yaml deleted file mode 100644 index 0835fcd4f..000000000 --- a/broker/profiles/builtin/FOLIO.yaml +++ /dev/null @@ -1,13 +0,0 @@ -# yaml-language-server: $schema=./profile.schema.json ---- -# Overrides of Generic defaults. Connection details and local policy belong in the directory. -lmsConfig: {} -catalogConfig: - holdingsFormat: - opac: - availabilityRule: availableNow - requireLocalLocation: true - includeItemId: true - includeItemLoanPolicy: true - allCirculations: true - includeTemporaryLocation: true diff --git a/broker/profiles/builtin/Generic.yaml b/broker/profiles/builtin/Generic.yaml deleted file mode 100644 index 4cea81b36..000000000 --- a/broker/profiles/builtin/Generic.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# yaml-language-server: $schema=./profile.schema.json ---- -# Common circulation defaults. Conditional catalog constructor defaults remain in Go. -lmsConfig: - ncipNamespaceEnabled: true - bibIdNormalization: none - requestItemRequestType: Page - requestItemRequestScopeType: Item - requestItemBibIdCode: SYSNUMBER - requestItemPickupLocationEnabled: true - lookupUserEnabled: true - acceptItemEnabled: true - checkInItemEnabled: true - checkOutItemEnabled: true - requestItemEnabled: true -catalogConfig: {} diff --git a/broker/profiles/builtin/Koha.yaml b/broker/profiles/builtin/Koha.yaml deleted file mode 100644 index 323ac943e..000000000 --- a/broker/profiles/builtin/Koha.yaml +++ /dev/null @@ -1,18 +0,0 @@ -# yaml-language-server: $schema=./profile.schema.json ---- -# Overrides of Generic defaults. MARC codes and values must remain strings. -lmsConfig: - ncipNamespaceEnabled: false -catalogConfig: - holdingsFormat: - marc: - mainField: "952" - locationSubField: b - shelvingLocationSubField: c - callNumberSubField: o - availability: - - subField: "7" - operator: equals - value: "0" - - subField: q - operator: absent diff --git a/broker/profiles/builtin/Sierra.yaml b/broker/profiles/builtin/Sierra.yaml deleted file mode 100644 index 62817d96e..000000000 --- a/broker/profiles/builtin/Sierra.yaml +++ /dev/null @@ -1,18 +0,0 @@ -# yaml-language-server: $schema=./profile.schema.json ---- -# Overrides of Generic defaults. -lmsConfig: - requestItemRequestType: Hold - requestItemRequestScopeType: Title - bibIdNormalization: sierra -catalogConfig: - holdingsFormat: - opac: - availabilityRule: publicNote - availablePublicNotes: - - AVAILABLE - - CHECK SHELVES - - CHECK SHELF - shelvingLocationSource: localLocation - includeItemId: false - includeItemLoanPolicy: false diff --git a/broker/profiles/builtin/profile.schema.json b/broker/profiles/builtin/profile.schema.json deleted file mode 100644 index d2663de8b..000000000 --- a/broker/profiles/builtin/profile.schema.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "title": "Crosslink host profile", - "description": "Built-in host defaults. Nested configuration fields are validated against the directory API by the Go profile loader.", - "type": "object", - "additionalProperties": false, - "properties": { - "lmsConfig": { - "type": "object", - "description": "LMS circulation defaults, using directory lmsConfig fields. Empty inherits Generic defaults." - }, - "catalogConfig": { - "type": "object", - "description": "Catalog defaults, using directory catalogConfig fields. Empty inherits Generic defaults." - } - } -} diff --git a/broker/profiles/builtin_test.go b/broker/profiles/builtin_test.go index 247dce2ce..18fa99845 100644 --- a/broker/profiles/builtin_test.go +++ b/broker/profiles/builtin_test.go @@ -1,31 +1,178 @@ package profiles import ( + "encoding/json" + "errors" + "os" + "sync" "testing" + dirapi "github.com/indexdata/crosslink/directory/api" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) func TestBuiltinDefinitions(t *testing.T) { + builtins, err := loadBuiltins() + require.NoError(t, err) require.Len(t, builtins, 5) for _, name := range []string{"Generic", "Alma", "Sierra", "Koha", "FOLIO"} { require.Contains(t, builtins, name) } before := asObject(builtins) - _, err := Resolve(entry(t, `{"lmsConfig":{"vendor":"Koha","ncipNamespaceEnabled":true},"catalogConfig":{"holdingsFormat":{"marc":{"mainField":"999","availability":[]}}}}`)) + _, err = Resolve(entry(t, `{"lmsConfig":{"vendor":"Koha","ncipNamespaceEnabled":true},"catalogConfig":{"holdingsFormat":{"marc":{"mainField":"999","availability":[]}}}}`)) require.NoError(t, err) - require.Equal(t, before, asObject(builtins), "directory overrides must not change embedded defaults") + require.Equal(t, before, asObject(builtins), "directory overrides must not change cached defaults") } -func TestDecodeBuiltinRejectsInvalidConfiguration(t *testing.T) { - for _, data := range []string{ - "illConfig:\n iso18626Vendor: ReShare\n", - "lmsConfig:\n misspelledSetting: true\n", - "catalogConfig:\n holdingsFormat:\n opac:\n includeTemporaryLocation: yesPlease\n", - "catalogConfig:\n holdingsFormat:\n marc:\n mainField: 952\n", - "lmsConfig: {}\nlmsConfig: {}\n", +func TestBuiltinDefinitionsMatchDirectorySchema(t *testing.T) { + data, err := os.ReadFile("../../directory/api.yaml") + require.NoError(t, err) + var schema struct { + Profiles map[string]any `yaml:"x-host-profiles"` + } + require.NoError(t, yaml.Unmarshal(data, &schema)) + builtins, err := loadBuiltins() + require.NoError(t, err) + require.Equal(t, asObject(schema.Profiles), asObject(builtins)) +} + +func TestDecodeBuiltinsRejectsInvalidDefinitions(t *testing.T) { + cases := []struct { + name, message string + mutate func(object) + }{ + {"missing", "missing or empty x-host-profiles", func(s object) { delete(s, "x-host-profiles") }}, + {"empty", "missing or empty x-host-profiles", func(s object) { s["x-host-profiles"] = object{} }}, + {"missing supported", "host profile Koha: missing definition", func(s object) { delete(s["x-host-profiles"].(object), "Koha") }}, + {"null supported", "host profile Koha: missing definition", func(s object) { s["x-host-profiles"].(object)["Koha"] = nil }}, + {"unknown name", "host profile Extra: not in HostLmsVendor enum", func(s object) { s["x-host-profiles"].(object)["Extra"] = object{} }}, + {"missing enum", "not in HostLmsVendor enum", func(s object) { delete(s, "components") }}, + {"wrong section", `invalid section "illConfig"`, func(s object) { s["x-host-profiles"].(object)["Alma"].(object)["illConfig"] = object{} }}, + {"unknown nested field", `json: unknown field "unknownSetting"`, func(s object) { + s["x-host-profiles"].(object)["Alma"].(object)["lmsConfig"] = object{"unknownSetting": true} + }}, + {"incorrect type", "json: cannot unmarshal", func(s object) { + s["x-host-profiles"].(object)["Alma"].(object)["lmsConfig"] = object{"ncipNamespaceEnabled": "yesPlease"} + }}, + {"numeric MARC code", "host profile Koha: validate definition: json: cannot unmarshal", func(s object) { + s["x-host-profiles"].(object)["Koha"].(object)["catalogConfig"] = object{"holdingsFormat": object{"marc": object{"mainField": 952}}} + }}, + } + data, err := dirapi.GetSpecJSON() + require.NoError(t, err) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var schema object + require.NoError(t, json.Unmarshal(data, &schema)) + tc.mutate(schema) + invalid, err := json.Marshal(schema) + require.NoError(t, err) + profiles, err := decodeBuiltins(invalid) + require.ErrorContains(t, err, tc.message) + require.Nil(t, profiles) + }) + } + t.Run("malformed JSON", func(t *testing.T) { + profiles, err := decodeBuiltins([]byte(`{"x-host-profiles":`)) + require.ErrorContains(t, err, "decode directory specification") + var syntaxError *json.SyntaxError + require.ErrorAs(t, err, &syntaxError) + require.Nil(t, profiles) + }) +} + +func TestDecodeBuiltinsRejectsSectionCapitalization(t *testing.T) { + data, err := dirapi.GetSpecJSON() + require.NoError(t, err) + for _, tc := range []struct{ canonical, invalid string }{ + {"lmsConfig", "LMSConfig"}, + {"lmsConfig", "lmsconfig"}, + {"catalogConfig", "CatalogConfig"}, + {"catalogConfig", "catalogconfig"}, } { - _, err := decodeBuiltin([]byte(data)) - require.Error(t, err, data) + for _, alongsideCanonical := range []bool{false, true} { + name := tc.invalid + if alongsideCanonical { + name += " alongside canonical" + } + t.Run(name, func(t *testing.T) { + var schema object + require.NoError(t, json.Unmarshal(data, &schema)) + alma := schema["x-host-profiles"].(object)["Alma"].(object) + alma[tc.invalid] = alma[tc.canonical] + if !alongsideCanonical { + delete(alma, tc.canonical) + } + invalid, err := json.Marshal(schema) + require.NoError(t, err) + profiles, err := decodeBuiltins(invalid) + require.ErrorContains(t, err, "host profile Alma: invalid section") + require.ErrorContains(t, err, `"`+tc.invalid+`"`) + require.ErrorContains(t, err, "allowed sections are lmsConfig and catalogConfig") + require.Nil(t, profiles) + }) + } } } + +func TestDecodeBuiltinsPreservesExplicitValues(t *testing.T) { + data, err := dirapi.GetSpecJSON() + require.NoError(t, err) + var schema object + require.NoError(t, json.Unmarshal(data, &schema)) + schema["x-host-profiles"].(object)["Koha"] = object{ + "lmsConfig": object{"ncipNamespaceEnabled": false, "requestItemRequestType": ""}, + "catalogConfig": object{"holdingsFormat": object{"marc": object{ + "mainField": "952", "availability": []any{}, + }}}, + } + data, err = json.Marshal(schema) + require.NoError(t, err) + profiles, err := decodeBuiltins(data) + require.NoError(t, err) + require.Equal(t, object{"ncipNamespaceEnabled": false, "requestItemRequestType": ""}, profiles["Koha"].LMS) + require.Equal(t, object{"holdingsFormat": object{"marc": object{"mainField": "952", "availability": []any{}}}}, profiles["Koha"].Catalog) + require.Empty(t, profiles["Alma"].LMS) +} + +func TestResolveReturnsCachedBuiltinError(t *testing.T) { + original := loadBuiltins + t.Cleanup(func() { loadBuiltins = original }) + cause := errors.New("invalid bundled profiles") + calls := 0 + loadBuiltins = sync.OnceValues(func() (map[string]builtinProfile, error) { + calls++ + return nil, cause + }) + for range 2 { + effective, err := Resolve(dirapi.Entry{}) + require.ErrorIs(t, err, cause) + require.ErrorContains(t, err, "resolve host profiles") + require.Nil(t, effective) + } + require.Equal(t, 1, calls) +} + +func TestConcurrentProfileResolution(t *testing.T) { + builtins, err := loadBuiltins() + require.NoError(t, err) + before := asObject(builtins) + // Parallel subtests finish before the parent resumes, so the final comparison + // checks the shared definitions after all concurrent overrides have resolved. + t.Run("concurrent overrides", func(t *testing.T) { + for _, name := range []string{"Generic", "Alma", "Sierra", "Koha", "FOLIO"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + for range 10 { + effective, err := Resolve(entry(t, `{"lmsConfig":{"vendor":"`+name+`","ncipNamespaceEnabled":false},"catalogConfig":{"holdingsFormat":{"marc":{"mainField":"999","availability":[]}}}}`)) + require.NoError(t, err) + require.False(t, *effective.LMS.NcipNamespaceEnabled) + require.Equal(t, "999", *effective.Catalog.HoldingsFormat.Marc.MainField) + require.Empty(t, *effective.Catalog.HoldingsFormat.Marc.Availability) + } + }) + } + }) + require.Equal(t, before, asObject(builtins)) +} diff --git a/broker/profiles/resolve.go b/broker/profiles/resolve.go index ed8bb52d4..ac498fa80 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -83,6 +83,10 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { if err := supported(profile, "catalogConfig.profile"); err != nil { return nil, err } + builtins, err := loadBuiltins() + if err != nil { + return nil, fmt.Errorf("resolve host profiles: %w", err) + } e := &Effective{LMSVendor: vendor, CatalogProfile: profile, Origins: map[string]string{}} l, c := object{}, object{} merge(l, builtins["Generic"].LMS, "lmsConfig", "Generic", e.Origins) diff --git a/directory/host-profiles.md b/directory/host-profiles.md index c2645d803..ffa2df26e 100644 --- a/directory/host-profiles.md +++ b/directory/host-profiles.md @@ -37,18 +37,28 @@ fallbacks remain unchanged. ## Built-in profiles -The broker embeds one [YAML file per supported profile](../broker/profiles/builtin) -using Go's `go:embed`. These files use the directory's `lmsConfig` and -`catalogConfig` structure and are the source of vendor defaults. Vendor files -contain overrides of Generic defaults; an empty section inherits those defaults. -`illConfig` remains independent and is not part of a host profile. +The directory's [OpenAPI schema](api.yaml) defines vendor defaults in +`x-host-profiles`. This is the sole source of profile values. Each profile uses +the directory's `lmsConfig` and `catalogConfig` structure; `illConfig` remains +independent and is not part of a host profile. + +Directory's existing API generation embeds the complete specification, including +`x-host-profiles`, in its Go package. Broker reads that bundled JSON through +`dirapi.GetSpecJSON()` on the first profile resolution, validates definitions +against the generated Directory API types, and caches the definitions or error +for the lifetime of the process. Invalid definitions return a resolution error; +they do not silently fall back to Generic. No separate profile generation step +or generated profile table is needed. + +Changing a profile requires regenerating Directory sources and rebuilding broker, +not migrating Directory records. Broker generation prepares Directory sources; +Docker also generates them inside the build. Broker does not parse profile YAML +or fetch profiles over HTTP at runtime. WMS and Aleph remain unsupported. Conditional catalog defaults (query language, parser replacement, and record -syntax based on the effective parser) remain in Go. The resolver checks YAML -field names and types against the directory API, retains value origins, and -applies explicit directory overrides without modifying the embedded defaults. -Changing a built-in file requires rebuilding the broker, not migrating directory -records. WMS and Aleph remain unsupported and have no built-in files. +syntax based on the effective parser) remain in Go. Resolution retains value +origins and applies explicit directory overrides without modifying the cached +defaults. Loading bundled definitions preserves existing resolution behavior. | Profile | Catalog defaults | Circulation defaults | | --- | --- | --- |