Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature key prefixes #56

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Feature key prefixes #56

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
28ff2c2
Key prefixes in Lock, Journal, SessionHandler, Storage INIT
Nov 23, 2015
b7dde89
configuration fixes
Nov 23, 2015
1eed9f0
key prefixes documentation
Nov 23, 2015
27657e1
revert gitignore
Nov 23, 2015
26f38c4
Fixes for comments in PR (https://github.com/Kdyby/Redis/pull/56)
Feb 15, 2016
fabf7fa
fix build error
Feb 16, 2016
8deb7c6
fix build error
Feb 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
vendor
composer.lock
.idea
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this to your gitignore_global, not here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted in 27657e1

33 changes: 33 additions & 0 deletions docs/en/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,36 @@ You can disable the native driver by this option (and the emulated will take con
redis:
session: {native: off}
```

## Key prefixes

When you use two instances of one application with one Redis server, it is possible, that your data can be overwritten by second application instance.

To avoid this problem you can define key prefixes in configuration, for example:

Instance 1
```yml
redis:
keyPrefix: "instance1_"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

namespace sounds better to me, doctrine cache also uses it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the _ is added automatically in the classes. The resulting key would be instance1__Nette:something

host: 127.0.0.1
port: 6379
journal: on
session: on
storage: on
debugger: off
```

Instance 2
```yml
redis:
keyPrefix: "instance2_"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about it, someone might misinterpret this as adding the prefix to the client object - it would be a little more writing, but it would also make a lot more sense to me having them under the specific sections.

parameters:
    redis:
        namespace: instance1

redis:
    host: 127.0.0.1
    port: 6379
    journal:
        namespace: %redis.namespace%
    session:
        namespace: %redis.namespace%
    storage:
        namespace: %redis.namespace%
    debugger: off

host: 127.0.0.1
port: 6379
journal: on
session: on
storage: on
debugger: off
```

After configration all keys will be prefixed "keyPrefix_key"

37 changes: 26 additions & 11 deletions src/Kdyby/Redis/DI/RedisExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use Nette\DI\Config;



/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not neccesary

* @author Filip Procházka <[email protected]>
*/
Expand Down Expand Up @@ -155,8 +154,13 @@ protected function loadJournal(array $config)

$builder = $this->getContainerBuilder();

$constructParams = array(
$this->prefix('@client'),
(!isset($config['keyPrefix']) ? null : $config['keyPrefix']),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • braces around such simple condition are not neccesary
  • it's better to switch the logic, as the exclamation mark can be easily overlooked and therefore not writing it is more error prone
  • contants with uppercase please
isset($config['keyPrefix']) ? $config['keyPrefix']) : NULL,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the default NULL should be probably included in the clientDefaults

);

$builder->addDefinition($this->prefix('cacheJournal'))
->setClass('Kdyby\Redis\RedisLuaJournal');
->setClass('Kdyby\Redis\RedisLuaJournal', $constructParams);

// overwrite
$journalService = $builder->getByType('Nette\Caching\Storages\IJournal') ?: 'nette.cacheJournal';
Expand All @@ -178,8 +182,14 @@ protected function loadStorage(array $config)
'locks' => TRUE,
));

$constructParams = array(
$this->prefix('@client'),
$this->prefix('@cacheJournal'),
(!isset($config['keyPrefix']) ? null : $config['keyPrefix']),
);

$cacheStorage = $builder->addDefinition($this->prefix('cacheStorage'))
->setClass('Kdyby\Redis\RedisStorage');
->setClass('Kdyby\Redis\RedisStorage', $constructParams);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should upgrade your PhpStorm, the new one doesn't break the indentation in here :)


if (!$storageConfig['locks']) {
$cacheStorage->addSetup('disableLocking');
Expand All @@ -206,7 +216,7 @@ protected function loadSession(array $config)
'weight' => 1,
'timeout' => $config['timeout'],
'database' => $config['database'],
'prefix' => self::DEFAULT_SESSION_PREFIX,
'prefix' => (!isset($config['keyPrefix']) ? self::DEFAULT_SESSION_PREFIX : $config['keyPrefix']),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here also #56 (diff) + other occurences

'auth' => $config['auth'],
'native' => TRUE,
'lockDuration' => $config['lockDuration'],
Expand All @@ -220,15 +230,20 @@ protected function loadSession(array $config)

if ($sessionConfig['native']) {
$this->loadNativeSessionHandler($sessionConfig);
return;
}

} else {
$builder->addDefinition($this->prefix('sessionHandler'))
->setClass('Kdyby\Redis\RedisSessionHandler', array($this->prefix('@sessionHandler_client')));
$constructParams = array(
$this->prefix('@sessionHandler_client'),
(!isset($config['keyPrefix']) ? null : $config['keyPrefix']),
);

$sessionService = $builder->getByType('Nette\Http\Session') ?: 'session';
$builder->getDefinition($sessionService)
->addSetup('?->bind(?)', array($this->prefix('@sessionHandler'), '@self'));
}
$builder->addDefinition($this->prefix('sessionHandler'))
->setClass('Kdyby\Redis\RedisSessionHandler', $constructParams);

$sessionService = $builder->getByType('Nette\Http\Session') ?: 'session';
$builder->getDefinition($sessionService)
->addSetup('?->bind(?)', array($this->prefix('@sessionHandler'), '@self'));
}


Expand Down
14 changes: 11 additions & 3 deletions src/Kdyby/Redis/ExclusiveLock.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class ExclusiveLock extends Nette\Object
*/
private $client;

/**
* @var string
*/
private $keyPrefix = '';

/**
* @var array
*/
Expand All @@ -48,13 +53,16 @@ class ExclusiveLock extends Nette\Object
public $acquireTimeout = FALSE;



/**
* @param RedisClient $redisClient
* @param string|null $keyPrefix
*/
public function __construct(RedisClient $redisClient)
public function __construct(RedisClient $redisClient, $keyPrefix = null)
{
$this->client = $redisClient;
if (!empty($keyPrefix)) {
$this->keyPrefix = $keyPrefix . '_';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd preffer : as it's prefixing a namespace and you're basically creating a namespace. Also in other places in this library : is already used as namespace separator.

}
}


Expand Down Expand Up @@ -216,7 +224,7 @@ public function getLockTimeout($key)
*/
protected function formatLock($key)
{
return $key . ':lock';
return $this->keyPrefix . $key . ':lock';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really neccesary to prefix the lock keys? This would probably result in something like instance1_instance1_:nette:cache:43243243242342432:lock which isn't adding much value IMHO.

}


Expand Down
13 changes: 10 additions & 3 deletions src/Kdyby/Redis/RedisJournal.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use Nette\Caching\Cache;



/**
* Redis journal for tags and priorities of cached values.
*
Expand All @@ -36,14 +35,22 @@ class RedisJournal extends Nette\Object implements Nette\Caching\Storages\IJourn
*/
protected $client;

/**
* @var string
*/
private $keyPrefix = '';


/**
* @param RedisClient $client
* @param string|null $keyPrefix
*/
public function __construct(RedisClient $client)
public function __construct(RedisClient $client, $keyPrefix = null)
{
$this->client = $client;
if (!empty($keyPrefix)) {
$this->keyPrefix = $keyPrefix . '_';
}
}


Expand Down Expand Up @@ -183,7 +190,7 @@ private function tagEntries($tag)
*/
protected function formatKey($key, $suffix = NULL)
{
return self::NS_NETTE . ':' . $key . ($suffix ? ':' . $suffix : NULL);
return self::NS_NETTE . ':' . $this->keyPrefix . $key . ($suffix ? ':' . $suffix : NULL);
}

}
15 changes: 12 additions & 3 deletions src/Kdyby/Redis/RedisSessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ class RedisSessionHandler extends Nette\Object implements \SessionHandlerInterfa
*/
private $client;

/**
* @var string
*/
private $keyPrefix = '';

/**
* @var Nette\Http\Session
*/
Expand All @@ -51,13 +56,16 @@ class RedisSessionHandler extends Nette\Object implements \SessionHandlerInterfa
private $ttl;



/**
* @param RedisClient $redisClient
* @param string|null $keyPrefix
*/
public function __construct(RedisClient $redisClient)
public function __construct(RedisClient $redisClient, $keyPrefix = null)
{
$this->client = $redisClient;
if (!empty($keyPrefix)) {
$this->keyPrefix = $keyPrefix . '_';
}
}


Expand Down Expand Up @@ -228,7 +236,8 @@ protected function lock($id)
*/
private function formatKey($id)
{
return self::NS_NETTE . $id;
$key = $this->keyPrefix . $id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it makes more sense to have the instance prefix at the beggining of the key to me

return self::NS_NETTE . $key;
}

}
15 changes: 12 additions & 3 deletions src/Kdyby/Redis/RedisStorage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,29 @@ class RedisStorage extends Nette\Object implements IMultiReadStorage
*/
private $journal;

/**
* @var string
*/
private $keyPrefix = '';

/**
* @var bool
*/
private $useLocks = TRUE;



/**
* @param RedisClient $client
* @param \Nette\Caching\Storages\IJournal $journal
* @param string|null $keyPrefix
*/
public function __construct(RedisClient $client, IJournal $journal = NULL)
public function __construct(RedisClient $client, IJournal $journal = NULL, $keyPrefix = null)
{
$this->client = $client;
$this->journal = $journal;
if (!empty($keyPrefix)) {
$this->keyPrefix = $keyPrefix . '_';
}
}


Expand Down Expand Up @@ -301,7 +309,8 @@ public function clean(array $conds)
*/
protected function formatEntryKey($key)
{
return self::NS_NETTE . ':' . str_replace(Cache::NAMESPACE_SEPARATOR, ':', $key);
$prefixedKey = $this->keyPrefix . $key;
return self::NS_NETTE . ':' . str_replace(Cache::NAMESPACE_SEPARATOR, ':', $prefixedKey);
}


Expand Down