-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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.
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
[5.8] Add support for custom redis driver #29275
Merged
taylorotwell
merged 6 commits into
laravel:5.8
from
paulhenri-l:add-support-for-custom-redis-driver
Jul 30, 2019
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
847b196
Add support for custom drivers
paulhenri-l 45dd2c9
Adhere to the newly created contract
paulhenri-l 647cca5
Test the extend system
paulhenri-l 013b039
Style changes
paulhenri-l ef0f8ec
Update RedisManagerExtensionTest.php
paulhenri-l 5e3ebc5
The interface is not enough
paulhenri-l File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<?php | ||
|
||
namespace Illuminate\Contracts\Redis; | ||
|
||
interface Connector | ||
{ | ||
/** | ||
* Create a new clustered redis connection. | ||
* | ||
* @param array $config | ||
* @param array $options | ||
* @return \Illuminate\Redis\Connections\Connection | ||
*/ | ||
public function connect(array $config, array $options); | ||
|
||
/** | ||
* Create a new clustered redis connection. | ||
* | ||
* @param array $config | ||
* @param array $clusterOptions | ||
* @param array $options | ||
* @return \Illuminate\Redis\Connections\Connection | ||
*/ | ||
public function connectToCluster(array $config, array $clusterOptions, array $options); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
<?php | ||
|
||
namespace Illuminate\Tests\Redis; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Illuminate\Redis\RedisManager; | ||
use Illuminate\Foundation\Application; | ||
use Illuminate\Contracts\Redis\Connector; | ||
|
||
class RedisManagerExtensionTest extends TestCase | ||
{ | ||
/** | ||
* Redis manager instance. | ||
* | ||
* @var RedisManager | ||
*/ | ||
protected $redis; | ||
|
||
protected function setUp(): void | ||
{ | ||
parent::setUp(); | ||
|
||
$this->redis = new RedisManager(new Application(), 'my_custom_driver', [ | ||
'default' => [ | ||
'host' => 'some-host', | ||
'port' => 'some-port', | ||
'database' => 5, | ||
'timeout' => 0.5, | ||
], | ||
'clusters' => [ | ||
'my-cluster' => [ | ||
[ | ||
'host' => 'some-host', | ||
'port' => 'some-port', | ||
'database' => 5, | ||
'timeout' => 0.5, | ||
], | ||
], | ||
], | ||
]); | ||
|
||
$this->redis->extend('my_custom_driver', function () { | ||
return new FakeRedisConnnector(); | ||
}); | ||
} | ||
|
||
public function test_using_custom_redis_connector_with_single_redis_instance() | ||
{ | ||
$this->assertEquals( | ||
'my-redis-connection', $this->redis->resolve() | ||
); | ||
} | ||
|
||
public function test_using_custom_redis_connector_with_redis_cluster_instance() | ||
{ | ||
$this->assertEquals( | ||
'my-redis-cluster-connection', $this->redis->resolve('my-cluster') | ||
); | ||
} | ||
} | ||
|
||
class FakeRedisConnnector implements Connector | ||
{ | ||
/** | ||
* Create a new clustered Predis connection. | ||
* | ||
* @param array $config | ||
* @param array $options | ||
* @return \Illuminate\Contracts\Redis\Connection | ||
*/ | ||
public function connect(array $config, array $options) | ||
{ | ||
return 'my-redis-connection'; | ||
} | ||
|
||
/** | ||
* Create a new clustered Predis connection. | ||
* | ||
* @param array $config | ||
* @param array $clusterOptions | ||
* @param array $options | ||
* @return \Illuminate\Contracts\Redis\Connection | ||
*/ | ||
public function connectToCluster(array $config, array $clusterOptions, array $options) | ||
{ | ||
return 'my-redis-cluster-connection'; | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've realized that
\Illuminate\Contracts\Redis\Connection
is not sufficient.RedisManager has a method type hinted on
\Illuminate\Redis\Connections\Connection
.It also appears that
\Illuminate\Contracts\Redis\Connection
does not have all of the methods defined in\Illuminate\Redis\Connections\Connection
.So extensions will need to subclass
\Illuminate\Redis\Connections\Connection
. That does not seems to be a problem as there are no private methods or properties.The ideal solution would be to change the interface but that would break backwards compatibility.
We can also add a new interface inheriting from this one
\Illuminate\Contracts\Redis\Connection
. But having two interface for the same thing does not sounds ideal.