Scala 3 migration: Lift Web to http4s, Lift Mapper to Doobie, Flyway to Liquibase

.github/Dockerfile_PreBuild

@@ -8,10 +8,20 @@ COPY /obp-api/target/lib /app/lib
 COPY /obp-api/target/obp-api.jar /app/obp-api.jar
 WORKDIR /app
 USER obp
+# -cp, not -jar. The dependencies live in /app/lib and reach an ordinary classloader either way,
+# via the jar manifest's Class-Path - but a manifest Class-Path never appears in the
+# `java.class.path` system property, and under `-jar` that property is the single jar and nothing
+# else. Two things read it directly to build a compiler classpath at runtime: our own
+# DotcScalaCompiler (dynamic endpoints, dynamic connector methods, ABAC rules) and, inside json4s,
+# the ScalaSigReader that resolves Scala 3 case-class field types. Both then fail with
+# "Could not find package scala from compiler core libraries" / an unhandled staging-compiler
+# error, on a server that otherwise boots and looks healthy. Reproduced on this image: with -jar,
+# POST /obp/v2.1.0/sandbox/data-import returned 500 from the staging compiler; with the -cp form
+# below and nothing else changed, 201.
 ENTRYPOINT ["java", \
   "--add-opens", "java.base/java.lang=ALL-UNNAMED", \
   "--add-opens", "java.base/java.lang.reflect=ALL-UNNAMED", \
   "--add-opens", "java.base/java.io=ALL-UNNAMED", \
   "--add-opens", "java.base/java.util=ALL-UNNAMED", \
   "--add-opens", "java.base/java.util.concurrent=ALL-UNNAMED", \
-  "-jar", "/app/obp-api.jar"]
+  "-cp", "/app/obp-api.jar:/app/lib/*", "bootstrap.http4s.Http4sServer"]

.github/scripts/check_changelog_data_migrations.py

@@ -0,0 +1,89 @@
+#!/usr/bin/env python3
+"""The de-duplications the changelog carries by hand must stay in it.
+
+`liquibase generateChangeLog` takes a snapshot of a schema, and a statement that ran once and left
+no trace in the catalogue is invisible to it. Two of the Flyway scripts deleted duplicate rows
+before creating a unique index, and reverse-generating the changelog kept the indexes and dropped
+the DELETEs that made them creatable. They were written back into
+db/changelog/db.changelog-dedup.yaml by hand.
+
+Nothing else would notice them going missing again. The equivalence checks that guarded the
+generation compared schemas, and both built empty databases, where a de-duplication is a no-op
+either way; the loss only shows up on a real database that holds duplicates, at the moment the
+unique index fails to build. So the statements are frozen here, verbatim as they stood in the
+Flyway scripts that are now deleted - V057 (the three internal id-mapping tables) and V116 (five
+tables whose unique index the earliest migrations left out).
+
+Freezing rather than reading them from somewhere is the point: there is no longer another copy to
+compare against, and a check that derives its expectation from the file it is checking would pass
+whatever that file said.
+
+DedupChangesetsTest is the other half - it runs the changesets against a table that actually holds
+duplicates. This one only asserts they are present.
+"""
+import re
+import sys
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[2]
+CHANGELOG_DIR = ROOT / "obp-api/src/main/resources/db/changelog"
+
+EXPECTED = [
+    # Boot's own de-duplication, moved into the changelog: it ran after the index it existed to
+    # precede, and named a table that does not exist. Unlike the eight below these carry no dbms
+    # restriction, so they use the ROW_NUMBER() derived-table form MySQL's ERROR 1093 permits -
+    # see the changelog file's header.
+    "delete from mappedentitlement where id in ( select id from ( select id, row_number() over ( partition by mbankid, muserid, mrolename order by id asc) as rn from mappedentitlement where mbankid is not null and muserid is not null and mrolename is not null ) tmp where rn > 1 )",
+    "delete from mapperaccountholders where id in ( select id from ( select id, row_number() over ( partition by user_c, accountbankpermalink, accountpermalink order by id asc) as rn from mapperaccountholders where user_c is not null and accountbankpermalink is not null and accountpermalink is not null ) tmp where rn > 1 )",
+    # V057: accountidmapping
+    "delete from accountidmapping where maccountplaintextreference is not null and id not in ( select min(id) from accountidmapping where maccountplaintextreference is not null group by maccountplaintextreference )",
+    # V057: mappedcustomeridmapping
+    "delete from mappedcustomeridmapping where mcustomerplaintextreference is not null and id not in ( select min(id) from mappedcustomeridmapping where mcustomerplaintextreference is not null group by mcustomerplaintextreference )",
+    # V057: transactionidmapping
+    "delete from transactionidmapping where transactionplaintextreference is not null and id not in ( select min(id) from transactionidmapping where transactionplaintextreference is not null group by transactionplaintextreference )",
+    # V116: mappedatm
+    "delete from mappedatm where mbankid is not null and matmid is not null and id not in ( select min(id) from mappedatm where mbankid is not null and matmid is not null group by mbankid, matmid )",
+    # V116: mappedcomment
+    "delete from mappedcomment where apiid is not null and id not in (select min(id) from mappedcomment where apiid is not null group by apiid)",
+    # V116: mappedtag
+    "delete from mappedtag where tagid is not null and id not in (select min(id) from mappedtag where tagid is not null group by tagid)",
+    # V116: mappedtransactionimage
+    "delete from mappedtransactionimage where imageid is not null and id not in ( select min(id) from mappedtransactionimage where imageid is not null group by imageid )",
+    # V116: consent_item
+    "delete from consent_item where consent_item_id is not null and id not in ( select min(id) from consent_item where consent_item_id is not null group by consent_item_id )",
+]
+
+
+def normalise(sql: str) -> str:
+    """Collapse whitespace, so YAML indentation does not count as a difference."""
+    return re.sub(r"\s+", " ", sql).strip().rstrip(";").lower()
+
+
+def main() -> int:
+    if not CHANGELOG_DIR.is_dir():
+        print(f"check_changelog_data_migrations: {CHANGELOG_DIR} does not exist", file=sys.stderr)
+        return 1
+
+    changelog = "\n".join(p.read_text() for p in sorted(CHANGELOG_DIR.rglob("*.yaml")))
+    haystack = normalise(changelog)
+
+    missing = [stmt for stmt in EXPECTED if stmt not in haystack]
+
+    print(f"check_changelog_data_migrations: {len(EXPECTED)} de-duplication statement(s) expected, "
+          f"{len(EXPECTED) - len(missing)} present in the changelog")
+    if missing:
+        print("", file=sys.stderr)
+        print("These statements are missing from the changelog:", file=sys.stderr)
+        for stmt in missing:
+            print(f"  {stmt[:150]}", file=sys.stderr)
+        print("", file=sys.stderr)
+        print("They belong in db/changelog/db.changelog-dedup.yaml as `sql` changesets, ordered "
+              "before the createIndex they clear the way for. Without them a unique index cannot "
+              "be built on a database that already holds duplicate rows - which is every existing "
+              "deployment whose constraint was missing.", file=sys.stderr)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

