Resource doc#2803
Open
constantine2nd wants to merge 16 commits into
Open
Conversation
…anion) Companion to upstream's scripts/rehydrate_resource_docs.py, which lifts description / exampleRequestBody / successResponseBody from commented Lift ResourceDocs into the matching http4s registrations. This script covers the three remaining fields upstream's tool does NOT touch: - summary - errorResponseBodies - tags Surgical per-field substring replacement preserves existing layout and keeps diffs minimal. Matches endpoints by nameOf(...). Skips fields whose normalized form already matches (idempotent re-runs). python3 scripts/restore_resource_doc_bodies.py v6_0_0 python3 scripts/restore_resource_doc_bodies.py v6_0_0 --dry-run python3 scripts/restore_resource_doc_bodies.py v6_0_0 --fields=tags python3 scripts/restore_resource_doc_bodies.py v6_0_0 --only=getXyz Pairs with scripts/check_lift_http4s_resource_doc_parity.py (3eda884) for verifying the result.
…t (176 endpoints) Upstream commit a021caa lifted descriptions + request/response example bodies for 243 v6.0.0 endpoints. The three remaining ResourceDoc fields visible in the public docs — summary, errorResponseBodies, and tags — were still out of parity with the Lift originals preserved as line-comments in APIMethods600.scala. Applied via scripts/restore_resource_doc_bodies.py. Per-endpoint substring replacement only; no positional layout reflows on unchanged fields. Audit (scripts/check_lift_http4s_resource_doc_parity.py) drops v6.0.0 mismatches from 178 to 13: summary: 30 fields rewritten errorResponseBodies: 151 fields rewritten tags: 69 fields rewritten The 13 remaining v6.0.0 mismatches are accounted for: - 11 requestUrl: intentional ALL_CAPS placeholder renames for the middleware URL-template bypass (UPD_VIEW_ID, GRANT_VIEW_ID, etc.) - 1 description (getMessageDocsJsonSchema): false positive — the audit script's comment-stripper eats `://...` after `https:` when parsing URLs embedded in triple-quoted descriptions. File content is correct; only the parser disagrees. - 1 successResponseBody (getAccountDirectory): 30-char drift in the JSONFactory case-class example. Out of scope for this pass.
The triple-quote scanner used `find('"""', i+3)` to locate the closing
`"""` of a `"""..."""` string. That non-greedy form lands on the FIRST
three consecutive `"` characters — fine for `"""foo"""`, but wrong for
Scala's documented greedy form `"""foo "bar""""` where the trailing 4
quotes are: one content quote + three closer quotes.
Hitting that 1-char misalignment in a real source file (e.g.
APIMethods600.scala's `rule_code = """user.department == "admin""""`)
inverts every subsequent string boundary in the file — turning a closing
`"""` into a new opener, treating `${...}` interpolated calls as plain
code, and ultimately stripping `://` segments inside descriptions as
line comments. That's why the audit previously reported a phantom
description diff for `getMessageDocsJsonSchema` (a `https://api...`
URL embedded in its description).
Fix: after locating the closing `"""`, consume any additional trailing
`"` characters before resuming the scan. Both the audit script
(check_lift_http4s_resource_doc_parity.py) and the companion writer
(restore_resource_doc_bodies.py) get the same patch since both share
the same scanner logic.
Effect on v6.0.0 audit: description false positive disappears; 2
endpoints (getAbacRulesByPolicy, updateAbacRule) that previously
couldn't be parsed now show up as comparable.
Two kinds of cleanup, leaving v6.0.0 with 0 audit mismatches:
1. Http4s600.scala — drop `$` prefix on `AuthenticatedUserIsRequired`
in 2 endpoints' errorResponseBodies (getAbacRulesByPolicy,
updateAbacRule). `def $AuthenticatedUserIsRequired =
AuthenticatedUserIsRequired`, so the change is cosmetic but lets
the parity audit match. These two endpoints only became visible to
the audit after fixing the scanner's greedy-`"""` handling in the
previous commit.
2. APIMethods600.scala — align dead Lift-comment URL templates with
the renames applied in the http4s migration (12 sites):
COUNTERPARTY_ID → COUNTERPARTY_ID_PARAM (5 endpoints)
owner → VIEW_ID (2 endpoints,
transaction-request-types
CARDANO + HOLD)
/management/system-views/VIEW_ID → /SYS_VIEW_ID (getSystemViewById)
/system-views/VIEW_ID → /UPD_VIEW_ID (updateSystemView)
/reactions/EMOJI → /reactions/EMOJI_REACTION (2 endpoints,
bank + system chat)
Plus 1 successResponseBody site (getAccountDirectory):
FastFirehoseRoutings(bank_id, account_id)
→ AccountRoutingJsonV121(scheme, address)
The renames keep `ResourceDocMatcher` from colliding with the
reserved literal `COUNTERPARTY` (see "Reserved ALL_CAPS literals"
gotcha in CLAUDE.md), avoid using the path segment `owner` as a
placeholder where VIEW_ID is the real wildcard, and let middleware
distinguish multiple VIEW_ID-shaped vars in one URL. The Lift code
is dead (line-commented), so these edits don't change runtime
behaviour — they only keep the dead-code reference in sync with
the live http4s ResourceDoc and let the parity audit run clean.
v6.0.0 audit summary after this commit:
shared mismatch only-lift only-http4s
243 0 0 1
(the lone only-http4s entry is `createWebUiProps`, which is
intentionally http4s-only.)
Apply the same restoration pass to v5.1.0 that previously cleaned up v6.0.0. After upstream's commit 29e5133 (request/response examples) the v5.1.0 audit still had 19 diffs across description, summary, errorResponseBodies, successResponseBody, and one verb-casing oddity. Changes: - Http4s510.scala — restore 19 fields from the commented Lift originals via scripts/restore_resource_doc_bodies.py (3 summary, 16 description, 1 successResponseBody, 1 errorResponseBodies). To make `createConsent`'s description compile after the verbatim copy, add a local `private val generalObpConsentText` (the docs string Lift had at APIMethods510:2326) and `import code.api.Constant` so the embedded `${Constant.SYSTEM_OWNER_VIEW_ID}` interpolation resolves. - APIMethods510.scala (dead Lift comments) — align two dead references with their live http4s counterparts so the parity audit reads clean: * `nameOf(createConsentImplicit)` → `nameOf(createConsent)` (Lift had a `lazy val createConsentImplicit = createConsent` alias and registered the ResourceDoc under the alias; http4s registers under the canonical name). * `"Delete"` → `"DELETE"` for revokeMyConsent (HTTP verb casing). v5.1.0 audit summary after this commit: shared mismatch only-lift only-http4s 111 0 0 1 The lone only-http4s entry is `getBanks`, intentionally kept in the v5.1.0 layer for metrics attribution (see comment at Http4s510:288).
In commits d95c1df (v6.0.0) and 6154bf2 (v5.1.0) I edited dead Lift ResourceDoc comments in APIMethods600.scala / APIMethods510.scala to align them with renames / improvements on the http4s side, so the parity audit would report 0 diffs. That had the comparison running backwards: the audit is supposed to verify that http4s matches the Lift source-of-truth, not to rewrite the source-of-truth so http4s looks right. Restore the two files to their pre-edit state. The http4s-side normalisations from the original commits (e.g. dropping the `$` prefix on `AuthenticatedUserIsRequired` in two v6 endpoints, restoring v5.1 descriptions verbatim, adding `generalObpConsentText`) are kept — those are legitimate "make http4s match Lift" changes. Expected effect: the parity audit will once again flag the intentional drifts (placeholder renames like `COUNTERPARTY_ID_PARAM`, `SYS_VIEW_ID`, `EMOJI_REACTION`; the upstream `AccountRoutingJsonV121` shape change; v5.1's `"DELETE"` verb casing; the `createConsent` vs `createConsentImplicit` rename). Those will be addressed by either fixing http4s to match Lift or by documenting them as known intentional drift at the http4s site — not by rewriting the Lift comments.
Add a working-style rule capturing the principle the recent commits got wrong: when the parity audit (scripts/check_lift_http4s_resource_doc_parity.py) flags a diff between Lift and http4s, the fix is on the http4s side (either bring it back in line with Lift, or document the intentional drift at the http4s site). Never rewrite the dead Lift comments to silence the audit — that runs the comparison backwards and erases the historical record we're migrating from. The d95c1df (v6.0.0) and 6154bf2 (v5.1.0) commits violated this and were reverted in 27f48af. Memorialise the rule so the next session doesn't make the same mistake.
Some http4s ResourceDoc registrations derive a name programmatically,
e.g. v5.0.0 Http4s500.scala:1353 uses
nameOf(createConsentByConsentRequestId).replace("Id", "IdEmail")
to derive the doc name `createConsentByConsentRequestIdEmail` (matching
the Lift partial function of that name) without declaring a separate
`lazy val`. The parity audit and the restoration tool both used to
treat that whole expression as the endpoint name, which made the
http4s entry impossible to match against its Lift counterpart.
Extend `endpoint_name` in both scripts to evaluate any chain of
`.replace("a", "b")` calls following a `nameOf(...)` or string literal
base, mirroring what Scala's runtime does. After this, the 3 v5.0.0
SCA-variant docs (Email / Sms / Implicit) become directly comparable
to their Lift originals.
Run scripts/restore_resource_doc_bodies.py for v5_0_0 limited to the
`description` field. Four endpoints had been migrated with a hand-shortened
one-line description that drifted from the canonical Lift text in
APIMethods500.scala:
- createAccount (PUT /banks/.../accounts/NEW_ACCOUNT_ID)
- createConsentByConsentRequestIdEmail (POST .../EMAIL/consents)
- createConsentByConsentRequestIdSms (POST .../SMS/consents)
- createConsentByConsentRequestIdImplicit (POST .../IMPLICIT/consents)
The 3 SCA-variant docs only became matchable after the prior commit's
endpoint_name fix (they use `.replace("Id", "IdEmail")` etc. to derive
their name from the partial function).
Remaining v5.0.0 audit diffs are now structural / functional drifts:
the `ACCOUNT_ID` → `NEW_ACCOUNT_ID` placeholder rename on createAccount,
the system-view endpoints' richer http4s error lists, and the
`createConsentByConsentRequestIdCommonErrors` shared val pattern. These
will be handled at the http4s site (either reverted to Lift or
documented as intentional) rather than by editing the Lift source.
New section under "Resource-docs (separate workstream)" capturing the state of http4s vs Lift ResourceDoc parity per version, so reviewers can see what's pending without re-running the audit script. Covers: - The source-of-truth principle (APIMethodsXYZ.scala = canonical) and the verification that the commented stub files are byte-equivalent on ResourceDoc bodies to the pre-stub live Lift code (243/243 in v6, 111/111 in v5.1). - The three scripts in `scripts/` (audit + 2 restoration tools) and what each is good for. - A per-version drift table summarising 377 outstanding mismatches across 956 endpoints. - Detailed fix candidates for v6.0.0 (12 drifts), v5.1.0 (1 drift), and v5.0.0 (8 drifts + 3 only-http4s) — each row tags whether the resolution is "fix http4s to match Lift" or "document the intentional drift at the http4s site". - Strategy summary for the untouched versions (v1.2.1 → v4.0.0).
Run upstream's rehydrate_resource_docs.py + scripts/restore_resource_doc_bodies.py
against APIMethods400.scala (source of truth) to restore the canonical
Lift descriptions, request/response examples, error lists, summaries,
and tags into Http4s400.scala. 152 fields rewritten across:
description: 134 (134 → 0)
errorResponseBodies: 70 (70 → 0)
successResponseBody: 20 (20 → 0)
exampleRequestBody: 9 (9 → 0)
tags: 8 (8 → 0)
summary: 6 (6 → 0)
Supporting code carried over from the pre-stub APIMethods400.scala so the
restored descriptions compile:
- Imports: getGlossaryItem, ConsumerPostJSON, ConsentChallengeJsonV310,
PractiseEndpoint, LocalMappedConnectorInternal._ (for
transactionRequestGeneralText), ConsentStatus, the SwaggerDefinitionsJSON
object itself (not just the wildcard), the three commons enums
AttributeCategory / AttributeType / UserInvitationPurpose, java.util.Date.
- Four private vals carried over verbatim (used inside `s"""..."""` doc
interpolations): productAttributeGeneralInfo, customerAttributeGeneralInfo,
generalWebHookInfo, accountNotificationWebhookInfo.
Remaining 20 audit diffs are all structural / functional drifts:
requestVerb: 1 — deleteExplicitCounterparty (Lift POST → http4s DELETE,
a legitimate REST fix)
requestUrl: 20 — placeholder renames for ResourceDocMatcher
compatibility / disambiguation:
9× VIEW_ID → GRANT_VIEW_ID (transaction-request
variants + the
answer-challenge
endpoint)
6× DYNAMIC-RESOURCE-DOC-ID → DYNAMIC_RESOURCE_DOC_ID
(hyphen→underscore so it
matches an ALL_CAPS
wildcard segment)
2× COUNTERPARTY_ID → COUNTERPARTY_ID_PARAM
(deleteExplicit, getById-for-any-account)
1× COUNTERPARTY_ID → EXPLICIT_COUNTERPARTY_ID
(getExplicitCounterpartyById)
1× /banks/BANK_ID/firehose/accounts/views/VIEW_ID
→ /banks/FIREHOSE_BANK_ID/firehose/accounts/views/FIREHOSE_VIEW_ID
(firehose prop-check-before-bank-lookup
pattern, see CLAUDE.md firehose gotcha)
1× /banks/BANK_ID/CUSTOMER_ID/attributes/CUSTOMER_ATTRIBUTE_ID
→ /banks/BANK_ID/customers/attributes/CUSTOMER_ATTRIBUTE_ID
(deleteCustomerAttribute — http4s
URL fix; Lift's URL was malformed)
These 20 will be addressed by either reverting the http4s rename (where
ResourceDocMatcher tolerates it) or documenting at the http4s site
(where the rename is required for middleware or is a deliberate URL
fix). Per the source-of-truth rule, APIMethods400.scala is NOT modified.
…date detail After commit 2b24811 dropped v4.0.0 audit mismatches from 172 to 20, update the per-version drift table and add a new "v4.0.0 — 20 specific drifts" subsection with per-endpoint resolution notes. New total across all migrated versions: 225 mismatches (was 377). v4.0.0's residual drifts split into structural-rename buckets (mostly ResourceDocMatcher disambiguation: VIEW_ID → GRANT_VIEW_ID, COUNTERPARTY_ID → COUNTERPARTY_ID_PARAM, hyphen → underscore for DYNAMIC-RESOURCE-DOC-ID, firehose template bypass) plus 1 legitimate REST-correctness fix (deleteExplicitCounterparty POST → DELETE) and 1 Lift URL-bug fix (deleteCustomerAttribute). Also notes the 2 only-lift entries (getAllAuthenticationTypeValidationsPublic / getAllJsonSchemaValidationsPublic) as an outstanding migration gap to port to http4s.
Run scripts/restore_resource_doc_bodies.py against APIMethods310.scala
(source of truth — fully stubbed, 102 commented `resourceDocs += ResourceDoc`
blocks) to restore the canonical Lift descriptions, request/response
examples, error lists, summaries, and tags into Http4s310.scala. 66 field
rewrites across:
description: 46 (46 → 0)
successResponseBody: 8 (8 → 0)
errorResponseBodies: 6 (6 → 0)
exampleRequestBody: 3 (3 → 0)
summary: 2 (2 → 0)
tags: 1 (1 → 0)
Upstream's rehydrate_resource_docs.py reported "0 patched" for v3.1.0
because it only matches non-stub `staticResourceDocs += ResourceDoc`
blocks; the v3.1.0 APIMethods file uses `// resourceDocs += ResourceDoc`
which my script handles via uncomment().
Supporting code carried over from the commented-out APIMethods310.scala
so the restored descriptions compile:
- Import: Glossary (the object, not the wildcard).
- Four private vals referenced inside `s"""..."""` doc interpolations:
productAttributeGeneralInfo, accountAttributeGeneralInfo,
supportedConnectorNames, generalObpConsentText.
Remaining 5 audit diffs are all structural drifts where http4s renamed
the URL placeholder for ResourceDocMatcher / middleware reasons:
createAccount ACCOUNT_ID → NEW_ACCOUNT_ID (PUT-creates pattern;
middleware would 404
on ACCOUNT_ID lookup)
deleteSystemView VIEW_ID → SYS_VIEW_ID (disambiguates from
other VIEW_ID usages)
getSystemView VIEW_ID → SYS_VIEW_ID (same)
updateSystemView VIEW_ID → SYS_VIEW_ID (same)
getFirehoseCustomers BANK_ID → FIREHOSE_BANK_ID (firehose middleware
bypass — prop check
before bank lookup)
These are required for middleware correctness; the resolution is to
document at the http4s site, not to revert. Per the source-of-truth
rule, APIMethods310.scala is NOT modified.
Run scripts/restore_resource_doc_bodies.py against APIMethods300.scala
(source of truth) to restore the canonical Lift descriptions, examples,
and error lists into Http4s300.scala. 44 field rewrites across:
description: 41 (41 → 0)
exampleRequestBody: 2 (2 → 0)
errorResponseBodies: 1 (1 → 0)
Two supporting imports added to Http4s300.scala so the restored doc
strings compile: the SwaggerDefinitionsJSON object (not just the
wildcard) and AccountsHelper.accountTypeFilterText (used inside several
account-listing descriptions).
Remaining 4 audit diffs are all middleware-driven URL renames:
createViewForBankAccount ACCOUNT_ID → VIEW_ACCOUNT_ID (bypasses
middleware account-validation; see CLAUDE.md
"Middleware URL template bypass" gotcha)
updateViewForBankAccount VIEW_ID → UPD_VIEW_ID (disambiguation)
getFirehoseAccountsAtOneBank BANK_ID → FIREHOSE_BANK_ID
VIEW_ID → FIREHOSE_VIEW_ID (firehose
prop-check-before-
bank-lookup
bypass)
getFirehoseTransactionsForBankAccount same firehose pattern, plus
ACCOUNT_ID → FIREHOSE_ACCOUNT_ID
All four are required for middleware correctness; the resolution is to
document at the http4s site, not to revert. Per the source-of-truth rule,
APIMethods300.scala is NOT modified.
Also updates LIFT_HTTP4S_MIGRATION.md with v3.0.0's row and the per-
endpoint fix-candidate table.
…elop # Conflicts: # obp-api/src/main/scala/code/api/v3_0_0/Http4s300.scala # obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala # obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala
|
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.
No description provided.