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

Switch proxies to LazyGhostTrait #2700

Open
wants to merge 5 commits into
base: 2.10.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 21 additions & 0 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ jobs:
- "highest"
symfony-version:
- "stable"
proxy:
- "lazy-ghost"
include:
# Test against lowest dependencies
- dependencies: "lowest"
Expand All @@ -42,20 +44,30 @@ jobs:
driver-version: "1.17.0"
topology: "server"
symfony-version: "stable"
proxy: "lazy-ghost"
# Test with highest dependencies
- topology: "server"
php-version: "8.2"
mongodb-version: "7.0"
driver-version: "stable"
dependencies: "highest"
symfony-version: "7"
proxy: "lazy-ghost"
# Test with a 5.0 replica set
- topology: "replica_set"
php-version: "8.2"
mongodb-version: "5.0"
driver-version: "stable"
dependencies: "highest"
symfony-version: "stable"
proxy: "lazy-ghost"
# Test with ProxyManager
- php-version: "8.2"
mongodb-version: "5.0"
driver-version: "stable"
dependencies: "highest"
symfony-version: "stable"
proxy: "proxy-manager"
# Test with a 5.0 sharded cluster
# Currently disabled due to a bug where MongoDB reports "sharding status unknown"
# - topology: "sharded_cluster"
Expand All @@ -64,6 +76,7 @@ jobs:
# driver-version: "stable"
# dependencies: "highest"
# symfony-version: "stable"
# proxy: "lazy-ghost"

steps:
- name: "Checkout"
Expand Down Expand Up @@ -118,6 +131,13 @@ jobs:
# https://github.com/vimeo/psalm/pull/10928
composer remove --no-update --dev vimeo/psalm
- name: "Remove proxy-manager-lts"
if: "${{ matrix.proxy != 'proxy-manager' }}"
run: |
# proxy-manager-lts is not installed by default and must not be used
# unless explicitly requested
composer remove --no-update --dev friendsofphp/proxy-manager-lts
- name: "Install dependencies with Composer"
uses: "ramsey/composer-install@v3"
with:
Expand All @@ -134,3 +154,4 @@ jobs:
run: "vendor/bin/phpunit"
env:
DOCTRINE_MONGODB_SERVER: ${{ steps.setup-mongodb.outputs.cluster-uri }}
USE_LAZY_GHOST_OBJECTS: ${{ matrix.proxy == 'lazy-ghost' && '1' || '0' }}"
5 changes: 3 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,20 @@
"doctrine/event-manager": "^1.0 || ^2.0",
"doctrine/instantiator": "^1.1 || ^2",
"doctrine/persistence": "^3.2",
"friendsofphp/proxy-manager-lts": "^1.0",
"jean85/pretty-package-versions": "^1.3.0 || ^2.0.1",
"mongodb/mongodb": "^1.17.0",
"psr/cache": "^1.0 || ^2.0 || ^3.0",
"symfony/console": "^5.4 || ^6.0 || ^7.0",
"symfony/deprecation-contracts": "^2.2 || ^3.0",
"symfony/var-dumper": "^5.4 || ^6.0 || ^7.0"
"symfony/var-dumper": "^5.4 || ^6.0 || ^7.0",
"symfony/var-exporter": "^6.2 || ^7.0"
},
"require-dev": {
"ext-bcmath": "*",
"doctrine/annotations": "^1.12 || ^2.0",
"doctrine/coding-standard": "^12.0",
"doctrine/orm": "^3.2",
"friendsofphp/proxy-manager-lts": "^1.0",
"jmikola/geojson": "^1.0",
"phpbench/phpbench": "^1.0.0",
"phpstan/phpstan": "~1.10.67",
Expand Down
115 changes: 75 additions & 40 deletions lib/Doctrine/ODM/MongoDB/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use Doctrine\Persistence\ObjectRepository;
use InvalidArgumentException;
use LogicException;
use MongoDB\Driver\WriteConcern;
use ProxyManager\Configuration as ProxyManagerConfiguration;
use ProxyManager\Factory\LazyLoadingGhostFactory;
Expand All @@ -33,6 +34,7 @@
use ReflectionClass;

use function array_key_exists;
use function class_exists;
use function interface_exists;
use function trigger_deprecation;
use function trim;
Expand Down Expand Up @@ -82,12 +84,23 @@ class Configuration
*/
public const AUTOGENERATE_EVAL = 3;

/**
* Autogenerate the proxy class when the proxy file does not exist or
* when the proxied file changed.
*
* This strategy causes a file_exists() call whenever any proxy is used the
* first time in a request. When the proxied file is changed, the proxy will
* be updated.
*/
public const AUTOGENERATE_FILE_NOT_EXISTS_OR_CHANGED = 4;

/**
* Array of attributes for this configuration instance.
*
* @phpstan-var array{
* autoGenerateHydratorClasses?: self::AUTOGENERATE_*,
* autoGeneratePersistentCollectionClasses?: self::AUTOGENERATE_*,
* autoGenerateProxyClasses?: self::AUTOGENERATE_*,
* classMetadataFactoryName?: class-string<ClassMetadataFactoryInterface>,
* defaultCommitOptions?: CommitOptions,
* defaultDocumentRepositoryClassName?: class-string<ObjectRepository<object>>,
Expand All @@ -106,24 +119,18 @@ class Configuration
* persistentCollectionGenerator?: PersistentCollectionGenerator,
* persistentCollectionDir?: string,
* persistentCollectionNamespace?: string,
* proxyDir?: string,
* proxyNamespace?: string,
* repositoryFactory?: RepositoryFactory
* }
*/
private array $attributes = [];

private ?CacheItemPoolInterface $metadataCache = null;

private ProxyManagerConfiguration $proxyManagerConfiguration;

private int $autoGenerateProxyClasses = self::AUTOGENERATE_EVAL;

private bool $useTransactionalFlush = false;

public function __construct()
{
$this->proxyManagerConfiguration = new ProxyManagerConfiguration();
$this->setAutoGenerateProxyClasses(self::AUTOGENERATE_FILE_NOT_EXISTS);
}
private bool $useLazyGhostObject = true;

/**
* Adds a namespace under a certain alias.
Expand Down Expand Up @@ -248,68 +255,49 @@ public function setMetadataCache(CacheItemPoolInterface $cache): void
*/
public function setProxyDir(string $dir): void
{
$this->getProxyManagerConfiguration()->setProxiesTargetDir($dir);

// Recreate proxy generator to ensure its path was updated
if ($this->autoGenerateProxyClasses !== self::AUTOGENERATE_FILE_NOT_EXISTS) {
return;
}

$this->setAutoGenerateProxyClasses($this->autoGenerateProxyClasses);
$this->attributes['proxyDir'] = $dir;
}

/**
* Gets the directory where Doctrine generates any necessary proxy class files.
*/
public function getProxyDir(): ?string
{
return $this->getProxyManagerConfiguration()->getProxiesTargetDir();
return $this->attributes['proxyDir'] ?? null;
}

/**
* Gets an int flag that indicates whether proxy classes should always be regenerated
* during each script execution.
*
* @return self::AUTOGENERATE_*
*/
public function getAutoGenerateProxyClasses(): int
{
return $this->autoGenerateProxyClasses;
return $this->attributes['autoGenerateProxyClasses'] ?? self::AUTOGENERATE_FILE_NOT_EXISTS;
}

/**
* Sets an int flag that indicates whether proxy classes should always be regenerated
* during each script execution.
*
* @param self::AUTOGENERATE_* $mode
*
* @throws InvalidArgumentException If an invalid mode was given.
*/
public function setAutoGenerateProxyClasses(int $mode): void
{
$this->autoGenerateProxyClasses = $mode;
$proxyManagerConfig = $this->getProxyManagerConfiguration();

switch ($mode) {
case self::AUTOGENERATE_FILE_NOT_EXISTS:
$proxyManagerConfig->setGeneratorStrategy(new FileWriterGeneratorStrategy(
new FileLocator($proxyManagerConfig->getProxiesTargetDir()),
));

break;
case self::AUTOGENERATE_EVAL:
$proxyManagerConfig->setGeneratorStrategy(new EvaluatingGeneratorStrategy());

break;
default:
throw new InvalidArgumentException('Invalid proxy generation strategy given - only AUTOGENERATE_FILE_NOT_EXISTS and AUTOGENERATE_EVAL are supported.');
}
$this->attributes['autoGenerateProxyClasses'] = $mode;
}

public function getProxyNamespace(): ?string
{
return $this->getProxyManagerConfiguration()->getProxiesNamespace();
return $this->attributes['proxyNamespace'] ?? null;
}

public function setProxyNamespace(string $ns): void
{
$this->getProxyManagerConfiguration()->setProxiesNamespace($ns);
$this->attributes['proxyNamespace'] = $ns;
}

public function setHydratorDir(string $dir): void
Expand Down Expand Up @@ -589,14 +577,35 @@ public function getPersistentCollectionGenerator(): PersistentCollectionGenerato
return $this->attributes['persistentCollectionGenerator'];
}

/** @deprecated */
public function buildGhostObjectFactory(): LazyLoadingGhostFactory
{
return new LazyLoadingGhostFactory(clone $this->getProxyManagerConfiguration());
return new LazyLoadingGhostFactory($this->getProxyManagerConfiguration());
}

/** @deprecated */
public function getProxyManagerConfiguration(): ProxyManagerConfiguration
{
return $this->proxyManagerConfiguration;
$proxyManagerConfiguration = new ProxyManagerConfiguration();
$proxyManagerConfiguration->setProxiesTargetDir($this->getProxyDir());
$proxyManagerConfiguration->setProxiesNamespace($this->getProxyNamespace());

switch ($this->getAutoGenerateProxyClasses()) {
case self::AUTOGENERATE_FILE_NOT_EXISTS:
$proxyManagerConfiguration->setGeneratorStrategy(new FileWriterGeneratorStrategy(
new FileLocator($proxyManagerConfiguration->getProxiesTargetDir()),
));

break;
case self::AUTOGENERATE_EVAL:
$proxyManagerConfiguration->setGeneratorStrategy(new EvaluatingGeneratorStrategy());

break;
default:
throw new InvalidArgumentException('Invalid proxy generation strategy given - only AUTOGENERATE_FILE_NOT_EXISTS and AUTOGENERATE_EVAL are supported.');
}

return $proxyManagerConfiguration;
Comment on lines +589 to +608
Copy link
Member Author

Choose a reason for hiding this comment

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

Creating ProxyManagerConfiguration on-demand as the class is not required when LaryGhostObjects are used.

}

public function setUseTransactionalFlush(bool $useTransactionalFlush): void
Expand All @@ -608,6 +617,32 @@ public function isTransactionalFlushEnabled(): bool
{
return $this->useTransactionalFlush;
}

/**
* Generate proxy classes using Symfony VarExporter's LazyGhostTrait if true.
* Otherwise, use ProxyManager's LazyLoadingGhostFactory (deprecated)
*/
public function setUseLazyGhostObject(bool $flag): void
{
if ($flag === false) {
if (! class_exists(ProxyManagerConfiguration::class)) {
throw new LogicException('Package "friendsofphp/proxy-manager-lts" is required to disable LazyGhostObject.');
}

trigger_deprecation(
'doctrine/mongodb-odm',
'2.6',
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
'2.6',
'2.10',

'Using "friendsofphp/proxy-manager-lts" is deprecated. Use "symfony/var-exporter" LazyGhostObjects instead.',
);
}

$this->useLazyGhostObject = $flag;
}

public function isLazyGhostObjectEnabled(): bool
{
return $this->useLazyGhostObject ?? true;
}
Comment on lines +620 to +645
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 we use the same method names as the ORM: isLazyGhostObjectEnabled and setLazyGhostObjectEnabled?

}

interface_exists(MappingDriver::class);
10 changes: 8 additions & 2 deletions lib/Doctrine/ODM/MongoDB/DocumentManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
use Doctrine\ODM\MongoDB\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ODM\MongoDB\Mapping\MappingException;
use Doctrine\ODM\MongoDB\Proxy\Factory\LazyGhostProxyFactory;
use Doctrine\ODM\MongoDB\Proxy\Factory\ProxyFactory;
use Doctrine\ODM\MongoDB\Proxy\Factory\StaticProxyFactory;
use Doctrine\ODM\MongoDB\Proxy\Resolver\CachingClassNameResolver;
use Doctrine\ODM\MongoDB\Proxy\Resolver\ClassNameResolver;
use Doctrine\ODM\MongoDB\Proxy\Resolver\LazyGhostProxyClassNameResolver;
use Doctrine\ODM\MongoDB\Proxy\Resolver\ProxyManagerClassNameResolver;
use Doctrine\ODM\MongoDB\Query\FilterCollection;
use Doctrine\ODM\MongoDB\Repository\DocumentRepository;
Expand Down Expand Up @@ -156,7 +158,9 @@ protected function __construct(?Client $client = null, ?Configuration $config =
],
);

$this->classNameResolver = new CachingClassNameResolver(new ProxyManagerClassNameResolver($this->config));
$this->classNameResolver = $config->isLazyGhostObjectEnabled()
? new CachingClassNameResolver(new LazyGhostProxyClassNameResolver())
: new CachingClassNameResolver(new ProxyManagerClassNameResolver($this->config));

$metadataFactoryClassName = $this->config->getClassMetadataFactoryName();
$this->metadataFactory = new $metadataFactoryClassName();
Expand All @@ -181,7 +185,9 @@ protected function __construct(?Client $client = null, ?Configuration $config =

$this->unitOfWork = new UnitOfWork($this, $this->eventManager, $this->hydratorFactory);
$this->schemaManager = new SchemaManager($this, $this->metadataFactory);
$this->proxyFactory = new StaticProxyFactory($this);
$this->proxyFactory = $config->isLazyGhostObjectEnabled()
? new LazyGhostProxyFactory($this, $config->getProxyDir(), $config->getProxyNamespace(), $config->getAutoGenerateProxyClasses())
: new StaticProxyFactory($this);
$this->repositoryFactory = $this->config->getRepositoryFactory();
}

Expand Down
7 changes: 7 additions & 0 deletions lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Doctrine\ODM\MongoDB\Event\PreLoadEventArgs;
use Doctrine\ODM\MongoDB\Events;
use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
use Doctrine\ODM\MongoDB\Proxy\InternalProxy;
use Doctrine\ODM\MongoDB\Types\Type;
use Doctrine\ODM\MongoDB\UnitOfWork;
use ProxyManager\Proxy\GhostObjectInterface;
Expand Down Expand Up @@ -448,6 +449,12 @@ public function hydrate(object $document, array $data, array $hints = []): array
}
}

if ($document instanceof InternalProxy) {
// Skip initialization to not load any object data
$document->__setInitialized(true);
}

// Support for legacy proxy-manager-lts
if ($document instanceof GhostObjectInterface && $document->getProxyInitializer() !== null) {
// Inject an empty initialiser to not load any object data
$document->setProxyInitializer(static function (
Expand Down
9 changes: 7 additions & 2 deletions lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Doctrine\ODM\MongoDB\Id\IdGenerator;
use Doctrine\ODM\MongoDB\LockException;
use Doctrine\ODM\MongoDB\Mapping\Annotations\TimeSeries;
use Doctrine\ODM\MongoDB\Proxy\InternalProxy;
use Doctrine\ODM\MongoDB\Types\Incrementable;
use Doctrine\ODM\MongoDB\Types\Type;
use Doctrine\ODM\MongoDB\Types\Versionable;
Expand Down Expand Up @@ -1893,9 +1894,11 @@ public function getIdentifierObject(object $document)
*/
public function setFieldValue(object $document, string $field, $value): void
{
if ($document instanceof GhostObjectInterface && ! $document->isProxyInitialized()) {
if ($document instanceof InternalProxy && ! $document->__isInitialized()) {
//property changes to an uninitialized proxy will not be tracked or persisted,
//so the proxy needs to be loaded first.
$document->__load();
} elseif ($document instanceof GhostObjectInterface && ! $document->isProxyInitialized()) {
$document->initializeProxy();
}

Expand All @@ -1909,7 +1912,9 @@ public function setFieldValue(object $document, string $field, $value): void
*/
public function getFieldValue(object $document, string $field)
{
if ($document instanceof GhostObjectInterface && $field !== $this->identifier && ! $document->isProxyInitialized()) {
if ($document instanceof InternalProxy && $field !== $this->identifier && ! $document->__isInitialized()) {
$document->__load();
} elseif ($document instanceof GhostObjectInterface && $field !== $this->identifier && ! $document->isProxyInitialized()) {
$document->initializeProxy();
}

Expand Down
Loading