.github/scripts/check_changelog_preconditions.py

@@ -0,0 +1,146 @@
+#!/usr/bin/env python3
+"""Every baseline changeset must decide for itself whether its object is already there.
+
+`LiquibaseSchemaSetup.bringUpToDate` is a plain `update`, for every state a database can be in when
+the application boots - empty, or brought by an existing deployment with tables and no Liquibase
+record, or left half-way by a start that was killed. What makes one code path right for all three is
+that each changeset in the baseline carries `not tableExists` / `not indexExists` with
+`onFail: MARK_RAN`: it records itself without running when its object exists, and runs when it does
+not.
+
+Take those away and both of the failures they replaced come back. A blanket `changeLogSync` marks
+the de-duplications and the unique indexes they clear the way for as applied on the strength of the
+tables being present - and Schemifier never created those indexes, which is why V057 and V116
+existed, so the databases that needed them were exactly the ones that skipped them. And a sync
+commits row by row, so a start killed during one leaves a partial DATABASECHANGELOG that sent the
+next start into a plain `update` over objects that already existed:
+`MigrationFailedException ... Index "METRIC_CONSUMERID" already exists`, on that start and every one
+after it.
+
+The baseline is generated (scripts/GenerateChangelog.java) and normalised
+(scripts/normalise_generated_changelog.py, which inserts these). A regeneration that lost the
+insertion step would produce a changelog that looks right, passes every fresh-database test in the
+suite, and breaks only on the upgrades this exists to serve - so the invariant is checked here
+rather than left to be noticed later.
+
+Preconditions are not part of a changeset's checksum - ChangeSet.generateCheckSum reads only the
+changes and the sql visitors - so this costs nothing on databases that have already run them.
+"""
+import re
+import sys
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[2]
+CHANGELOG_DIR = ROOT / "obp-api/src/main/resources/db/changelog"
+
+# Every changelog that creates or alters schema. The view changelogs are deliberately absent:
+# they use createView with replaceIfExists under runOnChange, which is idempotent by
+# construction, and db.changelog-dedup.yaml has its own guard
+# (check_changelog_data_migrations.py) because its DELETEs cannot be expressed this way.
+# Listing them rather than globbing so that adding a changelog is a decision, not an accident -
+# a new schema file has to be named here, which is the moment to ask which guard it needs.
+SCHEMA_CHANGELOGS = [
+    CHANGELOG_DIR / "db.changelog-baseline.yaml",
+    CHANGELOG_DIR / "db.changelog-provenance.yaml",
+]
+
+
+def changesets(text):
+    """Each changeset as (id, body), split on the `- changeSet:` marker.
+
+    The indentation is not fixed: the generated baseline puts `- changeSet:` at column 0, while a
+    hand-written changelog nests it under `databaseChangeLog:`. Anchoring on column 0, as this did,
+    silently matched nothing in the second shape - the guard reported success over a file it had
+    not read. Accept either.
+    """
+    parts = re.split(r"(?m)^[ \t]*- changeSet:\n", text)[1:]
+    for body in parts:
+        m = re.search(r"^\s*id: (\S+)$", body, re.M)
+        yield (m.group(1) if m else "<unnamed>"), body
+
+
+def main():
+    missing = [c for c in SCHEMA_CHANGELOGS if not c.exists()]
+    if missing:
+        for c in missing:
+            print(f"check_changelog_preconditions: {c} not found", file=sys.stderr)
+        return 1
+
+    problems = []
+    checked = 0
+
+    for changelog in SCHEMA_CHANGELOGS:
+      where = changelog.name
+      for cs_id, body in changesets(changelog.read_text()):
+        cs_id = f"{where}::{cs_id}"
+        checked += 1
+        creates_table = "- createTable:" in body
+        creates_index = "- createIndex:" in body
+        adds_column = "- addColumn:" in body
+        if not (creates_table or creates_index or adds_column):
+            problems.append(f"{cs_id}: none of createTable / createIndex / addColumn - this check "
+                            f"does not know what precondition it needs; teach it, do not skip it")
+            continue
+
+        if "preConditions:" not in body:
+            problems.append(f"{cs_id}: no preConditions block")
+            continue
+        if "onFail: MARK_RAN" not in body:
+            problems.append(f"{cs_id}: precondition does not say onFail: MARK_RAN")
+        if not re.search(r"^\s*- not:$", body, re.M):
+            problems.append(f"{cs_id}: precondition is not negated - it must fire when the object "
+                            f"is ABSENT")
+
+        table = re.search(r"^\s*tableName: (\S+)$", body, re.M)
+        if creates_index:
+            index = re.search(r"^\s*indexName: (\S+)$", body, re.M)
+            if not re.search(r"^\s*- indexExists:$", body, re.M):
+                problems.append(f"{cs_id}: createIndex needs an indexExists precondition")
+            elif index is None:
+                problems.append(f"{cs_id}: createIndex has no indexName to check")
+            elif body.count(f"indexName: {index.group(1)}") < 2:
+                problems.append(f"{cs_id}: the precondition names an index other than "
+                                f"{index.group(1)}")
+        elif adds_column:
+            # A column cannot be checked with tableExists - the table is there either way. The
+            # precondition has to name one of the columns the changeset adds, so a database that
+            # already went through this change marks it run instead of failing on a duplicate.
+            if not re.search(r"^\s*- columnExists:$", body, re.M):
+                problems.append(f"{cs_id}: addColumn needs a columnExists precondition")
+            else:
+                guarded = re.search(r"^\s*columnName: (\S+)$", body, re.M)
+                added = set(re.findall(r"\{name: ([a-z_]+),", body))
+                if guarded is None:
+                    problems.append(f"{cs_id}: columnExists has no columnName to check")
+                elif added and guarded.group(1) not in added:
+                    problems.append(f"{cs_id}: the precondition checks {guarded.group(1)}, which "
+                                    f"is not among the columns this changeset adds")
+        else:
+            if not re.search(r"^\s*- tableExists:$", body, re.M):
+                problems.append(f"{cs_id}: createTable needs a tableExists precondition")
+            elif table is None:
+                problems.append(f"{cs_id}: createTable has no tableName to check")
+            elif body.count(f"tableName: {table.group(1)}") < 2:
+                problems.append(f"{cs_id}: the precondition names a table other than "
+                                f"{table.group(1)}")
+
+    print(f"check_changelog_preconditions: {checked} schema changeset(s) checked, "
+          f"{len(problems)} without a usable existence precondition")
+    if problems:
+        print("", file=sys.stderr)
+        for p in problems[:40]:
+            print(f"  {p}", file=sys.stderr)
+        if len(problems) > 40:
+            print(f"  ... and {len(problems) - 40} more", file=sys.stderr)
+        print("", file=sys.stderr)
+        print("Each schema changeset needs `preConditions: [onFail: MARK_RAN, not: "
+              "[tableExists|indexExists: <the object it creates>]]`. "
+              "scripts/normalise_generated_changelog.py inserts them; if the changelog was "
+              "regenerated without it, re-run the normaliser rather than adding them by hand.",
+              file=sys.stderr)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

