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

Generating proxy classes only when file not exists #1920

Closed
Rybbow opened this issue Jan 3, 2019 · 3 comments
Closed

Generating proxy classes only when file not exists #1920

Rybbow opened this issue Jan 3, 2019 · 3 comments
Assignees
Milestone

Comments

@Rybbow
Copy link
Contributor

Rybbow commented Jan 3, 2019

Bug Report

Q A
BC Break ?
Version ?

Summary

The Doctrine\ODM\MongoDB\Configuration::AUTOGENERATE_FILE_NOT_EXISTS configuration option should prevent the proxy generator from generating a given proxy file if it already exists, but actually the proxy file is regenerated on each request.

Current behavior

Even though Configuration::setAutoGenerateProxyClasses() is called with Configuration::AUTOGENERATE_FILE_NOT_EXISTS the proxy generator regenerates each affected proxy file once per request.

How to reproduce

Make sure document proxies are generated. Then in a separate process use DocumentManager::getReference() with a document that is not in the identity map - it will causes proxies to be regenerated.

Expected behavior

Generated proxy file should not change?

If you take a look here at what this configuration option in fact does when set, it turns out it simply changes proxy generator strategy. And inspecting the set FileWriterGeneratorStrategy and the FileLocator it uses, it can be seen that there is no file_exists check in place (and it seems it has never existed, which means this never worked?).

I guess a custom decorator for \ProxyManager\GeneratorStrategy\GeneratorStrategyInterface, that would check the existence of the file and wrap the original strategy would solve the issue. However, this would probably mean that proxies are no longer automatically regenerated for dev/debug environments in projects using this library. Which would mean that proxies do not contain up-to-date metadata for document classes that were edited until they are manually regenerated or unless some alternative tracking method is introduced.

What should be the course of action here?

@alcaeus
Copy link
Member

alcaeus commented Jan 4, 2019

The generator is only hit if the class does not exist (or could not be autoloader), see https://github.com/Ocramius/ProxyManager/blob/8d0fa8e3e11cdadaaf4c41732366b5ed48bb0aed/src/ProxyManager/Factory/AbstractBaseFactory.php#L64..L71.

Maybe @Ocramius can shed some light on this, are Proxy files supposed to be autoloaded, and if so, are we missing some config here?

@alcaeus
Copy link
Member

alcaeus commented Jan 4, 2019

After some discussion, this is somewhat expected: the documentation recommends registering an autoloader for production, which doesn't happen. In this case, we'd expect the framework integration (e.g. doctrine/mongodb-odm-bundle) to take care of this. I've created doctrine/DoctrineMongoDBBundle#519 to track this in the bundle.

For users that aren't using the bundle, we'll need to add this bit to the documentation. I'm leaving this issue open to track that. Thanks for investigating this!

@alcaeus alcaeus added Documentation and removed Bug labels Jan 4, 2019
@alcaeus alcaeus added this to the 2.0.0-Beta2 milestone Jan 4, 2019
@Ocramius
Copy link
Member

Ocramius commented Jan 4, 2019

The idea presented in https://github.com/Ocramius/ProxyManager/blob/2.2.2/docs/tuning-for-production.md is that the the user would call spl_autoload_register($proxyManagerConfig->getAutoloader()) somewhere early in the setup process.

Since the autoloader coming with ProxyManager is quite naïve, this is best left to the composer autoloader, if build and run steps can be clearly separated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants