test: endpoint sweeps and serialization contracts, and the defects they found

.github/workflows/build_container.yml

@@ -27,6 +27,9 @@ jobs:
   # --------------------------------------------------------------------------
   compile:
     runs-on: ubuntu-latest
+    # The test job has carried a timeout since it was written; compile and report never did,
+    # so a hung Maven resolve blocks the build until GitHub's own 6-hour ceiling.
+    timeout-minutes: 25
     steps:
       - uses: actions/checkout@v4
 
@@ -460,6 +463,7 @@ jobs:
     needs: test
     runs-on: ubuntu-latest
     if: always()
+    timeout-minutes: 10
     steps:
       - uses: actions/checkout@v4
 

.github/workflows/build_pull_request.yml

@@ -25,6 +25,9 @@ jobs:
   # --------------------------------------------------------------------------
   compile:
     runs-on: ubuntu-latest
+    # The test job has carried a timeout since it was written; compile and report never did,
+    # so a hung Maven resolve blocks the build until GitHub's own 6-hour ceiling.
+    timeout-minutes: 25
     steps:
       - uses: actions/checkout@v4
 
@@ -302,6 +305,17 @@ jobs:
           echo allow_user_generated_scala_code=true >> obp-api/src/main/resources/props/test.default.props
 
       - name: Run tests — shard ${{ matrix.shard }} (${{ matrix.name }})
+        env:
+          # This job has declared a redis service since it was written, but nothing ever failed
+          # when the service was absent: ConcurrentRateLimiterRaceTest and
+          # MethodRoutingCacheInvalidationTest each `assume` a reachable Redis and cancel
+          # otherwise, and a cancelled test reports as a pass. Dropping the services: block, or
+          # a container that never became healthy, would have taken the rate-limiter and
+          # cache-invalidation races out of the run without changing a single report.
+          #
+          # RedisTestTarget turns that cancellation into a failure wherever this is set.
+          # Developers leave it unset and keep the skip.
+          OBP_TEST_REDIS_REQUIRED: "true"
         run: |
           # wildcardSuites requires comma-separated package prefixes (-w per entry).
           # The YAML >- scalar collapses newlines to spaces, so we convert here.
@@ -453,6 +467,7 @@ jobs:
     needs: test
     runs-on: ubuntu-latest
     if: always()
+    timeout-minutes: 10
     steps:
       - uses: actions/checkout@v4
 

obp-api/src/main/scala/code/api/UKOpenBanking/v2_0_0/Http4sUKOBv200.scala

@@ -4,6 +4,7 @@ import cats.data.{Kleisli, OptionT}
 import cats.effect._
 import code.api.util.APIUtil.ResourceDoc
 import code.api.util.http4s.ResourceDocMiddleware
+import code.api.util.http4s.IdempotencyMiddleware
 import code.util.Helper.MdcLoggable
 import com.openbankproject.commons.util.ApiVersion
 import org.http4s._
@@ -33,5 +34,5 @@ object Http4sUKOBv200 extends MdcLoggable {
     Http4sUKOBv200AIS.routes(req)
   }
 
-  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(allRoutes)
+  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(IdempotencyMiddleware(allRoutes))
 }

obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310.scala

@@ -4,6 +4,7 @@ import cats.data.{Kleisli, OptionT}
 import cats.effect._
 import code.api.util.APIUtil.ResourceDoc
 import code.api.util.http4s.ResourceDocMiddleware
+import code.api.util.http4s.IdempotencyMiddleware
 import code.util.Helper.MdcLoggable
 import com.openbankproject.commons.util.ApiVersion
 import org.http4s._
@@ -74,5 +75,5 @@ object Http4sUKOBv310 extends MdcLoggable {
       .orElse(Http4sUKOBv310InternationalStandingOrders.routes(req))
   }
 
-  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(allRoutes)
+  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(IdempotencyMiddleware(allRoutes))
 }

obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401.scala

@@ -5,6 +5,7 @@ import cats.effect._
 import code.api.util.APIUtil
 import code.api.util.APIUtil.ResourceDoc
 import code.api.util.http4s.ResourceDocMiddleware
+import code.api.util.http4s.IdempotencyMiddleware
 import code.util.Helper.MdcLoggable
 import com.openbankproject.commons.util.ApiVersion
 import org.http4s._
@@ -62,5 +63,5 @@ object Http4sUKOBv401 extends MdcLoggable {
       routes(req).map(_.putHeaders(Header.Raw(fapiInteractionIdHeader, interactionId)))
     }
 
