From 04febacd7717e41a477ea594784289f692ccf699 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 7 Sep 2026 13:31:28 +0200 Subject: [PATCH] Filter bots before application listeners enrich the event The filters ran after the application's listeners, so enrichment work was done for bot traffic and then thrown away. Move the filter band above PRIORITY_ENRICH, keeping the no-pixels check late because application listeners may add pixels themselves. Fixes #23 --- README.md | 13 +-- UPGRADE.md | 12 +++ src/Event/ConversionsApiEventRaised.php | 23 +++--- .../PopulatePixelsSubscriber.php | 2 +- .../Event/ConversionsApiEventRaisedTest.php | 22 ++++- .../FilterBotsSubscriberTest.php | 82 +++++++++++++++++++ 6 files changed, 135 insertions(+), 19 deletions(-) create mode 100644 tests/Unit/EventSubscriber/FilterBotsSubscriberTest.php diff --git a/README.md b/README.md index 8718f50..5f93b48 100644 --- a/README.md +++ b/README.md @@ -152,11 +152,11 @@ 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) | @@ -164,8 +164,9 @@ event first, then leaves a gap for your own listeners, then filters and sends: 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. diff --git a/UPGRADE.md b/UPGRADE.md index 1a2dc66..1d56060 100644 --- a/UPGRADE.md +++ b/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 diff --git a/src/Event/ConversionsApiEventRaised.php b/src/Event/ConversionsApiEventRaised.php index b5a8f0a..1d914aa 100644 --- a/src/Event/ConversionsApiEventRaised.php +++ b/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 diff --git a/src/EventSubscriber/PopulatePixelsSubscriber.php b/src/EventSubscriber/PopulatePixelsSubscriber.php index f55c1b0..84595cf 100644 --- a/src/EventSubscriber/PopulatePixelsSubscriber.php +++ b/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], ]; } diff --git a/tests/Unit/Event/ConversionsApiEventRaisedTest.php b/tests/Unit/Event/ConversionsApiEventRaisedTest.php index a56624d..049f303 100644 --- a/tests/Unit/Event/ConversionsApiEventRaisedTest.php +++ b/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, diff --git a/tests/Unit/EventSubscriber/FilterBotsSubscriberTest.php b/tests/Unit/EventSubscriber/FilterBotsSubscriberTest.php new file mode 100644 index 0000000..53e9c68 --- /dev/null +++ b/tests/Unit/EventSubscriber/FilterBotsSubscriberTest.php @@ -0,0 +1,82 @@ +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; + } + }; + } +}