-
Notifications
You must be signed in to change notification settings - Fork 27
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
Allowing use of DSN for redis and mongo factories #24
Conversation
if (empty($config['dsn'])) { | ||
$manager = new Manager(sprintf('mongodb://%s:%s', $config['host'], $config['port'])); | ||
} else { | ||
$manager = new Manager($config['dsn']); |
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.
You forgotten validate here
Cool solution with the DSN object. Maybe you could override the validate function to make sure we validate the DSN on build time? /**
* {@inheritdoc}
*/
public static function validate(array $options, $adapterName)
{
parent::validate(options, adapterName);
if (empty($options['dsn'])) {
return;
}
$dsn = new DSN($options['dsn']);
if (!$dsn->isValid()) {
throw new \InvalidArgumentException('Invalid DSN: '.$config['dsn']);
}
} |
|
||
private function parseProtocol($dsn) | ||
{ | ||
$regex = '/^(redis|mysql|mongodb|tcp):\/\//i'; |
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.
Should we be more generic with?:
$regex = '|^(.*?)://|i';
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.
That would allow for any protocol to be considered valid. I only want redis, mysql, mongodb, and tcp to be considered valid for now
use Symfony\Component\OptionsResolver\OptionsResolver; | ||
|
||
/** | ||
* @author Tobias Nyholm <tobias.nyholm@gmail.com> |
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.
Typo
Nice work. The DSN factory really makes sen and made the code cleaner. |
Creating AbstractDsnAdapterFactory Updating DSN to parse multiple hosts. Adding tests for DSN
Allowing use of DSN for redis and mongo factories
Resolves: php-cache/issues#26