-  val wrappedRoutes: HttpRoutes[IO] = withFapiInteractionId(ResourceDocMiddleware.apply(resourceDocs)(allRoutes))
+  val wrappedRoutes: HttpRoutes[IO] = withFapiInteractionId(ResourceDocMiddleware.apply(resourceDocs)(IdempotencyMiddleware(allRoutes)))
 }

obp-api/src/main/scala/code/api/berlin/group/v1_3/Http4sBGv13.scala

@@ -5,6 +5,7 @@ import cats.effect._
 import code.api.berlin.group.ConstantsBG
 import code.api.util.APIUtil.ResourceDoc
 import code.api.util.http4s.ResourceDocMiddleware
+import code.api.util.http4s.IdempotencyMiddleware
 import code.util.Helper.MdcLoggable
 import org.http4s._
 
@@ -35,5 +36,5 @@ object Http4sBGv13 extends MdcLoggable {
       .orElse(Http4sBGv13SigningBaskets.routes(req))
   }
 
-  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(allRoutes)
+  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(IdempotencyMiddleware(allRoutes))
 }

obp-api/src/main/scala/code/api/berlin/group/v2/Http4sBGv2.scala

@@ -5,6 +5,7 @@ import cats.effect._
 import code.api.berlin.group.ConstantsBG
 import code.api.util.APIUtil.ResourceDoc
 import code.api.util.http4s.ResourceDocMiddleware
+import code.api.util.http4s.IdempotencyMiddleware
 import code.util.Helper.MdcLoggable
 import org.http4s._
 
@@ -27,5 +28,5 @@ object Http4sBGv2 extends MdcLoggable {
       .orElse(Http4sBGv2PIIS.routes(req))
   }
 
-  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(allRoutes)
+  val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(IdempotencyMiddleware(allRoutes))
 }

obp-api/src/main/scala/code/api/cache/Redis.scala

@@ -7,7 +7,7 @@ import code.util.Helper.MdcLoggable
 import com.openbankproject.commons.ExecutionContext.Implicits.global
 import redis.clients.jedis.{Jedis, JedisPool, JedisPoolConfig}
 import scalacache.memoization.{cacheKeyExclude, memoizeF, memoizeSync}
-import scalacache.{Cache, Flags}
+import scalacache.{Cache, CacheConfig, DefaultCacheKeyBuilder, Flags}
 import scalacache.redis.RedisCache
 import scalacache.serialization.{Codec, FailedToDecode}
 import redis.clients.jedis.{Jedis, JedisPool, JedisPoolConfig}
@@ -321,6 +321,50 @@ object Redis extends MdcLoggable {
   // building one per call only put two allocations in front of every cache read on the request
   // path. RedisCache is a thin wrapper over the pool built above and opens nothing of its own, so
   // the pool, its authentication and its SSL configuration stay shared.
+  /**
+   * The serialization identity these cached bytes were produced under.
+   *
+   * Cache entries are Kryo-encoded, and what Kryo produces depends on the Scala library and the
+   * chill build that encoded it. Two OBP-API versions compiled against different ones therefore
+   * write mutually unreadable bytes into the same keys -- and "unreadable" is the optimistic
+   * case. Measured across the 2.12 -> 2.13 migration: an EMPTY `List`, written by chill 0.9.3,
+   * decodes under 0.9.5 into a `scala.collection.immutable.Queue`. That decode SUCCEEDS. It is
+   * only at the call site, whose signature says `List`, that it fails --
+   *
+   *     class scala.collection.immutable.Queue cannot be cast to
+   *     class scala.collection.immutable.List
+   *
+   * -- so the caller gets a 500 rather than a cache miss, and gets it for the whole TTL, because
+   * a failed read does not evict the entry. Reproduced on `GET /management/dynamic-message-docs`
+   * and `GET /management/connector-methods`: 200 on 2.12, 500 on 2.13 reading 2.12's entry, and
+   * fine in either version on its own. That is a rolling upgrade, or any upgrade against a warm
+   * Redis.
+   *
+   * The migration note anticipated the risk and described the consequence as a cold cache. For
+   * values that fail to decode that is exactly right. This handles the ones that do not fail.
+   *
+   * Namespacing the key is the fix rather than casting defensively at each call site: there are
+   * eight `List`-returning memoized methods today, the same drift can hit any other type, and no
+   * amount of care at the call sites can make bytes already in Redis readable. Entries written by
+   * another version simply stop being addressable and age out on their own TTL.
+   *
+   * The Scala binary version is the axis that moved here and is the one derived automatically.
+   * `obp.cache.serialization.version` is for the case it does not cover -- a dependency upgrade
+   * that changes the encoding without changing the Scala version, which is what chill 0.9.3 to
+   * 0.9.5 would have been on its own. Bump it in that situation; the cost is one cold cache.
+   */
+  private val serializationNamespace: String = {
+    val scalaBinary = scala.util.Properties.versionNumberString.split('.').take(2).mkString(".")
+    val manual = APIUtil.getPropsValue("obp.cache.serialization.version", "1")
+    s"obpser$manual-scala$scalaBinary"
+  }
+
+  // Prefixing happens here, in the key builder, rather than at the call sites: scalacache derives
+  // the rest of the key from the enclosing method and its arguments, and every caller goes through
+  // it. `memoizeSync` and `memoizeF` both read this same implicit config.
+  implicit val cacheConfig: CacheConfig =
+    CacheConfig(cacheKeyBuilder = DefaultCacheKeyBuilder(keyPrefix = Some(serializationNamespace)))
+
   private val sharedCache: Cache[Any] = RedisCache[Any](jedisPool)
   private def cacheFor[A]: Cache[A] = sharedCache.asInstanceOf[Cache[A]]
 

obp-api/src/main/scala/code/api/sandbox/example_data/2016-04-28/example_import.json

@@ -64,7 +64,7 @@
       "currency":"GBP",
       "amount":"8084.32"
     },
