Skip to content

Commit

Permalink
Merge pull request #210 from splitio/redisoptions
Browse files Browse the repository at this point in the history
forwarding predis options directly without wrapping it
  • Loading branch information
mmelograno authored Feb 1, 2024
2 parents 74cbd32 + 333457d commit 151b796
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 433 deletions.
16 changes: 6 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
name: ci
on:
pull_request:
branches:
- develop
- master
push:
branches:
- develop
- master
branches-ignore:
- none
pull_request:
branches-ignore:
- none

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
Expand All @@ -31,7 +29,7 @@ jobs:
- '8.2'
steps:
- name: Checkout code
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
fetch-depth: 0

Expand Down Expand Up @@ -66,7 +64,6 @@ jobs:
projectBaseDir: .
args: >
-Dsonar.host.url=${{ secrets.SONARQUBE_HOST }}
-Dsonar.projectVersion=${{ env.VERSION }}
- name: SonarQube Scan (Pull Request)
if: matrix.version == '8.2' && github.event_name == 'pull_request'
Expand All @@ -78,7 +75,6 @@ jobs:
projectBaseDir: .
args: >
-Dsonar.host.url=${{ secrets.SONARQUBE_HOST }}
-Dsonar.projectVersion=${{ env.VERSION }}
-Dsonar.pullrequest.key=${{ github.event.pull_request.number }}
-Dsonar.pullrequest.branch=${{ github.event.pull_request.head.ref }}
-Dsonar.pullrequest.base=${{ github.event.pull_request.base.ref }}
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
},

