ILLDEV-423 by adamdickmeiss · Pull Request #653 · indexdata/crosslink

adamdickmeiss · 2026-07-03T15:18:16Z

https://index-data.atlassian.net/browse/ILLDEV-423

metadata parser and lookup method

To distinguish it from MetadataLookup

Copilot

Pull request overview

This PR introduces configurable metadata enrichment for ISO18626 bibliographic data by extending holdings lookup to optionally fetch/parse metadata (not just holdings) and applying it to outgoing requests/transactions based on directory configuration. It also refactors adapter selection (including consortium config resolution) behind a new factory.

Changes:

Extend Directory API holdings configuration with metadataUpdateMode and metadataFormat (MARC21 parser config).
Split holdings adapter responsibilities into HoldingsLookup vs MetadataLookup, add MARC metadata parsing, and add request metadata update helpers.
Refactor supplier locating and patron-request creation paths to use LookupAdapterFactory and apply metadata updates when configured.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
directory/directory_api.yaml	Adds metadata update mode + metadata parser config schemas to holdings config.
broker/service/supplierlocator.go	Uses `LookupAdapterFactory`, derives lookup params centrally, and applies optional metadata enrichment before holdings lookup.
broker/service/supplierlocator_test.go	Updates constructor usage and adds tests around metadata-update behavior and persistence.
broker/service/lookupadapterfactory.go	New: centralizes adapter selection + consortium config resolution and exposes config entry access.
broker/patron_request/api/api-handler.go	Adds optional metadata enrichment during patron request creation using lookup adapters and directory config.
broker/patron_request/api/api-handler_test.go	Adds unit tests for `metadataUpdate` behavior and error paths.
broker/holdings/sru_holdings_test.go	Updates SRU adapter construction and switches tests to `HoldingsLookup`.
broker/holdings/metadata.go	New: applies metadata to ISO18626 bibliographic info with replace/merge semantics.
broker/holdings/metadata_parser_marc.go	New: MARCXML (and OPAC-wrapped MARCXML) metadata parser.
broker/holdings/metadata_parser_marc_test.go	New: tests for MARC metadata parsing defaults, overrides, and error handling.
broker/holdings/holdings.go	Expands adapter interface to support `HoldingsLookup` + `MetadataLookup`, and adds metadata types/parser interface.
broker/holdings/gvi_holdings_test.go	Updates to `HoldingsLookup`.
broker/holdings/creator_test.go	Renames parser helper usage to `getHoldingsParser`.
broker/holdings/creator_impl.go	Wires metadata parser config into adapter creation and adds shared lookup-param derivation helper.
broker/holdings/create_holdings.go	Updates SRU adapter construction for new metadata-parser parameter.
broker/holdings/adapter_zoom.go	Renames lookup method and adds Z39.50-backed metadata lookup implementation.
broker/holdings/adapter_zoom_test.go	Updates to `HoldingsLookup` and adds coverage for `MetadataLookup`.
broker/holdings/adapter_sru.go	Renames lookup method and adds `MetadataLookup` stub (currently returns empty metadata).
broker/holdings/adapter_mock.go	Updates mock adapter to satisfy the new interface, including metadata return.
broker/holdings/adapter_mock_shared.go	Updates shared mock to satisfy the new interface, including trivial metadata return.
broker/holdings/adapter_metaproxy.go	Updates wrapper adapter to forward both holdings and metadata lookup calls.
broker/handler/iso18626-handler.go	Updates retry flow to derive holdings params via new helper.
broker/app/app.go	Wires `LookupAdapterFactory` into supplier locator and patron request API handler.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

adamdickmeiss · 2026-07-09T10:28:08Z

 	var queryParams string
