From baf8f9802ca67a0cfe7bc9d184cf7f6d5c1304c3 Mon Sep 17 00:00:00 2001 From: hongwei Date: Wed, 8 Nov 2023 15:13:47 +0100 Subject: [PATCH] feature/use our own cache method - use on redis and tweak the error handling --- .../src/main/scala/code/api/cache/Redis.scala | 13 ++++++------ .../code/api/util/RateLimitingUtil.scala | 20 +------------------ .../scala/code/api/v3_1_0/APIMethods310.scala | 5 +++-- .../scala/code/api/v3_1_0/RateLimitTest.scala | 14 +++++++------ .../code/api/v4_0_0/RateLimitingTest.scala | 19 +++++++++--------- 5 files changed, 29 insertions(+), 42 deletions(-) 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 803a9388e9..d4e4a5faf1 100644 --- a/obp-api/src/main/scala/code/api/cache/Redis.scala +++ b/obp-api/src/main/scala/code/api/cache/Redis.scala @@ -22,14 +22,15 @@ object Redis extends MdcLoggable { def isRedisAvailable() = { try { - val key = "OBP_Check_isRedisAvailable" - jedis.connect() - jedis.set(key, "10") - jedis.exists(key) == true + val status = jedis.isConnected + if (!status) { + logger.warn("------------| Redis is not connected|------------") + } + status } catch { case e: Throwable => - logger.warn("------------| Redis.isRedisAvailable |------------") - logger.warn(e) + logger.error("------------| Redis throw exception|------------") + logger.error(e) false } } diff --git a/obp-api/src/main/scala/code/api/util/RateLimitingUtil.scala b/obp-api/src/main/scala/code/api/util/RateLimitingUtil.scala index d1c36fc335..5e780d9243 100644 --- a/obp-api/src/main/scala/code/api/util/RateLimitingUtil.scala +++ b/obp-api/src/main/scala/code/api/util/RateLimitingUtil.scala @@ -1,6 +1,7 @@ package code.api.util import code.api.APIFailureNewStyle +import code.api.cache.Redis.{isRedisAvailable, jedis} import code.api.util.APIUtil.fullBoxOrException import code.api.util.ErrorMessages.TooManyRequests import code.api.util.RateLimitingJson.CallLimit @@ -70,28 +71,9 @@ object RateLimitingJson { object RateLimitingUtil extends MdcLoggable { import code.api.util.RateLimitingPeriod._ - - val port = APIUtil.getPropsAsIntValue("redis_port", 6379) - val url = APIUtil.getPropsValue("redis_address", "127.0.0.1") def useConsumerLimits = APIUtil.getPropsAsBoolValue("use_consumer_limits", false) - lazy val jedis = new Jedis(url, port) - - def isRedisAvailable() = { - try { - val key = "OBP_Check_isRedisAvailable" - jedis.connect() - jedis.set(key, "10") - jedis.exists(key) == true - } catch { - case e : Throwable => - logger.warn("------------| RateLimitUtil.isRedisAvailable |------------") - logger.warn(e) - false - } - } - private def createUniqueKey(consumerKey: String, period: LimitCallPeriod) = consumerKey + RateLimitingPeriod.toString(period) private def underConsumerLimits(consumerKey: String, period: LimitCallPeriod, limit: Long): Boolean = { diff --git a/obp-api/src/main/scala/code/api/v3_1_0/APIMethods310.scala b/obp-api/src/main/scala/code/api/v3_1_0/APIMethods310.scala index ba3997e138..21730c908b 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/APIMethods310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/APIMethods310.scala @@ -2,12 +2,13 @@ package code.api.v3_1_0 import code.api.Constant import code.api.Constant.{SYSTEM_OWNER_VIEW_ID, localIdentityProvider} + import java.text.SimpleDateFormat import java.util.UUID import java.util.regex.Pattern - import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON._ import code.api.ResourceDocs1_4_0.{MessageDocsSwaggerDefinitions, ResourceDocsAPIMethodsUtil, SwaggerDefinitionsJSON, SwaggerJSONFactory} +import code.api.cache.Redis import code.api.util.APIUtil.{getWebUIPropsPairs, _} import code.api.util.ApiRole._ import code.api.util.ApiTag._ @@ -1242,7 +1243,7 @@ trait APIMethods310 { for { (_, callContext) <- anonymousAccess(cc) rateLimiting <- NewStyle.function.tryons("", 400, callContext) { - val isRedisAvailable = RateLimitingUtil.isRedisAvailable() + val isRedisAvailable = Redis.isRedisAvailable() val isActive = if (RateLimitingUtil.useConsumerLimits && isRedisAvailable) true else false RateLimiting(RateLimitingUtil.useConsumerLimits, "REDIS", isRedisAvailable, isActive) } diff --git a/obp-api/src/test/scala/code/api/v3_1_0/RateLimitTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/RateLimitTest.scala index 2c0f4a832a..f89718c257 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/RateLimitTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/RateLimitTest.scala @@ -25,6 +25,8 @@ TESOBE (http://www.tesobe.com/) */ package code.api.v3_1_0 +import code.api.cache.Redis + import java.time.format.DateTimeFormatter import java.time.{ZoneId, ZonedDateTime} import java.util.Date @@ -176,7 +178,7 @@ class RateLimitTest extends V310ServerSetup with PropsReset { response310.body.extract[CallLimitJson] } scenario("We will set calls limit per second for a Consumer", ApiEndpoint, VersionOfApi) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v3.1.0 with a Role " + ApiRole.canSetCallLimits) val Some((c, _)) = user1 val consumerId = Consumers.consumers.vend.getConsumerByConsumerKey(c.key).map(_.consumerId.get).getOrElse("") @@ -201,7 +203,7 @@ class RateLimitTest extends V310ServerSetup with PropsReset { Consumers.consumers.vend.updateConsumerCallLimits(id, Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1")) } scenario("We will set calls limit per minute for a Consumer", ApiEndpoint, VersionOfApi) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v3.1.0 with a Role " + ApiRole.canSetCallLimits) val Some((c, _)) = user1 val consumerId = Consumers.consumers.vend.getConsumerByConsumerKey(c.key).map(_.consumerId.get).getOrElse("") @@ -226,7 +228,7 @@ class RateLimitTest extends V310ServerSetup with PropsReset { Consumers.consumers.vend.updateConsumerCallLimits(id, Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1")) } scenario("We will set calls limit per hour for a Consumer", ApiEndpoint, VersionOfApi) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v3.1.0 with a Role " + ApiRole.canSetCallLimits) val Some((c, _)) = user1 val consumerId = Consumers.consumers.vend.getConsumerByConsumerKey(c.key).map(_.consumerId.get).getOrElse("") @@ -251,7 +253,7 @@ class RateLimitTest extends V310ServerSetup with PropsReset { Consumers.consumers.vend.updateConsumerCallLimits(id, Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1")) } scenario("We will set calls limit per day for a Consumer", ApiEndpoint, VersionOfApi) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v3.1.0 with a Role " + ApiRole.canSetCallLimits) val Some((c, _)) = user1 val consumerId = Consumers.consumers.vend.getConsumerByConsumerKey(c.key).map(_.consumerId.get).getOrElse("") @@ -276,7 +278,7 @@ class RateLimitTest extends V310ServerSetup with PropsReset { Consumers.consumers.vend.updateConsumerCallLimits(id, Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1")) } scenario("We will set calls limit per week for a Consumer", ApiEndpoint, VersionOfApi) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v3.1.0 with a Role " + ApiRole.canSetCallLimits) val Some((c, _)) = user1 val consumerId = Consumers.consumers.vend.getConsumerByConsumerKey(c.key).map(_.consumerId.get).getOrElse("") @@ -301,7 +303,7 @@ class RateLimitTest extends V310ServerSetup with PropsReset { Consumers.consumers.vend.updateConsumerCallLimits(id, Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1"), Some("-1")) } scenario("We will set calls limit per month for a Consumer", ApiEndpoint, VersionOfApi) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v3.1.0 with a Role " + ApiRole.canSetCallLimits) val Some((c, _)) = user1 val consumerId = Consumers.consumers.vend.getConsumerByConsumerKey(c.key).map(_.consumerId.get).getOrElse("") diff --git a/obp-api/src/test/scala/code/api/v4_0_0/RateLimitingTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/RateLimitingTest.scala index 73ffe73e58..dc2c0b25a0 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/RateLimitingTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/RateLimitingTest.scala @@ -25,6 +25,7 @@ TESOBE (http://www.tesobe.com/) */ package code.api.v4_0_0 +import code.api.cache.Redis import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole.{CanSetCallLimits, canCreateDynamicEndpoint} import code.api.util.ErrorMessages.{UserHasMissingRoles, UserNotLoggedIn} @@ -92,7 +93,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { feature("Rate Limit - " + ApiCallsLimit + " - " + ApiVersion400) { scenario("We will try to set Rate Limiting per minute for a Consumer - unauthorized access", ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0") val response400 = setRateLimitingAnonymousAccess(callLimitJsonInitial) Then("We should get a 401") @@ -101,7 +102,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { response400.body.extract[ErrorMessage].message should equal (UserNotLoggedIn) } scenario("We will try to set Rate Limiting per minute without a proper Role " + ApiRole.canSetCallLimits, ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0 without a Role " + ApiRole.canSetCallLimits) val response400 = setRateLimitingWithoutRole(user1, callLimitJsonInitial) Then("We should get a 403") @@ -110,7 +111,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { response400.body.extract[ErrorMessage].message should equal (UserHasMissingRoles + CanSetCallLimits) } scenario("We will try to set Rate Limiting per minute with a proper Role " + ApiRole.canSetCallLimits, ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0 with a Role " + ApiRole.canSetCallLimits) val response400 = setRateLimiting(user1, callLimitJsonInitial) Then("We should get a 200") @@ -118,7 +119,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { response400.body.extract[CallLimitJsonV400] } scenario("We will set Rate Limiting per second for an Endpoint", ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0 with a Role " + ApiRole.canSetCallLimits) val response01 = setRateLimiting(user1, callLimitJsonSecond) Then("We should get a 200") @@ -141,7 +142,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { response04.code should equal(200) } scenario("We will set Rate Limiting per minute for an Endpoint", ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0 with a Role " + ApiRole.canSetCallLimits) val response01 = setRateLimiting(user1, callLimitJsonMinute) Then("We should get a 200") @@ -163,7 +164,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { response04.code should equal(200) } scenario("We will set Rate Limiting per hour for an Endpoint", ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0 with a Role " + ApiRole.canSetCallLimits) val response01 = setRateLimiting(user1, callLimitJsonHour) Then("We should get a 200") @@ -185,7 +186,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { response04.code should equal(200) } scenario("We will set Rate Limiting per week for an Endpoint", ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0 with a Role " + ApiRole.canSetCallLimits) val response01 = setRateLimiting(user1, callLimitJsonWeek) Then("We should get a 200") @@ -207,7 +208,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { response04.code should equal(200) } scenario("We will set Rate Limiting per month for an Endpoint", ApiCallsLimit, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0 with a Role " + ApiRole.canSetCallLimits) val response01 = setRateLimiting(user1, callLimitJsonMonth) Then("We should get a 200") @@ -232,7 +233,7 @@ class RateLimitingTest extends V400ServerSetup with PropsReset { feature(s"Dynamic Endpoint: test $ApiCreateDynamicEndpoint version $ApiVersion400 - authorized access - with role - should be success!") { scenario("We will call the endpoint with user credentials", ApiCreateDynamicEndpoint, ApiVersion400) { - assume(RateLimitingUtil.isRedisAvailable()) + assume(Redis.isRedisAvailable()) When("We make a request v4.0.0") val postDynamicEndpointRequestBodyExample = ExampleValue.dynamicEndpointRequestBodyExample When("We make a request v4.0.0")