From 02abae42447854580a5634d6f7959203ae71a580 Mon Sep 17 00:00:00 2001 From: alexcardell <29524087+alexcardell@users.noreply.github.com> Date: Fri, 13 Sep 2024 16:11:15 +0100 Subject: [PATCH 1/4] Add AfterHook type --- .../scala/io/cardell/openfeature/Hook.scala | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hook.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hook.scala index 68b0939..117d0f2 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hook.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hook.scala @@ -65,3 +65,25 @@ object ErrorHook { } } + +trait AfterHook[F[_]] extends Hook[F] { + + def apply(context: HookContext, hints: HookHints): F[Unit] + +} + +object AfterHook { + + def apply[F[_]]( + f: (HookContext, HookHints) => F[Unit] + ): AfterHook[F] = + new AfterHook[F] { + + def apply( + context: HookContext, + hints: HookHints + ): F[Unit] = f(context, hints) + + } + +} From c906658fbb287bd2cef4ea6046eb31466020ff6f Mon Sep 17 00:00:00 2001 From: alexcardell <29524087+alexcardell@users.noreply.github.com> Date: Fri, 13 Sep 2024 16:18:24 +0100 Subject: [PATCH 2/4] Add Provider After hooks --- .../openfeature/FeatureClientImpl.scala | 1 + .../scala/io/cardell/openfeature/Hooks.scala | 15 +++++---- .../openfeature/provider/ProviderImpl.scala | 31 ++++++++++++++----- .../provider/ProviderImplTest.scala | 24 ++++++++++++++ 4 files changed, 58 insertions(+), 13 deletions(-) diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala index 2b47cea..b9a3cd6 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala @@ -61,6 +61,7 @@ protected[openfeature] final class FeatureClientImpl[F[_]]( beforeHooks, errorHooks.appended(h) ) + case _ => ??? } override def getBooleanValue(flagKey: String, default: Boolean): F[Boolean] = diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala index 4387a43..84a28bc 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala @@ -24,6 +24,7 @@ import io.cardell.openfeature.FlagValue.DoubleValue import io.cardell.openfeature.FlagValue.IntValue import io.cardell.openfeature.FlagValue.StringValue import io.cardell.openfeature.FlagValue.StructureValue +import cats.Applicative sealed trait FlagValueType @@ -98,12 +99,14 @@ object Hooks { aux(hooks, context).map(_.getOrElse(context.evaluationContext)) } - def runErrors[F[_]: Monad]( + def runErrors[F[_]: Applicative]( hooks: List[ErrorHook[F]] - )( - context: HookContext, - hints: HookHints, - error: Throwable - ): F[Unit] = hooks.traverse(_.apply(context, hints, error)).void + )(context: HookContext, hints: HookHints, error: Throwable): F[Unit] = + hooks.traverse(_.apply(context, hints, error)).void + + def runAfter[F[_]: Applicative]( + hooks: List[AfterHook[F]] + )(context: HookContext, hints: HookHints): F[Unit] = + hooks.traverse(_.apply(context, hints)).void } diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala index a1f47d7..2f1617e 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala @@ -21,6 +21,7 @@ import cats.syntax.all._ import io.cardell.openfeature.BeforeHook import io.cardell.openfeature.ErrorHook +import io.cardell.openfeature.AfterHook import io.cardell.openfeature.EvaluationContext import io.cardell.openfeature.FlagValue import io.cardell.openfeature.Hook @@ -32,24 +33,34 @@ import io.cardell.openfeature.StructureDecoder protected class ProviderImpl[F[_]: MonadThrow]( evaluationProvider: EvaluationProvider[F], val beforeHooks: List[BeforeHook[F]], - val errorHooks: List[ErrorHook[F]] + val errorHooks: List[ErrorHook[F]], + val afterHooks: List[AfterHook[F]] ) extends Provider[F] { override def metadata: ProviderMetadata = evaluationProvider.metadata override def withHook(hook: Hook[F]): Provider[F] = hook match { - case bh: BeforeHook[F] => + case h: BeforeHook[F] => new ProviderImpl[F]( evaluationProvider = evaluationProvider, - beforeHooks = beforeHooks.appended(bh), - errorHooks = errorHooks + beforeHooks = beforeHooks.appended(h), + errorHooks = errorHooks, + afterHooks = afterHooks ) - case eh: ErrorHook[F] => + case h: ErrorHook[F] => new ProviderImpl[F]( evaluationProvider = evaluationProvider, beforeHooks = beforeHooks, - errorHooks = errorHooks.appended(eh) + errorHooks = errorHooks.appended(h), + afterHooks = afterHooks + ) + case h: AfterHook[F] => + new ProviderImpl[F]( + evaluationProvider = evaluationProvider, + beforeHooks = beforeHooks, + errorHooks = errorHooks, + afterHooks = afterHooks.appended(h) ) } @@ -137,6 +148,11 @@ protected class ProviderImpl[F[_]: MonadThrow]( for { context <- Hooks.runBefore(beforeHooks)(hc, hints) res <- resolve(context) + _ <- + Hooks.runAfter(afterHooks)( + hc.copy(evaluationContext = context), + hints + ) } yield res run.onError(error => @@ -154,7 +170,8 @@ object ProviderImpl { new ProviderImpl[F]( evaluationProvider = evaluationProvider, beforeHooks = List.empty[BeforeHook[F]], - errorHooks = List.empty[ErrorHook[F]] + errorHooks = List.empty[ErrorHook[F]], + afterHooks = List.empty[AfterHook[F]] ) } diff --git a/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala b/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala index e3dbfe2..2540874 100644 --- a/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala +++ b/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala @@ -22,6 +22,7 @@ import munit.CatsEffectSuite import io.cardell.openfeature.BeforeHook import io.cardell.openfeature.ErrorHook +import io.cardell.openfeature.AfterHook import io.cardell.openfeature.EvaluationContext import io.cardell.openfeature.HookContext import io.cardell.openfeature.HookHints @@ -135,4 +136,27 @@ class ProviderImplTest extends CatsEffectSuite { } yield assert(result.isLeft, "result was not a Left") } + test("after hooks run after successful evaluation") { + val ref = Ref.unsafe[IO, Int](0) + + val afterHook = AfterHook[IO] { case _ => ref.update(_ + 2) } + + val provider = ProviderImpl(evaluationProvider) + .withHook(afterHook) + + val expected = 2 + + for { + _ <- + provider + .resolveBooleanValue( + "test-flag", + false, + EvaluationContext.empty + ) + .attempt + result <- ref.get + } yield assertEquals(result, expected) + } + } From 07287044d89416eb8f6a042dbcacc921f84e6fac Mon Sep 17 00:00:00 2001 From: alexcardell <29524087+alexcardell@users.noreply.github.com> Date: Fri, 13 Sep 2024 16:25:54 +0100 Subject: [PATCH 3/4] Add FeatureClient After hooks --- .../cardell/openfeature/FeatureClient.scala | 2 -- .../openfeature/FeatureClientImpl.scala | 33 ++++++++++++++----- .../scala/io/cardell/openfeature/Hooks.scala | 2 +- .../openfeature/provider/Provider.scala | 1 + .../openfeature/provider/ProviderImpl.scala | 2 +- .../openfeature/FeatureClientImplTest.scala | 21 +++++++++++- .../provider/ProviderImplTest.scala | 2 +- 7 files changed, 49 insertions(+), 14 deletions(-) diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClient.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClient.scala index 73fba68..7b47e30 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClient.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClient.scala @@ -25,8 +25,6 @@ trait FeatureClient[F[_]] { def evaluationContext: EvaluationContext def withEvaluationContext(context: EvaluationContext): FeatureClient[F] - def beforeHooks: List[BeforeHook[F]] - def withHook(hook: Hook[F]): FeatureClient[F] // def withHooks(hooks: List[Hook]): FeatureClient[F] diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala index b9a3cd6..8f68271 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/FeatureClientImpl.scala @@ -27,7 +27,8 @@ protected[openfeature] final class FeatureClientImpl[F[_]]( provider: EvaluationProvider[F], val clientEvaluationContext: EvaluationContext, val beforeHooks: List[BeforeHook[F]], - val errorHooks: List[ErrorHook[F]] + val errorHooks: List[ErrorHook[F]], + val afterHooks: List[AfterHook[F]] )(implicit M: MonadThrow[F]) extends FeatureClient[F] { @@ -42,7 +43,8 @@ protected[openfeature] final class FeatureClientImpl[F[_]]( provider, clientEvaluationContext ++ context, beforeHooks, - errorHooks + errorHooks, + afterHooks ) override def withHook(hook: Hook[F]): FeatureClient[F] = @@ -52,16 +54,25 @@ protected[openfeature] final class FeatureClientImpl[F[_]]( provider, clientEvaluationContext, beforeHooks.appended(h), - errorHooks + errorHooks, + afterHooks ) case h: ErrorHook[F] => new FeatureClientImpl[F]( provider, clientEvaluationContext, beforeHooks, - errorHooks.appended(h) + errorHooks.appended(h), + afterHooks + ) + case h: AfterHook[F] => + new FeatureClientImpl[F]( + provider, + clientEvaluationContext, + beforeHooks, + errorHooks, + afterHooks.appended(h) ) - case _ => ??? } override def getBooleanValue(flagKey: String, default: Boolean): F[Boolean] = @@ -361,11 +372,16 @@ protected[openfeature] final class FeatureClientImpl[F[_]]( for { newContext <- Hooks.runBefore[F](beforeHooks)(hookContext, hookHints) evaluation <- evaluate(newContext) + _ <- + Hooks.runAfter[F](afterHooks)( + hookContext.copy(evaluationContext = newContext), + hookHints + ) } yield evaluation run - .onError { case e => - Hooks.runErrors(errorHooks)(hookContext, hookHints, e) + .onError { case error => + Hooks.runErrors(errorHooks)(hookContext, hookHints, error) } .handleError(error => EvaluationDetails(flagKey, ResolutionDetails.error(default, error)) @@ -384,7 +400,8 @@ object FeatureClientImpl { provider = provider, clientEvaluationContext = EvaluationContext.empty, beforeHooks = List.empty[BeforeHook[F]], - errorHooks = List.empty[ErrorHook[F]] + errorHooks = List.empty[ErrorHook[F]], + afterHooks = List.empty[AfterHook[F]] ) } diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala index 84a28bc..af6316f 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/Hooks.scala @@ -16,6 +16,7 @@ package io.cardell.openfeature +import cats.Applicative import cats.Monad import cats.syntax.all._ @@ -24,7 +25,6 @@ import io.cardell.openfeature.FlagValue.DoubleValue import io.cardell.openfeature.FlagValue.IntValue import io.cardell.openfeature.FlagValue.StringValue import io.cardell.openfeature.FlagValue.StructureValue -import cats.Applicative sealed trait FlagValueType diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala index c7a090d..5f43108 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala @@ -21,6 +21,7 @@ import io.cardell.openfeature.ErrorHook import io.cardell.openfeature.Hook trait Provider[F[_]] extends EvaluationProvider[F] { + // TODO remove def beforeHooks: List[BeforeHook[F]] def errorHooks: List[ErrorHook[F]] diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala index 2f1617e..bef7827 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/ProviderImpl.scala @@ -19,9 +19,9 @@ package io.cardell.openfeature.provider import cats.MonadThrow import cats.syntax.all._ +import io.cardell.openfeature.AfterHook import io.cardell.openfeature.BeforeHook import io.cardell.openfeature.ErrorHook -import io.cardell.openfeature.AfterHook import io.cardell.openfeature.EvaluationContext import io.cardell.openfeature.FlagValue import io.cardell.openfeature.Hook diff --git a/openfeature/sdk/src/test/scala/io/cardell/openfeature/FeatureClientImplTest.scala b/openfeature/sdk/src/test/scala/io/cardell/openfeature/FeatureClientImplTest.scala index 5b82965..93a1553 100644 --- a/openfeature/sdk/src/test/scala/io/cardell/openfeature/FeatureClientImplTest.scala +++ b/openfeature/sdk/src/test/scala/io/cardell/openfeature/FeatureClientImplTest.scala @@ -48,7 +48,11 @@ class FeatureClientImplTest extends CatsEffectSuite { val client = FeatureClientImpl[IO](provider) - val result = client.withHook(beforeHook1).beforeHooks + val result = + client + .withHook(beforeHook1) + .asInstanceOf[FeatureClientImpl[IO]] + .beforeHooks assertEquals(expected, result) } @@ -114,6 +118,21 @@ class FeatureClientImplTest extends CatsEffectSuite { } } + test("before hooks run on boolean evaluation") { + val ref = Ref.unsafe[IO, Int](0) + + val afterHook = AfterHook[IO] { case _ => ref.update(_ + 2) } + + val client = FeatureClientImpl[IO](provider).withHook(afterHook) + + val expected = 2 + + for { + _ <- client.getBooleanValue("test-flag", false) + result <- ref.get + } yield assertEquals(result, expected) + } + } object ThrowingEvaluationProvider extends EvaluationProvider[IO] { diff --git a/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala b/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala index 2540874..1c1c67d 100644 --- a/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala +++ b/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala @@ -20,9 +20,9 @@ import cats.effect.IO import cats.effect.kernel.Ref import munit.CatsEffectSuite +import io.cardell.openfeature.AfterHook import io.cardell.openfeature.BeforeHook import io.cardell.openfeature.ErrorHook -import io.cardell.openfeature.AfterHook import io.cardell.openfeature.EvaluationContext import io.cardell.openfeature.HookContext import io.cardell.openfeature.HookHints From c69f7aded63cea4ff5a82faf1f96d264c28a61b9 Mon Sep 17 00:00:00 2001 From: alexcardell <29524087+alexcardell@users.noreply.github.com> Date: Fri, 13 Sep 2024 16:28:28 +0100 Subject: [PATCH 4/4] Remove hook accessors from traits --- .../scala/io/cardell/openfeature/provider/Provider.scala | 6 ------ .../io/cardell/openfeature/provider/ProviderImplTest.scala | 6 ++++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala index 5f43108..cd87c8b 100644 --- a/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala +++ b/openfeature/sdk/src/main/scala/io/cardell/openfeature/provider/Provider.scala @@ -16,15 +16,9 @@ package io.cardell.openfeature.provider -import io.cardell.openfeature.BeforeHook -import io.cardell.openfeature.ErrorHook import io.cardell.openfeature.Hook trait Provider[F[_]] extends EvaluationProvider[F] { - // TODO remove - def beforeHooks: List[BeforeHook[F]] - def errorHooks: List[ErrorHook[F]] - def withHook(hook: Hook[F]): Provider[F] // def withHooks(hooks: List[Hook[F]]): Provider[F] diff --git a/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala b/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala index 1c1c67d..55d564a 100644 --- a/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala +++ b/openfeature/sdk/src/test/scala/io/cardell/openfeature/provider/ProviderImplTest.scala @@ -48,7 +48,8 @@ class ProviderImplTest extends CatsEffectSuite { val provider = ProviderImpl(evaluationProvider) - val result = provider.withHook(beforeHook1).beforeHooks + val result = + provider.withHook(beforeHook1).asInstanceOf[ProviderImpl[IO]].beforeHooks assertEquals(result, expected) } @@ -63,7 +64,8 @@ class ProviderImplTest extends CatsEffectSuite { val provider = ProviderImpl(evaluationProvider).withHook(beforeHook1) - val result = provider.withHook(beforeHook2).beforeHooks + val result = + provider.withHook(beforeHook2).asInstanceOf[ProviderImpl[IO]].beforeHooks assertEquals(result, expected) }