CROSSLINK-299 renames catalog#661
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements CROSSLINK-299 by renaming the “holdings” lookup/config surface to “catalog” across the broker and directory API models, updating adapters, factories, handlers, and test fixtures to use the new naming and schema.
Changes:
- Rename Directory API model fields/schemas from
holdingsConfig/ParserConfigtocatalogConfig/HoldingsParserConfig(and associated parser config types). - Move/rename broker lookup code from
broker/holdingssemantics tobroker/catalogsemantics and update all call sites. - Update unit/integration tests and fixtures (JSON/XML) to match the new Directory schema and catalog lookup behavior.
Reviewed changes
Copilot reviewed 40 out of 47 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| directory/directory_api.yaml | Renames holdings configuration schema to catalog configuration and updates parser schema names/refs. |
| broker/test/service/supplierlocator_test.go | Updates supplier locator service tests to construct peers with CatalogConfig. |
| broker/test/service/e2e_test.go | Switches e2e test wiring from holdings adapter mock to catalog adapter mock. |
| broker/test/catalog/request-1.xml | Adds ISO18626 request fixture used by catalog-related tests. |
| broker/test/catalog/request-2.xml | Adds ISO18626 request fixture used by catalog-related tests. |
| broker/test/catalog/request-3.xml | Adds ISO18626 request fixture used by catalog-related tests. |
| broker/test/catalog/request-4.xml | Adds ISO18626 request fixture used by catalog-related tests. |
| broker/test/catalog/request-5.xml | Adds ISO18626 request fixture used by catalog-related tests. |
| broker/test/catalog/holdings_test.go | Renames test package/imports to catalog and updates adapter/config usage. |
| broker/test/catalog/gvi_sru_response.xml | Adds SRU response fixture for catalog holdings parsing tests. |
| broker/test/catalog/gvi_sru_response_3.xml | Adds additional SRU response fixture for catalog holdings parsing tests. |
| broker/test/catalog/gvi_directory.json | Updates mock directory entry JSON to use catalogConfig and holdingsFormat. |
| broker/service/supplierlocator.go | Switches supplier location/availability logic from holdings package to catalog package. |
| broker/service/supplierlocator_test.go | Updates supplier locator unit tests to use catalog adapters/config and metadata update flow. |
| broker/service/lookupadapterfactory.go | Updates factory to produce/return catalog lookup adapters and creator interfaces. |
| broker/patron_request/api/api-handler.go | Switches patron-request metadata update path from holdings lookup to catalog lookup. |
| broker/patron_request/api/api-handler_test.go | Updates metadata update tests to use catalog mocks and CatalogConfig. |
| broker/holdings/adapter_zoom_nocgo.go | Removes the old non-cgo Zoom adapter stub from the holdings package. |
| broker/holdings/adapter_metaproxy.go | Removes the old metaproxy adapter from the holdings package. |
| broker/handler/iso18626-handler.go | Updates retry/lookup param comparison logic to use catalog lookup params. |
| broker/catalog/sru_holdings_test.go | Updates SRU adapter tests to use the renamed SRU adapter constructor/type. |
| broker/catalog/query_builder_isxn.go | Renames package to catalog for query builder implementation. |
| broker/catalog/query_builder_general.go | Renames package to catalog for query builder implementation. |
| broker/catalog/query_builder_general_test.go | Renames package to catalog for query builder tests. |
| broker/catalog/metadata.go | Renames package to catalog for metadata utilities. |
| broker/catalog/metadata_parser_marc.go | Renames package to catalog for MARC metadata parsing. |
| broker/catalog/metadata_parser_marc_test.go | Renames package to catalog for metadata parser tests. |
| broker/catalog/holdings_parser_reservoir.go | Renames package to catalog for Reservoir holdings parsing. |
| broker/catalog/holdings_parser_opac.go | Updates OPAC holdings parser to use renamed directory config types. |
| broker/catalog/holdings_parser_marc21_plus.go | Renames package to catalog for MARC21plus1 holdings parsing. |
| broker/catalog/holdings_parser_marc21_plus_test.go | Renames package to catalog for MARC21plus1 parser tests. |
| broker/catalog/holdings_parser_marc.go | Updates MARC holdings parser to use renamed directory config types. |
| broker/catalog/gvi_holdings_test.go | Updates GVI holdings test to use CatalogConfig and HoldingsParserConfig. |
| broker/catalog/creator.go | Renames package to catalog for adapter creator definitions. |
| broker/catalog/creator_test.go | Updates creator tests to use renamed config types and adapter types. |
| broker/catalog/creator_impl.go | Updates adapter creation to read from CatalogConfig and renamed parser fields. |
| broker/catalog/create_holdings.go | Renames env-based factory to CreateLookupAdapterFromEnv and updates SRU/mock adapter construction. |
| broker/catalog/create_holdings_test.go | Updates env-based factory tests for the renamed constructor. |
| broker/catalog/catalog.go | Renames package to catalog for core interfaces (LookupAdapter/LookupResult). |
| broker/catalog/adapter_zoom.go | Renames package/type to catalog/ZoomLookupAdapter for cgo Z39.50/ZOOM adapter. |
| broker/catalog/adapter_zoom_test.go | Updates Zoom adapter tests for renamed config/types and adapter type name. |
| broker/catalog/adapter_zoom_nocgo.go | Adds non-cgo Zoom adapter stub under the new catalog package. |
| broker/catalog/adapter_sru.go | Renames SRU adapter types/constructors to SruLookupAdapter/CreateSruLookupAdapter. |
| broker/catalog/adapter_mock.go | Updates mock availability adapter to accept directory.CatalogConfig. |
| broker/catalog/adapter_mock_shared.go | Renames shared-index mock adapter/result types to MockLookupAdapter/MockLookupResult. |
| broker/catalog/adapter_metaproxy.go | Adds metaproxy-based lookup adapter under catalog package. |
| broker/app/app.go | Switches app initialization to build lookup adapter via catalog.CreateLookupAdapterFromEnv and catalog.NewAvailabilityCreator. |
Comments suppressed due to low confidence (3)
broker/catalog/creator_impl.go:38
- This error message still refers to
holdingsConfig.metadataFormat, but the config was renamed tocatalogConfigin this PR. The message should match the actual config path to avoid confusion when debugging configuration issues.
broker/catalog/creator_impl.go:57 - This error message mentions
holdingsConfig.holdingsFormat, but the config key is nowcatalogConfig.holdingsFormatper the rename in this PR. Aligning the message with the new schema makes misconfiguration easier to diagnose.
broker/patron_request/api/api-handler_test.go:1251 - This test/comment still refers to "HoldingsConfig absent" even though the helper and production code now use
CatalogConfig. The mismatch can be confusing when reading the test intent.
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>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 47 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
broker/patron_request/api/api-handler_test.go:1251
- Inline comment still refers to HoldingsConfig even though the test is now specifically about CatalogConfig being absent; this is misleading when reading failures.
broker/catalog/creator_impl.go:64 - Comment says "No holdings adapter" but this code path now means no catalog/lookup adapter (CatalogConfig is nil). Updating the comment keeps terminology consistent with the rename.
broker/catalog/create_adapter_env_test.go:12 - Test name still says "CreateHoldings" even though the helper under test is now CreateLookupAdapterFromEnv; renaming the test will make intent clearer and keep grep/search results consistent after the rename.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 49 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
broker/catalog/create_adapter_env_test.go:9
- Test name still uses the old holdings terminology even though the function under test is now CreateLookupAdapterFromEnv. Renaming the test improves clarity and makes grep/search results more accurate.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 48 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
broker/catalog/create_adapter_env_test.go:9
- The test name still uses "Holdings" although the function under test is now CreateLookupAdapterFromEnv. Renaming improves readability when scanning
go testoutput.
broker/catalog/adapter_mock_shared.go:20 - Terminology in this shared mock still says "holdings adapter". Updating the comment to "lookup adapter" keeps it consistent with the catalog/lookup rename.
broker/catalog/adapter_mock_shared.go:24 - The mock-injected error strings used below (e.g., "there is error") are grammatically unclear and not very searchable in failing test output. Prefer clearer, specific mock error messages.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 48 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
broker/catalog/create_adapter_env_test.go:9
- Test name
TestCreateHoldingsno longer matches what the test actually covers (CreateLookupAdapterFromEnv), which makes failures harder to interpret when readinggo testoutput.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 48 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
broker/catalog/create_adapter_env_test.go:9
- The test name still refers to “Holdings”, but the function under test is now CreateLookupAdapterFromEnv; renaming the test makes intent clearer when scanning failures.
https://index-data.atlassian.net/browse/CROSSLINK-299