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; + } + }; + } +}