Skip to content

ILLDEV-423#653

Merged
adamdickmeiss merged 62 commits into
mainfrom
ILLDEV-423-redo
Jul 13, 2026
Merged

ILLDEV-423#653
adamdickmeiss merged 62 commits into
mainfrom
ILLDEV-423-redo

Conversation

@adamdickmeiss

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread broker/service/supplierlocator.go Outdated
Comment thread broker/service/supplierlocator.go Outdated
Comment thread broker/service/supplierlocator.go Outdated
Comment thread broker/patron_request/api/api-handler.go Outdated
Comment thread broker/patron_request/api/api-handler.go
Comment thread broker/holdings/adapter_zoom.go Outdated
Comment thread broker/holdings/metadata.go
Comment thread broker/holdings/adapter_sru.go Outdated
Comment thread broker/holdings/creator_impl.go
Comment thread broker/holdings/adapter_zoom_test.go Outdated
adamdickmeiss and others added 2 commits July 3, 2026 17:29
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
adamdickmeiss and others added 3 commits July 3, 2026 17:31
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread broker/holdings/creator_impl.go
Comment thread broker/holdings/adapter_zoom.go Outdated
Comment thread broker/holdings/adapter_sru.go Outdated
Comment thread broker/holdings/metadata.go Outdated
Comment thread broker/holdings/metadata.go Outdated
Comment thread broker/patron_request/api/api-handler.go
Comment thread broker/service/supplierlocator.go Outdated
Copilot AI review requested due to automatic review settings July 3, 2026 15:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

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
Copilot AI review requested due to automatic review settings July 8, 2026 14:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread broker/holdings/adapter_sru.go Outdated
@jakub-id

jakub-id commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@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

Copy link
Copy Markdown
Contributor Author

@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 AI review requested due to automatic review settings July 9, 2026 10:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread broker/holdings/adapter_sru.go
Comment thread broker/holdings/query_builder_general.go
Copilot AI review requested due to automatic review settings July 9, 2026 11:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread broker/holdings/adapter_sru.go
Copilot AI review requested due to automatic review settings July 9, 2026 12:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread broker/holdings/adapter_sru.go Outdated
Comment thread broker/patron_request/api/api-handler.go
Copilot AI review requested due to automatic review settings July 13, 2026 11:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread broker/holdings/metadata.go
Comment thread broker/holdings/adapter_sru.go
Copilot AI review requested due to automatic review settings July 13, 2026 12:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

@adamdickmeiss adamdickmeiss merged commit ba7d952 into main Jul 13, 2026
8 checks passed
@adamdickmeiss adamdickmeiss deleted the ILLDEV-423-redo branch July 13, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants