From 4df146bdfe81c150bb91bedfc7b1d9d9ad9cdf26 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Thu, 5 Oct 2023 15:50:19 +0200 Subject: [PATCH 01/17] Configure using one time banners, configure persistent redis connection --- Campaign/.env.example | 10 ++++++++ Campaign/app/Http/Showtime/Showtime.php | 24 ++++++++++--------- Campaign/app/Http/Showtime/ShowtimeConfig.php | 14 +++++++++++ Campaign/app/Providers/AppServiceProvider.php | 5 ++++ Campaign/config/banners.php | 3 ++- Campaign/config/database.php | 1 + Campaign/public/showtime.php | 2 ++ 7 files changed, 47 insertions(+), 12 deletions(-) diff --git a/Campaign/.env.example b/Campaign/.env.example index bd33fa0a2..f11804396 100644 --- a/Campaign/.env.example +++ b/Campaign/.env.example @@ -85,6 +85,9 @@ REDIS_HOST=redis # Redis connection port. 6379 is the default port used by Redis installation. REDIS_PORT=6379 +# Specifies if the underlying redis connection resource should be left open when a script ends its lifecycle. Default: false +REDIS_PERSISTENT=false + # Alternative Redis connection configuration. # The accepted format of a single connection is # @@ -249,3 +252,10 @@ MAXMIND_DATABASE=resources/assets/maxmind/GeoLite2-Country.mmdb # In the case of same amount of variants the banner from more recent campaign is prioritized. #PRIORITIZE_BANNERS_ON_SAME_POSITION= + +# Flag whether the one time banners should be enabled or not. +# +# If this option is set to `true`, the user and browser one time banners are used in Showtime. +# If you are not using the one time banners, set this option to `false` for better performance. +# The default value is `true`. +ONE_TIME_BANNER_ENABLED=true diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index 6e2fc9dd8..dca909cef 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -158,17 +158,19 @@ public function showtime(string $userData, string $callback, ShowtimeResponse $s $displayData = []; - // Try to load one-time banners (having precedence over campaigns) - $banner = null; - if ($userId) { - $banner = $this->loadOneTimeUserBanner($userId); - } - if (!$banner) { - $banner = $this->loadOneTimeBrowserBanner($browserId); - } - if ($banner) { - $displayData[] = $showtimeResponse->renderBanner($banner, $alignments, $dimensions, $positions, $snippets); - return $showtimeResponse->success($callback, $displayData, [], $segmentAggregator->getProviderData()); + if ($this->showtimeConfig->isOneTimeBannerEnabled()) { + // Try to load one-time banners (having precedence over campaigns) + $banner = null; + if ($userId) { + $banner = $this->loadOneTimeUserBanner($userId); + } + if (!$banner) { + $banner = $this->loadOneTimeBrowserBanner($browserId); + } + if ($banner) { + $displayData[] = $showtimeResponse->renderBanner($banner, $alignments, $dimensions, $positions, $snippets); + return $showtimeResponse->success($callback, $displayData, [], $segmentAggregator->getProviderData()); + } } $campaignIds = json_decode($this->redis->get(Campaign::ACTIVE_CAMPAIGN_IDS) ?? '[]') ?? []; diff --git a/Campaign/app/Http/Showtime/ShowtimeConfig.php b/Campaign/app/Http/Showtime/ShowtimeConfig.php index 586bfb2f2..46972fe14 100644 --- a/Campaign/app/Http/Showtime/ShowtimeConfig.php +++ b/Campaign/app/Http/Showtime/ShowtimeConfig.php @@ -8,6 +8,8 @@ class ShowtimeConfig private bool $prioritizeBannerOnSamePosition = false; + private bool $oneTimeBannerEnabled = true; + public function setAcceptLanguage(string $language): void { $this->acceptLanguage = $language; @@ -27,4 +29,16 @@ public function setPrioritizeBannerOnSamePosition(bool $prioritizeBannerOnSamePo { $this->prioritizeBannerOnSamePosition = $prioritizeBannerOnSamePosition; } + + public function isOneTimeBannerEnabled(): bool + { + return $this->oneTimeBannerEnabled; + } + + public function setOneTimeBannerEnabled(bool $oneTimeBannerEnabled): self + { + $this->oneTimeBannerEnabled = $oneTimeBannerEnabled; + + return $this; + } } diff --git a/Campaign/app/Providers/AppServiceProvider.php b/Campaign/app/Providers/AppServiceProvider.php index 2fac4fb16..d765634a8 100644 --- a/Campaign/app/Providers/AppServiceProvider.php +++ b/Campaign/app/Providers/AppServiceProvider.php @@ -5,6 +5,7 @@ use App\Contracts\SegmentAggregator; use App\Http\Resources\SearchResource; use App\Http\Showtime\LazyGeoReader; +use App\Http\Showtime\ShowtimeConfig; use Illuminate\Database\Connection; use Illuminate\Foundation\Application; use Illuminate\Pagination\Paginator; @@ -47,6 +48,10 @@ public function register() return new LazyGeoReader(config("services.maxmind.database")); }); + $this->app->bind(ShowtimeConfig::class, function () { + return (new ShowtimeConfig())->setOneTimeBannerEnabled(config("banners.one_time_banner_enabled")); + }); + $this->app->bind(SegmentAggregator::class, function (Application $app) { return new SegmentAggregator($app->tagged(SegmentAggregator::TAG)); }); diff --git a/Campaign/config/banners.php b/Campaign/config/banners.php index 79b74d523..b38f16194 100644 --- a/Campaign/config/banners.php +++ b/Campaign/config/banners.php @@ -91,5 +91,6 @@ ], ], ], - 'prioritize_banners_on_same_position' => env('PRIORITIZE_BANNERS_ON_SAME_POSITION', false) + 'prioritize_banners_on_same_position' => env('PRIORITIZE_BANNERS_ON_SAME_POSITION', false), + 'one_time_banner_enabled' => env('ONE_TIME_BANNER_ENABLED', true) ]; diff --git a/Campaign/config/database.php b/Campaign/config/database.php index c56ba102a..4decd610c 100644 --- a/Campaign/config/database.php +++ b/Campaign/config/database.php @@ -38,6 +38,7 @@ function configure_redis($database) 'password' => env('REDIS_PASSWORD'), 'port' => env('REDIS_PORT', '6379'), 'database' => $database, + 'persistent' => env('REDIS_PERSISTENT', false), ]; } } diff --git a/Campaign/public/showtime.php b/Campaign/public/showtime.php index 95e4d166d..46738bc37 100644 --- a/Campaign/public/showtime.php +++ b/Campaign/public/showtime.php @@ -432,6 +432,7 @@ private function renderInternal( 'port' => env('REDIS_PORT') ?: 6379, 'password' => env('REDIS_PASSWORD') ?: null, 'database' => env('REDIS_DEFAULT_DATABASE') ?: 0, + 'persistent' => env('REDIS_PERSISTENT', false), ], [ 'prefix' => env('REDIS_PREFIX') ?: '', ]); @@ -457,6 +458,7 @@ private function renderInternal( ['options' => ['default' => false]] ); $showtimeConfig->setPrioritizeBannerOnSamePosition($prioritizeBannerOnSamePosition); + $showtimeConfig->setOneTimeBannerEnabled(env('ONE_TIME_BANNER_ENABLED', true)); $showtime = new Showtime($redis, $segmentAggregator, $geoReader, $showtimeConfig, $deviceDetector, $logger); $showtime->showtime($data, $callback, $showtimeResponse); From 855f27f6154180750318efde30fdd0d22d1fa347 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Fri, 6 Oct 2023 14:46:23 +0200 Subject: [PATCH 02/17] Fetch all running campaigns at once with `mget` --- Campaign/app/Http/Showtime/Showtime.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index dca909cef..40bf9a2bf 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -178,15 +178,26 @@ public function showtime(string $userData, string $callback, ShowtimeResponse $s return $showtimeResponse->success($callback, [], [], $segmentAggregator->getProviderData()); } + // prepare campaign IDs to fetch + $dbCampaignIds = []; + foreach ($campaignIds as $campaignId) { + $dbCampaignIds[] = Campaign::CAMPAIGN_TAG . ":{$campaignId}"; + } + + // fetch all running campaigns at once + $fetchedCampaigns = $this->redis->mget($dbCampaignIds); + $activeCampaigns = []; $campaigns = []; $campaignBanners = []; $suppressedBanners = []; - foreach ($campaignIds as $campaignId) { + reset($campaignIds); + foreach ($fetchedCampaigns as $fetchedCampaign) { /** @var Campaign $campaign */ - $campaign = unserialize($this->redis->get(Campaign::CAMPAIGN_TAG . ":{$campaignId}"), ['allowed_class' => Campaign::class]); - $campaigns[$campaignId] = $campaign; + $campaign = unserialize($fetchedCampaign, ['allowed_class' => Campaign::class]); + $campaigns[current($campaignIds)] = $campaign; $campaignBanners[] = $this->shouldDisplay($campaign, $data, $activeCampaigns); + next($campaignIds); } $campaignBanners = array_filter($campaignBanners); From 3115ed97acacdcc3ec19a8ba3d54b0f011121b98 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Fri, 6 Oct 2023 16:10:33 +0200 Subject: [PATCH 03/17] Optimize loading maps with `mget` --- Campaign/app/Http/Showtime/Showtime.php | 89 +++++++++++++------------ 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index 40bf9a2bf..436418eef 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -61,14 +61,6 @@ public function setPositionMap(\App\Models\Position\Map $positions) $this->positionMap = $positions->positions(); } - private function getPositionMap() - { - if (!$this->positionMap) { - $this->positionMap = json_decode($this->redis->get(\App\Models\Position\Map::POSITIONS_MAP_REDIS_KEY), true) ?? []; - } - return $this->positionMap; - } - public function getShowtimeConfig(): ShowtimeConfig { return $this->showtimeConfig; @@ -79,41 +71,11 @@ public function setDimensionMap(\App\Models\Dimension\Map $dimensions) $this->dimensionMap = $dimensions->dimensions(); } - private function getDimensionMap() - { - if (!$this->dimensionMap) { - $this->dimensionMap = json_decode($this->redis->get(\App\Models\Dimension\Map::DIMENSIONS_MAP_REDIS_KEY), true) ?? []; - } - return $this->dimensionMap; - } - public function setAlignmentsMap(\App\Models\Alignment\Map $alignments) { $this->alignmentsMap = $alignments->alignments(); } - private function getAlignmentsMap() - { - if (!$this->alignmentsMap) { - $this->alignmentsMap = json_decode($this->redis->get(\App\Models\Alignment\Map::ALIGNMENTS_MAP_REDIS_KEY), true) ?? []; - } - return $this->alignmentsMap; - } - - private function getSnippets() - { - if (!$this->snippets) { - $this->snippets = []; - - $redisCacheKey = $this->redis->get(\App\Snippet::REDIS_CACHE_KEY); - if (!is_null($redisCacheKey)) { - $this->snippets = json_decode($redisCacheKey, true) ?? []; - } - } - - return $this->snippets; - } - public function showtime(string $userData, string $callback, ShowtimeResponse $showtimeResponse) { try { @@ -151,10 +113,12 @@ public function showtime(string $userData, string $callback, ShowtimeResponse $s $segmentAggregator->setProviderData($data->cache); } - $positions = $this->getPositionMap(); - $dimensions = $this->getDimensionMap(); - $alignments = $this->getAlignmentsMap(); - $snippets = $this->getSnippets(); + $this->loadMaps(); + + $positions = $this->positionMap; + $dimensions = $this->dimensionMap; + $alignments = $this->alignmentsMap; + $snippets = $this->snippets; $displayData = []; @@ -660,6 +624,47 @@ public function displayForBrowser(Banner $banner, string $browserId, int $expire $this->redis->expire($key, $expiresInSeconds); } + private function loadMaps(): void + { + $keys = []; + if (!$this->positionMap) { + $keys[] = \App\Models\Position\Map::POSITIONS_MAP_REDIS_KEY; + } + + if (!$this->dimensionMap) { + $keys[] = \App\Models\Dimension\Map::DIMENSIONS_MAP_REDIS_KEY; + } + + if (!$this->alignmentsMap) { + $keys[] = \App\Models\Alignment\Map::ALIGNMENTS_MAP_REDIS_KEY; + } + + if (!$this->snippets) { + $keys[] = \App\Snippet::REDIS_CACHE_KEY; + } + + $maps = $this->redis->mget($keys); + reset($keys); + foreach ($maps as $map) { + $key = current($keys); + + switch ($key) { + case \App\Models\Position\Map::POSITIONS_MAP_REDIS_KEY: + $this->positionMap = json_decode($map, true) ?? []; + break; + case \App\Models\Dimension\Map::DIMENSIONS_MAP_REDIS_KEY: + $this->dimensionMap = json_decode($map, true) ?? []; + break; + case \App\Models\Alignment\Map::ALIGNMENTS_MAP_REDIS_KEY: + $this->alignmentsMap = json_decode($map, true) ?? []; + break; + case \App\Snippet::REDIS_CACHE_KEY: + $this->snippets = json_decode($map, true) ?? []; + break; + } + next($keys); + } + } private function loadOneTimeUserBanner($userId): ?Banner { From 8e0ea9482c7626bb729cf3079d705fff1201c205 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Fri, 6 Oct 2023 19:46:33 +0200 Subject: [PATCH 04/17] Parse the user agent just once --- Campaign/app/Http/Showtime/Showtime.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index 436418eef..b8b83b6ab 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -429,12 +429,17 @@ public function shouldDisplay(Campaign $campaign, $userData, array &$activeCampa // device rules if (!isset($userData->userAgent)) { $this->logger->error("Unable to load user agent for userId [{$userId}]"); - } else { - if (!in_array(Campaign::DEVICE_MOBILE, $campaign->devices) && $this->deviceDetector->get($userData->userAgent)->isMobile()) { + } else if (in_array(Campaign::DEVICE_MOBILE, $campaign->devices, true) + || in_array(Campaign::DEVICE_DESKTOP, $campaign->devices, true) + ) { + // parse user agent + $deviceDetector = $this->deviceDetector->get($userData->userAgent); + + if (!in_array(Campaign::DEVICE_MOBILE, $campaign->devices, true) && $deviceDetector->isMobile()) { return null; } - if (!in_array(Campaign::DEVICE_DESKTOP, $campaign->devices) && $this->deviceDetector->get($userData->userAgent)->isDesktop()) { + if (!in_array(Campaign::DEVICE_DESKTOP, $campaign->devices, true) && $deviceDetector->isDesktop()) { return null; } } From 596999a5604a08fd393e418c387b77d2ea591e32 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Fri, 6 Oct 2023 20:12:29 +0200 Subject: [PATCH 05/17] Check segment cache only once --- Campaign/app/Http/Showtime/Showtime.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index b8b83b6ab..88091402c 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -33,6 +33,8 @@ class Showtime private $snippets; + private array $segmentCheckCache = []; + public function __construct( private ClientInterface $redis, private SegmentAggregator $segmentAggregator, @@ -155,6 +157,7 @@ public function showtime(string $userData, string $callback, ShowtimeResponse $s $campaigns = []; $campaignBanners = []; $suppressedBanners = []; + $this->segmentCheckCache = []; reset($campaignIds); foreach ($fetchedCampaigns as $fetchedCampaign) { /** @var Campaign $campaign */ @@ -731,8 +734,15 @@ private function isCacheValid(CampaignSegment $campaignSegment): bool return true; } - $cacheKeyTimeStamp = $this->redis->get(SegmentAggregator::cacheKey($campaignSegment) . '|timestamp'); + $cacheKey = SegmentAggregator::cacheKey($campaignSegment); + if (isset($this->segmentCheckCache[$cacheKey])) { + return true; + } + + + $cacheKeyTimeStamp = $this->redis->get($cacheKey . '|timestamp'); if ($cacheKeyTimeStamp) { + $this->segmentCheckCache[$cacheKey] = true; return true; } From 0e8f43e99f995be9fa25070abb3cd960a6f58485 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Fri, 6 Oct 2023 20:37:48 +0200 Subject: [PATCH 06/17] Check user or browser in same segment just once --- Campaign/app/Http/Showtime/Showtime.php | 35 +++++++++++++++++++++---- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index 88091402c..73a746b64 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -710,9 +710,9 @@ private function evaluateSegmentRules(Campaign $campaign, $browserId, $userId = return false; } - $belongsToSegment = $this->segmentAggregator->checkUser($campaignSegment, (string)$userId); + $belongsToSegment = $this->checkUserInSegment($campaignSegment, $userId); } else { - $belongsToSegment = $this->segmentAggregator->checkBrowser($campaignSegment, (string)$browserId); + $belongsToSegment = $this->checkBrowserInSegment($campaignSegment, $browserId); } // user is member of segment, that's excluded from campaign; halt execution @@ -734,13 +734,12 @@ private function isCacheValid(CampaignSegment $campaignSegment): bool return true; } - $cacheKey = SegmentAggregator::cacheKey($campaignSegment); + $cacheKey = SegmentAggregator::cacheKey($campaignSegment). '|timestamp'; if (isset($this->segmentCheckCache[$cacheKey])) { return true; } - - $cacheKeyTimeStamp = $this->redis->get($cacheKey . '|timestamp'); + $cacheKeyTimeStamp = $this->redis->get($cacheKey ); if ($cacheKeyTimeStamp) { $this->segmentCheckCache[$cacheKey] = true; return true; @@ -767,4 +766,30 @@ private function loadOneTimeBanner(array $bannerKeys): ?Banner } return null; } + + private function checkUserInSegment(CampaignSegment $campaignSegment, $userId): bool + { + $cacheKey = SegmentAggregator::cacheKey($campaignSegment). '|user|'.$userId; + if (isset($this->segmentCheckCache[$cacheKey])) { + return $this->segmentCheckCache[$cacheKey]; + } + + $belongsToSegment = $this->segmentAggregator->checkUser($campaignSegment, (string) $userId); + $this->segmentCheckCache[$cacheKey] = $belongsToSegment; + + return $belongsToSegment; + } + + private function checkBrowserInSegment(CampaignSegment $campaignSegment, $browserId): bool + { + $cacheKey = SegmentAggregator::cacheKey($campaignSegment). '|browser|'.$browserId; + if (isset($this->segmentCheckCache[$cacheKey])) { + return $this->segmentCheckCache[$cacheKey]; + } + + $belongsToSegment = $this->segmentAggregator->checkBrowser($campaignSegment, (string)$browserId); + $this->segmentCheckCache[$cacheKey] = $belongsToSegment; + + return $belongsToSegment; + } } From 88443888ea36d4abef807d73642f28ee92be2a57 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Sat, 7 Oct 2023 18:59:01 +0200 Subject: [PATCH 07/17] Store result of device detection in redis --- Campaign/app/Http/Showtime/Showtime.php | 74 ++++++++++++++++++++----- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index 73a746b64..a2f2327af 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -19,6 +19,10 @@ class Showtime private const BANNER_ONETIME_BROWSER_KEY = 'banner_onetime_browser'; + private const DEVICE_DETECTION_KEY = 'device_detection'; + + private const DEVICE_DETECTION_TTL = 86400; + private const PAGEVIEW_ATTRIBUTE_OPERATOR_IS = '='; private const PAGEVIEW_ATTRIBUTE_OPERATOR_IS_NOT = '!='; @@ -430,21 +434,8 @@ public function shouldDisplay(Campaign $campaign, $userData, array &$activeCampa } // device rules - if (!isset($userData->userAgent)) { - $this->logger->error("Unable to load user agent for userId [{$userId}]"); - } else if (in_array(Campaign::DEVICE_MOBILE, $campaign->devices, true) - || in_array(Campaign::DEVICE_DESKTOP, $campaign->devices, true) - ) { - // parse user agent - $deviceDetector = $this->deviceDetector->get($userData->userAgent); - - if (!in_array(Campaign::DEVICE_MOBILE, $campaign->devices, true) && $deviceDetector->isMobile()) { - return null; - } - - if (!in_array(Campaign::DEVICE_DESKTOP, $campaign->devices, true) && $deviceDetector->isDesktop()) { - return null; - } + if (false === $this->isAcceptedByDeviceRules($userData, $campaign, $userId)) { + return null; } // country rules @@ -792,4 +783,57 @@ private function checkBrowserInSegment(CampaignSegment $campaignSegment, $browse return $belongsToSegment; } + + private function isAcceptedByDeviceRules($userData, Campaign $campaign, $userId): bool + { + if (!isset($userData->userAgent)) { + $this->logger->error("Unable to load user agent for userId [{$userId}]"); + + return true; + } + + if (!in_array(Campaign::DEVICE_MOBILE, $campaign->devices, true) + && !in_array(Campaign::DEVICE_DESKTOP, $campaign->devices, true) + ) { + return true; + } + + // check result of device detection in redis + $deviceKey = self::DEVICE_DETECTION_KEY.':'.md5($userData->userAgent); + $deviceDetection = $this->redis->get($deviceKey); + if ($deviceDetection) { + if ($deviceDetection === Campaign::DEVICE_MOBILE && !in_array(Campaign::DEVICE_MOBILE, $campaign->devices,true)) { + return false; + } + if ($deviceDetection === Campaign::DEVICE_DESKTOP && !in_array(Campaign::DEVICE_DESKTOP, $campaign->devices,true)) { + return false; + } + } + + // parse user agent + $deviceDetector = $this->deviceDetector->get($userData->userAgent); + $isMobile = $deviceDetector->isMobile(); + $isDesktop = $deviceDetector->isDesktop(); + + if ($isMobile) { + $deviceDetectionValue = Campaign::DEVICE_MOBILE; + } else if ($isDesktop) { + $deviceDetectionValue = Campaign::DEVICE_DESKTOP; + } else { + $deviceDetectionValue = 'other'; + } + + // store result to redis to faster resolution on the next attempt + $this->redis->setex($deviceKey, self::DEVICE_DETECTION_TTL, $deviceDetectionValue); + + if ($isMobile && !in_array(Campaign::DEVICE_MOBILE, $campaign->devices, true)) { + return false; + } + + if ($isDesktop && !in_array(Campaign::DEVICE_DESKTOP, $campaign->devices, true)) { + return false; + } + + return true; + } } From cbe89f4563afae5f5adae0c8140f6f9880e65564 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Mon, 9 Oct 2023 09:47:48 +0200 Subject: [PATCH 08/17] Fix error `Too few arguments to function PlainPhpShowtimeResponse::success` --- Campaign/app/Http/Showtime/ControllerShowtimeResponse.php | 2 +- Campaign/app/Http/Showtime/ShowtimeResponse.php | 2 +- Campaign/public/showtime.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php b/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php index cfc424d73..556593c79 100644 --- a/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php +++ b/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php @@ -18,7 +18,7 @@ public function error($callback, int $statusCode, array $errors) ]); } - public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners) + public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners = []) { return response() ->jsonp($callback, [ diff --git a/Campaign/app/Http/Showtime/ShowtimeResponse.php b/Campaign/app/Http/Showtime/ShowtimeResponse.php index 91956af15..2e7b1f9b9 100644 --- a/Campaign/app/Http/Showtime/ShowtimeResponse.php +++ b/Campaign/app/Http/Showtime/ShowtimeResponse.php @@ -10,7 +10,7 @@ interface ShowtimeResponse { public function error($callback, int $statusCode, array $errors); - public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners); + public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners = []); public function renderBanner(Banner $banner, array $alignments, array $dimensions, array $positions, array $snippets): string; diff --git a/Campaign/public/showtime.php b/Campaign/public/showtime.php index 46738bc37..09b88c669 100644 --- a/Campaign/public/showtime.php +++ b/Campaign/public/showtime.php @@ -140,7 +140,7 @@ public function error($callback, int $statusCode, array $errors) ], $statusCode); } - public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners) + public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners = []) { $this->jsonpResponse($callback, [ 'success' => true, From 986f2ac7547c3f037c9de3d3f44b4d6ef0192d4e Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Mon, 9 Oct 2023 16:00:26 +0200 Subject: [PATCH 09/17] Add phpredis support --- Campaign/.env.example | 1 + Campaign/app/Contracts/Crm/Segment.php | 2 +- Campaign/app/Contracts/SegmentAggregator.php | 14 +++- .../app/Http/Showtime/LazyDeviceDetector.php | 12 ++- Campaign/app/Http/Showtime/Showtime.php | 2 +- Campaign/app/Providers/AppServiceProvider.php | 11 +++ .../DeviceDetectorServiceProvider.php | 8 +- .../app/Providers/ShowtimeServiceProvider.php | 42 ++++++++++ Campaign/composer.json | 1 + Campaign/composer.lock | 78 ++++++++++++++++++- Campaign/config/app.php | 3 +- Campaign/config/database.php | 1 + Campaign/public/showtime.php | 42 +++++++--- 13 files changed, 192 insertions(+), 25 deletions(-) create mode 100644 Campaign/app/Providers/ShowtimeServiceProvider.php diff --git a/Campaign/.env.example b/Campaign/.env.example index f11804396..81dfb079f 100644 --- a/Campaign/.env.example +++ b/Campaign/.env.example @@ -72,6 +72,7 @@ DB_PORT=3306 # Redis client to be used. Available values are: # - predis: default value, used if nothing is set +# - phpredis: use if you have installed the php-redis extension for better performance # - mock: fake implementation for tests # It's recommended to set this variable only for testing scenarios. # REDIS_CLIENT= diff --git a/Campaign/app/Contracts/Crm/Segment.php b/Campaign/app/Contracts/Crm/Segment.php index 7f6080574..328665500 100644 --- a/Campaign/app/Contracts/Crm/Segment.php +++ b/Campaign/app/Contracts/Crm/Segment.php @@ -27,7 +27,7 @@ class Segment implements SegmentContract private $redis; - public function __construct(Client $client, \Predis\Client $redis) + public function __construct(Client $client, \Predis\Client|\Redis $redis) { $this->client = $client; $this->providerData = new \stdClass; diff --git a/Campaign/app/Contracts/SegmentAggregator.php b/Campaign/app/Contracts/SegmentAggregator.php index f99a2dc56..fd93c5e7d 100644 --- a/Campaign/app/Contracts/SegmentAggregator.php +++ b/Campaign/app/Contracts/SegmentAggregator.php @@ -48,12 +48,20 @@ public function list(): Collection public function checkUser(CampaignSegment $campaignSegment, string $userId): bool { + if (!isset($this->contracts[$campaignSegment->provider])) { + return false; + } + return $this->contracts[$campaignSegment->provider] ->checkUser($campaignSegment, $userId); } public function checkBrowser(CampaignSegment $campaignSegment, string $browserId): bool { + if (!isset($this->contracts[$campaignSegment->provider])) { + return false; + } + return $this->contracts[$campaignSegment->provider] ->checkBrowser($campaignSegment, $browserId); } @@ -66,6 +74,10 @@ public function users(CampaignSegment $campaignSegment): Collection public function cacheEnabled(CampaignSegment $campaignSegment): bool { + if (!isset($this->contracts[$campaignSegment->provider])) { + return false; + } + return $this->contracts[$campaignSegment->provider] ->cacheEnabled($campaignSegment); } @@ -158,7 +170,7 @@ public function serializeToRedis() Redis::set(\App\Models\Alignment\Map::ALIGNMENTS_MAP_REDIS_KEY, $alignmentsMap->alignments()->toJson()); } - public static function unserializeFromRedis(Client $redisClient): ?SegmentAggregator + public static function unserializeFromRedis(Client|\Redis $redisClient): ?SegmentAggregator { $serializedClosure = $redisClient->get(self::SEGMENT_AGGREGATOR_REDIS_KEY); return $serializedClosure ? unserialize($serializedClosure)() : null; diff --git a/Campaign/app/Http/Showtime/LazyDeviceDetector.php b/Campaign/app/Http/Showtime/LazyDeviceDetector.php index 021d76bfe..033889a10 100644 --- a/Campaign/app/Http/Showtime/LazyDeviceDetector.php +++ b/Campaign/app/Http/Showtime/LazyDeviceDetector.php @@ -3,17 +3,15 @@ namespace App\Http\Showtime; use DeviceDetector\Cache\PSR6Bridge; -use Predis\ClientInterface; +use Psr\Cache\CacheItemPoolInterface; class LazyDeviceDetector { private $detector; - private $redis; - - public function __construct(ClientInterface $redis) - { - $this->redis = $redis; + public function __construct( + private readonly CacheItemPoolInterface $cachePool + ) { } public function get($userAgent) @@ -22,7 +20,7 @@ public function get($userAgent) $this->detector = new \DeviceDetector\DeviceDetector(); $this->detector->setCache( new PSR6Bridge( - new \Cache\Adapter\Predis\PredisCachePool($this->redis) + $this->cachePool ) ); } diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index a2f2327af..c1032581b 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -40,7 +40,7 @@ class Showtime private array $segmentCheckCache = []; public function __construct( - private ClientInterface $redis, + private ClientInterface|\Redis $redis, private SegmentAggregator $segmentAggregator, private LazyGeoReader $geoReader, private ShowtimeConfig $showtimeConfig, diff --git a/Campaign/app/Providers/AppServiceProvider.php b/Campaign/app/Providers/AppServiceProvider.php index d765634a8..2c4e6067c 100644 --- a/Campaign/app/Providers/AppServiceProvider.php +++ b/Campaign/app/Providers/AppServiceProvider.php @@ -4,6 +4,7 @@ use App\Contracts\SegmentAggregator; use App\Http\Resources\SearchResource; +use App\Http\Showtime\LazyDeviceDetector; use App\Http\Showtime\LazyGeoReader; use App\Http\Showtime\ShowtimeConfig; use Illuminate\Database\Connection; @@ -55,6 +56,16 @@ public function register() $this->app->bind(SegmentAggregator::class, function (Application $app) { return new SegmentAggregator($app->tagged(SegmentAggregator::TAG)); }); + + $this->app->bind(LazyDeviceDetector::class, function () { + if (env('REDIS_CLIENT', 'phpredis') === 'phpredis') { + $cachePool = new \Cache\Adapter\Redis\RedisCachePool(Redis::connection()->client()); + } else { + $cachePool = new \Cache\Adapter\Predis\PredisCachePool(Redis::connection()->client()); + } + + return (new LazyDeviceDetector($cachePool)); + }); } /** diff --git a/Campaign/app/Providers/DeviceDetectorServiceProvider.php b/Campaign/app/Providers/DeviceDetectorServiceProvider.php index 751087e99..02cd2a740 100644 --- a/Campaign/app/Providers/DeviceDetectorServiceProvider.php +++ b/Campaign/app/Providers/DeviceDetectorServiceProvider.php @@ -3,7 +3,6 @@ namespace App\Providers; use Illuminate\Contracts\Support\DeferrableProvider; -use Madewithlove\IlluminatePsrCacheBridge\Laravel\CacheItemPool; use Illuminate\Support\ServiceProvider; use DeviceDetector\Cache\PSR6Bridge; use DeviceDetector\DeviceDetector; @@ -14,10 +13,15 @@ class DeviceDetectorServiceProvider extends ServiceProvider implements Deferrabl public function register() { $this->app->bind(DeviceDetector::class, function ($app) { + if (env('REDIS_CLIENT', 'phpredis') === 'phpredis') { + $cachePool = new \Cache\Adapter\Redis\RedisCachePool(Redis::connection()->client()); + } else { + $cachePool = new \Cache\Adapter\Predis\PredisCachePool(Redis::connection()->client()); + } $dd = new DeviceDetector(); $dd->setCache( new PSR6Bridge( - new \Cache\Adapter\Predis\PredisCachePool(Redis::connection()) + $cachePool ) ); diff --git a/Campaign/app/Providers/ShowtimeServiceProvider.php b/Campaign/app/Providers/ShowtimeServiceProvider.php new file mode 100644 index 000000000..3c94da17c --- /dev/null +++ b/Campaign/app/Providers/ShowtimeServiceProvider.php @@ -0,0 +1,42 @@ +app->bind(Showtime::class, function ($app) { + $dd = new Showtime( + Redis::connection()->client(), + $this->app->get(SegmentAggregator::class), + $this->app->get(LazyGeoReader::class), + $this->app->get(ShowtimeConfig::class), + $this->app->get(LazyDeviceDetector::class), + $this->app->get(LoggerInterface::class), + ); + + return $dd; + }); + } + + public function provides() + { + return [Showtime::class]; + } +} diff --git a/Campaign/composer.json b/Campaign/composer.json index 9d1e0117c..777bfdcfd 100644 --- a/Campaign/composer.json +++ b/Campaign/composer.json @@ -23,6 +23,7 @@ "ext-intl": "*", "ext-json": "*", "cache/predis-adapter": "^1.0", + "cache/redis-adapter": "^1.2", "doctrine/dbal": "^3.1", "fico7489/laravel-pivot": "^3.0", "fideloper/proxy": "^4.4", diff --git a/Campaign/composer.lock b/Campaign/composer.lock index 9acff7f7f..3612baecd 100644 --- a/Campaign/composer.lock +++ b/Campaign/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "fecb74637ce336a90ee8c0a188624aaa", + "content-hash": "359d0e48f88453026c57be691a4dd448", "packages": [ { "name": "airbrake/phpbrake", @@ -444,6 +444,82 @@ }, "time": "2022-01-15T15:47:19+00:00" }, + { + "name": "cache/redis-adapter", + "version": "1.2.0", + "source": { + "type": "git", + "url": "https://github.com/php-cache/redis-adapter.git", + "reference": "558fd52bd813930c8e6dc83ffd8044ad1abec242" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-cache/redis-adapter/zipball/558fd52bd813930c8e6dc83ffd8044ad1abec242", + "reference": "558fd52bd813930c8e6dc83ffd8044ad1abec242", + "shasum": "" + }, + "require": { + "cache/adapter-common": "^1.0", + "cache/hierarchical-cache": "^1.0", + "php": ">=7.4", + "psr/cache": "^1.0 || ^2.0", + "psr/simple-cache": "^1.0" + }, + "provide": { + "psr/cache-implementation": "^1.0", + "psr/simple-cache-implementation": "^1.0" + }, + "require-dev": { + "cache/integration-tests": "^0.17", + "phpunit/phpunit": "^7.5.20 || ^9.5.10" + }, + "suggest": { + "ext-redis": "The extension required to use this pool." + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.1-dev" + } + }, + "autoload": { + "psr-4": { + "Cache\\Adapter\\Redis\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Aaron Scherer", + "email": "aequasi@gmail.com", + "homepage": "https://github.com/aequasi" + }, + { + "name": "Tobias Nyholm", + "email": "tobias.nyholm@gmail.com", + "homepage": "https://github.com/nyholm" + } + ], + "description": "A PSR-6 cache implementation using Redis (PhpRedis). This implementation supports tags", + "homepage": "http://www.php-cache.com/en/latest/", + "keywords": [ + "cache", + "phpredis", + "psr-6", + "redis", + "tag" + ], + "support": { + "source": "https://github.com/php-cache/redis-adapter/tree/1.2.0" + }, + "time": "2022-01-15T15:47:19+00:00" + }, { "name": "cache/tag-interop", "version": "1.1.0", diff --git a/Campaign/config/app.php b/Campaign/config/app.php index c5651d4d8..834dd8968 100644 --- a/Campaign/config/app.php +++ b/Campaign/config/app.php @@ -159,6 +159,7 @@ App\Providers\BeamSegmentsServiceProvider::class, App\Providers\PythiaSegmentsServiceProvider::class, App\Providers\StatsServiceProvider::class, + App\Providers\ShowtimeServiceProvider::class, ], /* @@ -227,6 +228,6 @@ | Url of the site, where remplib.js is implemented | */ - + 'client_site_url' => env('CLIENT_SITE_URL'), ]; diff --git a/Campaign/config/database.php b/Campaign/config/database.php index 4decd610c..b2438278f 100644 --- a/Campaign/config/database.php +++ b/Campaign/config/database.php @@ -164,6 +164,7 @@ function configure_redis($database) 'client' => env('REDIS_CLIENT', 'predis'), 'options' => [ + 'serializer' => \Redis::SERIALIZER_NONE, 'cluster' => env('REDIS_CLUSTER', 'predis'), 'prefix' => env('REDIS_PREFIX', ''), ], diff --git a/Campaign/public/showtime.php b/Campaign/public/showtime.php index 09b88c669..f506b2173 100644 --- a/Campaign/public/showtime.php +++ b/Campaign/public/showtime.php @@ -426,16 +426,36 @@ private function renderInternal( try { // dependencies initialization - $redis = new \Predis\Client([ - 'scheme' => 'tcp', - 'host' => env('REDIS_HOST'), - 'port' => env('REDIS_PORT') ?: 6379, - 'password' => env('REDIS_PASSWORD') ?: null, - 'database' => env('REDIS_DEFAULT_DATABASE') ?: 0, - 'persistent' => env('REDIS_PERSISTENT', false), - ], [ - 'prefix' => env('REDIS_PREFIX') ?: '', - ]); + if (env('REDIS_CLIENT', 'predis') === 'phpredis') { + $redis = new \Redis(); + if (env('REDIS_PERSISTENT', false)) { + $redis->pconnect(env('REDIS_HOST'), (int) env('REDIS_PORT', 6379), 5, 'showtime-'.env('REDIS_DEFAULT_DATABASE')); + } else { + $redis->connect(env('REDIS_HOST')); + } + if (env('REDIS_PASSWORD')) { + $redis->auth(env('REDIS_PASSWORD')); + } + $redis->setOption(\Redis::OPT_PREFIX, env('REDIS_PREFIX', '')); + //$redis->setOption(\Redis::OPT_SERIALIZER, \Redis::SERIALIZER_PHP); + $redis->setOption(\Redis::OPT_SERIALIZER, \Redis::SERIALIZER_NONE); + $redis->select((int) env('REDIS_DEFAULT_DATABASE', 0)); + + $cachePool = new \Cache\Adapter\Redis\RedisCachePool($redis); + + } else { + $redis = new \Predis\Client([ + 'scheme' => 'tcp', + 'host' => env('REDIS_HOST'), + 'port' => env('REDIS_PORT') ?: 6379, + 'password' => env('REDIS_PASSWORD') ?: null, + 'database' => env('REDIS_DEFAULT_DATABASE') ?: 0, + 'persistent' => env('REDIS_PERSISTENT', false), + ], [ + 'prefix' => env('REDIS_PREFIX') ?: '', + ]); + $cachePool = new \Cache\Adapter\Predis\PredisCachePool($redis); + } $segmentAggregator = SegmentAggregator::unserializeFromRedis($redis); if (!$segmentAggregator) { @@ -443,7 +463,7 @@ private function renderInternal( $showtimeResponse->error($callback, 500, ['Internal error, application might not have been correctly initialized.']); } - $deviceDetector = new LazyDeviceDetector($redis); + $deviceDetector = new LazyDeviceDetector($cachePool); if (file_exists(env('MAXMIND_DATABASE'))) { $maxmindDbPath = env('MAXMIND_DATABASE'); From 3772b1e967c87e2ab8160b831b793d0a43113a86 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Mon, 9 Oct 2023 22:12:27 +0200 Subject: [PATCH 10/17] Fix phpredis compatibility --- Campaign/app/Contracts/Crm/Segment.php | 5 ++++- Campaign/app/Jobs/CacheSegmentJob.php | 15 +++++++++++---- Campaign/public/showtime.php | 7 ++++--- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/Campaign/app/Contracts/Crm/Segment.php b/Campaign/app/Contracts/Crm/Segment.php index 328665500..0acaea24e 100644 --- a/Campaign/app/Contracts/Crm/Segment.php +++ b/Campaign/app/Contracts/Crm/Segment.php @@ -128,7 +128,10 @@ public function getProviderData() public function addUserToCache(CampaignSegment $campaignSegment, string $userId): bool { - return $this->redis->sadd(SegmentAggregator::cacheKey($campaignSegment), [$userId]) ?: false; + return $this->redis->sadd( + SegmentAggregator::cacheKey($campaignSegment), + $this->redis instanceof \Redis ? $userId : [$userId] + ) ?: false; } public function removeUserFromCache(CampaignSegment $campaignSegment, string $userId): bool diff --git a/Campaign/app/Jobs/CacheSegmentJob.php b/Campaign/app/Jobs/CacheSegmentJob.php index ba016efd9..09d395d53 100644 --- a/Campaign/app/Jobs/CacheSegmentJob.php +++ b/Campaign/app/Jobs/CacheSegmentJob.php @@ -60,11 +60,18 @@ public function handle(SegmentAggregator $segmentAggregator) throw $e; } - Redis::connection()->set($cacheTimestampKey, date('U'), 'EX', 60*60*24); - Redis::connection()->del([$cacheKey]); + $redis = Redis::connection()->client(); + + $redis->setex($cacheTimestampKey, 60*60*24, date('U')); + $redis->del([$cacheKey]); if ($users->isNotEmpty()) { - Redis::connection()->sadd($cacheKey, $users->toArray()); - Redis::connection()->expire($cacheKey, 60*60*24); + $users = $users->toArray(); + if ($redis instanceof \Redis) { + $redis->sadd($cacheKey, ...$users); + } else { + $redis->sadd($cacheKey, $users); + } + $redis->expire($cacheKey, 60*60*24); } } } diff --git a/Campaign/public/showtime.php b/Campaign/public/showtime.php index f506b2173..5c11db5a4 100644 --- a/Campaign/public/showtime.php +++ b/Campaign/public/showtime.php @@ -15,6 +15,7 @@ use Monolog\Logger; require_once __DIR__ . '/../vendor/autoload.php'; +ini_set('display_errors', 1); /** * asset overrides Laravel's helper function to prevent usage of Laravel's app() @@ -431,10 +432,10 @@ private function renderInternal( if (env('REDIS_PERSISTENT', false)) { $redis->pconnect(env('REDIS_HOST'), (int) env('REDIS_PORT', 6379), 5, 'showtime-'.env('REDIS_DEFAULT_DATABASE')); } else { - $redis->connect(env('REDIS_HOST')); + $redis->connect(env('REDIS_HOST'), (int) env('REDIS_PORT', 6379), 5); } - if (env('REDIS_PASSWORD')) { - $redis->auth(env('REDIS_PASSWORD')); + if ($pwd = env('REDIS_PASSWORD')) { + $redis->auth($pwd); } $redis->setOption(\Redis::OPT_PREFIX, env('REDIS_PREFIX', '')); //$redis->setOption(\Redis::OPT_SERIALIZER, \Redis::SERIALIZER_PHP); From 31843636190818cd5a18077bd1c3af0c1bfaa560 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Mon, 9 Oct 2023 23:12:19 +0200 Subject: [PATCH 11/17] Refresh redis instance after deserialization to avoid duplicated connection --- Campaign/app/Contracts/Crm/Segment.php | 8 +++++++- Campaign/app/Contracts/SegmentAggregator.php | 18 +++++++++++++++++- .../Providers/CrmSegmentServiceProvider.php | 3 +-- Campaign/composer.json | 3 +++ Campaign/public/showtime.php | 1 - 5 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Campaign/app/Contracts/Crm/Segment.php b/Campaign/app/Contracts/Crm/Segment.php index 0acaea24e..de0447c0e 100644 --- a/Campaign/app/Contracts/Crm/Segment.php +++ b/Campaign/app/Contracts/Crm/Segment.php @@ -6,7 +6,6 @@ use App\Contracts\SegmentAggregator; use App\Contracts\SegmentContract; use App\Contracts\SegmentException; -use App\Jobs\CacheSegmentJob; use GuzzleHttp\Client; use GuzzleHttp\Exception\ConnectException; use Illuminate\Support\Collection; @@ -34,6 +33,13 @@ public function __construct(Client $client, \Predis\Client|\Redis $redis) $this->redis = $redis; } + public function setRedisClient(\Predis\Client|\Redis $redis): self + { + $this->redis = $redis; + + return $this; + } + public function provider(): string { return static::PROVIDER_ALIAS; diff --git a/Campaign/app/Contracts/SegmentAggregator.php b/Campaign/app/Contracts/SegmentAggregator.php index fd93c5e7d..683cb3e58 100644 --- a/Campaign/app/Contracts/SegmentAggregator.php +++ b/Campaign/app/Contracts/SegmentAggregator.php @@ -148,6 +148,15 @@ public function getErrors() return $this->errors; } + public function refreshRedisClient(Client|\Redis $redisClient): void + { + foreach ($this->contracts as $contract) { + if (method_exists($contract, 'setRedisClient')) { + $contract->setRedisClient($redisClient); + } + } + } + /** * SegmentAggregator contains Guzzle clients which have properties defined as closures. * It's not possible to serialize closures in plain PHP, but Laravel provides a workaround. @@ -173,6 +182,13 @@ public function serializeToRedis() public static function unserializeFromRedis(Client|\Redis $redisClient): ?SegmentAggregator { $serializedClosure = $redisClient->get(self::SEGMENT_AGGREGATOR_REDIS_KEY); - return $serializedClosure ? unserialize($serializedClosure)() : null; + + /* @var SegmentAggregator $segmentAggregator */ + $segmentAggregator = $serializedClosure ? unserialize($serializedClosure)() : null; + + // set the redis to avoid duplicated connection + $segmentAggregator->refreshRedisClient($redisClient); + + return $segmentAggregator; } } diff --git a/Campaign/app/Providers/CrmSegmentServiceProvider.php b/Campaign/app/Providers/CrmSegmentServiceProvider.php index dda41d50c..cb7b79d56 100644 --- a/Campaign/app/Providers/CrmSegmentServiceProvider.php +++ b/Campaign/app/Providers/CrmSegmentServiceProvider.php @@ -7,7 +7,6 @@ use App\Contracts\SegmentContract; use GuzzleHttp\Client; use Illuminate\Foundation\Application; -use Illuminate\Redis\RedisManager; use Illuminate\Support\ServiceProvider; class CrmSegmentServiceProvider extends ServiceProvider @@ -36,7 +35,7 @@ public function register() 'Authorization' => 'Bearer ' . $app['config']->get('services.crm_segment.token'), ], ]); - /** @var RedisManager $redis */ + /** @var \Predis\Client|\Redis $redis */ $redis = $app->make('redis')->connection()->client(); return new Segment($client, $redis); }); diff --git a/Campaign/composer.json b/Campaign/composer.json index 777bfdcfd..8075a2ccd 100644 --- a/Campaign/composer.json +++ b/Campaign/composer.json @@ -63,6 +63,9 @@ "phpunit/phpunit": "^9.3.3", "squizlabs/php_codesniffer": "^3.6" }, + "suggest": { + "ext-redis": "The extension required for better performance." + }, "autoload": { "classmap": [ "database" diff --git a/Campaign/public/showtime.php b/Campaign/public/showtime.php index 5c11db5a4..a6e080da4 100644 --- a/Campaign/public/showtime.php +++ b/Campaign/public/showtime.php @@ -15,7 +15,6 @@ use Monolog\Logger; require_once __DIR__ . '/../vendor/autoload.php'; -ini_set('display_errors', 1); /** * asset overrides Laravel's helper function to prevent usage of Laravel's app() From 27d3514734af54e3344a06cc1c2963ddd3b9378f Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Tue, 10 Oct 2023 08:14:34 +0200 Subject: [PATCH 12/17] Refresh redis instance in `Segment` after deserialization to avoid duplicated redis connection --- Campaign/app/Contracts/Crm/Segment.php | 7 +++++ Campaign/app/Contracts/SegmentAggregator.php | 30 +++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/Campaign/app/Contracts/Crm/Segment.php b/Campaign/app/Contracts/Crm/Segment.php index 7f6080574..5fb7c7fa1 100644 --- a/Campaign/app/Contracts/Crm/Segment.php +++ b/Campaign/app/Contracts/Crm/Segment.php @@ -34,6 +34,13 @@ public function __construct(Client $client, \Predis\Client $redis) $this->redis = $redis; } + public function setRedisClient(\Predis\Client $redis): self + { + $this->redis = $redis; + + return $this; + } + public function provider(): string { return static::PROVIDER_ALIAS; diff --git a/Campaign/app/Contracts/SegmentAggregator.php b/Campaign/app/Contracts/SegmentAggregator.php index f99a2dc56..750014480 100644 --- a/Campaign/app/Contracts/SegmentAggregator.php +++ b/Campaign/app/Contracts/SegmentAggregator.php @@ -48,12 +48,20 @@ public function list(): Collection public function checkUser(CampaignSegment $campaignSegment, string $userId): bool { + if (!isset($this->contracts[$campaignSegment->provider])) { + return false; + } + return $this->contracts[$campaignSegment->provider] ->checkUser($campaignSegment, $userId); } public function checkBrowser(CampaignSegment $campaignSegment, string $browserId): bool { + if (!isset($this->contracts[$campaignSegment->provider])) { + return false; + } + return $this->contracts[$campaignSegment->provider] ->checkBrowser($campaignSegment, $browserId); } @@ -66,6 +74,10 @@ public function users(CampaignSegment $campaignSegment): Collection public function cacheEnabled(CampaignSegment $campaignSegment): bool { + if (!isset($this->contracts[$campaignSegment->provider])) { + return false; + } + return $this->contracts[$campaignSegment->provider] ->cacheEnabled($campaignSegment); } @@ -136,6 +148,15 @@ public function getErrors() return $this->errors; } + public function refreshRedisClient(Client $redisClient): void + { + foreach ($this->contracts as $contract) { + if (method_exists($contract, 'setRedisClient')) { + $contract->setRedisClient($redisClient); + } + } + } + /** * SegmentAggregator contains Guzzle clients which have properties defined as closures. * It's not possible to serialize closures in plain PHP, but Laravel provides a workaround. @@ -161,6 +182,13 @@ public function serializeToRedis() public static function unserializeFromRedis(Client $redisClient): ?SegmentAggregator { $serializedClosure = $redisClient->get(self::SEGMENT_AGGREGATOR_REDIS_KEY); - return $serializedClosure ? unserialize($serializedClosure)() : null; + + /* @var ?SegmentAggregator $segmentAggregator */ + $segmentAggregator = $serializedClosure ? unserialize($serializedClosure)() : null; + + // set the redis to avoid duplicated connection + $segmentAggregator?->refreshRedisClient($redisClient); + + return $segmentAggregator; } } From 66187a65829466f9168f2fd21dbe6acd271a1481 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Tue, 10 Oct 2023 10:20:09 +0200 Subject: [PATCH 13/17] Code cleanup --- Campaign/config/app.php | 1 - Campaign/public/showtime.php | 1 - 2 files changed, 2 deletions(-) diff --git a/Campaign/config/app.php b/Campaign/config/app.php index 834dd8968..fa3903a99 100644 --- a/Campaign/config/app.php +++ b/Campaign/config/app.php @@ -228,6 +228,5 @@ | Url of the site, where remplib.js is implemented | */ - 'client_site_url' => env('CLIENT_SITE_URL'), ]; diff --git a/Campaign/public/showtime.php b/Campaign/public/showtime.php index a6e080da4..4a55a4567 100644 --- a/Campaign/public/showtime.php +++ b/Campaign/public/showtime.php @@ -437,7 +437,6 @@ private function renderInternal( $redis->auth($pwd); } $redis->setOption(\Redis::OPT_PREFIX, env('REDIS_PREFIX', '')); - //$redis->setOption(\Redis::OPT_SERIALIZER, \Redis::SERIALIZER_PHP); $redis->setOption(\Redis::OPT_SERIALIZER, \Redis::SERIALIZER_NONE); $redis->select((int) env('REDIS_DEFAULT_DATABASE', 0)); From 1fcd2c558efb62d8fecaf983e491c61068c95322 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Tue, 10 Oct 2023 12:45:17 +0200 Subject: [PATCH 14/17] Store device detection to local cache --- Campaign/app/Http/Showtime/Showtime.php | 26 ++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index a2f2327af..717074d77 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -37,7 +37,7 @@ class Showtime private $snippets; - private array $segmentCheckCache = []; + private array $localCache = []; public function __construct( private ClientInterface $redis, @@ -161,7 +161,7 @@ public function showtime(string $userData, string $callback, ShowtimeResponse $s $campaigns = []; $campaignBanners = []; $suppressedBanners = []; - $this->segmentCheckCache = []; + $this->localCache = []; reset($campaignIds); foreach ($fetchedCampaigns as $fetchedCampaign) { /** @var Campaign $campaign */ @@ -726,13 +726,13 @@ private function isCacheValid(CampaignSegment $campaignSegment): bool } $cacheKey = SegmentAggregator::cacheKey($campaignSegment). '|timestamp'; - if (isset($this->segmentCheckCache[$cacheKey])) { + if (isset($this->localCache[$cacheKey])) { return true; } $cacheKeyTimeStamp = $this->redis->get($cacheKey ); if ($cacheKeyTimeStamp) { - $this->segmentCheckCache[$cacheKey] = true; + $this->localCache[$cacheKey] = true; return true; } @@ -761,12 +761,12 @@ private function loadOneTimeBanner(array $bannerKeys): ?Banner private function checkUserInSegment(CampaignSegment $campaignSegment, $userId): bool { $cacheKey = SegmentAggregator::cacheKey($campaignSegment). '|user|'.$userId; - if (isset($this->segmentCheckCache[$cacheKey])) { - return $this->segmentCheckCache[$cacheKey]; + if (isset($this->localCache[$cacheKey])) { + return $this->localCache[$cacheKey]; } $belongsToSegment = $this->segmentAggregator->checkUser($campaignSegment, (string) $userId); - $this->segmentCheckCache[$cacheKey] = $belongsToSegment; + $this->localCache[$cacheKey] = $belongsToSegment; return $belongsToSegment; } @@ -774,12 +774,12 @@ private function checkUserInSegment(CampaignSegment $campaignSegment, $userId): private function checkBrowserInSegment(CampaignSegment $campaignSegment, $browserId): bool { $cacheKey = SegmentAggregator::cacheKey($campaignSegment). '|browser|'.$browserId; - if (isset($this->segmentCheckCache[$cacheKey])) { - return $this->segmentCheckCache[$cacheKey]; + if (isset($this->localCache[$cacheKey])) { + return $this->localCache[$cacheKey]; } $belongsToSegment = $this->segmentAggregator->checkBrowser($campaignSegment, (string)$browserId); - $this->segmentCheckCache[$cacheKey] = $belongsToSegment; + $this->localCache[$cacheKey] = $belongsToSegment; return $belongsToSegment; } @@ -800,7 +800,8 @@ private function isAcceptedByDeviceRules($userData, Campaign $campaign, $userId) // check result of device detection in redis $deviceKey = self::DEVICE_DETECTION_KEY.':'.md5($userData->userAgent); - $deviceDetection = $this->redis->get($deviceKey); + $deviceDetection = $this->localCache[$deviceKey] ?? $this->redis->get($deviceKey); + if ($deviceDetection) { if ($deviceDetection === Campaign::DEVICE_MOBILE && !in_array(Campaign::DEVICE_MOBILE, $campaign->devices,true)) { return false; @@ -808,6 +809,8 @@ private function isAcceptedByDeviceRules($userData, Campaign $campaign, $userId) if ($deviceDetection === Campaign::DEVICE_DESKTOP && !in_array(Campaign::DEVICE_DESKTOP, $campaign->devices,true)) { return false; } + + return true; } // parse user agent @@ -824,6 +827,7 @@ private function isAcceptedByDeviceRules($userData, Campaign $campaign, $userId) } // store result to redis to faster resolution on the next attempt + $this->localCache[$deviceKey] = $deviceDetectionValue; $this->redis->setex($deviceKey, self::DEVICE_DETECTION_TTL, $deviceDetectionValue); if ($isMobile && !in_array(Campaign::DEVICE_MOBILE, $campaign->devices, true)) { From b9b918ef8fc243d78f02b3e6efc9827467709835 Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Tue, 10 Oct 2023 12:55:16 +0200 Subject: [PATCH 15/17] Store device detection to local cache --- Campaign/app/Http/Showtime/Showtime.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index 717074d77..7f04499e2 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -800,7 +800,14 @@ private function isAcceptedByDeviceRules($userData, Campaign $campaign, $userId) // check result of device detection in redis $deviceKey = self::DEVICE_DETECTION_KEY.':'.md5($userData->userAgent); - $deviceDetection = $this->localCache[$deviceKey] ?? $this->redis->get($deviceKey); + if (isset($this->localCache[$deviceKey])) { + $deviceDetection = $this->localCache[$deviceKey]; + } else { + $deviceDetection = $this->redis->get($deviceKey); + if ($deviceDetection) { + $this->localCache[$deviceKey] = $deviceDetection; + } + } if ($deviceDetection) { if ($deviceDetection === Campaign::DEVICE_MOBILE && !in_array(Campaign::DEVICE_MOBILE, $campaign->devices,true)) { From 0ec4b93c2bbbedf1d7f159a54fb571a7697a69ce Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Tue, 10 Oct 2023 14:41:26 +0200 Subject: [PATCH 16/17] Fix tests --- Campaign/app/Http/Showtime/Showtime.php | 9 ++++++++- Campaign/tests/Feature/ShowtimeTest.php | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Campaign/app/Http/Showtime/Showtime.php b/Campaign/app/Http/Showtime/Showtime.php index 7f04499e2..3f20f2fea 100644 --- a/Campaign/app/Http/Showtime/Showtime.php +++ b/Campaign/app/Http/Showtime/Showtime.php @@ -82,6 +82,13 @@ public function setAlignmentsMap(\App\Models\Alignment\Map $alignments) $this->alignmentsMap = $alignments->alignments(); } + public function flushLocalCache(): self + { + $this->localCache = []; + + return $this; + } + public function showtime(string $userData, string $callback, ShowtimeResponse $showtimeResponse) { try { @@ -161,7 +168,7 @@ public function showtime(string $userData, string $callback, ShowtimeResponse $s $campaigns = []; $campaignBanners = []; $suppressedBanners = []; - $this->localCache = []; + $this->flushLocalCache(); reset($campaignIds); foreach ($fetchedCampaigns as $fetchedCampaign) { /** @var Campaign $campaign */ diff --git a/Campaign/tests/Feature/ShowtimeTest.php b/Campaign/tests/Feature/ShowtimeTest.php index ab87d0ee6..d7b92d3de 100644 --- a/Campaign/tests/Feature/ShowtimeTest.php +++ b/Campaign/tests/Feature/ShowtimeTest.php @@ -432,6 +432,7 @@ public function testSegmentRules() $this->assertNull($this->showtime->shouldDisplay($this->campaign, $userData, $activeCampaignUuids)); $this->assertCount(0, $activeCampaignUuids); + $this->showtime->flushLocalCache(); $this->assertNotNull($this->showtime->shouldDisplay($this->campaign, $userData, $activeCampaignUuids)); $this->assertCount(1, $activeCampaignUuids); @@ -442,6 +443,7 @@ public function testSegmentRules() $this->segmentAggregator->shouldReceive('checkBrowser')->andReturn(false, true); $this->assertNotNull($this->showtime->shouldDisplay($this->campaign, $userData, $activeCampaignUuids)); + $this->showtime->flushLocalCache(); $this->assertNull($this->showtime->shouldDisplay($this->campaign, $userData, $activeCampaignUuids)); } From 8aba3c95a4f57a68f016eee47d48ce5e3be3492b Mon Sep 17 00:00:00 2001 From: Andrej Hudec Date: Fri, 27 Oct 2023 09:12:04 +0200 Subject: [PATCH 17/17] Rollback default value in ShowtimeResponse::success --- Campaign/app/Http/Showtime/ControllerShowtimeResponse.php | 2 +- Campaign/app/Http/Showtime/ShowtimeResponse.php | 2 +- Campaign/public/showtime.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php b/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php index 556593c79..cfc424d73 100644 --- a/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php +++ b/Campaign/app/Http/Showtime/ControllerShowtimeResponse.php @@ -18,7 +18,7 @@ public function error($callback, int $statusCode, array $errors) ]); } - public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners = []) + public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners) { return response() ->jsonp($callback, [ diff --git a/Campaign/app/Http/Showtime/ShowtimeResponse.php b/Campaign/app/Http/Showtime/ShowtimeResponse.php index 2e7b1f9b9..91956af15 100644 --- a/Campaign/app/Http/Showtime/ShowtimeResponse.php +++ b/Campaign/app/Http/Showtime/ShowtimeResponse.php @@ -10,7 +10,7 @@ interface ShowtimeResponse { public function error($callback, int $statusCode, array $errors); - public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners = []); + public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners); public function renderBanner(Banner $banner, array $alignments, array $dimensions, array $positions, array $snippets): string; diff --git a/Campaign/public/showtime.php b/Campaign/public/showtime.php index 09b88c669..46738bc37 100644 --- a/Campaign/public/showtime.php +++ b/Campaign/public/showtime.php @@ -140,7 +140,7 @@ public function error($callback, int $statusCode, array $errors) ], $statusCode); } - public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners = []) + public function success(string $callback, $data, $activeCampaigns, $providerData, $suppressedBanners) { $this->jsonpResponse($callback, [ 'success' => true,