diff --git a/src/Admin/Repository/OfferRepository.php b/src/Admin/Repository/OfferRepository.php index f1aa004..cabd797 100644 --- a/src/Admin/Repository/OfferRepository.php +++ b/src/Admin/Repository/OfferRepository.php @@ -16,6 +16,37 @@ public function findById(string $id): ?Offer return $row === null ? null : Offer::fromRow($row); } + /** + * Load active offers by ID, keyed by their ID for routing lookups. + * + * @param list $ids + * @return array + */ + public function findActiveByIds(array $ids): array + { + $ids = array_values(array_unique(array_filter($ids, static fn (string $id): bool => $id !== ''))); + if ($ids === []) return []; + + $placeholders = []; + $params = []; + foreach ($ids as $i => $id) { + $key = 'id' . $i; + $placeholders[] = ':' . $key; + $params[$key] = $id; + } + + $rows = $this->db->fetchAll( + 'SELECT * FROM core.offers WHERE is_active AND id IN (' . implode(',', $placeholders) . ')', + $params, + ); + $offers = []; + foreach ($rows as $row) { + $offer = Offer::fromRow($row); + $offers[$offer->id] = $offer; + } + return $offers; + } + public function findByToken(string $token): ?Offer { $row = $this->db->fetchOne('SELECT * FROM core.offers WHERE postback_token = :t', ['t' => $token]); diff --git a/src/Engine/ClickHandler.php b/src/Engine/ClickHandler.php index b6efef0..e386540 100644 --- a/src/Engine/ClickHandler.php +++ b/src/Engine/ClickHandler.php @@ -20,6 +20,14 @@ final class ClickHandler { + /** + * Offer ids reach `core.offers.id`, which is a `uuid` column: a malformed id + * is not a miss, it aborts the whole lookup. Anything that does not match is + * dropped from the candidate list instead, the way OfferPicker already drops + * an empty id. + */ + private const OFFER_ID = '/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i'; + public function __construct( private readonly CampaignRepository $campaigns, private readonly OfferRepository $offers, @@ -84,9 +92,18 @@ public function handle(ServerRequestInterface $request, ResponseInterface $respo if ($flow !== null && $flow->targetType === 'offers' && $flow->targetOffers !== []) { // Flow override wins; null = inherit the campaign default. $sticky = $flow->stickyOffer ?? $campaign->stickyOffer; - $offerId = $this->picker->pick($flow->targetOffers, $ctx, $sticky); + $targetIds = array_map( + self::candidateOfferId(...), + $flow->targetOffers, + ); + $activeOffers = $this->offers->findActiveByIds($targetIds); + $activeTargets = array_values(array_filter( + $flow->targetOffers, + static fn (array $target): bool => isset($activeOffers[self::candidateOfferId($target)]), + )); + $offerId = $this->picker->pick($activeTargets, $ctx, $sticky); if ($offerId !== null) { - $offer = $this->offers->findById($offerId); + $offer = $activeOffers[$offerId] ?? null; if ($offer !== null) { $outUrl = $this->macros->expand($offer->url, $ctx); } @@ -196,14 +213,22 @@ private function resolveTrashUrl(Campaign $c, Context $ctx): string return $this->macros->expand($raw, $ctx); } - /** Resolve an {offer:X} reference by UUID id, falling back to exact name. */ + /** @param array $target */ + private static function candidateOfferId(array $target): string + { + $offerId = $target['offer_id'] ?? null; + return is_string($offerId) && preg_match(self::OFFER_ID, $offerId) ? $offerId : ''; + } + + /** Resolve an active {offer:X} reference by UUID id, falling back to exact name. */ private function resolveOffer(string $ref): ?Offer { - if (preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i', $ref)) { + if (preg_match(self::OFFER_ID, $ref)) { $byId = $this->offers->findById($ref); - if ($byId !== null) return $byId; + if ($byId !== null) return $byId->isActive ? $byId : null; } - return $this->offers->findByName($ref); + $byName = $this->offers->findByName($ref); + return $byName !== null && $byName->isActive ? $byName : null; } private function trash(?Campaign $c, string $url, ResponseInterface $response): ResponseInterface diff --git a/tests/Integration/Engine/ClickHandlerTest.php b/tests/Integration/Engine/ClickHandlerTest.php index 86c882e..79a468b 100644 --- a/tests/Integration/Engine/ClickHandlerTest.php +++ b/tests/Integration/Engine/ClickHandlerTest.php @@ -34,6 +34,7 @@ $cRepo = new CampaignRepository($this->db, new CampaignIdGenerator()); $oRepo = new OfferRepository($this->db); $fRepo = new FlowRepository($this->db); + $this->offers = $oRepo; $compiler = new FilterCompiler(); $this->handler = new ClickHandler( $cRepo, $oRepo, @@ -53,7 +54,7 @@ $this->camp = $cRepo->create(['name' => 'Click test', 'slug' => 'clkt01', 'is_active' => '1']); $this->offer = $oRepo->create(['name' => 'O', 'url' => 'https://example.com/?c={country}&cid={click_id}', 'is_active' => '1']); - $fRepo->create($this->camp->id, [ + $this->flow = $fRepo->create($this->camp->id, [ 'name' => 'all → offer', 'filters' => [], 'target_type' => 'offers', @@ -86,6 +87,99 @@ expect($resp->getStatusCode())->toBe(404); }); +test('inactive offer does not receive new traffic', function (): void { + $this->db->execute( + 'UPDATE core.offers SET is_active = false WHERE id = :id', + ['id' => $this->offer->id], + ); + + $resp = $this->handler->handle( + (new ServerRequestFactory())->createServerRequest('GET', '/clkt01'), + new Response(), + 'clkt01', + ); + + expect($resp->getStatusCode())->toBe(204); + expect($resp->getHeaderLine('Location'))->toBe(''); +}); + +test('inactive offer is removed before active offer weights are evaluated', function (): void { + $activeOffer = $this->offers->create([ + 'name' => 'Active fallback', + 'url' => 'https://active.example/?cid={click_id}', + 'is_active' => '1', + ]); + $this->db->execute( + 'UPDATE core.offers SET is_active = false WHERE id = :id', + ['id' => $this->offer->id], + ); + $this->db->execute( + 'UPDATE core.flows SET target_offers = :targets::jsonb WHERE id = :id', + [ + 'id' => $this->flow->id, + 'targets' => json_encode([ + ['offer_id' => $this->offer->id, 'weight' => 100], + ['offer_id' => $activeOffer->id, 'weight' => 1], + ], JSON_THROW_ON_ERROR), + ], + ); + + $req = (new ServerRequestFactory()) + ->createServerRequest('GET', '/clkt01') + ->withCookieParams(['vu' => '019dc137-724a-756c-923a-a392001e3d79']); + $resp = $this->handler->handle($req, new Response(), 'clkt01'); + + expect($resp->getStatusCode())->toBe(302); + expect($resp->getHeaderLine('Location'))->toStartWith('https://active.example/'); +}); + +test('a malformed offer_id does not take the rest of the flow down with it', function (): void { + // core.offers.id is a uuid column, so a non-uuid candidate aborts the lookup + // for every visitor — not just the share of them the picker would have sent + // to that entry. The healthy offer next to it must still be served. + $this->db->execute( + 'UPDATE core.flows SET target_offers = :targets::jsonb WHERE id = :id', + [ + 'id' => $this->flow->id, + 'targets' => json_encode([ + ['offer_id' => 'not-a-uuid', 'weight' => 100], + ['offer_id' => $this->offer->id, 'weight' => 1], + ], JSON_THROW_ON_ERROR), + ], + ); + + $resp = $this->handler->handle( + (new ServerRequestFactory())->createServerRequest('GET', '/clkt01'), + new Response(), + 'clkt01', + ); + + expect($resp->getStatusCode())->toBe(302); + expect($resp->getHeaderLine('Location'))->toStartWith('https://example.com/'); +}); + +test('an offer_id that no longer exists is skipped in favour of a live one', function (): void { + $this->db->execute( + 'UPDATE core.flows SET target_offers = :targets::jsonb WHERE id = :id', + [ + 'id' => $this->flow->id, + 'targets' => json_encode([ + ['offer_id' => '019dc137-724a-756c-923a-a39200000000', 'weight' => 100], + ['offer_id' => $this->offer->id, 'weight' => 1], + ], JSON_THROW_ON_ERROR), + ], + ); + + $resp = $this->handler->handle( + (new ServerRequestFactory())->createServerRequest('GET', '/clkt01'), + new Response(), + 'clkt01', + ); + + expect($resp->getStatusCode())->toBe(302); + expect($resp->getHeaderLine('Location'))->toStartWith('https://example.com/'); +}); + test('trash {offer:name} redirects to the offer url, macro-expanded', function (): void { $cRepo = new CampaignRepository($this->db, new CampaignIdGenerator()); $c2 = $cRepo->create(['name' => 'Trash offer', 'slug' => 'trsh01', 'is_active' => '1']); @@ -107,6 +201,52 @@ expect($loc)->not->toContain('{offer:'); // reference resolved, not passed through literally }); +test('trash {offer:name} does not redirect to an inactive offer', function (): void { + $this->db->execute( + 'UPDATE core.offers SET is_active = false WHERE id = :id', + ['id' => $this->offer->id], + ); + $cRepo = new CampaignRepository($this->db, new CampaignIdGenerator()); + $campaign = $cRepo->create(['name' => 'Inactive trash offer', 'slug' => 'trsh04', 'is_active' => '1']); + $this->db->execute( + 'UPDATE core.campaigns SET trash_mode = 1, trash_url = :url WHERE id = :id', + ['url' => '{offer:O}', 'id' => $campaign->id], + ); + + $resp = $this->handler->handle( + (new ServerRequestFactory())->createServerRequest('GET', '/trsh04'), + new Response(), + 'trsh04', + ); + + expect($resp->getStatusCode())->toBe(204); + expect($resp->getHeaderLine('Location'))->toBe(''); +}); + +test('trash {offer:uuid} does not redirect to an inactive offer either', function (): void { + // The reference resolves by id before it falls back to the name, so the + // active-only check has to hold on both branches, not just the named one. + $this->db->execute( + 'UPDATE core.offers SET is_active = false WHERE id = :id', + ['id' => $this->offer->id], + ); + $cRepo = new CampaignRepository($this->db, new CampaignIdGenerator()); + $campaign = $cRepo->create(['name' => 'Inactive trash offer by id', 'slug' => 'trsh05', 'is_active' => '1']); + $this->db->execute( + 'UPDATE core.campaigns SET trash_mode = 1, trash_url = :url WHERE id = :id', + ['url' => '{offer:' . $this->offer->id . '}', 'id' => $campaign->id], + ); + + $resp = $this->handler->handle( + (new ServerRequestFactory())->createServerRequest('GET', '/trsh05'), + new Response(), + 'trsh05', + ); + + expect($resp->getStatusCode())->toBe(204); + expect($resp->getHeaderLine('Location'))->toBe(''); +}); + test('trash {offer:missing} falls back to 204 (no leaked macro)', function (): void { $cRepo = new CampaignRepository($this->db, new CampaignIdGenerator()); $c2 = $cRepo->create(['name' => 'Trash miss', 'slug' => 'trsh02', 'is_active' => '1']);