-    "IBAN":"BA12 1234 5123 4513 7599 6969 977",
+    "IBAN":"BA463990000000000001",
     "owners":["robert.xuk.x@example.com"],
     "generate_public_view":false,
     "generate_accountants_view":true,
@@ -79,7 +79,7 @@
       "currency":"GBP",
       "amount":"8084.32"
     },
-    "IBAN":"BA12 1234 5123 4513 7599 6969 977",
+    "IBAN":"BA924990000000000001",
     "owners":["robert.yuk.y@example.com"],
     "generate_public_view":false,
     "generate_accountants_view":true,

obp-api/src/main/scala/code/api/sandbox/example_data/example_import.json

@@ -64,7 +64,7 @@
       "currency":"GBP",
       "amount":"6599.63"
     },
-    "IBAN":"BA12 1234 5123 4518 4490 1189 877",
+    "IBAN":"BA511990000000000001",
     "owners":["Susan.X.0.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -79,7 +79,7 @@
       "currency":"GBP",
       "amount":"6379.63"
     },
-    "IBAN":"BA12 1234 5123 4511 8754 4625 177",
+    "IBAN":"BA241990000000000002",
     "owners":["Robert.X.0.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -94,7 +94,7 @@
       "currency":"GBP",
       "amount":"7588.25"
     },
-    "IBAN":"BA12 1234 5123 4510 4337 1399 677",
+    "IBAN":"BA941990000000000003",
     "owners":["Anil.X.0.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -109,7 +109,7 @@
       "currency":"GBP",
       "amount":"6662.05"
     },
-    "IBAN":"BA12 1234 5123 4514 4440 2184 977",
+    "IBAN":"BA671990000000000004",
     "owners":["Robert.X.0.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -124,7 +124,7 @@
       "currency":"GBP",
       "amount":"3748.57"
     },
-    "IBAN":"BA12 1234 5123 4518 9534 3427 277",
+    "IBAN":"BA401990000000000005",
     "owners":["Ellie.X.0.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -139,7 +139,7 @@
       "currency":"GBP",
       "amount":"15860.50"
     },
-    "IBAN":"BA12 1234 5123 4512 1957 2301 577",
+    "IBAN":"BA131990000000000006",
     "owners":["Anil.X.0.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -154,7 +154,7 @@
       "currency":"GBP",
       "amount":"7724.41"
     },
-    "IBAN":"BA12 1234 5123 4512 6914 8586 977",
+    "IBAN":"BA831990000000000007",
     "owners":["Anil.X.0.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -169,7 +169,7 @@
       "currency":"GBP",
       "amount":"6599.63"
     },
-    "IBAN":"BA12 1234 5123 4518 4490 1189 877",
+    "IBAN":"BA972990000000000001",
     "owners":["Susan.Y.9.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -184,7 +184,7 @@
       "currency":"GBP",
       "amount":"6379.63"
     },
-    "IBAN":"BA12 1234 5123 4511 8754 4625 177",
+    "IBAN":"BA702990000000000002",
     "owners":["Robert.Y.9.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -199,7 +199,7 @@
       "currency":"GBP",
       "amount":"7588.25"
     },
-    "IBAN":"BA12 1234 5123 4510 4337 1399 677",
+    "IBAN":"BA432990000000000003",
     "owners":["Anil.Y.9.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -214,7 +214,7 @@
       "currency":"GBP",
       "amount":"6662.05"
     },
