From 0ff8b093e6e88191a49923af67b2d71491d3fb8a Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 11:20:05 +0200 Subject: [PATCH 1/2] fix(cache): treat Redis deserialization failure as a cache miss Redis.deserialize returned the sentinel "NONE".asInstanceOf[T] when Kryo failed to decode the cached bytes (corrupt entry, class-shape change across a redeploy, Kryo registration drift). scalacache then treated that sentinel as a cache HIT and handed a String to callers expecting the real type, producing ClassCastException on every request until the TTL expired. Rethrow the decode failure instead: scalacache.TypedApi._caching treats a throwing cache read as a miss, recomputes the value from the source block, and repopulates the key with a fresh, valid serialization - the cache self-heals on the next call instead of staying poisoned. --- .../src/main/scala/code/api/cache/Redis.scala | 11 ++++- .../api/cache/RedisDeserializeMissTest.scala | 40 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 obp-api/src/test/scala/code/api/cache/RedisDeserializeMissTest.scala diff --git a/obp-api/src/main/scala/code/api/cache/Redis.scala b/obp-api/src/main/scala/code/api/cache/Redis.scala index 4b3a7477ca..74c37e69bd 100644 --- a/obp-api/src/main/scala/code/api/cache/Redis.scala +++ b/obp-api/src/main/scala/code/api/cache/Redis.scala @@ -310,8 +310,15 @@ object Redis extends MdcLoggable { tryDecode match { case Success(v) => v.asInstanceOf[T] case Failure(e) => - logger.error(e) - "NONE".asInstanceOf[T] + // Deserialization failed: corrupt bytes, a class-shape change across a redeploy, + // Kryo registration drift, etc. Returning a sentinel value cast to T poisons the + // cache - scalacache treats it as a HIT and hands e.g. a String to a caller + // expecting List[MethodRoutingT], throwing ClassCastException for the whole TTL. + // Rethrow instead: scalacache.TypedApi._caching treats a failed read as a cache + // miss, recomputes from the source block, and repopulates the key with a fresh, + // valid serialization (self-healing). + logger.error("Redis cache deserialization failed; treating as a cache miss and recomputing.", e) + throw e } } } diff --git a/obp-api/src/test/scala/code/api/cache/RedisDeserializeMissTest.scala b/obp-api/src/test/scala/code/api/cache/RedisDeserializeMissTest.scala new file mode 100644 index 0000000000..c44b3f1ec4 --- /dev/null +++ b/obp-api/src/test/scala/code/api/cache/RedisDeserializeMissTest.scala @@ -0,0 +1,40 @@ +package code.api.cache + +import org.scalatest.{FlatSpec, Matchers} + +/** + * Guards the cache self-healing contract of Redis.deserialize. + * + * The Kryo codec used for Redis-backed memoization must THROW when the cached + * bytes cannot be decoded (corrupt entry, class-shape change across a redeploy, + * Kryo registration drift). scalacache treats a throwing cache read as a MISS: + * it recomputes the value from the source block and repopulates the key, so the + * cache self-heals on the next call. + * + * The old behaviour returned the sentinel "NONE".asInstanceOf[T] instead, which + * scalacache treated as a valid HIT — every caller expecting the real type got a + * ClassCastException for the whole TTL. These tests fail if that sentinel ever + * comes back. + */ +class RedisDeserializeMissTest extends FlatSpec with Matchers { + + private def codec[T](implicit m: Manifest[T]) = Redis.anyToByte[T] + + "Redis codec deserialize" should "throw on undecodable bytes instead of returning a sentinel value" in { + val garbage: Array[Byte] = Array[Byte](0x7f, 0x00, 0x33, -1, 42, 9, 88, 0x11) + an[Exception] should be thrownBy codec[List[String]].deserialize(garbage) + } + + it should "never yield the legacy \"NONE\" sentinel for corrupt bytes" in { + val garbage: Array[Byte] = Array[Byte](-128, -1, -2, -3, 0, 1, 2, 3) + val outcome = scala.util.Try(codec[String].deserialize(garbage)) + outcome.isFailure shouldBe true + outcome.toOption should not be Some("NONE") + } + + it should "round-trip a value serialized by the same codec" in { + val value = List("mapped", "rest_vMar2019", "rabbitmq_vOct2024") + val bytes = codec[List[String]].serialize(value) + codec[List[String]].deserialize(bytes) shouldBe value + } +} From 10e779c916a03b9277de83ad965bdacbbf4dc6f4 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 7 Jul 2026 11:39:01 +0200 Subject: [PATCH 2/2] fix(routing): invalidate and enable the MethodRouting lookup cache The getMethodRoutings cache was shipped disabled (TTL default 0) because nothing invalidated it: createOrUpdateMethodRouting/deleteMethodRouting only touched the DB provider, so an enabled cache would serve stale routings for a full TTL after every /management/method_routings write. With the cache off, StarConnector re-queries the method_routing table on every intercepted connector call - up to twice per call - so a single API request issues dozens of identical getMethodRoutings queries. - Invalidate on write: createOrUpdateMethodRouting and deleteMethodRouting now pattern-delete '*getMethodRoutings*'. The scalacache memoize key embeds the literal method name, so one pattern delete clears all argument variants. - Enable the cache: methodRouting.cache.ttl.seconds now defaults to 30, guarded to 0 in test/dev mode (same pattern as dynamicEntityTTL) so tests that mutate routings and assert immediately are unaffected. - Document the prop in sample.props.template. The cache-invalidation and corrupt-entry self-healing behaviour is covered by MethodRoutingCacheInvalidationTest (cancelled when no Redis is reachable). This change depends on the previous commit: an enabled cache is only safe when a corrupt Redis entry heals as a miss instead of poisoning reads for the whole TTL. --- .../resources/props/sample.props.template | 4 +- .../main/scala/code/api/util/NewStyle.scala | 35 +++++++-- .../MethodRoutingCacheInvalidationTest.scala | 72 +++++++++++++++++++ 3 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 obp-api/src/test/scala/code/api/cache/MethodRoutingCacheInvalidationTest.scala diff --git a/obp-api/src/main/resources/props/sample.props.template b/obp-api/src/main/resources/props/sample.props.template index 82ba3abbfb..3c534c25a9 100644 --- a/obp-api/src/main/resources/props/sample.props.template +++ b/obp-api/src/main/resources/props/sample.props.template @@ -80,7 +80,9 @@ starConnector_supported_types=mapped,internal #this cache is used in api level, will cache whole endpoint : v121.getTransactionsForBankAccount #api.cache.ttl.seconds.APIMethods121.getTransactions=0 -## MethodRouting cache time-to-live in seconds +# TTL (seconds) for the MethodRouting lookup cache used by StarConnector on every connector +# call. 0 disables the cache (every connector call re-queries the DB). Writes via +# /management/method_routings invalidate the cache immediately, so this is a staleness backstop. #methodRouting.cache.ttl.seconds=30 ## EndpointMapping cache time-to-live in seconds diff --git a/obp-api/src/main/scala/code/api/util/NewStyle.scala b/obp-api/src/main/scala/code/api/util/NewStyle.scala index f05ac16782..62809729ac 100644 --- a/obp-api/src/main/scala/code/api/util/NewStyle.scala +++ b/obp-api/src/main/scala/code/api/util/NewStyle.scala @@ -5,7 +5,7 @@ import org.apache.pekko.http.scaladsl.model.HttpMethod import code.DynamicEndpoint.{DynamicEndpointProvider, DynamicEndpointT} import code.api.Constant.{SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID, SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_ID} // checkPaymentServerTypeError was inlined from the retired BG v1.3 PIS builder (see PaymentInitiationServicePISApi.scala) -import code.api.cache.Caching +import code.api.cache.{Caching, Redis} import code.api.dynamic.endpoint.helper.DynamicEndpointHelper import code.api.dynamic.entity.helper.{DynamicEntityHelper, DynamicEntityInfo} import code.api.util.APIUtil._ @@ -3283,11 +3283,27 @@ object NewStyle extends MdcLoggable{ } def createOrUpdateMethodRouting(methodRouting: MethodRoutingT) = Future { - MethodRoutingProvider.connectorMethodProvider.vend.createOrUpdate(methodRouting) + val result = MethodRoutingProvider.connectorMethodProvider.vend.createOrUpdate(methodRouting) + invalidateMethodRoutingCache() + result } def deleteMethodRouting(methodRoutingId: String) = Future { - MethodRoutingProvider.connectorMethodProvider.vend.delete(methodRoutingId) + val result = MethodRoutingProvider.connectorMethodProvider.vend.delete(methodRoutingId) + invalidateMethodRoutingCache() + result + } + + /** + * Drop every memoized `getMethodRoutings(...)` entry after a routing is created, + * updated, or deleted, so the change takes effect on the next connector call instead + * of waiting out `methodRouting.cache.ttl.seconds`. The scalacache/Redis memoize key + * for these entries embeds the literal method name `getMethodRoutings`, so a single + * pattern delete clears all argument variants. No-op / logged when Redis is + * unavailable (deleteKeysByPattern swallows and returns 0). + */ + private def invalidateMethodRoutingCache(): Unit = { + Redis.deleteKeysByPattern("*getMethodRoutings*") } def getMethodRoutingById(methodRoutingId : String, callContext: Option[CallContext]): OBPReturnType[MethodRoutingT] = { @@ -3298,7 +3314,18 @@ object NewStyle extends MdcLoggable{ } } - private[this] val methodRoutingTTL = APIUtil.getPropsValue(s"methodRouting.cache.ttl.seconds", "0").toInt + // Default 30s. MethodRouting rows change only via the /management/method_routings API, + // which now invalidates this cache on write (see invalidateMethodRoutingCache), so the + // TTL is only a backstop against missed invalidations rather than the propagation path. + // 30s is enough to collapse the per-request DB storm: StarConnector intercepts every + // connector call and resolves routings up to twice per call, so a single API request + // issues dozens of identical getMethodRoutings queries. + // Set methodRouting.cache.ttl.seconds=0 to disable (e.g. in tests that mutate routings + // and assert immediately without going through NewStyle). + private[this] val methodRoutingTTL = { + if(Props.testMode || Props.devMode) 0 + else APIUtil.getPropsValue(s"methodRouting.cache.ttl.seconds", "30").toInt + } def getMethodRoutings(methodName: Option[String], isBankIdExactMatch: Option[Boolean] = None, bankIdPattern: Option[String] = None): List[MethodRoutingT] = { import scala.concurrent.duration._ diff --git a/obp-api/src/test/scala/code/api/cache/MethodRoutingCacheInvalidationTest.scala b/obp-api/src/test/scala/code/api/cache/MethodRoutingCacheInvalidationTest.scala new file mode 100644 index 0000000000..f3d759b45b --- /dev/null +++ b/obp-api/src/test/scala/code/api/cache/MethodRoutingCacheInvalidationTest.scala @@ -0,0 +1,72 @@ +package code.api.cache + +import java.util.UUID + +import org.scalatest.{FlatSpec, Matchers} + +import scala.concurrent.duration._ + +/** + * Exercises the two cache behaviours the MethodRouting cache relies on, end-to-end + * against a real Redis (each scenario is cancelled via assume() when no Redis is + * reachable, so the suite is safe in environments without one): + * + * 1. Pattern invalidation: NewStyle.invalidateMethodRoutingCache() issues + * Redis.deleteKeysByPattern("*getMethodRoutings*"). The memoize key embeds the + * cacheKey argument verbatim, so seeding a key whose cacheKey contains the literal + * "getMethodRoutings" and pattern-deleting it must force the next read to recompute. + * + * 2. Self-healing on corrupt entries: overwriting a memoized key's bytes with garbage + * must NOT surface a sentinel/ClassCastException on the next read — the codec throws, + * scalacache treats the read as a miss, recomputes, and repopulates the key. + */ +class MethodRoutingCacheInvalidationTest extends FlatSpec with Matchers { + + private def memoize[A](cacheKey: String, ttl: Duration)(f: => A)(implicit m: Manifest[A]): A = + Caching.memoizeSyncWithProvider(Some(cacheKey))(ttl)(f) + + "deleteKeysByPattern(*getMethodRoutings*)" should "invalidate memoized entries so the next read recomputes" in { + assume(Redis.isRedisReady, "requires a reachable Redis") + val marker = s"inv-${UUID.randomUUID().toString}" + val cacheKey = s"(MethodRoutingCacheInvalidationTest,getMethodRoutings,$marker)" + var computations = 0 + def compute: List[String] = { computations += 1; List(s"value-$computations") } + + memoize(cacheKey, 30.seconds)(compute) shouldBe List("value-1") + memoize(cacheKey, 30.seconds)(compute) shouldBe List("value-1") + computations shouldBe 1 + + val deleted = Redis.deleteKeysByPattern(s"*$marker*") + deleted should be >= 1 + + memoize(cacheKey, 30.seconds)(compute) shouldBe List("value-2") + computations shouldBe 2 + } + + "a corrupted cache entry" should "behave as a miss: recompute once and repopulate with valid bytes" in { + assume(Redis.isRedisReady, "requires a reachable Redis") + val marker = s"poison-${UUID.randomUUID().toString}" + val cacheKey = s"(MethodRoutingCacheInvalidationTest,getMethodRoutings,$marker)" + var computations = 0 + def compute: List[String] = { computations += 1; List(s"value-$computations") } + + memoize(cacheKey, 30.seconds)(compute) shouldBe List("value-1") + computations shouldBe 1 + + val keys = Redis.scanKeys(s"*$marker*") + keys should not be empty + val jedis = Redis.jedisPool.getResource + try keys.foreach(k => jedis.set(k.getBytes("UTF-8"), Array[Byte](0x7f, -1, 3, 9, 42, 0, 0x11))) + finally jedis.close() + + // Corrupt read -> codec throws -> scalacache miss -> exactly one recompute, no sentinel/CCE. + memoize(cacheKey, 30.seconds)(compute) shouldBe List("value-2") + computations shouldBe 2 + + // The key was repopulated with valid bytes: the next read is a HIT again. + memoize(cacheKey, 30.seconds)(compute) shouldBe List("value-2") + computations shouldBe 2 + + Redis.deleteKeysByPattern(s"*$marker*") + } +}