Filter bots before application listeners enrich the event

README.md

@@ -152,20 +152,21 @@ event first, then leaves a gap for your own listeners, then filters and sends:
 | `PRIORITY_POPULATE` (1000)            | `PopulateRequestPropertiesSubscriber`             | Source url, client ip and user agent from the request   |
 | 900                                   | `PopulateFbpAndFbcPropertiesSubscriber`           | `fbp` and `fbc`                                         |
 | 800                                   | `PopulateTestEventCodePropertySubscriber`         | Test event code                                         |
-| 700                                   | `PopulatePixelsSubscriber`                        | Pixels from the pixel provider                          |
+| 650                                   | `FilterEmptyUserAgentSubscriber`                  | Stops events without a user agent                       |
+| 625                                   | `FilterConfiguredUserAgentsSubscriber`            | Stops events matching `filters.user_agent`              |
+| `PRIORITY_FILTER` (600)               | `FilterBotsSubscriber`                            | Stops events from bots                                  |
+| 500                                   | `PopulatePixelsSubscriber`                        | Pixels from the pixel provider                          |
 | **`PRIORITY_ENRICH` (0)**             | **your listeners**                                | **Email, phone, external id, custom data**              |
-| -850                                  | `FilterEmptyUserAgentSubscriber`                  | Stops events without a user agent                       |
-| -875                                  | `FilterConfiguredUserAgentsSubscriber`            | Stops events matching `filters.user_agent`              |
-| `PRIORITY_FILTER` (-900)              | `FilterBotsSubscriber`                            | Stops events from bots                                  |
 | -950                                  | `StopPropagationIfNoPixelsHasBeenAddedSubscriber` | Stops events without pixels                             |
 | `PRIORITY_SEND` (-1000)               | `AddEventToTagBagSubscriber`                      | Renders the `fbq()` calls (client side)                 |
 | `PRIORITY_SEND` (-1000)               | `DispatchOnCommandBusSubscriber`                  | Dispatches `SendEvent` (server side)                    |
 
 Two things follow from this:
 
 - **Enrich at `PRIORITY_ENRICH`**, which is the default priority of any listener. Everything the bundle knows about
-  the request is populated by then, and nothing has been filtered or sent yet.
-- **A listener below `PRIORITY_FILTER` may never run**, because the filters stop propagation.
+  the request is populated by then, and traffic the bundle does not want to track has already been discarded, so
+  your listeners never do work for a bot.
+- **A listener below `PRIORITY_ENRICH` may never run**, because propagation can already have been stopped.
 
 The constants live on `ConversionsApiEventRaised`, so you can position your listener without hard coding a number.
 

UPGRADE.md

@@ -75,6 +75,18 @@ anything to `framework.messenger`. `SendEvent` is dispatched on your application
   `?ConsentContextInterface $consentContext` and `bool $consentEnabled` / `bool $clientSideEnabled` /
   `bool $serverSideEnabled` arguments. Adapt subclasses, decorators and custom service definitions.
 
+## Event pipeline
+
+`ConversionsApiEventRaised` now carries `PRIORITY_POPULATE`, `PRIORITY_FILTER`, `PRIORITY_ENRICH` and
+`PRIORITY_SEND` constants. Use them instead of hard coded numbers.
+
+The bot and user agent filters moved from -850/-875/-900 to 650/625/600, i.e. **above** the priority your own
+listeners run at, so enrichment is no longer performed for traffic that is discarded straight after. If you
+registered a listener between the old and the new filter positions expecting it to run for every event, move it to
+`PRIORITY_ENRICH`.
+
+`PopulatePixelsSubscriber` moved from 700 to 500 so the filters sit between the request populators and it.
+
 ## Pixel access token
 
 `pixels[].access_token` is no longer required. Client side tracking only needs the pixel id, so a client-side-only

src/Event/ConversionsApiEventRaised.php