-    "IBAN":"BA12 1234 5123 4514 4440 2184 977",
+    "IBAN":"BA162990000000000004",
     "owners":["Robert.Y.9.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -229,7 +229,7 @@
       "currency":"GBP",
       "amount":"3748.57"
     },
-    "IBAN":"BA12 1234 5123 4518 9534 3427 277",
+    "IBAN":"BA862990000000000005",
     "owners":["Ellie.Y.9.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -244,7 +244,7 @@
       "currency":"GBP",
       "amount":"15860.50"
     },
-    "IBAN":"BA12 1234 5123 4512 1957 2301 577",
+    "IBAN":"BA592990000000000006",
     "owners":["Anil.Y.9.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,
@@ -259,7 +259,7 @@
       "currency":"GBP",
       "amount":"7724.41"
     },
-    "IBAN":"BA12 1234 5123 4512 6914 8586 977",
+    "IBAN":"BA322990000000000007",
     "owners":["Anil.Y.9.GH"],
     "generate_public_view":true,
     "generate_accountants_view":true,

obp-api/src/main/scala/code/api/util/DynamicUtil.scala

@@ -360,6 +360,26 @@ object DynamicUtil extends MdcLoggable{
 
   object Validation {
 
+    /**
+     * Turn the `dynamic_code_compile_validate_dependencies` props value into the Scala source
+     * that, once compiled, yields the whitelist.
+     *
+     * A named function rather than an inline expression so a test can drive the real thing.
+     * DynamicUtilTest used to hold a character-for-character copy of it, which meant the two
+     * could diverge with the test still green -- the copy was only kept in step here because
+     * whoever edited one happened to see the other. This is the only compile that happens
+     * reflectively at boot, so nothing at compile time would have caught the divergence either.
+     *
+     * `Map[String, String](` rather than `Map(`: the props default is an empty list, and a bare
+     * `Map()` leaves its type parameters undetermined, so the trailing `.toMap` cannot prove the
+     * elements are pairs and the reflective compilation fails. The `.toMap` is itself needed
+     * because `mapValues` returns a view rather than a Map on 2.13.
+     */
+    def dependenciesScalaCode(dependenciesString: String): String =
+      s"${DynamicUtil.importStatements}" +
+        dependenciesString.replaceFirst("\\[", "Map[String, String](").dropRight(1) +
+        ").mapValues(v => StringUtils.split(v, ',').map(_.trim).toSet).toMap"
+
     val dynamicCodeSandboxPermissions = APIUtil.getPropsValue("dynamic_code_sandbox_permissions", "[]").trim
     val scalaCodePermissioins = "List[java.security.Permission]"+dynamicCodeSandboxPermissions.replaceFirst("\\[","(").dropRight(1)+")"
     val permissions:Box[List[java.security.Permission]] = DynamicUtil.compileScalaCodeUnchecked(scalaCodePermissioins)
@@ -385,11 +405,7 @@ object DynamicUtil extends MdcLoggable{
     val allowedRuntimePermissions = permissions.openOrThrowException("Can not compile the props `dynamic_code_sandbox_permissions` to permissions")
 
     val dependenciesString = APIUtil.getPropsValue("dynamic_code_compile_validate_dependencies", "[]").trim
-    // `Map[String, String](` rather than `Map(`: the props default is an empty list, and a bare
-    // `Map()` leaves its type parameters undetermined, so the trailing .toMap cannot prove the
-    // elements are pairs and the reflective compilation fails. The .toMap itself is needed because
-    // mapValues returns a view rather than a Map.
-    val scalaCodeDependencies = s"${DynamicUtil.importStatements}"+dependenciesString.replaceFirst("\\[","Map[String, String](").dropRight(1) +").mapValues(v => StringUtils.split(v, ',').map(_.trim).toSet).toMap"
+    val scalaCodeDependencies = dependenciesScalaCode(dependenciesString)
     val dependenciesBox: Box[Map[String, Set[String]]] = DynamicUtil.compileScalaCodeUnchecked(scalaCodeDependencies)
 
     /**

obp-api/src/main/scala/code/api/util/ErrorMessages.scala

@@ -828,6 +828,7 @@ object ErrorMessages {
   val ChatMessageTypeNotAllowed = "OBP-39018: Invalid message_type. Allowed values: text, system."
   val SignalMessageTooLong = "OBP-39019: Signal message exceeds the maximum allowed length."
   val SignalMessageContainsDangerousCharacters = "OBP-39020: Signal message contains control or bidirectional-override characters, which are not allowed."
+  val SignalChannelNotFound = "OBP-39021: Signal Channel not found."
 
   // Transaction Request related messages (OBP-40XXX)
   val InvalidTransactionRequestType = "OBP-40001: Invalid value for TRANSACTION_REQUEST_TYPE"