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.

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

Drop support for Doctrine cache adapter for metadata #804

Merged
merged 6 commits into from
Dec 19, 2023

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Dec 11, 2023

Remove BC layer introduced in #690 to keep support for Doctrine cache adapters. PSR-6 is now required.

@GromNaN GromNaN added the Task label Dec 11, 2023
@GromNaN GromNaN added this to the 5.0.0 milestone Dec 11, 2023
@GromNaN
Copy link
Member Author

GromNaN commented Dec 11, 2023

@Ahummeling, you introduced this compatibility layer, I'd be glad to have your review if you can find the time.

@franmomu
Copy link
Contributor

There are also these class definitions involved:

<!-- cache -->
<parameter key="doctrine_mongodb.odm.cache.array.class">Doctrine\Common\Cache\ArrayCache</parameter>
<parameter key="doctrine_mongodb.odm.cache.apc.class">Doctrine\Common\Cache\ApcCache</parameter>
<parameter key="doctrine_mongodb.odm.cache.apcu.class">Doctrine\Common\Cache\ApcuCache</parameter>
<parameter key="doctrine_mongodb.odm.cache.memcache.class">Doctrine\Common\Cache\MemcacheCache</parameter>
<parameter key="doctrine_mongodb.odm.cache.memcache_host">localhost</parameter>
<parameter key="doctrine_mongodb.odm.cache.memcache_port">11211</parameter>
<parameter key="doctrine_mongodb.odm.cache.memcache_instance.class">Memcache</parameter>
<parameter key="doctrine_mongodb.odm.cache.xcache.class">Doctrine\Common\Cache\XcacheCache</parameter>

I don't know (I haven't properly checked) if changing them would also allow us to remove:

protected function loadCacheDriver(string $cacheName, string $objectManagerName, array $cacheDriver, ContainerBuilder $container): string
{
if (isset($cacheDriver['namespace'])) {
return parent::loadCacheDriver($cacheName, $objectManagerName, $cacheDriver, $container);
}
$cacheDriverServiceId = $this->getObjectManagerElementName($objectManagerName . '_' . $cacheName);
switch ($cacheDriver['type']) {
case 'service':
$container->setAlias($cacheDriverServiceId, new Alias($cacheDriver['id'], false));
return $cacheDriverServiceId;
case 'memcached':
if (! empty($cacheDriver['class']) && $cacheDriver['class'] !== MemcacheCache::class) {
return parent::loadCacheDriver($cacheName, $objectManagerName, $cacheDriver, $container);
}
$memcachedInstanceClass = ! empty($cacheDriver['instance_class']) ? $cacheDriver['instance_class'] : '%' . $this->getObjectManagerElementName('cache.memcached_instance.class') . '%';
$memcachedHost = ! empty($cacheDriver['host']) ? $cacheDriver['host'] : '%' . $this->getObjectManagerElementName('cache.memcached_host') . '%';
$memcachedPort = ! empty($cacheDriver['port']) ? $cacheDriver['port'] : '%' . $this->getObjectManagerElementName('cache.memcached_port') . '%';
$memcachedInstance = new Definition($memcachedInstanceClass);
$memcachedInstance->addMethodCall('addServer', [
$memcachedHost,
$memcachedPort,
]);
$container->setDefinition($this->getObjectManagerElementName(sprintf('%s_memcached_instance', $objectManagerName)), $memcachedInstance);
$cacheDef = new Definition(MemcachedAdapter::class, [new Reference($this->getObjectManagerElementName(sprintf('%s_memcached_instance', $objectManagerName)))]);
break;
case 'redis':
if (! empty($cacheDriver['class']) && $cacheDriver['class'] !== RedisCache::class) {
return parent::loadCacheDriver($cacheName, $objectManagerName, $cacheDriver, $container);
}
$redisInstanceClass = ! empty($cacheDriver['instance_class']) ? $cacheDriver['instance_class'] : '%' . $this->getObjectManagerElementName('cache.redis_instance.class') . '%';
$redisHost = ! empty($cacheDriver['host']) ? $cacheDriver['host'] : '%' . $this->getObjectManagerElementName('cache.redis_host') . '%';
$redisPort = ! empty($cacheDriver['port']) ? $cacheDriver['port'] : '%' . $this->getObjectManagerElementName('cache.redis_port') . '%';
$redisInstance = new Definition($redisInstanceClass);
$redisInstance->addMethodCall('connect', [
$redisHost,
$redisPort,
]);
$container->setDefinition($this->getObjectManagerElementName(sprintf('%s_redis_instance', $objectManagerName)), $redisInstance);
$cacheDef = new Definition(RedisAdapter::class, [new Reference($this->getObjectManagerElementName(sprintf('%s_redis_instance', $objectManagerName)))]);
break;
case 'apcu':
$cacheDef = new Definition(ApcuAdapter::class);
break;
case 'array':
$cacheDef = new Definition(ArrayAdapter::class);
break;
default:
return parent::loadCacheDriver($cacheName, $objectManagerName, $cacheDriver, $container);
}
$cacheDef->setPublic(false);
$container->setDefinition($cacheDriverServiceId, $cacheDef);
return $cacheDriverServiceId;
}

since apparently these classes are used by AbstractDoctrineExtension.

@Ahummeling
Copy link
Contributor

@Ahummeling, you introduced this compatibility layer, I'd be glad to have your review if you can find the time.

It's been a while since I've touched this code, I might be able to find some time over the weekend. It was quite a complex task, so would like to take the time for it to properly review this.

@franmomu franmomu mentioned this pull request Dec 15, 2023
Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

LGTM. Do we have any other remnants of doctrine/cache to clean up?

DependencyInjection/DoctrineMongoDBExtension.php Outdated Show resolved Hide resolved
DependencyInjection/DoctrineMongoDBExtension.php Outdated Show resolved Hide resolved
DependencyInjection/DoctrineMongoDBExtension.php Outdated Show resolved Hide resolved
@@ -49,7 +39,6 @@

<services>
<!-- defaults -->
<service id="doctrine_mongodb.odm.cache" alias="doctrine_mongodb.odm.cache.array" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be deprecated in 4.7. This is not used.

@GromNaN GromNaN marked this pull request as ready for review December 19, 2023 16:00
@GromNaN
Copy link
Member Author

GromNaN commented Dec 19, 2023

@alcaeus @franmomu I updated the extension and service definitions to not rely on the abstract extension from doctrine-bridge.

@GromNaN GromNaN merged commit 232f6c3 into doctrine:5.0.x Dec 19, 2023
12 checks passed
@GromNaN GromNaN deleted the psr6-required branch December 19, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants