Add withoutAutomaticTagging method to CacheActivator

[jdreesen]

This allows the automatic tagging to be disabled for certain code paths.

Summary by CodeRabbit

• Performance Improvements

  • Enhanced HTTP cache handling with optimized lazy-loading of cache components for improved efficiency.

• Enhancements

  • Added capability to temporarily disable automatic caching with automatic tag aggregation and restoration to original state.

[coderabbitai]

Walkthrough

The PR introduces lazy-loaded dependency injection for the response tagger in the CacheActivator service via service closure, and adds a new withoutAutomaticTagging() method that allows code to temporarily disable automatic caching while collecting cache tags for later application.

Changes

Cohort / File(s)	Summary
Service Configuration config/services.php	Added import for service_closure and updated neusta_pimcore_http_cache.cache_activator service wiring to inject the response tagger via closure for lazy resolution.
CacheActivator Logic src/CacheActivator.php	Added constructor to accept $responseTagger closure, introduced withoutAutomaticTagging() method to temporarily suspend automatic caching while collecting and validating cache tags from a callable, then restore state and apply tags.

Sequence Diagram

sequenceDiagram
    actor Caller
    participant CacheActivator
    participant ClosureFn as Closure ($fn)
    participant TagCollection as CacheTags
    participant ResponseTagger

    Caller->>CacheActivator: withoutAutomaticTagging($fn)
    CacheActivator->>CacheActivator: Disable automatic tagging
    CacheActivator->>ClosureFn: Execute closure
    ClosureFn->>CacheActivator: yield CacheTag/CacheTags
    CacheActivator->>TagCollection: Aggregate yielded tags
    CacheActivator->>CacheActivator: Validate tags
    ClosureFn-->>CacheActivator: Return result
    CacheActivator->>CacheActivator: Restore automatic tagging state
    CacheActivator->>ResponseTagger: Apply accumulated tags
    CacheActivator-->>Caller: Return closure result

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Define public autowiring API #32: Established the neusta_pimcore_http_cache.cache_activator service definition that is now being updated with lazy-loaded response tagger injection and corresponding constructor changes.

Suggested reviewers

• jan888adams

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the primary change: introducing a new withoutAutomaticTagging method to the CacheActivator class, which is confirmed by the substantial code additions in src/CacheActivator.php.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch conveniently-disable-auto-tagging

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jan888adams]

I’m not a fan of the MR title. It explains the changes, but it doesn’t clearly describe the feature.

In my understanding (and preference), a good title should not describe the implementation, it should describe intent. What do you think?

I think a better title would be: “Allow suppressing automatic cache tagging for specific code blocks.”

[jan888adams]

I really like the approach with the closure. When I first heard about it, I was a little unsure, but after seeing the code, it feels like a very elegant solution.

At first, the use of yield made me wonder if it might be a bit overengineered, but now that I see the overall picture, it actually comes together nicely and feels like a clean way to handle this.

I’m still wondering whether CacheActivator is the best place for this method, although I don’t currently have a better suggestion. I also briefly thought about if there may be a better name for the method, but the current name does describe well what’s happening.

Regarding some of the variable names, they felt a bit unfamiliar to me at first, but within the context of the method they make sense.

Thanks for sharing this approach

[jan888adams]

I created a brach, with testcases and documentation: for this MR:
see: #43