From 42e4293e13886e6150a595135d766c8092636739 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sun, 16 Aug 2026 09:17:20 +0200 Subject: [PATCH] fix: drop CallContext from the endpoint-mapping and FX rate cache keys CacheKeyFromArguments renders every parameter that is not annotated @CacheKeyOmit. CallContext carries per-request state (startTime, correlationId, url, verb, ipAddress, user), so both keys were unique per request: the cache could never hit, and getCurrentFxRateCached wrote a fresh Redis entry per call that lived out its TTL. getEndpointMappings additionally cached the (mappings, callContext) tuple. chill/Kryo cannot encode the lambda reachable through CallContext.resourceDocument, so every write failed and cachePut swallowed it as "result served uncached" - endpointMapping.cache.ttl.seconds bought nothing but a WARN per call. A hit would also have handed the caller the originating request's CallContext. Split the memoized half into getEndpointMappingsCached(bankId) rather than annotating callContext on the caller: CacheKeyFromArguments reads the parameters of the method whose body ends in buildCacheKey, so binding the result to a val first leaves it with no parameters and it emits Nil.mkString("_") - an empty argument segment, i.e. every bankId sharing one entry. Verified with javap that the key now renders bankId :: Nil, and that getCurrentFxRateCached renders bankId :: from :: to :: Nil. Add invalidateEndpointMappingCache() on create/update/delete, mirroring invalidateMethodRoutingCache: while callContext was in the key nothing could hit, so a stale entry was unreachable by construction; now that the cache works, writes have to publish themselves. --- .../main/scala/code/api/util/NewStyle.scala | 56 +++++++++++++++++-- .../LocalMappedConnectorInternal.scala | 11 +++- 2 files changed, 59 insertions(+), 8 deletions(-) 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 62809729ac..5380c8372a 100644 --- a/obp-api/src/main/scala/code/api/util/NewStyle.scala +++ b/obp-api/src/main/scala/code/api/util/NewStyle.scala @@ -3341,7 +3341,9 @@ object NewStyle extends MdcLoggable{ def createOrUpdateEndpointMapping(bankId: Option[String], endpointMapping: EndpointMappingT, callContext: Option[CallContext]) = { validateBankId(bankId, callContext) Future { - (EndpointMappingProvider.endpointMappingProvider.vend.createOrUpdate(bankId, endpointMapping), callContext) + val result = EndpointMappingProvider.endpointMappingProvider.vend.createOrUpdate(bankId, endpointMapping) + invalidateEndpointMappingCache() + (result, callContext) } map { i => (connectorEmptyResponse(i._1, callContext), i._2) } @@ -3350,12 +3352,31 @@ object NewStyle extends MdcLoggable{ def deleteEndpointMapping(bankId: Option[String], endpointMappingId: String, callContext: Option[CallContext]) = { validateBankId(bankId, callContext) Future { - (EndpointMappingProvider.endpointMappingProvider.vend.delete(bankId, endpointMappingId), callContext) + val result = EndpointMappingProvider.endpointMappingProvider.vend.delete(bankId, endpointMappingId) + invalidateEndpointMappingCache() + (result, callContext) } map { i => (connectorEmptyResponse(i._1, callContext), i._2) } } + /** + * Drop every memoized `getEndpointMappings(...)` entry after a mapping is created, + * updated, or deleted, so the change takes effect on the next request instead of waiting + * out `endpointMapping.cache.ttl.seconds`. Mirrors invalidateMethodRoutingCache: the + * memoize key embeds the literal method name, so one pattern delete clears every bankId + * variant. The pattern is a prefix of the actual name the macro renders + * (`getEndpointMappingsCached`), which is what makes one wildcard cover it. No-op / logged + * when Redis is unavailable (deleteKeysByPattern swallows and returns 0). + * + * This became necessary with the cache key fix below. While callContext was part of the + * key nothing could ever hit, so a stale entry was unreachable by construction; now that + * the cache works, writes have to publish themselves. + */ + private def invalidateEndpointMappingCache(): Unit = { + Redis.deleteKeysByPattern("*getEndpointMappings*") + } + def getEndpointMappingById(bankId: Option[String], endpointMappingId : String, callContext: Option[CallContext]): OBPReturnType[EndpointMappingT] = { validateBankId(bankId, callContext) @@ -3378,18 +3399,41 @@ object NewStyle extends MdcLoggable{ private[this] val endpointMappingTTL = APIUtil.getPropsValue(s"endpointMapping.cache.ttl.seconds", "0").toInt - def getEndpointMappings(bankId: Option[String], callContext: Option[CallContext]): OBPReturnType[List[EndpointMappingT]] = Future{ + /** + * The memoized half of getEndpointMappings, split into its own method for two reasons. + * + * Neither the key nor the cached value may mention the callContext. The key, because + * CacheKeyFromArguments renders every un-annotated parameter and CallContext carries + * per-request state (startTime, correlationId, url, verb, ipAddress, user) - keying on it + * made the key unique per request, so the cache could never hit. The value, because a hit + * would hand the caller the originating request's CallContext, and because chill/Kryo + * cannot encode the lambda reachable through CallContext.resourceDocument: every write of + * the old (mappings, callContext) tuple failed and cachePut swallowed it as "result served + * uncached", so endpointMapping.cache.ttl.seconds bought nothing but a WARN per call. + * + * A parameter-less signature rather than `@CacheKeyOmit callContext` on the caller, because + * CacheKeyFromArguments reads the parameters of the method whose body ENDS in buildCacheKey. + * Binding the result to a val first (`val x = buildCacheKey {...}; (x, callContext)`) leaves + * the macro with no parameters to render and it emits `Nil.mkString("_")` - an empty + * argument segment, i.e. every bankId sharing one entry. Keep buildCacheKey as the tail + * expression here; `NewStyle.function.getEndpointMappings` is verified by javap to render + * `bankId :: Nil`. + */ + private def getEndpointMappingsCached(bankId: Option[String]): List[EndpointMappingT] = { import scala.concurrent.duration._ - validateBankId(bankId, callContext) - var cacheKey = (randomUUID().toString, randomUUID().toString, randomUUID().toString) CacheKeyFromArguments.buildCacheKey { Caching.memoizeSyncWithProvider(Some(cacheKey.toString()))(endpointMappingTTL.second) { - {(EndpointMappingProvider.endpointMappingProvider.vend.getAllEndpointMappings(bankId), callContext)} + EndpointMappingProvider.endpointMappingProvider.vend.getAllEndpointMappings(bankId) } } } + + def getEndpointMappings(bankId: Option[String], callContext: Option[CallContext]): OBPReturnType[List[EndpointMappingT]] = Future{ + validateBankId(bankId, callContext) + (getEndpointMappingsCached(bankId), callContext) + } /** * Invalidate the Redis-backed resource-doc caches whose contents include diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala index 9d05ca346c..0467216cd9 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala @@ -32,7 +32,7 @@ import com.openbankproject.commons.model._ import com.openbankproject.commons.model.enums.ChallengeType.OBP_TRANSACTION_REQUEST_CHALLENGE import com.openbankproject.commons.model.enums.TransactionRequestTypes._ import com.openbankproject.commons.model.enums.{TransactionRequestStatus, _} -import com.tesobe.CacheKeyFromArguments +import com.tesobe.{CacheKeyFromArguments, CacheKeyOmit} import net.liftweb.common._ import org.json4s.JsonAST.JValue import org.json4s.native.Serialization.write @@ -478,7 +478,14 @@ object LocalMappedConnectorInternal extends MdcLoggable { Full(cardList) } - def getCurrentFxRateCached(bankId: BankId, fromCurrencyCode: String, toCurrencyCode: String, callContext: Option[CallContext]): Box[FXRate] = { + // @CacheKeyOmit on callContext: the rate depends on the bank and the currency pair only, but + // CacheKeyFromArguments renders every un-annotated parameter into the key, and CallContext + // carries per-request state (startTime, correlationId, url, verb, ipAddress, user). Keying on + // it made the key unique per request: the cache could never hit, and every call wrote a fresh + // Redis entry that lived out code.fx.exchangeRate.cache.ttl.seconds. The generated connectors + // have always annotated their callContext (see ConnectorBuilderUtil); this hand-written site + // simply never did. + def getCurrentFxRateCached(bankId: BankId, fromCurrencyCode: String, toCurrencyCode: String, @CacheKeyOmit callContext: Option[CallContext]): Box[FXRate] = { /** * Please note that "var cacheKey = (randomUUID().toString, randomUUID().toString, randomUUID().toString)" * is just a temporary value field with UUID values in order to prevent any ambiguity.