Centralize type dispatch in ClientEvaluator, fix addApiHook singleton bypass, simplify TestFeatureProvider Hub (#109)

Author: EtaCassiopeia

core/src/main/scala/zio/openfeature/FeatureFlags.scala

@@ -159,6 +159,12 @@ trait FeatureFlags {
   def clearHooks: UIO[Unit]
   def hooks: UIO[List[FeatureHook]]
 
+  /** Add an API-level hook that applies to all clients sharing this instance's OpenFeatureAPI. */
+  def addApiHook(hook: dev.openfeature.sdk.Hook[_]): UIO[Unit]
+
+  /** Clear all API-level hooks on this instance's OpenFeatureAPI. */
+  def clearApiHooks: UIO[Unit]
+
   /** Replace the underlying provider at runtime.
     *
     * The old provider is shut down, the new provider is initialized, and the status transitions through `NotReady`
@@ -367,13 +373,11 @@ object FeatureFlags {
 
   // API-level Hooks (per OpenFeature spec 4.4.1)
 
-  /** Add an API-level hook that applies to all clients. */
-  def addApiHook(hook: dev.openfeature.sdk.Hook[_]): UIO[Unit] =
-    ZIO.succeed(OpenFeatureAPI.getInstance().addHooks(hook))
+  def addApiHook(hook: dev.openfeature.sdk.Hook[_]): ZIO[FeatureFlags, Nothing, Unit] =
+    ZIO.serviceWithZIO(_.addApiHook(hook))
 
-  /** Clear all API-level hooks. */
-  def clearApiHooks: UIO[Unit] =
-    ZIO.succeed(OpenFeatureAPI.getInstance().clearHooks())
+  def clearApiHooks: ZIO[FeatureFlags, Nothing, Unit] =
+    ZIO.serviceWithZIO(_.clearApiHooks)
 
   // Tracking API
 

core/src/main/scala/zio/openfeature/FeatureFlagsLive.scala

@@ -276,27 +276,6 @@ final private[openfeature] class FeatureFlagsLive(
       _          <- txState.record(eval)
     } yield resolution
 
-  // Evaluate a flag using a ClientEvaluator typeclass instance for type-safe SDK dispatch
-  private def evaluateViaTypeclass[A](
-    key: String,
-    default: A,
-    ofContext: dev.openfeature.sdk.EvaluationContext,
-    timeout: Option[Duration] = None
-  )(implicit ev: ClientEvaluator[A]): IO[FeatureFlagError, FlagResolution[A]] = {
-    val rawEval = ev.evaluate(client, key, default, ofContext)
-    val timedEval = timeout match {
-      case Some(d) =>
-        rawEval.disconnect
-          .timeoutFail(new java.util.concurrent.TimeoutException(s"Evaluation of '$key' timed out after $d"))(d)
-      case None => rawEval
-    }
-    timedEval
-      .mapError(e => FeatureFlagError.ProviderError(e))
-      .flatMap { details =>
-        toFlagResolution(key, details).map(_.copy(value = ev.extractValue(details)))
-      }
-  }
-
   private def evaluateFromClient[A: FlagType](
     key: String,
     default: A,
@@ -314,88 +293,78 @@ final private[openfeature] class FeatureFlagsLive(
         case None => effect
       }
 
-    val evaluation: IO[FeatureFlagError, FlagResolution[A]] = flagType.typeName match {
-      case "Boolean" =>
-        evaluateViaTypeclass(key, default.asInstanceOf[Boolean], ofContext, timeout)
-          .map(_.asInstanceOf[FlagResolution[A]])
-
-      case "String" =>
-        evaluateViaTypeclass(key, default.asInstanceOf[String], ofContext, timeout)
-          .map(_.asInstanceOf[FlagResolution[A]])
-
-      case "Int" =>
-        evaluateViaTypeclass(key, default.asInstanceOf[Int], ofContext, timeout)
-          .map(_.asInstanceOf[FlagResolution[A]])
-
-      case "Long" =>
-        evaluateViaTypeclass(key, default.asInstanceOf[Long], ofContext, timeout)
-          .map(_.asInstanceOf[FlagResolution[A]])
-
-      case "Float" =>
-        evaluateViaTypeclass(key, default.asInstanceOf[Float], ofContext, timeout)
-          .map(_.asInstanceOf[FlagResolution[A]])
-
-      case "Double" =>
-        evaluateViaTypeclass(key, default.asInstanceOf[Double], ofContext, timeout)
-          .map(_.asInstanceOf[FlagResolution[A]])
-
-      case "Object" =>
-        withTimeout(
-          ZIO.attemptBlocking {
-            val defaultValue = new dev.openfeature.sdk.Value(
-              dev.openfeature.sdk.Structure.mapToStructure(
-                default.asInstanceOf[Map[String, Any]].map { case (k, v) => k -> anyToObject(v) }.asJava
-              )
-            )
-            client.getObjectDetails(key, defaultValue, ofContext)
+    val evaluation: IO[FeatureFlagError, FlagResolution[A]] =
+      ClientEvaluator.evaluateStandard[A](flagType.typeName, client, key, default, ofContext) match {
+        case Some(erased) =>
+          val timedEval = timeout match {
+            case Some(d) =>
+              erased.task.disconnect
+                .timeoutFail(new java.util.concurrent.TimeoutException(s"Evaluation of '$key' timed out after $d"))(d)
+            case None => erased.task
           }
-        ).mapError(e => FeatureFlagError.ProviderError(e))
-          .flatMap { details =>
-            val value = valueToMap(details.getValue)
-            toFlagMetadata(details.getFlagMetadata).map { metadata =>
-              FlagResolution(
-                value = value.asInstanceOf[A],
-                variant = Option(details.getVariant),
-                reason = toResolutionReason(details.getReason),
-                metadata = metadata,
-                flagKey = key,
-                errorCode = Option(details.getErrorCode).map(ErrorCodeConverter.fromJava),
-                errorMessage = Option(details.getErrorMessage)
+          timedEval
+            .mapError(e => FeatureFlagError.ProviderError(e))
+            .flatMap { details =>
+              toFlagResolution(key, details).map(r => r.copy(value = erased.extract(details)))
+            }
+
+        case None if flagType.typeName == "Object" =>
+          withTimeout(
+            ZIO.attemptBlocking {
+              val defaultValue = new dev.openfeature.sdk.Value(
+                dev.openfeature.sdk.Structure.mapToStructure(
+                  default.asInstanceOf[Map[String, Any]].map { case (k, v) => k -> anyToObject(v) }.asJava
+                )
               )
+              client.getObjectDetails(key, defaultValue, ofContext)
+            }
+          ).mapError(e => FeatureFlagError.ProviderError(e))
+            .flatMap { details =>
+              val value = valueToMap(details.getValue)
+              toFlagMetadata(details.getFlagMetadata).map { metadata =>
+                FlagResolution(
+                  value = value.asInstanceOf[A],
+                  variant = Option(details.getVariant),
+                  reason = toResolutionReason(details.getReason),
+                  metadata = metadata,
+                  flagKey = key,
+                  errorCode = Option(details.getErrorCode).map(ErrorCodeConverter.fromJava),
+                  errorMessage = Option(details.getErrorMessage)
+                )
+              }
             }
-          }
 
-      case _ =>
-        // Custom type - try to decode from object
-        withTimeout(
-          ZIO.attemptBlocking {
-            client.getObjectDetails(key, new dev.openfeature.sdk.Value(), ofContext)
-          }
-        ).mapError(e => FeatureFlagError.ProviderError(e))
-          .flatMap { details =>
-            valueToAny(details.getValue) match {
-              case Some(rawValue) =>
-                flagType.decode(rawValue) match {
-                  case Right(decoded) =>
-                    toFlagMetadata(details.getFlagMetadata).map { metadata =>
-                      FlagResolution(
-                        value = decoded,
-                        variant = Option(details.getVariant),
-                        reason = toResolutionReason(details.getReason),
-                        metadata = metadata,
-                        flagKey = key,
-                        errorCode = Option(details.getErrorCode).map(ErrorCodeConverter.fromJava),
-                        errorMessage = Option(details.getErrorMessage)
-                      )
-                    }
-                  case Left(_) =>
-                    ZIO.fail(FeatureFlagError.TypeMismatch(key, flagType.typeName, "Object"))
-                }
-              case None =>
-                ZIO.fail(FeatureFlagError.TypeMismatch(key, flagType.typeName, "null"))
+        case None =>
+          // Custom type - try to decode from object
+          withTimeout(
+            ZIO.attemptBlocking {
+              client.getObjectDetails(key, new dev.openfeature.sdk.Value(), ofContext)
             }
-          }
-    }
+          ).mapError(e => FeatureFlagError.ProviderError(e))
+            .flatMap { details =>
+              valueToAny(details.getValue) match {
+                case Some(rawValue) =>
+                  flagType.decode(rawValue) match {
+                    case Right(decoded) =>
+                      toFlagMetadata(details.getFlagMetadata).map { metadata =>
+                        FlagResolution(
+                          value = decoded,
+                          variant = Option(details.getVariant),
+                          reason = toResolutionReason(details.getReason),
+                          metadata = metadata,
+                          flagKey = key,
+                          errorCode = Option(details.getErrorCode).map(ErrorCodeConverter.fromJava),
+                          errorMessage = Option(details.getErrorMessage)
+                        )
+                      }
+                    case Left(_) =>
+                      ZIO.fail(FeatureFlagError.TypeMismatch(key, flagType.typeName, "Object"))
+                  }
+                case None =>
+                  ZIO.fail(FeatureFlagError.TypeMismatch(key, flagType.typeName, "null"))
+              }
+            }
+      }
 
     // Check resolution error codes for provider-level failures (handles TOCTOU race
     // where checkProviderStatus passes but the Java SDK's internal state is stale)
@@ -716,6 +685,12 @@ final private[openfeature] class FeatureFlagsLive(
   override def hooks: UIO[List[FeatureHook]] =
     state.hooksRef.get
 
+  override def addApiHook(hook: dev.openfeature.sdk.Hook[_]): UIO[Unit] =
+    ZIO.succeed(api.addHooks(hook))
+
+  override def clearApiHooks: UIO[Unit] =
+    ZIO.succeed(api.clearHooks())
+
   // Provider hooks (spec: provider hooks included in hook pipeline)
 
   private def getProviderHooks: UIO[List[FeatureHook]] =

core/src/main/scala/zio/openfeature/internal/ClientEvaluator.scala

@@ -40,6 +40,40 @@ private[openfeature] trait ClientEvaluator[A] {
 
 private[openfeature] object ClientEvaluator {
 
+  /** A standard-type evaluation produced by [[evaluateStandard]], with the typed extractor pre-applied to the caller's
+    * `A`. The `asInstanceOf[A]` cast lives once here, not at each call site.
+    */
+  final case class Erased[A](
+    task: Task[FlagEvaluationDetails[_]],
+    extract: FlagEvaluationDetails[_] => A
+  )
+
+  /** Look up the evaluator for a standard type by name and produce the type-erased evaluation. Returns None for
+    * non-standard types (Object, custom) which need special handling.
+    */
+  def evaluateStandard[A](
+    typeName: String,
+    client: OFClient,
+    key: String,
+    default: A,
+    context: dev.openfeature.sdk.EvaluationContext
+  ): Option[Erased[A]] = {
+    def erased[T](ev: ClientEvaluator[T]): Erased[A] =
+      Erased[A](
+        ev.evaluate(client, key, default.asInstanceOf[T], context),
+        details => ev.extractValue(details).asInstanceOf[A]
+      )
+    typeName match {
+      case "Boolean" => Some(erased(booleanEvaluator))
+      case "String"  => Some(erased(stringEvaluator))
+      case "Int"     => Some(erased(intEvaluator))
+      case "Long"    => Some(erased(longEvaluator))
+      case "Float"   => Some(erased(floatEvaluator))
+      case "Double"  => Some(erased(doubleEvaluator))
+      case _         => None
+    }
+  }
+
   implicit val booleanEvaluator: ClientEvaluator[Boolean] = new ClientEvaluator[Boolean] {
     def evaluate(
       client: OFClient,

core/src/test/scala/zio/openfeature/ApiHookIsolationSpec.scala

@@ -0,0 +1,125 @@
+package zio.openfeature
+
+import dev.openfeature.sdk.{
+  EvaluationContext => OFEvaluationContext,
+  EventProvider,
+  HookContext => JavaHookContext,
+  Metadata,
+  OpenFeatureAPIFactory,
+  ProviderEvaluation,
+  ProviderState
+}
+import zio._
+import zio.test._
+import java.util.concurrent.atomic.AtomicInteger
+
+object ApiHookIsolationSpec extends ZIOSpecDefault {
+
+  private class SimpleBooleanProvider(name: String, value: Boolean) extends EventProvider {
+    @scala.annotation.nowarn("msg=deprecated")
+    override def getMetadata: Metadata                      = new Metadata { override def getName: String = name }
+    override def getState: ProviderState                    = ProviderState.READY
+    override def initialize(ctx: OFEvaluationContext): Unit = ()
+    override def shutdown(): Unit                           = ()
+
+    override def getBooleanEvaluation(
+      key: String,
+      defaultValue: java.lang.Boolean,
+      ctx: OFEvaluationContext
+    ): ProviderEvaluation[java.lang.Boolean] =
+      ProviderEvaluation.builder[java.lang.Boolean]().value(value).reason("STATIC").build()
+
+    override def getStringEvaluation(
+      key: String,
+      defaultValue: String,
+      ctx: OFEvaluationContext
+    ): ProviderEvaluation[String] =
+      ProviderEvaluation.builder[String]().value(defaultValue).reason("STATIC").build()
+
+    override def getIntegerEvaluation(
+      key: String,
+      defaultValue: java.lang.Integer,
+      ctx: OFEvaluationContext
+    ): ProviderEvaluation[java.lang.Integer] =
+      ProviderEvaluation.builder[java.lang.Integer]().value(defaultValue).reason("STATIC").build()
+
+    override def getDoubleEvaluation(
+      key: String,
+      defaultValue: java.lang.Double,
+      ctx: OFEvaluationContext
+    ): ProviderEvaluation[java.lang.Double] =
+      ProviderEvaluation.builder[java.lang.Double]().value(defaultValue).reason("STATIC").build()
+
+    override def getObjectEvaluation(
+      key: String,
+      defaultValue: dev.openfeature.sdk.Value,
+      ctx: OFEvaluationContext
+    ): ProviderEvaluation[dev.openfeature.sdk.Value] =
+      ProviderEvaluation.builder[dev.openfeature.sdk.Value]().value(defaultValue).reason("STATIC").build()
+  }
+
+  // Counts before-hook invocations from the Java SDK thread.
+  private class CountingJavaHook extends dev.openfeature.sdk.Hook[java.lang.Boolean] {
+    val count = new AtomicInteger(0)
+    override def before(
+      ctx: JavaHookContext[java.lang.Boolean],
+      hints: java.util.Map[String, AnyRef]
+    ): java.util.Optional[OFEvaluationContext] = {
+      count.incrementAndGet()
+      java.util.Optional.empty()
+    }
+  }
+
+  private def buildIsolated(provider: EventProvider): ZIO[Scope, Throwable, FeatureFlags] = {
+    val api    = OpenFeatureAPIFactory.create()
+    val domain = s"api-hook-iso-${java.util.UUID.randomUUID()}"
+    for {
+      ff <- FeatureFlags.build(
+        provider,
+        domain = Some(domain),
+        version = None,
+        initialHooks = Nil,
+        statusRef = None,
+        addShutdownFinalizer = false,
+        apiOverride = Some(api)
+      )
+      _ <- ZIO.attemptBlocking(Thread.sleep(50)).ignore
+    } yield ff
+  }
+
+  def spec = suite("API Hook Isolation")(
+    test("addApiHook installs the hook on this FeatureFlags' isolated API") {
+      ZIO.scoped {
+        for {
+          ff <- buildIsolated(new SimpleBooleanProvider("p", true))
+          hook = new CountingJavaHook
+          _ <- ff.addApiHook(hook)
+          _ <- ff.boolean("flag", default = false).provideEnvironment(ZEnvironment(ff))
+        } yield assertTrue(hook.count.get() == 1)
+      }
+    },
+    test("addApiHook on one isolated FeatureFlags does not leak to another") {
+      ZIO.scoped {
+        for {
+          ffA <- buildIsolated(new SimpleBooleanProvider("pA", true))
+          ffB <- buildIsolated(new SimpleBooleanProvider("pB", true))
+          hookA = new CountingJavaHook
+          _ <- ffA.addApiHook(hookA)
+          _ <- ffA.boolean("flag", default = false).provideEnvironment(ZEnvironment(ffA))
+          _ <- ffB.boolean("flag", default = false).provideEnvironment(ZEnvironment(ffB))
+        } yield assertTrue(hookA.count.get() == 1)
+      }
+    },
+    test("clearApiHooks removes previously-installed API hooks") {
+      ZIO.scoped {
+        for {
+          ff <- buildIsolated(new SimpleBooleanProvider("p", true))
+          hook = new CountingJavaHook
+          _ <- ff.addApiHook(hook)
+          _ <- ff.clearApiHooks
+          _ <- ff.boolean("flag", default = false).provideEnvironment(ZEnvironment(ff))
+        } yield assertTrue(hook.count.get() == 0)
+      }
+    }
+  )
+}