From 815dcb081aa4c8f8848160eb16ee60289fe7dca7 Mon Sep 17 00:00:00 2001 From: "v.karamyshev" Date: Wed, 14 Jan 2026 18:46:48 +0500 Subject: [PATCH] Fixes issue #912: Executes liftF effect twice in a product composition When combining a Done fetch with a Blocked fetch in product/productR/map2, the previous implementation would call product(fa, c) or map2(fa, c)(f), causing the already-completed effect `fa` to be re-run. The fix uses the already-computed result value instead of re-running the entire fetch: - product: use c.map((a, _)) instead of product(fa, c) - productR: use c or c.as(b) instead of productR(fa)(c) - map2: use c.map(b => f(a, b)) instead of map2(fa, c)(f) Added unit tests to verify that side effects are executed exactly once. --- fetch/src/main/scala/fetch.scala | 12 +++---- fetch/src/test/scala/FetchTests.scala | 47 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/fetch/src/main/scala/fetch.scala b/fetch/src/main/scala/fetch.scala index 3f40b653..b963565a 100644 --- a/fetch/src/main/scala/fetch.scala +++ b/fetch/src/main/scala/fetch.scala @@ -301,9 +301,9 @@ object `package` { case (Done(a), Done(b)) => Done[F, Z](f(a, b)) case (Done(a), Blocked(br, c)) => - Blocked[F, Z](br, map2(fa, c)(f)) + Blocked[F, Z](br, c.map(b => f(a, b))) case (Blocked(br, c), Done(b)) => - Blocked[F, Z](br, map2(c, fb)(f)) + Blocked[F, Z](br, c.map(a => f(a, b))) case (Blocked(br, c), Blocked(br2, c2)) => Blocked[F, Z](combineRequestMaps(br, br2), map2(c, c2)(f)) case (_, Throw(e)) => @@ -324,9 +324,9 @@ object `package` { case (Done(a), Done(b)) => Done[F, (A, B)]((a, b)) case (Done(a), Blocked(br, c)) => - Blocked[F, (A, B)](br, product(fa, c)) + Blocked[F, (A, B)](br, c.map((a, _))) case (Blocked(br, c), Done(b)) => - Blocked[F, (A, B)](br, product(c, fb)) + Blocked[F, (A, B)](br, c.map((_, b))) case (Blocked(br, c), Blocked(br2, c2)) => Blocked[F, (A, B)](combineRequestMaps(br, br2), product(c, c2)) case (_, Throw(e)) => @@ -343,9 +343,9 @@ object `package` { case (Done(a), Done(b)) => Done[F, B](b) case (Done(a), Blocked(br, c)) => - Blocked[F, B](br, productR(fa)(c)) + Blocked[F, B](br, c) case (Blocked(br, c), Done(b)) => - Blocked[F, B](br, productR(c)(fb)) + Blocked[F, B](br, c.as(b)) case (Blocked(br, c), Blocked(br2, c2)) => Blocked[F, B](combineRequestMaps(br, br2), productR(c)(c2)) case (_, Throw(e)) => diff --git a/fetch/src/test/scala/FetchTests.scala b/fetch/src/test/scala/FetchTests.scala index dcbed965..dc8b3b2c 100644 --- a/fetch/src/test/scala/FetchTests.scala +++ b/fetch/src/test/scala/FetchTests.scala @@ -942,4 +942,51 @@ class FetchTests extends FetchSpec { io.map(_ shouldEqual List(0, 1, 2, 42)).unsafeToFuture() } + + // Side effect re-execution tests (Issue #912) + + "Product should not re-execute side effects from completed fetches" in { + Ref[IO] + .of(0) + .flatMap { counter => + val fetchWithSideEffect: Fetch[IO, Unit] = Fetch.liftF(counter.update(_ + 1)) + + val program = ( + List.range(1, 10).map(one[IO]).reduce(_ >> _), + fetchWithSideEffect + ).tupled + + Fetch.run[IO](program) >> counter.get.map(_ shouldEqual 1) + } + .unsafeToFuture() + } + + "ProductR should not re-execute side effects from completed fetches" in { + Ref[IO] + .of(0) + .flatMap { counter => + val fetchWithSideEffect: Fetch[IO, Unit] = Fetch.liftF(counter.update(_ + 1)) + + val program = List.range(1, 10).map(one[IO]).reduce(_ >> _) *> fetchWithSideEffect + + Fetch.run[IO](program) >> counter.get.map(_ shouldEqual 1) + } + .unsafeToFuture() + } + + "mapN should not re-execute side effects from completed fetches" in { + Ref[IO] + .of(0) + .flatMap { counter => + val fetchWithSideEffect: Fetch[IO, Int] = Fetch.liftF(counter.updateAndGet(_ + 1)) + + val program = ( + List.range(1, 10).map(one[IO]).reduce(_ >> _), + fetchWithSideEffect + ).mapN((_, n) => n) + + Fetch.run[IO](program) >> counter.get.map(_ shouldEqual 1) + } + .unsafeToFuture() + } }