@@ -13,14 +13,15 @@
  * The bundle's own listeners run in four bands. Use the constants below to position your own listener relative to
  * them instead of hard coding a number:
  *
- * | Priority                       | What happens                                                          |
- * |--------------------------------|-----------------------------------------------------------------------|
+ * | Priority                       | What happens                                                            |
+ * |--------------------------------|-------------------------------------------------------------------------|
  * | PRIORITY_POPULATE (and below)  | The bundle fills in request properties, fbp/fbc, test event code, pixels |
- * | PRIORITY_ENRICH                | Your listeners add user data and custom data                          |
  * | PRIORITY_FILTER                | The bundle drops events it should not track (bots, filtered user agents) |
- * | PRIORITY_SEND                  | The bundle renders the client side tags and dispatches the command      |
+ * | PRIORITY_ENRICH                | Your listeners add user data and custom data                            |
+ * | PRIORITY_SEND                  | The bundle renders the client side tags and dispatches the command       |
  *
- * A listener below PRIORITY_FILTER may never run, because the filters stop propagation
+ * Filtering happens before PRIORITY_ENRICH so that the work your listeners do is not spent on traffic that is
+ * discarded anyway. A listener below PRIORITY_ENRICH may never run, because propagation can already be stopped
  */
 final class ConversionsApiEventRaised extends StoppableEvent
 {
@@ -30,15 +31,17 @@ final class ConversionsApiEventRaised extends StoppableEvent
     public const PRIORITY_POPULATE = 1000;
 
     /**
-     * The priority your own listeners should use. Everything the bundle knows about the request is populated by
-     * now, and nothing has been filtered or sent yet. This is the default priority of an event listener
+     * The bundle decides here whether the event should be tracked at all. This runs before PRIORITY_ENRICH so
+     * that enrichment is not performed for bots and other traffic that is discarded anyway
      */
-    public const PRIORITY_ENRICH = 0;
+    public const PRIORITY_FILTER = 600;
 
     /**
-     * The bundle decides here whether the event should be tracked at all
+     * The priority your own listeners should use. Everything the bundle knows about the request is populated by
+     * now, traffic the bundle does not want to track has already been discarded, and nothing has been sent yet.
+     * This is the default priority of an event listener
      */
-    public const PRIORITY_FILTER = -900;
+    public const PRIORITY_ENRICH = 0;
 
     /**
      * The bundle hands the event to the tag bag and the command bus at this priority

src/EventSubscriber/PopulatePixelsSubscriber.php

@@ -17,7 +17,7 @@ public function __construct(private readonly PixelProviderInterface $pixelProvid
     public static function getSubscribedEvents(): array
     {
         return [
-            ConversionsApiEventRaised::class => ['populate', ConversionsApiEventRaised::PRIORITY_POPULATE - 300],
+            ConversionsApiEventRaised::class => ['populate', ConversionsApiEventRaised::PRIORITY_ENRICH + 500],
         ];
     }
 

tests/Unit/Event/ConversionsApiEventRaisedTest.php

@@ -33,10 +33,10 @@ final class ConversionsApiEventRaisedTest extends TestCase
         PopulateRequestPropertiesSubscriber::class,
         PopulateFbpAndFbcPropertiesSubscriber::class,
         PopulateTestEventCodePropertySubscriber::class,
-        PopulatePixelsSubscriber::class,
         FilterEmptyUserAgentSubscriber::class,
         FilterConfiguredUserAgentsSubscriber::class,
         FilterBotsSubscriber::class,
+        PopulatePixelsSubscriber::class,
         StopPropagationIfNoPixelsHasBeenAddedSubscriber::class,
         AddEventToTagBagSubscriber::class,
         DispatchOnCommandBusSubscriber::class,
@@ -84,13 +84,31 @@ public function everything_is_populated_before_your_listeners_run(): void
         }
     }
 
+    /**
+     * Bot traffic must be discarded before the application spends anything on enriching the event
+     */
     #[Test]
-    public function filtering_and_sending_happen_after_your_listeners(): void
+    public function the_request_filters_run_before_your_listeners(): void
     {
         foreach ([
             FilterEmptyUserAgentSubscriber::class,
             FilterConfiguredUserAgentsSubscriber::class,
             FilterBotsSubscriber::class,
+        ] as $subscriber) {
+            $priority = self::priority($subscriber);
+
+            self::assertGreaterThan(ConversionsApiEventRaised::PRIORITY_ENRICH, $priority);
+
+            // ... but after the request properties they filter on have been populated
+            self::assertLessThan(self::priority(PopulateRequestPropertiesSubscriber::class), $priority);
+        }
+    }
+
+    #[Test]
+    public function sending_happens_after_your_listeners(): void
+    {
+        foreach ([
+            // Your listeners may add pixels themselves, so this one has to stay late
             StopPropagationIfNoPixelsHasBeenAddedSubscriber::class,
             AddEventToTagBagSubscriber::class,
             DispatchOnCommandBusSubscriber::class,

tests/Unit/EventSubscriber/FilterBotsSubscriberTest.php

@@ -0,0 +1,82 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\MetaConversionsApiBundle\Tests\Unit\EventSubscriber;
+
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+use Setono\BotDetectionBundle\BotDetector\BotDetectorInterface;
+use Setono\MetaConversionsApi\Event\Event;
+use Setono\MetaConversionsApiBundle\Event\ConversionsApiEventRaised;
+use Setono\MetaConversionsApiBundle\EventSubscriber\FilterBotsSubscriber;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\HttpFoundation\Request;
+
+#[CoversClass(FilterBotsSubscriber::class)]
+final class FilterBotsSubscriberTest extends TestCase
+{
+    #[Test]
+    public function it_stops_a_bot_request(): void
+    {
+        $event = new ConversionsApiEventRaised(new Event(Event::EVENT_VIEW_CONTENT));
+
+        (new FilterBotsSubscriber(self::botDetector(true)))->filter($event);
+
+        self::assertTrue($event->isPropagationStopped());
+    }
+
+    #[Test]
+    public function it_does_not_stop_a_regular_request(): void
+    {
+        $event = new ConversionsApiEventRaised(new Event(Event::EVENT_VIEW_CONTENT));
+
+        (new FilterBotsSubscriber(self::botDetector(false)))->filter($event);
+
+        self::assertFalse($event->isPropagationStopped());
+    }
+
+    /**
+     * The point of filtering above PRIORITY_ENRICH: an application listener that loads the customer, the order and
+     * its addresses must not do any of that for traffic the bundle discards anyway
+     */
+    #[Test]
+    public function it_stops_before_application_listeners_enrich_the_event(): void
+    {
+        $enriched = false;
+
+        $dispatcher = new EventDispatcher();
+        $dispatcher->addSubscriber(new FilterBotsSubscriber(self::botDetector(true)));
+        $dispatcher->addListener(
+            ConversionsApiEventRaised::class,
+            static function () use (&$enriched): void {
+                $enriched = true;
+            },
+            ConversionsApiEventRaised::PRIORITY_ENRICH,
+        );
+
+        $dispatcher->dispatch(new ConversionsApiEventRaised(new Event(Event::EVENT_VIEW_CONTENT)), ConversionsApiEventRaised::class);
+
+        self::assertFalse($enriched);
+    }
+
+    private static function botDetector(bool $isBot): BotDetectorInterface
+    {
+        return new class($isBot) implements BotDetectorInterface {
+            public function __construct(private readonly bool $isBot)
+            {
+            }
+
+            public function isBot(string $userAgent): bool
+            {
+                return $this->isBot;
+            }
+
+            public function isBotRequest(?Request $request = null): bool
+            {
+                return $this->isBot;
+            }
+        };
+    }
+}