Cleaner fetchRealm; add test

BlueBrain · Oct 5, 2023 · 5569c8e · 5569c8e
1 parent b862700
commit 5569c8e
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 21 deletions.
diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImpl.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImpl.scala
@@ -2,7 +2,7 @@ package ch.epfl.bluebrain.nexus.delta.sdk.identities
 
 import akka.http.scaladsl.model.headers.{Authorization, OAuth2BearerToken}
 import akka.http.scaladsl.model.{HttpRequest, StatusCodes, Uri}
-import cats.data.NonEmptySet
+import cats.data.{NonEmptySet, OptionT}
 import cats.effect.IO
 import cats.syntax.all._
 import ch.epfl.bluebrain.nexus.delta.kernel.Logger
@@ -62,13 +62,10 @@ class IdentitiesImpl private[identities] (
       )
     }
 
-    def fetchRealm(parsedToken: ParsedToken): IO[Realm] =
-      realm
-        .getOrElseUpdate(parsedToken.rawToken, findActiveRealm(parsedToken.issuer))
-        .flatMap {
-          case Some(realm) => IO.pure(realm)
-          case None        => IO.raiseError(UnknownAccessTokenIssuer)
-        }
+    def fetchRealm(parsedToken: ParsedToken): IO[Realm] = {
+      val getRealm = realm.getOrElseAttemptUpdate(parsedToken.rawToken, findActiveRealm(parsedToken.issuer))
+      OptionT(getRealm).getOrRaise(UnknownAccessTokenIssuer)
+    }
 
     def fetchGroups(parsedToken: ParsedToken, realm: Realm): IO[Set[Group]] = {
       parsedToken.groups
@@ -103,7 +100,7 @@ class IdentitiesImpl private[identities] (
 object IdentitiesImpl {
 
   type GroupsCache = LocalCache[String, Set[Group]]
-  type RealmCache  = LocalCache[String, Option[Realm]]
+  type RealmCache  = LocalCache[String, Realm]
 
   private val logger = Logger.cats[this.type]
 
@@ -143,7 +140,7 @@ object IdentitiesImpl {
     */
   def apply(realms: Realms, hc: HttpClient, config: CacheConfig): IO[Identities] = {
     val groupsCache = LocalCache[String, Set[Group]](config)
-    val realmCache  = LocalCache[String, Option[Realm]](config)
+    val realmCache  = LocalCache[String, Realm](config)
 
     val findActiveRealm: String => IO[Option[Realm]] = { (issuer: String) =>
       val pagination = FromPagination(0, 1000)

diff --git a/...sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImplSuite.scala b/...sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImplSuite.scala
@@ -4,6 +4,7 @@ import akka.http.scaladsl.model.headers.OAuth2BearerToken
 import akka.http.scaladsl.model.{HttpRequest, Uri}
 import cats.data.NonEmptySet
 import cats.effect.IO
+import cats.effect.concurrent.Ref
 import cats.implicits._
 import ch.epfl.bluebrain.nexus.delta.kernel.cache.LocalCache
 import ch.epfl.bluebrain.nexus.delta.sdk.generators.{RealmGen, WellKnownGen}
@@ -109,6 +110,8 @@ class IdentitiesImplSuite extends CatsEffectSuite with TestHelpers with IOFromMa
       keys = Set(parser.parse(rsaKey.toPublicJWK.toJSONString).rightValue)
     )
 
+  type FindRealm = String => IO[Option[Realm]]
+
   private val findActiveRealm: String => IO[Option[Realm]] = ioFromMap[String, Realm](
     githubLabel.value  -> github,
     githubLabel2.value -> github2,
@@ -121,18 +124,21 @@ class IdentitiesImplSuite extends CatsEffectSuite with TestHelpers with IOFromMa
       (_: Uri) => HttpUnexpectedError(HttpRequest(), "Error while getting response")
     )(uri)
 
-  private val realmCache  = LocalCache[String, Option[Realm]]()
+  private val realmCache  = LocalCache[String, Realm]()
   private val groupsCache = LocalCache[String, Set[Group]]()
 
-  private val identitiesFromCaches: (RealmCache, GroupsCache) => Identities = (realmCache, groupsCache) =>
-    new IdentitiesImpl(
-      realmCache,
-      findActiveRealm,
-      (uri: Uri, _: OAuth2BearerToken) => userInfo(uri),
-      groupsCache
-    )
+  private val identitiesFromCaches: (RealmCache, GroupsCache) => FindRealm => Identities =
+    (realmCache, groupsCache) =>
+      findRealm =>
+        new IdentitiesImpl(
+          realmCache,
+          findRealm,
+          (uri: Uri, _: OAuth2BearerToken) => userInfo(uri),
+          groupsCache
+        )
 
-  private val identities = identitiesFromCaches(realmCache.unsafeRunSync(), groupsCache.unsafeRunSync())
+  private val identities =
+    identitiesFromCaches(realmCache.unsafeRunSync(), groupsCache.unsafeRunSync())(findActiveRealm)
 
   private val auth   = Authenticated(githubLabel)
   private val group1 = Group("group1", githubLabel)
@@ -346,10 +352,37 @@ class IdentitiesImplSuite extends CatsEffectSuite with TestHelpers with IOFromMa
       groups      <- groupsCache
       _           <- realm.get(parsedToken.rawToken).assertNone
       _           <- groups.get(parsedToken.rawToken).assertNone
-      _           <- identitiesFromCaches(realm, groups).exchange(token)
-      _           <- realm.get(parsedToken.rawToken).assertSome(Some(github))
+      _           <- identitiesFromCaches(realm, groups)(findActiveRealm).exchange(token)
+      _           <- realm.get(parsedToken.rawToken).assertSome(github)
       _           <- groups.get(parsedToken.rawToken).assertSome(Set(group3, group4))
     } yield ()
   }
 
+  test("Find active realm function should not run once value is cached") {
+    val token = generateToken(
+      subject = "Robert",
+      issuer = githubLabel,
+      rsaKey = rsaKey,
+      expires = nowPlus1h,
+      groups = Some(Set("group1", "group2"))
+    )
+
+    def findRealmOnce: Ref[IO, Boolean] => String => IO[Option[Realm]] = ref =>
+      _ =>
+        for {
+          flag <- ref.get
+          _    <- IO.raiseWhen(!flag)(new RuntimeException("Function executed more than once!"))
+          _    <- ref.set(false)
+        } yield Some(github)
+
+    for {
+      sem       <- Ref.of[IO, Boolean](true)
+      realm     <- realmCache
+      groups    <- groupsCache
+      identities = identitiesFromCaches(realm, groups)(findRealmOnce(sem))
+      _         <- identities.exchange(token)
+      _         <- identities.exchange(token)
+    } yield ()
+  }
+
 }