feat: Add init with sdk key #189
Conversation
src/Optimizely/Optimizely.php
Outdated
| @@ -122,6 +124,7 @@ public function __construct( | |||
| ErrorHandlerInterface $errorHandler = null, | |||
| $skipJsonValidation = false, | |||
| UserProfileServiceInterface $userProfileService = null, | |||
| $sdkKey = null, | |||
There was a problem hiding this comment.
Wouldn't this break previous versions since the order changed? Shouldn't this become the last parameter?
Also ok if we file this as a breaking change.
There was a problem hiding this comment.
I just tried to be consistent with python and ruby sdks. I would recommend to go for a breaking change and make sdkKey the first param of constructor. What do you suggest?
| @@ -96,7 +97,7 @@ class Optimizely | |||
| /** | |||
| * @var ProjectConfigManagerInterface | |||
| */ | |||
| private $_projectConfigManager; | |||
| public $configManager; | |||
There was a problem hiding this comment.
Why does this have to be public now?
There was a problem hiding this comment.
HTTPProjectConfigManager exposes a public method fetch, which the user can use to get the latest datafile from cdn. Previously, we were expecting the user to create it's own config manager and pass. Now, we will be creating httpconfigmanager given the sdk key so the user must be able to access it to fetch the latest datafile.
|
@aliabbasrizvi and @msohailhussain Added a default instance method in OptimizelyFactory. |
|
|
||
| // client hasn't been mocked yet. Hence, config manager should return config of hardcoded | ||
| // datafile. | ||
| $this->assertEquals('15', $optimizelyClient->configManager->getConfig()->getRevision()); |
There was a problem hiding this comment.
We should mock it anyway, to prevent it from ever making a real HTTP request. Even if it won't because of hard coded datafile.
There was a problem hiding this comment.
Given the way PHP Unit allows to use doubles, I don't think we can mock the first http call that a config manager makes when it's initialized inside Optly client. I can only mock and set a double, after the Optimizely client has been created and in that case, it has already made a real HTTP request.
Summary: - This adds sdk key param to Optimizely constructor. The user can now create an Optimizely instance only using sdk key. Previously he was required to create a HTTPProjectConfigManager, and pass it as a config manager in the constructor. - The sdk key param in constructor params sequence has been set as last item as to not break. - Made configManager inside Optimizely class **public** so that the user is able to call fetch() which is a public method defined over HTTPProjectConfigManager. - Adds a factory method to create a client with sdk key and datafile. **todo** Update readme accordingly. Test plan: - Added unit tests to verify init with sdk key. Issues: - OASIS-5770
Summary
todo
Update readme accordingly.
Test plan
Issues