diff --git a/broker/Dockerfile.dockerignore b/broker/Dockerfile.dockerignore index c54502c7d..7a8ef0a78 100644 --- a/broker/Dockerfile.dockerignore +++ b/broker/Dockerfile.dockerignore @@ -21,6 +21,9 @@ 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/catalog/adapter_metaproxy.go b/broker/catalog/adapter_metaproxy.go index c120ee64e..4e172815c 100644 --- a/broker/catalog/adapter_metaproxy.go +++ b/broker/catalog/adapter_metaproxy.go @@ -11,8 +11,12 @@ type MetaproxyLookupAdapter struct { } func NewMetaproxyLookupAdapter(config dirapi.ZoomConfig, metaproxyUrl string, queryBuilder LookupQueryBuilder, holdingsParser HoldingsParser, metadataParser MetadataParser) (LookupAdapter, error) { + schema := "marcxml" + if config.Options != nil && (*config.Options)["preferredRecordSyntax"] == "opac" { + schema = "opac" + } a := &MetaproxyLookupAdapter{ - holdingsLookupAdapter: CreateSruLookupAdapter(http.DefaultClient, []string{metaproxyUrl}, config.Address, queryBuilder, holdingsParser, metadataParser, "marcxml"), + holdingsLookupAdapter: CreateSruLookupAdapter(http.DefaultClient, []string{metaproxyUrl}, config.Address, queryBuilder, holdingsParser, metadataParser, schema), } return a, nil } diff --git a/broker/catalog/creator.go b/broker/catalog/creator.go index 2ac305f1f..0beb7fbc1 100644 --- a/broker/catalog/creator.go +++ b/broker/catalog/creator.go @@ -1,9 +1,10 @@ package catalog import ( + "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/ill_db" ) type LookupAdapterCreator interface { - GetAdapter(peer ill_db.Peer) (LookupAdapter, error) + GetAdapter(ctx common.ExtendedContext, peer ill_db.Peer) (LookupAdapter, error) } diff --git a/broker/catalog/creator_impl.go b/broker/catalog/creator_impl.go index 12eb41ac5..8b8d282ce 100644 --- a/broker/catalog/creator_impl.go +++ b/broker/catalog/creator_impl.go @@ -3,7 +3,9 @@ package catalog import ( "fmt" + "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/ill_db" + "github.com/indexdata/crosslink/broker/profiles" dirapi "github.com/indexdata/crosslink/directory/api" ) @@ -54,9 +56,15 @@ func getHoldingsParser(config *dirapi.HoldingsParserConfig) (HoldingsParser, err return nil, fmt.Errorf("catalogConfig.holdingsFormat must set marc, opac, reservoir, or marc21plus1 properties") } -func (c *LookupAdapterCreatorImpl) GetAdapter(peer ill_db.Peer) (LookupAdapter, error) { +func (c *LookupAdapterCreatorImpl) GetAdapter(ctx common.ExtendedContext, peer ill_db.Peer) (LookupAdapter, error) { entry := peer.CustomData - config := entry.CatalogConfig + effective, err := profiles.Resolve(entry) + if err != nil { + return nil, err + } + // Diagnostics excludes endpoints, credentials, and other sensitive settings. + ctx.Logger().Debug("resolved host profiles", "configuration", effective.Diagnostics()) + config := effective.Catalog // CatalogConfig also contains settings unrelated to availability, such as // metadataUpdateMode. Only an SRU or ZOOM definition enables the check. if config == nil || (config.Sru == nil && config.Zoom == nil) { diff --git a/broker/catalog/creator_test.go b/broker/catalog/creator_test.go index 9734069c9..9b240ce42 100644 --- a/broker/catalog/creator_test.go +++ b/broker/catalog/creator_test.go @@ -1,17 +1,20 @@ package catalog import ( + "context" "testing" + "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/ill_db" dirapi "github.com/indexdata/crosslink/directory/api" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetAdapterEmpty(t *testing.T) { creator := NewLookupAdapterCreator(LookupAdapterZoom, "") peer := ill_db.Peer{} - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.NoError(t, err) assert.Nil(t, aa) } @@ -19,7 +22,7 @@ func TestGetAdapterEmpty(t *testing.T) { func TestGetAdapterOtherNoConfig(t *testing.T) { creator := NewLookupAdapterCreator("other", "") peer := ill_db.Peer{} - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.NoError(t, err) assert.Nil(t, aa) } @@ -63,13 +66,34 @@ func TestGetAdapterBadParser(t *testing.T) { Zoom: &dirapi.ZoomConfig{ Address: "a", }, + HoldingsFormat: &dirapi.HoldingsParserConfig{ + Marc: &dirapi.MarcHoldingsParserConfig{}, + Opac: &dirapi.OpacHoldingsParserConfig{}, + }, + }, + }, + } + adapter, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) + require.ErrorContains(t, err, "exactly one parser") + require.Nil(t, adapter) +} + +func TestGetAdapterEmptyHoldingsUsesMarcDefaults(t *testing.T) { + creator := NewLookupAdapterCreator(LookupAdapterZoom, "") + peer := ill_db.Peer{ + CustomData: dirapi.Entry{ + CatalogConfig: &dirapi.CatalogConfig{ + Sru: &dirapi.SruConfig{Address: "https://catalog.example/sru"}, HoldingsFormat: &dirapi.HoldingsParserConfig{}, }, }, } - _, err := creator.GetAdapter(peer) - assert.Error(t, err) - assert.Contains(t, err.Error(), "must set marc") + adapter, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) + require.NoError(t, err) + require.IsType(t, &SruLookupAdapter{}, adapter) + sru := adapter.(*SruLookupAdapter) + require.Equal(t, NewMarcHoldingsParser(dirapi.MarcHoldingsParserConfig{}), sru.holdingsParser) + require.Equal(t, "marcxml", sru.recordSchema) } func TestGetAdapterOtherWithConfig(t *testing.T) { @@ -83,7 +107,7 @@ func TestGetAdapterOtherWithConfig(t *testing.T) { }, }, } - _, err := creator.GetAdapter(peer) + _, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.Error(t, err) assert.Contains(t, err.Error(), "unsupported lookup adapter type: other") } @@ -96,7 +120,7 @@ func TestGetAdapterMetadataOnly(t *testing.T) { CatalogConfig: &dirapi.CatalogConfig{MetadataUpdateMode: &mode}, }, } - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.NoError(t, err) assert.Nil(t, aa) } @@ -112,7 +136,7 @@ func TestGetAdapterMock(t *testing.T) { }, } creator := NewLookupAdapterCreator(LookupAdapterMock, "") - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.NoError(t, err) assert.IsType(t, &MockLookupAdapter{}, aa) } @@ -128,7 +152,7 @@ func TestGetAdapterZoom(t *testing.T) { }, } creator := NewLookupAdapterCreator(LookupAdapterZoom, "") - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) if !cgoEnabled() { assert.Error(t, err) assert.Contains(t, err.Error(), "requires cgo") @@ -150,7 +174,7 @@ func TestGetAdapterMetaproxy(t *testing.T) { }, } creator := NewLookupAdapterCreator(LookupAdapterMetaproxy, "http://metaproxy.indexdata.com") - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.NoError(t, err) assert.IsType(t, &MetaproxyLookupAdapter{}, aa) } @@ -166,7 +190,7 @@ func TestGetAdapterMetaproxyMissingProxy(t *testing.T) { }, } creator := NewLookupAdapterCreator(LookupAdapterMetaproxy, "") - _, err := creator.GetAdapter(peer) + _, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.Error(t, err) assert.Contains(t, err.Error(), "METAPROXY_URL") } @@ -182,7 +206,7 @@ func TestGetAdapterSRU(t *testing.T) { }, } creator := NewLookupAdapterCreator(LookupAdapterZoom, "") - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) assert.NoError(t, err) assert.IsType(t, &SruLookupAdapter{}, aa) } diff --git a/broker/catalog/gvi_holdings_test.go b/broker/catalog/gvi_holdings_test.go index 6c6a67fa1..74423d7b7 100644 --- a/broker/catalog/gvi_holdings_test.go +++ b/broker/catalog/gvi_holdings_test.go @@ -1,10 +1,12 @@ package catalog import ( + "context" "net/http" "net/http/httptest" "testing" + "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/ill_db" dirapi "github.com/indexdata/crosslink/directory/api" "github.com/stretchr/testify/assert" @@ -416,7 +418,7 @@ func TestGviHoldings(t *testing.T) { }, } - aa, err := creator.GetAdapter(peer) + aa, err := creator.GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), peer) if cgoEnabled() { assert.NoError(t, err) assert.NotNil(t, aa) diff --git a/broker/catalog/holdings_parser_marc.go b/broker/catalog/holdings_parser_marc.go index 79ee6429d..5d65e8ee3 100644 --- a/broker/catalog/holdings_parser_marc.go +++ b/broker/catalog/holdings_parser_marc.go @@ -14,7 +14,7 @@ type MarcHoldingsParser struct { } func NewMarcHoldingsParser(config dirapi.MarcHoldingsParserConfig) HoldingsParser { - if config.MainField == nil && config.LocationSubField == nil && config.ShelvingLocationSubField == nil && config.CallNumberSubField == nil && config.ItemIdSubField == nil && config.RestrictedSubField == nil { + if config.MainField == nil && config.LocationSubField == nil && config.ShelvingLocationSubField == nil && config.CallNumberSubField == nil && config.ItemIdSubField == nil && config.RestrictedSubField == nil && config.Availability == nil { config.MainField = NewString("852") config.LocationSubField = NewString("b") config.ShelvingLocationSubField = NewString("c") @@ -66,6 +66,22 @@ func (p *MarcHoldingsParser) Parse(record []byte, params LookupParams) ([]Holdin restricted = true } } + if p.config.Availability != nil { + for _, rule := range *p.config.Availability { + found, matches := false, false + for _, sub := range field.Subfield { + if sub.Code == rule.SubField { + found = true + if rule.Value != nil && string(sub.Text) == *rule.Value { + matches = true + } + } + } + if (rule.Operator == "absent" && found) || (rule.Operator == "equals" && !matches) { + restricted = true + } + } + } if !restricted && location != "" { holdings = append(holdings, Holding{ Location: location, diff --git a/broker/catalog/holdings_parser_opac.go b/broker/catalog/holdings_parser_opac.go index 43a4aefeb..d5e5f4b79 100644 --- a/broker/catalog/holdings_parser_opac.go +++ b/broker/catalog/holdings_parser_opac.go @@ -3,47 +3,72 @@ package catalog import ( "encoding/xml" "fmt" + "slices" "strings" dirapi "github.com/indexdata/crosslink/directory/api" "github.com/indexdata/crosslink/marcxml" ) -type OpacHoldingsParser struct{} +type OpacHoldingsParser struct { + config dirapi.OpacHoldingsParserConfig +} func NewOpacHoldingsParser(config dirapi.OpacHoldingsParserConfig) HoldingsParser { - return &OpacHoldingsParser{} + return &OpacHoldingsParser{config: config} +} +func enabled(value *bool, fallback bool) bool { + if value != nil { + return *value + } + return fallback } func (p *OpacHoldingsParser) Parse(record []byte, params LookupParams) ([]Holding, error) { var opacRecord marcxml.OpacRecord - err := xml.Unmarshal(record, &opacRecord) - if err != nil { + if err := xml.Unmarshal(record, &opacRecord); err != nil { return nil, fmt.Errorf("failed to unmarshal OPAC XML: %w", err) } var result []Holding for _, holding := range opacRecord.Holdings.Holding { - availableNow := false - itemId := "" - itemLoanPolicy := "" + if enabled(p.config.RequireLocalLocation, false) && strings.TrimSpace(holding.LocalLocation) == "" { + continue + } + base := Holding{Location: holding.LocalLocation, ShelvingLocation: holding.ShelvingLocation, CallNumber: holding.CallNumber} + if p.config.ShelvingLocationSource != nil && *p.config.ShelvingLocationSource == "localLocation" { + base.ShelvingLocation = holding.LocalLocation + } + if p.config.AvailabilityRule != nil && *p.config.AvailabilityRule == "publicNote" { + if p.config.AvailablePublicNotes != nil && slices.Contains(*p.config.AvailablePublicNotes, holding.PublicNote) { + result = append(result, base) + } + continue + } + if holding.Circulations == nil { + continue + } for _, circ := range holding.Circulations.Circulation { - // regrettably, YAZ uses 0 or 1 to indicate availability, instead of a boolean value - if circ.AvailableNow.Value == "1" { - itemId = circ.ItemId - itemLoanPolicy = strings.TrimSpace(circ.AvailableThru) - availableNow = true + if circ.AvailableNow.Value != "1" { + continue + } + h := base + if enabled(p.config.IncludeItemId, true) { + h.ItemId = circ.ItemId + } + if enabled(p.config.IncludeItemLoanPolicy, true) { + h.ItemLoanPolicy = strings.TrimSpace(circ.AvailableThru) + } + if enabled(p.config.IncludeTemporaryLocation, false) { + // Supplier policy and ordering consume the effective shelving location. + if temporaryLocation := strings.TrimSpace(circ.TemporaryLocation); temporaryLocation != "" { + h.ShelvingLocation = temporaryLocation + } + } + result = append(result, h) + if !enabled(p.config.AllCirculations, false) { break } } - if availableNow { - result = append(result, Holding{ - Location: holding.LocalLocation, - ShelvingLocation: holding.ShelvingLocation, - CallNumber: holding.CallNumber, - ItemId: itemId, - ItemLoanPolicy: itemLoanPolicy, - }) - } } return result, nil } diff --git a/broker/catalog/profiles_test.go b/broker/catalog/profiles_test.go new file mode 100644 index 000000000..71281424f --- /dev/null +++ b/broker/catalog/profiles_test.go @@ -0,0 +1,158 @@ +package catalog + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/indexdata/crosslink/broker/common" + "github.com/indexdata/crosslink/broker/ill_db" + "github.com/indexdata/crosslink/broker/profiles" + dirapi "github.com/indexdata/crosslink/directory/api" + "github.com/stretchr/testify/require" +) + +func profileParser(t *testing.T, name string) HoldingsParser { + t.Helper() + var entry dirapi.Entry + require.NoError(t, json.Unmarshal([]byte(`{"catalogConfig":{"profile":"`+name+`"}}`), &entry)) + e, err := profiles.Resolve(entry) + require.NoError(t, err) + p, err := getHoldingsParser(e.Catalog.HoldingsFormat) + require.NoError(t, err) + return p +} +func TestKohaAvailability(t *testing.T) { + p := profileParser(t, "Koha") + for _, tc := range []struct { + sub string + available bool + }{ + {`0`, true}, + {``, false}, {`1`, false}, + {`0`, false}, + {`02026-01-01`, false}, + } { + record := `MAINSTACKSQA1IGNORED` + tc.sub + `` + h, err := p.Parse([]byte(record), LookupParams{}) + require.NoError(t, err) + if tc.available { + require.Len(t, h, 1) + require.Equal(t, Holding{Location: "MAIN", ShelvingLocation: "STACKS", CallNumber: "QA1"}, h[0]) + } else { + require.Empty(t, h) + } + } +} +func TestSierraAvailability(t *testing.T) { + p := profileParser(t, "Sierra") + for _, note := range []string{"AVAILABLE", "CHECK SHELVES", "CHECK SHELF", "available", " AVAILABLE", "ON LOAN", ""} { + h, err := p.Parse([]byte(`MAINIGNOREDQA1`+note+``), LookupParams{}) + require.NoError(t, err) + if note == "AVAILABLE" || note == "CHECK SHELVES" || note == "CHECK SHELF" { + require.Equal(t, []Holding{{Location: "MAIN", ShelvingLocation: "MAIN", CallNumber: "QA1"}}, h) + } else { + require.Empty(t, h) + } + } +} +func TestOpacProfiles(t *testing.T) { + record := `MAINSTACKSQA11 LOAN TEMP2` + for _, name := range []string{"Alma", "FOLIO"} { + h, err := profileParser(t, name).Parse([]byte(record), LookupParams{}) + require.NoError(t, err) + require.Len(t, h, 2) + require.Equal(t, "1", h[0].ItemId) + require.Equal(t, "LOAN", h[0].ItemLoanPolicy) + if name == "FOLIO" { + require.Equal(t, "TEMP", h[0].ShelvingLocation) + } else { + require.Equal(t, "STACKS", h[0].ShelvingLocation) + } + require.Equal(t, "STACKS", h[1].ShelvingLocation) + } + // An explicitly selected generic OPAC parser keeps the old first-circulation behavior. + h, err := NewOpacHoldingsParser(dirapi.OpacHoldingsParserConfig{}).Parse([]byte(record), LookupParams{}) + require.NoError(t, err) + require.Len(t, h, 2) + require.Empty(t, h[1].Location) +} +func TestFolioEffectiveShelvingLocation(t *testing.T) { + for _, tc := range []struct { + name, temporary, override, want string + }{ + {name: "temporary", temporary: ` TEMP `, want: "TEMP"}, + {name: "missing", want: "STACKS"}, + {name: "empty", temporary: ``, want: "STACKS"}, + {name: "whitespace", temporary: ` `, want: "STACKS"}, + {name: "disabled", temporary: `TEMP`, override: `,"holdingsFormat":{"opac":{"includeTemporaryLocation":false}}`, want: "STACKS"}, + } { + t.Run(tc.name, func(t *testing.T) { + var entry dirapi.Entry + require.NoError(t, json.Unmarshal([]byte(`{"catalogConfig":{"profile":"FOLIO"`+tc.override+`}}`), &entry)) + effective, err := profiles.Resolve(entry) + require.NoError(t, err) + parser, err := getHoldingsParser(effective.Catalog.HoldingsFormat) + require.NoError(t, err) + record := `MAINSTACKS` + tc.temporary + `` + holdings, err := parser.Parse([]byte(record), LookupParams{}) + require.NoError(t, err) + require.Equal(t, []Holding{{Location: "MAIN", ShelvingLocation: tc.want}}, holdings) + }) + } +} + +func TestProfileLookupAggregationAndFallback(t *testing.T) { + for _, name := range []string{"Alma", "Sierra", "Koha", "FOLIO"} { + t.Run(name, func(t *testing.T) { + var queries []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + q := r.URL.Query().Get("x-pquery") + queries = append(queries, q) + fmt.Fprint(w, `1.14`) + for i := 0; i < 4; i++ { + available := strings.Contains(q, "1=8") + var record string + if name == "Koha" { + status := "1" + if available { + status = "0" + } + record = `MAIN` + status + `` + } else { + flag, note := "0", "ON LOAN" + if available { + flag, note = "1", "AVAILABLE" + } + record = `MAIN` + note + `` + } + fmt.Fprintf(w, `%s`, record) + } + fmt.Fprint(w, ``) + })) + defer server.Close() + var entry dirapi.Entry + require.NoError(t, json.Unmarshal([]byte(`{"lmsConfig":{"vendor":"`+name+`"},"catalogConfig":{"sru":{"address":"`+server.URL+`"}}}`), &entry)) + ad, err := NewLookupAdapterCreator(LookupAdapterZoom, "").GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), ill_db.Peer{CustomData: entry}) + require.NoError(t, err) + result, err := ad.Lookup(LookupParams{Identifier: "id", Isbn: "isbn", Issn: "issn", Title: "title"}) + require.NoError(t, err) + holdings, err := result.GetHoldings() + require.NoError(t, err) + require.Len(t, holdings, 4) + require.Equal(t, []string{`@attr 1=12 "id"`, `@attr 1=7 "isbn"`, `@attr 1=8 "issn"`}, queries) + }) + } +} +func TestProfileOnlyDoesNotEnableCatalog(t *testing.T) { + var entry dirapi.Entry + require.NoError(t, json.Unmarshal([]byte(`{"lmsConfig":{"vendor":"Sierra"},"catalogConfig":{"profile":"Koha"}}`), &entry)) + a, err := NewLookupAdapterCreator(LookupAdapterZoom, "").GetAdapter(common.CreateExtCtxWithArgs(context.Background(), nil), ill_db.Peer{CustomData: entry}) + require.NoError(t, err) + require.Nil(t, a) +} diff --git a/broker/go.mod b/broker/go.mod index 4b77ec4cd..1204f7261 100644 --- a/broker/go.mod +++ b/broker/go.mod @@ -156,7 +156,7 @@ require ( google.golang.org/grpc v1.83.1 // indirect google.golang.org/protobuf v1.36.12 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + gopkg.in/yaml.v3 v3.0.1 ) tool ( diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index ca59b2815..6c916fb96 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -4,6 +4,7 @@ import ( "encoding/xml" "errors" "fmt" + "github.com/indexdata/crosslink/broker/profiles" "net/http" "strings" @@ -50,6 +51,14 @@ func (l *LmsAdapterNcip) requestItemRequestScopeType() string { } func CreateLmsAdapterNcip(lmsConfig dirapi.LmsConfig) (LmsAdapter, error) { + effective, err := profiles.Resolve(dirapi.Entry{LmsConfig: &lmsConfig}) + if err != nil { + return nil, err + } + return createResolvedLmsAdapterNcip(*effective.LMS) +} + +func createResolvedLmsAdapterNcip(lmsConfig dirapi.LmsConfig) (LmsAdapter, error) { l := &LmsAdapterNcip{config: lmsConfig} toAgency := "default-to-agency" if l.config.ToAgency != nil { @@ -65,7 +74,8 @@ func CreateLmsAdapterNcip(lmsConfig dirapi.LmsConfig) (LmsAdapter, error) { if l.config.FromAgency == "" { return nil, fmt.Errorf("missing From Agency in LMS configuration") } - l.ncipClient = ncipclient.NewNcipClient(http.DefaultClient, l.config.Address, l.config.FromAgency, toAgency, FromAgencyAuthentication) + disableNamespace := l.config.NcipNamespaceEnabled != nil && !*l.config.NcipNamespaceEnabled + l.ncipClient = ncipclient.NewNcipClient(http.DefaultClient, l.config.Address, l.config.FromAgency, toAgency, FromAgencyAuthentication, disableNamespace) return l, nil } @@ -321,6 +331,12 @@ func (l *LmsAdapterNcip) RequestItem( if l.config.RequestItemBibIdCode != nil { code = *l.config.RequestItemBibIdCode } + if l.config.BibIdNormalization != nil && *l.config.BibIdNormalization == "sierra" { + itemId = strings.TrimPrefix(itemId, ".b") + if len(itemId) > 1 && itemId[len(itemId)-1] >= '0' && itemId[len(itemId)-1] <= '9' { + itemId = itemId[:len(itemId)-1] + } + } bibIdField := ncip.BibliographicId{ BibliographicRecordId: &ncip.BibliographicRecordId{ BibliographicRecordIdentifier: itemId, diff --git a/broker/lms/lms_adapter_test.go b/broker/lms/lms_adapter_test.go index 39f42ffe5..2a8be8ae6 100644 --- a/broker/lms/lms_adapter_test.go +++ b/broker/lms/lms_adapter_test.go @@ -26,14 +26,14 @@ func TestCreateLmsAdapterNcip(t *testing.T) { } _, err = CreateLmsAdapterNcip(config) assert.Error(t, err) - assert.Equal(t, "missing NCIP address in LMS configuration", err.Error()) + assert.Contains(t, err.Error(), "lmsConfig.address and fromAgency") config = dirapi.LmsConfig{ Address: "http://ncip.example.com", } _, err = CreateLmsAdapterNcip(config) assert.Error(t, err) - assert.Equal(t, "missing From Agency in LMS configuration", err.Error()) + assert.Contains(t, err.Error(), "lmsConfig.address and fromAgency") } func TestLookupUser(t *testing.T) { diff --git a/broker/lms/lms_creator_impl.go b/broker/lms/lms_creator_impl.go index 078e5297c..844f701a4 100644 --- a/broker/lms/lms_creator_impl.go +++ b/broker/lms/lms_creator_impl.go @@ -4,6 +4,7 @@ import ( "github.com/indexdata/crosslink/broker/adapter" "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/ill_db" + "github.com/indexdata/crosslink/broker/profiles" ) type lmsCreatorImpl struct { @@ -25,8 +26,14 @@ func (l *lmsCreatorImpl) GetAdapter(ctx common.ExtendedContext, symbol string) ( } for _, peer := range peers { entry := peer.CustomData - if entry.LmsConfig != nil { - return CreateLmsAdapterNcip(*entry.LmsConfig) + effective, err := profiles.Resolve(entry) + if err != nil { + return nil, err + } + // Diagnostics excludes endpoints, credentials, and other sensitive settings. + ctx.Logger().Debug("resolved host profiles", "configuration", effective.Diagnostics()) + if effective.LMS != nil && effective.LMS.Address != "" { + return createResolvedLmsAdapterNcip(*effective.LMS) } } return CreateLmsAdapterMockOK(), nil diff --git a/broker/lms/lms_creator_test.go b/broker/lms/lms_creator_test.go index 026560201..093ad417b 100644 --- a/broker/lms/lms_creator_test.go +++ b/broker/lms/lms_creator_test.go @@ -59,7 +59,7 @@ func TestGetAdapterNcipOK(t *testing.T) { assert.IsType(t, &LmsAdapterNcip{}, LmsAdapter) } -func TestGetAdapterNcipFail(t *testing.T) { +func TestGetAdapterEmptyConfigDoesNotEnableNcip(t *testing.T) { illRepo := &MockIllRepo{} peer := ill_db.Peer{ CustomData: dirapi.Entry{ @@ -71,8 +71,7 @@ func TestGetAdapterNcipFail(t *testing.T) { ctx := common.CreateExtCtxWithArgs(context.Background(), nil) symbol := "TEST" _, err := creator.GetAdapter(ctx, symbol) - assert.Error(t, err) - assert.Equal(t, "missing NCIP address in LMS configuration", err.Error()) + assert.NoError(t, err) } type MockIllRepo struct { diff --git a/broker/lms/profiles_test.go b/broker/lms/profiles_test.go new file mode 100644 index 000000000..34197379b --- /dev/null +++ b/broker/lms/profiles_test.go @@ -0,0 +1,46 @@ +package lms + +import ( + "encoding/json" + "testing" + + dirapi "github.com/indexdata/crosslink/directory/api" + "github.com/indexdata/crosslink/ncip" + "github.com/stretchr/testify/require" +) + +func TestLmsProfileDefaults(t *testing.T) { + for _, vendor := range []string{"Alma", "Sierra", "Koha", "FOLIO", "Generic"} { + var cfg dirapi.LmsConfig + require.NoError(t, json.Unmarshal([]byte(`{"vendor":"`+vendor+`","address":"https://ncip","fromAgency":"agency"}`), &cfg)) + adapter, err := CreateLmsAdapterNcip(cfg) + require.NoError(t, err) + a := adapter.(*LmsAdapterNcip) + require.Equal(t, vendor != "Koha", *a.config.NcipNamespaceEnabled) + if vendor == "Sierra" { + require.Equal(t, "Hold", a.requestItemRequestType()) + require.Equal(t, "Title", a.requestItemRequestScopeType()) + } else { + require.Equal(t, "Page", a.requestItemRequestType()) + } + } +} +func TestSierraNormalizationAndExplicitOverrides(t *testing.T) { + for _, tc := range []struct{ id, normalization, want string }{{".b1234567", "sierra", "123456"}, {"1234567", "sierra", "123456"}, {".b123456x", "sierra", "123456x"}, {".b1234567", "none", ".b1234567"}} { + var cfg dirapi.LmsConfig + require.NoError(t, json.Unmarshal([]byte(`{"vendor":"Sierra","address":"https://ncip","fromAgency":"agency","bibIdNormalization":"`+tc.normalization+`","requestItemPickupLocationEnabled":false}`), &cfg)) + adapter, err := CreateLmsAdapterNcip(cfg) + require.NoError(t, err) + a := adapter.(*LmsAdapterNcip) + client := new(ncipClientMock) + a.ncipClient = client + + _, err = a.RequestItem("request", tc.id, "user", "pickup", "") + require.NoError(t, err) + arg := client.lastRequest.(ncip.RequestItem) + require.Equal(t, tc.want, arg.BibliographicId[0].BibliographicRecordId.BibliographicRecordIdentifier) + require.Nil(t, arg.PickupLocation) + require.Equal(t, "Hold", arg.RequestType.Text) + require.Equal(t, "Title", arg.RequestScopeType.Text) + } +} diff --git a/broker/ncipclient/namespace_test.go b/broker/ncipclient/namespace_test.go new file mode 100644 index 000000000..175e13519 --- /dev/null +++ b/broker/ncipclient/namespace_test.go @@ -0,0 +1,41 @@ +package ncipclient + +import ( + "encoding/xml" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/indexdata/crosslink/ncip" + "github.com/stretchr/testify/require" +) + +func TestNamespaceFreeExchange(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + data, err := io.ReadAll(r.Body) + require.NoError(t, err) + require.NotContains(t, string(data), "xmlns") + require.Contains(t, string(data), "patron&1") + decoder := xml.NewDecoder(strings.NewReader(string(data))) + for { + token, err := decoder.Token() + if err == io.EOF { + break + } + require.NoError(t, err) + if el, ok := token.(xml.StartElement); ok { + require.Empty(t, el.Name.Space) + } + } + _, err = w.Write([]byte(`patron&1`)) + require.NoError(t, err) + })) + defer server.Close() + client := NewNcipClient(server.Client(), server.URL, "agency", "", "", true) + resp, err := client.LookupUser(ncip.LookupUser{UserId: &ncip.UserId{UserIdentifierValue: "patron&1"}}) + require.NoError(t, err) + require.Equal(t, "patron&1", resp.UserId.UserIdentifierValue) +} diff --git a/broker/ncipclient/ncipclient_impl.go b/broker/ncipclient/ncipclient_impl.go index 70049bc5d..26449e8bb 100644 --- a/broker/ncipclient/ncipclient_impl.go +++ b/broker/ncipclient/ncipclient_impl.go @@ -1,8 +1,10 @@ package ncipclient import ( + "bytes" "encoding/xml" "fmt" + "io" "net/http" "reflect" @@ -12,6 +14,7 @@ import ( ) type NcipClientImpl struct { + disableNamespace bool client *http.Client address string fromAgency string @@ -20,8 +23,9 @@ type NcipClientImpl struct { logFunc NcipLogFunc } -func NewNcipClient(client *http.Client, address string, fromAgency string, toAgency string, fromAgencyAuthentication string) NcipClient { +func NewNcipClient(client *http.Client, address string, fromAgency string, toAgency string, fromAgencyAuthentication string, disableNamespace bool) NcipClient { return &NcipClientImpl{ + disableNamespace: disableNamespace, client: client, address: address, fromAgency: fromAgency, @@ -221,7 +225,7 @@ func (n *NcipClientImpl) sendReceiveMessage(message *ncip.NCIPMessage) (*ncip.NC var respMessage ncip.NCIPMessage err := httpclient.NewClient().RequestResponse(n.client, http.MethodPost, []string{httpclient.ContentTypeApplicationXml}, - n.address, message, &respMessage, xml.Marshal, xml.Unmarshal) + n.address, message, &respMessage, n.marshal, n.unmarshal) if err != nil { return &respMessage, fmt.Errorf("NCIP message exchange failed: %s", err.Error()) } @@ -312,3 +316,74 @@ func traverse(v reflect.Value, level int) { } } } + +// transformNamespace handles namespace-free NCIP integrations while keeping +// the strongly typed NCIP model and all text/attribute values intact. +func transformNamespace(data []byte, remove bool) ([]byte, error) { + const ns = "http://www.niso.org/2008/ncip" + decoder := xml.NewDecoder(bytes.NewReader(data)) + var out bytes.Buffer + encoder := xml.NewEncoder(&out) + for { + token, err := decoder.Token() + if err == io.EOF { + break + } + if err != nil { + return nil, err + } + switch t := token.(type) { + case xml.StartElement: + if remove && t.Name.Space == ns { + t.Name.Space = "" + } else if !remove && t.Name.Space == "" { + t.Name.Space = ns + } + attrs := t.Attr[:0] + for _, a := range t.Attr { + if a.Name.Local == "xmlns" || a.Name.Space == "xmlns" { + continue + } + if remove && a.Name.Space == ns { + a.Name.Space = "" + } + attrs = append(attrs, a) + } + t.Attr = attrs + token = t + case xml.EndElement: + if remove && t.Name.Space == ns { + t.Name.Space = "" + } else if !remove && t.Name.Space == "" { + t.Name.Space = ns + } + token = t + } + if err := encoder.EncodeToken(token); err != nil { + return nil, err + } + } + if err := encoder.Flush(); err != nil { + return nil, err + } + return out.Bytes(), nil +} + +func (n *NcipClientImpl) marshal(v any) ([]byte, error) { + b, err := xml.Marshal(v) + if err != nil || !n.disableNamespace { + return b, err + } + return transformNamespace(b, true) +} + +func (n *NcipClientImpl) unmarshal(b []byte, v any) error { + if n.disableNamespace { + var err error + b, err = transformNamespace(b, false) + if err != nil { + return err + } + } + return xml.Unmarshal(b, v) +} diff --git a/broker/ncipclient/ncipclient_test.go b/broker/ncipclient/ncipclient_test.go index 55fabb583..aace398ce 100644 --- a/broker/ncipclient/ncipclient_test.go +++ b/broker/ncipclient/ncipclient_test.go @@ -40,7 +40,7 @@ func createTestClient() NcipClient { "http://localhost:"+os.Getenv("HTTP_PORT")+"/ncip", "ILL-MOCK", "ILL-MOCK", - "pass").(*NcipClientImpl) + "pass", false) } func TestPrepareHeaderValues(t *testing.T) { diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index a5271d996..5919bd88d 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -5688,7 +5688,7 @@ type mockLookupCreator struct { err error } -func (m *mockLookupCreator) GetAdapter(peer ill_db.Peer) (catalog.LookupAdapter, error) { +func (m *mockLookupCreator) GetAdapter(ctx common.ExtendedContext, peer ill_db.Peer) (catalog.LookupAdapter, error) { return m.adapter, m.err } diff --git a/broker/profiles/builtin.go b/broker/profiles/builtin.go new file mode 100644 index 000000000..5aa75063e --- /dev/null +++ b/broker/profiles/builtin.go @@ -0,0 +1,68 @@ +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 new file mode 100644 index 000000000..1b6062716 --- /dev/null +++ b/broker/profiles/builtin/Alma.yaml @@ -0,0 +1,12 @@ +# 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 new file mode 100644 index 000000000..0835fcd4f --- /dev/null +++ b/broker/profiles/builtin/FOLIO.yaml @@ -0,0 +1,13 @@ +# 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 new file mode 100644 index 000000000..4cea81b36 --- /dev/null +++ b/broker/profiles/builtin/Generic.yaml @@ -0,0 +1,16 @@ +# 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 new file mode 100644 index 000000000..323ac943e --- /dev/null +++ b/broker/profiles/builtin/Koha.yaml @@ -0,0 +1,18 @@ +# 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 new file mode 100644 index 000000000..62817d96e --- /dev/null +++ b/broker/profiles/builtin/Sierra.yaml @@ -0,0 +1,18 @@ +# 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 new file mode 100644 index 000000000..d2663de8b --- /dev/null +++ b/broker/profiles/builtin/profile.schema.json @@ -0,0 +1,17 @@ +{ + "$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 new file mode 100644 index 000000000..247dce2ce --- /dev/null +++ b/broker/profiles/builtin_test.go @@ -0,0 +1,31 @@ +package profiles + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBuiltinDefinitions(t *testing.T) { + 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":[]}}}}`)) + require.NoError(t, err) + require.Equal(t, before, asObject(builtins), "directory overrides must not change embedded 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) + } +} diff --git a/broker/profiles/defaults.go b/broker/profiles/defaults.go new file mode 100644 index 000000000..4d411c395 --- /dev/null +++ b/broker/profiles/defaults.go @@ -0,0 +1,81 @@ +package profiles + +// fillMissing records constructor defaults without overwriting explicit values. +func fillMissing(dst, defaults object, path string, origins map[string]string) { + for key, value := range defaults { + fullPath := path + "." + key + if nested, ok := value.(object); ok { + current, exists := dst[key].(object) + if !exists { + if _, present := dst[key]; present { + continue + } + current = object{} + dst[key] = current + } + fillMissing(current, nested, fullPath, origins) + } else if _, exists := dst[key]; !exists { + dst[key] = value + origins[fullPath] = "Generic" + } + } +} + +// materializeCatalogDefaults mirrors the established catalog constructors, +// including the legacy rule that a partial MARC mapping supplies only its +// explicitly configured fields. Vendor MARC mappings have already been merged. +func materializeCatalogDefaults(c object, origins map[string]string) { + fillMissing(c, object{"queryConfig": object{"type": "pqf"}}, "catalogConfig", origins) + q := c["queryConfig"].(object) + query := object{"identifier": "@attr 1=12 {term}", "isbn": "@attr 1=7 {term}", "issn": "@attr 1=8 {term}", "title": "@attr 1=4 {term}"} + if q["type"] == "cql" { + query = object{"identifier": "rec.id = {term}", "isbn": "isbn = {term}", "issn": "issn = {term}", "title": "title = {term}"} + } + fillMissing(q, query, "catalogConfig.queryConfig", origins) + if _, ok := c["metadataFormat"]; !ok { + c["metadataFormat"] = object{"marc21": object{}} + } + if format, ok := c["metadataFormat"].(object); ok { + if m, ok := format["marc21"].(object); ok { + fillMissing(m, object{"identifier": "001", "title": "245$a$n$p", "subtitle": "245$b", "isbn": "020$a", "issn": "022$a", "author": "100$a/100$?/110$a/110$?/111$a/111$?/245$c", "edition": "250$a"}, "catalogConfig.metadataFormat.marc21", origins) + } + } + if _, ok := c["holdingsFormat"]; !ok { + c["holdingsFormat"] = object{"marc": object{}} + } + if format, ok := c["holdingsFormat"].(object); ok { + if m, ok := format["marc"].(object); ok && len(m) == 0 { + fillMissing(m, object{"mainField": "852", "locationSubField": "b", "shelvingLocationSubField": "c", "callNumberSubField": "h", "itemIdSubField": "p", "restrictedSubField": "r"}, "catalogConfig.holdingsFormat.marc", origins) + } + if o, ok := format["opac"].(object); ok { + fillMissing(o, object{"availabilityRule": "availableNow", "requireLocalLocation": false, "shelvingLocationSource": "shelvingLocation", "includeItemId": true, "includeItemLoanPolicy": true, "includeTemporaryLocation": false, "allCirculations": false}, "catalogConfig.holdingsFormat.opac", origins) + } + } +} + +// Diagnostics returns behavior settings only. Connection options, addresses, +// authentication, agencies, patrons and local supply policies are excluded. +func (e *Effective) Diagnostics() object { + result := object{"lmsVendor": e.LMSVendor, "catalogProfile": e.CatalogProfile} + if e.LMS != nil { + raw := asObject(e.LMS) + safe := object{} + for _, key := range []string{"ncipNamespaceEnabled", "bibIdNormalization", "requestItemRequestType", "requestItemRequestScopeType", "requestItemBibIdCode", "requestItemPickupLocationEnabled", "lookupUserEnabled", "acceptItemEnabled", "checkInItemEnabled", "checkOutItemEnabled", "requestItemEnabled"} { + safe[key] = raw[key] + } + result["lmsConfig"] = safe + } + if e.Catalog != nil { + raw := asObject(e.Catalog) + safe := object{"queryConfig": raw["queryConfig"], "metadataFormat": raw["metadataFormat"], "holdingsFormat": raw["holdingsFormat"]} + if e.Catalog.Sru != nil { + safe["recordSchema"] = e.Catalog.Sru.RecordSchema + } + if e.Catalog.Zoom != nil && e.Catalog.Zoom.Options != nil { + safe["preferredRecordSyntax"] = (*e.Catalog.Zoom.Options)["preferredRecordSyntax"] + } + result["catalogConfig"] = safe + } + result["origins"] = e.Origins + return result +} diff --git a/broker/profiles/resolve.go b/broker/profiles/resolve.go new file mode 100644 index 000000000..44c2387ff --- /dev/null +++ b/broker/profiles/resolve.go @@ -0,0 +1,235 @@ +// Package profiles resolves host integrations without modifying directory records. +package profiles + +import ( + "encoding/json" + "fmt" + "strings" + + dirapi "github.com/indexdata/crosslink/directory/api" +) + +// Effective holds resolved LMS and catalog configurations and their setting origins. +// Values returned by Resolve are validated and independent of the directory entry. +// LMS and Catalog remain nil when the corresponding entry configuration is absent; +// selecting a profile alone does not enable a catalog lookup endpoint. +type Effective struct { + LMS *dirapi.LmsConfig + Catalog *dirapi.CatalogConfig + LMSVendor string + CatalogProfile string + // Origins maps configuration paths to Generic, a profile name, or directory. + Origins map[string]string +} + +type object = map[string]any + +func asObject(v any) object { + b, _ := json.Marshal(v) + var m object + _ = json.Unmarshal(b, &m) + if m == nil { + m = object{} + } + return m +} + +func merge(dst, src object, path, origin string, origins map[string]string) { + for k, v := range src { + if v == nil { + continue + } + key := path + "." + k + if sub, ok := v.(map[string]any); ok { + old, ok := dst[k].(map[string]any) + if !ok { + old = object{} + dst[k] = old + } + merge(old, sub, key, origin, origins) + } else { + dst[k] = v + origins[key] = origin + } + } +} + +func selected(m object, key, fallback string) string { + if v, ok := m[key].(string); ok { + return v + } + return fallback +} +func supported(name, setting string) error { + switch name { + case "Generic", "Alma", "Sierra", "Koha", "FOLIO": + return nil + case "WMS", "Aleph": + return fmt.Errorf("%s profile %s is not yet supported", setting, name) + default: + return fmt.Errorf("%s: unknown profile %q", setting, name) + } +} + +// Resolve applies profile defaults and explicit directory overrides to produce +// validated effective LMS and catalog configurations without mutating entry. +func Resolve(entry dirapi.Entry) (*Effective, error) { + rawL, rawC := asObject(entry.LmsConfig), asObject(entry.CatalogConfig) + vendor := selected(rawL, "vendor", "Generic") + profile := selected(rawC, "profile", vendor) + if err := supported(vendor, "lmsConfig.vendor"); err != nil { + return nil, err + } + if err := supported(profile, "catalogConfig.profile"); err != nil { + return nil, err + } + e := &Effective{LMSVendor: vendor, CatalogProfile: profile, Origins: map[string]string{}} + l, c := object{}, object{} + merge(l, builtins["Generic"].LMS, "lmsConfig", "Generic", e.Origins) + if vendor != "Generic" { + merge(l, builtins[vendor].LMS, "lmsConfig", vendor, e.Origins) + } + merge(l, rawL, "lmsConfig", "directory", e.Origins) + merge(c, builtins[profile].Catalog, "catalogConfig", profile, e.Origins) + // Switching parser replaces the entire profile parser, including its rules. + if h, ok := rawC["holdingsFormat"].(object); ok && len(h) > 0 { + for parser := range h { + if defaults, ok := c["holdingsFormat"].(object); ok { + if _, same := defaults[parser]; !same { + delete(c, "holdingsFormat") + for key := range e.Origins { + if strings.HasPrefix(key, "catalogConfig.holdingsFormat.") { + delete(e.Origins, key) + } + } + } + } + } + } + // An empty holdings object inherits the selected profile's defaults, + // including the legacy MARC defaults for Generic. + if h, ok := rawC["holdingsFormat"].(object); ok && len(h) == 0 { + delete(rawC, "holdingsFormat") + } + merge(c, rawC, "catalogConfig", "directory", e.Origins) + if z, ok := c["zoom"].(object); ok { + // MARC and reservoir use the adapter's legacy MARC wire syntax; + // ZOOM converts received records to XML before passing them to parsers. + syntax := "usmarc" + if h, ok := c["holdingsFormat"].(object); ok { + if _, ok := h["opac"]; ok { + syntax = "opac" + } else if _, ok := h["marc21plus1"]; ok { + syntax = "xml" + } + } + options, ok := z["options"].(object) + if !ok { + options = object{} + z["options"] = options + } + if _, ok := options["preferredRecordSyntax"]; !ok { + options["preferredRecordSyntax"] = syntax + e.Origins["catalogConfig.zoom.options.preferredRecordSyntax"] = profile + } + } + if s, ok := c["sru"].(object); ok { + // SRU delivers MARC-based formats as marcxml and OPAC as opac. + schema := "marcxml" + if h, ok := c["holdingsFormat"].(object); ok { + if _, ok := h["opac"]; ok { + schema = "opac" + } + } + if _, ok := s["recordSchema"]; !ok { + s["recordSchema"] = schema + e.Origins["catalogConfig.sru.recordSchema"] = profile + } + } + materializeCatalogDefaults(c, e.Origins) + + if entry.LmsConfig != nil { + b, _ := json.Marshal(l) + if err := json.Unmarshal(b, &e.LMS); err != nil { + return nil, err + } + } + if entry.CatalogConfig != nil { + b, _ := json.Marshal(c) + if err := json.Unmarshal(b, &e.Catalog); err != nil { + return nil, err + } + } + if err := e.validate(); err != nil { + return nil, err + } + return e, nil +} + +func (e *Effective) validate() error { + if l := e.LMS; l != nil { + if (l.Address == "") != (l.FromAgency == "") { + return fmt.Errorf("LMS profile %s: lmsConfig.address and fromAgency must both be configured", e.LMSVendor) + } + if l.BibIdNormalization != nil && *l.BibIdNormalization != "none" && *l.BibIdNormalization != "sierra" { + return fmt.Errorf("LMS profile %s: unsupported bibIdNormalization", e.LMSVendor) + } + } + if c := e.Catalog; c != nil { + bad := func(setting string) error { + return fmt.Errorf("catalog profile %s: invalid %s", e.CatalogProfile, setting) + } + if c.Sru != nil && c.Sru.Address == "" { + return bad("sru.address") + } + if c.Zoom != nil && c.Zoom.Address == "" { + return bad("zoom.address") + } + if c.Sru != nil && c.Zoom != nil { + return bad("simultaneous sru and zoom endpoints") + } + if c.MetadataFormat != nil && c.MetadataFormat.Marc21 == nil { + return bad("metadataFormat.marc21") + } + if c.QueryConfig != nil && c.QueryConfig.Type != nil && *c.QueryConfig.Type != dirapi.Pqf && *c.QueryConfig.Type != dirapi.Cql { + return bad("queryConfig.type") + } + if h := c.HoldingsFormat; h != nil { + n := 0 + if h.Marc != nil { + n++ + } + if h.Opac != nil { + n++ + } + if h.Reservoir != nil { + n++ + } + if h.Marc21plus1 != nil { + n++ + } + if n != 1 { + return bad("holdingsFormat must set marc, opac, reservoir, or marc21plus1 (exactly one parser)") + } + if m := h.Marc; m != nil && m.Availability != nil { + for _, r := range *m.Availability { + if r.SubField == "" || (r.Operator != "equals" && r.Operator != "absent") || (r.Operator == "equals" && r.Value == nil) { + return bad("holdingsFormat.marc.availability") + } + } + } + if o := h.Opac; o != nil { + if o.AvailabilityRule != nil && *o.AvailabilityRule != "availableNow" && *o.AvailabilityRule != "publicNote" { + return bad("holdingsFormat.opac.availabilityRule") + } + if o.ShelvingLocationSource != nil && *o.ShelvingLocationSource != "localLocation" && *o.ShelvingLocationSource != "shelvingLocation" { + return bad("holdingsFormat.opac.shelvingLocationSource") + } + if o.AvailabilityRule != nil && *o.AvailabilityRule == "publicNote" && o.AvailablePublicNotes == nil { + return bad("holdingsFormat.opac.availablePublicNotes is required") + } + } + } + } + return nil +} diff --git a/broker/profiles/resolve_test.go b/broker/profiles/resolve_test.go new file mode 100644 index 000000000..2533760a3 --- /dev/null +++ b/broker/profiles/resolve_test.go @@ -0,0 +1,223 @@ +package profiles + +import ( + "encoding/json" + "testing" + + dirapi "github.com/indexdata/crosslink/directory/api" + "github.com/stretchr/testify/require" +) + +func entry(t *testing.T, data string) dirapi.Entry { + t.Helper() + var e dirapi.Entry + require.NoError(t, json.Unmarshal([]byte(data), &e)) + return e +} +func TestProfiles(t *testing.T) { + for _, name := range []string{"Generic", "Alma", "Sierra", "Koha", "FOLIO"} { + t.Run(name, func(t *testing.T) { + raw := entry(t, `{"vendor":"Alma","illConfig":{"iso18626Vendor":"ReShare"},"lmsConfig":{"vendor":"`+name+`"},"catalogConfig":{"zoom":{"address":"catalog:210"}}}`) + before, _ := json.Marshal(raw) + e, err := Resolve(raw) + require.NoError(t, err) + after, _ := json.Marshal(raw) + require.Equal(t, string(before), string(after)) + require.Equal(t, name, e.CatalogProfile) + require.Empty(t, e.LMS.Address) + require.Empty(t, e.LMS.FromAgency) + switch name { + case "Generic": + require.Equal(t, "852", *e.Catalog.HoldingsFormat.Marc.MainField) + require.Equal(t, "usmarc", (*e.Catalog.Zoom.Options)["preferredRecordSyntax"]) + case "Koha": + require.False(t, *e.LMS.NcipNamespaceEnabled) + require.Equal(t, "952", *e.Catalog.HoldingsFormat.Marc.MainField) + require.Equal(t, "usmarc", (*e.Catalog.Zoom.Options)["preferredRecordSyntax"]) + case "Sierra": + require.Equal(t, "Hold", *e.LMS.RequestItemRequestType) + require.Equal(t, "Title", *e.LMS.RequestItemRequestScopeType) + require.Equal(t, "sierra", *e.LMS.BibIdNormalization) + require.Equal(t, "publicNote", *e.Catalog.HoldingsFormat.Opac.AvailabilityRule) + default: + require.True(t, *e.Catalog.HoldingsFormat.Opac.RequireLocalLocation) + require.Equal(t, "opac", (*e.Catalog.Zoom.Options)["preferredRecordSyntax"]) + } + }) + } +} +func TestEmptyHoldingsFormatUsesDefaults(t *testing.T) { + for _, profile := range []string{"", "Generic", "Alma", "Sierra", "Koha", "FOLIO"} { + t.Run("profile="+profile, func(t *testing.T) { + raw := entry(t, `{"catalogConfig":{"zoom":{"address":"catalog:210"},"holdingsFormat":{}}}`) + if profile != "" { + raw.CatalogConfig.Profile.Set(profile) + } + before, err := json.Marshal(raw) + require.NoError(t, err) + effective, err := Resolve(raw) + require.NoError(t, err) + after, err := json.Marshal(raw) + require.NoError(t, err) + require.JSONEq(t, string(before), string(after)) + raw.CatalogConfig.HoldingsFormat = nil + defaults, err := Resolve(raw) + require.NoError(t, err) + require.Equal(t, defaults, effective) + if profile == "" || profile == "Generic" { + require.Equal(t, "852", *effective.Catalog.HoldingsFormat.Marc.MainField) + require.Equal(t, "usmarc", (*effective.Catalog.Zoom.Options)["preferredRecordSyntax"]) + } + }) + } +} + +func TestIndependentOverrides(t *testing.T) { + raw := entry(t, `{"lmsConfig":{"vendor":"Sierra","requestItemPickupLocationEnabled":false,"requestItemRequestType":"","bibIdNormalization":"none"},"catalogConfig":{"profile":"Koha","zoom":{"address":"site","options":{"preferredRecordSyntax":"custom"}},"holdingsFormat":{"marc":{"callNumberSubField":"x","availability":[]}}}}`) + e, err := Resolve(raw) + require.NoError(t, err) + require.Equal(t, "Koha", e.CatalogProfile) + require.False(t, *e.LMS.RequestItemPickupLocationEnabled) + require.Empty(t, *e.LMS.RequestItemRequestType) + require.Equal(t, "none", *e.LMS.BibIdNormalization) + require.Equal(t, "952", *e.Catalog.HoldingsFormat.Marc.MainField) + require.Equal(t, "x", *e.Catalog.HoldingsFormat.Marc.CallNumberSubField) + require.Empty(t, *e.Catalog.HoldingsFormat.Marc.Availability) + require.Equal(t, "custom", (*e.Catalog.Zoom.Options)["preferredRecordSyntax"]) + require.Equal(t, "Koha", e.Origins["catalogConfig.holdingsFormat.marc.mainField"]) + require.Equal(t, "directory", e.Origins["lmsConfig.requestItemPickupLocationEnabled"]) +} +func TestParserReplacement(t *testing.T) { + for _, parser := range []string{"marc", "reservoir", "marc21plus1"} { + e, err := Resolve(entry(t, `{"lmsConfig":{"vendor":"Sierra"},"catalogConfig":{"holdingsFormat":{"`+parser+`":{}}}}`)) + require.NoError(t, err) + require.Nil(t, e.Catalog.HoldingsFormat.Opac) + for key := range e.Origins { + require.NotContains(t, key, ".opac.") + } + } + e, err := Resolve(entry(t, `{"lmsConfig":{"vendor":"Koha"},"catalogConfig":{"holdingsFormat":{"opac":{}}}}`)) + require.NoError(t, err) + require.Nil(t, e.Catalog.HoldingsFormat.Marc) + require.Equal(t, "availableNow", *e.Catalog.HoldingsFormat.Opac.AvailabilityRule) +} +func TestFallbackAndProfileOnly(t *testing.T) { + for _, data := range []string{`{}`, `{"vendor":"Alma","illConfig":{"iso18626Vendor":"Alma"}}`} { + e, err := Resolve(entry(t, data)) + require.NoError(t, err) + require.Equal(t, "Generic", e.CatalogProfile) + require.Equal(t, "Generic", e.LMSVendor) + } + e, err := Resolve(entry(t, `{"lmsConfig":{"vendor":"Sierra"},"catalogConfig":{"profile":"Generic"}}`)) + require.NoError(t, err) + require.Equal(t, "Generic", e.CatalogProfile) + require.Nil(t, e.Catalog.Sru) + require.Nil(t, e.Catalog.Zoom) + require.Equal(t, "852", *e.Catalog.HoldingsFormat.Marc.MainField) + e, err = Resolve(entry(t, `{"catalogConfig":{"profile":"Alma"}}`)) + require.NoError(t, err) + require.Nil(t, e.LMS) + require.Nil(t, e.Catalog.Zoom) +} +func TestZoomParserSyntax(t *testing.T) { + for _, profile := range []string{"Generic", "Koha", "Alma"} { + for _, tc := range []struct { + name, holdings, parser, syntax string + }{ + {"marc", `"marc":{}`, "marc", "usmarc"}, + {"opac", `"opac":{}`, "opac", "opac"}, + {"reservoir", `"reservoir":{}`, "reservoir", "usmarc"}, + {"marc21plus1", `"marc21plus1":{}`, "marc21plus1", "xml"}, + } { + t.Run(profile+"/"+tc.name, func(t *testing.T) { + raw := entry(t, `{"catalogConfig":{"profile":"`+profile+`","zoom":{"address":"catalog:210"},"holdingsFormat":{`+tc.holdings+`}}}`) + before, err := json.Marshal(raw) + require.NoError(t, err) + e, err := Resolve(raw) + require.NoError(t, err) + h := asObject(e.Catalog.HoldingsFormat) + require.Len(t, h, 1) + require.Contains(t, h, tc.parser) + require.Equal(t, tc.syntax, (*e.Catalog.Zoom.Options)["preferredRecordSyntax"]) + if profile == "Koha" && tc.parser == "marc" { + require.Equal(t, "952", *e.Catalog.HoldingsFormat.Marc.MainField) + } + after, err := json.Marshal(raw) + require.NoError(t, err) + require.JSONEq(t, string(before), string(after)) + }) + } + } +} +func TestRejectConflictingHoldingsParsers(t *testing.T) { + parsers := []string{"marc", "opac", "reservoir", "marc21plus1"} + for _, profile := range []string{"Generic", "Alma", "Sierra", "Koha", "FOLIO"} { + for i, first := range parsers { + for _, second := range parsers[i+1:] { + t.Run(profile+"/"+first+"/"+second, func(t *testing.T) { + raw := entry(t, `{"catalogConfig":{"profile":"`+profile+`","holdingsFormat":{"`+first+`":{},"`+second+`":{}}}}`) + before, err := json.Marshal(raw) + require.NoError(t, err) + _, err = Resolve(raw) + require.ErrorContains(t, err, "exactly one parser") + after, err := json.Marshal(raw) + require.NoError(t, err) + require.JSONEq(t, string(before), string(after)) + }) + } + } + } +} + +func TestSruSchemaDefaults(t *testing.T) { + for _, profile := range []string{"Generic", "Koha", "Alma"} { + for _, tc := range []struct{ parser, schema string }{ + {"marc", "marcxml"}, + {"opac", "opac"}, + {"reservoir", "marcxml"}, + {"marc21plus1", "marcxml"}, + } { + t.Run(profile+"/"+tc.parser, func(t *testing.T) { + for _, explicit := range []bool{false, true} { + raw := entry(t, `{"catalogConfig":{"profile":"`+profile+`","sru":{"address":"https://catalog/sru"},"holdingsFormat":{"`+tc.parser+`":{}}}}`) + want := tc.schema + if explicit { + want = "custom" + raw.CatalogConfig.Sru.RecordSchema = &want + } + e, err := Resolve(raw) + require.NoError(t, err) + require.Equal(t, want, *e.Catalog.Sru.RecordSchema) + require.Nil(t, e.Catalog.Zoom) + } + }) + } + } +} + +func TestValidation(t *testing.T) { + for _, data := range []string{ + `{"lmsConfig":{"vendor":"WMS"}}`, `{"catalogConfig":{"profile":"Aleph"}}`, `{"lmsConfig":{"vendor":"bad"}}`, + `{"lmsConfig":{"vendor":"Sierra","address":"x"}}`, + `{"catalogConfig":{"profile":"Alma","zoom":{"address":""}}}`, + `{"catalogConfig":{"profile":"Koha","holdingsFormat":{"marc":{"availability":[{"operator":"equals","subField":"7"}]}}}}`, + `{"catalogConfig":{"profile":"Alma","holdingsFormat":{"opac":{"availabilityRule":"bad"}}}}`, + } { + _, err := Resolve(entry(t, data)) + require.Error(t, err) + require.Contains(t, err.Error(), "profile") + } +} + +func TestRejectConflictingCatalogEndpoints(t *testing.T) { + _, err := Resolve(entry(t, `{"catalogConfig":{"sru":{"address":"https://catalog/sru"},"zoom":{"address":"catalog:210"}}}`)) + require.ErrorContains(t, err, "simultaneous sru and zoom endpoints") +} +func TestDiagnosticsProtectCredentials(t *testing.T) { + effective, err := Resolve(entry(t, `{"lmsConfig":{"vendor":"Sierra","address":"https://secret-address","fromAgency":"secret-agency","fromAgencyAuthentication":"secret-password"},"catalogConfig":{"zoom":{"address":"secret-catalog","options":{"password":"secret-password"}}}}`)) + require.NoError(t, err) + diagnostics, err := json.Marshal(effective.Diagnostics()) + require.NoError(t, err) + require.NotContains(t, string(diagnostics), "secret-") + require.Contains(t, string(diagnostics), "Sierra") +} diff --git a/broker/service/lookupadapterfactory.go b/broker/service/lookupadapterfactory.go index f23a5fe17..0f8983785 100644 --- a/broker/service/lookupadapterfactory.go +++ b/broker/service/lookupadapterfactory.go @@ -61,7 +61,7 @@ func (s *LookupAdapterFactory) GetAdapterRequester(ctx common.ExtendedContext, r if s.lookupAdapterCreator == nil { return nil, dirapi.Entry{}, fmt.Errorf("lookup adapter factory misconfigured: lookupAdapterCreator is nil") } - lookupAdapter, err := s.lookupAdapterCreator.GetAdapter(peer) + lookupAdapter, err := s.lookupAdapterCreator.GetAdapter(ctx, peer) if err != nil { return nil, dirapi.Entry{}, fmt.Errorf("failed to get adapter for peer: %w", err) } @@ -72,5 +72,5 @@ func (s *LookupAdapterFactory) GetAdapterSupplier(ctx common.ExtendedContext, su if s.lookupAdapterCreator == nil { return nil, fmt.Errorf("lookup adapter factory misconfigured: lookupAdapterCreator is nil") } - return s.lookupAdapterCreator.GetAdapter(supplier) + return s.lookupAdapterCreator.GetAdapter(ctx, supplier) } diff --git a/broker/test/catalog/holdings_test.go b/broker/test/catalog/holdings_test.go index 4e19f4d23..afb8ac5c0 100644 --- a/broker/test/catalog/holdings_test.go +++ b/broker/test/catalog/holdings_test.go @@ -4,10 +4,12 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "os" "strconv" + "strings" "sync/atomic" "testing" @@ -256,7 +258,7 @@ func TestRequestRequestSruServerLoaned(t *testing.T) { "NOTICE, supplier-msg-received = SUCCESS\n" + "TASK, message-requester = SUCCESS\n" + "TASK, confirm-supplier-msg = SUCCESS\n" - apptest.EventsCompareString(appCtx, eventRepo, t, illTrans.ID, exp) + compareLoanEvents(appCtx, t, illTrans.ID, exp) } // should locate three candidate suppliers via SRU; the second selected supplier fulfills the loan with scenario LOANED in note @@ -313,5 +315,19 @@ func TestRequestRequestSruServerLoanedMultiple(t *testing.T) { "NOTICE, supplier-msg-received = SUCCESS\n" + "TASK, message-requester = SUCCESS\n" + "TASK, confirm-supplier-msg = SUCCESS\n" - apptest.EventsCompareString(appCtx, eventRepo, t, illTrans.ID, exp) + compareLoanEvents(appCtx, t, illTrans.ID, exp) +} + +func compareLoanEvents(appCtx common.ExtendedContext, t *testing.T, illId, expected string) { + t.Helper() + actual := apptest.EventsToCompareStringFunc(appCtx, eventRepo, t, illId, strings.Count(expected, "\n"), false, func(e events.Event) string { + return fmt.Sprintf(apptest.EventRecordFormat, e.EventType, e.EventName, e.EventStatus) + }) + // The requester's reply can arrive while the broker is still confirming the + // supplier message. Accept either order for this adjacent pair only; all + // event counts, statuses, and the rest of the sequence must still match. + const reply = "NOTICE, requester-msg-received = SUCCESS\n" + const confirmation = "TASK, confirm-supplier-msg = SUCCESS\n" + actual = strings.ReplaceAll(actual, reply+confirmation, confirmation+reply) + assert.Equal(t, expected, actual) } diff --git a/directory/README.md b/directory/README.md index 9d262db1e..ecc990819 100644 --- a/directory/README.md +++ b/directory/README.md @@ -64,6 +64,14 @@ rejected; see the `ImportEntryRecord`, `ImportTierRecord`, and `ImportNetworkRecord` schemas in [api.yaml](api.yaml) for the complete contract. Database IDs are generated by the service. +Complete imports include raw host settings: `lmsConfig.vendor`, +`ncipNamespaceEnabled`, `bibIdNormalization`, `catalogConfig.profile`, MARC +availability predicates, and OPAC parser options. These fields are required in +their respective non-null configuration objects; use `null` to inherit defaults. +Explicit `false` and empty arrays are preserved. Imports replace stored +configuration, so include all overrides when restoring an entry. Configure at +most one SRU/ZOOM endpoint and at most one holdings parser. + ### Import with curl For the local service started above: @@ -170,3 +178,8 @@ Run `make generate` before invoking `go build` or `go test` directly. | `TENANT_SYMBOL_AUTHORITY` | Authority paired with an incoming institution/tenant to form a complete symbol | `TEST` | | `LOG_LEVEL` | Log level: `debug`, `info`, `warn`, or `error` | `info` | | `LOG_FORMAT` | Log output format; set to `json` for structured JSON logs | `text` | + +## Host integration profiles + +See [Host LMS and catalog profiles](host-profiles.md) for `lmsConfig.vendor`, +`catalogConfig.profile`, precedence, parser overrides, diagnostics and migration. diff --git a/directory/RELEASE-NOTES.md b/directory/RELEASE-NOTES.md new file mode 100644 index 000000000..3fb822f09 --- /dev/null +++ b/directory/RELEASE-NOTES.md @@ -0,0 +1,20 @@ +# Unreleased + +- Added independent host LMS and catalog profiles for Alma, Sierra, Koha, FOLIO + and Generic. WMS and Aleph are reserved and report unsupported-profile errors. +- Added Sierra OPAC availability, Koha MARC value/absence predicates, namespace-free + NCIP, and configurable Sierra bib-ID normalization. +- Migration 009 preserves existing holdings configuration and adds nullable profile + fields. Entries without profiles retain Generic behavior; no preset values are + written into directory records. +- LMS `address` and `fromAgency` are optional in directory records so a host vendor + can be selected without enabling circulation. Both are required to create an + NCIP adapter. Catalog profiles also require a connection to enable lookup. +- Directory responses now omit unspecified LMS defaults instead of materializing + them during POST. Omitted query types also remain omitted on POST/PATCH. Broker + runtime defaults remain unchanged. Existing stored defaults remain overrides; + clear optional LMS values with PATCH null to inherit a profile's defaults. +- Holdings overrides now merge for the same parser and replace on parser changes. + Empty absent holdings configurations are omitted from directory responses. + +See [host profile documentation](host-profiles.md) for configuration and upgrade details. diff --git a/directory/api.yaml b/directory/api.yaml index 362357a3a..daeab05a0 100644 --- a/directory/api.yaml +++ b/directory/api.yaml @@ -1506,8 +1506,11 @@ components: ImportLmsConfig: type: object additionalProperties: false - required: [address, fromAgency, fromAgencyAuthentication, toAgency, lookupUserEnabled, acceptItemEnabled, checkInItemEnabled, checkOutItemEnabled, itemLocation, requestItemRequestType, requestItemRequestScopeType, requestItemBibIdCode, requestItemEnabled, requestItemPickupLocationEnabled, requesterPickupLocation, supplierPickupLocation, requesterPatronPattern, patronProfiles] + required: [vendor, ncipNamespaceEnabled, bibIdNormalization, address, fromAgency, fromAgencyAuthentication, toAgency, lookupUserEnabled, acceptItemEnabled, checkInItemEnabled, checkOutItemEnabled, itemLocation, requestItemRequestType, requestItemRequestScopeType, requestItemBibIdCode, requestItemEnabled, requestItemPickupLocationEnabled, requesterPickupLocation, supplierPickupLocation, requesterPatronPattern, patronProfiles] properties: + vendor: { type: string, nullable: true, enum: [Alma, Sierra, Koha, WMS, Aleph, FOLIO, Generic], x-go-type: string } + ncipNamespaceEnabled: { type: boolean, nullable: true } + bibIdNormalization: { type: string, nullable: true, enum: [none, sierra], x-go-type: string } address: { type: string } fromAgency: { type: string } fromAgencyAuthentication: { type: string, nullable: true } @@ -1553,8 +1556,9 @@ components: ImportCatalogConfig: type: object additionalProperties: false - required: [metadataUpdateMode, sru, zoom, queryConfig, holdingsFormat, metadataFormat] + required: [profile, metadataUpdateMode, sru, zoom, queryConfig, holdingsFormat, metadataFormat] properties: + profile: { type: string, nullable: true, enum: [Alma, Sierra, Koha, WMS, Aleph, FOLIO, Generic], x-go-type: string } metadataUpdateMode: allOf: [{ $ref: '#/components/schemas/MetadataUpdateMode' }] nullable: true @@ -1612,15 +1616,21 @@ components: marc: allOf: [{ $ref: '#/components/schemas/ImportMarcHoldingsParserConfig' }] nullable: true - marc21plus1: { type: object, nullable: true } - opac: { type: object, nullable: true } - reservoir: { type: object, nullable: true } + marc21plus1: { type: object, nullable: true, additionalProperties: false } + opac: + allOf: [{ $ref: '#/components/schemas/ImportOpacHoldingsParserConfig' }] + nullable: true + reservoir: { type: object, nullable: true, additionalProperties: false } ImportMarcHoldingsParserConfig: type: object additionalProperties: false - required: [callNumberSubField, itemIdSubField, locationSubField, mainField, restrictedSubField, shelvingLocationSubField] + required: [availability, callNumberSubField, itemIdSubField, locationSubField, mainField, restrictedSubField, shelvingLocationSubField] properties: + availability: + type: array + nullable: true + items: { $ref: '#/components/schemas/ImportMarcAvailabilityPredicate' } callNumberSubField: { type: string, nullable: true } itemIdSubField: { type: string, nullable: true } locationSubField: { type: string, nullable: true } @@ -1628,6 +1638,32 @@ components: restrictedSubField: { type: string, nullable: true } shelvingLocationSubField: { type: string, nullable: true } + ImportMarcAvailabilityPredicate: + type: object + additionalProperties: false + required: [subField, operator, value] + properties: + subField: { type: string, minLength: 1 } + operator: { type: string, enum: [equals, absent], x-go-type: string } + value: { type: string, nullable: true } + + ImportOpacHoldingsParserConfig: + type: object + additionalProperties: false + required: [availabilityRule, availablePublicNotes, requireLocalLocation, shelvingLocationSource, includeItemId, includeItemLoanPolicy, includeTemporaryLocation, allCirculations] + properties: + availabilityRule: { type: string, nullable: true, enum: [availableNow, publicNote], x-go-type: string } + availablePublicNotes: + type: array + nullable: true + items: { type: string } + requireLocalLocation: { type: boolean, nullable: true } + shelvingLocationSource: { type: string, nullable: true, enum: [shelvingLocation, localLocation], x-go-type: string } + includeItemId: { type: boolean, nullable: true } + includeItemLoanPolicy: { type: boolean, nullable: true } + includeTemporaryLocation: { type: boolean, nullable: true } + allCirculations: { type: boolean, nullable: true } + ImportMetadataParserConfig: type: object additionalProperties: false @@ -2028,8 +2064,20 @@ components: - ILLiad - Unknown CatalogConfig: + description: >- + Raw catalog overrides. Defaults resolve as Generic, then profile (or + lmsConfig.vendor when profile is unset), then explicit values. False, + empty values where valid, and explicit parser choices take precedence. + Same-parser holdings fields merge; a different parser replaces its defaults. + Selecting a profile does not enable lookup without an SRU or ZOOM address. type: object properties: + profile: + type: string + nullable: true + enum: [Alma, Sierra, Koha, WMS, Aleph, FOLIO, Generic] + x-go-type: string + description: Host integration profile, independent of the ILL vendor. WMS and Aleph are reserved and currently unsupported. sru: $ref: '#/components/schemas/SruConfig' zoom: @@ -2046,6 +2094,12 @@ components: CatalogConfigPatch: type: object properties: + profile: + type: string + nullable: true + enum: [Alma, Sierra, Koha, WMS, Aleph, FOLIO, Generic] + x-go-type: string + description: Host integration profile, independent of the ILL vendor. WMS and Aleph are reserved and currently unsupported. sru: $ref: '#/components/schemas/SruConfigPatch' zoom: @@ -2108,7 +2162,6 @@ components: type: type: string enum: [cql, pqf] - default: pqf title: type: string isbn: @@ -2152,6 +2205,11 @@ components: MarcHoldingsParserConfig: type: object properties: + availability: + type: array + description: All predicates must match a candidate item. Equals requires a present matching subfield; absent rejects any occurrence. + items: + $ref: '#/components/schemas/MarcAvailabilityPredicate' mainField: { type: string } locationSubField: { type: string } shelvingLocationSubField: { type: string } @@ -2159,8 +2217,36 @@ components: restrictedSubField: { type: string } callNumberSubField: { type: string } additionalProperties: false + MarcAvailabilityPredicate: + type: object + required: [subField, operator] + properties: + subField: { type: string, minLength: 1 } + operator: + type: string + enum: [equals, absent] + x-go-type: string + value: { type: string } + additionalProperties: false OpacHoldingsParserConfig: type: object + properties: + availabilityRule: + type: string + enum: [availableNow, publicNote] + x-go-type: string + availablePublicNotes: + type: array + items: { type: string } + requireLocalLocation: { type: boolean } + shelvingLocationSource: + type: string + enum: [shelvingLocation, localLocation] + x-go-type: string + includeItemId: { type: boolean } + includeItemLoanPolicy: { type: boolean } + includeTemporaryLocation: { type: boolean } + allCirculations: { type: boolean } additionalProperties: false EntryPatch: type: object @@ -2346,16 +2432,33 @@ components: description: Supply preference where -1 disables supply, 0 means no preference, and 10000 is the highest preference. LmsConfig: + description: >- + Raw circulation overrides, independent of illConfig.iso18626Vendor and + the deprecated top-level vendor. Generic defaults are followed by the + selected host vendor and explicit values. Defaults are never persisted. + A vendor alone does not enable NCIP; address and fromAgency must both + be configured to create a circulation adapter. type: object - required: - - address - - fromAgency properties: + ncipNamespaceEnabled: + type: boolean + bibIdNormalization: + type: string + enum: [none, sierra] + x-go-type: string + vendor: + type: string + nullable: true + enum: [Alma, Sierra, Koha, WMS, Aleph, FOLIO, Generic] + x-go-type: string + description: Host integration profile, independent of the ILL vendor. WMS and Aleph are reserved and currently unsupported. address: type: string + x-go-type-skip-optional-pointer: true description: Base URL of the LMS API fromAgency: type: string + x-go-type-skip-optional-pointer: true description: Agency code used when the entry is acting as the supplying agency fromAgencyAuthentication: type: string @@ -2368,55 +2471,42 @@ components: lookupUserEnabled: type: boolean description: Whether user lookup is enabled - default: true acceptItemEnabled: type: boolean description: Whether item acceptance is enabled - default: true checkInItemEnabled: type: boolean description: Whether item check-in is enabled - default: true checkOutItemEnabled: type: boolean description: Whether item check-out is enabled - default: true requestItemEnabled: type: boolean description: Whether item requesting is enabled - default: true itemLocation: type: string description: Location code to include in NCIP RequestItem messages - default: "" requestItemRequestType: type: string description: For Request Item requests, the type of request to create - default: Page requestItemRequestScopeType: type: string description: For Request Item requests, the scope type of request to create - default: Item requestItemBibIdCode: type: string description: For Request Item requests, the code indicating how to interpret the bib id - default: SYSNUMBER requestItemPickupLocationEnabled: type: boolean description: Whether to include a pickup location when making Request Item requests - default: true requesterPickupLocation: type: string description: Pickup location code used when acting as requesting agency - default: Main Library supplierPickupLocation: type: string description: Pickup location code used when acting as supplying agency - default: ILL Office requesterPatronPattern: type: string description: "'{requesterSymbol}' occurrences are replaced with requesting agency ISIL" - default: "INST-{requesterSymbol}" patronProfiles: description: Patron profiles used to determine whether a user returned by NCIP LookupUser is eligible to create ILL requests. allOf: @@ -2449,6 +2539,20 @@ components: LmsConfigPatch: type: object properties: + ncipNamespaceEnabled: + type: boolean + nullable: true + bibIdNormalization: + type: string + nullable: true + enum: [none, sierra] + x-go-type: string + vendor: + type: string + nullable: true + enum: [Alma, Sierra, Koha, WMS, Aleph, FOLIO, Generic] + x-go-type: string + description: Host integration profile, independent of the ILL vendor. WMS and Aleph are reserved and currently unsupported. address: type: string fromAgency: diff --git a/directory/api/catalog_config.go b/directory/api/catalog_config.go index f442daf2a..fdbbecae8 100644 --- a/directory/api/catalog_config.go +++ b/directory/api/catalog_config.go @@ -12,7 +12,8 @@ import ( func catalogConfigToDBParams(entryID uuid.UUID, cfg CatalogConfig) db.UpsertCatalogConfigParams { params := db.UpsertCatalogConfigParams{ - Entry: &entryID, + Entry: &entryID, + Profile: maybeUpdateCol[string](nil, cfg.Profile), } if cfg.MetadataUpdateMode != nil { @@ -40,6 +41,7 @@ func catalogConfigToDBParams(entryID uuid.UUID, cfg CatalogConfig) db.UpsertCata params.QueryTitle = cfg.QueryConfig.Title } if cfg.HoldingsFormat != nil { + params.HoldingsConfig, _ = json.Marshal(cfg.HoldingsFormat) if cfg.HoldingsFormat.Marc != nil { marc := cfg.HoldingsFormat.Marc params.HoldingsMarcCallNumberSubfield = marc.CallNumberSubField @@ -67,6 +69,28 @@ func catalogConfigToDBParams(entryID uuid.UUID, cfg CatalogConfig) db.UpsertCata return params } +func validateCatalogConfigParams(params db.UpsertCatalogConfigParams) error { + if params.SruAddress != nil && params.ZoomAddress != nil { + return errors.New("catalogConfig cannot configure both SRU and ZOOM endpoints") + } + if len(params.HoldingsConfig) > 0 { + var holdings HoldingsParserConfig + if err := json.Unmarshal(params.HoldingsConfig, &holdings); err != nil { + return err + } + count := 0 + for _, present := range []bool{holdings.Marc != nil, holdings.Opac != nil, holdings.Reservoir != nil, holdings.Marc21plus1 != nil} { + if present { + count++ + } + } + if count > 1 { + return errors.New("catalogConfig.holdingsFormat must set at most one of marc, opac, reservoir, or marc21plus1") + } + } + return nil +} + func validateCatalogConfigPatch(cfg CatalogConfigPatch, original db.CatalogConfig) error { if cfg.Sru != nil && cfg.Sru.Address == nil && original.SruAddress == nil { return errors.New("catalogConfig.sru.address is required when creating SRU configuration") @@ -80,6 +104,8 @@ func validateCatalogConfigPatch(cfg CatalogConfigPatch, original db.CatalogConfi func catalogConfigPatchToDBParams(entryID uuid.UUID, cfg CatalogConfigPatch, original db.CatalogConfig) (db.UpsertCatalogConfigParams, error) { params := db.UpsertCatalogConfigParams{ Entry: &entryID, + Profile: maybeUpdateCol(original.Profile, cfg.Profile), + HoldingsConfig: original.HoldingsConfig, MetadataUpdateMode: original.MetadataUpdateMode, SruAddress: original.SruAddress, SruRecordSchema: original.SruRecordSchema, @@ -153,6 +179,11 @@ func catalogConfigPatchToDBParams(entryID uuid.UUID, cfg CatalogConfigPatch, ori params.QueryTitle = derefOrDefaultPtr(cfg.QueryConfig.Title, params.QueryTitle) } if cfg.HoldingsFormat != nil { + var mergeErr error + params.HoldingsConfig, mergeErr = mergeHoldingsConfig(original.HoldingsConfig, cfg.HoldingsFormat) + if mergeErr != nil { + return params, mergeErr + } if cfg.HoldingsFormat.Marc != nil { marc := cfg.HoldingsFormat.Marc params.HoldingsMarcCallNumberSubfield = derefOrDefaultPtr(marc.CallNumberSubField, params.HoldingsMarcCallNumberSubfield) @@ -276,3 +307,35 @@ func symbolsToFullSymbols(symbols *[]Symbol) []string { } return values } + +// mergeHoldingsConfig merges administrator overrides only. Defaults never enter persistence. +func mergeHoldingsConfig(original []byte, patch *HoldingsParserConfig) ([]byte, error) { + var old map[string]map[string]json.RawMessage + if len(original) > 0 { + if err := json.Unmarshal(original, &old); err != nil { + return nil, err + } + } + data, err := json.Marshal(patch) + if err != nil { + return nil, err + } + var next map[string]map[string]json.RawMessage + if err := json.Unmarshal(data, &next); err != nil { + return nil, err + } + if len(next) == 0 { + return original, nil + } + if len(next) == 1 { + for parser, fields := range next { + if prev, ok := old[parser]; ok { + for key, val := range fields { + prev[key] = val + } + next[parser] = prev + } + } + } + return json.Marshal(next) +} diff --git a/directory/api/entries.go b/directory/api/entries.go index 88186a71e..4f8a47cfa 100644 --- a/directory/api/entries.go +++ b/directory/api/entries.go @@ -299,12 +299,15 @@ func buildEntrySQL(whereClause string) string { )) FROM ill_configs i WHERE i.entry = e.id) as ill_config, ( SELECT - json_build_object( + json_strip_nulls(json_build_object( + 'vendor', l.vendor, + 'ncipNamespaceEnabled', l.ncip_namespace_enabled, + 'bibIdNormalization', l.bib_id_normalization, 'acceptItemEnabled', l.accept_item_enabled, - 'address',l.address, + 'address', NULLIF(l.address, ''), 'checkInItemEnabled', l.checkin_item_enabled, 'checkOutItemEnabled', l.checkout_item_enabled, - 'fromAgency',l.from_agency, + 'fromAgency', NULLIF(l.from_agency, ''), 'fromAgencyAuthentication', l.from_agency_authentication, 'itemLocation', l.item_location, 'lookupUserEnabled', l.lookup_user_enabled, @@ -318,11 +321,12 @@ func buildEntrySQL(whereClause string) string { 'supplierPickupLocation', l.supplier_pickup_location, 'patronProfiles', l.patron_profiles, 'toAgency', l.to_agency - ) + )) from lms_configs l WHERE l.entry = e.id) as lms_config, ( SELECT json_strip_nulls(json_build_object( + 'profile', h.profile, 'metadataUpdateMode', h.metadata_update_mode, 'sru', CASE WHEN h.sru_address IS NULL THEN NULL ELSE json_strip_nulls(json_build_object( 'address', h.sru_address, @@ -339,7 +343,7 @@ func buildEntrySQL(whereClause string) string { 'issn', h.query_issn, 'title', h.query_title )) END, - 'holdingsFormat', json_strip_nulls(json_build_object( + 'holdingsFormat', COALESCE(h.holdings_config, NULLIF(jsonb_strip_nulls(jsonb_build_object( 'marc', CASE WHEN h.holdings_marc_call_number_subfield IS NULL AND h.holdings_marc_item_id_subfield IS NULL AND h.holdings_marc_location_subfield IS NULL @@ -356,7 +360,7 @@ func buildEntrySQL(whereClause string) string { 'marc21plus1', CASE WHEN h.holdings_marc21plus1_enabled THEN json_build_object() ELSE NULL END, 'opac', CASE WHEN h.holdings_opac_enabled THEN json_build_object() ELSE NULL END, 'reservoir', CASE WHEN h.holdings_reservoir_enabled THEN json_build_object() ELSE NULL END - )), + )), '{}'::jsonb)), 'metadataFormat', CASE WHEN h.metadata_marc21_author IS NULL AND h.metadata_marc21_edition IS NULL AND h.metadata_marc21_identifier IS NULL @@ -764,6 +768,9 @@ func (a ApiImpl) AddEntry(ctx context.Context, request AddEntryRequestObject) (A lmsConfig := request.Body.LmsConfig _, err := qtx.UpsertLMSConfig(ctx, db.UpsertLMSConfigParams{ Entry: &insertedEntry.ID, + Vendor: maybeUpdateCol[string](nil, lmsConfig.Vendor), + NcipNamespaceEnabled: lmsConfig.NcipNamespaceEnabled, + BibIDNormalization: lmsConfig.BibIdNormalization, Address: lmsConfig.Address, FromAgency: lmsConfig.FromAgency, FromAgencyAuthentication: lmsConfig.FromAgencyAuthentication, @@ -790,7 +797,11 @@ func (a ApiImpl) AddEntry(ctx context.Context, request AddEntryRequestObject) (A } if request.Body.CatalogConfig != nil { - _, err := qtx.UpsertCatalogConfig(ctx, catalogConfigToDBParams(insertedEntry.ID, *request.Body.CatalogConfig)) + params := catalogConfigToDBParams(insertedEntry.ID, *request.Body.CatalogConfig) + if validationErr := validateCatalogConfigParams(params); validationErr != nil { + return AddEntry400TextResponse(validationErr.Error()), nil + } + _, err := qtx.UpsertCatalogConfig(ctx, params) if err != nil { slog.ErrorContext(ctx, "failed to create catalogConfig component", "error", err) return AddEntry500TextResponse("Internal server error"), nil @@ -1139,6 +1150,9 @@ func (a ApiImpl) UpdateEntry(ctx context.Context, request UpdateEntryRequestObje _, err = qtx.UpsertLMSConfig(ctx, db.UpsertLMSConfigParams{ Entry: &orig.ID, + Vendor: maybeUpdateCol(originalLMSConfig.Vendor, lmsConfig.Vendor), + NcipNamespaceEnabled: maybeUpdateCol(originalLMSConfig.NcipNamespaceEnabled, lmsConfig.NcipNamespaceEnabled), + BibIDNormalization: maybeUpdateCol(originalLMSConfig.BibIDNormalization, lmsConfig.BibIdNormalization), Address: derefOrDefault(lmsConfig.Address, originalLMSConfig.Address), FromAgency: derefOrDefault(lmsConfig.FromAgency, originalLMSConfig.FromAgency), FromAgencyAuthentication: maybeUpdateCol(originalLMSConfig.FromAgencyAuthentication, lmsConfig.FromAgencyAuthentication), @@ -1187,6 +1201,9 @@ func (a ApiImpl) UpdateEntry(ctx context.Context, request UpdateEntryRequestObje slog.ErrorContext(ctx, "unable to merge catalogConfig", "error", mergeErr) return UpdateEntry500TextResponse("Internal server error"), nil } + if validationErr := validateCatalogConfigParams(params); validationErr != nil { + return UpdateEntry400TextResponse(validationErr.Error()), nil + } _, err = qtx.UpsertCatalogConfig(ctx, params) if err != nil { slog.ErrorContext(ctx, "unexpected database error during catalogConfig upsert", "error", err) diff --git a/directory/api/host_profiles_test.go b/directory/api/host_profiles_test.go new file mode 100644 index 000000000..98b1c3bb8 --- /dev/null +++ b/directory/api/host_profiles_test.go @@ -0,0 +1,77 @@ +package api + +import ( + "encoding/json" + "github.com/google/uuid" + "github.com/indexdata/crosslink/directory/db" + "github.com/stretchr/testify/require" + "testing" +) + +func TestPersistRawProfileOverrides(t *testing.T) { + var cfg CatalogConfig + require.NoError(t, json.Unmarshal([]byte(`{"profile":"Sierra","holdingsFormat":{"opac":{"requireLocalLocation":false}}}`), &cfg)) + p := catalogConfigToDBParams(uuid.New(), cfg) + require.Equal(t, "Sierra", *p.Profile) + require.JSONEq(t, `{"opac":{"requireLocalLocation":false}}`, string(p.HoldingsConfig)) + require.Nil(t, p.ZoomAddress) + var patch CatalogConfigPatch + require.NoError(t, json.Unmarshal([]byte(`{"profile":null,"holdingsFormat":{"opac":{"includeItemId":false}}}`), &patch)) + next, err := catalogConfigPatchToDBParams(uuid.New(), patch, db.CatalogConfig{Profile: p.Profile, HoldingsConfig: p.HoldingsConfig}) + require.NoError(t, err) + require.Nil(t, next.Profile) + require.JSONEq(t, `{"opac":{"requireLocalLocation":false,"includeItemId":false}}`, string(next.HoldingsConfig)) + patch = CatalogConfigPatch{} + require.NoError(t, json.Unmarshal([]byte(`{"holdingsFormat":{"marc":{"mainField":"999"}}}`), &patch)) + next, err = catalogConfigPatchToDBParams(uuid.New(), patch, db.CatalogConfig{HoldingsConfig: p.HoldingsConfig}) + require.NoError(t, err) + require.JSONEq(t, `{"marc":{"mainField":"999"}}`, string(next.HoldingsConfig)) +} + +func TestEmptyHoldingsFormatPatchPreservesConfig(t *testing.T) { + for _, tc := range []struct { + name string + profile string + holdings []byte + }{ + {"generic overrides", "Generic", []byte(`{"marc":{"mainField":"999","itemIdSubField":"i"}}`)}, + {"vendor overrides", "Sierra", []byte(`{"opac":{"requireLocalLocation":false,"includeItemId":false}}`)}, + {"vendor defaults", "Sierra", nil}, + } { + t.Run(tc.name, func(t *testing.T) { + var patch CatalogConfigPatch + require.NoError(t, json.Unmarshal([]byte(`{"holdingsFormat":{}}`), &patch)) + original := db.CatalogConfig{Profile: &tc.profile, HoldingsConfig: tc.holdings} + params, err := catalogConfigPatchToDBParams(uuid.New(), patch, original) + require.NoError(t, err) + require.Equal(t, original.HoldingsConfig, params.HoldingsConfig) + require.Equal(t, original.Profile, params.Profile) + }) + } +} + +func TestValidateMergedHoldingsParsers(t *testing.T) { + original := db.CatalogConfig{HoldingsConfig: []byte(`{"marc":{"mainField":"999"},"opac":{}}`)} + for _, tc := range []struct { + name, patch string + valid bool + }{ + {"omitted parsers", `{}`, false}, + {"empty parsers", `{"holdingsFormat":{}}`, false}, + {"select one parser", `{"holdingsFormat":{"marc":{}}}`, true}, + {"replace parsers", `{"holdingsFormat":{"reservoir":{}}}`, true}, + } { + t.Run(tc.name, func(t *testing.T) { + var patch CatalogConfigPatch + require.NoError(t, json.Unmarshal([]byte(tc.patch), &patch)) + params, err := catalogConfigPatchToDBParams(uuid.New(), patch, original) + require.NoError(t, err) + err = validateCatalogConfigParams(params) + if tc.valid { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, "at most one") + } + }) + } +} diff --git a/directory/api/import_contract_test.go b/directory/api/import_contract_test.go index b73fef388..7acd98b21 100644 --- a/directory/api/import_contract_test.go +++ b/directory/api/import_contract_test.go @@ -43,3 +43,27 @@ func TestImportOpenAPIContract(t *testing.T) { require.Contains(t, entryData.Required, "lmsConfig") require.Contains(t, entryData.Required, "holdingsPolicy") } + +func TestImportHostSettingsMatchEntryContract(t *testing.T) { + spec, err := GetSpec() + require.NoError(t, err) + source, err := openapi3.NewLoader().LoadFromFile("../api.yaml") + require.NoError(t, err) + for _, name := range []string{"LmsConfig", "CatalogConfig", "HoldingsParserConfig", "MarcHoldingsParserConfig", "OpacHoldingsParserConfig", "MarcAvailabilityPredicate"} { + t.Run(name, func(t *testing.T) { + for _, contract := range []*openapi3.T{source, spec} { + entry := contract.Components.Schemas[name].Value + imported := contract.Components.Schemas["Import"+name].Value + fields := make([]string, 0, len(entry.Properties)) + for field := range entry.Properties { + fields = append(fields, field) + require.Contains(t, imported.Properties, field) + } + require.Len(t, imported.Properties, len(fields)) + require.ElementsMatch(t, fields, imported.Required) + require.NotNil(t, imported.AdditionalProperties.Has) + require.False(t, *imported.AdditionalProperties.Has) + } + }) + } +} diff --git a/directory/host-profiles.md b/directory/host-profiles.md new file mode 100644 index 000000000..c2645d803 --- /dev/null +++ b/directory/host-profiles.md @@ -0,0 +1,176 @@ +# Host LMS and catalog profiles + +`lmsConfig.vendor` selects circulation defaults. `catalogConfig.profile` selects +catalog query, metadata, holdings and technical availability defaults. Both are +independent of `illConfig.iso18626Vendor`, which selects the ILL/ISO 18626 +implementation. The deprecated top-level `vendor` never supplies a host profile. + +For LMS settings, the broker applies Generic defaults, then the selected vendor, +then explicit directory values. For catalog settings, it applies Generic defaults, +then `catalogConfig.profile` (or `lmsConfig.vendor` when no catalog profile is set), +then explicit catalog values. Missing or null profiles resolve to Generic, except +that an unset catalog profile inherits the LMS vendor. Setting the catalog profile +to `Generic` explicitly prevents that inheritance. + +```yaml +illConfig: + iso18626Vendor: ReShare +lmsConfig: + vendor: Sierra + address: https://library.example.org/ncip + fromAgency: EXAMPLE + requestItemPickupLocationEnabled: false +catalogConfig: + zoom: + address: z3950.library.example.org:210/catalog + holdingsFormat: + opac: + availablePublicNotes: [AVAILABLE, CHECK SHELVES, CHECK SHELF] +``` + +A profile alone does not enable an adapter. NCIP requires both `address` and +`fromAgency`; omit both for a catalog-only institution. A partially configured +NCIP connection fails broker validation. Catalog lookup requires an SRU or ZOOM +address. Profiles never supply endpoints, credentials, agencies, databases, patron +identifiers, pickup codes, or lending policies. Existing Generic circulation +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. + +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. + +| Profile | Catalog defaults | Circulation defaults | +| --- | --- | --- | +| Generic | Existing MARC parser (852: location b, shelving c, call number h, item p, restricted r); existing PQF or configured CQL queries | Existing NCIP 2 behavior: Page, Item scope, pickup enabled | +| Alma | OPAC; nonempty local location and availableNow value 1; item ID and availableThru loan policy | NCIP 2 | +| Sierra | OPAC; publicNote exactly AVAILABLE, CHECK SHELVES, or CHECK SHELF; localLocation supplies both location and shelving; no item ID or loan policy | NCIP 2; Hold, Title scope; Sierra bib-ID normalization | +| Koha | MARC holdings (ZOOM requests `usmarc`, converted to XML for parsing); one candidate per 952; b location, c shelving, o call number; 7 must equal 0 and q must be absent; no item ID | NCIP 2 with XML namespace disabled | +| FOLIO | Alma OPAC mappings plus temporaryLocation as temporary shelving location | NCIP 2; Page; Item scope (Title can be configured); pickup enabled when a location is supplied | + +WMS and Aleph are reserved enum values for follow-up work. Selecting either +produces a broker validation error identifying the unsupported profile. + +All supported vendor profiles use the existing PQF indexes: identifier 12, +ISBN 7, ISSN 8, title 4. Explicit `queryConfig.type: cql` selects the existing CQL +mappings (`rec.id`, `isbn`, `issn`, `title`). Each query template can be overridden; +an empty template disables that field. Metadata uses the existing MARC mappings +and can extract the bibliographic MARC record embedded in OPAC. + +Record defaults follow the effective holdings parser, including for Generic: + +| Holdings parser | Native ZOOM `preferredRecordSyntax` | SRU `recordSchema` | +| --- | --- | --- | +| `marc` (including Koha) | `usmarc` | `marcxml` | +| `opac` | `opac` | `opac` | +| `reservoir` | `usmarc` | `marcxml` | +| `marc21plus1` | `xml` | `marcxml` | + +For Koha, native ZOOM requests USMARC over Z39.50 and converts the returned records +to MARCXML before parsing the holdings. The metaproxy adapter requests SRU schema +`opac` when its ZOOM settings select OPAC, and `marcxml` otherwise; backend syntax +conversion is controlled by the proxy configuration. Changing the holdings parser +changes the default syntax to match. Explicit syntax/schema settings take precedence. + +Every returned record within existing adapter limits is processed. Available +holdings are concatenated across records. Lookup stops at the first query with +holdings; otherwise it continues through identifier, ISBN, ISSN and title. +Profiles do not change this sequence or the directory's `holdingsPolicy`. + +## Overrides + +Explicit values win, including `false`, empty strings where valid, and empty +arrays. Objects merge recursively. Arrays replace their defaults. A partial +`holdingsFormat` using the same parser merges with that profile's mapping and +rules. Selecting a different parser discards the previous parser and its defaults. +Select exactly one of `marc`, `opac`, `reservoir`, or `marc21plus1`. + +For example, this keeps Sierra circulation but uses Koha catalog defaults: + +```yaml +lmsConfig: + vendor: Sierra +catalogConfig: + profile: Koha + sru: + address: https://catalog.example.org/sru + holdingsFormat: + marc: + callNumberSubField: h +``` + +MARC `availability` is an array of predicates that must all pass. `equals` +requires a present subfield with the exact configured value; `absent` rejects +any occurrence, including an empty subfield. Koha's default is: + +```yaml +availability: + - subField: '7' + operator: equals + value: '0' + - subField: q + operator: absent +``` + +`availability: []` clears these predicates. The existing `restrictedSubField` +remains available independently. Generic partial MARC mappings retain legacy +behavior: only specified fields are mapped; an empty MARC configuration uses +852 defaults. A custom MARC mapping should provide `mainField`. + +OPAC overrides include: + +- `availabilityRule`: `availableNow` or `publicNote`. +- `availablePublicNotes`: exact accepted strings; required for `publicNote`. +- `requireLocalLocation`: require a nonempty local location. +- `shelvingLocationSource`: `shelvingLocation` or `localLocation`. +- `includeItemId`, `includeItemLoanPolicy`: circulation mappings. +- `includeTemporaryLocation`: use a nonempty, trimmed circulation temporary location as the effective shelving location for holdings policy and supplier ordering; otherwise retain the permanent shelving location. Enabled by default for FOLIO. +- `allCirculations`: emit every available circulation, instead of the first per holding. + +Generic OPAC retains its previous first-available-circulation behavior and does +not require a local location. Alma and FOLIO emit every technically available +circulation with a nonempty local location. Sierra evaluates each holding's +public note even when no circulation elements are present. + +LMS overrides include existing operation enablement, RequestItem type, scope, +bib-ID code and pickup behavior, plus `ncipNamespaceEnabled` and +`bibIdNormalization` (`none` or `sierra`). Sierra normalization strips a leading +`.b` and removes a trailing digit when more than one character remains, matching +mod-rs. Set `none` for an endpoint accepting the original bibliographic ID. + +## Persistence, diagnostics, and migration + +Directory GET responses represent administrator overrides. The API does not +materialize LMS or query defaults on POST/PATCH. Omitted settings inherit at +adapter creation; default corrections therefore take effect without rewriting +entries. PATCH a profile to null to clear its explicit selection. The new LMS +namespace and normalization overrides also support null to resume inheritance. +Partial holdings PATCHes merge stored overrides for the same parser and replace +them when switching parser. + +Migration `009_host_profiles` adds nullable profile/protocol columns and a JSON +holdings configuration. It preserves existing explicit holdings settings. It does +not select profiles or copy preset values into any row. Existing entries without +host profiles continue to resolve as Generic. Values previously materialized by +older directory versions remain explicit overrides; clear those LMS values with +PATCH null where profile inheritance is desired. + +At debug level the broker logs `resolved host profiles`, including effective +behavior settings and a per-field `origins` map (`Generic`, profile name, or +`directory`). Endpoints, credentials, agencies, patrons and local policies are +excluded. The same representation is available internally through +`profiles.Effective.Diagnostics()`. + +Generated Go API models and embedded schemas are rebuilt with +`make -C directory generate`; generated files remain ignored according to the +repository convention. diff --git a/directory/import/db/entry.go b/directory/import/db/entry.go index 6bb4d6f15..715c4d53c 100644 --- a/directory/import/db/entry.go +++ b/directory/import/db/entry.go @@ -506,6 +506,7 @@ func replaceEntryConfigs(ctx context.Context, queries *db.Queries, entryID uuid. patronProfiles, _ = json.Marshal(cfg.PatronProfiles) } if _, err := queries.UpsertLMSConfig(ctx, db.UpsertLMSConfigParams{ + Vendor: cfg.Vendor, NcipNamespaceEnabled: cfg.NcipNamespaceEnabled, BibIDNormalization: cfg.BibIDNormalization, Entry: &entryID, Address: cfg.Address, FromAgency: cfg.FromAgency, FromAgencyAuthentication: cfg.FromAgencyAuthentication, ToAgency: cfg.ToAgency, LookupUserEnabled: cfg.LookupUserEnabled, AcceptItemEnabled: cfg.AcceptItemEnabled, CheckinItemEnabled: cfg.CheckInItemEnabled, CheckoutItemEnabled: cfg.CheckOutItemEnabled, ItemLocation: cfg.ItemLocation, @@ -543,7 +544,7 @@ func replaceCatalogConfig(ctx context.Context, queries *db.Queries, entryID uuid if err := queries.DeleteCatalogConfigByEntry(ctx, entryID); err != nil || config == nil { return err } - params := db.UpsertCatalogConfigParams{Entry: &entryID, MetadataUpdateMode: config.MetadataUpdateMode} + params := db.UpsertCatalogConfigParams{Entry: &entryID, Profile: config.Profile, MetadataUpdateMode: config.MetadataUpdateMode} if config.SRU != nil { params.SruAddress, params.SruRecordSchema = &config.SRU.Address, config.SRU.RecordSchema } @@ -557,6 +558,11 @@ func replaceCatalogConfig(ctx context.Context, queries *db.Queries, entryID uuid params.QueryType, params.QueryIdentifier, params.QueryIsbn, params.QueryIssn, params.QueryTitle = config.Query.Type, config.Query.Identifier, config.Query.ISBN, config.Query.ISSN, config.Query.Title } if config.HoldingsFormat != nil { + var err error + params.HoldingsConfig, err = json.Marshal(config.HoldingsFormat) + if err != nil { + return err + } if config.HoldingsFormat.Marc != nil { marc := config.HoldingsFormat.Marc params.HoldingsMarcCallNumberSubfield, params.HoldingsMarcItemIDSubfield = marc.CallNumberSubField, marc.ItemIDSubField diff --git a/directory/import/db/repo_test.go b/directory/import/db/repo_test.go index b16da5d01..4c8407d1f 100644 --- a/directory/import/db/repo_test.go +++ b/directory/import/db/repo_test.go @@ -1327,7 +1327,6 @@ func completeEntryAggregate(symbol string) model.EntryAggregate { aggregate.Data.LMSConfig = &model.LMSConfig{Address: "https://example.test/ncip", FromAgency: "FROM", FromAgencyAuthentication: stringPointer("credential-value")} aggregate.Data.CatalogConfig = &model.CatalogConfig{ MetadataUpdateMode: &metadataMode, - SRU: &model.SRUConfig{Address: "https://example.test/sru"}, Zoom: &model.ZoomConfig{Address: "example.test:210", Options: &map[string]string{"user": "private"}}, Query: &model.QueryConfig{Identifier: &text}, HoldingsFormat: &model.HoldingsParserConfig{Marc: &model.MarcHoldingsParserConfig{MainField: &text}}, diff --git a/directory/import/model/config.go b/directory/import/model/config.go index 6047cdf6b..1f358c11d 100644 --- a/directory/import/model/config.go +++ b/directory/import/model/config.go @@ -6,6 +6,9 @@ import ( ) type LMSConfig struct { + Vendor *string `json:"vendor"` + NcipNamespaceEnabled *bool `json:"ncipNamespaceEnabled"` + BibIDNormalization *string `json:"bibIdNormalization"` Address string `json:"address"` FromAgency string `json:"fromAgency"` FromAgencyAuthentication *string `json:"fromAgencyAuthentication"` @@ -47,6 +50,7 @@ type ILLConfig struct { } type CatalogConfig struct { + Profile *string `json:"profile"` MetadataUpdateMode *string `json:"metadataUpdateMode"` SRU *SRUConfig `json:"sru"` Zoom *ZoomConfig `json:"zoom"` @@ -74,19 +78,37 @@ type QueryConfig struct { } type HoldingsParserConfig struct { - Marc *MarcHoldingsParserConfig `json:"marc"` - Marc21Plus1 *map[string]any `json:"marc21plus1"` - OPAC *map[string]any `json:"opac"` - Reservoir *map[string]any `json:"reservoir"` + Marc *MarcHoldingsParserConfig `json:"marc,omitempty"` + Marc21Plus1 *map[string]any `json:"marc21plus1,omitempty"` + OPAC *OpacHoldingsParserConfig `json:"opac,omitempty"` + Reservoir *map[string]any `json:"reservoir,omitempty"` } type MarcHoldingsParserConfig struct { - CallNumberSubField *string `json:"callNumberSubField"` - ItemIDSubField *string `json:"itemIdSubField"` - LocationSubField *string `json:"locationSubField"` - MainField *string `json:"mainField"` - RestrictedSubField *string `json:"restrictedSubField"` - ShelvingLocationSubField *string `json:"shelvingLocationSubField"` + Availability *[]MarcAvailabilityPredicate `json:"availability,omitempty"` + CallNumberSubField *string `json:"callNumberSubField,omitempty"` + ItemIDSubField *string `json:"itemIdSubField,omitempty"` + LocationSubField *string `json:"locationSubField,omitempty"` + MainField *string `json:"mainField,omitempty"` + RestrictedSubField *string `json:"restrictedSubField,omitempty"` + ShelvingLocationSubField *string `json:"shelvingLocationSubField,omitempty"` +} + +type MarcAvailabilityPredicate struct { + SubField string `json:"subField"` + Operator string `json:"operator"` + Value *string `json:"value,omitempty"` +} + +type OpacHoldingsParserConfig struct { + AvailabilityRule *string `json:"availabilityRule,omitempty"` + AvailablePublicNotes *[]string `json:"availablePublicNotes,omitempty"` + RequireLocalLocation *bool `json:"requireLocalLocation,omitempty"` + ShelvingLocationSource *string `json:"shelvingLocationSource,omitempty"` + IncludeItemID *bool `json:"includeItemId,omitempty"` + IncludeItemLoanPolicy *bool `json:"includeItemLoanPolicy,omitempty"` + IncludeTemporaryLocation *bool `json:"includeTemporaryLocation,omitempty"` + AllCirculations *bool `json:"allCirculations,omitempty"` } type MetadataParserConfig struct { @@ -130,7 +152,49 @@ type HoldingsItemLoanPolicy struct { Lendable bool `json:"lendable"` } -func validateConfigEnums(catalog *CatalogConfig, ill *ILLConfig) error { +func validateConfigEnums(lms *LMSConfig, catalog *CatalogConfig, ill *ILLConfig) error { + if lms != nil { + if lms.Vendor != nil && !oneOf(*lms.Vendor, "Generic", "Alma", "Sierra", "Koha", "FOLIO", "WMS", "Aleph") { + return fmt.Errorf("invalid lmsConfig.vendor") + } + if lms.BibIDNormalization != nil && !oneOf(*lms.BibIDNormalization, "none", "sierra") { + return fmt.Errorf("invalid lmsConfig.bibIdNormalization") + } + } + if catalog != nil { + if catalog.Profile != nil && !oneOf(*catalog.Profile, "Generic", "Alma", "Sierra", "Koha", "FOLIO", "WMS", "Aleph") { + return fmt.Errorf("invalid catalogConfig.profile") + } + if catalog.SRU != nil && catalog.Zoom != nil { + return fmt.Errorf("catalogConfig cannot configure both SRU and ZOOM endpoints") + } + if h := catalog.HoldingsFormat; h != nil { + count := 0 + for _, present := range []bool{h.Marc != nil, h.OPAC != nil, h.Reservoir != nil, h.Marc21Plus1 != nil} { + if present { + count++ + } + } + if count > 1 { + return fmt.Errorf("catalogConfig.holdingsFormat must set at most one of marc, opac, reservoir, or marc21plus1") + } + if h.Marc != nil && h.Marc.Availability != nil { + for _, predicate := range *h.Marc.Availability { + if predicate.SubField == "" || !oneOf(predicate.Operator, "equals", "absent") || (predicate.Operator == "equals" && predicate.Value == nil) { + return fmt.Errorf("invalid catalogConfig.holdingsFormat.marc.availability predicate") + } + } + } + if h.OPAC != nil { + if h.OPAC.AvailabilityRule != nil && !oneOf(*h.OPAC.AvailabilityRule, "availableNow", "publicNote") { + return fmt.Errorf("invalid catalogConfig.holdingsFormat.opac.availabilityRule") + } + if h.OPAC.ShelvingLocationSource != nil && !oneOf(*h.OPAC.ShelvingLocationSource, "shelvingLocation", "localLocation") { + return fmt.Errorf("invalid catalogConfig.holdingsFormat.opac.shelvingLocationSource") + } + } + } + } if ill != nil && ill.ISO18626Vendor != nil && !oneOf(*ill.ISO18626Vendor, "Alma", "ReShare", "CrossLink", "ILLiad", "Unknown") { return fmt.Errorf("invalid ILL vendor: %s", *ill.ISO18626Vendor) } diff --git a/directory/import/model/models.go b/directory/import/model/models.go index 1381cd32d..6b373bf2e 100644 --- a/directory/import/model/models.go +++ b/directory/import/model/models.go @@ -188,7 +188,7 @@ func (a *EntryAggregate) NormalizeAndValidate() error { return fmt.Errorf("closure %d endDate must not precede startDate", index+1) } } - return validateConfigEnums(a.Data.CatalogConfig, a.Data.ILLConfig) + return validateConfigEnums(a.Data.LMSConfig, a.Data.CatalogConfig, a.Data.ILLConfig) } type TierKey struct { diff --git a/directory/import/service/importer_test.go b/directory/import/service/importer_test.go index 86955f4f6..04bca727b 100644 --- a/directory/import/service/importer_test.go +++ b/directory/import/service/importer_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "encoding/json" "errors" "strings" "testing" @@ -233,7 +234,7 @@ func TestImportAcceptsNullLMSPatronProfiles(t *testing.T) { } func validLMSConfig() string { - return `"lmsConfig":{"address":"https://example.test/ncip","fromAgency":"FROM","fromAgencyAuthentication":null,"toAgency":null,"lookupUserEnabled":true,"acceptItemEnabled":true,"checkInItemEnabled":true,"checkOutItemEnabled":true,"itemLocation":null,"requestItemRequestType":null,"requestItemRequestScopeType":null,"requestItemBibIdCode":null,"requestItemEnabled":true,"requestItemPickupLocationEnabled":true,"requesterPickupLocation":null,"supplierPickupLocation":null,"requesterPatronPattern":null,"patronProfiles":[{"code":"STAFF","canCreateRequests":true}]}` + return `"lmsConfig":{"vendor":null,"ncipNamespaceEnabled":null,"bibIdNormalization":null,"address":"https://example.test/ncip","fromAgency":"FROM","fromAgencyAuthentication":null,"toAgency":null,"lookupUserEnabled":true,"acceptItemEnabled":true,"checkInItemEnabled":true,"checkOutItemEnabled":true,"itemLocation":null,"requestItemRequestType":null,"requestItemRequestScopeType":null,"requestItemBibIdCode":null,"requestItemEnabled":true,"requestItemPickupLocationEnabled":true,"requesterPickupLocation":null,"supplierPickupLocation":null,"requesterPatronPattern":null,"patronProfiles":[{"code":"STAFF","canCreateRequests":true}]}` } func validILLConfig() string { @@ -384,3 +385,71 @@ func loadImportSpec(t *testing.T) *openapi3.T { require.NoError(t, spec.Validate(context.Background())) return spec } + +func TestImportValidatesHostSettings(t *testing.T) { + const catalogJSON = `{"profile":"Koha","metadataUpdateMode":null,"sru":null,"zoom":null,"queryConfig":null,"metadataFormat":null,"holdingsFormat":{"marc":{"availability":[{"subField":"7","operator":"equals","value":"0"}],"mainField":null,"callNumberSubField":null,"itemIdSubField":null,"locationSubField":null,"restrictedSubField":null,"shelvingLocationSubField":null},"opac":null,"reservoir":null,"marc21plus1":null}}` + const opacJSON = `{"availabilityRule":"publicNote","availablePublicNotes":[],"requireLocalLocation":false,"shelvingLocationSource":"localLocation","includeItemId":false,"includeItemLoanPolicy":false,"includeTemporaryLocation":true,"allCirculations":true}` + for _, tc := range []struct { + name string + edit func(lms, catalog map[string]any) + }{ + {"unknown vendor", func(l, c map[string]any) { l["vendor"] = "bad" }}, + {"missing vendor", func(l, c map[string]any) { delete(l, "vendor") }}, + {"unknown normalization", func(l, c map[string]any) { l["bibIdNormalization"] = "bad" }}, + {"namespace type", func(l, c map[string]any) { l["ncipNamespaceEnabled"] = "false" }}, + {"unknown profile", func(l, c map[string]any) { c["profile"] = "bad" }}, + {"missing profile", func(l, c map[string]any) { delete(c, "profile") }}, + {"two endpoints", func(l, c map[string]any) { + c["sru"] = map[string]any{"address": "https://example/sru", "recordSchema": nil} + c["zoom"] = map[string]any{"address": "example:210", "options": nil} + }}, + {"two parsers", func(l, c map[string]any) { c["holdingsFormat"].(map[string]any)["reservoir"] = map[string]any{} }}, + {"unknown parser option", func(l, c map[string]any) { + c["holdingsFormat"].(map[string]any)["marc"].(map[string]any)["unknown"] = true + }}, + {"missing availability", func(l, c map[string]any) { + delete(c["holdingsFormat"].(map[string]any)["marc"].(map[string]any), "availability") + }}, + {"equals without value", func(l, c map[string]any) { + c["holdingsFormat"].(map[string]any)["marc"].(map[string]any)["availability"].([]any)[0].(map[string]any)["value"] = nil + }}, + {"invalid predicate operator", func(l, c map[string]any) { + c["holdingsFormat"].(map[string]any)["marc"].(map[string]any)["availability"].([]any)[0].(map[string]any)["operator"] = "bad" + }}, + {"empty predicate subfield", func(l, c map[string]any) { + c["holdingsFormat"].(map[string]any)["marc"].(map[string]any)["availability"].([]any)[0].(map[string]any)["subField"] = "" + }}, + {"invalid OPAC rule", func(l, c map[string]any) { + var opac map[string]any + require.NoError(t, json.Unmarshal([]byte(opacJSON), &opac)) + opac["availabilityRule"] = "bad" + h := c["holdingsFormat"].(map[string]any) + h["marc"], h["opac"] = nil, opac + }}, + {"invalid OPAC location", func(l, c map[string]any) { + var opac map[string]any + require.NoError(t, json.Unmarshal([]byte(opacJSON), &opac)) + opac["shelvingLocationSource"] = "bad" + h := c["holdingsFormat"].(map[string]any) + h["marc"], h["opac"] = nil, opac + }}, + } { + t.Run(tc.name, func(t *testing.T) { + repo := &recordingRepo{result: model.RepoResult{Outcome: model.OutcomeImported}} + var record map[string]any + require.NoError(t, json.Unmarshal([]byte(strings.Replace(validEntryRecord(), `"lmsConfig":null`, validLMSConfig(), 1)), &record)) + data := record["data"].(map[string]any) + var catalog map[string]any + require.NoError(t, json.Unmarshal([]byte(catalogJSON), &catalog)) + data["catalogConfig"] = catalog + tc.edit(data["lmsConfig"].(map[string]any), catalog) + payload, err := json.Marshal(record) + require.NoError(t, err) + result, err := newTestImporter(t, repo).Import(context.Background(), model.ConflictPolicyFail, strings.NewReader(string(payload))) + require.NoError(t, err) + require.Equal(t, int32(1), result.Entries.Failed) + require.Len(t, result.Errors, 1) + require.Zero(t, repo.entryCalls) + }) + } +} diff --git a/directory/migrations/009_host_profiles.down.sql b/directory/migrations/009_host_profiles.down.sql new file mode 100644 index 000000000..89b9c7d49 --- /dev/null +++ b/directory/migrations/009_host_profiles.down.sql @@ -0,0 +1,5 @@ +ALTER TABLE lms_configs DROP COLUMN vendor; +ALTER TABLE lms_configs DROP COLUMN ncip_namespace_enabled; +ALTER TABLE lms_configs DROP COLUMN bib_id_normalization; +ALTER TABLE catalog_configs DROP COLUMN profile; +ALTER TABLE catalog_configs DROP COLUMN holdings_config; diff --git a/directory/migrations/009_host_profiles.up.sql b/directory/migrations/009_host_profiles.up.sql new file mode 100644 index 000000000..f9c44399a --- /dev/null +++ b/directory/migrations/009_host_profiles.up.sql @@ -0,0 +1,25 @@ +ALTER TABLE lms_configs ADD COLUMN vendor text; +ALTER TABLE lms_configs ADD COLUMN ncip_namespace_enabled boolean; +ALTER TABLE lms_configs ADD COLUMN bib_id_normalization text; +ALTER TABLE catalog_configs ADD COLUMN profile text; +ALTER TABLE catalog_configs ADD COLUMN holdings_config jsonb; + +-- Preserve existing administrator parser settings for subsequent partial PATCHes. +UPDATE catalog_configs h SET holdings_config = NULLIF((json_strip_nulls(json_build_object( + 'marc', CASE WHEN h.holdings_marc_call_number_subfield IS NULL + AND h.holdings_marc_item_id_subfield IS NULL + AND h.holdings_marc_location_subfield IS NULL + AND h.holdings_marc_main_field IS NULL + AND h.holdings_marc_restricted_subfield IS NULL + AND h.holdings_marc_shelving_location_subfield IS NULL THEN NULL ELSE json_strip_nulls(json_build_object( + 'callNumberSubField', h.holdings_marc_call_number_subfield, + 'itemIdSubField', h.holdings_marc_item_id_subfield, + 'locationSubField', h.holdings_marc_location_subfield, + 'mainField', h.holdings_marc_main_field, + 'restrictedSubField', h.holdings_marc_restricted_subfield, + 'shelvingLocationSubField', h.holdings_marc_shelving_location_subfield + )) END, + 'marc21plus1', CASE WHEN h.holdings_marc21plus1_enabled THEN json_build_object() ELSE NULL END, + 'opac', CASE WHEN h.holdings_opac_enabled THEN json_build_object() ELSE NULL END, + 'reservoir', CASE WHEN h.holdings_reservoir_enabled THEN json_build_object() ELSE NULL END + )))::jsonb, '{}'::jsonb); diff --git a/directory/query.sql b/directory/query.sql index 62b974115..6981d90cb 100644 --- a/directory/query.sql +++ b/directory/query.sql @@ -222,7 +222,7 @@ INSERT INTO catalog_configs ( holdings_marc_main_field, holdings_marc_restricted_subfield, holdings_marc_shelving_location_subfield, holdings_marc21plus1_enabled, holdings_opac_enabled, holdings_reservoir_enabled, metadata_marc21_author, metadata_marc21_edition, metadata_marc21_identifier, metadata_marc21_isbn, - metadata_marc21_issn, metadata_marc21_subtitle, metadata_marc21_title + metadata_marc21_issn, metadata_marc21_subtitle, metadata_marc21_title, profile, holdings_config ) VALUES ( coalesce(sqlc.narg('id'), gen_random_uuid()), @entry, @@ -251,9 +251,13 @@ INSERT INTO catalog_configs ( @metadata_marc21_isbn, @metadata_marc21_issn, @metadata_marc21_subtitle, - @metadata_marc21_title + @metadata_marc21_title, + @profile, + @holdings_config ) ON CONFLICT (entry) DO UPDATE SET + profile = @profile, + holdings_config = @holdings_config, metadata_update_mode = @metadata_update_mode, sru_address = @sru_address, sru_record_schema = @sru_record_schema, @@ -309,7 +313,7 @@ INSERT INTO lms_configs ( accept_item_enabled, checkin_item_enabled, checkout_item_enabled, item_location, request_item_request_type, request_item_scope_type, request_item_bib_code, request_item_enabled, request_item_pickup_location_enabled, requester_pickup_location, supplier_pickup_location, - requester_patron_pattern, patron_profiles + requester_patron_pattern, patron_profiles, vendor, ncip_namespace_enabled, bib_id_normalization ) VALUES ( coalesce(sqlc.narg('id'), gen_random_uuid()), @entry, @@ -330,9 +334,15 @@ INSERT INTO lms_configs ( @requester_pickup_location, @supplier_pickup_location, @requester_patron_pattern, - @patron_profiles + @patron_profiles, + @vendor, + @ncip_namespace_enabled, + @bib_id_normalization ) ON CONFLICT (entry) DO UPDATE SET + vendor = @vendor, + ncip_namespace_enabled = @ncip_namespace_enabled, + bib_id_normalization = @bib_id_normalization, address = @address, from_agency = @from_agency, from_agency_authentication = @from_agency_authentication, diff --git a/directory/test/apifixtures/entry-new-lmsconfig-incomplete.patch.refetch.json b/directory/test/apifixtures/entry-new-lmsconfig-incomplete.patch.refetch.json index 1764ee370..8598e3f3a 100644 --- a/directory/test/apifixtures/entry-new-lmsconfig-incomplete.patch.refetch.json +++ b/directory/test/apifixtures/entry-new-lmsconfig-incomplete.patch.refetch.json @@ -27,8 +27,6 @@ } ], "lmsConfig" : { - "address" : "", - "fromAgency" : "", "acceptItemEnabled":false } } diff --git a/directory/test/apifixtures/entry-with-lmsconfig.post.refetch.json b/directory/test/apifixtures/entry-with-lmsconfig.post.refetch.json index 0ffd641b8..7a0fa181f 100644 --- a/directory/test/apifixtures/entry-with-lmsconfig.post.refetch.json +++ b/directory/test/apifixtures/entry-with-lmsconfig.post.refetch.json @@ -5,23 +5,10 @@ "lmsConfig" : { "address" : "https://do.not.taunt.happy.fun.ball", "fromAgency" : "happyfunballdotcom", - "acceptItemEnabled":true, - "checkInItemEnabled":true, - "checkOutItemEnabled":true, - "itemLocation":"", - "lookupUserEnabled":true, "patronProfiles":[ {"code":"STAFF","name":"Staff","canCreateRequests":true}, {"code":"BLOCKED","canCreateRequests":false}, {"canCreateRequests":true} - ], - "requestItemBibIdCode":"SYSNUMBER", - "requestItemEnabled":true, - "requestItemPickupLocationEnabled":true, - "requestItemRequestScopeType":"Item", - "requestItemRequestType":"Page", - "requesterPatronPattern":"INST-{requesterSymbol}", - "requesterPickupLocation":"Main Library", - "supplierPickupLocation":"ILL Office" + ] } } diff --git a/directory/test/entries_test.go b/directory/test/entries_test.go index 95cbe56ba..74c98ca40 100644 --- a/directory/test/entries_test.go +++ b/directory/test/entries_test.go @@ -759,8 +759,7 @@ func TestEntryDirectoryContractFieldsAndCatalogConfig(t *testing.T) { "callNumberSubField":"c", "restrictedSubField":"r", "shelvingLocationSubField":"s" - }, - "opac":{} + } }, "metadataFormat":{ "marc21":{ @@ -1027,7 +1026,6 @@ func TestPatchCatalogConfigRequiresAddressForCreation(t *testing.T) { res, data := jsonReq(t, http.MethodPatch, "/entries/by-id/"+entryID, `{ "catalogConfig":{ - "sru":{"address":"https://catalog.example/sru","recordSchema":"marcxml"}, "zoom":{"address":"catalog.example:210/db","options":{"count":"10"}} } }`, headers) @@ -1037,7 +1035,6 @@ func TestPatchCatalogConfigRequiresAddressForCreation(t *testing.T) { res, data = jsonReq(t, http.MethodPatch, "/entries/by-id/"+entryID, `{ "catalogConfig":{ - "sru":{"recordSchema":"mods"}, "zoom":{"options":{"count":"20"}} } }`, headers) @@ -1054,11 +1051,9 @@ func TestPatchCatalogConfigRequiresAddressForCreation(t *testing.T) { t.Fatalf("failed to parse entry after catalogConfig updates: %v", err) } catalogConfig := entry["catalogConfig"].(map[string]any) - sru := catalogConfig["sru"].(map[string]any) zoom := catalogConfig["zoom"].(map[string]any) options := zoom["options"].(map[string]any) - if sru["address"] != "https://catalog.example/sru" || sru["recordSchema"] != "mods" || - zoom["address"] != "catalog.example:210/db" || options["count"] != "20" { + if zoom["address"] != "catalog.example:210/db" || options["count"] != "20" { t.Fatalf("catalogConfig creation or partial update did not round-trip: %#v", catalogConfig) } diff --git a/directory/test/host_profiles_migration_test.go b/directory/test/host_profiles_migration_test.go new file mode 100644 index 000000000..92f51b546 --- /dev/null +++ b/directory/test/host_profiles_migration_test.go @@ -0,0 +1,112 @@ +package test + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" +) + +func TestHostProfilesMigrationPreservesLegacyHoldings(t *testing.T) { + ctx := context.Background() + tx, err := dbpool.Begin(ctx) + require.NoError(t, err) + defer func() { _ = tx.Rollback(ctx) }() + + exec := func(query string, args ...any) { + t.Helper() + _, err := tx.Exec(ctx, query, args...) + require.NoError(t, err) + } + // Build a real pre-009 schema without changing the integration suite's tables. + exec("CREATE SCHEMA host_profiles_migration_test; SET LOCAL search_path TO host_profiles_migration_test") + apply := func(version int) { + t.Helper() + paths, err := filepath.Glob(fmt.Sprintf("../migrations/%03d_*.up.sql", version)) + require.NoError(t, err) + require.Len(t, paths, 1) + data, err := os.ReadFile(paths[0]) + require.NoError(t, err) + exec(string(data)) + } + for version := 1; version <= 8; version++ { + apply(version) + } + + type legacyHoldings struct { + name string + marc [6]*string + opac, reservoir, marc21plus1 *bool + want string // Empty means SQL NULL, not an empty JSON object. + } + str := func(s string) *string { return &s } + enabled, disabled := true, false + cases := []legacyHoldings{ + {name: "unset"}, + {name: "disabled flags", opac: &disabled, reservoir: &disabled, marc21plus1: &disabled}, + {name: "opac", opac: &enabled, reservoir: &disabled, want: `{"opac":{}}`}, + {name: "reservoir", reservoir: &enabled, marc21plus1: &disabled, want: `{"reservoir":{}}`}, + {name: "marc21plus1", marc21plus1: &enabled, opac: &disabled, want: `{"marc21plus1":{}}`}, + { + name: "all MARC fields", + marc: [6]*string{str("h"), str("p"), str("b"), str("952"), str("r"), str("c")}, + want: `{"marc":{"callNumberSubField":"h","itemIdSubField":"p","locationSubField":"b","mainField":"952","restrictedSubField":"r","shelvingLocationSubField":"c"}}`, + }, + // Preserve mixed selections so the broker can retain its precedence: + // marc, opac, reservoir, marc21plus1. JSON key order has no significance. + { + name: "MARC precedes all flags", marc: [6]*string{str("x")}, + opac: &enabled, reservoir: &enabled, marc21plus1: &enabled, + want: `{"marc":{"callNumberSubField":"x"},"opac":{},"reservoir":{},"marc21plus1":{}}`, + }, + { + name: "OPAC precedes other flags", opac: &enabled, reservoir: &enabled, marc21plus1: &enabled, + want: `{"opac":{},"reservoir":{},"marc21plus1":{}}`, + }, + { + name: "reservoir precedes MARC21plus1", opac: &disabled, reservoir: &enabled, marc21plus1: &enabled, + want: `{"reservoir":{},"marc21plus1":{}}`, + }, + {name: "explicit empty MARC field", marc: [6]*string{str("")}, want: `{"marc":{"callNumberSubField":""}}`}, + } + // Each field alone must be enough to preserve MARC selection; missing fields + // must stay absent so subsequent profile/default merging remains possible. + for i, field := range []string{"callNumberSubField", "itemIdSubField", "locationSubField", "mainField", "restrictedSubField", "shelvingLocationSubField"} { + c := legacyHoldings{name: "MARC only " + field, want: fmt.Sprintf(`{"marc":{%q:"x"}}`, field)} + c.marc[i] = str("x") + cases = append(cases, c) + } + + ids := make([]string, len(cases)) + for i, c := range cases { + ids[i] = uuid.NewString() + exec("INSERT INTO entries (id, name, type) VALUES ($1, $2, 'institution')", ids[i], c.name) + exec(`INSERT INTO catalog_configs ( + entry, holdings_marc_call_number_subfield, holdings_marc_item_id_subfield, + holdings_marc_location_subfield, holdings_marc_main_field, + holdings_marc_restricted_subfield, holdings_marc_shelving_location_subfield, + holdings_opac_enabled, holdings_reservoir_enabled, holdings_marc21plus1_enabled + ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10)`, + ids[i], c.marc[0], c.marc[1], c.marc[2], c.marc[3], c.marc[4], c.marc[5], + c.opac, c.reservoir, c.marc21plus1) + } + + apply(9) + + for i, c := range cases { + t.Run(c.name, func(t *testing.T) { + var got *string + require.NoError(t, tx.QueryRow(ctx, "SELECT holdings_config::text FROM catalog_configs WHERE entry=$1", ids[i]).Scan(&got)) + if c.want == "" { + require.Nil(t, got, "unconfigured parsers must remain SQL NULL") + } else { + require.NotNil(t, got) + require.JSONEq(t, c.want, *got) + } + }) + } +} diff --git a/directory/test/host_profiles_test.go b/directory/test/host_profiles_test.go new file mode 100644 index 000000000..c96fbf9da --- /dev/null +++ b/directory/test/host_profiles_test.go @@ -0,0 +1,143 @@ +package test + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestHostProfilePersistence(t *testing.T) { + resetDb() + headers := map[string]string{"X-Okapi-Tenant": "ANINST", "X-Okapi-Permissions": `["directory.consortium.all"]`} + + res, data := jsonReq(t, http.MethodPost, "/entries", `{"name":"Profile only","type":"Institution","lmsConfig":{"vendor":"Sierra","requestItemPickupLocationEnabled":false},"catalogConfig":{"profile":"Koha","holdingsFormat":{"marc":{"callNumberSubField":"x"}}}}`, headers) + require.Equal(t, http.StatusCreated, res.StatusCode, data) + var created struct { + Id string `json:"id"` + } + require.NoError(t, json.Unmarshal([]byte(data), &created)) + path := "/entries/by-id/" + created.Id + get := func() map[string]any { + res, data := jsonReq(t, http.MethodGet, path, "", headers) + require.Equal(t, http.StatusOK, res.StatusCode, data) + var e map[string]any + require.NoError(t, json.Unmarshal([]byte(data), &e)) + return e + } + e := get() + l := e["lmsConfig"].(map[string]any) + require.Equal(t, map[string]any{"vendor": "Sierra", "requestItemPickupLocationEnabled": false}, l) + c := e["catalogConfig"].(map[string]any) + require.Equal(t, "Koha", c["profile"]) + require.Equal(t, map[string]any{"marc": map[string]any{"callNumberSubField": "x"}}, c["holdingsFormat"]) + res, data = jsonReq(t, http.MethodPatch, path, `{"lmsConfig":{"ncipNamespaceEnabled":false,"bibIdNormalization":"none"},"catalogConfig":{"profile":null,"holdingsFormat":{"marc":{"locationSubField":"y"}}}}`, headers) + require.Equal(t, http.StatusNoContent, res.StatusCode, data) + e = get() + l = e["lmsConfig"].(map[string]any) + require.Equal(t, "Sierra", l["vendor"]) + require.Equal(t, false, l["ncipNamespaceEnabled"]) + c = e["catalogConfig"].(map[string]any) + require.NotContains(t, c, "profile") + require.Equal(t, map[string]any{"marc": map[string]any{"callNumberSubField": "x", "locationSubField": "y"}}, c["holdingsFormat"]) + res, data = jsonReq(t, http.MethodPatch, path, `{"lmsConfig":{"vendor":null,"ncipNamespaceEnabled":null,"bibIdNormalization":null},"catalogConfig":{"holdingsFormat":{"opac":{"availabilityRule":"publicNote","availablePublicNotes":[]}}}}`, headers) + require.Equal(t, http.StatusNoContent, res.StatusCode, data) + e = get() + l = e["lmsConfig"].(map[string]any) + require.Equal(t, map[string]any{"requestItemPickupLocationEnabled": false}, l) + c = e["catalogConfig"].(map[string]any) + require.Equal(t, map[string]any{"opac": map[string]any{"availabilityRule": "publicNote", "availablePublicNotes": []any{}}}, c["holdingsFormat"]) +} + +func TestCatalogEndpointConflicts(t *testing.T) { + resetDb() + headers := map[string]string{"X-Okapi-Tenant": "ANINST", "X-Okapi-Permissions": `["directory.consortium.all"]`} + const both = `{"sru":{"address":"https://catalog.example/sru"},"zoom":{"address":"catalog.example:210/db"}}` + res, data := jsonReq(t, http.MethodPost, "/entries", `{"name":"Conflicting endpoints","type":"Institution","catalogConfig":`+both+`}`, headers) + require.Equal(t, http.StatusBadRequest, res.StatusCode, data) + require.Contains(t, data, "both SRU and ZOOM") + + for _, tc := range []struct { + name, initial, update, conflict string + }{ + {"SRU", `{"sru":{"address":"https://catalog.example/sru"}}`, `{"sru":{"recordSchema":"mods"}}`, `{"zoom":{"address":"catalog.example:210/db"}}`}, + {"ZOOM", `{"zoom":{"address":"catalog.example:210/db"}}`, `{"zoom":{"options":{"count":"20"}}}`, `{"sru":{"address":"https://catalog.example/sru"}}`}, + {"no endpoint", `{}`, `{"profile":"Generic"}`, both}, + } { + t.Run(tc.name, func(t *testing.T) { + res, data := jsonReq(t, http.MethodPost, "/entries", `{"name":"Endpoint validation","type":"Institution","catalogConfig":`+tc.initial+`}`, headers) + require.Equal(t, http.StatusCreated, res.StatusCode, data) + var created struct{ Id string } + require.NoError(t, json.Unmarshal([]byte(data), &created)) + path := "/entries/by-id/" + created.Id + res, data = jsonReq(t, http.MethodPatch, path, `{"catalogConfig":`+tc.update+`}`, headers) + require.Equal(t, http.StatusNoContent, res.StatusCode, data) + res, before := jsonReq(t, http.MethodGet, path, "", headers) + require.Equal(t, http.StatusOK, res.StatusCode, before) + res, data = jsonReq(t, http.MethodPatch, path, `{"name":"Rejected change","catalogConfig":`+tc.conflict+`}`, headers) + require.Equal(t, http.StatusBadRequest, res.StatusCode, data) + require.Contains(t, data, "both SRU and ZOOM") + res, after := jsonReq(t, http.MethodGet, path, "", headers) + require.Equal(t, http.StatusOK, res.StatusCode, after) + require.JSONEq(t, before, after) + }) + } +} + +func TestHoldingsParserConflicts(t *testing.T) { + resetDb() + headers := map[string]string{"X-Okapi-Tenant": "ANINST", "X-Okapi-Permissions": `["directory.consortium.all"]`} + res, data := jsonReq(t, http.MethodPost, "/entries", `{"name":"Parser validation","type":"Institution","catalogConfig":{"holdingsFormat":{"marc":{"mainField":"999"}}}}`, headers) + require.Equal(t, http.StatusCreated, res.StatusCode, data) + var created struct{ Id string } + require.NoError(t, json.Unmarshal([]byte(data), &created)) + path := "/entries/by-id/" + created.Id + res, before := jsonReq(t, http.MethodGet, path, "", headers) + require.Equal(t, http.StatusOK, res.StatusCode, before) + + parsers := []string{"marc", "opac", "reservoir", "marc21plus1"} + for i, first := range parsers { + for _, second := range parsers[i+1:] { + t.Run(first+"/"+second, func(t *testing.T) { + body := `{"name":"Rejected change","type":"Institution","catalogConfig":{"holdingsFormat":{"` + first + `":{},"` + second + `":{}}}}` + for _, method := range []string{http.MethodPost, http.MethodPatch} { + endpoint := "/entries" + if method == http.MethodPatch { + endpoint = path + } + res, data := jsonReq(t, method, endpoint, body, headers) + require.Equal(t, http.StatusBadRequest, res.StatusCode, data) + require.Contains(t, data, "at most one of marc, opac, reservoir, or marc21plus1") + } + }) + } + } + res, after := jsonReq(t, http.MethodGet, path, "", headers) + require.Equal(t, http.StatusOK, res.StatusCode, after) + require.JSONEq(t, before, after) + + // Empty patches preserve overrides; selecting a different parser replaces them. + for _, parser := range append([]string{""}, parsers...) { + holdings := `{}` + if parser != "" { + holdings = `{"` + parser + `":{}}` + } + res, data = jsonReq(t, http.MethodPost, "/entries", `{"name":"Valid parser","type":"Institution","catalogConfig":{"holdingsFormat":`+holdings+`}}`, headers) + require.Equal(t, http.StatusCreated, res.StatusCode, data) + res, data = jsonReq(t, http.MethodPatch, path, `{"catalogConfig":{"holdingsFormat":`+holdings+`}}`, headers) + require.Equal(t, http.StatusNoContent, res.StatusCode, data) + res, data = jsonReq(t, http.MethodGet, path, "", headers) + require.Equal(t, http.StatusOK, res.StatusCode, data) + var saved struct { + CatalogConfig struct{ HoldingsFormat map[string]any } + } + require.NoError(t, json.Unmarshal([]byte(data), &saved)) + require.Len(t, saved.CatalogConfig.HoldingsFormat, 1) + if parser == "" || parser == "marc" { + require.Equal(t, map[string]any{"mainField": "999"}, saved.CatalogConfig.HoldingsFormat["marc"]) + } else { + require.Contains(t, saved.CatalogConfig.HoldingsFormat, parser) + } + } +} diff --git a/directory/test/import_test.go b/directory/test/import_test.go index 7d972f41e..abfd6905b 100644 --- a/directory/test/import_test.go +++ b/directory/test/import_test.go @@ -169,6 +169,7 @@ func entryImportRecord(key map[string]any, name string, parent map[string]any, e data["addresses"] = []any{map[string]any{"type": "Default", "addressComponents": []any{map[string]any{"seq": 1, "type": "Locality", "value": "Riga"}}}} data["closures"] = []any{map[string]any{"startDate": "2026-12-24", "endDate": "2026-12-26", "reason": "Holiday"}} data["lmsConfig"] = map[string]any{ + "vendor": nil, "ncipNamespaceEnabled": nil, "bibIdNormalization": nil, "address": "https://example.test/ncip", "fromAgency": "FROM", "fromAgencyAuthentication": "credential-value", "toAgency": nil, "lookupUserEnabled": nil, "acceptItemEnabled": nil, "checkInItemEnabled": nil, "checkOutItemEnabled": nil, "itemLocation": nil, "requestItemRequestType": nil, @@ -210,3 +211,77 @@ func mustJSON(t *testing.T, value string) string { require.NoError(t, err) return string(data) } + +func TestImportHostSettingsRoundTrip(t *testing.T) { + resetImportState(t) + record := entryImportRecord(symbolObject("ISIL", "HOST"), "Host settings", nil, "Consortium") + data := record["data"].(map[string]any) + lms := data["lmsConfig"].(map[string]any) + lms["vendor"], lms["ncipNamespaceEnabled"], lms["bibIdNormalization"] = "Sierra", false, "none" + // Selecting a profile without enabling NCIP is a valid complete import. + lms["address"], lms["fromAgency"] = "", "" + catalog := map[string]any{"profile": "Koha", "metadataUpdateMode": nil, "sru": nil, "zoom": nil, "queryConfig": nil, "metadataFormat": nil} + data["catalogConfig"] = catalog + for _, tc := range []struct { + name, holdings, expected string + }{ + {"MARC predicates", `{"marc":{"mainField":"999","callNumberSubField":null,"itemIdSubField":null,"locationSubField":null,"restrictedSubField":null,"shelvingLocationSubField":null,"availability":[{"subField":"7","operator":"equals","value":"0"},{"subField":"q","operator":"absent","value":null}]},"opac":null,"reservoir":null,"marc21plus1":null}`, `{"marc":{"mainField":"999","availability":[{"subField":"7","operator":"equals","value":"0"},{"subField":"q","operator":"absent"}]}}`}, + {"empty MARC predicates", `{"marc":{"mainField":null,"callNumberSubField":null,"itemIdSubField":null,"locationSubField":null,"restrictedSubField":null,"shelvingLocationSubField":null,"availability":[]},"opac":null,"reservoir":null,"marc21plus1":null}`, `{"marc":{"availability":[]}}`}, + {"OPAC overrides", `{"marc":null,"opac":{"availabilityRule":"publicNote","availablePublicNotes":["AVAILABLE"],"requireLocalLocation":false,"shelvingLocationSource":"localLocation","includeItemId":false,"includeItemLoanPolicy":false,"includeTemporaryLocation":true,"allCirculations":true},"reservoir":null,"marc21plus1":null}`, `{"opac":{"availabilityRule":"publicNote","availablePublicNotes":["AVAILABLE"],"requireLocalLocation":false,"shelvingLocationSource":"localLocation","includeItemId":false,"includeItemLoanPolicy":false,"includeTemporaryLocation":true,"allCirculations":true}}`}, + {"empty OPAC notes", `{"marc":null,"opac":{"availabilityRule":null,"availablePublicNotes":[],"requireLocalLocation":null,"shelvingLocationSource":null,"includeItemId":null,"includeItemLoanPolicy":null,"includeTemporaryLocation":null,"allCirculations":null},"reservoir":null,"marc21plus1":null}`, `{"opac":{"availablePublicNotes":[]}}`}, + {"reservoir", `{"marc":null,"opac":null,"reservoir":{},"marc21plus1":null}`, `{"reservoir":{}}`}, + {"marc21plus1", `{"marc":null,"opac":null,"reservoir":null,"marc21plus1":{}}`, `{"marc21plus1":{}}`}, + {"profile defaults", `{"marc":null,"opac":null,"reservoir":null,"marc21plus1":null}`, `{}`}, + } { + t.Run(tc.name, func(t *testing.T) { + var holdings map[string]any + require.NoError(t, json.Unmarshal([]byte(tc.holdings), &holdings)) + catalog["holdingsFormat"] = holdings + response, result := importRequest(t, []any{record}, "update", standardHeaders) + require.Equal(t, http.StatusOK, response.StatusCode) + require.Empty(t, result.Errors) + require.Equal(t, int32(1), result.Entries.Imported) + id := importedEntryID(t, "ISIL", "HOST") + var vendor, normalization, profile string + var namespace bool + var raw []byte + require.NoError(t, dbpool.QueryRow(context.Background(), `SELECT vendor, ncip_namespace_enabled, bib_id_normalization FROM lms_configs WHERE entry=$1`, id).Scan(&vendor, &namespace, &normalization)) + require.Equal(t, "Sierra", vendor) + require.False(t, namespace) + require.Equal(t, "none", normalization) + require.NoError(t, dbpool.QueryRow(context.Background(), `SELECT profile, holdings_config FROM catalog_configs WHERE entry=$1`, id).Scan(&profile, &raw)) + require.Equal(t, "Koha", profile) + require.JSONEq(t, tc.expected, string(raw)) + res, body := jsonReq(t, http.MethodGet, "/entries/by-id/"+id.String(), "", standardHeaders) + require.Equal(t, http.StatusOK, res.StatusCode, body) + var saved struct { + LmsConfig struct { + Vendor string + NcipNamespaceEnabled *bool + BibIdNormalization string + } + CatalogConfig struct { + Profile string + HoldingsFormat json.RawMessage + } + } + require.NoError(t, json.Unmarshal([]byte(body), &saved)) + require.Equal(t, "Sierra", saved.LmsConfig.Vendor) + require.NotNil(t, saved.LmsConfig.NcipNamespaceEnabled) + require.False(t, *saved.LmsConfig.NcipNamespaceEnabled) + require.Equal(t, "none", saved.LmsConfig.BibIdNormalization) + require.Equal(t, "Koha", saved.CatalogConfig.Profile) + require.JSONEq(t, tc.expected, string(saved.CatalogConfig.HoldingsFormat)) + }) + } + lms["vendor"], lms["ncipNamespaceEnabled"], lms["bibIdNormalization"] = nil, nil, nil + catalog["profile"], catalog["holdingsFormat"] = nil, nil + _, result := importRequest(t, []any{record}, "update", standardHeaders) + require.Empty(t, result.Errors) + id := importedEntryID(t, "ISIL", "HOST") + var cleared bool + require.NoError(t, dbpool.QueryRow(context.Background(), `SELECT vendor IS NULL AND ncip_namespace_enabled IS NULL AND bib_id_normalization IS NULL FROM lms_configs WHERE entry=$1`, id).Scan(&cleared)) + require.True(t, cleared) + require.NoError(t, dbpool.QueryRow(context.Background(), `SELECT profile IS NULL AND holdings_config IS NULL FROM catalog_configs WHERE entry=$1`, id).Scan(&cleared)) + require.True(t, cleared) +}