.github/scripts/check_no_blind_commons_casts.py

@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+"""A connector result must be converted to its Commons type, never cast to it.
+
+`list.asInstanceOf[List[XCommons]]` compiles and does nothing at runtime - the element type is
+erased, so the cast never checks anything. What it does is give the compiler licence to insert a
+checkcast at the first element access, and to serialize whatever the elements actually are. The
+premise it rests on - "the provider only ever constructs XCommons" - stopped being true when the
+providers moved to Doobie and started returning their own row types implementing the same trait
+(`ProductAttributeRow`, `CardAttributeRow`, ...).
+
+That is not hypothetical. Four sites failed exactly this way and were fixed in
+`fix: convert Commons list responses instead of casting them`: management/method_routings,
+management/endpoint-mappings, management/banks/BANK_ID/cards/CARD_ID and management/webui_props
+all threw ClassCastException instead of serving a response.
+
+Every XCommons companion extends Converter/ConverterWithType, whose `toCommonsList` does the
+conversion this cast pretends to do. Use it:
+
+    data = XCommons.toCommonsList(response)          // converts
+    data = response.asInstanceOf[List[XCommons]]     // lies
+
+Scope is deliberately `List[...Commons]`. A cast to a list of something that is not a Commons type
+is a different question - it has no Converter to reach for and no trait/implementation split behind
+it - so it is left alone rather than swept in here.
+"""
+import re
+import sys
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[2]
+SOURCE_ROOTS = [ROOT / "obp-api/src/main/scala", ROOT / "obp-commons/src/main/scala"]
+
+CAST = re.compile(r"asInstanceOf\[List\[\s*([A-Za-z0-9_.]*Commons)\s*\]\]")
+
+
+def code_of(line):
+    """The line with comment content removed - the pattern is about code, and the comment
+    explaining why the pattern is banned necessarily contains it.
+
+    String-aware: a `//` inside a string literal ("http://...") must not truncate the line, or a
+    cast written after such a literal is never seen. Block-comment interiors (` * ...`) are dropped
+    wholesale; a `/*` opener keeps what precedes it. Line-local by design - a cast inside a
+    multi-line block comment would be flagged, which errs on the side the lint should err on.
+    """
+    stripped = line.lstrip()
+    if stripped.startswith("*") or stripped.startswith("/*"):
+        return ""
+    out = []
+    in_string = False
+    i, n = 0, len(line)
+    while i < n:
+        c = line[i]
+        if in_string:
+            if c == "\\" and i + 1 < n:
+                out.append("  ")
+                i += 2
+                continue
+            if c == '"':
+                in_string = False
+            out.append(c)
+            i += 1
+            continue
+        if c == '"':
+            in_string = True
+            out.append(c)
+            i += 1
+            continue
+        if c == "/" and i + 1 < n and line[i + 1] in "/*":
+            break
+        out.append(c)
+        i += 1
+    return "".join(out)
+
+
+def main():
+    offenders = []
+    scanned = 0
+    for root in SOURCE_ROOTS:
+        for path in sorted(root.rglob("*.scala")):
+            scanned += 1
+            for n, line in enumerate(path.read_text().splitlines(), 1):
+                m = CAST.search(code_of(line))
+                if m:
+                    offenders.append((path.relative_to(ROOT), n, m.group(1), line.strip()))
+
+    print(f"check_no_blind_commons_casts: {scanned} source file(s) scanned, "
+          f"{len(offenders)} blind cast(s) to a Commons list")
+    if offenders:
+        print("", file=sys.stderr)
+        for path, n, kind, line in offenders:
+            print(f"  {path}:{n}", file=sys.stderr)
+            print(f"      {line}", file=sys.stderr)
+            print(f"      -> {kind}.toCommonsList(...)", file=sys.stderr)
+        print("", file=sys.stderr)
+        print("Each of these casts a provider result to a Commons list without converting it. The "
+              "cast is erased, so it checks nothing and defers the failure to the first element "
+              "access or to serialization. Use the companion's toCommonsList instead.",
+              file=sys.stderr)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())