+	var found bool
 	for _, cql := range cqlList {
 		sruQuery := "query=" + url.QueryEscape(cql)
-		holdings, queryParams, err = s.search(sruUrl, params, sruQuery)
-		if err != nil {
-			return nil, queryParams, err
-		}
-		if len(holdings) > 0 {
-			return holdings, queryParams, nil
+		found, queryParams, err = s.search(sruUrl, params, sruQuery, processRecord)


@jakub-id this is existing behavior for the sru adapter. Want me to just return the cql , which would be easier to read rather than path with URI encoded chars.

jakub-id · 2026-07-09T09:47:38Z

@adamdickmeiss Question regarding the global vs per-peer/directory adapter: I see that they use a different search strategy, the global one creates a single OR query for different params and the per-peer one tries multiple queries one by one. Was this an intended difference in behavior? This is not introduced in this PR but this PR changes the previous behavior and stops trying queries when metadata is successfully parsed (previously it required holdings to be parsed only).

adamdickmeiss · 2026-07-09T10:25:09Z

@adamdickmeiss Question regarding the global vs per-peer/directory adapter: I see that they use a different search strategy, the global one creates a single OR query for different params and the per-peer one tries multiple queries one by one. Was this an intended difference in behavior? This is not introduced in this PR but this PR changes the previous behavior and stops trying queries when metadata is successfully parsed (previously it required holdings to be parsed only).

The adapters (sru, zoom) fetches all records to be conservative. If a search returns > 0 records, it will not try further queries.
Should I change it so it stops if any holdings comes back instead to preserve existing behavior?

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

+func (s *SruHoldingsLookupAdapter) lookupServer(sruUrl string, params LookupParams, processRecord func([]byte) (bool, error)) (bool, string, error) {
 	cqlList, pqfList, err := s.queryBuilder.Build(params)
 	if err != nil {
-		return nil, "", err
+		return false, "", err
 	}


Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

 	for _, cql := range cqlList {
 		sruQuery := "query=" + url.QueryEscape(cql)
-		holdings, queryParams, err = s.search(sruUrl, params, sruQuery)
-		if err != nil {
-			return nil, queryParams, err
-		}
-		if len(holdings) > 0 {
-			return holdings, queryParams, nil
+		found, queryParams, err = s.search(sruUrl, params, sruQuery, processRecord)
+		if err != nil || found {
+			return found, queryParams, err


+	lookupParams := holdings.LookupParamsFromBibliographicInfo(illRequest.BibliographicInfo, illRequest.ServiceInfo)
+
+	lookupResult, err := lookupAdapter.Lookup(lookupParams)
+	if err != nil {
+		return fmt.Errorf("failed to perform lookup for patron request: %w", err)
+	}
+	metadata, err := lookupResult.GetMetadata()
+	if err != nil {
+		return fmt.Errorf("failed to get metadata for patron request: %w", err)
+	}
+	return holdings.MetadataRequestUpdate(&illRequest.BibliographicInfo, metadata, lookupParams, mode)
+}


adamdickmeiss added 10 commits July 3, 2026 11:49

ILLDEV-423 metadata lookup

704dcb0

metadata parser and lookup method

Lookup -> HoldingsLookup

59c468f

To distinguish it from MetadataLookup

Add test

05588b3

metadata parsing handles opac; testing

eb2831c

Metadata: add subtitle, author, edition

3effdc9

Improve matching

3825f4a

LookupAdapterFactory

2bf4828

supplierlocator uses LookupAdapterFactory

6488e7e

Test metadata lookup for post patron requests

416fe34

Test supplierlocator with with metadata lookup

15e723d

Copilot AI review requested due to automatic review settings July 3, 2026 15:18

adamdickmeiss requested review from JanisSaldabols and jakub-id as code owners July 3, 2026 15:18

Copilot started reviewing on behalf of adamdickmeiss July 3, 2026 15:18 View session

adamdickmeiss mentioned this pull request Jul 3, 2026

ILLDEV-423 metadata lookup #648

Closed

Copilot AI reviewed Jul 3, 2026

View reviewed changes

adamdickmeiss and others added 2 commits July 3, 2026 17:29

Fix check for auto

fa50974

Potential fix for pull request finding

30f8406

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 3, 2026 15:31

Copilot started reviewing on behalf of adamdickmeiss July 3, 2026 15:31 View session

adamdickmeiss and others added 3 commits July 3, 2026 17:31

Potential fix for pull request finding

e3a043a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

70d79eb

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

b7846c8

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 3, 2026 15:36

Copilot started reviewing on behalf of adamdickmeiss July 3, 2026 15:36 View session

adamdickmeiss added 2 commits July 3, 2026 17:37

Fix up test

fbab30a

Comment

b05f57c

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Comment thread broker/patron_request/api/api-handler.go

Comment thread broker/holdings/adapter_sru.go Outdated

Comment thread broker/holdings/metadata.go Outdated

Copilot AI review requested due to automatic review settings July 3, 2026 15:41

Let Lookup check if query is empty

3a2dc2c

Copilot AI review requested due to automatic review settings July 6, 2026 11:15

Copilot started reviewing on behalf of adamdickmeiss July 6, 2026 11:15 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Comment thread broker/holdings/creator_impl.go

Comment thread broker/service/lookupadapterfactory.go Outdated

adamdickmeiss added 2 commits July 6, 2026 13:39

CP

5ec95a7

GetAdapterRequester

83bfac2

adamdickmeiss requested a review from Copilot July 6, 2026 13:05

Copilot started reviewing on behalf of adamdickmeiss July 6, 2026 13:06 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Comment thread broker/service/supplierlocator.go

adamdickmeiss added 2 commits July 6, 2026 15:19

CP

6a46e9d

Merge branch 'main' into ILLDEV-423-redo

94677bc

Copilot AI review requested due to automatic review settings July 8, 2026 14:04

Copilot started reviewing on behalf of adamdickmeiss July 8, 2026 14:04 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into ILLDEV-423-redo

492a1c2

Copilot AI review requested due to automatic review settings July 9, 2026 10:26

Copilot started reviewing on behalf of adamdickmeiss July 9, 2026 10:27 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread broker/holdings/adapter_sru.go

Comment thread broker/holdings/query_builder_general.go

SRU/ZOOM check holdings when iterating over result set

56c1774

Copilot AI review requested due to automatic review settings July 9, 2026 11:54

Copilot started reviewing on behalf of adamdickmeiss July 9, 2026 11:55 View session

GetHoldings error

dd55815

Copilot AI reviewed Jul 9, 2026

View reviewed changes

More tests

a824dc5

Copilot AI review requested due to automatic review settings July 9, 2026 12:00

Copilot started reviewing on behalf of adamdickmeiss July 9, 2026 12:00 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

jakub-id approved these changes Jul 10, 2026

View reviewed changes

Uh oh!

Conversation

adamdickmeiss commented Jul 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

adamdickmeiss Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

jakub-id commented Jul 9, 2026

Uh oh!

adamdickmeiss commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants