From 32c1ad58db1af32a64a540c242a9ef418d1642d4 Mon Sep 17 00:00:00 2001 From: Kurt Nordstrom Date: Mon, 12 Oct 2026 17:19:13 -0400 Subject: [PATCH 1/2] Use the directory yaml to populate the broker vendor profiles at compile-time --- broker/Dockerfile | 5 + broker/Dockerfile.dockerignore | 4 +- broker/Makefile | 22 ++- broker/profiles/builtin.go | 62 -------- broker/profiles/builtin/Alma.yaml | 12 -- broker/profiles/builtin/FOLIO.yaml | 13 -- broker/profiles/builtin/Generic.yaml | 16 -- broker/profiles/builtin/Koha.yaml | 18 --- broker/profiles/builtin/Sierra.yaml | 18 --- broker/profiles/builtin/profile.schema.json | 17 -- broker/profiles/builtin_test.go | 21 ++- broker/profiles/cmd/genprofiles/main.go | 159 +++++++++++++++++++ broker/profiles/cmd/genprofiles/main_test.go | 137 ++++++++++++++++ directory/host-profiles.md | 25 +-- 14 files changed, 346 insertions(+), 183 deletions(-) delete mode 100644 broker/profiles/builtin/Alma.yaml delete mode 100644 broker/profiles/builtin/FOLIO.yaml delete mode 100644 broker/profiles/builtin/Generic.yaml delete mode 100644 broker/profiles/builtin/Koha.yaml delete mode 100644 broker/profiles/builtin/Sierra.yaml delete mode 100644 broker/profiles/builtin/profile.schema.json create mode 100644 broker/profiles/cmd/genprofiles/main.go create mode 100644 broker/profiles/cmd/genprofiles/main_test.go diff --git a/broker/Dockerfile b/broker/Dockerfile index 0205b060e..9264f89ee 100644 --- a/broker/Dockerfile +++ b/broker/Dockerfile @@ -56,6 +56,11 @@ WORKDIR /app/broker # Copy Broker sources after dependencies have been downloaded. COPY broker/ ./ +# Generate host defaults from the directory schema before compiling broker. +RUN --mount=type=cache,sharing=shared,target=/go/pkg/mod \ + --mount=type=cache,sharing=shared,target=/root/.cache/go-build \ + go run ./profiles/cmd/genprofiles + # Build, reuse both module and build caches RUN --mount=type=cache,sharing=shared,target=/go/pkg/mod \ --mount=type=cache,sharing=shared,target=/root/.cache/go-build \ diff --git a/broker/Dockerfile.dockerignore b/broker/Dockerfile.dockerignore index 7a8ef0a78..97bc3d175 100644 --- a/broker/Dockerfile.dockerignore +++ b/broker/Dockerfile.dockerignore @@ -21,8 +21,8 @@ broker/**/*_test.go # Include OpenAPI specs !broker/oapi/*.yaml -# Include embedded built-in profiles -!broker/profiles/builtin/*.yaml +# Generate host profiles inside the build. +broker/profiles/builtin_gen.go # Include statemodels !broker/patron_request/service/statemodels diff --git a/broker/Makefile b/broker/Makefile index 9278343fa..3f47dc3a2 100644 --- a/broker/Makefile +++ b/broker/Makefile @@ -23,6 +23,11 @@ COVERAGE=coverage.out STATE_MODELS_JSON=patron_request/service/statemodels/state-models.json STATE_MODELS_YAML=../misc/state-models.yaml +# Host profiles +PROFILE_GEN = profiles/builtin_gen.go +PROFILE_SCHEMA = ../directory/api.yaml +PROFILE_GENERATOR = $(wildcard profiles/cmd/genprofiles/*.go) + # SQLC SQLC ?= $(GO) tool sqlc SQLC_CONFIG = sqlc/sqlc.yaml @@ -68,17 +73,25 @@ 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 generate-profiles 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 generate-profiles $(STATE_MODELS_JSON) generate-commit-id: $(COMMIT_ID) +generate-profiles: $(PROFILE_GEN) + +directory-generate: + $(MAKE) -C ../directory generate + +$(PROFILE_GEN): $(PROFILE_SCHEMA) $(PROFILE_GENERATOR) go.mod go.sum ../directory/go.mod ../directory/go.sum | directory-generate + $(GO) run ./profiles/cmd/genprofiles -input $(PROFILE_SCHEMA) -output $@ + generate-sqlc: $(SQL_GEN_OUT) generate-api: $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(IMPORT_OAPI_GEN) @@ -109,10 +122,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) $(PROFILE_GEN) $(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) $(PROFILE_GEN) $(GO) build -v -o archive ./cmd/archive check: generate @@ -154,6 +167,7 @@ clean: rm -f $(COMMIT_ID) rm -f $(STATE_MODELS_JSON) $(STATE_MODELS_JSON).tmp rmdir $(dir $(STATE_MODELS_JSON)) 2>/dev/null || true + rm -f $(PROFILE_GEN) rm -f $(SQL_GEN_OUT) rm -f $(OAPI_GEN) rm -f $(PR_OAPI_GEN) diff --git a/broker/profiles/builtin.go b/broker/profiles/builtin.go index 5aa75063e..37717faba 100644 --- a/broker/profiles/builtin.go +++ b/broker/profiles/builtin.go @@ -1,68 +1,6 @@ package profiles -import ( - "bytes" - "embed" - "encoding/json" - "fmt" - "strings" - - 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") - if err != nil { - panic(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 -} - -func decodeBuiltin(data []byte) (builtinProfile, error) { - var raw object - if err := yaml.Unmarshal(data, &raw); err != nil { - return builtinProfile{}, err - } - data, err := json.Marshal(raw) - if err != nil { - return builtinProfile{}, 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"` - } - decoder := json.NewDecoder(bytes.NewReader(data)) - decoder.DisallowUnknownFields() - if err := decoder.Decode(&config); err != nil { - return builtinProfile{}, err - } - var profile builtinProfile - err = json.Unmarshal(data, &profile) - return profile, err -} 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..fe5a52132 100644 --- a/broker/profiles/builtin_test.go +++ b/broker/profiles/builtin_test.go @@ -1,9 +1,11 @@ package profiles import ( + "os" "testing" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) func TestBuiltinDefinitions(t *testing.T) { @@ -14,18 +16,15 @@ func TestBuiltinDefinitions(t *testing.T) { before := asObject(builtins) _, 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 generated 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", - } { - _, err := decodeBuiltin([]byte(data)) - require.Error(t, err, data) +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)) + require.Equal(t, asObject(schema.Profiles), asObject(builtins)) } diff --git a/broker/profiles/cmd/genprofiles/main.go b/broker/profiles/cmd/genprofiles/main.go new file mode 100644 index 000000000..19177a9c9 --- /dev/null +++ b/broker/profiles/cmd/genprofiles/main.go @@ -0,0 +1,159 @@ +// Command genprofiles compiles directory host defaults into broker's profile table. +package main + +import ( + "bytes" + "encoding/json" + "flag" + "fmt" + "go/format" + "os" + "sort" + "strconv" + + dirapi "github.com/indexdata/crosslink/directory/api" + "gopkg.in/yaml.v3" +) + +func main() { + input := flag.String("input", "../directory/api.yaml", "directory OpenAPI schema") + output := flag.String("output", "profiles/builtin_gen.go", "generated Go output") + flag.Parse() + if err := run(*input, *output); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} + +func run(input, output string) error { + data, err := os.ReadFile(input) + if err != nil { + return fmt.Errorf("read directory schema: %w", err) + } + generated, err := generate(data) + if err != nil { + return err + } + if err := os.WriteFile(output, generated, 0600); err != nil { + return fmt.Errorf("write profile table: %w", err) + } + return nil +} + +func generate(data []byte) ([]byte, error) { + var schema struct { + Profiles map[string]map[string]any `yaml:"x-host-profiles"` + Components struct { + Schemas map[string]struct { + Enum []string `yaml:"enum"` + } `yaml:"schemas"` + } `yaml:"components"` + } + if err := yaml.Unmarshal(data, &schema); err != nil { + return nil, fmt.Errorf("decode directory schema: %w", err) + } + if len(schema.Profiles) == 0 { + return nil, fmt.Errorf("directory schema: missing or empty x-host-profiles") + } + 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 := map[string]bool{} + for _, name := range schema.Components.Schemas["HostLmsVendor"].Enum { + vendors[name] = true + } + var out bytes.Buffer + out.WriteString("// Code generated by genprofiles from directory/api.yaml; DO NOT EDIT.\n\npackage profiles\n\n// Merge copies nested maps before applying directory overrides.\nvar builtins = map[string]builtinProfile{\n") + for _, name := range keys(schema.Profiles) { + if !vendors[name] { + return nil, fmt.Errorf("host profile %s: not in HostLmsVendor enum", name) + } + raw := schema.Profiles[name] + // JSON decoding accepts case variants; table generation uses exact keys. + for _, section := range 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: %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: %w", name, err) + } + // Normalize values to the same JSON map types used by the resolver. + var profile map[string]any + if err := json.Unmarshal(encoded, &profile); err != nil { + return nil, fmt.Errorf("host profile %s: %w", name, err) + } + lms, err := literal(profile["lmsConfig"]) + if err != nil { + return nil, fmt.Errorf("host profile %s: %w", name, err) + } + catalog, err := literal(profile["catalogConfig"]) + if err != nil { + return nil, fmt.Errorf("host profile %s: %w", name, err) + } + fmt.Fprintf(&out, "%q: {LMS: %s, Catalog: %s},\n", name, lms, catalog) + } + out.WriteString("}\n") + return format.Source(out.Bytes()) +} + +func keys[V any](m map[string]V) []string { + result := make([]string, 0, len(m)) + for key := range m { + result = append(result, key) + } + sort.Strings(result) + return result +} + +func literal(value any) (string, error) { + switch v := value.(type) { + case nil: + return "nil", nil + case string: + return strconv.Quote(v), nil + case bool: + return strconv.FormatBool(v), nil + case float64: + return "float64(" + strconv.FormatFloat(v, 'g', -1, 64) + ")", nil + case []any: + var out bytes.Buffer + out.WriteString("[]any{") + for _, item := range v { + itemLiteral, err := literal(item) + if err != nil { + return "", err + } + out.WriteString(itemLiteral) + out.WriteByte(',') + } + out.WriteByte('}') + return out.String(), nil + case map[string]any: + var out bytes.Buffer + out.WriteString("object{") + for _, key := range keys(v) { + itemLiteral, err := literal(v[key]) + if err != nil { + return "", err + } + fmt.Fprintf(&out, "%q: %s,", key, itemLiteral) + } + out.WriteByte('}') + return out.String(), nil + default: + return "", fmt.Errorf("unexpected JSON value %T", value) + } +} diff --git a/broker/profiles/cmd/genprofiles/main_test.go b/broker/profiles/cmd/genprofiles/main_test.go new file mode 100644 index 000000000..996159be7 --- /dev/null +++ b/broker/profiles/cmd/genprofiles/main_test.go @@ -0,0 +1,137 @@ +package main + +import ( + "flag" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func schema(t *testing.T) string { + t.Helper() + data, err := os.ReadFile("../../../../directory/api.yaml") + require.NoError(t, err) + return string(data) +} + +func TestGenerate(t *testing.T) { + input := schema(t) + first, err := generate([]byte(input)) + require.NoError(t, err) + second, err := generate([]byte(input)) + require.NoError(t, err) + require.Equal(t, first, second) + checked, err := os.ReadFile("../../builtin_gen.go") + require.NoError(t, err) + require.Equal(t, checked, first) + output := string(first) + for _, value := range []string{`"mainField": "952"`, `"value": "0"`, `"ncipNamespaceEnabled": false`, `"allCirculations": true`, `LMS: object{}`, `"CHECK SHELVES"`} { + require.Contains(t, output, value) + } +} + +func TestGenerateRejectsInvalidDefinitions(t *testing.T) { + input := schema(t) + cases := []struct{ name, input, message string }{ + {"malformed", "x-host-profiles: [", "decode directory schema"}, + {"missing", "openapi: 3.1.0", "missing or empty x-host-profiles"}, + {"empty", "x-host-profiles: {}", "missing or empty x-host-profiles"}, + {"missing supported", strings.Replace(input, " Koha:\n", " Other:\n", 1), "host profile Koha: missing definition"}, + {"unknown name", strings.Replace(input, " Alma:\n", " Extra: {}\n Alma:\n", 1), "host profile Extra: not in HostLmsVendor enum"}, + {"wrong section", strings.Replace(input, " lmsConfig: {}", " illConfig: {}", 1), `host profile Alma: invalid section "illConfig"; allowed sections are lmsConfig and catalogConfig`}, + {"unknown nested field", strings.Replace(input, "availabilityRule: availableNow", "unknownSetting: availableNow", 1), "host profile Alma: json: unknown field"}, + {"incorrect type", strings.Replace(input, "allCirculations: true", "allCirculations: yesPlease", 1), "host profile Alma: json: cannot unmarshal"}, + {"nonfinite value", strings.Replace(input, "allCirculations: true", "allCirculations: .nan", 1), "host profile Alma: json: unsupported value"}, + {"numeric MARC code", strings.Replace(input, `mainField: "952"`, "mainField: 952", 1), "host profile Koha: json: cannot unmarshal"}, + {"duplicate field", strings.Replace(input, " lmsConfig: {}", " lmsConfig: {}\n lmsConfig: {}", 1), "decode directory schema"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := generate([]byte(tc.input)) + require.ErrorContains(t, err, tc.message) + }) + } +} + +func TestGenerateRejectsSectionCapitalization(t *testing.T) { + input := schema(t) + for _, tc := range []struct{ canonical, invalid string }{ + {"lmsConfig", "LMSConfig"}, + {"lmsConfig", "lmsconfig"}, + {"catalogConfig", "CatalogConfig"}, + {"catalogConfig", "catalogconfig"}, + } { + for _, alongsideCanonical := range []bool{false, true} { + name := tc.invalid + replacement := " " + tc.invalid + ":" + if alongsideCanonical { + name += " alongside canonical" + replacement = " " + tc.invalid + ": {}\n " + tc.canonical + ":" + } + t.Run(name, func(t *testing.T) { + invalid := strings.Replace(input, " "+tc.canonical+":", replacement, 1) + generated, err := generate([]byte(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, generated) + }) + } + } +} + +func TestRunPreservesOutputOnInvalidSection(t *testing.T) { + dir := t.TempDir() + input, output := filepath.Join(dir, "api.yaml"), filepath.Join(dir, "profiles.go") + require.NoError(t, os.WriteFile(input, []byte(schema(t)), 0600)) + require.NoError(t, run(input, output)) + before, err := os.ReadFile(output) + require.NoError(t, err) + invalid := strings.Replace(schema(t), " Koha:\n lmsConfig:", " Koha:\n LMSConfig:", 1) + require.NoError(t, os.WriteFile(input, []byte(invalid), 0600)) + require.ErrorContains(t, run(input, output), `host profile Koha: invalid section "LMSConfig"`) + after, err := os.ReadFile(output) + require.NoError(t, err) + require.Equal(t, before, after) +} + +func TestRun(t *testing.T) { + dir := t.TempDir() + input, output := filepath.Join(dir, "api.yaml"), filepath.Join(dir, "profiles.go") + require.NoError(t, os.WriteFile(input, []byte(schema(t)), 0600)) + require.NoError(t, run(input, output)) + data, err := os.ReadFile(output) + require.NoError(t, err) + require.Contains(t, string(data), "var builtins") + require.ErrorContains(t, run(filepath.Join(dir, "missing"), output), "read directory schema") + require.ErrorContains(t, run(input, dir), "write profile table") + require.NoError(t, os.WriteFile(input, []byte("{}"), 0600)) + require.ErrorContains(t, run(input, output), "missing or empty x-host-profiles") +} + +func TestLiteralValues(t *testing.T) { + value, err := literal(map[string]any{"empty": []any{}, "nil": nil, "number": float64(1.5)}) + require.NoError(t, err) + require.Equal(t, `object{"empty": []any{},"nil": nil,"number": float64(1.5),}`, value) + for _, unsupported := range []any{1, []any{1}, map[string]any{"bad": 1}} { + _, err := literal(unsupported) + require.ErrorContains(t, err, "unexpected JSON value int") + } +} + +func TestCommand(t *testing.T) { + output := filepath.Join(t.TempDir(), "profiles.go") + oldArgs, oldFlags := os.Args, flag.CommandLine + defer func() { os.Args, flag.CommandLine = oldArgs, oldFlags }() + os.Args = []string{"genprofiles", "-input", "../../../../directory/api.yaml", "-output", output} + flag.CommandLine = flag.NewFlagSet("genprofiles", flag.ContinueOnError) + main() + generated, err := os.ReadFile(output) + require.NoError(t, err) + expected, err := generate([]byte(schema(t))) + require.NoError(t, err) + require.Equal(t, expected, generated) +} diff --git a/directory/host-profiles.md b/directory/host-profiles.md index c2645d803..aa790ae20 100644 --- a/directory/host-profiles.md +++ b/directory/host-profiles.md @@ -37,18 +37,23 @@ 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. + +`make -C broker generate-profiles` validates these definitions against the +generated directory API types and generates a Go table compiled into broker. +The target is included in broker generation and builds; Docker generates the +table inside the build. The generated table is ignored and removed by `clean`. +Changing a profile requires regenerating and rebuilding broker, not migrating +directory records. 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 generated +defaults. This build-time source change preserves existing resolution behavior. | Profile | Catalog defaults | Circulation defaults | | --- | --- | --- | From e562b2175d0b7f228bf8009d372129e90cd35dd0 Mon Sep 17 00:00:00 2001 From: Kurt Nordstrom Date: Thu, 15 Oct 2026 12:56:31 -0400 Subject: [PATCH 2/2] Change to get the profile/vendor data in the earlier directory yaml consumption step --- broker/Dockerfile | 5 - broker/Dockerfile.dockerignore | 3 - broker/Makefile | 19 +-- broker/profiles/builtin.go | 79 +++++++++ broker/profiles/builtin_test.go | 152 +++++++++++++++++- broker/profiles/cmd/genprofiles/main.go | 159 ------------------- broker/profiles/cmd/genprofiles/main_test.go | 137 ---------------- broker/profiles/resolve.go | 4 + directory/host-profiles.md | 23 +-- 9 files changed, 251 insertions(+), 330 deletions(-) delete mode 100644 broker/profiles/cmd/genprofiles/main.go delete mode 100644 broker/profiles/cmd/genprofiles/main_test.go diff --git a/broker/Dockerfile b/broker/Dockerfile index 9264f89ee..0205b060e 100644 --- a/broker/Dockerfile +++ b/broker/Dockerfile @@ -56,11 +56,6 @@ WORKDIR /app/broker # Copy Broker sources after dependencies have been downloaded. COPY broker/ ./ -# Generate host defaults from the directory schema before compiling broker. -RUN --mount=type=cache,sharing=shared,target=/go/pkg/mod \ - --mount=type=cache,sharing=shared,target=/root/.cache/go-build \ - go run ./profiles/cmd/genprofiles - # Build, reuse both module and build caches RUN --mount=type=cache,sharing=shared,target=/go/pkg/mod \ --mount=type=cache,sharing=shared,target=/root/.cache/go-build \ diff --git a/broker/Dockerfile.dockerignore b/broker/Dockerfile.dockerignore index 97bc3d175..c54502c7d 100644 --- a/broker/Dockerfile.dockerignore +++ b/broker/Dockerfile.dockerignore @@ -21,9 +21,6 @@ broker/**/*_test.go # Include OpenAPI specs !broker/oapi/*.yaml -# Generate host profiles inside the build. -broker/profiles/builtin_gen.go - # Include statemodels !broker/patron_request/service/statemodels diff --git a/broker/Makefile b/broker/Makefile index 3f47dc3a2..a70d0b882 100644 --- a/broker/Makefile +++ b/broker/Makefile @@ -23,11 +23,6 @@ COVERAGE=coverage.out STATE_MODELS_JSON=patron_request/service/statemodels/state-models.json STATE_MODELS_YAML=../misc/state-models.yaml -# Host profiles -PROFILE_GEN = profiles/builtin_gen.go -PROFILE_SCHEMA = ../directory/api.yaml -PROFILE_GENERATOR = $(wildcard profiles/cmd/genprofiles/*.go) - # SQLC SQLC ?= $(GO) tool sqlc SQLC_CONFIG = sqlc/sqlc.yaml @@ -73,25 +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 generate-profiles directory-generate 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 generate-profiles $(STATE_MODELS_JSON) +generate: generate-commit-id generate-sqlc generate-api directory-generate $(STATE_MODELS_JSON) generate-commit-id: $(COMMIT_ID) -generate-profiles: $(PROFILE_GEN) - directory-generate: $(MAKE) -C ../directory generate -$(PROFILE_GEN): $(PROFILE_SCHEMA) $(PROFILE_GENERATOR) go.mod go.sum ../directory/go.mod ../directory/go.sum | directory-generate - $(GO) run ./profiles/cmd/genprofiles -input $(PROFILE_SCHEMA) -output $@ - generate-sqlc: $(SQL_GEN_OUT) generate-api: $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(IMPORT_OAPI_GEN) @@ -122,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) $(PROFILE_GEN) +$(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) $(PROFILE_GEN) +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 @@ -167,7 +157,6 @@ clean: rm -f $(COMMIT_ID) rm -f $(STATE_MODELS_JSON) $(STATE_MODELS_JSON).tmp rmdir $(dir $(STATE_MODELS_JSON)) 2>/dev/null || true - rm -f $(PROFILE_GEN) rm -f $(SQL_GEN_OUT) rm -f $(OAPI_GEN) rm -f $(PR_OAPI_GEN) diff --git a/broker/profiles/builtin.go b/broker/profiles/builtin.go index 37717faba..cee21f67a 100644 --- a/broker/profiles/builtin.go +++ b/broker/profiles/builtin.go @@ -1,6 +1,85 @@ package profiles +import ( + "bytes" + "encoding/json" + "fmt" + "maps" + "slices" + "sync" + + dirapi "github.com/indexdata/crosslink/directory/api" +) + type builtinProfile struct { LMS object `json:"lmsConfig"` Catalog object `json:"catalogConfig"` } + +// 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 { + return nil, fmt.Errorf("load bundled directory specification: %w", err) + } + return decodeBuiltins(data) +}) + +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"` + } + if err := json.Unmarshal(data, &schema); err != nil { + return nil, fmt.Errorf("decode directory specification: %w", err) + } + if len(schema.Profiles) == 0 { + return nil, fmt.Errorf("directory specification: missing or empty x-host-profiles") + } + 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 + } + return profiles, nil +} diff --git a/broker/profiles/builtin_test.go b/broker/profiles/builtin_test.go index fe5a52132..18fa99845 100644 --- a/broker/profiles/builtin_test.go +++ b/broker/profiles/builtin_test.go @@ -1,22 +1,28 @@ 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 generated defaults") + require.Equal(t, before, asObject(builtins), "directory overrides must not change cached defaults") } func TestBuiltinDefinitionsMatchDirectorySchema(t *testing.T) { @@ -26,5 +32,147 @@ func TestBuiltinDefinitionsMatchDirectorySchema(t *testing.T) { 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"}, + } { + 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/cmd/genprofiles/main.go b/broker/profiles/cmd/genprofiles/main.go deleted file mode 100644 index 19177a9c9..000000000 --- a/broker/profiles/cmd/genprofiles/main.go +++ /dev/null @@ -1,159 +0,0 @@ -// Command genprofiles compiles directory host defaults into broker's profile table. -package main - -import ( - "bytes" - "encoding/json" - "flag" - "fmt" - "go/format" - "os" - "sort" - "strconv" - - dirapi "github.com/indexdata/crosslink/directory/api" - "gopkg.in/yaml.v3" -) - -func main() { - input := flag.String("input", "../directory/api.yaml", "directory OpenAPI schema") - output := flag.String("output", "profiles/builtin_gen.go", "generated Go output") - flag.Parse() - if err := run(*input, *output); err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } -} - -func run(input, output string) error { - data, err := os.ReadFile(input) - if err != nil { - return fmt.Errorf("read directory schema: %w", err) - } - generated, err := generate(data) - if err != nil { - return err - } - if err := os.WriteFile(output, generated, 0600); err != nil { - return fmt.Errorf("write profile table: %w", err) - } - return nil -} - -func generate(data []byte) ([]byte, error) { - var schema struct { - Profiles map[string]map[string]any `yaml:"x-host-profiles"` - Components struct { - Schemas map[string]struct { - Enum []string `yaml:"enum"` - } `yaml:"schemas"` - } `yaml:"components"` - } - if err := yaml.Unmarshal(data, &schema); err != nil { - return nil, fmt.Errorf("decode directory schema: %w", err) - } - if len(schema.Profiles) == 0 { - return nil, fmt.Errorf("directory schema: missing or empty x-host-profiles") - } - 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 := map[string]bool{} - for _, name := range schema.Components.Schemas["HostLmsVendor"].Enum { - vendors[name] = true - } - var out bytes.Buffer - out.WriteString("// Code generated by genprofiles from directory/api.yaml; DO NOT EDIT.\n\npackage profiles\n\n// Merge copies nested maps before applying directory overrides.\nvar builtins = map[string]builtinProfile{\n") - for _, name := range keys(schema.Profiles) { - if !vendors[name] { - return nil, fmt.Errorf("host profile %s: not in HostLmsVendor enum", name) - } - raw := schema.Profiles[name] - // JSON decoding accepts case variants; table generation uses exact keys. - for _, section := range 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: %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: %w", name, err) - } - // Normalize values to the same JSON map types used by the resolver. - var profile map[string]any - if err := json.Unmarshal(encoded, &profile); err != nil { - return nil, fmt.Errorf("host profile %s: %w", name, err) - } - lms, err := literal(profile["lmsConfig"]) - if err != nil { - return nil, fmt.Errorf("host profile %s: %w", name, err) - } - catalog, err := literal(profile["catalogConfig"]) - if err != nil { - return nil, fmt.Errorf("host profile %s: %w", name, err) - } - fmt.Fprintf(&out, "%q: {LMS: %s, Catalog: %s},\n", name, lms, catalog) - } - out.WriteString("}\n") - return format.Source(out.Bytes()) -} - -func keys[V any](m map[string]V) []string { - result := make([]string, 0, len(m)) - for key := range m { - result = append(result, key) - } - sort.Strings(result) - return result -} - -func literal(value any) (string, error) { - switch v := value.(type) { - case nil: - return "nil", nil - case string: - return strconv.Quote(v), nil - case bool: - return strconv.FormatBool(v), nil - case float64: - return "float64(" + strconv.FormatFloat(v, 'g', -1, 64) + ")", nil - case []any: - var out bytes.Buffer - out.WriteString("[]any{") - for _, item := range v { - itemLiteral, err := literal(item) - if err != nil { - return "", err - } - out.WriteString(itemLiteral) - out.WriteByte(',') - } - out.WriteByte('}') - return out.String(), nil - case map[string]any: - var out bytes.Buffer - out.WriteString("object{") - for _, key := range keys(v) { - itemLiteral, err := literal(v[key]) - if err != nil { - return "", err - } - fmt.Fprintf(&out, "%q: %s,", key, itemLiteral) - } - out.WriteByte('}') - return out.String(), nil - default: - return "", fmt.Errorf("unexpected JSON value %T", value) - } -} diff --git a/broker/profiles/cmd/genprofiles/main_test.go b/broker/profiles/cmd/genprofiles/main_test.go deleted file mode 100644 index 996159be7..000000000 --- a/broker/profiles/cmd/genprofiles/main_test.go +++ /dev/null @@ -1,137 +0,0 @@ -package main - -import ( - "flag" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/stretchr/testify/require" -) - -func schema(t *testing.T) string { - t.Helper() - data, err := os.ReadFile("../../../../directory/api.yaml") - require.NoError(t, err) - return string(data) -} - -func TestGenerate(t *testing.T) { - input := schema(t) - first, err := generate([]byte(input)) - require.NoError(t, err) - second, err := generate([]byte(input)) - require.NoError(t, err) - require.Equal(t, first, second) - checked, err := os.ReadFile("../../builtin_gen.go") - require.NoError(t, err) - require.Equal(t, checked, first) - output := string(first) - for _, value := range []string{`"mainField": "952"`, `"value": "0"`, `"ncipNamespaceEnabled": false`, `"allCirculations": true`, `LMS: object{}`, `"CHECK SHELVES"`} { - require.Contains(t, output, value) - } -} - -func TestGenerateRejectsInvalidDefinitions(t *testing.T) { - input := schema(t) - cases := []struct{ name, input, message string }{ - {"malformed", "x-host-profiles: [", "decode directory schema"}, - {"missing", "openapi: 3.1.0", "missing or empty x-host-profiles"}, - {"empty", "x-host-profiles: {}", "missing or empty x-host-profiles"}, - {"missing supported", strings.Replace(input, " Koha:\n", " Other:\n", 1), "host profile Koha: missing definition"}, - {"unknown name", strings.Replace(input, " Alma:\n", " Extra: {}\n Alma:\n", 1), "host profile Extra: not in HostLmsVendor enum"}, - {"wrong section", strings.Replace(input, " lmsConfig: {}", " illConfig: {}", 1), `host profile Alma: invalid section "illConfig"; allowed sections are lmsConfig and catalogConfig`}, - {"unknown nested field", strings.Replace(input, "availabilityRule: availableNow", "unknownSetting: availableNow", 1), "host profile Alma: json: unknown field"}, - {"incorrect type", strings.Replace(input, "allCirculations: true", "allCirculations: yesPlease", 1), "host profile Alma: json: cannot unmarshal"}, - {"nonfinite value", strings.Replace(input, "allCirculations: true", "allCirculations: .nan", 1), "host profile Alma: json: unsupported value"}, - {"numeric MARC code", strings.Replace(input, `mainField: "952"`, "mainField: 952", 1), "host profile Koha: json: cannot unmarshal"}, - {"duplicate field", strings.Replace(input, " lmsConfig: {}", " lmsConfig: {}\n lmsConfig: {}", 1), "decode directory schema"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - _, err := generate([]byte(tc.input)) - require.ErrorContains(t, err, tc.message) - }) - } -} - -func TestGenerateRejectsSectionCapitalization(t *testing.T) { - input := schema(t) - for _, tc := range []struct{ canonical, invalid string }{ - {"lmsConfig", "LMSConfig"}, - {"lmsConfig", "lmsconfig"}, - {"catalogConfig", "CatalogConfig"}, - {"catalogConfig", "catalogconfig"}, - } { - for _, alongsideCanonical := range []bool{false, true} { - name := tc.invalid - replacement := " " + tc.invalid + ":" - if alongsideCanonical { - name += " alongside canonical" - replacement = " " + tc.invalid + ": {}\n " + tc.canonical + ":" - } - t.Run(name, func(t *testing.T) { - invalid := strings.Replace(input, " "+tc.canonical+":", replacement, 1) - generated, err := generate([]byte(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, generated) - }) - } - } -} - -func TestRunPreservesOutputOnInvalidSection(t *testing.T) { - dir := t.TempDir() - input, output := filepath.Join(dir, "api.yaml"), filepath.Join(dir, "profiles.go") - require.NoError(t, os.WriteFile(input, []byte(schema(t)), 0600)) - require.NoError(t, run(input, output)) - before, err := os.ReadFile(output) - require.NoError(t, err) - invalid := strings.Replace(schema(t), " Koha:\n lmsConfig:", " Koha:\n LMSConfig:", 1) - require.NoError(t, os.WriteFile(input, []byte(invalid), 0600)) - require.ErrorContains(t, run(input, output), `host profile Koha: invalid section "LMSConfig"`) - after, err := os.ReadFile(output) - require.NoError(t, err) - require.Equal(t, before, after) -} - -func TestRun(t *testing.T) { - dir := t.TempDir() - input, output := filepath.Join(dir, "api.yaml"), filepath.Join(dir, "profiles.go") - require.NoError(t, os.WriteFile(input, []byte(schema(t)), 0600)) - require.NoError(t, run(input, output)) - data, err := os.ReadFile(output) - require.NoError(t, err) - require.Contains(t, string(data), "var builtins") - require.ErrorContains(t, run(filepath.Join(dir, "missing"), output), "read directory schema") - require.ErrorContains(t, run(input, dir), "write profile table") - require.NoError(t, os.WriteFile(input, []byte("{}"), 0600)) - require.ErrorContains(t, run(input, output), "missing or empty x-host-profiles") -} - -func TestLiteralValues(t *testing.T) { - value, err := literal(map[string]any{"empty": []any{}, "nil": nil, "number": float64(1.5)}) - require.NoError(t, err) - require.Equal(t, `object{"empty": []any{},"nil": nil,"number": float64(1.5),}`, value) - for _, unsupported := range []any{1, []any{1}, map[string]any{"bad": 1}} { - _, err := literal(unsupported) - require.ErrorContains(t, err, "unexpected JSON value int") - } -} - -func TestCommand(t *testing.T) { - output := filepath.Join(t.TempDir(), "profiles.go") - oldArgs, oldFlags := os.Args, flag.CommandLine - defer func() { os.Args, flag.CommandLine = oldArgs, oldFlags }() - os.Args = []string{"genprofiles", "-input", "../../../../directory/api.yaml", "-output", output} - flag.CommandLine = flag.NewFlagSet("genprofiles", flag.ContinueOnError) - main() - generated, err := os.ReadFile(output) - require.NoError(t, err) - expected, err := generate([]byte(schema(t))) - require.NoError(t, err) - require.Equal(t, expected, generated) -} 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 aa790ae20..ffa2df26e 100644 --- a/directory/host-profiles.md +++ b/directory/host-profiles.md @@ -42,18 +42,23 @@ The directory's [OpenAPI schema](api.yaml) defines vendor defaults in the directory's `lmsConfig` and `catalogConfig` structure; `illConfig` remains independent and is not part of a host profile. -`make -C broker generate-profiles` validates these definitions against the -generated directory API types and generates a Go table compiled into broker. -The target is included in broker generation and builds; Docker generates the -table inside the build. The generated table is ignored and removed by `clean`. -Changing a profile requires regenerating and rebuilding broker, not migrating -directory records. Broker does not parse profile YAML or fetch profiles over HTTP -at runtime. WMS and Aleph remain unsupported. +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. Resolution retains value -origins and applies explicit directory overrides without modifying the generated -defaults. This build-time source change preserves existing resolution behavior. +origins and applies explicit directory overrides without modifying the cached +defaults. Loading bundled definitions preserves existing resolution behavior. | Profile | Catalog defaults | Circulation defaults | | --- | --- | --- |