Prepare for 2.0.1 (#116)

Author: aliabbasrizvi

.travis.yml

@@ -6,6 +6,8 @@ php:
   - '7.1'
 before_install:
 - composer install --dev
+addons:
+  srcclr: true
 script:
 - mkdir -p build/logs
 - ./vendor/bin/phpunit --coverage-clover build/logs/clover.xml

CHANGELOG.md

@@ -1,3 +1,8 @@
+# 2.0.1
+June 19th, 2018
+
+- Fix: send impression event for Feature Test when Feature is disabled ([#114](https://github.com/optimizely/php-sdk/pull/114)).
+
 ## 2.0.0
 April 12th, 2018
 

src/Optimizely/Bucketer.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * Copyright 2016-2017, Optimizely
+ * Copyright 2016-2018, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -127,21 +127,21 @@ private function findBucket($bucketingId, $userId, $parentId, $trafficAllocation
     public function bucket(ProjectConfig $config, Experiment $experiment, $bucketingId, $userId)
     {
         if (is_null($experiment->getKey())) {
-            return new Variation();
+            return null;
         }
 
         // Determine if experiment is in a mutually exclusive group.
         if ($experiment->isInMutexGroup()) {
             $group = $config->getGroup($experiment->getGroupId());
 
             if (is_null($group->getId())) {
-                return new Variation();
+                return null;
             }
 
             $userExperimentId = $this->findBucket($bucketingId, $userId, $group->getId(), $group->getTrafficAllocation());
             if (empty($userExperimentId)) {
                 $this->_logger->log(Logger::INFO, sprintf('User "%s" is in no experiment.', $userId));
-                return new Variation();
+                return null;
             }
 
             if ($userExperimentId != $experiment->getId()) {
@@ -154,7 +154,7 @@ public function bucket(ProjectConfig $config, Experiment $experiment, $bucketing
                         $experiment->getGroupId()
                     )
                 );
-                return new Variation();
+                return null;
             }
 
             $this->_logger->log(
@@ -186,6 +186,6 @@ public function bucket(ProjectConfig $config, Experiment $experiment, $bucketing
         }
 
         $this->_logger->log(Logger::INFO, sprintf('User "%s" is in no variation.', $userId));
-        return new Variation();
+        return null;
     }
 }

src/Optimizely/Event/Builder/EventBuilder.php

@@ -37,7 +37,7 @@ class EventBuilder
     /**
      * @const string Version of the Optimizely PHP SDK.
      */
-    const SDK_VERSION = '2.0.0';
+    const SDK_VERSION = '2.0.1';
 
     /**
      * @var string URL to send event to.

src/Optimizely/Optimizely.php

@@ -24,9 +24,7 @@
 use Optimizely\DecisionService\DecisionService;
 use Optimizely\DecisionService\FeatureDecision;
 use Optimizely\Entity\Experiment;
-use Optimizely\Entity\FeatureFlag;
 use Optimizely\Entity\FeatureVariable;
-use Optimizely\Entity\Rollout;
 use Optimizely\Logger\DefaultLogger;
 use Optimizely\ErrorHandler\ErrorHandlerInterface;
 use Optimizely\ErrorHandler\NoOpErrorHandler;
@@ -38,7 +36,6 @@
 use Optimizely\Notification\NotificationCenter;
 use Optimizely\Notification\NotificationType;
 use Optimizely\UserProfile\UserProfileServiceInterface;
-use Optimizely\Utils\EventTagUtils;
 use Optimizely\Utils\Validator;
 use Optimizely\Utils\VariableTypeUtils;
 
@@ -502,20 +499,19 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null)
         $experiment = $decision->getExperiment();
         $variation = $decision->getVariation();
 
-        if (is_null($variation) || !$variation->getFeatureEnabled()) {
-            $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is not enabled for user '{$userId}'.");
-            return false;
-        }
-
         if ($decision->getSource() == FeatureDecision::DECISION_SOURCE_EXPERIMENT) {
             $this->sendImpressionEvent($experiment->getKey(), $variation->getKey(), $userId, $attributes);
         } else {
             $this->_logger->log(Logger::INFO, "The user '{$userId}' is not being experimented on Feature Flag '{$featureFlagKey}'.");
         }
 
-        $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is enabled for user '{$userId}'.");
+        if ($variation->getFeatureEnabled()) {
+            $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is enabled for user '{$userId}'.");
+            return true;
+        }
 
-        return true;
+        $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is not enabled for user '{$userId}'.");
+        return false;
     }
 
     /**

tests/BucketerTest.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * Copyright 2016-2017, Optimizely
+ * Copyright 2016-2018, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -107,8 +107,7 @@ public function testBucketValidExperimentNotInGroup()
             ->method('log')
             ->with(Logger::INFO, 'User "testUserId" is in no variation.');
 
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket(
                 $this->config,
                 $this->config->getExperimentFromKey('test_experiment'),
@@ -167,8 +166,7 @@ public function testBucketValidExperimentNotInGroup()
             ->method('log')
             ->with(Logger::INFO, 'User "testUserId" is in no variation.');
 
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket(
                 $this->config,
                 $this->config->getExperimentFromKey('test_experiment'),
@@ -271,8 +269,7 @@ public function testBucketValidExperimentInGroup()
             ->method('log')
             ->with(Logger::INFO, 'User "testUserId" is not in experiment group_experiment_1 of group 7722400015.');
 
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket(
                 $this->config,
                 $this->config->getExperimentFromKey('group_experiment_1'),
@@ -290,8 +287,7 @@ public function testBucketValidExperimentInGroup()
             ->method('log')
             ->with(Logger::INFO, 'User "testUserId" is in no experiment.');
 
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket(
                 $this->config,
                 $this->config->getExperimentFromKey('group_experiment_1'),
@@ -308,8 +304,7 @@ public function testBucketValidExperimentInGroup()
         $this->loggerMock->expects($this->at(1))
             ->method('log')
             ->with(Logger::INFO, 'User "testUserId" is in no experiment.');
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket(
                 $this->config,
                 $this->config->getExperimentFromKey('group_experiment_1'),
@@ -325,8 +320,7 @@ public function testBucketInvalidExperiment()
         $this->loggerMock->expects($this->never())
             ->method('log');
 
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket($this->config, new Experiment(), $this->testBucketingIdControl, $this->testUserId)
         );
     }
@@ -356,8 +350,7 @@ public function testBucketVariationInvalidExperimentsWithBucketingId()
         $bucketer = new TestBucketer($this->loggerMock);
         $bucketer->setBucketValues([1000, 3000, 7000, 9000]);
 
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket(
                 $this->config,
                 $this->config->getExperimentFromKey('invalid_experiment'),
@@ -430,8 +423,7 @@ public function testBucketWithRolloutRule()
             )
         );
 
-        $this->assertEquals(
-            new Variation(),
+        $this->assertNull(
             $bucketer->bucket(
                 $this->config,
                 $rollout_rule,

tests/EventTests/EventBuilderTest.php

@@ -68,7 +68,7 @@ public function setUp()
                 ]],
                 'revision' => '15',
                 'client_name' => 'php-sdk',
-                'client_version' => '2.0.0',
+                'client_version' => '2.0.1',
                 'anonymize_ip'=> false,
             ];
         $this->expectedEventHttpVerb = 'POST';

tests/OptimizelyTest.php

@@ -2087,8 +2087,9 @@ public function testIsFeatureEnabledGivenFeatureExperimentAndFeatureEnabledIsFal
             ->method('getVariationForFeature')
             ->will($this->returnValue($expected_decision));
 
-        $optimizelyMock->expects($this->never())
-            ->method('sendImpressionEvent');
+        $optimizelyMock->expects($this->exactly(1))
+            ->method('sendImpressionEvent')
+            ->with('test_experiment_double_feature', 'variation', 'user_id', []);
 
         $this->loggerMock->expects($this->at(0))
             ->method('log')
@@ -2188,7 +2189,12 @@ public function testIsFeatureEnabledGivenFeatureRolloutAndFeatureEnabledIsFalse(
         $optimizelyMock->expects($this->never())
             ->method('sendImpressionEvent');
 
+        // confirm log messages seen
         $this->loggerMock->expects($this->at(0))
+            ->method('log')
+            ->with(Logger::INFO, "The user 'user_id' is not being experimented on Feature Flag 'boolean_single_variable_feature'.");
+
+        $this->loggerMock->expects($this->at(1))
             ->method('log')
             ->with(Logger::INFO, "Feature Flag 'boolean_single_variable_feature' is not enabled for user 'user_id'.");