Skip to content

Commit 9b238b0

Browse files
committed
fix: rank the Berlin Group v1.3 alias by identity, not by its configured name
standardPrecedence ranked a version by its apiStandard string, and the alias takes that string from the first segment of berlin_group_v1_3_alias_path. A deployment may point it at a name an existing standard already uses: configured as "BG/v9" the alias reports ScannedApiVersion("BG", "BG", "v9"), ranks alongside Berlin Group, and -- sorting after "v2" on the tie-breaker -- comes last, so its re-stamped copies won getBalances, getAccountList and getAccountBalances away from the canonical docs it had copied. Metrics, top-apis and popular-apis would then report BGv9-getBalances instead of BGv1.3-getBalances. The comment on standardPrecedence claimed the opposite, that the alias "can never override a first-class standard no matter how a deployment configures it". Match the alias by identity instead and rank it below every listed standard, which makes that claim true for any configuration. sortKey takes the derived alias version as a curried parameter and is package-private so the guarantee can be tested against a synthetic alias, rather than only under whichever berlin_group_v1_3_alias_path the JVM happens to have booted with. Verified both directions with the new scenario: reverting to the string-based rank fails it with "(1,BG,v9) was not less than (1,BG,v2)" -- the mechanism itself -- and it passes with the fix. Full local suite 3576/0.
1 parent 45a965a commit 9b238b0

2 files changed

Lines changed: 46 additions & 8 deletions

File tree

‎obp-api/src/main/scala/code/api/util/ResourceDocRegistry.scala‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ object ResourceDocRegistry {
7373
ScannedApis.versionMapScannedApis.toSeq
7474
.collect { case (version: ScannedApiVersion, apis) if !explicit.contains(version) =>
7575
version -> (() => apis.allResourceDocs.toSeq) }
76-
.sortBy(entry => sortKey(entry._1))
76+
.sortBy(entry => sortKey(code.api.berlin.group.v1_3.OBP_BERLIN_GROUP_1_3_Alias.apiVersion)(entry._1))
7777
.foldLeft(ListMap.empty[ApiVersion, () => Seq[ResourceDoc]])(_ + _)
7878
explicit ++ scanned
7979
}
@@ -82,27 +82,43 @@ object ResourceDocRegistry {
8282
* Standards in ASCENDING precedence: a standard later in this list wins a partialFunctionName it
8383
* shares with an earlier one, because the `.toMap` consumers keep the last entry. This
8484
* reproduces the order of the hand-written union that preceded this registry (UK Open Banking,
85-
* then Berlin Group).
86-
*
87-
* A standard that is not listed ranks below all of them -- including the Berlin Group v1.3 alias,
88-
* whose apiStandard is whatever `berlin_group_v1_3_alias_path` names, so it can never override a
89-
* first-class standard no matter how a deployment configures it.
85+
* then Berlin Group). A standard that is not listed ranks below all of them.
9086
*/
9187
private val standardPrecedence: List[String] =
9288
List(ApiVersion.ukOpenBankingV20.apiStandard, ConstantsBG.berlinGroupVersion1.apiStandard)
9389

90+
/** Below every entry of standardPrecedence, whose lowest index is -1 for an unlisted standard. */
91+
private val derivedStandardRank: Int = -2
92+
9493
/**
9594
* Total order over registry keys: precedence first, then the version's own identity.
9695
*
96+
* `derivedAliasVersion` is the version of a standard that merely re-publishes another standard's
97+
* docs -- today only the Berlin Group v1.3 alias. It is matched by identity, NOT by its
98+
* apiStandard, because that string is the first segment of `berlin_group_v1_3_alias_path` and a
99+
* deployment may legitimately choose one that an existing standard already uses: configured as
100+
* "BG/v9" the alias would otherwise rank alongside Berlin Group and, sorting after "v2", let its
101+
* re-stamped copies win getBalances, getAccountList and getAccountBalances away from the
102+
* canonical docs it copied. Ranking it derivedStandardRank keeps that impossible for any
103+
* configuration.
104+
*
97105
* The tie-breaker is (apiStandard, apiShortVersion) rather than fullyQualifiedVersion because
98106
* that pair is exactly ScannedApiVersion's equals/hashCode key, so two distinct keys of a Map
99107
* keyed by version always differ in it and sortBy never has to fall back to the unordered input.
100108
* fullyQualifiedVersion concatenates the two (apiStandard.toUpperCase + apiShortVersion) and can
101109
* therefore collide across distinct keys -- ("BG", "v1.3") and ("BGV", "1.3") both render
102110
* "BGV1.3" -- which a deployment could reach through berlin_group_v1_3_alias_path.
111+
*
112+
* Curried and package-private so a test can rank against a synthetic alias without having to
113+
* restart the JVM under a different berlin_group_v1_3_alias_path.
103114
*/
104-
private def sortKey(version: ScannedApiVersion): (Int, String, String) =
105-
(standardPrecedence.indexOf(version.apiStandard), version.apiStandard, version.apiShortVersion)
115+
private[util] def sortKey(derivedAliasVersion: ScannedApiVersion)
116+
(version: ScannedApiVersion): (Int, String, String) = {
117+
val rank =
118+
if (version == derivedAliasVersion) derivedStandardRank
119+
else standardPrecedence.indexOf(version.apiStandard)
120+
(rank, version.apiStandard, version.apiShortVersion)
121+
}
106122

107123
/** What the per-version resource-docs dispatcher serves for this version (empty if unknown). */
108124
def docsFor(version: ApiVersion): Seq[ResourceDoc] = registry.get(version).map(_ ()).getOrElse(Nil)

‎obp-api/src/test/scala/code/api/util/ResourceDocRegistryParityTest.scala‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package code.api.util
22

3+
import code.api.berlin.group.ConstantsBG
34
import code.api.berlin.group.v1_3.Http4sBGv13Alias
45
import code.setup.ServerSetup
56
import com.openbankproject.commons.util.{ApiStandards, ApiVersion, ScannedApiVersion}
@@ -117,6 +118,27 @@ class ResourceDocRegistryParityTest extends ServerSetup {
117118
resolved.get("getAccountBalances") shouldBe Some("BGv2-getAccountBalances")
118119
}
119120

121+
// The Berlin Group v1.3 alias only re-publishes the canonical BG v1.3 docs, so it must never
122+
// win a partialFunctionName away from the standard it copied. Its apiStandard is the first
123+
// segment of berlin_group_v1_3_alias_path, so a deployment can point it at a name an existing
124+
// standard already uses ("BG/v9"); ranking by that string alone put the alias alongside Berlin
125+
// Group and, sorting after "v2", ahead of it. Ranking is by identity instead, and the synthetic
126+
// alias below exercises the colliding configuration without needing a JVM under that prop.
127+
scenario("a derived alias never outranks the standard it re-publishes", RegistryParityTag) {
128+
val syntheticAlias = ScannedApiVersion("BG", "BG", "v9")
129+
val rankOf = ResourceDocRegistry.sortKey(syntheticAlias) _
130+
withClue("the alias must sort before Berlin Group, i.e. lose the `.toMap` last-wins race ") {
131+
rankOf(syntheticAlias) should be < rankOf(ConstantsBG.berlinGroupVersion2)
132+
rankOf(syntheticAlias) should be < rankOf(ConstantsBG.berlinGroupVersion1)
133+
}
134+
withClue("the alias must also sort before UK Open Banking ") {
135+
rankOf(syntheticAlias) should be < rankOf(ApiVersion.ukOpenBankingV401)
136+
}
137+
withClue("UK must still sort before Berlin Group, so BG keeps the names they share ") {
138+
rankOf(ApiVersion.ukOpenBankingV401) should be < rankOf(ConstantsBG.berlinGroupVersion2)
139+
}
140+
}
141+
120142
// An unconfigured configuration-gated standard reports ScannedApiVersion("", "", ""), whose
121143
// fullyQualifiedVersion is "" as well. While ScannedApis kept it, ApiVersionUtils.valueOf("")
122144
// resolved successfully and GET /obp/v7.0.0/resource-docs//obp answered 200 with an empty

0 commit comments

Comments
 (0)