From 2181fc813dfbe5bd52908b22bc0fe9c809c45545 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Mon, 6 Jul 2026 00:07:13 +0200 Subject: [PATCH 01/14] build: replace fat jar with thin jar + lib/ directory packaging Drop maven-shade-plugin in favor of maven-jar-plugin + maven-dependency-plugin: obp-api.jar now contains only this module's classes/resources (287MB -> ~31MB), with runtime dependencies copied to a sibling target/lib/ directory. The jar manifest carries Main-Class, a Class-Path pointing at lib/, and the same Add-Opens list the shaded jar used, so `java -jar` keeps working unchanged. This lets Docker layer caching skip re-pulling/pushing the full dependency set on every build when only application code changes. All four Dockerfiles that run `java -jar` (which only honors manifest Class-Path, not -cp) are updated to copy lib/ alongside the jar; Dockerfile_PreBuild's ENTRYPOINT and explicit add-opens flags are untouched. Dockerfile.dev is not touched: it is currently broken independently of this change (COPY build.sbt fails since build.sbt was removed repo-wide), so its `-pl .,obp-commons` reactor-scope question never gets reached. --- .github/Dockerfile_PreBuild | 3 +- .github/workflows/build_container.yml | 15 ++-- .github/workflows/build_pull_request.yml | 5 +- README.md | 24 ++++++- development/docker/Dockerfile | 1 + development/docker/Dockerfile.local | 19 +++++ flushall_build_and_run.sh | 7 +- flushall_fast_build_and_run.sh | 7 +- obp-api/pom.xml | 89 ++++++++++++------------ 9 files changed, 112 insertions(+), 58 deletions(-) create mode 100644 development/docker/Dockerfile.local diff --git a/.github/Dockerfile_PreBuild b/.github/Dockerfile_PreBuild index df557a7748..ae93c7d9bd 100644 --- a/.github/Dockerfile_PreBuild +++ b/.github/Dockerfile_PreBuild @@ -3,7 +3,8 @@ FROM eclipse-temurin:25-jre-alpine RUN addgroup -S obp && adduser -S -h /app -G obp obp # Copy OBP source code -# Copy build artifact (JAR file) from maven build +# Copy build artifact (JAR file) and its runtime dependencies from maven build +COPY /obp-api/target/lib /app/lib COPY /obp-api/target/obp-api.jar /app/obp-api.jar WORKDIR /app USER obp diff --git a/.github/workflows/build_container.yml b/.github/workflows/build_container.yml index f67c68d97f..6bb3a3cb4a 100644 --- a/.github/workflows/build_container.yml +++ b/.github/workflows/build_container.yml @@ -51,10 +51,12 @@ jobs: # Test classes must be in target/test-classes for the test shards. # `clean` for a guaranteed-correct build. A no-clean Zinc incremental cache # (actions/cache of target/) was measured and saved only ~17s: compile time is - # dominated by Maven startup + dependency resolution + packaging the ~297MB fat - # jar + install — NOT by Scala compilation (a zero-source-change rebuild was - # still 333s vs 350s). The stale-risk class of no-clean incremental builds is - # not worth ~2.7% of wall-clock, so we keep a full clean build. + # dominated by Maven startup + dependency resolution + install — NOT by Scala + # compilation (a zero-source-change rebuild was still 333s vs 350s; this was + # measured before the fat-jar-to-thin-jar switch, so the packaging share of + # that number no longer applies and hasn't been re-measured). The stale-risk + # class of no-clean incremental builds is not worth ~2.7% of wall-clock, so we + # keep a full clean build. MAVEN_OPTS="-Xmx3G -Xss2m -XX:MaxMetaspaceSize=1G" \ mvn clean install -T 4 -Pprod -DskipTests @@ -69,7 +71,10 @@ jobs: obp-commons/target/ - name: Save .jar artifact - run: mkdir -p ./push && cp obp-api/target/obp-api.jar ./push/ + run: | + mkdir -p ./push + cp obp-api/target/obp-api.jar ./push/ + cp -r obp-api/target/lib ./push/lib - uses: actions/upload-artifact@v4 with: diff --git a/.github/workflows/build_pull_request.yml b/.github/workflows/build_pull_request.yml index 4b1e97eea3..3c50033855 100644 --- a/.github/workflows/build_pull_request.yml +++ b/.github/workflows/build_pull_request.yml @@ -65,7 +65,10 @@ jobs: obp-commons/target/ - name: Save .jar artifact - run: mkdir -p ./pull && cp obp-api/target/obp-api.jar ./pull/ + run: | + mkdir -p ./pull + cp obp-api/target/obp-api.jar ./pull/ + cp -r obp-api/target/lib ./pull/lib - uses: actions/upload-artifact@v4 with: diff --git a/README.md b/README.md index 49f20ed776..c0ab9916fe 100644 --- a/README.md +++ b/README.md @@ -74,11 +74,29 @@ java -jar obp-api/target/obp-api.jar The http4s server binds to `hostname` / `dev.port` as configured in your props file (defaults are `127.0.0.1` and `8080`). -No `--add-opens` flags are needed on the command line: the executable jar's manifest carries +`obp-api.jar` is a thin jar: it contains only this module's classes and resources. Its +runtime dependencies are copied to the sibling `obp-api/target/lib/` directory by the build, +and the jar's manifest carries a `Class-Path` entry pointing at `lib/`, so the jar and `lib/` +must stay next to each other — copying the jar alone is not enough to run it. + +No `--add-opens` flags are needed on the command line: the jar's manifest also carries an `Add-Opens` attribute (JEP 261) with all modules the runtime needs (CGLib proxy generation, Kryo serialization, Pekko remoting, Scala runtime reflection). Only when launching via a -custom classpath (`java -cp ... bootstrap.http4s.Http4sServer`) do the flags need to be passed -explicitly, since the manifest is only honored by `java -jar`. +custom classpath does the manifest not apply, since it is only honored by `java -jar`; the +equivalent form needs the flags passed explicitly: + +```sh +java --add-opens java.base/java.lang=ALL-UNNAMED \ + --add-opens java.base/java.lang.reflect=ALL-UNNAMED \ + --add-opens java.base/java.util=ALL-UNNAMED \ + --add-opens java.base/java.lang.invoke=ALL-UNNAMED \ + --add-opens java.base/java.util.jar=ALL-UNNAMED \ + --add-opens java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED \ + --add-opens java.base/java.io=ALL-UNNAMED \ + --add-opens java.base/java.util.concurrent=ALL-UNNAMED \ + --add-opens java.base/java.security=ALL-UNNAMED \ + -cp "obp-api/target/obp-api.jar:obp-api/target/lib/*" bootstrap.http4s.Http4sServer +``` [Note: How to run via IntelliJ IDEA](obp-api/src/main/docs/glossary/Run_via_IntelliJ_IDEA.md) diff --git a/development/docker/Dockerfile b/development/docker/Dockerfile index 65af3bfb2b..a9182de764 100644 --- a/development/docker/Dockerfile +++ b/development/docker/Dockerfile @@ -9,5 +9,6 @@ RUN --mount=type=cache,target=$HOME/.m2 MAVEN_OPTS="-Xmx3G -Xss2m" mvn install - RUN --mount=type=cache,target=$HOME/.m2 MAVEN_OPTS="-Xmx3G -Xss2m" mvn install -DskipTests -pl obp-api FROM eclipse-temurin:25-jre-alpine +COPY --from=maven /usr/src/OBP-API/obp-api/target/lib /app/lib COPY --from=maven /usr/src/OBP-API/obp-api/target/obp-api.jar /app/obp-api.jar ENTRYPOINT ["java", "-jar", "/app/obp-api.jar"] \ No newline at end of file diff --git a/development/docker/Dockerfile.local b/development/docker/Dockerfile.local new file mode 100644 index 0000000000..6e0ef3d1f0 --- /dev/null +++ b/development/docker/Dockerfile.local @@ -0,0 +1,19 @@ +FROM eclipse-temurin:25-jre-alpine + +WORKDIR /app + +# Copy the locally compiled jar and its runtime dependencies into the container +COPY obp-api/target/lib /app/lib +COPY obp-api/target/obp-api.jar /app/obp-api.jar + +# Expose the API port +EXPOSE 8080 + +# Run the jar with the JVM options needed for reflection in modern JDK versions +ENTRYPOINT ["java", \ + "-Xss128m", \ + "--add-opens=java.base/java.util.jar=ALL-UNNAMED", \ + "--add-opens=java.base/java.lang=ALL-UNNAMED", \ + "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED", \ + "-jar", \ + "/app/obp-api.jar"] diff --git a/flushall_build_and_run.sh b/flushall_build_and_run.sh index d37c73782b..13a8bfa6c5 100755 --- a/flushall_build_and_run.sh +++ b/flushall_build_and_run.sh @@ -124,7 +124,7 @@ fi echo "" echo "✓ Build completed successfully" -echo "✓ JAR created: obp-api/target/obp-api.jar" +echo "✓ JAR created: obp-api/target/obp-api.jar (thin jar — runtime deps in obp-api/target/lib/)" echo "✓ Build log saved to: build.log" echo "" @@ -147,7 +147,10 @@ JAVA_OPTS="--add-opens java.base/java.lang=ALL-UNNAMED \ --add-opens java.base/java.util=ALL-UNNAMED \ --add-opens java.base/java.lang.invoke=ALL-UNNAMED \ --add-opens java.base/java.util.jar=ALL-UNNAMED \ ---add-opens java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED" +--add-opens java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED \ +--add-opens java.base/java.io=ALL-UNNAMED \ +--add-opens java.base/java.util.concurrent=ALL-UNNAMED \ +--add-opens java.base/java.security=ALL-UNNAMED" RUNTIME_LOG=/tmp/obp-api.log diff --git a/flushall_fast_build_and_run.sh b/flushall_fast_build_and_run.sh index 0ad7da5ed6..88669ab6e9 100755 --- a/flushall_fast_build_and_run.sh +++ b/flushall_fast_build_and_run.sh @@ -292,7 +292,7 @@ fi echo "" echo "✓ Fast build completed successfully" -echo "✓ JAR created: obp-api/target/obp-api.jar" +echo "✓ JAR created: obp-api/target/obp-api.jar (thin jar — runtime deps in obp-api/target/lib/)" echo "✓ Build log saved to: fast_build.log" echo "" @@ -315,7 +315,10 @@ JAVA_OPTS="--add-opens java.base/java.lang=ALL-UNNAMED \ --add-opens java.base/java.util=ALL-UNNAMED \ --add-opens java.base/java.lang.invoke=ALL-UNNAMED \ --add-opens java.base/java.util.jar=ALL-UNNAMED \ ---add-opens java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED" +--add-opens java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED \ +--add-opens java.base/java.io=ALL-UNNAMED \ +--add-opens java.base/java.util.concurrent=ALL-UNNAMED \ +--add-opens java.base/java.security=ALL-UNNAMED" if [ "$RUN_BACKGROUND" = true ]; then # Run in background with output to log file diff --git a/obp-api/pom.xml b/obp-api/pom.xml index 3d2de8b34d..9e93b7b8ef 100644 --- a/obp-api/pom.xml +++ b/obp-api/pom.xml @@ -568,6 +568,7 @@ + obp-api org.apache.maven.plugins @@ -586,8 +587,8 @@ once . WDF TestSuite.txt - @@ -738,57 +739,57 @@ ${java.version} - + org.apache.maven.plugins - maven-shade-plugin - 3.5.1 + maven-jar-plugin + 3.4.2 - false - false - - + + bootstrap.http4s.Http4sServer - - - java.base/java.lang java.base/java.lang.reflect java.base/java.util java.base/java.lang.invoke java.base/java.util.jar java.base/sun.reflect.generics.reflectiveObjects java.base/java.io java.base/java.util.concurrent java.base/java.security - - - - reference.conf - - - - - *:* - - META-INF/*.SF - META-INF/*.DSA - META-INF/*.RSA - - - - obp-api + true + lib/ + + + java.base/java.lang java.base/java.lang.reflect java.base/java.util java.base/java.lang.invoke java.base/java.util.jar java.base/sun.reflect.generics.reflectiveObjects java.base/java.io java.base/java.util.concurrent java.base/java.security + + + + + + + org.apache.maven.plugins + maven-dependency-plugin + 3.8.1 - make-fat-jar - package + copy-runtime-deps + prepare-package - shade + copy-dependencies + + ${project.build.directory}/lib + runtime + false + true + From ad00710079312f0f20063d0ae876f5fb5541281f Mon Sep 17 00:00:00 2001 From: Hongwei Date: Mon, 6 Jul 2026 23:15:57 +0200 Subject: [PATCH 02/14] chore: remove residual sbt build files (project/, Dockerfile.dev COPY lines) The project migrated fully to Maven; build.sbt was already deleted. This removes the last git-tracked sbt remnants so the repo is unambiguously Maven-only: - Delete project/ (build.properties, plugins.sbt). No pom.xml or build script references it; the macro/paradise compiler plugin is already configured via scala-maven-plugin in pom.xml. - Drop the two dead COPY lines in development/docker/Dockerfile.dev: 'COPY build.sbt .' (referenced the already-deleted build.sbt, which broke this Dockerfile) and 'COPY project/ ./project/'. The sbt commands in the introductory system documentation are left as-is: they are the build instructions for the separate OBP-SEPA-Adapter project, which genuinely uses sbt. --- development/docker/Dockerfile.dev | 2 -- project/build.properties | 1 - project/plugins.sbt | 10 ---------- 3 files changed, 13 deletions(-) delete mode 100644 project/build.properties delete mode 100644 project/plugins.sbt diff --git a/development/docker/Dockerfile.dev b/development/docker/Dockerfile.dev index cf5e24a76b..25daf558c8 100644 --- a/development/docker/Dockerfile.dev +++ b/development/docker/Dockerfile.dev @@ -4,12 +4,10 @@ WORKDIR /app # Copy Maven configuration files COPY pom.xml . -COPY build.sbt . # Copy source code and necessary project files COPY obp-api/ ./obp-api/ COPY obp-commons/ ./obp-commons/ -COPY project/ ./project/ # Copy other necessary files for the build COPY jitpack.yml . diff --git a/project/build.properties b/project/build.properties deleted file mode 100644 index 46e43a97ed..0000000000 --- a/project/build.properties +++ /dev/null @@ -1 +0,0 @@ -sbt.version=1.8.2 diff --git a/project/plugins.sbt b/project/plugins.sbt deleted file mode 100644 index c5bdd04dcc..0000000000 --- a/project/plugins.sbt +++ /dev/null @@ -1,10 +0,0 @@ -// SBT plugins for OBP project -addSbtPlugin("ch.epfl.scala" % "sbt-bloop" % "1.5.6") -addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.10.0-RC1") -addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "1.2.0") - -// Scala compiler plugin for macros (equivalent to paradise plugin in Maven) -addCompilerPlugin("org.scalamacros" % "paradise" % "2.1.1" cross CrossVersion.full) - -// SemanticDB for Metals support -addCompilerPlugin("org.scalameta" % "semanticdb-scalac" % "4.13.9" cross CrossVersion.full) From 306318c648d7bcf82b506706e63778ab9f6dd99f Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 00:15:40 +0200 Subject: [PATCH 03/14] docs: remove outdated branding.md --- branding.md | 49 ------------------------------------------------- 1 file changed, 49 deletions(-) delete mode 100644 branding.md diff --git a/branding.md b/branding.md deleted file mode 100644 index 535d9fcffb..0000000000 --- a/branding.md +++ /dev/null @@ -1,49 +0,0 @@ -# Branding of the OBP API landing page - -Look and feel of the API landing page can be modified via css and with a number of variables in the props file (production.default.props). - -## CSS - -### Main CSS - -The main css is located here: - -OBP-API/src/main/webapp/media/css/website.css - -In case you want to keep it outside of the public code you can specify a new URI via props `webui_main_style_sheet`. For instance: - -webui_override_style_sheet = https://static.openbankproject.com/test/css/website.css - -### Override CSS - -The override css is used if you use `OBP-API/src/main/webapp/media/css/website.css` but you want to override some instance specific values. -In that case you can do it via props `webui_override_style_sheet` i.e. - -webui_override_style_sheet = https://static.openbankproject.com/test/css/override.css - -where `override.css` could be: - -```css -.navbar-default .navbar-nav > li #navitem-logo { - padding-top: 11px; - padding-bottom: 11px; - margin-right: 16px; - height: 88px; - margin-left: 0; -} - -.navbar-default .navbar-nav > li #navitem-logo img { - width: 67px; - height: 67px; -} - - - -#main-about { - height: 552px; -} -``` - -## Props - -There's a number of props variables starting with webui_* - see OBP-API/src/main/resources/props/sample.props.template for a comprehensive list and default values. From 297aaa21366b5287ac305c6f048c6c079cf02af1 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 00:16:47 +0200 Subject: [PATCH 04/14] chore: remove unused web-app_2_3.dtd and jitpack.yml from Docker build context --- development/docker/Dockerfile.dev | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/development/docker/Dockerfile.dev b/development/docker/Dockerfile.dev index 25daf558c8..bd3f74201d 100644 --- a/development/docker/Dockerfile.dev +++ b/development/docker/Dockerfile.dev @@ -9,9 +9,7 @@ COPY pom.xml . COPY obp-api/ ./obp-api/ COPY obp-commons/ ./obp-commons/ -# Copy other necessary files for the build -COPY jitpack.yml . -COPY web-app_2_3.dtd . + EXPOSE 8080 From 09ed3a5ee8c72b176a1451358add4cb758250a79 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 01:19:19 +0200 Subject: [PATCH 05/14] chore: remove unused web-app_2_3.dtd at root --- web-app_2_3.dtd | 1022 ----------------------------------------------- 1 file changed, 1022 deletions(-) delete mode 100644 web-app_2_3.dtd diff --git a/web-app_2_3.dtd b/web-app_2_3.dtd deleted file mode 100644 index b4bd3908db..0000000000 --- a/web-app_2_3.dtd +++ /dev/null @@ -1,1022 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From bcb58bb75f3abf249d6b8d2b9190dd95c08890c2 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 01:22:04 +0200 Subject: [PATCH 06/14] chore: remove unused web-app_2_3.dtd in submodules --- obp-api/web-app_2_3.dtd | 1022 ----------------------------------- obp-commons/web-app_2_3.dtd | 1022 ----------------------------------- 2 files changed, 2044 deletions(-) delete mode 100644 obp-api/web-app_2_3.dtd delete mode 100644 obp-commons/web-app_2_3.dtd diff --git a/obp-api/web-app_2_3.dtd b/obp-api/web-app_2_3.dtd deleted file mode 100644 index b4bd3908db..0000000000 --- a/obp-api/web-app_2_3.dtd +++ /dev/null @@ -1,1022 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/obp-commons/web-app_2_3.dtd b/obp-commons/web-app_2_3.dtd deleted file mode 100644 index b4bd3908db..0000000000 --- a/obp-commons/web-app_2_3.dtd +++ /dev/null @@ -1,1022 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From fc499cf7e5ea76b868af6def06e058bbe71877fc Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 01:42:58 +0200 Subject: [PATCH 07/14] chore: remove unused Dockerfile.local --- development/docker/Dockerfile.local | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 development/docker/Dockerfile.local diff --git a/development/docker/Dockerfile.local b/development/docker/Dockerfile.local deleted file mode 100644 index 6e0ef3d1f0..0000000000 --- a/development/docker/Dockerfile.local +++ /dev/null @@ -1,19 +0,0 @@ -FROM eclipse-temurin:25-jre-alpine - -WORKDIR /app - -# Copy the locally compiled jar and its runtime dependencies into the container -COPY obp-api/target/lib /app/lib -COPY obp-api/target/obp-api.jar /app/obp-api.jar - -# Expose the API port -EXPOSE 8080 - -# Run the jar with the JVM options needed for reflection in modern JDK versions -ENTRYPOINT ["java", \ - "-Xss128m", \ - "--add-opens=java.base/java.util.jar=ALL-UNNAMED", \ - "--add-opens=java.base/java.lang=ALL-UNNAMED", \ - "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED", \ - "-jar", \ - "/app/obp-api.jar"] From 88f64ac8208c9862a5320cac233d9842ea34cbef Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 09:19:56 +0200 Subject: [PATCH 08/14] fix: await SCA payment status persistence, redact JWT secret log, raise default DB pool - Http4sBGv13PIS.updatePaymentPsuDataAll: in the SCA-finalised branch the status save (saveTransactionRequestStatusImpl) was nested inside map, so it ran fire-and-forget and the response could be built before COMPLETED was persisted. Switch map to flatMap so the save is awaited, matching the sibling failed/default branches. Mirrors the earlier cancelPayment fix. - JwtUtil.verifyHmacSignedJwt: the entry DEBUG log printed the full consent JWT and its HMAC shared secret, letting anyone with log-read access forge consent JWTs. Drop the token and secret from the message. - CustomDBVendor: raise the hikari.maximumPoolSize default from 10 to 20. Each request holds its transaction connection for its whole lifetime, so a pool of 10 exhausts at ~5 concurrent requests. Still prop-overridable. --- .../src/main/scala/bootstrap/liftweb/CustomDBVendor.scala | 5 ++++- .../scala/code/api/berlin/group/v1_3/Http4sBGv13PIS.scala | 2 +- obp-api/src/main/scala/code/api/util/JwtUtil.scala | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala b/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala index 90f9e5987d..30c22a567a 100644 --- a/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala +++ b/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala @@ -28,7 +28,10 @@ class CustomDBVendor(driverName: String, val config = new HikariConfig() val connectionTimeout = APIUtil.getPropsAsLongValue("hikari.connectionTimeout", 30000L) - val maximumPoolSize = APIUtil.getPropsAsIntValue("hikari.maximumPoolSize", 10) + // Default 20: each request holds its transaction connection for its whole lifetime, + // so a pool of 10 exhausts at ~5 concurrent requests (rate-limit queries need a 2nd connection). + // Kept prop-overridable so ops can tune higher. + val maximumPoolSize = APIUtil.getPropsAsIntValue("hikari.maximumPoolSize", 20) val idleTimeout = APIUtil.getPropsAsLongValue("hikari.idleTimeout", 600000L) val keepaliveTime = APIUtil.getPropsAsLongValue("hikari.keepaliveTime", 30000L) val maxLifetime = APIUtil.getPropsAsLongValue("hikari.maxLifetime", 1800000L) diff --git a/obp-api/src/main/scala/code/api/berlin/group/v1_3/Http4sBGv13PIS.scala b/obp-api/src/main/scala/code/api/berlin/group/v1_3/Http4sBGv13PIS.scala index 7bd2e2bc28..d363d29a1a 100644 --- a/obp-api/src/main/scala/code/api/berlin/group/v1_3/Http4sBGv13PIS.scala +++ b/obp-api/src/main/scala/code/api/berlin/group/v1_3/Http4sBGv13PIS.scala @@ -624,7 +624,7 @@ object Http4sBGv13PIS extends MdcLoggable { ) _ <- challenge.scaStatus match { case Some(status) if status == StrongCustomerAuthenticationStatus.finalised => - NewStyle.function.createTransactionAfterChallengeV210(fromAccount, existingTransactionRequest, callContext) map { _ => + NewStyle.function.createTransactionAfterChallengeV210(fromAccount, existingTransactionRequest, callContext) flatMap { _ => NewStyle.function.saveTransactionRequestStatusImpl(existingTransactionRequest.id, COMPLETED.toString, callContext) } case Some(status) if status == StrongCustomerAuthenticationStatus.failed => diff --git a/obp-api/src/main/scala/code/api/util/JwtUtil.scala b/obp-api/src/main/scala/code/api/util/JwtUtil.scala index 3aec89156e..b6ddeabb15 100644 --- a/obp-api/src/main/scala/code/api/util/JwtUtil.scala +++ b/obp-api/src/main/scala/code/api/util/JwtUtil.scala @@ -77,7 +77,7 @@ object JwtUtil extends MdcLoggable { * @return True or False */ def verifyHmacSignedJwt(jwtToken: String, sharedSecret: String): Boolean = { - logger.debug(s"code.api.util.JwtUtil.verifyHmacSignedJwt beginning:: jwtToken($jwtToken), sharedSecret($sharedSecret)") + logger.debug("code.api.util.JwtUtil.verifyHmacSignedJwt beginning") val signedJWT = SignedJWT.parse(jwtToken) val verifier = new MACVerifier(sharedSecret) logger.debug(s"code.api.util.JwtUtil.verifyHmacSignedJwt beginning:: signedJWT($signedJWT)") From b80258f18061627e434cdac8d74300e4a60e017c Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 09:36:36 +0200 Subject: [PATCH 09/14] fix: self-heal StarConnector backoff counter on hung futures incrementFutureCounter runs synchronously but the matching decrement only ran inside the wrapped Future's map/recover. A Future that never completes (hung backend) never decremented, leaving openFuturesCount to grow unbounded, ratcheting getBackOffFactor to its worst tier and causing canOpenFuture to reject nearly all calls with ServiceIsTooBusy until the JVM restarted. Add a self-cancelling reaper TimerTask that decrements the counter on a timeout ceiling if the Future hasn't completed by then, guarded by an AtomicBoolean so the reaper and the Future's own completion can never both decrement. The 2-arg overload is preserved unchanged for existing callers; a new 3-arg overload takes an explicit reaper timeout. --- .../main/scala/code/api/util/FutureUtil.scala | 44 +++++++--- ...ConcurrentBackoffCounterSelfHealTest.scala | 85 +++++++++++++++++++ 2 files changed, 119 insertions(+), 10 deletions(-) create mode 100644 obp-api/src/test/scala/code/concurrency/ConcurrentBackoffCounterSelfHealTest.scala diff --git a/obp-api/src/main/scala/code/api/util/FutureUtil.scala b/obp-api/src/main/scala/code/api/util/FutureUtil.scala index 8e821a974f..fdaec20dfe 100644 --- a/obp-api/src/main/scala/code/api/util/FutureUtil.scala +++ b/obp-api/src/main/scala/code/api/util/FutureUtil.scala @@ -70,18 +70,42 @@ object FutureUtil { p.future } - def futureWithLimits[T](future: Future[T], serviceName: String)(implicit ec: ExecutionContext): Future[T] = { + def futureWithLimits[T](future: Future[T], serviceName: String)(implicit ec: ExecutionContext): Future[T] = + futureWithLimits(future, serviceName, Constant.longEndpointTimeoutInMillis) + + /** + * Bound the open-futures counter with a self-healing reaper. + * + * incrementFutureCounter runs synchronously; the matching decrement must run exactly once, + * no matter whether the wrapped Future completes, fails, or NEVER completes (e.g. a connector + * call to a hung backend). Without the reaper a never-completing Future keeps its slot in + * openFuturesCount forever, ratcheting getBackOffFactor to its worst tier (1024) so that + * canOpenFuture rejects ~all subsequent calls with ServiceIsTooBusy until the JVM restarts. + * + * The reaper only frees the accounting slot; the underlying Future is left untouched (giving + * the returned Future a timeout is a separate concern — see the RabbitMQ no-timeout finding). + * decrementOnce is guarded by an AtomicBoolean so the reaper and the Future's own completion + * can never decrement twice (which would drive the counter negative). + */ + def futureWithLimits[T](future: Future[T], serviceName: String, reaperTimeoutMillis: Long) + (implicit ec: ExecutionContext): Future[T] = { incrementFutureCounter(serviceName) + + val decremented = new java.util.concurrent.atomic.AtomicBoolean(false) + def decrementOnce(): Unit = + if (decremented.compareAndSet(false, true)) decrementFutureCounter(serviceName) + + val reaper = new TimerTask() { + def run(): Unit = decrementOnce() + } + timer.schedule(reaper, reaperTimeoutMillis) + + future.onComplete { _ => + reaper.cancel() + decrementOnce() + } + future - .map( - value => { - decrementFutureCounter(serviceName) - value - }).recover{ - case exception: Throwable => - decrementFutureCounter(serviceName) - throw exception - } } } diff --git a/obp-api/src/test/scala/code/concurrency/ConcurrentBackoffCounterSelfHealTest.scala b/obp-api/src/test/scala/code/concurrency/ConcurrentBackoffCounterSelfHealTest.scala new file mode 100644 index 0000000000..a3c3c38cff --- /dev/null +++ b/obp-api/src/test/scala/code/concurrency/ConcurrentBackoffCounterSelfHealTest.scala @@ -0,0 +1,85 @@ +package code.concurrency + +import code.api.util.{APIUtil, FutureUtil} +import org.scalatest.{FlatSpec, Matchers} + +import java.util.UUID +import scala.concurrent.{Await, Future, Promise} +import scala.concurrent.duration._ +import scala.util.{Failure, Success, Try} + +/** + * A: futureWithLimits must self-heal the open-futures counter. + * + * THE HAZARD: + * incrementFutureCounter runs synchronously; decrementFutureCounter only ran inside the + * wrapped Future's .map/.recover. A Future that never completes (hung backend) never + * decremented, so openFuturesCount for that service grew unbounded, ratcheting + * APIUtil.getBackOffFactor to its worst tier (1024) — canOpenFuture's modulo check then + * rejected ~all subsequent calls with ServiceIsTooBusy, permanently, until JVM restart. + * + * The fix adds a reaper TimerTask (3-arg futureWithLimits overload) that decrements the + * counter on a timeout ceiling if the wrapped Future hasn't completed by then, guarded by + * an AtomicBoolean so the reaper and the Future's own completion can never both decrement. + * + * This is a pure Future/counter test — it does not touch the DB or HTTP layer, so it does + * not extend ConcurrentRaceSetup/ServerSetupWithTestData (avoids an unnecessary full Lift + * server boot). Tagged ConcurrencyRace for consistency with the rest of the suite. + */ +class ConcurrentBackoffCounterSelfHealTest extends FlatSpec with Matchers { + + private implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.Implicits.global + + private def openFuturesCount(serviceName: String): Int = + APIUtil.serviceNameCountersMap.getOrDefault(serviceName, (0, 0))._2 + + "futureWithLimits" should "self-heal the open-futures counter when the wrapped Future never completes" taggedAs ConcurrencyRace in { + val serviceName = s"__conc_backoff_selfheal_${UUID.randomUUID.toString.take(8)}" + val neverCompletes = Promise[Unit]().future + + FutureUtil.futureWithLimits(neverCompletes, serviceName, reaperTimeoutMillis = 200) + Thread.sleep(600) + + withClue(s"openFuturesCount=${openFuturesCount(serviceName)} after reaper should have fired: ") { + openFuturesCount(serviceName) shouldBe 0 + } + APIUtil.canOpenFuture(serviceName) shouldBe true + } + + it should "not double-decrement when the underlying Future completes late, after the reaper already fired" taggedAs ConcurrencyRace in { + val serviceName = s"__conc_backoff_idempotent_${UUID.randomUUID.toString.take(8)}" + val promise = Promise[String]() + + val wrapped = FutureUtil.futureWithLimits(promise.future, serviceName, reaperTimeoutMillis = 200) + Thread.sleep(400) + promise.success("late value") + Await.result(wrapped, 5.seconds) + // give the onComplete callback a moment to run after the promise resolved + Thread.sleep(200) + + withClue(s"openFuturesCount=${openFuturesCount(serviceName)}: reaper and completion both firing must not double-decrement: ") { + openFuturesCount(serviceName) shouldBe 0 + } + } + + it should "behave exactly like the original Future when it completes immediately (no regression)" taggedAs ConcurrencyRace in { + val serviceNameOk = s"__conc_backoff_ok_${UUID.randomUUID.toString.take(8)}" + val serviceNameFail = s"__conc_backoff_fail_${UUID.randomUUID.toString.take(8)}" + + val okResult = Try(Await.result(FutureUtil.futureWithLimits(Future.successful("ok"), serviceNameOk, reaperTimeoutMillis = 5000), 5.seconds)) + val failResult = Try(Await.result(FutureUtil.futureWithLimits(Future.failed[String](new RuntimeException("boom")), serviceNameFail, reaperTimeoutMillis = 5000), 5.seconds)) + + okResult shouldBe Success("ok") + failResult match { + case Failure(e) => e.getMessage shouldBe "boom" + case Success(_) => fail("expected the failed Future to propagate its failure") + } + + withClue(s"openFuturesCount(ok)=${openFuturesCount(serviceNameOk)}: ") { + openFuturesCount(serviceNameOk) shouldBe 0 + } + withClue(s"openFuturesCount(fail)=${openFuturesCount(serviceNameFail)}: ") { + openFuturesCount(serviceNameFail) shouldBe 0 + } + } +} From caf674f8d7588ae5f74a62efc2b973ccfe00a48e Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 09:36:53 +0200 Subject: [PATCH 10/14] fix: add reply timeout and fix promise-never-completes bug in RabbitMQ connector Two related bugs in the RabbitMQ RPC path: - ResponseCallback.handle called promise.success with a block that could throw while evaluating the channel.close() cleanup. Since the throw happened while evaluating the argument to success, a channel-close failure meant the promise was never completed at all, and the exception escaped onto the RabbitMQ client dispatch thread. Fix: complete the promise with trySuccess before touching the channel, and only log a close failure instead of letting it prevent completion. - The RPC reply wait had no timeout: if an adapter never replied, the future returned by sendRequestUndGetResponseFromRabbitMQ hung forever, holding open the borrowed connection's per-request channel and the caller's slot in the backoff counter. Wrap the consume/take in FutureUtil.futureWithTimeout, bounded by a new rabbitmq_connector.response_timeout prop (default: the existing long_endpoint_timeout convention), kept under the reply queue's own 60s TTL/x-expires. On timeout the per-request channel is closed to release it back cleanly. --- .../rabbitmq/RabbitMQUtils.scala | 39 ++++++---- .../RabbitMQUtilsResponseCallbackTest.scala | 77 +++++++++++++++++++ 2 files changed, 102 insertions(+), 14 deletions(-) create mode 100644 obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala diff --git a/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala b/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala index 627c5c23ca..aafd608c6c 100644 --- a/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala +++ b/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala @@ -57,6 +57,10 @@ object RabbitMQUtils extends MdcLoggable{ val RPC_QUEUE_NAME: String = APIUtil.getPropsValue("rabbitmq_connector.request_queue", "obp_rpc_queue") val RPC_REPLY_TO_QUEUE_NAME_PREFIX: String = APIUtil.getPropsValue("rabbitmq_connector.response_queue_prefix", "obp_reply_queue") + // Application-level cap on how long to wait for an adapter reply. Kept below the 60s reply-queue TTL/x-expires above. + val RABBITMQ_RESPONSE_TIMEOUT_IN_MILLIS: Long = + APIUtil.getPropsAsLongValue("rabbitmq_connector.response_timeout", code.api.Constant.longEndpointTimeoutInMillis) + class ResponseCallback(val rabbitCorrelationId: String, channel: Channel) extends DeliverCallback { val promise = Promise[String]() @@ -64,19 +68,16 @@ object RabbitMQUtils extends MdcLoggable{ override def handle(consumerTag: String, message: Delivery): Unit = { if (message.getProperties.getCorrelationId.equals(rabbitCorrelationId)) { - { - promise.success { - val response =new String(message.getBody, "UTF-8"); - try { - if (channel.isOpen) channel.close(); - } catch { - case e: Throwable =>{ - logger.debug(s"$AdapterUnknownError Can not close the channel properly! Details:$e") - throw new RuntimeException(s"$AdapterUnknownError Can not close the channel properly! Details:$e") - } - } - response - } + val response = new String(message.getBody, "UTF-8") + // Complete the promise BEFORE touching the channel. A channel-close failure must never + // prevent the waiting request from being satisfied, and must never escape onto the + // RabbitMQ dispatch thread. trySuccess is idempotent against duplicate deliveries. + promise.trySuccess(response) + try { + if (channel.isOpen) channel.close() + } catch { + case e: Throwable => + logger.debug(s"$AdapterUnknownError Can not close the channel properly! Details:$e") } } } @@ -146,7 +147,17 @@ object RabbitMQUtils extends MdcLoggable{ val responseCallback = new ResponseCallback(rabbitMQCorrelationId, channel) channel.basicConsume(replyQueueName, true, responseCallback, cancelCallback) - responseCallback.take() + // Application-level timeout: if the adapter never replies, fail with 408 instead of hanging forever. + code.api.util.FutureUtil.futureWithTimeout(responseCallback.take())( + code.api.util.FutureUtil.EndpointTimeout(RABBITMQ_RESPONSE_TIMEOUT_IN_MILLIS), + code.api.util.FutureUtil.EndpointContext(None), + scala.concurrent.ExecutionContext.Implicits.global + ).andThen { + // On timeout/failure the ResponseCallback never ran, so the per-request channel is still open — release it. + case scala.util.Failure(_) => + try { if (channel.isOpen) channel.close() } + catch { case e: Throwable => logger.debug(s"$AdapterUnknownError timeout channel cleanup failed: $e") } + } } catch { case e: RuntimeException if e.getMessage != null && e.getMessage.startsWith("OBP-") => logger.debug(s"${RabbitMQConnector_vOct2024.toString} inBoundJson exception: $messageId = ${e}") diff --git a/obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala b/obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala new file mode 100644 index 0000000000..a224851fcd --- /dev/null +++ b/obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala @@ -0,0 +1,77 @@ +package code.bankconnectors.rabbitmq + +import com.rabbitmq.client.AMQP.BasicProperties +import com.rabbitmq.client.{Channel, Delivery, Envelope} +import org.scalatest.{FlatSpec, Matchers} + +import java.lang.reflect.{InvocationHandler, Method, Proxy} +import java.util.UUID +import scala.concurrent.Await +import scala.concurrent.duration._ + +/** + * O3: ResponseCallback.handle must complete the promise even when closing the channel fails. + * + * THE HAZARD (before the fix): `promise.success { ...; throw new RuntimeException(...) }` — the + * throw happens while evaluating the argument to `success`, so a channel-close failure meant the + * promise was never completed at all, and the exception escaped onto the RabbitMQ client dispatch + * thread. The fix completes the promise (`trySuccess`) BEFORE attempting to close the channel, so + * a close failure is merely logged and never blocks the waiting caller. + * + * No broker needed: `channel` is a dynamic proxy whose `close()` always throws, verifying the + * promise still resolves with the delivered message body. + */ +class RabbitMQUtilsResponseCallbackTest extends FlatSpec with Matchers { + + private def channelWithFailingClose(): Channel = { + val handler = new InvocationHandler { + override def invoke(proxy: Any, method: Method, args: Array[AnyRef]): AnyRef = { + method.getName match { + case "isOpen" => Boolean.box(true) + case "close" => throw new java.io.IOException("simulated channel close failure") + case "hashCode" => Int.box(System.identityHashCode(proxy)) + case "equals" => Boolean.box(proxy eq (if (args != null) args(0) else null)) + case "toString" => "FakeChannel" + case _ => null + } + } + } + Proxy.newProxyInstance( + classOf[Channel].getClassLoader, + Array(classOf[Channel]), + handler + ).asInstanceOf[Channel] + } + + "ResponseCallback.handle" should "complete the promise with the message body even when channel.close() throws" in { + val correlationId = UUID.randomUUID().toString + val channel = channelWithFailingClose() + val callback = new RabbitMQUtils.ResponseCallback(correlationId, channel) + + val properties = new BasicProperties.Builder().correlationId(correlationId).build() + val envelope = new Envelope(1L, false, "", "") + val body = "hello from adapter".getBytes("UTF-8") + val delivery = new Delivery(envelope, properties, body) + + // Must not throw, even though channel.close() will fail internally. + callback.handle("consumer-tag", delivery) + + val result = Await.result(callback.take(), 5.seconds) + result shouldBe "hello from adapter" + } + + it should "ignore deliveries whose correlationId does not match (promise stays incomplete)" in { + val correlationId = UUID.randomUUID().toString + val otherCorrelationId = UUID.randomUUID().toString + val channel = channelWithFailingClose() + val callback = new RabbitMQUtils.ResponseCallback(correlationId, channel) + + val properties = new BasicProperties.Builder().correlationId(otherCorrelationId).build() + val envelope = new Envelope(1L, false, "", "") + val delivery = new Delivery(envelope, properties, "irrelevant".getBytes("UTF-8")) + + callback.handle("consumer-tag", delivery) + + callback.promise.isCompleted shouldBe false + } +} From 97841e336083f016d134086f6c41f450d691dbe0 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Fri, 10 Jul 2026 09:52:50 +0200 Subject: [PATCH 11/14] fix: make run_tests_parallel.sh portable, detect silently-aborted suites in CI (#50) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: make run_tests_parallel.sh portable, detect silently-aborted suites in CI run_tests_parallel.sh no longer depends on one developer's machine: - Discover JDK 25 across macOS/SDKMAN/Linux instead of a hard-coded path; abort with clear guidance if none is found instead of silently falling back to whatever JDK is active. - Replace the python3/xml.etree surefire-report audit with a pure-shell parser, so a broken or missing python3 (e.g. pyexpat/libexpat ABI mismatch) can no longer turn a green run into a false FAIL. Lint and the speed report (both non-authoritative) degrade to a visible skip instead when python3 is unavailable. - Inject harmless placeholder rabbitmq_connector.* props (matching the existing local default.props convention) so RabbitMQUtilsResponseCallbackTest actually executes instead of aborting during RabbitMQUtils' static init. That last point exposed a real CI gap: comparing the local abort against a downloaded CI shard log showed CI hits the exact same ExceptionInInitializerError, but has been reporting green regardless, because the "Report failing tests" step only grepped for "*** FAILED ***" and never checked for "*** RUN ABORTED ***" / "*** SUITE ABORTED ***" — which is what scalatest prints for an aborted suite, and maven.test.failure.ignore=true makes mvn exit 0 either way. Fixed the same grep pattern in both build_pull_request.yml and build_container.yml. Verified locally: full run now reports ALL SHARDS PASSED (2926 tests, 0 failures, 0 errors). * fix: give CI shard 8 the rabbitmq_connector.* props it needs The abort-detection fix in the previous commit now correctly surfaces RabbitMQUtilsResponseCallbackTest's pre-existing *** RUN ABORTED *** (RabbitMQUtils' object initializer throws ExceptionInInitializerError without these 5 mandatory props) instead of silently ignoring it. Add the same inert placeholder values used locally (default.props convention: localhost/5671/obp/obp//) to both workflows' "Setup props" step. The test mocks the channel and connector=star never routes to the real rabbitmq connector, so nothing here opens a real broker connection — this only lets the object initializer complete. --- .github/workflows/build_container.yml | 23 ++- .github/workflows/build_pull_request.yml | 23 ++- run_tests_parallel.sh | 201 ++++++++++++++++++----- 3 files changed, 204 insertions(+), 43 deletions(-) diff --git a/.github/workflows/build_container.yml b/.github/workflows/build_container.yml index 6bb3a3cb4a..86076d1f5b 100644 --- a/.github/workflows/build_container.yml +++ b/.github/workflows/build_container.yml @@ -301,6 +301,19 @@ jobs: # can do JSON extraction (reflection) and read OBP props (getenv); without it the sandbox # denies these and DynamicResourceDocTest's native-execution scenarios fail. echo 'dynamic_code_sandbox_permissions=[new java.net.NetPermission("specifyStreamHandler"), new java.lang.reflect.ReflectPermission("suppressAccessChecks"), new java.lang.RuntimePermission("getenv.*"), new java.util.PropertyPermission("cglib.useCache", "read"), new java.util.PropertyPermission("net.sf.cglib.test.stressHashCodes", "read"), new java.util.PropertyPermission("cglib.debugLocation", "read"), new java.lang.RuntimePermission("accessDeclaredMembers"), new java.lang.RuntimePermission("getClassLoader")]' >> obp-api/src/main/resources/props/test.default.props + # RabbitMQUtils' object initializer reads these 5 as mandatory props + # (openOrThrowException) purely to construct a client the tests never + # connect with — RabbitMQUtilsResponseCallbackTest mocks the channel and + # connector=star/starConnector_supported_types above never routes to the + # real rabbitmq connector. Without them the shard-8 catch-all aborts that + # suite with ExceptionInInitializerError (silently, until the abort-detection + # fix below started catching *** RUN ABORTED ***). Same inert placeholder + # values as the local default.props convention. + echo rabbitmq_connector.host=localhost >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.port=5671 >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.username=obp >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.password=obp >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.virtual_host=/ >> obp-api/src/main/resources/props/test.default.props - name: Run tests — shard ${{ matrix.shard }} (${{ matrix.name }}) run: | @@ -414,8 +427,14 @@ jobs: maven-build-shard${{ matrix.shard }}.log | head -200 || true echo "" echo "=== FAILING TEST SCENARIOS (with 30 lines context) ===" - if grep -C 30 -n "\*\*\* FAILED \*\*\*" maven-build-shard${{ matrix.shard }}.log; then - echo "Failing tests detected in shard ${{ matrix.shard }}." + # maven.test.failure.ignore=true (root pom) makes mvn exit 0 even when a suite + # aborts entirely — "*** FAILED ***" alone misses that, since scalatest prints + # "*** RUN ABORTED ***" / "*** SUITE ABORTED ***" instead for e.g. an + # ExceptionInInitializerError, and neither pattern was being checked. That let + # a genuinely aborted suite report CI green silently. + if grep -C 30 -n -E "\*\*\* FAILED \*\*\*|\*\*\* RUN ABORTED \*\*\*|\*\*\* SUITE ABORTED \*\*\*" \ + maven-build-shard${{ matrix.shard }}.log; then + echo "Failing/aborted tests detected in shard ${{ matrix.shard }}." exit 1 else echo "No failing tests detected in shard ${{ matrix.shard }}." diff --git a/.github/workflows/build_pull_request.yml b/.github/workflows/build_pull_request.yml index 3c50033855..acc6677885 100644 --- a/.github/workflows/build_pull_request.yml +++ b/.github/workflows/build_pull_request.yml @@ -295,6 +295,19 @@ jobs: # can do JSON extraction (reflection) and read OBP props (getenv); without it the sandbox # denies these and DynamicResourceDocTest's native-execution scenarios fail. echo 'dynamic_code_sandbox_permissions=[new java.net.NetPermission("specifyStreamHandler"), new java.lang.reflect.ReflectPermission("suppressAccessChecks"), new java.lang.RuntimePermission("getenv.*"), new java.util.PropertyPermission("cglib.useCache", "read"), new java.util.PropertyPermission("net.sf.cglib.test.stressHashCodes", "read"), new java.util.PropertyPermission("cglib.debugLocation", "read"), new java.lang.RuntimePermission("accessDeclaredMembers"), new java.lang.RuntimePermission("getClassLoader")]' >> obp-api/src/main/resources/props/test.default.props + # RabbitMQUtils' object initializer reads these 5 as mandatory props + # (openOrThrowException) purely to construct a client the tests never + # connect with — RabbitMQUtilsResponseCallbackTest mocks the channel and + # connector=star/starConnector_supported_types above never routes to the + # real rabbitmq connector. Without them the shard-8 catch-all aborts that + # suite with ExceptionInInitializerError (silently, until the abort-detection + # fix below started catching *** RUN ABORTED ***). Same inert placeholder + # values as the local default.props convention. + echo rabbitmq_connector.host=localhost >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.port=5671 >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.username=obp >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.password=obp >> obp-api/src/main/resources/props/test.default.props + echo rabbitmq_connector.virtual_host=/ >> obp-api/src/main/resources/props/test.default.props - name: Run tests — shard ${{ matrix.shard }} (${{ matrix.name }}) run: | @@ -407,8 +420,14 @@ jobs: maven-build-shard${{ matrix.shard }}.log | head -200 || true echo "" echo "=== FAILING TEST SCENARIOS (with 30 lines context) ===" - if grep -C 30 -n "\*\*\* FAILED \*\*\*" maven-build-shard${{ matrix.shard }}.log; then - echo "Failing tests detected in shard ${{ matrix.shard }}." + # maven.test.failure.ignore=true (root pom) makes mvn exit 0 even when a suite + # aborts entirely — "*** FAILED ***" alone misses that, since scalatest prints + # "*** RUN ABORTED ***" / "*** SUITE ABORTED ***" instead for e.g. an + # ExceptionInInitializerError, and neither pattern was being checked. That let + # a genuinely aborted suite report CI green silently. + if grep -C 30 -n -E "\*\*\* FAILED \*\*\*|\*\*\* RUN ABORTED \*\*\*|\*\*\* SUITE ABORTED \*\*\*" \ + maven-build-shard${{ matrix.shard }}.log; then + echo "Failing/aborted tests detected in shard ${{ matrix.shard }}." exit 1 else echo "No failing tests detected in shard ${{ matrix.shard }}." diff --git a/run_tests_parallel.sh b/run_tests_parallel.sh index 2c9b57eb2d..4ce0e5be96 100755 --- a/run_tests_parallel.sh +++ b/run_tests_parallel.sh @@ -1,12 +1,95 @@ #!/bin/bash # Local parallel test runner — mirrors CI's test coverage on a single machine. -# Pinned to JDK 25 (Scala 2.12.21+). Override JAVA_HOME before running if a -# different JDK is needed. -JAVA25_HOME="/Library/Java/JavaVirtualMachines/zulu-25.jdk/Contents/Home" -if [[ -d "$JAVA25_HOME" ]]; then - export JAVA_HOME="$JAVA25_HOME" - export PATH="$JAVA_HOME/bin:$PATH" +# Pinned to JDK 25 (Scala 2.12.21+): the suite MUST run on JDK 25, because a +# different JDK produces different results. This script is portable — it does not +# hard-code any one developer's install path. It discovers a JDK 25 across macOS +# and Linux (see resolve_jdk25 below) and ABORTS with guidance if none is found, +# rather than silently falling back to whatever JDK happens to be active. +# +# Override order (first match wins): +# 1. $OBP_JDK25_HOME — explicit escape hatch for non-standard installs +# 2. $JAVA_HOME — but only if it already points at a JDK 25 +# 3. macOS /usr/libexec/java_home -v 25 (any vendor: Temurin/Zulu/Oracle/…) +# 4. SDKMAN ~/.sdkman/candidates/java/*25* +# 5. Linux /usr/lib/jvm/*25*, /opt/java/*25*, etc. +# 6. a `java` already on PATH that reports version 25 + +# _java_is_25 : true iff it runs and reports Java 25. +_java_is_25() { + local jb="$1" + [[ -d "$jb" ]] && jb="$jb/bin/java" + [[ -x "$jb" ]] || return 1 + "$jb" -version 2>&1 | grep -qE 'version "25(\.|")' +} + +resolve_jdk25() { + local c cand=() + + # 1. Respect an already-correct JAVA_HOME (explicit user override). + if [[ -n "${JAVA_HOME:-}" ]] && _java_is_25 "$JAVA_HOME"; then + export PATH="$JAVA_HOME/bin:$PATH" + return 0 + fi + + # 2. Explicit escape hatch for odd install locations. + [[ -n "${OBP_JDK25_HOME:-}" ]] && cand+=("$OBP_JDK25_HOME") + + # 3. macOS canonical resolver — vendor-agnostic. + if [[ -x /usr/libexec/java_home ]]; then + local mh; mh=$(/usr/libexec/java_home -v 25 2>/dev/null) && [[ -n "$mh" ]] && cand+=("$mh") + fi + + # 4. SDKMAN-managed JDKs. + if [[ -d "${HOME:-}/.sdkman/candidates/java" ]]; then + for c in "$HOME/.sdkman/candidates/java"/*25*/; do [[ -d "$c" ]] && cand+=("${c%/}"); done + fi + + # 5. Common Linux + macOS-bundle JVM locations (unmatched globs stay literal and + # are filtered out by the [[ -d ]] test below). + for c in /usr/lib/jvm/*25* /usr/lib/jvm/*-25 /usr/lib/jvm/java-25* \ + /opt/java/*25* /Library/Java/JavaVirtualMachines/*25*/Contents/Home; do + [[ -d "$c" ]] && cand+=("$c") + done + + # 6. First candidate that actually reports Java 25 wins. + for c in "${cand[@]}"; do + if _java_is_25 "$c"; then + export JAVA_HOME="$c" + export PATH="$JAVA_HOME/bin:$PATH" + return 0 + fi + done + + # 7. Last resort: a `java` already on PATH that is version 25. Derive JAVA_HOME + # from it so child mvn/JVMs agree on the same JDK. + if command -v java >/dev/null 2>&1 && java -version 2>&1 | grep -qE 'version "25(\.|")'; then + local jbin; jbin=$(command -v java) + command -v realpath >/dev/null 2>&1 && jbin=$(realpath "$jbin" 2>/dev/null || echo "$jbin") + local jhome; jhome=$(cd "$(dirname "$jbin")/.." 2>/dev/null && pwd) + if [[ -n "$jhome" ]] && _java_is_25 "$jhome"; then + export JAVA_HOME="$jhome" + export PATH="$JAVA_HOME/bin:$PATH" + return 0 + fi + fi + + return 1 +} + +if ! resolve_jdk25; then + cat >&2 <<'EOF' +❌ JDK 25 not found. This suite is pinned to JDK 25 (Scala 2.12.21+); running on a + different JDK produces different results, so the script refuses to continue. + Install a JDK 25 and retry, e.g.: + • SDKMAN: sdk install java 25-tem + • macOS: brew install --cask temurin@25 (or download Zulu/Temurin 25) + • Linux: install a temurin-25 / java-25-openjdk package + Or point the script at an existing install: + OBP_JDK25_HOME=/path/to/jdk-25 ./run_tests_parallel.sh +EOF + exit 1 fi +echo "JDK: $("$JAVA_HOME/bin/java" -version 2>&1 | head -1) (JAVA_HOME=$JAVA_HOME)" # CI (build_pull_request.yml / build_container.yml) uses 9 shards across 9 VMs; # this script uses 4 coarser shards that achieve identical coverage via the # catch-all mechanism, without exhausting the single local DB connection pool @@ -55,6 +138,19 @@ else exit 1 fi +# Maven is required (every dev has it via the project README's setup). +command -v mvn >/dev/null 2>&1 || { echo "ERROR: 'mvn' (Maven) not found on PATH" >&2; exit 1; } + +# python3 is used ONLY for the (non-authoritative) test-isolation lint and the +# per-test speed report. The pass/fail verdict itself is computed in pure shell +# (see the surefire audit near the end), so a missing or broken python3 never turns +# a green run red — those two extras just skip with a visible warning. +if command -v python3 >/dev/null 2>&1 && python3 -c 'import sys' >/dev/null 2>&1; then + HAVE_PY3=1 +else + HAVE_PY3=0 +fi + # Cross-checkout mutex: the obp-commons `mvn install` writes to the shared ~/.m2. # Multiple checkouts starting this script simultaneously race on that write and can # corrupt each other's JARs (torn ZipFile). We use an atomic mkdir lock to serialise @@ -182,6 +278,14 @@ run_shard() { # OBP_DYNAMIC_CODE_SANDBOX_PERMISSIONS mirrors CI's dynamic_code_sandbox_permissions # props line: without it the dynamic-code sandbox denies reflection/getenv and # DynamicResourceDocTest's native-execution scenarios fail locally (CI green, local red). + # OBP_RABBITMQ_CONNECTOR_* : RabbitMQUtils' object initializer reads 5 mandatory + # rabbitmq_connector.* props via openOrThrowException; the local test.default.props + # omits them (CI resolves them from the committed default.props), so + # RabbitMQUtilsResponseCallbackTest aborts locally with ExceptionInInitializerError + # (CI green, local red). These are the same inert placeholder values as default.props + # (localhost/5671/obp/obp//); the test mocks the channel and connector=star with + # starConnector_supported_types=mapped,internal never activates the rabbitmq connector, + # so nothing here opens a real broker connection. # -pl obp-commons,obp-api mirrors CI: obp-commons' own util suites run on whichever # shard's filter matches com.openbankproject.* (the shard-4 catch-all); on every other # shard the filter matches nothing in obp-commons -> 0 tests there. @@ -195,6 +299,11 @@ run_shard() { OBP_HTTP4S_TEST_PORT="${http4s_port}" \ OBP_MAIL_TEST_MODE="true" \ OBP_DYNAMIC_CODE_SANDBOX_PERMISSIONS='[new java.net.NetPermission("specifyStreamHandler"), new java.lang.reflect.ReflectPermission("suppressAccessChecks"), new java.lang.RuntimePermission("getenv.*"), new java.util.PropertyPermission("cglib.useCache", "read"), new java.util.PropertyPermission("net.sf.cglib.test.stressHashCodes", "read"), new java.util.PropertyPermission("cglib.debugLocation", "read"), new java.lang.RuntimePermission("accessDeclaredMembers"), new java.lang.RuntimePermission("getClassLoader")]' \ + OBP_RABBITMQ_CONNECTOR_HOST="localhost" \ + OBP_RABBITMQ_CONNECTOR_PORT="5671" \ + OBP_RABBITMQ_CONNECTOR_USERNAME="obp" \ + OBP_RABBITMQ_CONNECTOR_PASSWORD="obp" \ + OBP_RABBITMQ_CONNECTOR_VIRTUAL_HOST="/" \ OBP_API_INSTANCE_ID="shard_${n}" \ "$TIMEOUT_BIN" 1200 mvn scalatest:test -pl obp-commons,obp-api -DfailIfNoTests=false \ "-DwildcardSuites=${filter}" \ @@ -227,10 +336,17 @@ run_shard() { START=$(date +%s) # ── Lint (CI compile job's first step): test-isolation static check; abort on fail ── -echo "Lint: test-isolation check..." -if ! python3 .github/scripts/check_test_isolation.py; then - echo "❌ Lint failed (setPropsValues at class/feature body). Fix before running." >&2 - exit 1 +# CI always has python3. Locally, if python3 is unavailable we SKIP the lint with a +# visible warning rather than fail — the authoritative test verdict below does not +# depend on it (so a missing tool never masquerades as a lint/test failure). +if [[ "$HAVE_PY3" = "1" ]]; then + echo "Lint: test-isolation check..." + if ! python3 .github/scripts/check_test_isolation.py; then + echo "❌ Lint failed (setPropsValues at class/feature body). Fix before running." >&2 + exit 1 + fi +else + echo "⚠ Lint SKIPPED: python3 not available (test-isolation static check not run)." >&2 fi echo "" @@ -359,35 +475,42 @@ if [[ $OVERALL_RC -ne 0 ]]; then done fi -# ── Authoritative verdict: audit the surefire XMLs. Per-shard mvn exit codes can lie -# (timeout-killed JVMs, plugins swallowing failures), so any failure/error recorded in -# the reports fails the run regardless of what the shards returned. -SF_AUDIT=$(python3 -c ' -import xml.etree.ElementTree as ET, glob, os -tot = fail = err = skip = broken = 0 -bad = [] -files = glob.glob("obp-api/target/surefire-reports/TEST-*.xml") + \ - glob.glob("obp-commons/target/surefire-reports/TEST-*.xml") -for f in files: - try: - r = ET.parse(f).getroot() - t, fa, e = int(r.get("tests", 0)), int(r.get("failures", 0)), int(r.get("errors", 0)) - skip += int(r.get("skipped", 0)) - tot += t; fail += fa; err += e - if fa or e: - bad.append(os.path.basename(f)[5:-4] + ": " + str(fa) + " failed, " + str(e) + " errors") - except Exception: - broken += 1 - bad.append(os.path.basename(f) + ": UNPARSEABLE report (JVM killed mid-write?)") -print(tot, fail, err, skip, broken) -for b in bad: - print(b) -' 2>/dev/null) -read -r SF_TOTAL SF_FAIL SF_ERR SF_SKIP SF_BROKEN <<< "$(echo "$SF_AUDIT" | head -1)" +# ── Authoritative verdict: audit the surefire XMLs in PURE SHELL (awk/grep) so the +# pass/fail verdict does NOT depend on python3 or its XML library being healthy — +# that dependency previously caused a false FAIL on a machine whose python3/expat +# was broken. Per-shard mvn exit codes can lie (timeout-killed JVMs, plugins +# swallowing failures), so any failure/error recorded in the reports fails the run +# regardless of what the shards returned. A file with no parseable +# root (truncated: JVM killed mid-write) counts as broken. +SF_TOTAL=0; SF_FAIL=0; SF_ERR=0; SF_SKIP=0; SF_BROKEN=0 +SF_BAD=() +_sf_files=$(find obp-api/target/surefire-reports obp-commons/target/surefire-reports \ + -name 'TEST-*.xml' 2>/dev/null) +while IFS= read -r _f; do + [[ -z "$_f" ]] && continue + # The root tag (carrying tests/failures/errors/skipped) sits at the + # very top of the file, before . Read only the head so we never match + # the same-looking text inside CDATA. Attributes may be split across + # lines, so grab the first match of each independently (attribute-order-agnostic). + _head=$(head -c 8000 "$_f" 2>/dev/null) + _t=$(printf '%s' "$_head" | grep -oE 'tests="[0-9]+"' | head -1 | grep -oE '[0-9]+') + if [[ -z "$_t" ]]; then + SF_BROKEN=$((SF_BROKEN+1)) + SF_BAD+=("$(basename "$_f"): UNPARSEABLE report (JVM killed mid-write?)") + continue + fi + _fa=$(printf '%s' "$_head" | grep -oE 'failures="[0-9]+"' | head -1 | grep -oE '[0-9]+'); _fa=${_fa:-0} + _e=$(printf '%s' "$_head" | grep -oE 'errors="[0-9]+"' | head -1 | grep -oE '[0-9]+'); _e=${_e:-0} + _sk=$(printf '%s' "$_head" | grep -oE 'skipped="[0-9]+"' | head -1 | grep -oE '[0-9]+'); _sk=${_sk:-0} + SF_TOTAL=$((SF_TOTAL+_t)); SF_FAIL=$((SF_FAIL+_fa)); SF_ERR=$((SF_ERR+_e)); SF_SKIP=$((SF_SKIP+_sk)) + if [[ $_fa -ne 0 || $_e -ne 0 ]]; then + SF_BAD+=("$(basename "$_f" | sed 's/^TEST-//; s/\.xml$//'): $_fa failed, $_e errors") + fi +done <<< "$_sf_files" echo "" -echo "Surefire audit: ${SF_TOTAL:-?} tests, ${SF_FAIL:-?} failures, ${SF_ERR:-?} errors, ${SF_SKIP:-?} skipped/canceled" -if [[ "${SF_FAIL:-1}" != "0" ]] || [[ "${SF_ERR:-1}" != "0" ]] || [[ "${SF_BROKEN:-1}" != "0" ]]; then - echo "$SF_AUDIT" | tail -n +2 | sed 's/^/ ✗ /' +echo "Surefire audit: ${SF_TOTAL} tests, ${SF_FAIL} failures, ${SF_ERR} errors, ${SF_SKIP} skipped/canceled" +if [[ "$SF_FAIL" != "0" ]] || [[ "$SF_ERR" != "0" ]] || [[ "$SF_BROKEN" != "0" ]]; then + [[ ${#SF_BAD[@]} -gt 0 ]] && printf ' ✗ %s\n' "${SF_BAD[@]}" OVERALL_RC=1 fi # Zero-test floor: -DfailIfNoTests=false means a broken wildcardSuites filter runs nothing @@ -401,7 +524,7 @@ fi # ── CI parity (report job): http4s vs Lift per-test speed table; best-effort, ── # does not affect the exit code. REPORTS_DIR="obp-api/target/surefire-reports" -if ls "$REPORTS_DIR"/*.xml >/dev/null 2>&1; then +if [[ "$HAVE_PY3" = "1" ]] && ls "$REPORTS_DIR"/*.xml >/dev/null 2>&1; then echo "" echo "── Per-test speed (CI report-job equivalent) ───────" python3 .github/scripts/test_speed_report.py "$REPORTS_DIR" 2>/dev/null \ From 17da1000ecff1b1f9e3f4ff37b57fe0838fdae90 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Fri, 10 Jul 2026 10:23:29 +0200 Subject: [PATCH 12/14] fix: move ResponseCallback out of RabbitMQUtils object to stop forcing its init in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RabbitMQUtilsResponseCallbackTest never touches RabbitMQUtils' real connector method, but referencing a class nested inside a Scala object forces the whole enclosing object to initialize first — including its 5 mandatory rabbitmq_connector.* props. That's what PR #50 worked around by injecting harmless placeholder values in three places (run_tests_parallel.sh env vars, both CI workflows' Setup props step). Moving ResponseCallback to a top-level class removes the need for that workaround entirely: the test now instantiates it directly without ever touching RabbitMQUtils' companion object, so those props are never read. Removed the now-unnecessary placeholder injections from all three places. Verified: RabbitMQUtilsResponseCallbackTest passes standalone with no rabbitmq_connector.* props or env vars set anywhere, and the full local suite reports ALL SHARDS PASSED (2926 tests, 0 failures, 0 errors). --- .github/workflows/build_container.yml | 13 ----- .github/workflows/build_pull_request.yml | 13 ----- .../rabbitmq/RabbitMQUtils.scala | 58 ++++++++++--------- .../RabbitMQUtilsResponseCallbackTest.scala | 4 +- run_tests_parallel.sh | 13 ----- 5 files changed, 34 insertions(+), 67 deletions(-) diff --git a/.github/workflows/build_container.yml b/.github/workflows/build_container.yml index 86076d1f5b..27c60a6df2 100644 --- a/.github/workflows/build_container.yml +++ b/.github/workflows/build_container.yml @@ -301,19 +301,6 @@ jobs: # can do JSON extraction (reflection) and read OBP props (getenv); without it the sandbox # denies these and DynamicResourceDocTest's native-execution scenarios fail. echo 'dynamic_code_sandbox_permissions=[new java.net.NetPermission("specifyStreamHandler"), new java.lang.reflect.ReflectPermission("suppressAccessChecks"), new java.lang.RuntimePermission("getenv.*"), new java.util.PropertyPermission("cglib.useCache", "read"), new java.util.PropertyPermission("net.sf.cglib.test.stressHashCodes", "read"), new java.util.PropertyPermission("cglib.debugLocation", "read"), new java.lang.RuntimePermission("accessDeclaredMembers"), new java.lang.RuntimePermission("getClassLoader")]' >> obp-api/src/main/resources/props/test.default.props - # RabbitMQUtils' object initializer reads these 5 as mandatory props - # (openOrThrowException) purely to construct a client the tests never - # connect with — RabbitMQUtilsResponseCallbackTest mocks the channel and - # connector=star/starConnector_supported_types above never routes to the - # real rabbitmq connector. Without them the shard-8 catch-all aborts that - # suite with ExceptionInInitializerError (silently, until the abort-detection - # fix below started catching *** RUN ABORTED ***). Same inert placeholder - # values as the local default.props convention. - echo rabbitmq_connector.host=localhost >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.port=5671 >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.username=obp >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.password=obp >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.virtual_host=/ >> obp-api/src/main/resources/props/test.default.props - name: Run tests — shard ${{ matrix.shard }} (${{ matrix.name }}) run: | diff --git a/.github/workflows/build_pull_request.yml b/.github/workflows/build_pull_request.yml index acc6677885..dbda7f5d3b 100644 --- a/.github/workflows/build_pull_request.yml +++ b/.github/workflows/build_pull_request.yml @@ -295,19 +295,6 @@ jobs: # can do JSON extraction (reflection) and read OBP props (getenv); without it the sandbox # denies these and DynamicResourceDocTest's native-execution scenarios fail. echo 'dynamic_code_sandbox_permissions=[new java.net.NetPermission("specifyStreamHandler"), new java.lang.reflect.ReflectPermission("suppressAccessChecks"), new java.lang.RuntimePermission("getenv.*"), new java.util.PropertyPermission("cglib.useCache", "read"), new java.util.PropertyPermission("net.sf.cglib.test.stressHashCodes", "read"), new java.util.PropertyPermission("cglib.debugLocation", "read"), new java.lang.RuntimePermission("accessDeclaredMembers"), new java.lang.RuntimePermission("getClassLoader")]' >> obp-api/src/main/resources/props/test.default.props - # RabbitMQUtils' object initializer reads these 5 as mandatory props - # (openOrThrowException) purely to construct a client the tests never - # connect with — RabbitMQUtilsResponseCallbackTest mocks the channel and - # connector=star/starConnector_supported_types above never routes to the - # real rabbitmq connector. Without them the shard-8 catch-all aborts that - # suite with ExceptionInInitializerError (silently, until the abort-detection - # fix below started catching *** RUN ABORTED ***). Same inert placeholder - # values as the local default.props convention. - echo rabbitmq_connector.host=localhost >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.port=5671 >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.username=obp >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.password=obp >> obp-api/src/main/resources/props/test.default.props - echo rabbitmq_connector.virtual_host=/ >> obp-api/src/main/resources/props/test.default.props - name: Run tests — shard ${{ matrix.shard }} (${{ matrix.name }}) run: | diff --git a/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala b/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala index aafd608c6c..e707101a87 100644 --- a/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala +++ b/obp-api/src/main/scala/code/bankconnectors/rabbitmq/RabbitMQUtils.scala @@ -19,6 +19,38 @@ import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.{Future, Promise} +// Deliberately top-level (not nested in `object RabbitMQUtils`): referencing a class +// nested inside an object forces Scala to fully initialize that object first (all its +// `val`s, including the 5 mandatory rabbitmq_connector.* props read below) — which would +// otherwise force any test exercising ResponseCallback in isolation to also configure +// those props, even though this class itself never touches RabbitMQUtils or opens a +// real connection. +class ResponseCallback(val rabbitCorrelationId: String, channel: Channel) extends DeliverCallback with MdcLoggable { + + val promise = Promise[String]() + val future: Future[String] = promise.future + + override def handle(consumerTag: String, message: Delivery): Unit = { + if (message.getProperties.getCorrelationId.equals(rabbitCorrelationId)) { + val response = new String(message.getBody, "UTF-8") + // Complete the promise BEFORE touching the channel. A channel-close failure must never + // prevent the waiting request from being satisfied, and must never escape onto the + // RabbitMQ dispatch thread. trySuccess is idempotent against duplicate deliveries. + promise.trySuccess(response) + try { + if (channel.isOpen) channel.close() + } catch { + case e: Throwable => + logger.debug(s"$AdapterUnknownError Can not close the channel properly! Details:$e") + } + } + } + + def take(): Future[String] = { + future + } +} + /** * RabbitMQ utils. * The reason of extract this util: if not call RabbitMQ connector method, the db connection of RabbitMQ will not be initialized. @@ -61,32 +93,6 @@ object RabbitMQUtils extends MdcLoggable{ val RABBITMQ_RESPONSE_TIMEOUT_IN_MILLIS: Long = APIUtil.getPropsAsLongValue("rabbitmq_connector.response_timeout", code.api.Constant.longEndpointTimeoutInMillis) - class ResponseCallback(val rabbitCorrelationId: String, channel: Channel) extends DeliverCallback { - - val promise = Promise[String]() - val future: Future[String] = promise.future - - override def handle(consumerTag: String, message: Delivery): Unit = { - if (message.getProperties.getCorrelationId.equals(rabbitCorrelationId)) { - val response = new String(message.getBody, "UTF-8") - // Complete the promise BEFORE touching the channel. A channel-close failure must never - // prevent the waiting request from being satisfied, and must never escape onto the - // RabbitMQ dispatch thread. trySuccess is idempotent against duplicate deliveries. - promise.trySuccess(response) - try { - if (channel.isOpen) channel.close() - } catch { - case e: Throwable => - logger.debug(s"$AdapterUnknownError Can not close the channel properly! Details:$e") - } - } - } - - def take(): Future[String] = { - future - } - } - val cancelCallback: CancelCallback = (consumerTag: String) => logger.info(s"consumerTag($consumerTag) is cancelled!!") def sendRequestUndGetResponseFromRabbitMQ[T: Manifest](messageId: String, outBound: TopicTrait): Future[Box[T]] = { diff --git a/obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala b/obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala index a224851fcd..33bc8de072 100644 --- a/obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala +++ b/obp-api/src/test/scala/code/bankconnectors/rabbitmq/RabbitMQUtilsResponseCallbackTest.scala @@ -46,7 +46,7 @@ class RabbitMQUtilsResponseCallbackTest extends FlatSpec with Matchers { "ResponseCallback.handle" should "complete the promise with the message body even when channel.close() throws" in { val correlationId = UUID.randomUUID().toString val channel = channelWithFailingClose() - val callback = new RabbitMQUtils.ResponseCallback(correlationId, channel) + val callback = new ResponseCallback(correlationId, channel) val properties = new BasicProperties.Builder().correlationId(correlationId).build() val envelope = new Envelope(1L, false, "", "") @@ -64,7 +64,7 @@ class RabbitMQUtilsResponseCallbackTest extends FlatSpec with Matchers { val correlationId = UUID.randomUUID().toString val otherCorrelationId = UUID.randomUUID().toString val channel = channelWithFailingClose() - val callback = new RabbitMQUtils.ResponseCallback(correlationId, channel) + val callback = new ResponseCallback(correlationId, channel) val properties = new BasicProperties.Builder().correlationId(otherCorrelationId).build() val envelope = new Envelope(1L, false, "", "") diff --git a/run_tests_parallel.sh b/run_tests_parallel.sh index 4ce0e5be96..82a0fae3ed 100755 --- a/run_tests_parallel.sh +++ b/run_tests_parallel.sh @@ -278,14 +278,6 @@ run_shard() { # OBP_DYNAMIC_CODE_SANDBOX_PERMISSIONS mirrors CI's dynamic_code_sandbox_permissions # props line: without it the dynamic-code sandbox denies reflection/getenv and # DynamicResourceDocTest's native-execution scenarios fail locally (CI green, local red). - # OBP_RABBITMQ_CONNECTOR_* : RabbitMQUtils' object initializer reads 5 mandatory - # rabbitmq_connector.* props via openOrThrowException; the local test.default.props - # omits them (CI resolves them from the committed default.props), so - # RabbitMQUtilsResponseCallbackTest aborts locally with ExceptionInInitializerError - # (CI green, local red). These are the same inert placeholder values as default.props - # (localhost/5671/obp/obp//); the test mocks the channel and connector=star with - # starConnector_supported_types=mapped,internal never activates the rabbitmq connector, - # so nothing here opens a real broker connection. # -pl obp-commons,obp-api mirrors CI: obp-commons' own util suites run on whichever # shard's filter matches com.openbankproject.* (the shard-4 catch-all); on every other # shard the filter matches nothing in obp-commons -> 0 tests there. @@ -299,11 +291,6 @@ run_shard() { OBP_HTTP4S_TEST_PORT="${http4s_port}" \ OBP_MAIL_TEST_MODE="true" \ OBP_DYNAMIC_CODE_SANDBOX_PERMISSIONS='[new java.net.NetPermission("specifyStreamHandler"), new java.lang.reflect.ReflectPermission("suppressAccessChecks"), new java.lang.RuntimePermission("getenv.*"), new java.util.PropertyPermission("cglib.useCache", "read"), new java.util.PropertyPermission("net.sf.cglib.test.stressHashCodes", "read"), new java.util.PropertyPermission("cglib.debugLocation", "read"), new java.lang.RuntimePermission("accessDeclaredMembers"), new java.lang.RuntimePermission("getClassLoader")]' \ - OBP_RABBITMQ_CONNECTOR_HOST="localhost" \ - OBP_RABBITMQ_CONNECTOR_PORT="5671" \ - OBP_RABBITMQ_CONNECTOR_USERNAME="obp" \ - OBP_RABBITMQ_CONNECTOR_PASSWORD="obp" \ - OBP_RABBITMQ_CONNECTOR_VIRTUAL_HOST="/" \ OBP_API_INSTANCE_ID="shard_${n}" \ "$TIMEOUT_BIN" 1200 mvn scalatest:test -pl obp-commons,obp-api -DfailIfNoTests=false \ "-DwildcardSuites=${filter}" \ From 9645d01b333436df7b3ec1624504966ecf5b908c Mon Sep 17 00:00:00 2001 From: Hongwei Date: Fri, 10 Jul 2026 10:30:24 +0200 Subject: [PATCH 13/14] refactor: extract shared digit regex in surefire audit (SonarCloud S1192) (#51) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pure-shell surefire-report parser repeated the "[0-9]+" literal 4 times across the tests/failures/errors/skipped extractions. Factored into a small _sf_attr() helper backed by one shared _SF_DIGITS pattern. Verified against the same fabricated pass/fail/truncated report fixtures used to validate the original parser — identical output. --- run_tests_parallel.sh | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/run_tests_parallel.sh b/run_tests_parallel.sh index 82a0fae3ed..83b6022d97 100755 --- a/run_tests_parallel.sh +++ b/run_tests_parallel.sh @@ -469,6 +469,14 @@ fi # swallowing failures), so any failure/error recorded in the reports fails the run # regardless of what the shards returned. A file with no parseable # root (truncated: JVM killed mid-write) counts as broken. +_SF_DIGITS='[0-9]+' # shared pattern so the digit-match regex isn't repeated per attribute + +# _sf_attr : print the integer value of the first attr="N" match, or +# nothing if the attribute isn't present. +_sf_attr() { + printf '%s' "$1" | grep -oE "${2}=\"${_SF_DIGITS}\"" | head -1 | grep -oE "$_SF_DIGITS" +} + SF_TOTAL=0; SF_FAIL=0; SF_ERR=0; SF_SKIP=0; SF_BROKEN=0 SF_BAD=() _sf_files=$(find obp-api/target/surefire-reports obp-commons/target/surefire-reports \ @@ -480,15 +488,15 @@ while IFS= read -r _f; do # the same-looking text inside CDATA. Attributes may be split across # lines, so grab the first match of each independently (attribute-order-agnostic). _head=$(head -c 8000 "$_f" 2>/dev/null) - _t=$(printf '%s' "$_head" | grep -oE 'tests="[0-9]+"' | head -1 | grep -oE '[0-9]+') + _t=$(_sf_attr "$_head" tests) if [[ -z "$_t" ]]; then SF_BROKEN=$((SF_BROKEN+1)) SF_BAD+=("$(basename "$_f"): UNPARSEABLE report (JVM killed mid-write?)") continue fi - _fa=$(printf '%s' "$_head" | grep -oE 'failures="[0-9]+"' | head -1 | grep -oE '[0-9]+'); _fa=${_fa:-0} - _e=$(printf '%s' "$_head" | grep -oE 'errors="[0-9]+"' | head -1 | grep -oE '[0-9]+'); _e=${_e:-0} - _sk=$(printf '%s' "$_head" | grep -oE 'skipped="[0-9]+"' | head -1 | grep -oE '[0-9]+'); _sk=${_sk:-0} + _fa=$(_sf_attr "$_head" failures); _fa=${_fa:-0} + _e=$(_sf_attr "$_head" errors); _e=${_e:-0} + _sk=$(_sf_attr "$_head" skipped); _sk=${_sk:-0} SF_TOTAL=$((SF_TOTAL+_t)); SF_FAIL=$((SF_FAIL+_fa)); SF_ERR=$((SF_ERR+_e)); SF_SKIP=$((SF_SKIP+_sk)) if [[ $_fa -ne 0 || $_e -ne 0 ]]; then SF_BAD+=("$(basename "$_f" | sed 's/^TEST-//; s/\.xml$//'): $_fa failed, $_e errors") From 754b7cc3b650419da3725b8108d4bca156e0c08f Mon Sep 17 00:00:00 2001 From: Hongwei Date: Fri, 10 Jul 2026 10:43:15 +0200 Subject: [PATCH 14/14] fix: address SonarCloud S7679/S7682 in _sf_attr helper - S7679: assign positional parameters ($1, $2) to local variables instead of using them directly in the function body. - S7682: add an explicit return statement at the end of the function. Verified: full local suite still reports ALL SHARDS PASSED (2926 tests, 0 failures, 0 errors). --- run_tests_parallel.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/run_tests_parallel.sh b/run_tests_parallel.sh index 83b6022d97..4f8e687328 100755 --- a/run_tests_parallel.sh +++ b/run_tests_parallel.sh @@ -474,7 +474,9 @@ _SF_DIGITS='[0-9]+' # shared pattern so the digit-match regex isn't repeated p # _sf_attr : print the integer value of the first attr="N" match, or # nothing if the attribute isn't present. _sf_attr() { - printf '%s' "$1" | grep -oE "${2}=\"${_SF_DIGITS}\"" | head -1 | grep -oE "$_SF_DIGITS" + local head="$1" attr="$2" + printf '%s' "$head" | grep -oE "${attr}=\"${_SF_DIGITS}\"" | head -1 | grep -oE "$_SF_DIGITS" + return $? } SF_TOTAL=0; SF_FAIL=0; SF_ERR=0; SF_SKIP=0; SF_BROKEN=0