Make the tracking snippet cache safe

CLAUDE.md

@@ -37,9 +37,9 @@ Note: `phpunit.xml.dist` bootstraps `tests/Application/config/bootstrap.php`, an
 The plugin tracks *where a visitor came from* and later ties that to the order they place. The flow has two halves:
 
 **Capture (visitor side):**
-1. `EventSubscriber\AddJavascriptSubscriber` (only loaded when `javascript.enabled` config is true, and requires `setono/tag-bag-bundle` — the extension throws a LogicException if it is missing) injects a JS `fetch()` POST to `/track` on GET HTML requests. It skips bots (`setono/bot-detection-bundle`) and skips visitors seen within `session_timeout` (default 1800s), using the cookie from `setono/client-bundle`.
-2. `Controller\Action\TrackAction` (route `setono_sylius_conversion_attribution_global_track`) maps the JSON payload to `ClientInformation\ClientInformation` via `#[MapRequestPayload]` and persists a `Model\Source` row (client id, page, referrer, source/medium/campaign).
-3. `Resolver\ClientInformationResolver` builds that payload server-side at injection time: client id from client-bundle's `ClientContext`, plus source/medium/campaign from the matcher chain (falls back to source `direct`).
+1. `EventSubscriber\AddJavascriptSubscriber` (only loaded when `javascript.enabled` config is true, and requires `setono/tag-bag-bundle` — the extension throws a LogicException if it is missing) injects a **static** JS snippet on main-request GET HTML responses. The snippet is byte-identical for every visitor so full page caches can safely store it (issue #7) — nothing per-visitor may influence the snippet or the injection decision. The JS skips automated browsers (`navigator.webdriver`), throttles itself via a rolling `localStorage` last-seen timestamp (`session_timeout`, default 1800s), bypassed when the URL has a query string or the referrer is cross-host, and POSTs only `{page, referrer}` to `/track`.
+2. `Controller\Action\TrackAction` (route `setono_sylius_conversion_attribution_global_track`) maps the JSON payload to `ClientInformation\ClientInformation` via `#[MapRequestPayload]`, drops bot traffic (`setono/bot-detection-bundle`), fills fields missing from the payload by running `ClientInformationResolver` against a synthetic `Request` built from the posted page/referrer plus the real request's IP/UA, and persists a `Model\Source` row. Posted values always win over resolved ones — pages cached with the pre-1.2 snippet keep POSTing full payloads (always containing `clientId` + `source`), which therefore skip resolution entirely.
+3. `Resolver\ClientInformationResolver` resolves the client id from client-bundle's `ClientContext` (which reads the `setono_client_id` cookie riding on the POST request), plus source/medium/campaign from the matcher chain (falls back to source `direct`).
 
 **Source matching chain:** `Matcher\CompositeSourceMatcher` is populated via `CompositeCompilerPass` from services tagged `setono_sylius_conversion_attribution.source_matcher` (see `SetonoSyliusConversionAttributionPlugin::build()`). First non-null match wins. Built-in matchers, in priority order (priorities set in `services/matcher.xml`):
 - `UtmQueryParameterBasedSourceMatcher` — utm_source/utm_medium/utm_campaign
@@ -82,6 +82,8 @@ This is a published library: adding a required constructor argument to a service
 4. Still inject it in the plugin's own XML service definition (also appended last) so normal installs get full functionality — the nullability only cushions downstream overrides.
 5. Add a `@deprecated will be required in 2.0` comment on the property so it's greppable alongside the `trigger_deprecation()` call.
 
-**Removing the shim in the next major:** grep the `src/` tree for `trigger_deprecation` — each hit is a nullable argument to make required (drop the `?`/`= null`, delete the `trigger_deprecation` block and the null-guards). Current shims: `EventSubscriber\AddJavascriptSubscriber` (`$sourceMatcher`) and `Controller\Action\TrackAction` (`$botDetector`).
+**Removing the shim in the next major:** grep the `src/` tree for `trigger_deprecation` — each hit is a nullable argument to make required (drop the `?`/`= null`, delete the `trigger_deprecation` block and the null-guards). Current shims: `Controller\Action\TrackAction` (`$botDetector`, `$clientInformationResolver`).
+
+**Unused-but-kept arguments:** `EventSubscriber\AddJavascriptSubscriber`'s constructor keeps four arguments it no longer uses (`$clientInformationResolver`, `$botDetector`, `$cookieProvider`, `$sourceMatcher`) purely for signature compatibility — removing or reordering them would break host apps that redefine the service. They do **not** trigger deprecations and are dropped in 2.0; grep for `@deprecated unused`.
 
 Test the legacy path by constructing the service without the new argument and asserting the deprecation fires (see `it_triggers_a_deprecation_*` tests, which register a temporary `E_USER_DEPRECATED` handler). Mark such tests `@group legacy`.

README.md

@@ -102,15 +102,33 @@ php bin/console setono:sylius-conversion-attribution:prune
 - The default JavaScript injection requires [`setono/tag-bag-bundle`](https://github.com/Setono/TagBagBundle).
   Without it, enabling the `javascript` feature throws at container build time. Install it, or set
   `setono_sylius_conversion_attribution.javascript.enabled: false` and inject the tracking snippet yourself.
-- Bot filtering (both for the injected snippet and the `/track` endpoint) is provided by
-  `setono/bot-detection-bundle`, which is installed automatically as a dependency.
+- Bot filtering (for the `/track` endpoint) is provided by `setono/bot-detection-bundle`, which is
+  installed automatically as a dependency. Most bots are filtered structurally anyway: they don't
+  execute the JavaScript snippet, so they never POST to `/track`. Automated browsers that *do*
+  execute JavaScript (Selenium, Puppeteer, Playwright, headless Chrome, …) are skipped by the
+  snippet itself via the standardized `navigator.webdriver` flag — this also keeps your own
+  end-to-end test runs out of the attribution data.
 
 ### Full page cache
 
-The injected tracking snippet embeds a server-resolved client id. If a full page / HTTP cache stores
-the rendered HTML, every visitor served that cached page shares the same embedded client id, which
-corrupts attribution. Exclude pages carrying the snippet from full page caching, or only enable the
-feature on responses that are not cached.
+The injected tracking snippet is static — byte-identical for every visitor — so pages carrying it
+are safe to store in a full page / HTTP cache (Varnish, a CDN, Symfony HttpCache, …). Nothing
+per-visitor is rendered into the HTML: the client id travels on the `setono_client_id` cookie that
+accompanies the snippet's `POST /track` request, and the traffic source is matched server side at
+POST time from the posted page URL and referrer. The session throttle (`session_timeout`) is
+evaluated in the browser against a rolling `localStorage` timestamp; a query string or a cross-host
+referrer bypasses it so campaign clicks are never lost.
+
+Two operational notes:
+
+- **Purge your page cache after upgrading to 1.2.** Pages cached with the pre-1.2 snippet keep
+  POSTing the client id that was baked in when the cache was filled (the `/track` endpoint still
+  accepts those payloads, so nothing breaks — but their attribution stays wrong until they expire).
+- Make sure your layout renders the tag bag on cacheable pages. If a page never renders it, the tag
+  bag stores the pending snippet in the session, which can make the response uncacheable.
+
+If your cache strips `Set-Cookie` from cached responses (it generally should), first-time visitors
+get their client cookie on the uncached `POST /track` response instead, so attribution still works.
 
 ### Privacy / GDPR
 

src/Controller/Action/TrackAction.php

@@ -9,6 +9,8 @@
 use Setono\Doctrine\ORMTrait;
 use Setono\SyliusConversionAttributionPlugin\ClientInformation\ClientInformation;
 use Setono\SyliusConversionAttributionPlugin\Factory\SourceFactoryInterface;
+use Setono\SyliusConversionAttributionPlugin\Resolver\ClientInformationResolverInterface;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Attribute\MapRequestPayload;
 
@@ -22,28 +24,93 @@ public function __construct(
         // Nullable and last for backwards compatibility: when not injected, bot filtering is skipped.
         // @deprecated will be required in 2.0 — grep "trigger_deprecation" to find shims to drop.
         private readonly ?BotDetectorInterface $botDetector = null,
+        // Nullable and last for backwards compatibility: when not injected, fields missing from
+        // the posted payload are simply not resolved server side.
+        // @deprecated will be required in 2.0 — grep "trigger_deprecation" to find shims to drop.
+        private readonly ?ClientInformationResolverInterface $clientInformationResolver = null,
     ) {
         $this->managerRegistry = $managerRegistry;
 
         if (null === $botDetector) {
             trigger_deprecation(
                 'setono/sylius-conversion-attribution-plugin',
-                '1.1',
+                '1.2',
                 'Not passing an instance of "%s" as argument "$botDetector" to "%s()" is deprecated and will be required in 2.0.',
                 BotDetectorInterface::class,
                 __METHOD__,
             );
         }
+
+        if (null === $clientInformationResolver) {
+            trigger_deprecation(
+                'setono/sylius-conversion-attribution-plugin',
+                '1.2',
+                'Not passing an instance of "%s" as argument "$clientInformationResolver" to "%s()" is deprecated and will be required in 2.0.',
+                ClientInformationResolverInterface::class,
+                __METHOD__,
+            );
+        }
     }
 
-    public function __invoke(#[MapRequestPayload] ClientInformation $clientInformation): Response
-    {
+    public function __invoke(
+        #[MapRequestPayload]
+        ClientInformation $clientInformation,
+        ?Request $request = null,
+    ): Response {
         // The endpoint is anonymous; drop bot traffic (matched on the real request User-Agent)
         // so automated hits don't flood the source table
         if (null !== $this->botDetector && $this->botDetector->isBotRequest()) {
             return new Response(status: Response::HTTP_NO_CONTENT);
         }
 
+        // The cache-safe snippet (1.2+) posts only page and referrer; everything per-visitor is
+        // resolved here, where the real request carries the setono_client_id cookie. Pages cached
+        // before the upgrade keep POSTing full pre-1.2 payloads (clientId, source, ...) for their
+        // TTL, so posted values always win and such payloads skip resolution entirely (they always
+        // contain both clientId and source).
+        if (null !== $this->clientInformationResolver &&
+            null !== $request &&
+            null !== $clientInformation->page &&
+            (null === $clientInformation->clientId || null === $clientInformation->source)
+        ) {
+            $server = [];
+
+            $ip = $request->getClientIp();
+            if (null !== $ip) {
+                $server['REMOTE_ADDR'] = $ip;
+            }
+
+            $userAgent = $request->headers->get('user-agent');
+            if (null !== $userAgent) {
+                $server['HTTP_USER_AGENT'] = $userAgent;
+            }
+
+            if (null !== $clientInformation->referrer && '' !== $clientInformation->referrer) {
+                $server['HTTP_REFERER'] = $clientInformation->referrer;
+            }
+
+            try {
+                // Reuses the whole existing resolution pipeline: the matcher chain sees the posted
+                // page's query parameters and host plus the posted referrer, while the client id is
+                // read by ClientContext from the cookie on the CURRENT (POST) request
+                $resolved = $this->clientInformationResolver->resolve(
+                    Request::create($clientInformation->page, server: $server),
+                );
+
+                $clientInformation->clientId ??= $resolved->clientId;
+                // Overlay ip/userAgent from the real request, not from $resolved: if the real
+                // request lacks them, Request::create()'s defaults (127.0.0.1, "Symfony") must
+                // never leak into the database
+                $clientInformation->ip ??= $ip;
+                $clientInformation->userAgent ??= $userAgent;
+                $clientInformation->source ??= $resolved->source;
+                $clientInformation->medium ??= $resolved->medium;
+                $clientInformation->campaign ??= $resolved->campaign;
+            } catch (\Throwable) {
+                // an unparsable posted page URL must not turn into a 500 — persist the payload as-is
+            }
+        }
+
         $source = $this->sourceFactory->createFromClientInformation($clientInformation);
 
         $manager = $this->getManager($source::class);

src/EventSubscriber/AddJavascriptSubscriber.php

@@ -19,25 +19,17 @@ final class AddJavascriptSubscriber implements EventSubscriberInterface
 {
     public function __construct(
         private readonly TagBagInterface $tagBag,
+        // @deprecated unused since 1.2, kept for signature compatibility — remove in 2.0
         private readonly ClientInformationResolverInterface $clientInformationResolver,
         private readonly UrlGeneratorInterface $urlGenerator,
+        // @deprecated unused since 1.2, kept for signature compatibility — remove in 2.0
         private readonly BotDetectorInterface $botDetector,
+        // @deprecated unused since 1.2, kept for signature compatibility — remove in 2.0
         private readonly CookieProviderInterface $cookieProvider,
         private readonly int $sessionTimeout,
-        // Nullable and last for backwards compatibility: when not injected, mid-session campaign
-        // detection is simply skipped and the previous first-touch-per-session behaviour applies.
-        // @deprecated will be required in 2.0 — grep "trigger_deprecation" to find shims to drop.
+        // @deprecated unused since 1.2, kept for signature compatibility — remove in 2.0
         private readonly ?SourceMatcherInterface $sourceMatcher = null,
     ) {
-        if (null === $sourceMatcher) {
-            trigger_deprecation(
-                'setono/sylius-conversion-attribution-plugin',
-                '1.1',
-                'Not passing an instance of "%s" as argument "$sourceMatcher" to "%s()" is deprecated and will be required in 2.0.',
-                SourceMatcherInterface::class,
-                __METHOD__,
-            );
-        }
     }
 
     public static function getSubscribedEvents(): array
@@ -51,50 +43,87 @@ public function addJavascript(RequestEvent $event): void
     {
         $request = $event->getRequest();
 
+        // Only conditions that are part of a full page cache's key (method, path, format) may
+        // influence whether the snippet is injected: the rendered HTML is shared between visitors,
+        // so any per-visitor decision (bot detection, session freshness) would be frozen into the
+        // cache by whichever visitor populated it. Per-visitor logic lives in the snippet itself
+        // (session throttle) or in TrackAction at POST time (bot filtering, client id, matching).
         if (!$event->isMainRequest() ||
             $request->isXmlHttpRequest() ||
             !$request->isMethod('GET') ||
-            !str_contains((string) $request->getRequestFormat(), 'html') ||
-            $this->botDetector->isBotRequest($request)
+            !str_contains((string) $request->getRequestFormat(), 'html')
         ) {
             return;
         }
 
-        // A request that carries campaign markers (utm/click id/cross-host referrer) is always
-        // tracked, even mid-session, so a paid click arriving after an organic entry isn't lost
-        $hasCampaign = null !== $this->sourceMatcher && null !== $this->sourceMatcher->match($request);
+        // %d = session timeout in milliseconds, %s = JSON-encoded track URL.
+        // NOTE: this is a sprintf template — a literal % in the JS must be written as %%.
+        // The snippet is deliberately identical for every visitor so full page caches can safely
+        // store it: the client id rides the setono_client_id cookie on the fetch() POST, and the
+        // session throttle is evaluated in the browser against a rolling localStorage timestamp.
+        // A query string or a cross-host referrer (campaign hints) bypasses the throttle; the
+        // server side source matcher makes the definitive call at POST time.
+        $javascript = <<<'JS'
+(function () {
+    // Automated browsers (Selenium, Puppeteer, Playwright, headless Chrome, ...) execute
+    // JavaScript, so they sail past the "bots don't run JS" filter, and many of them use a
+    // real browser User-Agent that the server side bot detection on /track cannot flag.
+    // They do expose the standardized automation flag, though — skip them here. Real
+    // browsers leave the flag false or undefined, so nothing legitimate is lost.
+    if (navigator.webdriver) {
+        return;
+    }
+
+    var now = Date.now();
+    var last = null;
 
-        if (!$hasCampaign) {
-            $clientCookie = $this->cookieProvider->getCookie();
-            if (null !== $clientCookie && $clientCookie->lastSeenAt >= (time() - $this->sessionTimeout)) {
-                return;
-            }
+    try {
+        last = parseInt(window.localStorage.getItem('setono_conversion_attribution_last_seen'), 10);
+        window.localStorage.setItem('setono_conversion_attribution_last_seen', String(now));
+    } catch (e) {
+        // storage unavailable (private mode, storage disabled): track every page view
+    }
+
+    var campaignHint = window.location.search !== '';
+    if (!campaignHint && document.referrer !== '') {
+        try {
+            campaignHint = new URL(document.referrer).host !== window.location.host;
+        } catch (e) {
+            campaignHint = true;
         }
+    }
 
-        $clientInformation = $this->clientInformationResolver->resolve($request);
-        $javascript = <<<'JS'
-fetch('%s', {
-    method: "POST",
-    headers: {
-      "Content-Type": "application/json"
-    },
-    body: '%s'
-});
+    if (!campaignHint && last !== null && !isNaN(last) && now - last < %d) {
+        return;
+    }
+
+    var payload = {page: window.location.href};
+    if (document.referrer !== '') {
+        payload.referrer = document.referrer;
+    }
+
+    fetch(%s, {
+        method: 'POST',
+        credentials: 'same-origin',
+        keepalive: true,
+        headers: {'Content-Type': 'application/json'},
+        body: JSON.stringify(payload)
+    });
+})();
 JS;
 
         try {
-            $javascript = sprintf(
+            $this->tagBag->add(InlineScriptTag::create(sprintf(
                 $javascript,
-                $this->urlGenerator->generate('setono_sylius_conversion_attribution_global_track'),
-                // The payload is interpolated raw into an inline <script>, so the encoding must
-                // neutralize ', ", <, >, & to prevent breaking out of the JS string or the script tag
+                $this->sessionTimeout * 1000,
+                // The URL is interpolated raw into an inline <script>, so the encoding must
+                // neutralize ', ", <, >, & to prevent breaking out of the JS string or the script
+                // tag. json_encode() emits the surrounding double quotes itself.
                 json_encode(
-                    $clientInformation,
+                    $this->urlGenerator->generate('setono_sylius_conversion_attribution_global_track'),
                     \JSON_THROW_ON_ERROR | \JSON_HEX_TAG | \JSON_HEX_APOS | \JSON_HEX_QUOT | \JSON_HEX_AMP,
                 ),
-            );
-
-            $this->tagBag->add(InlineScriptTag::create($javascript));
+            )));
         } catch (\JsonException) {
         }
     }