Skip to content

Commit 62223ae

Browse files
mikeproeng37mikeproeng37
authored
Merge branch 'master' into rashid/NC-deprecated-methods
2 parents cf765ad + 29db920 commit 62223ae

File tree

5 files changed

+243
-5
lines changed

5 files changed

+243
-5
lines changed

Diff for: ‎src/Optimizely/Optimizely.php

+58
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use Optimizely\Notification\NotificationCenter;
3737
use Optimizely\Notification\NotificationType;
3838
use Optimizely\UserProfile\UserProfileServiceInterface;
39+
use Optimizely\Utils\Errors;
3940
use Optimizely\Utils\Validator;
4041
use Optimizely\Utils\VariableTypeUtils;
4142

@@ -46,6 +47,10 @@
4647
*/
4748
class Optimizely
4849
{
50+
const USER_ID = 'User ID';
51+
const EVENT_KEY = 'Event Key';
52+
const EXPERIMENT_KEY ='Experiment Key';
53+
4954
/**
5055
* @var ProjectConfig
5156
*/
@@ -288,6 +293,16 @@ public function activate($experimentKey, $userId, $attributes = null)
288293
return null;
289294
}
290295

296+
if (!$this->validateInputs(
297+
[
298+
self::EXPERIMENT_KEY =>$experimentKey,
299+
self::USER_ID => $userId
300+
]
301+
)
302+
) {
303+
return null;
304+
}
305+
291306
$variationKey = $this->getVariation($experimentKey, $userId, $attributes);
292307
if (is_null($variationKey)) {
293308
$this->_logger->log(Logger::INFO, sprintf('Not activating user "%s".', $userId));
@@ -314,6 +329,16 @@ public function track($eventKey, $userId, $attributes = null, $eventTags = null)
314329
return;
315330
}
316331

332+
if (!$this->validateInputs(
333+
[
334+
self::EVENT_KEY =>$eventKey,
335+
self::USER_ID => $userId
336+
]
337+
)
338+
) {
339+
return null;
340+
}
341+
317342
if (!$this->validateUserInputs($attributes, $eventTags)) {
318343
return;
319344
}
@@ -401,6 +426,16 @@ public function getVariation($experimentKey, $userId, $attributes = null)
401426
return null;
402427
}
403428

429+
if (!$this->validateInputs(
430+
[
431+
self::EXPERIMENT_KEY =>$experimentKey,
432+
self::USER_ID => $userId
433+
]
434+
)
435+
) {
436+
return null;
437+
}
438+
404439
$experiment = $this->_config->getExperimentFromKey($experimentKey);
405440

406441
if (is_null($experiment->getKey())) {
@@ -728,4 +763,27 @@ public function getFeatureVariableString($featureFlagKey, $variableKey, $userId,
728763

729764
return $variableValue;
730765
}
766+
767+
/**
768+
* Calls Validator::validateNonEmptyString for each value in array
769+
* Logs for each invalid value
770+
*
771+
* @param array values to validate
772+
* @param logger
773+
*
774+
* @return bool True if all of the values are valid, False otherwise
775+
*/
776+
protected function validateInputs(array $values, $logLevel = Logger::ERROR)
777+
{
778+
$isValid = true;
779+
foreach ($values as $key => $value) {
780+
if (!Validator::validateNonEmptyString($value)) {
781+
$isValid = false;
782+
$message = sprintf(Errors::INVALID_FORMAT, $key);
783+
$this->_logger->log($logLevel, $message);
784+
}
785+
}
786+
787+
return $isValid;
788+
}
731789
}

Diff for: ‎src/Optimizely/Utils/Errors.php

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
/**
3+
* Copyright 2018, Optimizely
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
namespace Optimizely\Utils;
18+
19+
class Errors
20+
{
21+
const INVALID_FORMAT = 'Provided %s is in an invalid format.';
22+
}

Diff for: ‎src/Optimizely/Utils/Validator.php

+17-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22
/**
3-
* Copyright 2016-2017, Optimizely
3+
* Copyright 2016-2018, Optimizely
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -137,4 +137,20 @@ public static function isFeatureFlagValid($config, $featureFlag)
137137

138138
return true;
139139
}
140+
141+
/**
142+
* Checks if the provided value is a non-empty string
143+
*
144+
* @param $value The value to validate
145+
*
146+
* @return boolean True if $value is a non-empty string
147+
*/
148+
public static function validateNonEmptyString($value)
149+
{
150+
if (is_string($value) && $value!='') {
151+
return true;
152+
}
153+
154+
return false;
155+
}
140156
}

Diff for: ‎tests/OptimizelyTest.php

+138-4
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,29 @@ public function testActivateInvalidOptimizelyObject()
179179
$this->expectOutputRegex('/Datafile has invalid format. Failing "activate"./');
180180
}
181181

