From 5565e8e9995624b058e31e93c855ae1bce5060d0 Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 09:17:10 +0100 Subject: [PATCH 01/16] Fix review findings: index, partial tags, empty tag call, tests, and @internal - Fix 0-based index in error message (was 1-based due to pre-increment) - Use separate accumulator so partial tags are never applied on invalid yield - Guard tag() call in finally with !isEmpty() to avoid unnecessary FOSHttpCache calls - Fix PHPDoc Generator key type from int to array-key - Add @internal to CacheType interface to signal it is not for external implementation - Update CacheActivatorTest: fix broken constructor, add ProphecyTrait, add 10 tests covering all withoutAutomaticTagging() code paths Co-Authored-By: Claude Sonnet 4.6 --- src/Cache/CacheType.php | 3 + src/CacheActivator.php | 13 ++- tests/Unit/CacheActivatorTest.php | 159 +++++++++++++++++++++++++++++- 3 files changed, 169 insertions(+), 6 deletions(-) diff --git a/src/Cache/CacheType.php b/src/Cache/CacheType.php index b9a8b82..984f957 100644 --- a/src/Cache/CacheType.php +++ b/src/Cache/CacheType.php @@ -2,6 +2,9 @@ namespace Neusta\Pimcore\HttpCacheBundle\Cache; +/** + * @internal + */ interface CacheType { public function applyTo(string $tag): string; diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 66dd085..618d0f0 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -32,7 +32,7 @@ public function deactivateCaching(): void /** * @template T * - * @param \Closure(): (T|\Generator) $fn + * @param \Closure(): (T|\Generator) $fn * * @return T */ @@ -47,10 +47,9 @@ public function withoutAutomaticTagging(\Closure $fn): mixed $result = $fn(); if ($result instanceof \Generator) { + $accumulated = new CacheTags(); $index = 0; foreach ($result as $key => $yielded) { - ++$index; - if (!$yielded instanceof CacheTag && !$yielded instanceof CacheTags) { throw new \LogicException(\sprintf( 'Invalid yielded value at index %d (key: %s): Expected only "%s" or "%s", got "%s".', @@ -62,14 +61,18 @@ public function withoutAutomaticTagging(\Closure $fn): mixed )); } - $tags = $tags->with($yielded); + $accumulated = $accumulated->with($yielded); + ++$index; } + $tags = $accumulated; $result = $result->getReturn(); } } finally { $this->isCachingActive = $previous; - ($this->responseTagger)()->tag($tags); + if (!$tags->isEmpty()) { + ($this->responseTagger)()->tag($tags); + } } return $result; diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index c0084e5..0243815 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -2,16 +2,28 @@ namespace Neusta\Pimcore\HttpCacheBundle\Tests\Unit; +use Neusta\Pimcore\HttpCacheBundle\Cache\CacheTag; +use Neusta\Pimcore\HttpCacheBundle\Cache\CacheTags; +use Neusta\Pimcore\HttpCacheBundle\Cache\ResponseTagger; use Neusta\Pimcore\HttpCacheBundle\CacheActivator; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; +use Prophecy\PhpUnit\ProphecyTrait; +use Prophecy\Prophecy\ObjectProphecy; final class CacheActivatorTest extends TestCase { + use ProphecyTrait; + private CacheActivator $cacheActivator; + /** @var ObjectProphecy */ + private ObjectProphecy $responseTagger; + protected function setUp(): void { - $this->cacheActivator = new CacheActivator(); + $this->responseTagger = $this->prophesize(ResponseTagger::class); + $this->cacheActivator = new CacheActivator(fn () => $this->responseTagger->reveal()); } /** @@ -42,4 +54,149 @@ public function it_must_be_activated_after_activateCaching_is_called(): void self::assertTrue($this->cacheActivator->isCachingActive()); } + + /** + * @test + */ + public function without_automatic_tagging_returns_closure_result(): void + { + $result = $this->cacheActivator->withoutAutomaticTagging(static fn () => 'my_result'); + + self::assertSame('my_result', $result); + } + + /** + * @test + */ + public function without_automatic_tagging_disables_caching_during_execution(): void + { + $activator = $this->cacheActivator; + $wasCachingActive = null; + + $this->cacheActivator->withoutAutomaticTagging(static function () use ($activator, &$wasCachingActive) { + $wasCachingActive = $activator->isCachingActive(); + }); + + self::assertFalse($wasCachingActive); + } + + /** + * @test + */ + public function without_automatic_tagging_restores_caching_state_after_execution(): void + { + $this->cacheActivator->withoutAutomaticTagging(static fn () => null); + + self::assertTrue($this->cacheActivator->isCachingActive()); + } + + /** + * @test + */ + public function without_automatic_tagging_restores_inactive_state_when_caching_was_already_disabled(): void + { + $this->cacheActivator->deactivateCaching(); + + $this->cacheActivator->withoutAutomaticTagging(static fn () => null); + + self::assertFalse($this->cacheActivator->isCachingActive()); + } + + /** + * @test + */ + public function without_automatic_tagging_restores_caching_state_even_when_closure_throws(): void + { + try { + $this->cacheActivator->withoutAutomaticTagging(static fn () => throw new \RuntimeException('error')); + } catch (\RuntimeException) { + } + + self::assertTrue($this->cacheActivator->isCachingActive()); + } + + /** + * @test + */ + public function without_automatic_tagging_does_not_call_tag_when_closure_yields_nothing(): void + { + $this->cacheActivator->withoutAutomaticTagging(static fn () => null); + + $this->responseTagger->tag(Argument::any())->shouldNotHaveBeenCalled(); + } + + /** + * @test + */ + public function without_automatic_tagging_tags_a_yielded_cache_tag(): void + { + $tag = CacheTag::fromString('42'); + + $this->cacheActivator->withoutAutomaticTagging(static function () use ($tag) { + yield $tag; + }); + + $this->responseTagger->tag(Argument::that( + static fn (CacheTags $tags) => $tags->toString() === $tag->toString(), + ))->shouldHaveBeenCalledOnce(); + } + + /** + * @test + */ + public function without_automatic_tagging_tags_a_yielded_cache_tags_collection(): void + { + $tags = CacheTags::fromStrings(['17', '42']); + + $this->cacheActivator->withoutAutomaticTagging(static function () use ($tags) { + yield $tags; + }); + + $this->responseTagger->tag(Argument::that( + static fn (CacheTags $t) => $t->toString() === $tags->toString(), + ))->shouldHaveBeenCalledOnce(); + } + + /** + * @test + */ + public function without_automatic_tagging_returns_generator_return_value(): void + { + $result = $this->cacheActivator->withoutAutomaticTagging(static function () { + yield CacheTag::fromString('42'); + + return 'generator_result'; + }); + + self::assertSame('generator_result', $result); + } + + /** + * @test + */ + public function without_automatic_tagging_throws_logic_exception_for_invalid_yield(): void + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessageMatches('/Invalid yielded value at index 0/'); + + $this->cacheActivator->withoutAutomaticTagging(static function () { + yield 'not_a_cache_tag'; + }); + } + + /** + * @test + */ + public function without_automatic_tagging_does_not_apply_partial_tags_when_invalid_yield_encountered(): void + { + try { + $this->cacheActivator->withoutAutomaticTagging(static function () { + yield CacheTag::fromString('42'); + yield 'not_a_cache_tag'; + }); + } catch (\LogicException) { + } + + $this->responseTagger->tag(Argument::any())->shouldNotHaveBeenCalled(); + } } From 46326906eb9785a070b8ca61f0d0f440b1e99aba Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 09:18:38 +0100 Subject: [PATCH 02/16] Fix review findings: encapsulation, style, and naming - Make TraceableResponseTagger::$recordedTags private, expose via getRecordedTags() - Fix missing declare(strict_types=1) inline style (blank line after --- src/Cache/ResponseTagger/TraceableResponseTagger.php | 10 +++++++--- src/DataCollector.php | 2 +- .../ResponseTagger/TraceableResponseTaggerTest.php | 6 +++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Cache/ResponseTagger/TraceableResponseTagger.php b/src/Cache/ResponseTagger/TraceableResponseTagger.php index 930382f..8dd6646 100644 --- a/src/Cache/ResponseTagger/TraceableResponseTagger.php +++ b/src/Cache/ResponseTagger/TraceableResponseTagger.php @@ -1,5 +1,4 @@ -inner->tag($tags); } + public function getRecordedTags(): CacheTags + { + return $this->recordedTags; + } + public function reset(): void { $this->recordedTags = new CacheTags(); diff --git a/src/DataCollector.php b/src/DataCollector.php index b11ffe8..9f4513f 100644 --- a/src/DataCollector.php +++ b/src/DataCollector.php @@ -27,7 +27,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep public function lateCollect(): void { - foreach ($this->traceableResponseTagger->recordedTags as $tag) { + foreach ($this->traceableResponseTagger->getRecordedTags() as $tag) { $this->data['tags'][] = [ 'tag' => $tag->toString(), 'type' => $tag->type->identifier(), ]; diff --git a/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php b/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php index b7ba2ab..93bd0d4 100644 --- a/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php +++ b/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php @@ -14,7 +14,7 @@ final class TraceableResponseTaggerTest extends TestCase { use ProphecyTrait; - private TraceableResponseTagger $collectTagsResponseTagger; + private TraceableResponseTagger $traceableResponseTagger; /** @var ObjectProphecy */ private ObjectProphecy $innerTagger; @@ -38,7 +38,7 @@ public function tag_should_collect_tags(): void self::assertSame( 'tag1,tag2', - $this->collectTagsResponseTagger->recordedTags->toString(), + $this->collectTagsResponseTagger->getRecordedTags()->toString(), ); } @@ -71,7 +71,7 @@ public function reset_should_reset_collected_tags(): void $this->collectTagsResponseTagger->reset(); self::assertTrue( - $this->collectTagsResponseTagger->recordedTags->isEmpty(), + $this->collectTagsResponseTagger->getRecordedTags()->isEmpty(), ); } } From 0be11bb35923f3aa674c471c15b197516d79c7ef Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 09:20:15 +0100 Subject: [PATCH 03/16] =?UTF-8?q?Revert=20getRecordedTags()=20=E2=80=94=20?= =?UTF-8?q?public=20property=20is=20idiomatic=20here?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CacheTags is immutable and TraceableResponseTagger is final, so a getter adds ceremony with no encapsulation benefit. Co-Authored-By: Claude Sonnet 4.6 --- src/Cache/ResponseTagger/TraceableResponseTagger.php | 7 +------ src/DataCollector.php | 2 +- .../Cache/ResponseTagger/TraceableResponseTaggerTest.php | 4 ++-- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Cache/ResponseTagger/TraceableResponseTagger.php b/src/Cache/ResponseTagger/TraceableResponseTagger.php index 8dd6646..9963753 100644 --- a/src/Cache/ResponseTagger/TraceableResponseTagger.php +++ b/src/Cache/ResponseTagger/TraceableResponseTagger.php @@ -8,7 +8,7 @@ final class TraceableResponseTagger implements ResponseTagger, ResetInterface { - private CacheTags $recordedTags; + public CacheTags $recordedTags; public function __construct( private readonly ResponseTagger $inner, @@ -22,11 +22,6 @@ public function tag(CacheTags $tags): void $this->inner->tag($tags); } - public function getRecordedTags(): CacheTags - { - return $this->recordedTags; - } - public function reset(): void { $this->recordedTags = new CacheTags(); diff --git a/src/DataCollector.php b/src/DataCollector.php index 9f4513f..b11ffe8 100644 --- a/src/DataCollector.php +++ b/src/DataCollector.php @@ -27,7 +27,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep public function lateCollect(): void { - foreach ($this->traceableResponseTagger->getRecordedTags() as $tag) { + foreach ($this->traceableResponseTagger->recordedTags as $tag) { $this->data['tags'][] = [ 'tag' => $tag->toString(), 'type' => $tag->type->identifier(), ]; diff --git a/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php b/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php index 93bd0d4..0b2c695 100644 --- a/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php +++ b/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php @@ -38,7 +38,7 @@ public function tag_should_collect_tags(): void self::assertSame( 'tag1,tag2', - $this->collectTagsResponseTagger->getRecordedTags()->toString(), + $this->collectTagsResponseTagger->recordedTags->toString(), ); } @@ -71,7 +71,7 @@ public function reset_should_reset_collected_tags(): void $this->collectTagsResponseTagger->reset(); self::assertTrue( - $this->collectTagsResponseTagger->getRecordedTags()->isEmpty(), + $this->collectTagsResponseTagger->recordedTags->isEmpty(), ); } } From 13b75f9db04722033e3c86dcb46bee32a0ff494d Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 09:29:25 +0100 Subject: [PATCH 04/16] =?UTF-8?q?Remove=20isEmpty()=20guard=20=E2=80=94=20?= =?UTF-8?q?FOSHttpCache=20already=20handles=20empty=20tag=20arrays?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- src/CacheActivator.php | 4 +--- tests/Unit/CacheActivatorTest.php | 10 ---------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 618d0f0..6c517e8 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -70,9 +70,7 @@ public function withoutAutomaticTagging(\Closure $fn): mixed } } finally { $this->isCachingActive = $previous; - if (!$tags->isEmpty()) { - ($this->responseTagger)()->tag($tags); - } + ($this->responseTagger)()->tag($tags); } return $result; diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index 0243815..d38c8d2 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -115,16 +115,6 @@ public function without_automatic_tagging_restores_caching_state_even_when_closu self::assertTrue($this->cacheActivator->isCachingActive()); } - /** - * @test - */ - public function without_automatic_tagging_does_not_call_tag_when_closure_yields_nothing(): void - { - $this->cacheActivator->withoutAutomaticTagging(static fn () => null); - - $this->responseTagger->tag(Argument::any())->shouldNotHaveBeenCalled(); - } - /** * @test */ From e1a7cd18f10ad0364d6a564d1741089a6f44fab2 Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 09:31:24 +0100 Subject: [PATCH 05/16] Remove @internal from CacheType interface Co-Authored-By: Claude Sonnet 4.6 --- src/Cache/CacheType.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Cache/CacheType.php b/src/Cache/CacheType.php index 984f957..b9a8b82 100644 --- a/src/Cache/CacheType.php +++ b/src/Cache/CacheType.php @@ -2,9 +2,6 @@ namespace Neusta\Pimcore\HttpCacheBundle\Cache; -/** - * @internal - */ interface CacheType { public function applyTo(string $tag): string; From 823ab45e0460347ae20ad990efdbf1773acb3a45 Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 09:34:54 +0100 Subject: [PATCH 06/16] Use expectException and shouldNotBeCalled() instead of try/catch Co-Authored-By: Claude Sonnet 4.6 --- tests/Unit/CacheActivatorTest.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index d38c8d2..8b71f93 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -179,14 +179,12 @@ public function without_automatic_tagging_throws_logic_exception_for_invalid_yie */ public function without_automatic_tagging_does_not_apply_partial_tags_when_invalid_yield_encountered(): void { - try { - $this->cacheActivator->withoutAutomaticTagging(static function () { - yield CacheTag::fromString('42'); - yield 'not_a_cache_tag'; - }); - } catch (\LogicException) { - } + $this->responseTagger->tag(Argument::any())->shouldNotBeCalled(); + $this->expectException(\LogicException::class); - $this->responseTagger->tag(Argument::any())->shouldNotHaveBeenCalled(); + $this->cacheActivator->withoutAutomaticTagging(static function () { + yield CacheTag::fromString('42'); + yield 'not_a_cache_tag'; + }); } } From 1f53096c7e7ec01b8e693bef26ebf8f74b7afc6d Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 10:30:48 +0100 Subject: [PATCH 07/16] Remove redundant index counter from withoutAutomaticTagging error message The generator key already identifies the problematic yield sufficiently. Co-Authored-By: Claude Sonnet 4.6 --- src/CacheActivator.php | 5 +---- tests/Unit/CacheActivatorTest.php | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 6c517e8..20f8107 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -48,12 +48,10 @@ public function withoutAutomaticTagging(\Closure $fn): mixed if ($result instanceof \Generator) { $accumulated = new CacheTags(); - $index = 0; foreach ($result as $key => $yielded) { if (!$yielded instanceof CacheTag && !$yielded instanceof CacheTags) { throw new \LogicException(\sprintf( - 'Invalid yielded value at index %d (key: %s): Expected only "%s" or "%s", got "%s".', - $index, + 'Invalid yielded value (key: %s): Expected only "%s" or "%s", got "%s".', \is_int($key) || \is_string($key) ? $key : get_debug_type($key), CacheTag::class, CacheTags::class, @@ -62,7 +60,6 @@ public function withoutAutomaticTagging(\Closure $fn): mixed } $accumulated = $accumulated->with($yielded); - ++$index; } $tags = $accumulated; diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index 8b71f93..6747054 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -167,7 +167,7 @@ public function without_automatic_tagging_returns_generator_return_value(): void public function without_automatic_tagging_throws_logic_exception_for_invalid_yield(): void { $this->expectException(\LogicException::class); - $this->expectExceptionMessageMatches('/Invalid yielded value at index 0/'); + $this->expectExceptionMessageMatches('/Invalid yielded value \(key: 0\)/'); $this->cacheActivator->withoutAutomaticTagging(static function () { yield 'not_a_cache_tag'; From e4ef552a44094a3d2deb9f28840915416f366134 Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 11:01:13 +0100 Subject: [PATCH 08/16] =?UTF-8?q?Revert=20atomic=20tag=20accumulation=20?= =?UTF-8?q?=E2=80=94=20partial=20tags=20on=20invalid=20yield=20are=20inten?= =?UTF-8?q?tional?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- src/CacheActivator.php | 5 +---- tests/Unit/CacheActivatorTest.php | 13 ------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 20f8107..9bc6ad5 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -47,7 +47,6 @@ public function withoutAutomaticTagging(\Closure $fn): mixed $result = $fn(); if ($result instanceof \Generator) { - $accumulated = new CacheTags(); foreach ($result as $key => $yielded) { if (!$yielded instanceof CacheTag && !$yielded instanceof CacheTags) { throw new \LogicException(\sprintf( @@ -59,10 +58,8 @@ public function withoutAutomaticTagging(\Closure $fn): mixed )); } - $accumulated = $accumulated->with($yielded); + $tags = $tags->with($yielded); } - - $tags = $accumulated; $result = $result->getReturn(); } } finally { diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index 6747054..e575ca3 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -174,17 +174,4 @@ public function without_automatic_tagging_throws_logic_exception_for_invalid_yie }); } - /** - * @test - */ - public function without_automatic_tagging_does_not_apply_partial_tags_when_invalid_yield_encountered(): void - { - $this->responseTagger->tag(Argument::any())->shouldNotBeCalled(); - $this->expectException(\LogicException::class); - - $this->cacheActivator->withoutAutomaticTagging(static function () { - yield CacheTag::fromString('42'); - yield 'not_a_cache_tag'; - }); - } } From 505f4c9186e99ba30c152546367bc6dcffe4c1f8 Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 11:03:29 +0100 Subject: [PATCH 09/16] Fix incomplete rename: update all usages of collectTagsResponseTagger to traceableResponseTagger Co-Authored-By: Claude Sonnet 4.6 --- .../ResponseTagger/TraceableResponseTaggerTest.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php b/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php index 0b2c695..d9db01b 100644 --- a/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php +++ b/tests/Unit/Cache/ResponseTagger/TraceableResponseTaggerTest.php @@ -22,7 +22,7 @@ final class TraceableResponseTaggerTest extends TestCase protected function setUp(): void { $this->innerTagger = $this->prophesize(ResponseTagger::class); - $this->collectTagsResponseTagger = new TraceableResponseTagger($this->innerTagger->reveal()); + $this->traceableResponseTagger = new TraceableResponseTagger($this->innerTagger->reveal()); } /** @@ -30,7 +30,7 @@ protected function setUp(): void */ public function tag_should_collect_tags(): void { - $this->collectTagsResponseTagger->tag( + $this->traceableResponseTagger->tag( new CacheTags( CacheTag::fromString('tag1'), CacheTag::fromString('tag2'), @@ -38,7 +38,7 @@ public function tag_should_collect_tags(): void self::assertSame( 'tag1,tag2', - $this->collectTagsResponseTagger->recordedTags->toString(), + $this->traceableResponseTagger->recordedTags->toString(), ); } @@ -52,7 +52,7 @@ public function tag_should_forward_tags_to_inner_tagger(): void CacheTag::fromString('tag2'), ); - $this->collectTagsResponseTagger->tag($tags); + $this->traceableResponseTagger->tag($tags); $this->innerTagger->tag($tags)->shouldHaveBeenCalledOnce(); } @@ -62,16 +62,16 @@ public function tag_should_forward_tags_to_inner_tagger(): void */ public function reset_should_reset_collected_tags(): void { - $this->collectTagsResponseTagger->tag( + $this->traceableResponseTagger->tag( new CacheTags( CacheTag::fromString('tag1'), CacheTag::fromString('tag2'), )); - $this->collectTagsResponseTagger->reset(); + $this->traceableResponseTagger->reset(); self::assertTrue( - $this->collectTagsResponseTagger->recordedTags->isEmpty(), + $this->traceableResponseTagger->recordedTags->isEmpty(), ); } } From a3fad862fe91c31c594331fb7116775e06e5572b Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 11:15:38 +0100 Subject: [PATCH 10/16] Re-add iteration counter as \$position to error message for invalid yield MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generator key alone is ambiguous — a key of 1 could mean the first or second yield depending on whether keys are explicit. The position counter (0-based iteration count) provides unambiguous location information independent of the generator key. Co-Authored-By: Claude Sonnet 4.6 --- src/CacheActivator.php | 5 ++++- tests/Unit/CacheActivatorTest.php | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 9bc6ad5..9cfcf43 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -47,10 +47,12 @@ public function withoutAutomaticTagging(\Closure $fn): mixed $result = $fn(); if ($result instanceof \Generator) { + $position = 0; foreach ($result as $key => $yielded) { if (!$yielded instanceof CacheTag && !$yielded instanceof CacheTags) { throw new \LogicException(\sprintf( - 'Invalid yielded value (key: %s): Expected only "%s" or "%s", got "%s".', + 'Invalid yielded value at position %d (key: %s): Expected only "%s" or "%s", got "%s".', + $position, \is_int($key) || \is_string($key) ? $key : get_debug_type($key), CacheTag::class, CacheTags::class, @@ -59,6 +61,7 @@ public function withoutAutomaticTagging(\Closure $fn): mixed } $tags = $tags->with($yielded); + ++$position; } $result = $result->getReturn(); } diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index e575ca3..caee8a4 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -167,7 +167,7 @@ public function without_automatic_tagging_returns_generator_return_value(): void public function without_automatic_tagging_throws_logic_exception_for_invalid_yield(): void { $this->expectException(\LogicException::class); - $this->expectExceptionMessageMatches('/Invalid yielded value \(key: 0\)/'); + $this->expectExceptionMessageMatches('/Invalid yielded value at position 0 \(key: 0\)/'); $this->cacheActivator->withoutAutomaticTagging(static function () { yield 'not_a_cache_tag'; From 20d707306d2ce76caf38f24c1e7aefe93b28f8bb Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 11:19:00 +0100 Subject: [PATCH 11/16] Rename \$yielded to \$value in withoutAutomaticTagging generator loop Co-Authored-By: Claude Sonnet 4.6 --- src/CacheActivator.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 9cfcf43..827401a 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -48,19 +48,19 @@ public function withoutAutomaticTagging(\Closure $fn): mixed if ($result instanceof \Generator) { $position = 0; - foreach ($result as $key => $yielded) { - if (!$yielded instanceof CacheTag && !$yielded instanceof CacheTags) { + foreach ($result as $key => $value) { + if (!$value instanceof CacheTag && !$value instanceof CacheTags) { throw new \LogicException(\sprintf( 'Invalid yielded value at position %d (key: %s): Expected only "%s" or "%s", got "%s".', $position, \is_int($key) || \is_string($key) ? $key : get_debug_type($key), CacheTag::class, CacheTags::class, - get_debug_type($yielded), + get_debug_type($value), )); } - $tags = $tags->with($yielded); + $tags = $tags->with($value); ++$position; } $result = $result->getReturn(); From 4d6412839ebb458a04fdbe589ddf09cc80b76b2b Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 11:24:09 +0100 Subject: [PATCH 12/16] Make position counter 1-based by incrementing before the exception check Co-Authored-By: Claude Sonnet 4.6 --- src/CacheActivator.php | 3 ++- tests/Unit/CacheActivatorTest.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 827401a..6a5cc55 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -49,6 +49,8 @@ public function withoutAutomaticTagging(\Closure $fn): mixed if ($result instanceof \Generator) { $position = 0; foreach ($result as $key => $value) { + ++$position; + if (!$value instanceof CacheTag && !$value instanceof CacheTags) { throw new \LogicException(\sprintf( 'Invalid yielded value at position %d (key: %s): Expected only "%s" or "%s", got "%s".', @@ -61,7 +63,6 @@ public function withoutAutomaticTagging(\Closure $fn): mixed } $tags = $tags->with($value); - ++$position; } $result = $result->getReturn(); } diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index caee8a4..07cc850 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -167,7 +167,7 @@ public function without_automatic_tagging_returns_generator_return_value(): void public function without_automatic_tagging_throws_logic_exception_for_invalid_yield(): void { $this->expectException(\LogicException::class); - $this->expectExceptionMessageMatches('/Invalid yielded value at position 0 \(key: 0\)/'); + $this->expectExceptionMessageMatches('/Invalid yielded value at position 1 \(key: 0\)/'); $this->cacheActivator->withoutAutomaticTagging(static function () { yield 'not_a_cache_tag'; From d4c23f8219e6f315b5c1409cabc1f21e738c62ab Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 12:40:32 +0100 Subject: [PATCH 13/16] Document withoutAutomaticTagging() in disable caching behavior doc Co-Authored-By: Claude Sonnet 4.6 --- doc/8-disable-caching-behavior.md | 37 ++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/doc/8-disable-caching-behavior.md b/doc/8-disable-caching-behavior.md index 02771f2..5fead1c 100644 --- a/doc/8-disable-caching-behavior.md +++ b/doc/8-disable-caching-behavior.md @@ -14,8 +14,43 @@ To achieve this, you can use the `CacheActivator` to disable tagging and invalid self::getContainer()->get(CacheActivator::class)->deactivateCaching(); // Your test code here - + self::assertSame('this is amazing!', $result); } ``` +## Suppress automatic tagging for a specific code block + +Sometimes you need to load Pimcore elements for business logic without tagging the current response with those elements. For example, loading related products to calculate a price should not cause the response to be tagged with all those products. + +Use `CacheActivator::withoutAutomaticTagging()` to run a block of code with automatic tagging suppressed: + +```php +$result = $cacheActivator->withoutAutomaticTagging(function () use ($id) { + // Automatic tagging is disabled here — loading this element + // will not tag the current response. + return $this->repository->find($id); +}); +``` + +### Selectively tagging within the block + +If you still want to tag the response with specific tags inside the block, use a generator and `yield` the tags you need: + +```php +$result = $cacheActivator->withoutAutomaticTagging(function () use ($id) { + $element = $this->repository->find($id); + + // Yield only the tags you explicitly want applied to the response. + yield CacheTag::fromElement($element); + + return $element; +}); +``` + +You can yield both `CacheTag` and `CacheTags` objects. All yielded tags are collected and applied to the response after the block completes. The return value of the generator is returned by `withoutAutomaticTagging()`. + +### State restoration + +The previous caching state is always restored after the block completes, even if an exception is thrown. If caching was already disabled before calling `withoutAutomaticTagging()`, it remains disabled afterwards. + From bf6b3a9b323c3f6b5b64e40d3082fbfac72ee8ab Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 15:37:03 +0100 Subject: [PATCH 14/16] Add integration tests for withoutAutomaticTagging() Tests cover: - Asset loaded inside the block is not automatically tagged - Tag manually yielded inside the block does appear in the response Co-Authored-By: Claude Sonnet 4.6 --- .../without_automatic_tagging_route.php | 9 +++ .../Tagging/WithoutAutomaticTaggingTest.php | 63 +++++++++++++++++++ .../WithoutAutomaticTaggingController.php | 41 ++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 tests/Integration/Fixtures/without_automatic_tagging_route.php create mode 100644 tests/Integration/Tagging/WithoutAutomaticTaggingTest.php create mode 100644 tests/app/src/Controller/WithoutAutomaticTaggingController.php diff --git a/tests/Integration/Fixtures/without_automatic_tagging_route.php b/tests/Integration/Fixtures/without_automatic_tagging_route.php new file mode 100644 index 0000000..802031d --- /dev/null +++ b/tests/Integration/Fixtures/without_automatic_tagging_route.php @@ -0,0 +1,9 @@ +add('without_automatic_tagging', '/without-automatic-tagging') + ->controller(WithoutAutomaticTaggingController::class); +}; diff --git a/tests/Integration/Tagging/WithoutAutomaticTaggingTest.php b/tests/Integration/Tagging/WithoutAutomaticTaggingTest.php new file mode 100644 index 0000000..0338068 --- /dev/null +++ b/tests/Integration/Tagging/WithoutAutomaticTaggingTest.php @@ -0,0 +1,63 @@ +client = self::createClient(); + } + + /** + * @test + */ + #[ConfigureExtension('neusta_pimcore_http_cache', [ + 'elements' => [ + 'assets' => true, + ], + ])] + public function response_is_not_tagged_when_asset_is_loaded_without_automatic_tagging(): void + { + self::arrange(static fn () => TestAssetFactory::simpleAsset()->save()); + + $this->client->request('GET', '/without-automatic-tagging?id=42'); + + $response = $this->client->getResponse(); + self::assertSame(200, $response->getStatusCode()); + self::assertNull($response->headers->get('X-Cache-Tags')); + } + + /** + * @test + */ + #[ConfigureExtension('neusta_pimcore_http_cache', [ + 'elements' => [ + 'assets' => true, + ], + ])] + public function response_is_tagged_with_manually_yielded_tag_when_loaded_without_automatic_tagging(): void + { + self::arrange(static fn () => TestAssetFactory::simpleAsset()->save()); + + $this->client->request('GET', '/without-automatic-tagging?id=42&yield=true'); + + $response = $this->client->getResponse(); + self::assertSame(200, $response->getStatusCode()); + self::assertSame('a42', $response->headers->get('X-Cache-Tags')); + } +} diff --git a/tests/app/src/Controller/WithoutAutomaticTaggingController.php b/tests/app/src/Controller/WithoutAutomaticTaggingController.php new file mode 100644 index 0000000..58476ea --- /dev/null +++ b/tests/app/src/Controller/WithoutAutomaticTaggingController.php @@ -0,0 +1,41 @@ +query->get('id'); + $shouldYield = $request->query->getBoolean('yield', false); + + $asset = $this->cacheActivator->withoutAutomaticTagging(function () use ($id, $shouldYield) { + $asset = Asset::getById($id); + + if ($shouldYield && $asset) { + yield CacheTag::fromElement($asset); + } + + return $asset; + }); + + if (!$asset) { + return new Response('Asset not found', Response::HTTP_NOT_FOUND); + } + + return (new Response($asset->getData())) + ->setSharedMaxAge(3600) + ->setPublic(); + } +} From 95ef6f3eb1cc31fe7413fb084381e43ed001b7b3 Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Sun, 1 Mar 2026 15:38:50 +0100 Subject: [PATCH 15/16] Fix review findings: trailing blank line, reserved keyword param name, int cast Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/Tagging/WithoutAutomaticTaggingTest.php | 2 +- tests/Unit/CacheActivatorTest.php | 1 - .../app/src/Controller/WithoutAutomaticTaggingController.php | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/Integration/Tagging/WithoutAutomaticTaggingTest.php b/tests/Integration/Tagging/WithoutAutomaticTaggingTest.php index 0338068..49fc740 100644 --- a/tests/Integration/Tagging/WithoutAutomaticTaggingTest.php +++ b/tests/Integration/Tagging/WithoutAutomaticTaggingTest.php @@ -54,7 +54,7 @@ public function response_is_tagged_with_manually_yielded_tag_when_loaded_without { self::arrange(static fn () => TestAssetFactory::simpleAsset()->save()); - $this->client->request('GET', '/without-automatic-tagging?id=42&yield=true'); + $this->client->request('GET', '/without-automatic-tagging?id=42&manual_tag=true'); $response = $this->client->getResponse(); self::assertSame(200, $response->getStatusCode()); diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index 07cc850..abd1aa5 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -173,5 +173,4 @@ public function without_automatic_tagging_throws_logic_exception_for_invalid_yie yield 'not_a_cache_tag'; }); } - } diff --git a/tests/app/src/Controller/WithoutAutomaticTaggingController.php b/tests/app/src/Controller/WithoutAutomaticTaggingController.php index 58476ea..c78da03 100644 --- a/tests/app/src/Controller/WithoutAutomaticTaggingController.php +++ b/tests/app/src/Controller/WithoutAutomaticTaggingController.php @@ -17,8 +17,8 @@ public function __construct( public function __invoke(Request $request): Response { - $id = $request->query->get('id'); - $shouldYield = $request->query->getBoolean('yield', false); + $id = (int) $request->query->get('id'); + $shouldYield = $request->query->getBoolean('manual_tag', false); $asset = $this->cacheActivator->withoutAutomaticTagging(function () use ($id, $shouldYield) { $asset = Asset::getById($id); From f1c9e9b4df3c352e2722ecedf1a96f2a0f625d75 Mon Sep 17 00:00:00 2001 From: Jan Adams Date: Mon, 2 Mar 2026 07:03:30 +0100 Subject: [PATCH 16/16] Revert $value to $yielded and position to index in withoutAutomaticTagging Co-Authored-By: Claude Sonnet 4.6 --- src/CacheActivator.php | 16 ++++++++-------- tests/Unit/CacheActivatorTest.php | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/CacheActivator.php b/src/CacheActivator.php index 6a5cc55..6f0a46d 100644 --- a/src/CacheActivator.php +++ b/src/CacheActivator.php @@ -47,22 +47,22 @@ public function withoutAutomaticTagging(\Closure $fn): mixed $result = $fn(); if ($result instanceof \Generator) { - $position = 0; - foreach ($result as $key => $value) { - ++$position; + $index = 0; + foreach ($result as $key => $yielded) { + ++$index; - if (!$value instanceof CacheTag && !$value instanceof CacheTags) { + if (!$yielded instanceof CacheTag && !$yielded instanceof CacheTags) { throw new \LogicException(\sprintf( - 'Invalid yielded value at position %d (key: %s): Expected only "%s" or "%s", got "%s".', - $position, + 'Invalid yielded value at index %d (key: %s): Expected only "%s" or "%s", got "%s".', + $index, \is_int($key) || \is_string($key) ? $key : get_debug_type($key), CacheTag::class, CacheTags::class, - get_debug_type($value), + get_debug_type($yielded), )); } - $tags = $tags->with($value); + $tags = $tags->with($yielded); } $result = $result->getReturn(); } diff --git a/tests/Unit/CacheActivatorTest.php b/tests/Unit/CacheActivatorTest.php index abd1aa5..00a49d1 100644 --- a/tests/Unit/CacheActivatorTest.php +++ b/tests/Unit/CacheActivatorTest.php @@ -167,7 +167,7 @@ public function without_automatic_tagging_returns_generator_return_value(): void public function without_automatic_tagging_throws_logic_exception_for_invalid_yield(): void { $this->expectException(\LogicException::class); - $this->expectExceptionMessageMatches('/Invalid yielded value at position 1 \(key: 0\)/'); + $this->expectExceptionMessageMatches('/Invalid yielded value at index 1 \(key: 0\)/'); $this->cacheActivator->withoutAutomaticTagging(static function () { yield 'not_a_cache_tag';