refactor: single source of truth for resource-doc registry (closes BG v1.3 alias gap) - #2902
Open
hongwei1 wants to merge 7 commits into
Open
Conversation
…registry The resource-docs dispatcher serves the BG v1.3 alias (active only when berlin_group_v1_3_alias_path is set) through its ScannedApis registration, but APIUtil.allStaticResourceDocs never included it. Its docs carry their own operation ids, re-derived from the alias version string, so alias operation ids failed the getAllResourceDocs membership check used by api-collection-endpoint creation and other operation-id lookups -- the same gap BGv2 had before it was added to this union. Reproduced against a running instance with the alias prop set (OBP-40048 on a valid alias operation id) and confirmed the fix resolves it.
The alias surface is gated by berlin_group_v1_3_alias_path, which is unset in the default test environment, so its operation-id list is legitimately empty there -- skip the non-empty assertion for it while still running the membership check against getAllResourceDocs.
…ocs dispatcher Every one of its ~19 arms was `case X => resourceDocs`, unchanged -- a leftover from the pre-http4s Lift route-filter era that stopped doing any filtering once the corresponding version moved fully onto http4s. getResourceDocsList now feeds resourceDocs directly into activePlusLocalResourceDocs, with identical output.
…ion from one registry
Introduce ResourceDocRegistry as the single source of truth for "which
resource docs does version X serve", replacing two independently
hand-maintained registries: ResourceDocsAPIMethods.getResourceDocsList
(the per-version dispatcher used by /resource-docs/{VERSION}/... and API
Explorer) and APIUtil.allStaticResourceDocs (the union used wherever an
operation id must be resolved). These drifted three times by hand --
Berlin Group v2, v7-only operation ids, and the Berlin Group v1.3 alias
all had to be independently added to both places, and were each missed
at least once. Deriving both from one registry map makes that class of
drift structurally impossible going forward.
Http4sBGv2 becomes a ScannedApis registrant (its apiVersion is
ConstantsBG.berlinGroupVersion2), so it is now fully convention-driven
like the other Berlin Group / UK Open Banking standards and needs no
hand-maintained entry in the registry or a special case in
ApiVersionUtils.valueOf. The global union is now deduped by operationId
-- the underlying per-version buffers legitimately overlap (each
OBP-standard aggregation repeats every older version's docs), and
consumers only ever .find or build a lookup map from the result.
ResourceDocRegistryParityTest is rewritten to iterate the registry
itself rather than a hand-typed list of standards, so a future standard
reachable by the dispatcher is covered by construction and the test's
job narrows to catching an accidental regression back to two
independently maintained registries.
Verified live against a running instance, before and after: BGv2 and
Berlin Group v1.3 alias operation ids both still resolve through
POST /my/api-collections/{name}/api-collection-endpoints. Full local
suite: 3582 tests, 0 failures.
hongwei1
changed the title
…int operation id Adds an HTTP-level regression test for the sandbox bug report this branch started from: creating an API collection endpoint with operation_id=BGv2-getAccountDetails now returns 201, alongside the existing coverage for OBPv6.0.0, UK Open Banking, and Berlin Group v1.3 operation ids in the same scenario. Previously the only regression guard for this exact operation id was the unit-level membership check in ResourceDocRegistryParityTest; this exercises the actual endpoint.
…evel coverage berlin_group_v1_3_alias_path could not be toggled per-test at runtime: its ScannedApiVersion identity is captured once by ScannedApis. versionMapScannedApis' process-wide classpath scan (a lazy val, shared across the whole JVM/shard), which gets forced by the first unrelated request that falls through Http4sApp's route chain -- almost always long before any test-specific setPropsValues call. The only way to exercise a real alias operation id end to end is to have the prop already set before the JVM boots. Set berlin_group_v1_3_alias_path=0.6/v1 in test.default.props (local) and both CI workflows' generated test.default.props (build_pull_request. yml, build_container.yml). Add a regression test in ApiCollectionEndpointTest mirroring the existing per-standard coverage (OBPv6.0.0/UK Open Banking/Berlin Group v1.3 canonical) for the alias's BGv1-getPaymentInitiationStatus operation id, and pin the same operation id in ResourceDocRegistryParityTest alongside the existing BGv2-getAccountDetails pin.
…ift instance The global operation-id union used to be built from the v6.0.0 aggregation, so operation ids belonging to endpoints that exist only in v7.0.0 were absent from it and could not be added to an API collection. That drift instance had no regression test: the OBPv6.0.0-* cases in ApiCollectionEndpointTest pass under both the old v6-based union and the current v7-based one, so they cannot detect it. Pin OBPv7.0.0-getMyMetrics (v7-only -- not part of Http4sResourceDocAggregation.v600) as a real api-collection-endpoint request, and add the matching named pin in ResourceDocRegistryParityTest alongside the BGv2 and Berlin Group v1.3 alias ones, so all three historical drift instances now have both HTTP-level and registry-level coverage.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A bug report proposed a
ResourceDocRegistryrefactor for a production bug: addingBGv2-getAccountDetailsto an API Collection failed with an invalid-operation-id error, because OBP-API kept two hand-maintained resource-doc registries (the per-version dispatcher used by/resource-docs/{VERSION}/...and API Explorer, and the union inAPIUtil.allStaticResourceDocs/getAllResourceDocsused wherever an operation id must be resolved) and they had drifted.Two corrections to that report, verified directly against
developat the time this branch was cut:OBP-30051) is wrong — the actual failure isOBP-40048: Invalid operation_id.a1231e3f5/60b9f75d1) before this branch was cut. That merged fix closed the BGv2 gap and the v6-vs-v7 aggregation gap, and already addedResourceDocRegistryParityTest.This PR verifies that merged fix end-to-end, closes the drift instance it left behind (the Berlin Group v1.3 alias), and — following up on the repo owner's answers to the three open questions the original report raised — implements the proposed refactor: a single
ResourceDocRegistryobject that both the dispatcher and the global union now derive from, so this class of drift becomes structurally impossible rather than something that has to be remembered by hand each time a standard is added.Changes
fix: addOBP_BERLIN_GROUP_1_3_Alias.allResourceDocstoAPIUtil.allStaticResourceDocs(superseded by commit 4, kept as its own step for bisectability). Reproduced the failure first (OBP-40048on a valid alias operation id against an instance withberlin_group_v1_3_alias_pathset), then confirmed the fix resolves it.test: extendResourceDocRegistryParityTestto cover the BG v1.3 alias surface.refactor: removeResourceDocsAPIMethods'sactiveResourceDocsmatch block — all ~19 arms were confirmed pure identity functions, a leftover from the pre-http4s Lift route-filter era.refactor: introducecode.api.util.ResourceDocRegistryas the single source of truth for "which resource docs does version X serve".Http4sBGv2becomes aScannedApisregistrant (fully convention-driven, like the other Berlin Group / UK Open Banking standards — no more hand-maintained entry or special case inApiVersionUtils.valueOf). The global union is now deduped byoperationId.ResourceDocRegistryParityTestis rewritten to iterate the registry itself rather than a hand-typed list of standards, so a newly added standard is covered by construction.test: add an HTTP-level regression test pinningBGv2-getAccountDetailsas a resolvableapi-collection-endpointoperation id (the exact request from the sandbox bug report) — previously only covered by a unit-level membership check.test: enable the Berlin Group v1.3 alias by default in the test environment (berlin_group_v1_3_alias_path=0.6/v1intest.default.propsand both CI workflows' generated copy of it), so its operation ids get the same HTTP-level regression coverage as commit 5, not just a unit-level check. This couldn't be done by toggling the prop per-test at runtime — the alias'sScannedApiVersionidentity is captured once by a process-wide classpath scan (ScannedApis.versionMapScannedApis, alazy valshared across the whole JVM/shard) that gets forced by the first unrelated request that falls through the route chain, almost always before any test-specific prop override could run.Test plan
POST /obp/v4.0.0/my/api-collections/{name}/api-collection-endpointswithoperation_id=BGv2-getAccountDetails→201(re-confirms the already-merged fix, both before and after the registry refactor)Content.vue→createMyAPICollectionEndpoint), not just curlOBP-40048on a BG v1.3 alias operation id against an instance withberlin_group_v1_3_alias_pathset, before the fix;201after201ApiCollectionEndpointTestnow exercises bothBGv2-getAccountDetailsand the alias'sBGv1-getPaymentInitiationStatusas real HTTP requests, alongside existing OBPv6.0.0/UK Open Banking/Berlin Group v1.3 canonical coverageResourceDocRegistryParityTestpins both operation ids and is registry-driven end to endResourceDocsTest/SwaggerDocsTest/V7ResourceDocsAggregationTest/Http4sBGv2ResourceDocTest/RetiredApiStandardsTest/GetScannedApiVersionsTestgreen (123 tests) after the registry refactor./run_tests_parallel.sh— 3583 tests, 0 failures (final run, after all 6 commits, alias enabled by default)