182+
public function testActivateCallsValidateInputs()
183+
{
184+
$optimizelyMock = $this->getMockBuilder(Optimizely::class)
185+
->setConstructorArgs(array($this->datafile))
186+
->setMethods(array('validateInputs'))
187+
->getMock();
188+
189+
$experimentKey = 'test_experiment';
190+
$userId = 'test_user';
191+
$inputArray = [
192+
Optimizely::EXPERIMENT_KEY => $experimentKey,
193+
Optimizely::USER_ID => $userId
194+
];
195+
196+
// assert that validateInputs gets called with exactly same keys
197+
$optimizelyMock->expects($this->once())
198+
->method('validateInputs')
199+
->with($inputArray)
200+
->willReturn(false);
201+
202+
$this->assertNull($optimizelyMock->activate($experimentKey, $userId));
203+
}
204+
182205
public function testActivateInvalidAttributes()
183206
{
184207
$this->loggerMock->expects($this->exactly(2))
@@ -460,6 +483,29 @@ public function testGetVariationInvalidOptimizelyObject()
460483
$this->expectOutputRegex('/Datafile has invalid format. Failing "getVariation"./');
461484
}
462485

486+
public function testGetVariationCallsValidateInputs()
487+
{
488+
$optimizelyMock = $this->getMockBuilder(Optimizely::class)
489+
->setConstructorArgs(array($this->datafile))
490+
->setMethods(array('validateInputs'))
491+
->getMock();
492+
493+
$experimentKey = 'test_experiment';
494+
$userId = 'test_user';
495+
$inputArray = [
496+
Optimizely::EXPERIMENT_KEY => $experimentKey,
497+
Optimizely::USER_ID => $userId
498+
];
499+
500+
// assert that validateInputs gets called with exactly same keys
501+
$optimizelyMock->expects($this->once())
502+
->method('validateInputs')
503+
->with($inputArray)
504+
->willReturn(false);
505+
506+
$this->assertNull($optimizelyMock->getVariation($experimentKey, $userId));
507+
}
508+
463509
public function testGetVariationInvalidAttributes()
464510
{
465511
$this->loggerMock->expects($this->once())
@@ -679,6 +725,29 @@ public function testTrackInvalidOptimizelyObject()
679725
$this->expectOutputRegex('/Datafile has invalid format. Failing "track"./');
680726
}
681727

728+
public function testTrackCallsValidateInputs()
729+
{
730+
$optimizelyMock = $this->getMockBuilder(Optimizely::class)
731+
->setConstructorArgs(array($this->datafile))
732+
->setMethods(array('validateInputs'))
733+
->getMock();
734+
735+
$eventKey = 'test_event';
736+
$userId = 'test_user';
737+
$inputArray = [
738+
Optimizely::EVENT_KEY => $eventKey,
739+
Optimizely::USER_ID => $userId
740+
];
741+
742+
// assert that validateInputs gets called with exactly same keys
743+
$optimizelyMock->expects($this->once())
744+
->method('validateInputs')
745+
->with($inputArray)
746+
->willReturn(false);
747+
748+
$this->assertNull($optimizelyMock->track($eventKey, $userId));
749+
}
750+
682751
public function testTrackInvalidAttributes()
683752
{
684753
$this->loggerMock->expects($this->once())
@@ -1632,14 +1701,17 @@ public function testSetForcedVariationWithInvalidUserIDs()
16321701
'location' => 'San Francisco'
16331702
];
16341703

1635-
// null user ID --> set should fail, normal bucketing [TODO (Alda): getVariation on a null userID should return null]
1704+
// null user ID should fail setForcedVariation. getVariation on a null userID should return null
16361705
$this->assertFalse($optlyObject->setForcedVariation('test_experiment', null, 'bad_variation'), 'Set variation for null user ID should have failed.');
1706+
16371707
$variationKey = $optlyObject->getVariation('test_experiment', null, $userAttributes);
1638-
$this->assertEquals('variation', $variationKey, sprintf('Invalid variation "%s" for null user ID.', $variationKey));
1639-
// empty string user ID --> set should fail, normal bucketing [TODO (Alda): getVariation on an empty userID should return null]
1708+
$this->assertNull($variationKey);
1709+
1710+
// empty string user ID should fail setForcedVariation. getVariation on an empty userID should return null
16401711
$this->assertFalse($optlyObject->setForcedVariation('test_experiment', '', 'bad_variation'), 'Set variation for empty string user ID should have failed.');
1712+
16411713
$variationKey = $optlyObject->getVariation('test_experiment', '', $userAttributes);
1642-
$this->assertEquals('variation', $variationKey, sprintf('Invalid variation "%s" for empty string user ID.', $variationKey));
1714+
$this->assertNull($variationKey);
16431715
}
16441716

