refactor: delete the 12 Lift-era APIMethods*.scala shim files - #97
Merged
hongwei1 merged 9 commits intoSep 1, 2026
Merged
Conversation
Http4s400's getAllJsonSchemaValidations and getAllAuthenticationTypeValidations each serve two GET routes: /management/... (authenticated) and /endpoints/... (a public variant). The public variant was live but had no ResourceDoc of its own, so it silently piggy-backed on the management route's doc -- which lists $AuthenticatedUserIsRequired even though the public route enforces no such thing. The two props Lift used to gate this (read_json_schema_validation_requires_role, read_authentication_type_validation_requires_role) were fully dead: never read anywhere in the http4s port. Recovered the original Lift ResourceDoc text for both public variants (from the now-deleted APIMethods400.scala's commented-out history) and added them as dedicated ResourceDoc entries with roles=None and an error list that conditionally includes $AuthenticatedUserIsRequired based on the prop. ResourceDocMiddleware.needsAuthentication reads this per-request from the matched doc, so no handler-body branching is needed -- the existing executeAndRespond handler already works for both prop states, and the middleware enforces auth before it ever runs when the prop is true. Also corrected a stale "no Lift counterpart" comment on the existing getAllAuthenticationTypeValidationsPublic alias (a commented-out Lift counterpart does exist) and added the equivalent getAllJsonSchemaValidationsPublic alias, which was missing. Because the prop's effect on a ResourceDoc's error list is computed once at object-init time, a single running JVM only ever observes one value of it. Added two test classes tagged PropGatedPublicEndpoint that boot their own JVM with the props forced true (JsonSchemaValidationPublicPropTrueTest, AuthenticationTypeValidationPublicPropTrueTest), verifying anonymous 401 / authenticated 200. Every ordinary shard continues to exercise the default (false) branch via the existing JsonSchemaValidationTest / AuthenticationTypeValidationTest anonymous-200 scenarios. Wired PropGatedPublicEndpoint into pom.xml's default tagsToExclude (via a new test.tagsToExclude property, since a plain literal there can't be overridden by a command-line -D), run_tests_parallel.sh (a dedicated sequential step after the main shards), and both CI workflows (a new shard 10, mirroring the existing per-shard override pattern).
…line The 12 APIMethodsXYZ.scala files (about to be deleted in a follow-up commit) had shrunk to thin runtime shims plus ~60,000 lines of commented-out original Lift ResourceDoc text, kept only so check_lift_http4s_resource_doc_parity.py could audit http4s against it. Exported that text into scripts/resource_doc_baseline/ instead, one JSON file per API version, each field stored as the literal, unevaluated Scala source snippet it always was (not a parsed value) -- rehydrate_resource_docs.py and restore_resource_doc_bodies.py splice this text verbatim into a live .scala file, so the JSON has to stay reproducible Scala source text, not an evaluated representation. export_and_verify.py did the one-time export: extract every field from the live .scala source, write the JSON, read it back, and assert the two digest sets are identical -- same keys, same values, not just the same count -- before writing digest_manifest.json. That gate is what lets the .scala files be deleted with confidence nothing was lost in translation; --check-only re-verifies it any time after. check_lift_http4s_resource_doc_parity.py now reads the Lift side from this JSON baseline (the http4s side is untouched -- still live .scala). Added an allowlist mechanism (parity_allowlist.json) since none existed before: every difference between the two sides is either a new failure or a reviewed, digest-bound entry -- rename pairs, endpoints that only exist on one side, or individual field differences -- bound to the normalized value's digest so reformatting is tolerated but any real content change re-triggers a failure rather than being silently suppressed forever. allowlist_helper.py computes those digests from live data instead of hand-typing sha256 hashes. Populated the allowlist after reviewing all 154 current differences (2 rename pairs, 4 Lift-only, 13 http4s-only, 136 field mismatches): every one is an intentional http4s improvement over the Lift original (a fixed verb-casing bug, a corrected stale description, a clarified URL template variable, a documented new field) -- verified against the actual handler code on both sides, not just the doc text, with a handful of security-relevant fixes (a role gate added to signal-channel deletion, input validation added to signal messages, the password-reset endpoint no longer echoing the reset URL in its response) worth a specific mention for anyone auditing this later. None were regressions. check_lift_http4s_resource_doc_parity.py now exits 0. Also fixed a real, pre-existing bug in rehydrate_resource_docs.py and restore_resource_doc_bodies.py while updating them to read the JSON baseline: both hardcoded ResourceDoc constructor argument positions assuming the pre-Lift-teardown signature (a leading partialFunction parameter that no longer exists), so both silently matched zero endpoints against any current-signature Http4sXYZ.scala file. Fixed by detecting which signature a given call uses, the same way check_lift_http4s_resource_doc_parity.py already did, instead of a second hardcoked guess that had drifted out of sync with it.
…letion CLAUDE.md's migration rule pointed at the 12 APIMethodsXYZ.scala files as the source of truth for what http4s should match. Repointed it at scripts/resource_doc_baseline/ (see the previous commit) and the allowlist workflow for documenting reviewed differences, before those .scala files are deleted in a follow-up commit -- so the rule never points at files that don't exist. count_v7_endpoints.py's docstring/comments and its "no files found" error message still described APIMethods*.scala as a fallback discovery source; its actual discover_version_files() never read them (Http4s*.scala only), so this is a text-only correction.
…ents Every APIMethodsNNN.scala left in the tree is now a pure runtime shim -- a literal alias (val ImplementationsX = Http4sNNN.ImplementationsX) plus an empty trait, kept only so existing imports still compile. This redirects every remaining production and test reference to the real Http4sNNN object directly, clearing the way to delete the 12 shim files entirely in a follow-up commit. Production code: - ResourceDocsAPIMethods.scala: dropped the 6 empty-trait mixins (APIMethods220/210/200/140/130/121, confirmed empty by direct read) and its 9 APIMethods400.Implementations4_0_0.X call sites, now Http4s400.Implementations4_0_0.X. - APIUtil.scala: two nameOf(...) fully-qualified references (APIMethods510/500 -> Http4s510/500). - OBPAPI5_1_0.scala / OBPAPI6_0_0.scala / OBPAPI5_0_0.scala: dropped the APIMethods310/400/500/510 imports and mixins (all confirmed empty), consolidated the now-orphaned "re-export so this compiles after APIMethods* was replaced with a stub" comments into one accurate note per file (the stub isn't just replaced now, it's gone). ~41 test files: mechanical import + reference swap (code.api.vX.APIMethodsNNN -> code.api.vX.Http4sNNN; APIMethodsNNN.ImplementationsX -> Http4sNNN.ImplementationsX) across v3_1_0/v4_0_0/v5_0_0/v5_1_0/v6_0_0 test suites. These are literal aliases to the same object, so this is a pure text substitution with no behavior change -- verified by a full test-compile afterward. Cleaned up stale comments/docs that would otherwise reference deleted files after the follow-up deletion commit: Http4s600.scala, Http4s210.scala, Glossary.scala (incl. an ASCII diagram), RateLimitingUtil.scala, ChatEventPublisher.scala, FAQ.md, and introductory_system_documentation.md (including removing a logback logger entry targeting a class that would no longer exist). Left the "Intentional drift from Lift's APIMethodsNNN.scala source-of-truth" comments elsewhere untouched -- those are historical citations of where a documented behavioral decision came from, not claims that the file currently exists, so they stay accurate after the deletion.
Every one of these files had shrunk to a thin runtime shim (object
APIMethodsNNN { val ImplementationsX = Http4sNNN.ImplementationsX })
plus an empty trait, with everything else -- the original Lift
ResourceDoc registrations -- commented out. All production and test
code that imported them was redirected to the real HttpsNNN objects
directly in the previous commit; the parity audit that used to read
their commented-out text now reads scripts/resource_doc_baseline/'s
JSON export instead, verified byte-for-byte lossless against this
exact commit's source before it was allowed to proceed.
APIMethods121.scala, APIMethods130.scala, APIMethods140.scala,
APIMethods200.scala, APIMethods210.scala, APIMethods220.scala,
APIMethods300.scala, APIMethods310.scala, APIMethods400.scala,
APIMethods500.scala, APIMethods510.scala, APIMethods600.scala --
60,634 lines, all net removal, no archive copy kept in the tree
(git history is the archive).
check_lift_http4s_resource_doc_parity.py and
export_and_verify.py --check-only both still pass with these files
gone, confirming the JSON baseline and its digest manifest don't
depend on them being present.
…oints.py parse_file() hardcoded ResourceDoc constructor argument positions assuming the pre-Lift-teardown signature (a leading partialFunction parameter that no longer exists in any current file): name at args[2], verb at args[3], url at args[4]. Every current Http4sXYZ.scala uses the post-teardown signature (no leading partialFunction), where those same fields sit one position earlier. This meant args[3] -- read as the verb -- was actually the URL string, which never matched HTTP_VERBS, so every single ResourceDoc registration in every version file was silently discarded with a WARN and the script always reported 0 total endpoints. Fixed by detecting which signature a given call uses (checking whether args[2] looks like an HTTP verb literal) and computing the field positions from that, the same way check_lift_http4s_resource_doc_parity.py already does with its POSITIONAL_FIELDS / CURRENT_POSITIONAL_FIELDS split -- instead of a second hardcoded guess that had drifted out of sync with it. Confirmed via direct testing: parse_file on v1.2.1 alone went from 0 docs / 0 stderr warnings suppressed as "unexpected verb" (all 70 silently dropped) to 70 docs matched correctly. Full run now reports 852 endpoints reachable via /obp/v7.0.0/ with zero stderr warnings, in the same ballpark as V7ResourceDocsAggregationTest's independently measured 858 (the two won't match exactly -- this script's dedup/ exclude logic is a separate, simpler reimplementation of the runtime aggregation, not the same code path).
getAllJsonSchemaValidationsPublic and getAllAuthenticationTypeValidationsPublic are new v4.0.0 endpoints, so FrozenClassUtil's persisted snapshot needs regenerating to reflect the expected count increase.
Adds the two new v4.0.0 public endpoints to the text sibling. Also picks up APIUtil.JArrayBody dropping out of the reachable type set - that type stopped being reachable from any STABLE endpoint body before this branch started (confirmed against origin/develop), so this is stale-fixture drift the regeneration corrects, not a change introduced by this branch.
--allowlist was read without checking where it pointed, and the version positional was interpolated straight into a baseline filename. Constrain the former to the repo root and require the latter to match the same vN_N_N pattern the auto-discovery path already enforces.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for json_key, value in entry.items(): | ||
| if json_key == "extra_named_args": | ||
| for k, v in value.items(): | ||
| args[k] = v |
hongwei1
merged commit Sep 1, 2026
f68c9d0
into
chore/remove-retired-lift-standards
4 of 5 checks passed
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
2 participants
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
Card 2 of the Lift-era cleanup effort (Card 1: OpenBankProject#2904, merged/mergeable). Deletes the 12
APIMethodsXYZ.scalafiles — 60,634 lines that had shrunk to thin runtime shims plus commented-out LiftResourceDoctext — after moving that historical documentation into a digest-verified JSON baseline and fixing two real gaps in the process.BASE_SHA:
a98ba9847d843c6717f7f7aaa6c5cf6488819c9a(origin/develop), stacked on Card 1's branch (chore/remove-retired-lift-standards).What's in this PR
1. Two real endpoint gaps fixed (
Http4s400.scala):getAllJsonSchemaValidationsandgetAllAuthenticationTypeValidationseach serve a/management/...(authenticated) and a/endpoints/...(public) route, but the public variant had noResourceDocof its own — it silently piggy-backed on the management route's doc, which wrongly implied auth was required. Recovered the original Lift design (two props,read_json_schema_validation_requires_role/read_authentication_type_validation_requires_role, gating a dedicated public doc) from the doomedAPIMethods400.scala's commented-out history, added the missingResourceDocentries with a conditional error list, and addedgetAllJsonSchemaValidationsPublic/confirmedgetAllAuthenticationTypeValidationsPublicaliases. SinceResourceDocMiddleware.needsAuthenticationreads this per-request from the matched doc, no handler-body branching was needed. Added two new test classes (JsonSchemaValidationPublicPropTrueTest,AuthenticationTypeValidationPublicPropTrueTest, taggedPropGatedPublicEndpoint) that boot their own JVM with the props forced true — necessary because the prop's effect on the doc's error list is baked in at object-init time, so one running JVM only ever observes one branch. Wired intopom.xml(atest.tagsToExcludeproperty so the exclusion is actually overridable via-D),run_tests_parallel.sh(a dedicated sequential step), and both CI workflows (a new shard 10).2. JSON baseline replaces commented-out Lift source (
scripts/resource_doc_baseline/): one JSON file per API version, 964 endpoints total, each field stored as the literal unevaluated Scala source snippet it always was (not a parsed value —rehydrate_resource_docs.py/restore_resource_doc_bodies.pysplice this text verbatim into live.scalafiles).export_and_verify.pydid the one-time export and proved it lossless: extract from.scala, write JSON, read JSON back, assert the digest sets are identical — same keys, same values, not just the same count — before writingdigest_manifest.json. That gate is what makes deleting the.scalafiles safe.3. Parity audit rewritten + allowlist added:
check_lift_http4s_resource_doc_parity.pynow reads the Lift side from the JSON baseline (http4s side unchanged — still live.scala). Addedparity_allowlist.json, which didn't exist before — every previously-unreviewed difference is now either a new failure or a digest-bound, human-reviewed entry (rename pairs, endpoints unique to one side, individual field differences). Reviewed all 154 current differences (2 rename pairs, 4 Lift-only, 13 http4s-only, 136 field mismatches) against the actual handler code on both sides, not just the doc text — every one is an intentional http4s improvement (fixed verb-casing bugs, corrected stale descriptions, clarified URL template variables, documented new fields), including a few security-relevant fixes worth flagging explicitly: a role gate added to signal-channel deletion, input validation added to signal messages, and the password-reset endpoint no longer echoing the reset URL in its response. None were regressions.check_lift_http4s_resource_doc_parity.pynow exits 0.4. Fixed a real, independent bug found along the way:
rehydrate_resource_docs.pyandrestore_resource_doc_bodies.pyboth hardcodedResourceDocconstructor argument positions assuming the pre-Lift-teardown signature (a leadingpartialFunctionparameter that no longer exists in any current file), so both silently matched zero endpoints against any current-signatureHttp4sXYZ.scala— confirmed via direct testing before the fix (0 liftweb matchacross the board) and after (correct matches, e.g. 243/244 on v6.0.0). Fixed by detecting which signature a given call uses, reusing the same logiccheck_lift_http4s_resource_doc_parity.pyalready had, instead of a second hardcoded guess that had drifted out of sync with it.5. Import redirection across ~53 files (production + ~41 test files): every
APIMethodsNNNreference swapped for the realHttp4sNNNobject it was already aliasing (literal same-object substitution, zero behavior change) —ResourceDocsAPIMethods.scala(dropped 6 confirmed-empty trait mixins + 9 call sites),APIUtil.scala,OBPAPI5_1_0/OBPAPI6_0_0/OBPAPI5_0_0.scala(dropped 4 more confirmed-empty mixins), and the test suite.6. Deletion: the 12
APIMethodsXYZ.scalafiles, 60,634 lines, net removal — no archive copy kept (git history is the archive).CLAUDE.md's migration rule andcount_v7_endpoints.py's docs/error text were updated before this deletion, so nothing in the tree points at a file that no longer exists.Not touched
obp-commons(published artifact for sibling repos)OBPAPI*.scalaaggregator files (Card 3's scope — depends on this PR merging first)scripts/OpenAPI31Exporter.scala(already deleted in Card 1)Testing
export_and_verify.py --write: digest gate passed (9,288 field digests match exactly, byte-for-byte, across all 12 versions) before the.scalafiles were touched.export_and_verify.py --check-only: passes both before and after the deletion commit (the.scala-side half of the check gracefully skips once those files are gone).check_lift_http4s_resource_doc_parity.py --report-stale-allowlist-entries: exits 0, zero stale allowlist entries, both before and after the deletion commit — confirmed the JSON baseline reading path produces identical shared/mismatch/only-* counts (958/115/6/15) to the original.scala-reading path before any allowlist entries existed.mvn -o test-compile: clean, checked after every commit.rehydrate_resource_docs.py/restore_resource_doc_bodies.py's bug fix on scratch copies and via--dry-runagainst the real repo (no files modified) — confirmed 0-match before, correct match after.PropGatedPublicEndpoint-tagged tests pass in their own JVM (prop=true: anonymous 401, authenticated 200); existingJsonSchemaValidationTest/AuthenticationTypeValidationTeststill pass unchanged in every ordinary shard (prop=false default: anonymous 200).run_tests_parallel.sh: all 4 shards + the dedicatedPropGatedPublicEndpointstep green, 3688 tests, 0 failures, 0 errors, 16 skipped/canceled.grep -rn "APIMethods(121|130|140|200|210|220|300|310|400|500|510|600)\b" --include=*.scala .: zero hits.bootstrap.http4s.Http4sServer, isolated port, separate from any other running instance): with the props at their default (false),GET /obp/v4.0.0/endpoints/json-schema-validationsand/endpoints/authentication-type-validationsboth return 200 anonymously while the/management/...twins still 401 without auth; with both props forced true, the same two public routes return 401 anonymously. Root and the v4.0.0 resource-docs listing both responded normally throughout - no routing regression.frozen_type_meta_data/frozen_type_meta_data.txt(code.util.FrozenClassUtil/code.util.FrozenMetaDataText) to reflect the two new v4.0.0 endpoints. The text diff also surfaced thatAPIUtil.JArrayBodyhad already stopped being reachable from any STABLE endpoint body before this branch started (confirmed againstorigin/develop) - pre-existing fixture drift this regeneration corrects as a side effect, not a change this branch introduced.