From f7a8d13b9116c441a3a8cf238c9d3c1bf4ef76ba Mon Sep 17 00:00:00 2001 From: Matias Melograno Date: Wed, 14 Dec 2022 18:05:52 -0300 Subject: [PATCH 1/3] updated di logger --- src/SplitIO/Component/Common/Di.php | 81 +++++++++++-------- .../Condition/Matcher/ContainsString.php | 1 - .../Grammar/Condition/Matcher/EndsWith.php | 1 - .../Condition/Matcher/EqualToBoolean.php | 1 - .../Grammar/Condition/Matcher/Regex.php | 1 - .../Grammar/Condition/Matcher/StartsWith.php | 1 - .../Grammar/Condition/Matcher/Whitelist.php | 1 - src/SplitIO/Sdk.php | 28 ++----- src/SplitIO/functions.php | 3 +- .../DynamicConfigurations/EvaluatorTest.php | 1 - .../Suite/DynamicConfigurations/SplitTest.php | 1 - tests/Suite/Engine/HashTest.php | 1 - 12 files changed, 55 insertions(+), 66 deletions(-) diff --git a/src/SplitIO/Component/Common/Di.php b/src/SplitIO/Component/Common/Di.php index 270b966e..f4661469 100644 --- a/src/SplitIO/Component/Common/Di.php +++ b/src/SplitIO/Component/Common/Di.php @@ -1,8 +1,7 @@ factoryTracker += 1; + return $this->factoryTracker; + } + /** - * @param $key - * @param $instance + * @param \SplitIO\Component\Log\Logger $logger */ - private function setKey($key, $instance) + private function __setLogger(Logger $logger) { - $this->container[$key] = $instance; + if (is_null($this->logger)) { + $this->logger = $logger; + } } /** - * @param $key - * @return mixed + * @return null|\SplitIO\Component\Log\Logger */ - private function getKey($key) + private function __getLogger() { - return (isset($this->container[$key])) ? $this->container[$key] : null; + return $this->logger; } /** - * Set an object instance with its key - * @param $key - * @param $instance + * @return null|string */ - public static function set($key, $instance) + private function __getIPAddress() { - self::getInstance()->setKey($key, $instance); + return $this->ipAddress; } /** - * Given a key returns the object instance associated with this. - * @param $key - * @return mixed + * @return int */ - public static function get($key) + public static function trackFactory() { - return self::getInstance()->getKey($key); + return self::getInstance()->__trackFactory(); } - /** - * @param LoggerInterface $logger + * @param \SplitIO\Component\Log\Logger $logger */ - public static function setLogger($logger) + public static function setLogger(Logger $logger) { - self::set(self::KEY_LOG, $logger); + self::getInstance()->__setLogger($logger); } /** - * @return null|\Psr\Log\LoggerInterface + * @return null|\SplitIO\Component\Log\Logger */ public static function getLogger() { - return self::get(self::KEY_LOG); + return self::getInstance()->__getLogger(); + } + + /** + * @param string $ip + */ + public static function setIPAddress(string $ip) + { + if (is_null($this->ip)) { + $this->ip = $ip; + } + } + + /** + * @return null|string + */ + public static function getIPAddress() + { + return self::getInstance()->__getIPAddress(); } } diff --git a/src/SplitIO/Grammar/Condition/Matcher/ContainsString.php b/src/SplitIO/Grammar/Condition/Matcher/ContainsString.php index 4aeee124..87d45467 100644 --- a/src/SplitIO/Grammar/Condition/Matcher/ContainsString.php +++ b/src/SplitIO/Grammar/Condition/Matcher/ContainsString.php @@ -1,7 +1,6 @@ critical("Factory Instantiation: creating multiple factories is not possible. " - . "You have already created a factory."); - return true; - */ - return false; + \SplitIO\Component\Common\Di::setIPAddress('ipAddress', $ip); } /** @@ -118,6 +98,10 @@ private static function instanceExists() */ private static function registerInstance() { - Di::set(Di::KEY_FACTORY_TRACKER, true); + 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/src/SplitIO/functions.php b/src/SplitIO/functions.php index 0ef2da0f..f7fd4978 100644 --- a/src/SplitIO/functions.php +++ b/src/SplitIO/functions.php @@ -1,7 +1,6 @@ Date: Thu, 5 Jan 2023 14:48:19 -0300 Subject: [PATCH 2/3] updated Di component --- src/SplitIO/Component/Common/Di.php | 22 ++++- .../Component/Common/ServiceProvider.php | 10 --- src/SplitIO/Sdk.php | 4 +- src/SplitIO/functions.php | 2 +- tests/Suite/Adapter/RedisAdapterTest.php | 4 +- tests/Suite/Attributes/SdkAttributesTest.php | 2 - tests/Suite/Component/TrafficTypeTests.php | 2 +- tests/Suite/Engine/HashTest.php | 2 - tests/Suite/Engine/SplitterTest.php | 2 +- .../InputValidation/FactoryTrackerTest.php | 18 ++--- .../GetTreatmentValidationTest.php | 5 +- .../GetTreatmentsValidationTest.php | 5 +- .../InputValidation/ManagerValidationTest.php | 5 +- .../InputValidation/TrackValidationTest.php | 5 +- tests/Suite/Matchers/MatchersTest.php | 1 - tests/Suite/Redis/ReflectiveTools.php | 19 +++++ tests/Suite/Sdk/ImpressionWrapperTest.php | 7 +- tests/Suite/Sdk/ImpressionsTest.php | 2 - tests/Suite/Sdk/SdkClientTest.php | 80 +++++++++++++++---- tests/Suite/Sdk/SdkReadOnlyTest.php | 4 +- .../TrafficAllocationTest.php | 3 +- 21 files changed, 132 insertions(+), 72 deletions(-) delete mode 100644 src/SplitIO/Component/Common/ServiceProvider.php diff --git a/src/SplitIO/Component/Common/Di.php b/src/SplitIO/Component/Common/Di.php index f4661469..84f98300 100644 --- a/src/SplitIO/Component/Common/Di.php +++ b/src/SplitIO/Component/Common/Di.php @@ -13,7 +13,7 @@ class Di private int $factoryTracker = 0; - private string|null $ipAddress = null; + private string $ipAddress = ""; /** * @var Singleton The reference to *Singleton* instance of this class @@ -75,7 +75,9 @@ private function __setLogger(Logger $logger) { if (is_null($this->logger)) { $this->logger = $logger; + return; } + $this->logger->debug("logger was set before, ignoring new instance provided"); } /** @@ -86,6 +88,20 @@ private function __getLogger() return $this->logger; } + /** + * @param string $ip + */ + private function __setIPAddress(string $ip) + { + if (empty($this->ipAddress)) { + $this->ipAddress = $ip; + return; + } + if (!(is_null($this->logger))) { + $this->logger->debug("IPAddress was set before, ignoring new instance provided"); + } + } + /** * @return null|string */ @@ -123,9 +139,7 @@ public static function getLogger() */ public static function setIPAddress(string $ip) { - if (is_null($this->ip)) { - $this->ip = $ip; - } + self::getInstance()->__setIPAddress($ip); } /** diff --git a/src/SplitIO/Component/Common/ServiceProvider.php b/src/SplitIO/Component/Common/ServiceProvider.php deleted file mode 100644 index c792de67..00000000 --- a/src/SplitIO/Component/Common/ServiceProvider.php +++ /dev/null @@ -1,10 +0,0 @@ -getMock(); - Di::set(Di::KEY_LOG, $logger); + ReflectiveTools::overrideLogger($logger); return $logger; } diff --git a/tests/Suite/Attributes/SdkAttributesTest.php b/tests/Suite/Attributes/SdkAttributesTest.php index 6b3e050c..6678b17d 100644 --- a/tests/Suite/Attributes/SdkAttributesTest.php +++ b/tests/Suite/Attributes/SdkAttributesTest.php @@ -13,8 +13,6 @@ class SdkAttributesTest extends \PHPUnit\Framework\TestCase { public function testClient() { - Di::set(Di::KEY_FACTORY_TRACKER, false); - //Testing version string $this->assertTrue(is_string(\SplitIO\version())); diff --git a/tests/Suite/Component/TrafficTypeTests.php b/tests/Suite/Component/TrafficTypeTests.php index 69e5c781..fc169461 100644 --- a/tests/Suite/Component/TrafficTypeTests.php +++ b/tests/Suite/Component/TrafficTypeTests.php @@ -17,7 +17,7 @@ private function getMockedLogger() 'alert', 'notice', 'write', 'log')) ->getMock(); - Di::set(Di::KEY_LOG, $logger); + Di::setLogger($logger); return $logger; } diff --git a/tests/Suite/Engine/HashTest.php b/tests/Suite/Engine/HashTest.php index 18a20831..ea0ff26c 100644 --- a/tests/Suite/Engine/HashTest.php +++ b/tests/Suite/Engine/HashTest.php @@ -79,8 +79,6 @@ public function testMurmur3HashFunction() public function testAlgoField() { - Di::set(Di::KEY_FACTORY_TRACKER, false); - $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); diff --git a/tests/Suite/Engine/SplitterTest.php b/tests/Suite/Engine/SplitterTest.php index 4febbe73..36655a22 100644 --- a/tests/Suite/Engine/SplitterTest.php +++ b/tests/Suite/Engine/SplitterTest.php @@ -17,7 +17,7 @@ class SplitterTest extends \PHPUnit\Framework\TestCase public function testDiLog() { $logger = LoggerFactory::setupLogger(array('adapter' => 'stdout', 'level' => 'error')); - ServiceProvider::registerLogger($logger); + Di::setLogger($logger); $this->assertTrue(true); } diff --git a/tests/Suite/InputValidation/FactoryTrackerTest.php b/tests/Suite/InputValidation/FactoryTrackerTest.php index e0066aca..5776afbf 100644 --- a/tests/Suite/InputValidation/FactoryTrackerTest.php +++ b/tests/Suite/InputValidation/FactoryTrackerTest.php @@ -1,7 +1,7 @@ getMock(); - Di::set(Di::KEY_LOG, $logger); + ReflectiveTools::overrideLogger($logger); return $logger; } public function testMultipleClientInstantiation() { - // @TODO FIX WHEN WE ALLOW MULTIPLE - /* - Di::set(Di::KEY_FACTORY_TRACKER, false); $splitFactory = $this->getFactoryClient(); $this->assertNotNull($splitFactory->client()); $logger = $this->getMockedLogger(); $logger->expects($this->once()) - ->method('critical') - ->with($this->equalTo("Factory Instantiation: creating multiple factories is not possible. " - . "You have already created a factory.")); + ->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(); - $this->assertEquals(null, $splitFactory2); - */ - $this->assertEquals(true, true); + $this->assertNotNull($splitFactory2->client()); } } diff --git a/tests/Suite/InputValidation/GetTreatmentValidationTest.php b/tests/Suite/InputValidation/GetTreatmentValidationTest.php index 312ab2ac..4942ed6d 100644 --- a/tests/Suite/InputValidation/GetTreatmentValidationTest.php +++ b/tests/Suite/InputValidation/GetTreatmentValidationTest.php @@ -1,7 +1,7 @@ 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); @@ -36,7 +35,7 @@ private function getMockedLogger() 'alert', 'notice', 'write', 'log')) ->getMock(); - Di::set(Di::KEY_LOG, $logger); + ReflectiveTools::overrideLogger($logger); return $logger; } diff --git a/tests/Suite/InputValidation/GetTreatmentsValidationTest.php b/tests/Suite/InputValidation/GetTreatmentsValidationTest.php index 7e8d9da6..dfb2be12 100644 --- a/tests/Suite/InputValidation/GetTreatmentsValidationTest.php +++ b/tests/Suite/InputValidation/GetTreatmentsValidationTest.php @@ -1,8 +1,8 @@ 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); @@ -36,7 +35,7 @@ private function getMockedLogger() 'alert', 'notice', 'write', 'log')) ->getMock(); - Di::set(Di::KEY_LOG, $logger); + ReflectiveTools::overrideLogger($logger); return $logger; } diff --git a/tests/Suite/InputValidation/ManagerValidationTest.php b/tests/Suite/InputValidation/ManagerValidationTest.php index 77ef7bb9..09020a0f 100644 --- a/tests/Suite/InputValidation/ManagerValidationTest.php +++ b/tests/Suite/InputValidation/ManagerValidationTest.php @@ -1,13 +1,12 @@ 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); @@ -33,7 +32,7 @@ private function getMockedLogger() 'alert', 'notice', 'write', 'log')) ->getMock(); - Di::set(Di::KEY_LOG, $logger); + ReflectiveTools::overrideLogger($logger); return $logger; } diff --git a/tests/Suite/InputValidation/TrackValidationTest.php b/tests/Suite/InputValidation/TrackValidationTest.php index e42bb603..9f66e586 100644 --- a/tests/Suite/InputValidation/TrackValidationTest.php +++ b/tests/Suite/InputValidation/TrackValidationTest.php @@ -1,7 +1,7 @@ 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); @@ -36,7 +35,7 @@ private function getMockedLogger() 'alert', 'notice', 'write', 'log')) ->getMock(); - Di::set(Di::KEY_LOG, $logger); + ReflectiveTools::overrideLogger($logger); return $logger; } diff --git a/tests/Suite/Matchers/MatchersTest.php b/tests/Suite/Matchers/MatchersTest.php index 79bcaf6d..b48990d1 100644 --- a/tests/Suite/Matchers/MatchersTest.php +++ b/tests/Suite/Matchers/MatchersTest.php @@ -19,7 +19,6 @@ class MatcherTest extends \PHPUnit\Framework\TestCase private function setupSplitApp() { - Di::set(Di::KEY_FACTORY_TRACKER, false); $parameters = array( 'scheme' => 'redis', 'host' => "localhost", diff --git a/tests/Suite/Redis/ReflectiveTools.php b/tests/Suite/Redis/ReflectiveTools.php index 879d17f4..d128e510 100644 --- a/tests/Suite/Redis/ReflectiveTools.php +++ b/tests/Suite/Redis/ReflectiveTools.php @@ -3,6 +3,7 @@ namespace SplitIO\Test\Suite\Redis; use ReflectionClass; +use SplitIO\Component\Common\Di; class ReflectiveTools { @@ -54,4 +55,22 @@ public static function clientFromCachePool(\SplitIO\Component\Cache\Pool $cacheP $reflectionClient->setAccessible(true); return $reflectionClient->getValue($adapter); } + + public static function overrideLogger($logger) + { + $di = Di::getInstance(); + $reflection = new \ReflectionClass('SplitIO\Component\Common\Di'); + $property = $reflection->getProperty('logger'); + $property->setAccessible(true); + $property->setValue($di, $logger); + } + + public static function resetIPAddress() + { + $di = Di::getInstance(); + $reflection = new \ReflectionClass('SplitIO\Component\Common\Di'); + $property = $reflection->getProperty('ipAddress'); + $property->setAccessible(true); + $property->setValue($di, ""); + } } diff --git a/tests/Suite/Sdk/ImpressionWrapperTest.php b/tests/Suite/Sdk/ImpressionWrapperTest.php index 10d45d83..e2e79e19 100644 --- a/tests/Suite/Sdk/ImpressionWrapperTest.php +++ b/tests/Suite/Sdk/ImpressionWrapperTest.php @@ -8,7 +8,7 @@ use SplitIO\Test\Suite\Sdk\Helpers\ListenerClient; use SplitIO\Test\Suite\Sdk\Helpers\ListenerClientWithException; use SplitIO\Test\Suite\Sdk\Helpers\ListenerClientWrong; -use SplitIO\Component\Common\Di; +use SplitIO\Test\Suite\Redis\ReflectiveTools; use SplitIO\Test\Utils; @@ -42,7 +42,6 @@ public function testSendDataToClient() private function getFactoryClient($sdkConfig) { - Di::set(Di::KEY_FACTORY_TRACKER, false); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array(); @@ -121,6 +120,7 @@ public function testClient() $impressionClient = new ListenerClient(); + ReflectiveTools::resetIPAddress(); $sdkConfig = array( 'log' => array('adapter' => 'stdout'), 'impressionListener' => $impressionClient, @@ -162,6 +162,7 @@ public function testClientWithEmptyIpAddress() $impressionClient3 = new ListenerClient(); + ReflectiveTools::resetIPAddress(); $sdkConfig = array( 'log' => array('adapter' => 'stdout'), 'impressionListener' => $impressionClient3, @@ -194,6 +195,7 @@ public function testClientWithEmptyStringIpAddress() $impressionClient4 = new ListenerClient(); + ReflectiveTools::resetIPAddress(); $sdkConfig = array( 'log' => array('adapter' => 'stdout'), 'impressionListener' => $impressionClient4, @@ -226,6 +228,7 @@ public function testClientErasingServer() $impressionClient4 = new ListenerClient(); + ReflectiveTools::resetIPAddress(); $sdkConfig = array( 'log' => array('adapter' => 'stdout'), 'impressionListener' => $impressionClient4, diff --git a/tests/Suite/Sdk/ImpressionsTest.php b/tests/Suite/Sdk/ImpressionsTest.php index 0fe731ab..9fb1c182 100644 --- a/tests/Suite/Sdk/ImpressionsTest.php +++ b/tests/Suite/Sdk/ImpressionsTest.php @@ -13,7 +13,6 @@ class ImpressionsTest extends \PHPUnit\Framework\TestCase { public function testImpressionsAreAdded() { - Di::set(Di::KEY_FACTORY_TRACKER, false); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); @@ -63,7 +62,6 @@ public function testImpressionsAreAdded() public function testExpirationOnlyOccursOnce() { - Di::set(Di::KEY_FACTORY_TRACKER, false); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); diff --git a/tests/Suite/Sdk/SdkClientTest.php b/tests/Suite/Sdk/SdkClientTest.php index 3cb64311..844c8e7d 100644 --- a/tests/Suite/Sdk/SdkClientTest.php +++ b/tests/Suite/Sdk/SdkClientTest.php @@ -2,7 +2,6 @@ namespace SplitIO\Test\Suite\Sdk; use \stdClass; -use SplitIO\Component\Common\Di; use SplitIO\Test\Suite\Redis\ReflectiveTools; use SplitIO\Component\Cache\ImpressionCache; use SplitIO\Component\Cache\EventsCache; @@ -20,7 +19,6 @@ class SdkClientTest extends \PHPUnit\Framework\TestCase { public function testLocalClient() { - Di::set(Di::KEY_FACTORY_TRACKER, false); $options['splitFile'] = dirname(dirname(__DIR__)).'/files/.splits'; $splitFactory = \SplitIO\Sdk::factory('localhost', $options); $splitSdk = $splitFactory->client(); @@ -37,7 +35,6 @@ public function testLocalClient() public function testLocalClientYAML() { - Di::set(Di::KEY_FACTORY_TRACKER, false); $options['splitFile'] = dirname(dirname(__DIR__)).'/files/splits.yml'; $splitFactory = \SplitIO\Sdk::factory('localhost', $options); $splitSdk = $splitFactory->client(); @@ -210,7 +207,6 @@ private function validateLastImpression( public function testClient() { - Di::set(Di::KEY_FACTORY_TRACKER, false); //Testing version string $this->assertTrue(is_string(\SplitIO\version())); @@ -441,7 +437,6 @@ public function testClient() */ public function testCustomLog() { - Di::set(Di::KEY_FACTORY_TRACKER, false); // create a log channel $log = $this ->getMockBuilder('Psr\Log\LoggerInterface') @@ -477,7 +472,6 @@ public function testCustomLog() public function testInvalidCacheAdapter() { - Di::set(Di::KEY_FACTORY_TRACKER, false); $this->expectException('\SplitIO\Exception\Exception'); $sdkConfig = array( @@ -491,8 +485,6 @@ public function testInvalidCacheAdapter() public function testCacheExceptionReturnsControl() { - Di::set(Di::KEY_FACTORY_TRACKER, false); - $log = $this ->getMockBuilder('Psr\Log\LoggerInterface') ->disableOriginalConstructor() @@ -531,7 +523,6 @@ public function testCacheExceptionReturnsControl() public function testGetTreatmentsWithDistinctFeatures() { - Di::set(Di::KEY_FACTORY_TRACKER, false); //Testing version string $this->assertTrue(is_string(\SplitIO\version())); @@ -568,14 +559,13 @@ public function testGetTreatmentsWithDistinctFeatures() public function testGetTreatmentsWithRepeteadedFeatures() { - Di::set(Di::KEY_FACTORY_TRACKER, false); - //Testing version string $this->assertTrue(is_string(\SplitIO\version())); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); + ReflectiveTools::resetIPAddress(); $sdkConfig = array( 'log' => array('adapter' => 'stdout'), 'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options), @@ -607,18 +597,17 @@ public function testGetTreatmentsWithRepeteadedFeatures() public function testGetTreatmentsWithRepeteadedAndNullFeatures() { - Di::set(Di::KEY_FACTORY_TRACKER, false); - Di::set('ipAddress', null); // unset ipaddress from previous test - //Testing version string $this->assertTrue(is_string(\SplitIO\version())); $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); + ReflectiveTools::resetIPAddress(); $sdkConfig = array( 'log' => array('adapter' => 'stdout'), - 'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options) + 'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options), + 'ipAddress' => '' ); //Initializing the SDK instance. @@ -691,6 +680,67 @@ public function testGetTreatmentsFetchesSplitsInOneCall() $client->getTreatments('key1', array('split1', 'split2', 'split3')); } + public function testMultipleInstantiationNotOverrideIP() + { + //Testing version string + $this->assertTrue(is_string(\SplitIO\version())); + + $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); + $options = array('prefix' => TEST_PREFIX); + + ReflectiveTools::resetIPAddress(); + $sdkConfig = array( + 'log' => array('adapter' => 'stdout'), + 'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options), + 'ipAddress' => '1.2.3.4' + ); + + //Initializing the SDK instance. + $splitFactory = \SplitIO\Sdk::factory('asdqwe123456', $sdkConfig); + $splitSdk = $splitFactory->client(); + + //Populating the cache. + Utils\Utils::addSplitsInCache(file_get_contents(__DIR__."/files/splitChanges.json")); + Utils\Utils::addSegmentsInCache(file_get_contents(__DIR__."/files/segmentEmployeesChanges.json")); + Utils\Utils::addSegmentsInCache(file_get_contents(__DIR__."/files/segmentHumanBeignsChanges.json")); + + $treatmentResult = $splitSdk->getTreatments('user1', array('sample_feature', 'invalid_feature', + 'sample_feature', 'sample_feature'), null); + + //Assertions + $this->assertEquals(2, count(array_keys($treatmentResult))); + + $this->assertEquals('on', $treatmentResult['sample_feature']); + $this->assertEquals('control', $treatmentResult['invalid_feature']); + + // Check impressions + $redisClient = ReflectiveTools::clientFromFactory($splitFactory); + $this->validateLastImpression($redisClient, 'sample_feature', 'user1', 'on', 'ip-1-2-3-4', '1.2.3.4'); + + $sdkConfig = array( + 'log' => array('adapter' => 'stdout'), + 'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options), + 'ipAddress' => '1.2.3.4.5' + ); + + //Initializing the SDK instance. + $splitFactory2 = \SplitIO\Sdk::factory('asdqwe123456', $sdkConfig); + $splitSdk2 = $splitFactory2->client(); + + $treatmentResult2 = $splitSdk2->getTreatments('user1', array('sample_feature', 'invalid_feature', + 'sample_feature', 'sample_feature'), null); + + //Assertions + $this->assertEquals(2, count(array_keys($treatmentResult2))); + + $this->assertEquals('on', $treatmentResult2['sample_feature']); + $this->assertEquals('control', $treatmentResult2['invalid_feature']); + + // Check impressions + $redisClient = ReflectiveTools::clientFromFactory($splitFactory2); + $this->validateLastImpression($redisClient, 'sample_feature', 'user1', 'on', 'ip-1-2-3-4', '1.2.3.4'); + } + public static function tearDownAfterClass(): void { Utils\Utils::cleanCache(); diff --git a/tests/Suite/Sdk/SdkReadOnlyTest.php b/tests/Suite/Sdk/SdkReadOnlyTest.php index 01885856..8153aae9 100644 --- a/tests/Suite/Sdk/SdkReadOnlyTest.php +++ b/tests/Suite/Sdk/SdkReadOnlyTest.php @@ -18,8 +18,6 @@ class SdkReadOnlyTest extends \PHPUnit\Framework\TestCase { public function testClient() { - Di::set(Di::KEY_FACTORY_TRACKER, false); - $parameters = array('scheme' => 'redis', 'host' => REDIS_HOST, 'port' => REDIS_PORT, 'timeout' => 881); $options = array('prefix' => TEST_PREFIX); $sdkConfig = array( @@ -54,7 +52,7 @@ public function testClient() $this->equalTo('The SPLIT definition for \'mockedPRedisInvalid\' has not been found') )); - Di::set(Di::KEY_LOG, $logger); + Di::setLogger($logger); $this->assertEquals('on', $splitSdk->getTreatment('valid', 'mockedPRedis')); $this->assertEquals('off', $splitSdk->getTreatment('invalid', 'mockedPRedis')); diff --git a/tests/Suite/TrafficAllocation/TrafficAllocationTest.php b/tests/Suite/TrafficAllocation/TrafficAllocationTest.php index ee5e7af3..78e0ce3a 100644 --- a/tests/Suite/TrafficAllocation/TrafficAllocationTest.php +++ b/tests/Suite/TrafficAllocation/TrafficAllocationTest.php @@ -4,6 +4,7 @@ use SplitIO\Component\Initialization\LoggerFactory; use SplitIO\Component\Common\ServiceProvider; use SplitIO\Grammar\Split; +use SplitIO\Component\Common\Di; class TrafficAllocationTest extends \PHPUnit\Framework\TestCase { @@ -20,7 +21,7 @@ public function testTrafficAllocation() ]); $logger = LoggerFactory::setupLogger(array('adapter' => 'stdout', 'level' => 'error')); - ServiceProvider::registerLogger($logger); + Di::setLogger($logger); $rawSplit = array( 'name' => 'test1', From 4daa3be5ee296cf6fa6f4d2066300b20128dfb2f Mon Sep 17 00:00:00 2001 From: Matias Melograno Date: Fri, 6 Jan 2023 17:54:46 -0300 Subject: [PATCH 3/3] removed unnecesary private methods --- src/SplitIO/Component/Common/Di.php | 76 ++++++++--------------------- src/SplitIO/Sdk.php | 10 ++-- 2 files changed, 24 insertions(+), 62 deletions(-) diff --git a/src/SplitIO/Component/Common/Di.php b/src/SplitIO/Component/Common/Di.php index 84f98300..10a353a2 100644 --- a/src/SplitIO/Component/Common/Di.php +++ b/src/SplitIO/Component/Common/Di.php @@ -9,7 +9,7 @@ */ class Di { - private \SplitIO\Component\Log\Logger|null $logger = null; + private \SplitIO\Component\Log\Logger $logger; private int $factoryTracker = 0; @@ -62,60 +62,13 @@ public function __wakeup() { } - private function __trackFactory() - { - $this->factoryTracker += 1; - return $this->factoryTracker; - } - - /** - * @param \SplitIO\Component\Log\Logger $logger - */ - private function __setLogger(Logger $logger) - { - if (is_null($this->logger)) { - $this->logger = $logger; - return; - } - $this->logger->debug("logger was set before, ignoring new instance provided"); - } - - /** - * @return null|\SplitIO\Component\Log\Logger - */ - private function __getLogger() - { - return $this->logger; - } - - /** - * @param string $ip - */ - private function __setIPAddress(string $ip) - { - if (empty($this->ipAddress)) { - $this->ipAddress = $ip; - return; - } - if (!(is_null($this->logger))) { - $this->logger->debug("IPAddress was set before, ignoring new instance provided"); - } - } - - /** - * @return null|string - */ - private function __getIPAddress() - { - return $this->ipAddress; - } - /** * @return int */ public static function trackFactory() { - return self::getInstance()->__trackFactory(); + self::getInstance()->factoryTracker += 1; + return self::getInstance()->factoryTracker; } /** @@ -123,15 +76,22 @@ public static function trackFactory() */ public static function setLogger(Logger $logger) { - self::getInstance()->__setLogger($logger); + if (!isset(self::getInstance()->logger)) { + self::getInstance()->logger = $logger; + return; + } + self::getInstance()->logger->debug("logger was set before, ignoring new instance provided"); } /** - * @return null|\SplitIO\Component\Log\Logger + * @return \SplitIO\Component\Log\Logger */ public static function getLogger() { - return self::getInstance()->__getLogger(); + if (!isset(self::getInstance()->logger)) { + throw new Exception("logger was not set yet"); + } + return self::getInstance()->logger; } /** @@ -139,14 +99,18 @@ public static function getLogger() */ public static function setIPAddress(string $ip) { - self::getInstance()->__setIPAddress($ip); + if (empty(self::getInstance()->ipAddress)) { + self::getInstance()->ipAddress = $ip; + return; + } + self::getInstance()->getLogger()->debug("IPAddress was set before, ignoring new instance provided"); } /** - * @return null|string + * @return string */ public static function getIPAddress() { - return self::getInstance()->__getIPAddress(); + return self::getInstance()->ipAddress; } } diff --git a/src/SplitIO/Sdk.php b/src/SplitIO/Sdk.php index 07f7da90..5b89dbab 100644 --- a/src/SplitIO/Sdk.php +++ b/src/SplitIO/Sdk.php @@ -31,17 +31,15 @@ public static function factory($apiKey = 'localhost', array $options = array()) //Adding API Key into args array. $options['apiKey'] = $apiKey; + //Tracking Factory Instantiation self::registerInstance(); - if ($apiKey == 'localhost') { - //Register Logger - self::registerLogger((isset($options['log'])) ? $options['log'] : array()); + //Register Logger + self::registerLogger((isset($options['log'])) ? $options['log'] : array()); + if ($apiKey == 'localhost') { return new LocalhostSplitFactory($options); } else { - //Register Logger - self::registerLogger((isset($options['log'])) ? $options['log'] : array()); - //Register Cache $cache = self::configureCache((isset($options['cache'])) ? $options['cache'] : array());