"require-dev": {
"phpunit/phpunit": "^9.0.0",
"phpunit/phpunit": "^9.6",
"squizlabs/php_codesniffer": "3.*",
"rogervila/php-sonarqube-scanner": "1.1.0",
"mockery/mockery": "^1.5"
Expand Down
122 changes: 18 additions & 104 deletions src/SplitIO/Component/Cache/Storage/Adapter/PRedis.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use SplitIO\Component\Cache\Storage\Exception\AdapterException;
use SplitIO\Component\Utils as SplitIOUtils;
use SplitIO\Component\Common\Context;

/**
* Class PRedis
Expand All @@ -15,6 +14,9 @@ class PRedis implements CacheStorageAdapterInterface
/** @var \Predis\Client|null */
private $client = null;

/** @var string */
private $prefix = "";

/**
* @param array $options
* @throws AdapterException
Expand All @@ -26,7 +28,11 @@ public function __construct(array $options)
}
$_redisConfig = $this->getRedisConfiguration($options);

$this->client = new \Predis\Client($_redisConfig['redis'], $_redisConfig['options']);
$this->client = new \Predis\Client($_redisConfig['parameters'], $_redisConfig['options']);

if (isset($_redisConfig['options']['prefix'])) {
$this->prefix = $_redisConfig['options']['prefix'];
}
}

/**
Expand All @@ -48,24 +54,6 @@ private function isValidConfigArray($nodes, $type)
return null;
}

/**
* @param array $sentinels
* @param array $options
* @return bool
* @throws AdapterException
*/
private function isValidSentinelConfig($sentinels, $options)
{
$msg = $this->isValidConfigArray($sentinels, 'sentinel');
if (!is_null($msg)) {
throw new AdapterException($msg);
}
if (!isset($options['service'])) {
throw new AdapterException('Master name is required in replication mode for sentinel.');
}
return true;
}

private function validateKeyHashTag($keyHashTag)
{
if (!is_string($keyHashTag)) {
Expand Down Expand Up @@ -121,83 +109,32 @@ function ($value) {
if (empty($filteredArray)) {
throw new AdapterException('keyHashTags size is zero after filtering valid elements.');
}
return $selected = $filteredArray[array_rand($filteredArray, 1)];
return $filteredArray[array_rand($filteredArray, 1)];
}


/**
* @param array $clusters
* @return bool
* @throws AdapterException
*/
private function isValidClusterConfig($clusters)
{
$msg = $this->isValidConfigArray($clusters, 'clusterNode');
if (!is_null($msg)) {
throw new AdapterException($msg);
}
return true;
}

/**
* @param mixed $options
* @return array
* @throws AdapterException
*/
private function getRedisConfiguration($options)
{
$redisConfigutation = array(
'redis' => null,
'options' => null
'parameters' => (isset($options['parameters'])) ? $options['parameters'] : null,
'options' => null,
);

$parameters = (isset($options['parameters'])) ? $options['parameters'] : null;
$sentinels = (isset($options['sentinels'])) ? $options['sentinels'] : null;
$clusters = (isset($options['clusterNodes'])) ? $options['clusterNodes'] : null;
$_options = (isset($options['options'])) ? $options['options'] : null;

if (isset($_options['distributedStrategy']) && isset($parameters['tls'])) {
throw new AdapterException("SSL/TLS cannot be used together with sentinel/cluster yet");
}

if ($_options && isset($_options['prefix'])) {
$_options['prefix'] = self::normalizePrefix($_options['prefix']);
}

if (isset($parameters)) {
$redisConfigutation['redis'] = $parameters;
} else {
// @TODO remove this statement when replication will be deprecated
if (isset($_options['replication'])) {
Context::getLogger()->warning("'replication' option was deprecated please use 'distributedStrategy'");
if (!isset($_options['distributedStrategy'])) {
$_options['distributedStrategy'] = $_options['replication'];
}
}
if (isset($_options['distributedStrategy'])) {
switch ($_options['distributedStrategy']) {
case 'cluster':
if ($this->isValidClusterConfig($clusters)) {
$keyHashTag = $this->selectKeyHashTag($_options);
$_options['cluster'] = 'redis';
$redisConfigutation['redis'] = $clusters;
$prefix = isset($_options['prefix']) ? $_options['prefix'] : '';
$_options['prefix'] = $keyHashTag . $prefix;
}
break;
case 'sentinel':
if ($this->isValidSentinelConfig($sentinels, $_options)) {
$_options['replication'] = 'sentinel';
$redisConfigutation['redis'] = $sentinels;
}
break;
default:
throw new AdapterException("Wrong configuration of redis 'distributedStrategy'.");
}
} else {
throw new AdapterException("Wrong configuration of redis.");
}
if (isset($_options['cluster'])) {
$keyHashTag = $this->selectKeyHashTag($_options);
$prefix = isset($_options['prefix']) ? $_options['prefix'] : '';
$_options['prefix'] = $keyHashTag . $prefix;
}

$redisConfigutation['options'] = $_options;
return $redisConfigutation;
}
Expand Down Expand Up @@ -249,31 +186,8 @@ public function isOnList($key, $value)

public function getKeys($pattern = '*')
{
$prefix = null;
if ($this->client->getOptions()->__isset("prefix")) {
$prefix = $this->client->getOptions()->__get("prefix")->getPrefix();
}

if ($this->client->getOptions()->__isset("distributedStrategy") &&
$this->client->getOptions()->__get("distributedStrategy") == "cluster") {
$keys = array();
foreach ($this->client as $nodeClient) {
$nodeClientKeys = $nodeClient->keys($pattern);
$keys = array_merge($keys, $nodeClientKeys);
}
} else {
$keys = $this->client->keys($pattern);
}
if ($prefix) {
if (is_array($keys)) {
foreach ($keys as $index => $key) {
$keys[$index] = str_replace($prefix, '', $key);
}
} else {
$keys = str_replace($prefix, '', $keys);
}
}
return $keys;
$keys = $this->client->keys($pattern);
return str_replace($this->prefix, '', $keys);
}

private static function normalizePrefix($prefix)
Expand Down
4 changes: 0 additions & 4 deletions src/SplitIO/Sdk.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ private static function configureCache(array $options)
} elseif ($cacheAdapter == 'predis') {
$_options['options'] = isset($options['options']) ? $options['options'] : null;
$_options['parameters'] = isset($options['parameters']) ? $options['parameters'] : null;
$_options['sentinels'] = isset($options['sentinels']) ? $options['sentinels'] : null;
$_options['clusterNodes'] = isset($options['clusterNodes']) ? $options['clusterNodes'] : null;
$_options['distributedStrategy'] = isset($options['distributedStrategy'])
? $options['distributedStrategy'] : null;
} else {
throw new Exception("A valid cache system is required. Given: $cacheAdapter");
}
Expand Down
5 changes: 3 additions & 2 deletions src/SplitIO/Sdk/ClientInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,9 @@ public function isTreatment($key, $featureFlagName, $treatment);
* @param $key
* @param $trafficType
* @param $eventType
* @param null $value
* @param $value
* @param $properties
* @return boolean
*/
public function track($key, $trafficType, $eventType, $value = null);
public function track($key, $trafficType, $eventType, $value = null, $properties = null);
}
18 changes: 0 additions & 18 deletions src/SplitIO/Sdk/Factory/SplitFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ public function __construct($sdkKey, Pool $cache, array $options = array())
$this->options = $options;
$this->cache = $cache;

//Block until ready
$this->doBUR();

$eventCache = new EventsCache($cache);
$impressionCache = new ImpressionCache($cache);
$segmentCache = new SegmentCache($cache);
Expand All @@ -63,21 +60,6 @@ public function __construct($sdkKey, Pool $cache, array $options = array())
$this->manager = new SplitManager($splitCache);
}

private function doBUR()
{
/*
Deprecated
$ready = (isset($this->options['ready']) && $this->options['ready'] > 0) ? $this->options['ready'] : null;
//Block Until Ready
if ($ready) {
if (!$this->blockUntilReady($ready)) {
throw new TimeOutException("Cache data is not ready yet");
}
}
*/
}

/**
* @return \SplitIO\Sdk\ClientInterface
*/
Expand Down
2 changes: 1 addition & 1 deletion src/SplitIO/Sdk/LocalhostClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ public function isTreatment($key, $featureFlagName, $treatment)
/**
* @inheritdoc
*/
public function track($key, $trafficType, $eventType, $value = null)
public function track($key, $trafficType, $eventType, $value = null, $properties = null)
{
return true;
}
Expand Down
Loading

0 comments on commit 151b796

Please sign in to comment.