16451717
public function testSetForcedVariationWithInvalidExperimentKeys()
@@ -2850,4 +2922,66 @@ public function testSendImpressionEventWithAttributes()
28502922

28512923
$optlyObject->sendImpressionEvent('test_experiment', 'control', 'test_user', $userAttributes);
28522924
}
2925+
2926+
/*
2927+
* test ValidateInputs method validates and logs for different and multiple keys
2928+
*/
2929+
public function testValidateInputs()
2930+
{
2931+
$optlyObject = new OptimizelyTester($this->datafile, null, $this->loggerMock);
2932+
2933+
$INVALID_USER_ID_LOG = 'Provided User ID is in an invalid format.';
2934+
$INVALID_EVENT_KEY_LOG = 'Provided Event Key is in an invalid format.';
2935+
$INVALID_RANDOM_KEY_LOG = 'Provided ABCD is in an invalid format.';
2936+
2937+
$this->loggerMock->expects($this->at(0))
2938+
->method('log')
2939+
->with(Logger::ERROR, $INVALID_USER_ID_LOG);
2940+
$this->assertFalse($optlyObject->validateInputs([Optimizely::USER_ID => null]));
2941+
2942+
$this->loggerMock->expects($this->at(0))
2943+
->method('log')
2944+
->with(Logger::ERROR, $INVALID_RANDOM_KEY_LOG);
2945+
$this->assertFalse($optlyObject->validateInputs(['ABCD' => null]));
2946+
2947+
// Verify that multiple messages are logged.
2948+
$this->loggerMock->expects($this->at(0))
2949+
->method('log')
2950+
->with(Logger::ERROR, $INVALID_EVENT_KEY_LOG);
2951+
2952+
$this->loggerMock->expects($this->at(1))
2953+
->method('log')
2954+
->with(Logger::ERROR, $INVALID_USER_ID_LOG);
2955+
$this->assertFalse($optlyObject->validateInputs([Optimizely::EVENT_KEY => null, Optimizely::USER_ID => null]));
2956+
2957+
// Verify that logger level is taken into account
2958+
$this->loggerMock->expects($this->at(0))
2959+
->method('log')
2960+
->with(Logger::INFO, $INVALID_RANDOM_KEY_LOG);
2961+
$this->assertFalse($optlyObject->validateInputs(['ABCD' => null], Logger::INFO));
2962+
2963+
// Verify that it returns true and nothing is logged if valid values given
2964+
$this->loggerMock->expects($this->never())
2965+
->method('log');
2966+
$this->assertTrue($optlyObject->validateInputs([Optimizely::EVENT_KEY => 'test_event', Optimizely::USER_ID => 'test_user']));
2967+
}
2968+
2969+
/*
2970+
* test ValidateInputs method only returns true for non-empty string
2971+
*/
2972+
public function testValidateInputsWithDifferentValues()
2973+
{
2974+
$optlyObject = new OptimizelyTester($this->datafile);
2975+
2976+
$this->assertTrue($optlyObject->validateInputs(['key' => '0']));
2977+
$this->assertTrue($optlyObject->validateInputs(['key' => 'test_user']));
2978+
2979+
$this->assertFalse($optlyObject->validateInputs(['key' => '']));
2980+
$this->assertFalse($optlyObject->validateInputs(['key' => null]));
2981+
$this->assertFalse($optlyObject->validateInputs(['key' => false]));
2982+
$this->assertFalse($optlyObject->validateInputs(['key' => true]));
2983+
$this->assertFalse($optlyObject->validateInputs(['key' => 2]));
2984+
$this->assertFalse($optlyObject->validateInputs(['key' => 2.0]));
2985+
$this->assertFalse($optlyObject->validateInputs(['key' => array()]));
2986+
}
28532987
}

Diff for: ‎tests/TestData.php

+8
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
namespace Optimizely\Tests;
1818

1919
use Exception;
20+
21+
use Monolog\Logger;
22+
2023
use Optimizely\Bucketer;
2124
use Optimizely\Event\Dispatcher\EventDispatcherInterface;
2225
use Optimizely\Event\LogEvent;
@@ -797,6 +800,11 @@ public function sendImpressionEvent($experimentKey, $variationKey, $userId, $att
797800
{
798801
parent::sendImpressionEvent($experimentKey, $variationKey, $userId, $attributes);
799802
}
803+
804+
public function validateInputs(array $values, $logLevel = Logger::ERROR)
805+
{
806+
return parent::validateInputs($values, $logLevel);
807+
}
800808
}
801809

802810
class FireNotificationTester

0 commit comments

Comments
 (0)