Checking for attribute validity (#7)

Author: aliabbasrizvi

src/Optimizely/Optimizely.php

@@ -16,7 +16,6 @@
  */
 namespace Optimizely;
 
-use Doctrine\Instantiator\Exception\InvalidArgumentException;
 use Exception;
 use Optimizely\Exceptions\InvalidAttributeException;
 use Throwable;

src/Optimizely/Utils/Validator.php

@@ -45,8 +45,7 @@ public static function validateJsonSchema($datafile)
      */
     public static function areAttributesValid($attributes)
     {
-        //TODO(ali): Implement me
-        return true;
+        return is_array($attributes) && count(array_filter(array_keys($attributes), 'is_int')) == 0;
     }
 
     /**

tests/OptimizelyTest.php

@@ -21,6 +21,7 @@
 use Optimizely\Bucketer;
 use Optimizely\ErrorHandler\NoOpErrorHandler;
 use Optimizely\Event\LogEvent;
+use Optimizely\Exceptions\InvalidAttributeException;
 use Optimizely\Logger\NoOpLogger;
 use Optimizely\ProjectConfig;
 use TypeError;
@@ -250,6 +251,32 @@ public function testActivateInvalidOptimizelyObject()
         $this->expectOutputRegex('/Datafile has invalid format. Failing "activate"./');
     }
 
+    public function testActivateInvalidAttributes()
+    {
+        $this->loggerMock->expects($this->exactly(2))
+            ->method('log');
+        $this->loggerMock->expects($this->at(0))
+            ->method('log')
+            ->with(Logger::ERROR, 'Provided attributes are in an invalid format.');
+        $this->loggerMock->expects($this->at(1))
+            ->method('log')
+            ->with(Logger::INFO, 'Not activating user "test_user".');
+
+        $errorHandlerMock = $this->getMockBuilder(NoOpErrorHandler::class)
+            ->setMethods(array('handleError'))
+            ->getMock();
+        $errorHandlerMock->expects($this->once())
+            ->method('handleError')
+            ->with(new InvalidAttributeException('Provided attributes are in an invalid format.'));
+
+        $optlyObject = new Optimizely(
+            $this->datafile, new ValidEventDispatcher(), $this->loggerMock, $errorHandlerMock
+        );
+
+        // Call activate
+        $this->assertNull($optlyObject->activate('test_experiment', 'test_user', 42));
+    }
+
     public function testActivateNoAudienceNoAttributes()
     {
         $this->eventBuilderMock->expects($this->once())
@@ -400,6 +427,27 @@ public function testGetVariationInvalidOptimizelyObject()
         $this->expectOutputRegex('/Datafile has invalid format. Failing "getVariation"./');
     }
 
+    public function testGetVariationInvalidAttributes()
+    {
+        $this->loggerMock->expects($this->once())
+            ->method('log')
+            ->with(Logger::ERROR, 'Provided attributes are in an invalid format.');
+
+        $errorHandlerMock = $this->getMockBuilder(NoOpErrorHandler::class)
+            ->setMethods(array('handleError'))
+            ->getMock();
+        $errorHandlerMock->expects($this->once())
+            ->method('handleError')
+            ->with(new InvalidAttributeException('Provided attributes are in an invalid format.'));
+
+        $optlyObject = new Optimizely(
+            $this->datafile, new ValidEventDispatcher(), $this->loggerMock, $errorHandlerMock
+        );
+
+        // Call activate
+        $this->assertNull($optlyObject->getVariation('test_experiment', 'test_user', 42));
+    }
+
     public function testGetVariationAudienceMatch()
     {
         $this->loggerMock->expects($this->exactly(2))
@@ -447,6 +495,27 @@ public function testTrackInvalidOptimizelyObject()
         $this->expectOutputRegex('/Datafile has invalid format. Failing "track"./');
     }
 
+    public function testTrackInvalidAttributes()
+    {
+        $this->loggerMock->expects($this->once())
+            ->method('log')
+            ->with(Logger::ERROR, 'Provided attributes are in an invalid format.');
+
+        $errorHandlerMock = $this->getMockBuilder(NoOpErrorHandler::class)
+            ->setMethods(array('handleError'))
+            ->getMock();
+        $errorHandlerMock->expects($this->once())
+            ->method('handleError')
+            ->with(new InvalidAttributeException('Provided attributes are in an invalid format.'));
+
+        $optlyObject = new Optimizely(
+            $this->datafile, new ValidEventDispatcher(), $this->loggerMock, $errorHandlerMock
+        );
+
+        // Call activate
+        $this->assertNull($optlyObject->track('purchase', 'test_user', 42));
+    }
+
     public function testTrackNoAttributesNoEventValue()
     {
         $this->eventBuilderMock->expects($this->once())

tests/UtilsTests/ValidatorTest.php

@@ -42,9 +42,34 @@ public function testValidateJsonSchemaNoJsonContent()
         $this->assertFalse(Validator::validateJsonSchema($invalidDatafile));
     }
 
-    public function testAreAttributesValidReturnsTrue()
+    public function testAreAttributesValidValidAttributes()
     {
-        $this->assertTrue(Validator::areAttributesValid('some attributes here'));
+        // Empty attributes
+        $this->assertTrue(Validator::areAttributesValid([]));
+
+        // Valid attributes
+        $this->assertTrue(Validator::areAttributesValid([
+            'location' => 'San Francisco',
+            'browser' => 'Firefox'
+        ]));
+    }
+
+    public function testAreAttributesValidInvalidAttributes()
+    {
+        // String as attributes
+        $this->assertFalse(Validator::areAttributesValid('Invalid string attributes.'));
+
+        // Integer as attributes
+        $this->assertFalse(Validator::areAttributesValid(42));
+
+        // Boolean as attributes
+        $this->assertFalse(Validator::areAttributesValid(true));
+
+        // Sequential array as attributes
+        $this->assertFalse(Validator::areAttributesValid([0, 1, 2, 42]));
+
+        // Mixed array as attributes
+        $this->assertFalse(Validator::areAttributesValid([0, 1, 2, 42, 'abc' => 'def']));
     }
 
     public function testIsUserInExperimentNoAudienceUsedInExperiment()