Skip to content

Conversation

@BrandonLegault
Copy link
Contributor

@BrandonLegault BrandonLegault commented Jan 19, 2017

This PR adds the ability to configure a different cache driver for each of metadata, result and query caching through the doctrine.php config file 'cache' array.

Changes proposed in this pull request:

The current cache array format is as follows:

'cache'            => [
    'default'      => env('DOCTRINE_CACHE', 'array'),
    'namespace'    => null,
    'second_level' => false,
]

This branch introduces this format:

  'cache' => [
      'second_level'     => false,
      'default'          => env('DOCTRINE_CACHE', 'array'),
      'namespace'        => null,
      'metadata'         => [
          'type'         => 'file',
          'namespace'    => null,
      ],
      'query'            => [
          'type'         => 'redis',
          'namespace'    => null,
      ],
      'result'           => [
          'type'         => 'array',
          'namespace'    => null,
      ],
  ],

The configuration is currently applied in this PR by calling \Doctrine\ORM\Configuration->set[Result|Query|Metadata]CacheImpl after reading the values from the config.

The default cache array entries remain the same to ensure backwards compatibility. If any of the metadata, result or query entries are missing from the configuration then they will take whichever the default entry is. The default entry is also mirrored/stored directly in code in case it's accidentally deleted from the configuration.

Copy link
Contributor

@patrickbrouwers patrickbrouwers left a comment

Choose a reason for hiding this comment

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

Can you also update the config file with the new structure, thanks!
It would be good to provide env variables for the 3 drivers as well. They can all 3 default to the DOCTRINE_CACHE env variable.

$this->setSecondLevelCaching($configuration);
}

private function applyNamedCacheConfiguration($cacheName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Docblocks

$defaultDriver = $this->config->get('doctrine.cache.default', $this->defaultCache['type']);
$defaultNamespace = $this->config->get('doctrine.cache.namespace', $this->defaultCache['namespace']);

$driverType = $this->config->get('doctrine.cache.' . $cacheName . '.type', $defaultDriver);
Copy link
Contributor

Choose a reason for hiding this comment

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

type should be named driver

@BrandonLegault
Copy link
Contributor Author

I added the requested changes... I also rebased from 1.2 to keep my branch up to date. I'd already made the changes before I saw the DOCTRINE_CACHE env variable comment. I made them take on new env variables. Do you want me to change that?

'second_level' => false,
'cache' => [
'second_level' => false,
'default' => 'array',
Copy link
Contributor

Choose a reason for hiding this comment

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

'default' => env('DOCTRINE_CACHE', 'array'),

$defaultDriver = $this->config->get('doctrine.cache.default', $this->defaultCache['type']);
$defaultNamespace = $this->config->get('doctrine.cache.namespace', $this->defaultCache['namespace']);

$driver = $this->config->get('doctrine.cache.' . $cacheName . '.driver', $defaultDriver);
Copy link
Contributor

@patrickbrouwers patrickbrouwers Jan 25, 2017

Choose a reason for hiding this comment

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

$cache = $this->cache->driver($driver);

if ($namespace = $this->config->get('doctrine.cache.' . $cacheName . '.namespace', $defaultNamespace)) {
    $cache->setNamespace($namespace);
}

return $cache;

*/
private function applyNamedCacheConfiguration($cacheName)
{
$defaultDriver = $this->config->get('doctrine.cache.default', $this->defaultCache['type']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace $this->defaultCache['type'] by 'array'
Remove $this->defaultCache['namespace']


/**
* @var array
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary

@patrickbrouwers
Copy link
Contributor

I made them take on new env variables. Do you want me to change that?

No it's fine.

@patrickbrouwers patrickbrouwers merged commit ef17247 into laravel-doctrine:1.2 Jan 29, 2017
patrickbrouwers pushed a commit that referenced this pull request Feb 21, 2017
If you have memcached or redis extensions installed
doctrine will try to automatically connect to one of them
if no cache paramter is set when dev mode is off. Defaulting
to the default driver to prevent that behavior from occuring.

Signed-off-by: RJ Garcia <rj@bighead.net>
patrickbrouwers added a commit that referenced this pull request Feb 21, 2017
If you have memcached or redis extensions installed
doctrine will try to automatically connect to one of them
if no cache paramter is set when dev mode is off. Defaulting
to the default driver to prevent that behavior from occuring.

Signed-off-by: RJ Garcia <rj@bighead.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants