From eb1e87532ade3a1e45c52d1d58bd642b8b466fdf Mon Sep 17 00:00:00 2001 From: Matias Melograno Date: Wed, 11 Jan 2023 18:23:10 -0300 Subject: [PATCH 1/3] added tracker component --- src/SplitIO/Component/Common/Di.php | 27 +++++++-- src/SplitIO/Sdk.php | 14 +---- .../InputValidation/FactoryTrackerTest.php | 59 +++++++++++-------- .../GetTreatmentValidationTest.php | 1 + .../GetTreatmentsValidationTest.php | 1 + .../InputValidation/ManagerValidationTest.php | 1 + .../InputValidation/TrackValidationTest.php | 1 + tests/Suite/Redis/ReflectiveTools.php | 9 +++ 8 files changed, 72 insertions(+), 41 deletions(-) diff --git a/src/SplitIO/Component/Common/Di.php b/src/SplitIO/Component/Common/Di.php index 10a353a2..ac7aa4e9 100644 --- a/src/SplitIO/Component/Common/Di.php +++ b/src/SplitIO/Component/Common/Di.php @@ -11,7 +11,7 @@ class Di { private \SplitIO\Component\Log\Logger $logger; - private int $factoryTracker = 0; + private array $factoryTracker = array(); private string $ipAddress = ""; @@ -63,12 +63,31 @@ public function __wakeup() } /** + * @param string $apiKey * @return int */ - public static function trackFactory() + public static function trackFactory($apiKey) { - self::getInstance()->factoryTracker += 1; - return self::getInstance()->factoryTracker; + $tracked = 1; + if (isset(self::getInstance()->factoryTracker[$apiKey])) { + $currentInstances = self::getInstance()->factoryTracker[$apiKey]; + self::getInstance()->getLogger()->warning( + "Factory Instantiation: You already have " . $currentInstances . + " factory/ies with this API Key. " . + "We recommend keeping only one instance of the factory at all times " . + "(Singleton pattern) and reusing it throughout your application." + ); + $tracked = $currentInstances + $tracked; + } elseif (count(self::getInstance()->factoryTracker) > 0) { + self::getInstance()->getLogger()->warning( + "Factory Instantiation: You already have an instance of the Split factory. " . + "Make sure you definitely want this additional instance. " . + "We recommend keeping only one instance of the factory at all times " . + "(Singleton pattern) and reusing it throughout your application." + ); + } + self::getInstance()->factoryTracker[$apiKey] = $tracked; + return $tracked; } /** diff --git a/src/SplitIO/Sdk.php b/src/SplitIO/Sdk.php index 5b89dbab..ecb39081 100644 --- a/src/SplitIO/Sdk.php +++ b/src/SplitIO/Sdk.php @@ -32,7 +32,7 @@ public static function factory($apiKey = 'localhost', array $options = array()) $options['apiKey'] = $apiKey; //Tracking Factory Instantiation - self::registerInstance(); + Di::trackFactory($apiKey); //Register Logger self::registerLogger((isset($options['log'])) ? $options['log'] : array()); @@ -90,16 +90,4 @@ private static function setIP($ip) { \SplitIO\Component\Common\Di::setIPAddress($ip); } - - /** - * Register factory instance - */ - private static function registerInstance() - { - if (Di::trackFactory() > 1) { - Di::getLogger()->warning("Factory Instantiation: You already have an instance of the Split factory. " - . "Make sure you definitely want this additional instance. We recommend keeping only one instance of " - . "the factory at all times (Singleton pattern) and reusing it throughout your application."); - } - } } diff --git a/tests/Suite/InputValidation/FactoryTrackerTest.php b/tests/Suite/InputValidation/FactoryTrackerTest.php index 5776afbf..c784312a 100644 --- a/tests/Suite/InputValidation/FactoryTrackerTest.php +++ b/tests/Suite/InputValidation/FactoryTrackerTest.php @@ -5,22 +5,6 @@ class FactoryTrackerTest extends \PHPUnit\Framework\TestCase { - private function getFactoryClient() - { - $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); - $options = array('prefix' => TEST_PREFIX); - - $sdkConfig = array( - 'log' => array('adapter' => 'stdout'), - 'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options) - ); - - //Initializing the SDK instance. - $splitFactory = \SplitIO\Sdk::factory('asdqwe123456', $sdkConfig); - - return $splitFactory; - } - private function getMockedLogger() { //Initialize mock logger @@ -38,18 +22,45 @@ private function getMockedLogger() public function testMultipleClientInstantiation() { - $splitFactory = $this->getFactoryClient(); + ReflectiveTools::overrideTracker(); + $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); + $options = array('prefix' => TEST_PREFIX); + + $sdkConfig = array( + 'log' => array('adapter' => 'stdout'), + 'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options) + ); + + //Initializing the SDK instance. + $splitFactory = \SplitIO\Sdk::factory('asdqwe123456', $sdkConfig); $this->assertNotNull($splitFactory->client()); $logger = $this->getMockedLogger(); - - $logger->expects($this->once()) + $logger->expects($this->any()) ->method('warning') - ->with($this->equalTo("Factory Instantiation: You already have an instance of the Split factory. " - . "Make sure you definitely want this additional instance. We recommend keeping only one instance " - . "of the factory at all times (Singleton pattern) and reusing it throughout your application.")); - - $splitFactory2 = $this->getFactoryClient(); + ->with($this->logicalOr( + $this->equalTo("Factory Instantiation: You already have 1 factory/ies with this API Key. " + . "We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing " + . "it throughout your application."), + $this->equalTo("Factory Instantiation: You already have 2 factory/ies with this API Key. " + . "We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing " + . "it throughout your application."), + $this->equalTo("Factory Instantiation: You already have an instance of the Split factory. " + . "Make sure you definitely want this additional instance. We recommend keeping only one instance " + . "of the factory at all times (Singleton pattern) and reusing it throughout your application.") + ) + ); + + //Initializing the SDK instance2. + $splitFactory2 = \SplitIO\Sdk::factory('asdqwe123456', $sdkConfig); $this->assertNotNull($splitFactory2->client()); + + //Initializing the SDK instance3. + $splitFactory3 = \SplitIO\Sdk::factory('asdqwe123456', $sdkConfig); + $this->assertNotNull($splitFactory3->client()); + + //Initializing the SDK instance4. + $splitFactory4 = \SplitIO\Sdk::factory('other', $sdkConfig); + $this->assertNotNull($splitFactory4->client()); } } diff --git a/tests/Suite/InputValidation/GetTreatmentValidationTest.php b/tests/Suite/InputValidation/GetTreatmentValidationTest.php index 4942ed6d..d2ce149b 100644 --- a/tests/Suite/InputValidation/GetTreatmentValidationTest.php +++ b/tests/Suite/InputValidation/GetTreatmentValidationTest.php @@ -10,6 +10,7 @@ class GetTreatmentValidationTest extends \PHPUnit\Framework\TestCase { private function getFactoryClient() { + ReflectiveTools::overrideTracker(); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); diff --git a/tests/Suite/InputValidation/GetTreatmentsValidationTest.php b/tests/Suite/InputValidation/GetTreatmentsValidationTest.php index dfb2be12..899e83af 100644 --- a/tests/Suite/InputValidation/GetTreatmentsValidationTest.php +++ b/tests/Suite/InputValidation/GetTreatmentsValidationTest.php @@ -10,6 +10,7 @@ class GetTreatmentsValidationTest extends \PHPUnit\Framework\TestCase { private function getFactoryClient() { + ReflectiveTools::overrideTracker(); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); diff --git a/tests/Suite/InputValidation/ManagerValidationTest.php b/tests/Suite/InputValidation/ManagerValidationTest.php index 09020a0f..d2fd833f 100644 --- a/tests/Suite/InputValidation/ManagerValidationTest.php +++ b/tests/Suite/InputValidation/ManagerValidationTest.php @@ -7,6 +7,7 @@ class ManagerValidationTest extends \PHPUnit\Framework\TestCase { private function getFactoryClient() { + ReflectiveTools::overrideTracker(); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); diff --git a/tests/Suite/InputValidation/TrackValidationTest.php b/tests/Suite/InputValidation/TrackValidationTest.php index 9f66e586..3b1ea1f4 100644 --- a/tests/Suite/InputValidation/TrackValidationTest.php +++ b/tests/Suite/InputValidation/TrackValidationTest.php @@ -10,6 +10,7 @@ class TrackValidationTest extends \PHPUnit\Framework\TestCase { private function getFactoryClient() { + ReflectiveTools::overrideTracker(); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); diff --git a/tests/Suite/Redis/ReflectiveTools.php b/tests/Suite/Redis/ReflectiveTools.php index d128e510..fe5c1e0b 100644 --- a/tests/Suite/Redis/ReflectiveTools.php +++ b/tests/Suite/Redis/ReflectiveTools.php @@ -73,4 +73,13 @@ public static function resetIPAddress() $property->setAccessible(true); $property->setValue($di, ""); } + + public static function overrideTracker() + { + $di = Di::getInstance(); + $reflection = new \ReflectionClass('SplitIO\Component\Common\Di'); + $property = $reflection->getProperty('factoryTracker'); + $property->setAccessible(true); + $property->setValue($di, array()); + } } From ca411b97a6d1410de0882b61db20b565ea8e4d48 Mon Sep 17 00:00:00 2001 From: Matias Melograno Date: Wed, 11 Jan 2023 18:28:53 -0300 Subject: [PATCH 2/3] tracking instances --- src/SplitIO/Component/Common/Di.php | 19 +++++++++++++------ .../InputValidation/FactoryTrackerTest.php | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/SplitIO/Component/Common/Di.php b/src/SplitIO/Component/Common/Di.php index ac7aa4e9..5f76bc9a 100644 --- a/src/SplitIO/Component/Common/Di.php +++ b/src/SplitIO/Component/Common/Di.php @@ -71,12 +71,19 @@ public static function trackFactory($apiKey) $tracked = 1; if (isset(self::getInstance()->factoryTracker[$apiKey])) { $currentInstances = self::getInstance()->factoryTracker[$apiKey]; - self::getInstance()->getLogger()->warning( - "Factory Instantiation: You already have " . $currentInstances . - " factory/ies with this API Key. " . - "We recommend keeping only one instance of the factory at all times " . - "(Singleton pattern) and reusing it throughout your application." - ); + if ($currentInstances == 1) { + self::getInstance()->getLogger()->warning( + "Factory Instantiation: You already have 1 factory with this API Key. " . + "We recommend keeping only one instance of the factory at all times " . + "(Singleton pattern) and reusing it throughout your application." + ); + } else { + self::getInstance()->getLogger()->warning( + "Factory Instantiation: You already have " . $currentInstances . " factories with this API Key. " . + "We recommend keeping only one instance of the factory at all times " . + "(Singleton pattern) and reusing it throughout your application." + ); + } $tracked = $currentInstances + $tracked; } elseif (count(self::getInstance()->factoryTracker) > 0) { self::getInstance()->getLogger()->warning( diff --git a/tests/Suite/InputValidation/FactoryTrackerTest.php b/tests/Suite/InputValidation/FactoryTrackerTest.php index c784312a..f4a4af7b 100644 --- a/tests/Suite/InputValidation/FactoryTrackerTest.php +++ b/tests/Suite/InputValidation/FactoryTrackerTest.php @@ -39,10 +39,10 @@ public function testMultipleClientInstantiation() $logger->expects($this->any()) ->method('warning') ->with($this->logicalOr( - $this->equalTo("Factory Instantiation: You already have 1 factory/ies with this API Key. " + $this->equalTo("Factory Instantiation: You already have 1 factory with this API Key. " . "We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing " . "it throughout your application."), - $this->equalTo("Factory Instantiation: You already have 2 factory/ies with this API Key. " + $this->equalTo("Factory Instantiation: You already have 2 factories with this API Key. " . "We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing " . "it throughout your application."), $this->equalTo("Factory Instantiation: You already have an instance of the Split factory. " From 7dcc25e6511cf52d6921e933c7c890af2794ab61 Mon Sep 17 00:00:00 2001 From: Matias Melograno Date: Mon, 16 Jan 2023 18:47:37 -0300 Subject: [PATCH 3/3] improved logging multiple factories --- src/SplitIO/Component/Common/Di.php | 41 +++++++------------ .../InputValidation/FactoryTrackerTest.php | 4 +- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/SplitIO/Component/Common/Di.php b/src/SplitIO/Component/Common/Di.php index 5f76bc9a..bd8b6849 100644 --- a/src/SplitIO/Component/Common/Di.php +++ b/src/SplitIO/Component/Common/Di.php @@ -9,6 +9,13 @@ */ class Di { + const SAME_APIKEY = "Factory Instantiation: You already have %s factory/factories with this API Key. " + . "We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing it throughout your application."; + + const MULTIPLE_INSTANCES = "Factory Instantiation: You already have an instance of the Split factory. " . + "Make sure you definitely want this additional instance. We recommend keeping only one instance of the factory at all times " . + "(Singleton pattern) and reusing it throughout your application."; + private \SplitIO\Component\Log\Logger $logger; private array $factoryTracker = array(); @@ -68,33 +75,15 @@ public function __wakeup() */ public static function trackFactory($apiKey) { - $tracked = 1; - if (isset(self::getInstance()->factoryTracker[$apiKey])) { - $currentInstances = self::getInstance()->factoryTracker[$apiKey]; - if ($currentInstances == 1) { - self::getInstance()->getLogger()->warning( - "Factory Instantiation: You already have 1 factory with this API Key. " . - "We recommend keeping only one instance of the factory at all times " . - "(Singleton pattern) and reusing it throughout your application." - ); - } else { - self::getInstance()->getLogger()->warning( - "Factory Instantiation: You already have " . $currentInstances . " factories with this API Key. " . - "We recommend keeping only one instance of the factory at all times " . - "(Singleton pattern) and reusing it throughout your application." - ); - } - $tracked = $currentInstances + $tracked; - } elseif (count(self::getInstance()->factoryTracker) > 0) { - self::getInstance()->getLogger()->warning( - "Factory Instantiation: You already have an instance of the Split factory. " . - "Make sure you definitely want this additional instance. " . - "We recommend keeping only one instance of the factory at all times " . - "(Singleton pattern) and reusing it throughout your application." - ); + $current = self::getInstance()->factoryTracker[$apiKey] ?? 0; + if ($current == 0) { + self::getInstance()->getLogger()->warning(self::MULTIPLE_INSTANCES); + } else { + self::getInstance()->getLogger()->warning(sprintf(self::SAME_APIKEY, $current)); } - self::getInstance()->factoryTracker[$apiKey] = $tracked; - return $tracked; + $current += 1; + self::getInstance()->factoryTracker[$apiKey] = $current; + return $current; } /** diff --git a/tests/Suite/InputValidation/FactoryTrackerTest.php b/tests/Suite/InputValidation/FactoryTrackerTest.php index f4a4af7b..1f29109a 100644 --- a/tests/Suite/InputValidation/FactoryTrackerTest.php +++ b/tests/Suite/InputValidation/FactoryTrackerTest.php @@ -39,10 +39,10 @@ public function testMultipleClientInstantiation() $logger->expects($this->any()) ->method('warning') ->with($this->logicalOr( - $this->equalTo("Factory Instantiation: You already have 1 factory with this API Key. " + $this->equalTo("Factory Instantiation: You already have 1 factory/factories with this API Key. " . "We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing " . "it throughout your application."), - $this->equalTo("Factory Instantiation: You already have 2 factories with this API Key. " + $this->equalTo("Factory Instantiation: You already have 2 factory/factories with this API Key. " . "We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing " . "it throughout your application."), $this->equalTo("Factory Instantiation: You already have an instance of the Split factory. "