From f15fd8a66240911013bbc7b3774fa724c5db5d6c Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Fri, 11 Sep 2026 12:40:03 +0200 Subject: [PATCH 01/20] LMS vendor profiles --- broker/catalog/adapter_metaproxy.go | 6 +- broker/catalog/catalog.go | 15 +- broker/catalog/creator_impl.go | 7 +- broker/catalog/holdings_parser_marc.go | 18 +- broker/catalog/holdings_parser_opac.go | 64 +++-- broker/catalog/profiles_test.go | 131 ++++++++++ broker/lms/lms_adapter_ncip.go | 19 +- broker/lms/lms_adapter_test.go | 4 +- broker/lms/lms_creator_impl.go | 9 +- broker/lms/lms_creator_test.go | 5 +- broker/lms/profiles_test.go | 47 ++++ broker/ncipclient/namespace_test.go | 42 +++ broker/ncipclient/ncipclient_impl.go | 76 +++++- broker/profiles/defaults.go | 81 ++++++ broker/profiles/resolve.go | 246 ++++++++++++++++++ broker/profiles/resolve_test.go | 122 +++++++++ directory/README.md | 5 + directory/RELEASE-NOTES.md | 20 ++ directory/api.yaml | 102 ++++++-- directory/api/catalog_config.go | 40 ++- directory/api/entries.go | 18 +- directory/api/host_profiles_test.go | 29 +++ directory/host-profiles.md | 153 +++++++++++ .../migrations/008_host_profiles.down.sql | 5 + directory/migrations/008_host_profiles.up.sql | 25 ++ directory/query.sql | 18 +- ...ew-lmsconfig-incomplete.patch.refetch.json | 2 - .../entry-with-lmsconfig.post.refetch.json | 15 +- directory/test/host_profiles_test.go | 51 ++++ 29 files changed, 1293 insertions(+), 82 deletions(-) create mode 100644 broker/catalog/profiles_test.go create mode 100644 broker/lms/profiles_test.go create mode 100644 broker/ncipclient/namespace_test.go create mode 100644 broker/profiles/defaults.go create mode 100644 broker/profiles/resolve.go create mode 100644 broker/profiles/resolve_test.go create mode 100644 directory/RELEASE-NOTES.md create mode 100644 directory/api/host_profiles_test.go create mode 100644 directory/host-profiles.md create mode 100644 directory/migrations/008_host_profiles.down.sql create mode 100644 directory/migrations/008_host_profiles.up.sql create mode 100644 directory/test/host_profiles_test.go 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/catalog.go b/broker/catalog/catalog.go index bf40f4146..c61f75d6f 100644 --- a/broker/catalog/catalog.go +++ b/broker/catalog/catalog.go @@ -41,13 +41,14 @@ type LookupParams struct { } type Holding struct { - Symbol string - LocalIdentifier string - Location string - ShelvingLocation string - ItemLoanPolicy string - CallNumber string - ItemId string + Symbol string + LocalIdentifier string + Location string + ShelvingLocation string + TemporaryShelvingLocation string + ItemLoanPolicy string + CallNumber string + ItemId string } type HoldingsParser interface { diff --git a/broker/catalog/creator_impl.go b/broker/catalog/creator_impl.go index 12eb41ac5..46eb3f2ff 100644 --- a/broker/catalog/creator_impl.go +++ b/broker/catalog/creator_impl.go @@ -2,6 +2,7 @@ package catalog import ( "fmt" + "github.com/indexdata/crosslink/broker/profiles" "github.com/indexdata/crosslink/broker/ill_db" dirapi "github.com/indexdata/crosslink/directory/api" @@ -56,7 +57,11 @@ func getHoldingsParser(config *dirapi.HoldingsParserConfig) (HoldingsParser, err func (c *LookupAdapterCreatorImpl) GetAdapter(peer ill_db.Peer) (LookupAdapter, error) { entry := peer.CustomData - config := entry.CatalogConfig + effective, err := profiles.Resolve(entry) + if err != nil { + return nil, err + } + 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/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..4fdd39999 100644 --- a/broker/catalog/holdings_parser_opac.go +++ b/broker/catalog/holdings_parser_opac.go @@ -3,47 +3,69 @@ 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) { + h.TemporaryShelvingLocation = circ.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..d552bb696 --- /dev/null +++ b/broker/catalog/profiles_test.go @@ -0,0 +1,131 @@ +package catalog + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "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) + require.Equal(t, "STACKS", h[0].ShelvingLocation) + if name == "FOLIO" { + require.Equal(t, "TEMP", h[0].TemporaryShelvingLocation) + } else { + require.Empty(t, h[0].TemporaryShelvingLocation) + } + } + // 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 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(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(ill_db.Peer{CustomData: entry}) + require.NoError(t, err) + require.Nil(t, a) +} diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index 987f246e7..fb6bf96bf 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,9 @@ 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) + client := ncipclient.NewNcipClient(http.DefaultClient, l.config.Address, l.config.FromAgency, toAgency, FromAgencyAuthentication) + client.(*ncipclient.NcipClientImpl).DisableNamespace = l.config.NcipNamespaceEnabled != nil && !*l.config.NcipNamespaceEnabled + l.ncipClient = client return l, nil } @@ -321,6 +332,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..4696386a0 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,12 @@ 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 + } + 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..79a4f66a9 --- /dev/null +++ b/broker/lms/profiles_test.go @@ -0,0 +1,47 @@ +package lms + +import ( + "encoding/json" + "testing" + + "github.com/indexdata/crosslink/broker/ncipclient" + 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.ncipClient.(*ncipclient.NcipClientImpl).DisableNamespace) + 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..bb3910257 --- /dev/null +++ b/broker/ncipclient/namespace_test.go @@ -0,0 +1,42 @@ +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", "", "").(*NcipClientImpl) + client.DisableNamespace = 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..3dac8c666 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 @@ -221,7 +224,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 +315,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/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..a562f95e4 --- /dev/null +++ b/broker/profiles/resolve.go @@ -0,0 +1,246 @@ +// Package profiles resolves host integrations without modifying directory records. +package profiles + +import ( + "encoding/json" + "fmt" + "log/slog" + "strings" + + dirapi "github.com/indexdata/crosslink/directory/api" +) + +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) + } +} + +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{} + // These are the existing protocol defaults; no installation-specific values. + merge(l, object{"ncipNamespaceEnabled": true, "bibIdNormalization": "none", "requestItemRequestType": "Page", "requestItemRequestScopeType": "Item", "requestItemBibIdCode": "SYSNUMBER", "requestItemPickupLocationEnabled": true, "lookupUserEnabled": true, "acceptItemEnabled": true, "checkInItemEnabled": true, "checkOutItemEnabled": true, "requestItemEnabled": true}, "lmsConfig", "Generic", e.Origins) + if vendor == "Sierra" { + merge(l, object{"requestItemRequestType": "Hold", "requestItemRequestScopeType": "Title", "bibIdNormalization": "sierra"}, "lmsConfig", vendor, e.Origins) + } + if vendor == "Koha" { + merge(l, object{"ncipNamespaceEnabled": false}, "lmsConfig", vendor, e.Origins) + } + merge(l, rawL, "lmsConfig", "directory", e.Origins) + if profile != "Generic" { + h := object{"opac": object{"availabilityRule": "availableNow", "requireLocalLocation": true, "includeItemId": true, "includeItemLoanPolicy": true, "allCirculations": true}} + if profile == "Sierra" { + h = object{"opac": object{"availabilityRule": "publicNote", "availablePublicNotes": []any{"AVAILABLE", "CHECK SHELVES", "CHECK SHELF"}, "shelvingLocationSource": "localLocation", "includeItemId": false, "includeItemLoanPolicy": false}} + } + if profile == "FOLIO" { + h["opac"].(object)["includeTemporaryLocation"] = true + } + if profile == "Koha" { + h = object{"marc": object{"mainField": "952", "locationSubField": "b", "shelvingLocationSubField": "c", "callNumberSubField": "o", "availability": []any{object{"subField": "7", "operator": "equals", "value": "0"}, object{"subField": "q", "operator": "absent"}}}} + } + merge(c, object{"holdingsFormat": h}, "catalogConfig", profile, e.Origins) + } + // Switching parser replaces the entire profile parser, including its rules. + if h, ok := rawC["holdingsFormat"].(object); ok && len(h) > 0 { + if len(h) != 1 { + return nil, fmt.Errorf("catalog profile %s: holdingsFormat must select exactly one parser", profile) + } + 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 legacy holdings object means the existing generic default. + if h, ok := rawC["holdingsFormat"].(object); ok && len(h) == 0 && profile != "Generic" { + delete(rawC, "holdingsFormat") + } + merge(c, rawC, "catalogConfig", "directory", e.Origins) + if profile != "Generic" { + syntax := "opac" + if h, ok := c["holdingsFormat"].(object); ok { + if _, ok := h["opac"]; !ok { + syntax = "xml" + } + } + if z, ok := c["zoom"].(object); ok { + 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 { + if _, ok := s["recordSchema"]; !ok { + schema := "opac" + if syntax == "xml" { + schema = "marcxml" + } + 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 + } + // Only behavior settings are logged: endpoints, options, credentials, agencies, + // patron details, and local policies are deliberately excluded. + slog.Debug("resolved host profiles", "configuration", e.Diagnostics()) + 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..5216ef83f --- /dev/null +++ b/broker/profiles/resolve_test.go @@ -0,0 +1,122 @@ +package profiles + +import ( + "bytes" + "encoding/json" + "log/slog" + "strings" + "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.Nil(t, e.Catalog.Zoom.Options) + case "Koha": + require.False(t, *e.LMS.NcipNamespaceEnabled) + require.Equal(t, "952", *e.Catalog.HoldingsFormat.Marc.MainField) + require.Equal(t, "xml", (*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 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 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":"Alma","holdingsFormat":{"marc":{},"opac":{}}}}`, + `{"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 TestDiagnosticsProtectCredentials(t *testing.T) { + var buf bytes.Buffer + old := slog.Default() + slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))) + defer slog.SetDefault(old) + _, 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) + require.False(t, strings.Contains(buf.String(), "secret-")) + require.Contains(t, buf.String(), "Sierra") +} diff --git a/directory/README.md b/directory/README.md index d3e63a0a3..f66f9ee7b 100644 --- a/directory/README.md +++ b/directory/README.md @@ -62,3 +62,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..669762627 --- /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 008 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 e72599ac9..e0c982450 100644 --- a/directory/api.yaml +++ b/directory/api.yaml @@ -1559,8 +1559,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: @@ -1577,6 +1589,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: @@ -1639,7 +1657,6 @@ components: type: type: string enum: [cql, pqf] - default: pqf title: type: string isbn: @@ -1683,6 +1700,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 } @@ -1690,8 +1712,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 @@ -1877,16 +1927,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 @@ -1899,55 +1966,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. $ref: '#/components/schemas/PatronProfiles' @@ -1979,6 +2033,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..79cbd6a14 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 @@ -80,6 +82,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 +157,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 +285,32 @@ 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) == 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 638081916..e85fd5553 100644 --- a/directory/api/entries.go +++ b/directory/api/entries.go @@ -311,7 +311,10 @@ 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, 'checkInItemEnabled', l.checkin_item_enabled, @@ -330,11 +333,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, @@ -351,7 +355,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 @@ -368,7 +372,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 @@ -776,6 +780,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, @@ -1151,6 +1158,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), diff --git a/directory/api/host_profiles_test.go b/directory/api/host_profiles_test.go new file mode 100644 index 000000000..39f10ba44 --- /dev/null +++ b/directory/api/host_profiles_test.go @@ -0,0 +1,29 @@ +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)) +} diff --git a/directory/host-profiles.md b/directory/host-profiles.md new file mode 100644 index 000000000..eaa0613f6 --- /dev/null +++ b/directory/host-profiles.md @@ -0,0 +1,153 @@ +# 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 + +| 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 | MARCXML; 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. + +OPAC profiles request ZOOM `preferredRecordSyntax: opac`; Koha requests `xml` +(MARCXML). SRU requests use `opac` or `marcxml`, respectively. Explicit record +syntax/schema settings take precedence. The metaproxy adapter uses the OPAC schema +when its ZOOM settings select OPAC. Changing the holdings parser changes the +default syntax to match; explicit syntax options remain authoritative. + +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`, `includeTemporaryLocation`: circulation mappings. +- `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 `008_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/migrations/008_host_profiles.down.sql b/directory/migrations/008_host_profiles.down.sql new file mode 100644 index 000000000..89b9c7d49 --- /dev/null +++ b/directory/migrations/008_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/008_host_profiles.up.sql b/directory/migrations/008_host_profiles.up.sql new file mode 100644 index 000000000..f9c44399a --- /dev/null +++ b/directory/migrations/008_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 cf2997e2b..5c6abe425 100644 --- a/directory/query.sql +++ b/directory/query.sql @@ -209,7 +209,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, @@ -238,9 +238,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, @@ -296,7 +300,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, @@ -317,9 +321,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/host_profiles_test.go b/directory/test/host_profiles_test.go new file mode 100644 index 000000000..cdaa53339 --- /dev/null +++ b/directory/test/host_profiles_test.go @@ -0,0 +1,51 @@ +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"]) +} From a336777e480001a2258133b6ae9428d25e282094 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Fri, 11 Sep 2026 12:45:32 +0200 Subject: [PATCH 02/20] disableNamespace private --- broker/lms/lms_adapter_ncip.go | 5 ++--- broker/lms/profiles_test.go | 3 +-- broker/ncipclient/namespace_test.go | 3 +-- broker/ncipclient/ncipclient_impl.go | 9 +++++---- broker/ncipclient/ncipclient_test.go | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index fb6bf96bf..95cc93779 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -74,9 +74,8 @@ func createResolvedLmsAdapterNcip(lmsConfig dirapi.LmsConfig) (LmsAdapter, error if l.config.FromAgency == "" { return nil, fmt.Errorf("missing From Agency in LMS configuration") } - client := ncipclient.NewNcipClient(http.DefaultClient, l.config.Address, l.config.FromAgency, toAgency, FromAgencyAuthentication) - client.(*ncipclient.NcipClientImpl).DisableNamespace = l.config.NcipNamespaceEnabled != nil && !*l.config.NcipNamespaceEnabled - l.ncipClient = client + 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 } diff --git a/broker/lms/profiles_test.go b/broker/lms/profiles_test.go index 79a4f66a9..34197379b 100644 --- a/broker/lms/profiles_test.go +++ b/broker/lms/profiles_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "testing" - "github.com/indexdata/crosslink/broker/ncipclient" dirapi "github.com/indexdata/crosslink/directory/api" "github.com/indexdata/crosslink/ncip" "github.com/stretchr/testify/require" @@ -17,7 +16,7 @@ func TestLmsProfileDefaults(t *testing.T) { adapter, err := CreateLmsAdapterNcip(cfg) require.NoError(t, err) a := adapter.(*LmsAdapterNcip) - require.Equal(t, vendor == "Koha", a.ncipClient.(*ncipclient.NcipClientImpl).DisableNamespace) + require.Equal(t, vendor != "Koha", *a.config.NcipNamespaceEnabled) if vendor == "Sierra" { require.Equal(t, "Hold", a.requestItemRequestType()) require.Equal(t, "Title", a.requestItemRequestScopeType()) diff --git a/broker/ncipclient/namespace_test.go b/broker/ncipclient/namespace_test.go index bb3910257..175e13519 100644 --- a/broker/ncipclient/namespace_test.go +++ b/broker/ncipclient/namespace_test.go @@ -34,8 +34,7 @@ func TestNamespaceFreeExchange(t *testing.T) { require.NoError(t, err) })) defer server.Close() - client := NewNcipClient(server.Client(), server.URL, "agency", "", "").(*NcipClientImpl) - client.DisableNamespace = true + 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 3dac8c666..26449e8bb 100644 --- a/broker/ncipclient/ncipclient_impl.go +++ b/broker/ncipclient/ncipclient_impl.go @@ -14,7 +14,7 @@ import ( ) type NcipClientImpl struct { - DisableNamespace bool + disableNamespace bool client *http.Client address string fromAgency string @@ -23,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, @@ -370,14 +371,14 @@ func transformNamespace(data []byte, remove bool) ([]byte, error) { func (n *NcipClientImpl) marshal(v any) ([]byte, error) { b, err := xml.Marshal(v) - if err != nil || !n.DisableNamespace { + if err != nil || !n.disableNamespace { return b, err } return transformNamespace(b, true) } func (n *NcipClientImpl) unmarshal(b []byte, v any) error { - if n.DisableNamespace { + if n.disableNamespace { var err error b, err = transformNamespace(b, false) if err != nil { 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) { From c36a383751644ba668af6fa8133dc033a77b30b9 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 11:11:45 +0200 Subject: [PATCH 03/20] nonempty FOLIO temporary locations now become Holding.ShelvingLocation --- broker/catalog/catalog.go | 15 ++++++------- broker/catalog/holdings_parser_opac.go | 5 ++++- broker/catalog/profiles_test.go | 31 +++++++++++++++++++++++--- directory/host-profiles.md | 3 ++- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/broker/catalog/catalog.go b/broker/catalog/catalog.go index c61f75d6f..bf40f4146 100644 --- a/broker/catalog/catalog.go +++ b/broker/catalog/catalog.go @@ -41,14 +41,13 @@ type LookupParams struct { } type Holding struct { - Symbol string - LocalIdentifier string - Location string - ShelvingLocation string - TemporaryShelvingLocation string - ItemLoanPolicy string - CallNumber string - ItemId string + Symbol string + LocalIdentifier string + Location string + ShelvingLocation string + ItemLoanPolicy string + CallNumber string + ItemId string } type HoldingsParser interface { diff --git a/broker/catalog/holdings_parser_opac.go b/broker/catalog/holdings_parser_opac.go index 4fdd39999..d5e5f4b79 100644 --- a/broker/catalog/holdings_parser_opac.go +++ b/broker/catalog/holdings_parser_opac.go @@ -59,7 +59,10 @@ func (p *OpacHoldingsParser) Parse(record []byte, params LookupParams) ([]Holdin h.ItemLoanPolicy = strings.TrimSpace(circ.AvailableThru) } if enabled(p.config.IncludeTemporaryLocation, false) { - h.TemporaryShelvingLocation = circ.TemporaryLocation + // 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) { diff --git a/broker/catalog/profiles_test.go b/broker/catalog/profiles_test.go index d552bb696..2506b52d3 100644 --- a/broker/catalog/profiles_test.go +++ b/broker/catalog/profiles_test.go @@ -66,12 +66,12 @@ func TestOpacProfiles(t *testing.T) { require.Len(t, h, 2) require.Equal(t, "1", h[0].ItemId) require.Equal(t, "LOAN", h[0].ItemLoanPolicy) - require.Equal(t, "STACKS", h[0].ShelvingLocation) if name == "FOLIO" { - require.Equal(t, "TEMP", h[0].TemporaryShelvingLocation) + require.Equal(t, "TEMP", h[0].ShelvingLocation) } else { - require.Empty(t, h[0].TemporaryShelvingLocation) + 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{}) @@ -79,6 +79,31 @@ func TestOpacProfiles(t *testing.T) { 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) { diff --git a/directory/host-profiles.md b/directory/host-profiles.md index eaa0613f6..b896ae453 100644 --- a/directory/host-profiles.md +++ b/directory/host-profiles.md @@ -111,7 +111,8 @@ OPAC overrides include: - `availablePublicNotes`: exact accepted strings; required for `publicNote`. - `requireLocalLocation`: require a nonempty local location. - `shelvingLocationSource`: `shelvingLocation` or `localLocation`. -- `includeItemId`, `includeItemLoanPolicy`, `includeTemporaryLocation`: circulation mappings. +- `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 From 048f05e7af9ee01a82d2daf84b5abb1346264b33 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 11:15:16 +0200 Subject: [PATCH 04/20] NULLIF(..., '') lets json_strip_nulls omit empty address and fromAgency values --- directory/api/entries.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/directory/api/entries.go b/directory/api/entries.go index e85fd5553..3113010ad 100644 --- a/directory/api/entries.go +++ b/directory/api/entries.go @@ -316,10 +316,10 @@ func buildEntrySQL(whereClause string) string { '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, From 5ba7c914fa71581b694984556114450abb49ea76 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 11:33:01 +0200 Subject: [PATCH 05/20] Profiles are yaml files --- broker/go.mod | 2 +- broker/profiles/builtin.go | 68 +++++++++++++++++++++ broker/profiles/builtin/Alma.yaml | 12 ++++ broker/profiles/builtin/FOLIO.yaml | 13 ++++ broker/profiles/builtin/Generic.yaml | 16 +++++ broker/profiles/builtin/Koha.yaml | 18 ++++++ broker/profiles/builtin/Sierra.yaml | 18 ++++++ broker/profiles/builtin/profile.schema.json | 17 ++++++ broker/profiles/builtin_test.go | 31 ++++++++++ broker/profiles/resolve.go | 24 ++------ directory/host-profiles.md | 13 ++++ 11 files changed, 211 insertions(+), 21 deletions(-) create mode 100644 broker/profiles/builtin.go create mode 100644 broker/profiles/builtin/Alma.yaml create mode 100644 broker/profiles/builtin/FOLIO.yaml create mode 100644 broker/profiles/builtin/Generic.yaml create mode 100644 broker/profiles/builtin/Koha.yaml create mode 100644 broker/profiles/builtin/Sierra.yaml create mode 100644 broker/profiles/builtin/profile.schema.json create mode 100644 broker/profiles/builtin_test.go 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/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/resolve.go b/broker/profiles/resolve.go index a562f95e4..a26ec12cc 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -80,28 +80,12 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { } e := &Effective{LMSVendor: vendor, CatalogProfile: profile, Origins: map[string]string{}} l, c := object{}, object{} - // These are the existing protocol defaults; no installation-specific values. - merge(l, object{"ncipNamespaceEnabled": true, "bibIdNormalization": "none", "requestItemRequestType": "Page", "requestItemRequestScopeType": "Item", "requestItemBibIdCode": "SYSNUMBER", "requestItemPickupLocationEnabled": true, "lookupUserEnabled": true, "acceptItemEnabled": true, "checkInItemEnabled": true, "checkOutItemEnabled": true, "requestItemEnabled": true}, "lmsConfig", "Generic", e.Origins) - if vendor == "Sierra" { - merge(l, object{"requestItemRequestType": "Hold", "requestItemRequestScopeType": "Title", "bibIdNormalization": "sierra"}, "lmsConfig", vendor, e.Origins) - } - if vendor == "Koha" { - merge(l, object{"ncipNamespaceEnabled": false}, "lmsConfig", vendor, e.Origins) + 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) - if profile != "Generic" { - h := object{"opac": object{"availabilityRule": "availableNow", "requireLocalLocation": true, "includeItemId": true, "includeItemLoanPolicy": true, "allCirculations": true}} - if profile == "Sierra" { - h = object{"opac": object{"availabilityRule": "publicNote", "availablePublicNotes": []any{"AVAILABLE", "CHECK SHELVES", "CHECK SHELF"}, "shelvingLocationSource": "localLocation", "includeItemId": false, "includeItemLoanPolicy": false}} - } - if profile == "FOLIO" { - h["opac"].(object)["includeTemporaryLocation"] = true - } - if profile == "Koha" { - h = object{"marc": object{"mainField": "952", "locationSubField": "b", "shelvingLocationSubField": "c", "callNumberSubField": "o", "availability": []any{object{"subField": "7", "operator": "equals", "value": "0"}, object{"subField": "q", "operator": "absent"}}}} - } - merge(c, object{"holdingsFormat": h}, "catalogConfig", profile, 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 { if len(h) != 1 { diff --git a/directory/host-profiles.md b/directory/host-profiles.md index b896ae453..f8677fa11 100644 --- a/directory/host-profiles.md +++ b/directory/host-profiles.md @@ -37,6 +37,19 @@ 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 | From bef0608f25353a0c790abfabbe20dbcda4e299bc Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 11:42:00 +0200 Subject: [PATCH 06/20] docker: include builtin profiles --- broker/Dockerfile.dockerignore | 3 +++ 1 file changed, 3 insertions(+) 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 From 28d42acca7bda27e2360d76d17430d3fbc0f2f40 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 12:40:42 +0200 Subject: [PATCH 07/20] Preserve legacy precedence for parsers --- broker/profiles/resolve.go | 13 +++++++++---- broker/profiles/resolve_test.go | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/broker/profiles/resolve.go b/broker/profiles/resolve.go index a26ec12cc..a39ba8bdc 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -88,10 +88,14 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { 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 { - if len(h) != 1 { - return nil, fmt.Errorf("catalog profile %s: holdingsFormat must select exactly one parser", profile) - } - for parser := range h { + // Directory records may contain multiple parsers. Preserve legacy precedence + // and discard unused parsers before merging defaults or validating settings. + for _, parser := range []string{"marc", "opac", "reservoir", "marc21plus1"} { + config, present := h[parser] + if !present || config == nil { + continue + } + rawC["holdingsFormat"] = object{parser: config} if defaults, ok := c["holdingsFormat"].(object); ok { if _, same := defaults[parser]; !same { delete(c, "holdingsFormat") @@ -102,6 +106,7 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { } } } + break } } // An empty legacy holdings object means the existing generic default. diff --git a/broker/profiles/resolve_test.go b/broker/profiles/resolve_test.go index 5216ef83f..0844f3eab 100644 --- a/broker/profiles/resolve_test.go +++ b/broker/profiles/resolve_test.go @@ -96,12 +96,43 @@ func TestFallbackAndProfileOnly(t *testing.T) { require.Nil(t, e.LMS) require.Nil(t, e.Catalog.Zoom) } +func TestLegacyParserPrecedence(t *testing.T) { + for _, profile := range []string{"Generic", "Koha", "Alma"} { + for _, tc := range []struct { + name, holdings, parser, syntax string + }{ + {"marc first", `"marc":{},"opac":{"availabilityRule":"bad"},"reservoir":{},"marc21plus1":{}`, "marc", "xml"}, + {"opac second", `"opac":{},"reservoir":{},"marc21plus1":{}`, "opac", "opac"}, + {"reservoir third", `"reservoir":{},"marc21plus1":{}`, "reservoir", "xml"}, + {"marc21plus1 last", `"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) + if profile != "Generic" { + 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 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":"Alma","holdingsFormat":{"marc":{},"opac":{}}}}`, `{"catalogConfig":{"profile":"Koha","holdingsFormat":{"marc":{"availability":[{"operator":"equals","subField":"7"}]}}}}`, `{"catalogConfig":{"profile":"Alma","holdingsFormat":{"opac":{"availabilityRule":"bad"}}}}`, } { From 6b9c3d4757e732e2cfadee6303a8be2e7900f104 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 13:15:24 +0200 Subject: [PATCH 08/20] Resolve: fix syntax defaults --- broker/profiles/resolve.go | 47 ++++++++++++++++++--------------- broker/profiles/resolve_test.go | 38 +++++++++++++++++++++----- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/broker/profiles/resolve.go b/broker/profiles/resolve.go index a39ba8bdc..191b6d04a 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -114,34 +114,39 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { delete(rawC, "holdingsFormat") } merge(c, rawC, "catalogConfig", "directory", e.Origins) - if profile != "Generic" { - syntax := "opac" + 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 { + if _, ok := h["opac"]; ok { + syntax = "opac" + } else if _, ok := h["marc21plus1"]; ok { syntax = "xml" } } - if z, ok := c["zoom"].(object); ok { - 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 - } + options, ok := z["options"].(object) + if !ok { + options = object{} + z["options"] = options } - if s, ok := c["sru"].(object); ok { - if _, ok := s["recordSchema"]; !ok { - schema := "opac" - if syntax == "xml" { - schema = "marcxml" - } - s["recordSchema"] = schema - e.Origins["catalogConfig.sru.recordSchema"] = profile + 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) diff --git a/broker/profiles/resolve_test.go b/broker/profiles/resolve_test.go index 0844f3eab..18ca8bb4a 100644 --- a/broker/profiles/resolve_test.go +++ b/broker/profiles/resolve_test.go @@ -32,11 +32,11 @@ func TestProfiles(t *testing.T) { switch name { case "Generic": require.Equal(t, "852", *e.Catalog.HoldingsFormat.Marc.MainField) - require.Nil(t, e.Catalog.Zoom.Options) + 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, "xml", (*e.Catalog.Zoom.Options)["preferredRecordSyntax"]) + 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) @@ -101,9 +101,9 @@ func TestLegacyParserPrecedence(t *testing.T) { for _, tc := range []struct { name, holdings, parser, syntax string }{ - {"marc first", `"marc":{},"opac":{"availabilityRule":"bad"},"reservoir":{},"marc21plus1":{}`, "marc", "xml"}, + {"marc first", `"marc":{},"opac":{"availabilityRule":"bad"},"reservoir":{},"marc21plus1":{}`, "marc", "usmarc"}, {"opac second", `"opac":{},"reservoir":{},"marc21plus1":{}`, "opac", "opac"}, - {"reservoir third", `"reservoir":{},"marc21plus1":{}`, "reservoir", "xml"}, + {"reservoir third", `"reservoir":{},"marc21plus1":{}`, "reservoir", "usmarc"}, {"marc21plus1 last", `"marc21plus1":{}`, "marc21plus1", "xml"}, } { t.Run(profile+"/"+tc.name, func(t *testing.T) { @@ -115,9 +115,7 @@ func TestLegacyParserPrecedence(t *testing.T) { h := asObject(e.Catalog.HoldingsFormat) require.Len(t, h, 1) require.Contains(t, h, tc.parser) - if profile != "Generic" { - require.Equal(t, tc.syntax, (*e.Catalog.Zoom.Options)["preferredRecordSyntax"]) - } + 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) } @@ -128,6 +126,32 @@ func TestLegacyParserPrecedence(t *testing.T) { } } } +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"}}`, From 954dc2bfb8e0a356d6263dcd9e5088ad583ea206 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 13:21:49 +0200 Subject: [PATCH 09/20] GoDoc --- broker/profiles/resolve.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/broker/profiles/resolve.go b/broker/profiles/resolve.go index 191b6d04a..dd41481ed 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -10,6 +10,10 @@ import ( 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 @@ -68,6 +72,8 @@ func supported(name, setting string) error { } } +// 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") From 9b880732ed86b77275d0cc12cb7b6856649b69ca Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 13:34:53 +0200 Subject: [PATCH 10/20] Fix unstable holdings_test --- broker/test/catalog/holdings_test.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) 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) } From d9d27d152e81c46034771d17155df9d8ecbcd618 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 13:51:21 +0200 Subject: [PATCH 11/20] Add host profiles migration test --- .../test/host_profiles_migration_test.go | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 directory/test/host_profiles_migration_test.go diff --git a/directory/test/host_profiles_migration_test.go b/directory/test/host_profiles_migration_test.go new file mode 100644 index 000000000..31cdcd967 --- /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-008 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 <= 7; 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(8) + + 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) + } + }) + } +} From 7f6ccb3ffb2f5df67ef7826a9d70f811ea27656c Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 14:21:10 +0200 Subject: [PATCH 12/20] koha is Z39.50 + usmarc by default --- directory/host-profiles.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/directory/host-profiles.md b/directory/host-profiles.md index f8677fa11..ef4560bb9 100644 --- a/directory/host-profiles.md +++ b/directory/host-profiles.md @@ -55,7 +55,7 @@ records. WMS and Aleph remain unsupported and have no built-in files. | 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 | MARCXML; 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 | +| 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 @@ -67,11 +67,20 @@ mappings (`rec.id`, `isbn`, `issn`, `title`). Each query template can be overrid an empty template disables that field. Metadata uses the existing MARC mappings and can extract the bibliographic MARC record embedded in OPAC. -OPAC profiles request ZOOM `preferredRecordSyntax: opac`; Koha requests `xml` -(MARCXML). SRU requests use `opac` or `marcxml`, respectively. Explicit record -syntax/schema settings take precedence. The metaproxy adapter uses the OPAC schema -when its ZOOM settings select OPAC. Changing the holdings parser changes the -default syntax to match; explicit syntax options remain authoritative. +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 From f207d8ca008c2dc939f6748e43341fcbc57db84a Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 15 Sep 2026 14:29:22 +0200 Subject: [PATCH 13/20] catalog adapter can use contextual logger --- broker/catalog/creator.go | 3 ++- broker/catalog/creator_impl.go | 7 +++++-- broker/catalog/creator_test.go | 22 +++++++++++--------- broker/catalog/gvi_holdings_test.go | 4 +++- broker/catalog/profiles_test.go | 6 ++++-- broker/lms/lms_creator_impl.go | 2 ++ broker/patron_request/service/action_test.go | 2 +- broker/profiles/resolve.go | 4 ---- broker/profiles/resolve_test.go | 15 +++++-------- broker/service/lookupadapterfactory.go | 4 ++-- 10 files changed, 36 insertions(+), 33 deletions(-) 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 46eb3f2ff..8b8d282ce 100644 --- a/broker/catalog/creator_impl.go +++ b/broker/catalog/creator_impl.go @@ -2,9 +2,10 @@ package catalog import ( "fmt" - "github.com/indexdata/crosslink/broker/profiles" + "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" ) @@ -55,12 +56,14 @@ 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 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. diff --git a/broker/catalog/creator_test.go b/broker/catalog/creator_test.go index 9734069c9..21c045f33 100644 --- a/broker/catalog/creator_test.go +++ b/broker/catalog/creator_test.go @@ -1,8 +1,10 @@ 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" @@ -11,7 +13,7 @@ import ( 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 +21,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) } @@ -67,7 +69,7 @@ func TestGetAdapterBadParser(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(), "must set marc") } @@ -83,7 +85,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 +98,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 +114,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 +130,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 +152,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 +168,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 +184,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/profiles_test.go b/broker/catalog/profiles_test.go index 2506b52d3..71281424f 100644 --- a/broker/catalog/profiles_test.go +++ b/broker/catalog/profiles_test.go @@ -1,6 +1,7 @@ package catalog import ( + "context" "encoding/json" "fmt" "net/http" @@ -8,6 +9,7 @@ import ( "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" @@ -136,7 +138,7 @@ func TestProfileLookupAggregationAndFallback(t *testing.T) { 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(ill_db.Peer{CustomData: 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) @@ -150,7 +152,7 @@ func TestProfileLookupAggregationAndFallback(t *testing.T) { 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(ill_db.Peer{CustomData: 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/lms/lms_creator_impl.go b/broker/lms/lms_creator_impl.go index 4696386a0..844f701a4 100644 --- a/broker/lms/lms_creator_impl.go +++ b/broker/lms/lms_creator_impl.go @@ -30,6 +30,8 @@ func (l *lmsCreatorImpl) GetAdapter(ctx common.ExtendedContext, symbol string) ( 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) } 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/resolve.go b/broker/profiles/resolve.go index dd41481ed..fba3504c8 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -4,7 +4,6 @@ package profiles import ( "encoding/json" "fmt" - "log/slog" "strings" dirapi "github.com/indexdata/crosslink/directory/api" @@ -171,9 +170,6 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { if err := e.validate(); err != nil { return nil, err } - // Only behavior settings are logged: endpoints, options, credentials, agencies, - // patron details, and local policies are deliberately excluded. - slog.Debug("resolved host profiles", "configuration", e.Diagnostics()) return e, nil } diff --git a/broker/profiles/resolve_test.go b/broker/profiles/resolve_test.go index 18ca8bb4a..8d7707eaa 100644 --- a/broker/profiles/resolve_test.go +++ b/broker/profiles/resolve_test.go @@ -1,10 +1,7 @@ package profiles import ( - "bytes" "encoding/json" - "log/slog" - "strings" "testing" dirapi "github.com/indexdata/crosslink/directory/api" @@ -166,12 +163,10 @@ func TestValidation(t *testing.T) { } } func TestDiagnosticsProtectCredentials(t *testing.T) { - var buf bytes.Buffer - old := slog.Default() - slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))) - defer slog.SetDefault(old) - _, 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"}}}}`)) + 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) - require.False(t, strings.Contains(buf.String(), "secret-")) - require.Contains(t, buf.String(), "Sierra") + 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) } From f06ed42b2bdd218c4dd5f48d06b16272464ff242 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Wed, 16 Sep 2026 14:46:13 +0200 Subject: [PATCH 14/20] PATCH containing holdingsFormat: {} preserves stored info --- directory/api/catalog_config.go | 3 +++ directory/api/host_profiles_test.go | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/directory/api/catalog_config.go b/directory/api/catalog_config.go index 79cbd6a14..6c42fe771 100644 --- a/directory/api/catalog_config.go +++ b/directory/api/catalog_config.go @@ -302,6 +302,9 @@ func mergeHoldingsConfig(original []byte, patch *HoldingsParserConfig) ([]byte, 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 { diff --git a/directory/api/host_profiles_test.go b/directory/api/host_profiles_test.go index 39f10ba44..5cbc83bec 100644 --- a/directory/api/host_profiles_test.go +++ b/directory/api/host_profiles_test.go @@ -27,3 +27,25 @@ func TestPersistRawProfileOverrides(t *testing.T) { 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) + }) + } +} From cdfe6b39d493bb0b1fa81b7ff83f2e6a6f0334db Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Wed, 16 Sep 2026 14:52:26 +0200 Subject: [PATCH 15/20] Validate: sru or zoom. HoldingsParserConfig only one --- broker/profiles/resolve_test.go | 5 ++ directory/api/catalog_config.go | 22 +++++++ directory/api/entries.go | 9 ++- directory/api/host_profiles_test.go | 26 ++++++++ directory/test/entries_test.go | 9 +-- directory/test/host_profiles_test.go | 92 ++++++++++++++++++++++++++++ 6 files changed, 155 insertions(+), 8 deletions(-) diff --git a/broker/profiles/resolve_test.go b/broker/profiles/resolve_test.go index 8d7707eaa..a55528f2d 100644 --- a/broker/profiles/resolve_test.go +++ b/broker/profiles/resolve_test.go @@ -162,6 +162,11 @@ func TestValidation(t *testing.T) { 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) diff --git a/directory/api/catalog_config.go b/directory/api/catalog_config.go index 6c42fe771..fdbbecae8 100644 --- a/directory/api/catalog_config.go +++ b/directory/api/catalog_config.go @@ -69,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") diff --git a/directory/api/entries.go b/directory/api/entries.go index 969d22807..4f8a47cfa 100644 --- a/directory/api/entries.go +++ b/directory/api/entries.go @@ -797,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 @@ -1197,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 index 5cbc83bec..98b1c3bb8 100644 --- a/directory/api/host_profiles_test.go +++ b/directory/api/host_profiles_test.go @@ -49,3 +49,29 @@ func TestEmptyHoldingsFormatPatchPreservesConfig(t *testing.T) { }) } } + +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/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_test.go b/directory/test/host_profiles_test.go index cdaa53339..c96fbf9da 100644 --- a/directory/test/host_profiles_test.go +++ b/directory/test/host_profiles_test.go @@ -49,3 +49,95 @@ func TestHostProfilePersistence(t *testing.T) { 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) + } + } +} From 635915a9b25cce0ab90c9e6a64a076421a7db01c Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Wed, 16 Sep 2026 15:06:05 +0200 Subject: [PATCH 16/20] Only one place to check for holdingsFormat props --- broker/profiles/resolve.go | 10 +--------- broker/profiles/resolve_test.go | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/broker/profiles/resolve.go b/broker/profiles/resolve.go index fba3504c8..c0d6e1aaa 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -93,14 +93,7 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { 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 { - // Directory records may contain multiple parsers. Preserve legacy precedence - // and discard unused parsers before merging defaults or validating settings. - for _, parser := range []string{"marc", "opac", "reservoir", "marc21plus1"} { - config, present := h[parser] - if !present || config == nil { - continue - } - rawC["holdingsFormat"] = object{parser: config} + for parser := range h { if defaults, ok := c["holdingsFormat"].(object); ok { if _, same := defaults[parser]; !same { delete(c, "holdingsFormat") @@ -111,7 +104,6 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { } } } - break } } // An empty legacy holdings object means the existing generic default. diff --git a/broker/profiles/resolve_test.go b/broker/profiles/resolve_test.go index a55528f2d..c9b07b2ee 100644 --- a/broker/profiles/resolve_test.go +++ b/broker/profiles/resolve_test.go @@ -93,15 +93,15 @@ func TestFallbackAndProfileOnly(t *testing.T) { require.Nil(t, e.LMS) require.Nil(t, e.Catalog.Zoom) } -func TestLegacyParserPrecedence(t *testing.T) { +func TestZoomParserSyntax(t *testing.T) { for _, profile := range []string{"Generic", "Koha", "Alma"} { for _, tc := range []struct { name, holdings, parser, syntax string }{ - {"marc first", `"marc":{},"opac":{"availabilityRule":"bad"},"reservoir":{},"marc21plus1":{}`, "marc", "usmarc"}, - {"opac second", `"opac":{},"reservoir":{},"marc21plus1":{}`, "opac", "opac"}, - {"reservoir third", `"reservoir":{},"marc21plus1":{}`, "reservoir", "usmarc"}, - {"marc21plus1 last", `"marc21plus1":{}`, "marc21plus1", "xml"}, + {"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+`}}}`) @@ -123,6 +123,26 @@ func TestLegacyParserPrecedence(t *testing.T) { } } } +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 }{ From fbe6e6eab47f77b8c582ad6d029786f6ecdfeaf9 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Wed, 16 Sep 2026 15:24:24 +0200 Subject: [PATCH 17/20] migration moved to 009 --- directory/RELEASE-NOTES.md | 2 +- directory/host-profiles.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/directory/RELEASE-NOTES.md b/directory/RELEASE-NOTES.md index 669762627..3fb822f09 100644 --- a/directory/RELEASE-NOTES.md +++ b/directory/RELEASE-NOTES.md @@ -4,7 +4,7 @@ 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 008 preserves existing holdings configuration and adds nullable profile +- 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 diff --git a/directory/host-profiles.md b/directory/host-profiles.md index ef4560bb9..c2645d803 100644 --- a/directory/host-profiles.md +++ b/directory/host-profiles.md @@ -158,7 +158,7 @@ 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 `008_host_profiles` adds nullable profile/protocol columns and a JSON +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 From b854ae2af843e79ae65e11385d6b4ea691252178 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Wed, 16 Sep 2026 15:28:52 +0200 Subject: [PATCH 18/20] empty holdingsFormat now inherits defaults for Generic too --- broker/profiles/resolve.go | 5 +++-- broker/profiles/resolve_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/broker/profiles/resolve.go b/broker/profiles/resolve.go index c0d6e1aaa..44c2387ff 100644 --- a/broker/profiles/resolve.go +++ b/broker/profiles/resolve.go @@ -106,8 +106,9 @@ func Resolve(entry dirapi.Entry) (*Effective, error) { } } } - // An empty legacy holdings object means the existing generic default. - if h, ok := rawC["holdingsFormat"].(object); ok && len(h) == 0 && profile != "Generic" { + // 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) diff --git a/broker/profiles/resolve_test.go b/broker/profiles/resolve_test.go index c9b07b2ee..2533760a3 100644 --- a/broker/profiles/resolve_test.go +++ b/broker/profiles/resolve_test.go @@ -46,6 +46,32 @@ func TestProfiles(t *testing.T) { }) } } +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) From 32f98a82e3efc05f55e4760040dd19c6b173cc00 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Wed, 16 Sep 2026 15:38:38 +0200 Subject: [PATCH 19/20] Update import for the new profiles --- directory/README.md | 8 +++ directory/api.yaml | 48 +++++++++++-- directory/api/import_contract_test.go | 24 +++++++ directory/import/db/entry.go | 8 ++- directory/import/db/repo_test.go | 1 - directory/import/model/config.go | 86 ++++++++++++++++++++--- directory/import/model/models.go | 2 +- directory/import/service/importer_test.go | 71 ++++++++++++++++++- directory/test/import_test.go | 75 ++++++++++++++++++++ 9 files changed, 302 insertions(+), 21 deletions(-) diff --git a/directory/README.md b/directory/README.md index 5ea88c28e..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: diff --git a/directory/api.yaml b/directory/api.yaml index 0340f21fe..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 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/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/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) +} From c975b48c33f78119c668078aa48caf7f4011bef2 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Wed, 16 Sep 2026 15:42:52 +0200 Subject: [PATCH 20/20] empty holdings now uses MARC defaults --- broker/catalog/creator_test.go | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/broker/catalog/creator_test.go b/broker/catalog/creator_test.go index 21c045f33..9b240ce42 100644 --- a/broker/catalog/creator_test.go +++ b/broker/catalog/creator_test.go @@ -8,6 +8,7 @@ import ( "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) { @@ -65,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(common.CreateExtCtxWithArgs(context.Background(), nil), 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) {