Ocramius / ProxyManager

🎩✨🌈 OOP Proxy wrappers/utilities - generates and manages proxies of your objects
MIT License
4.96k stars 187 forks source link

FileLocator::getProxyFileName fails PSR-0/PSR-4 autoloading when no classmap is defined #371

Open temple opened 7 years ago

temple commented 7 years ago

At version 2.0.4

When using an AbstractBaseFactory descendant, configured with a FileWriterGeneratorStrategy using a bundled FileLocator instance, it SEEMS not providing a way to accomplish PSRs in its logic behind getProxyFileName function, and it crashes when checking the class signature during the Proxy generation

Ocramius commented 7 years ago

@temple can you please check 2.1.x first? 2.0.x will only receive security fixes

temple commented 7 years ago

Checking out 2.1.0 has "broken" my SF2 application cache. Is there any known bug or unexplicited behaviour behind 2.1.x?

I think I've found a bug, motivated by "gos/web-socket-bundle", i'll bring you more details

Ocramius commented 7 years ago

Not really, it actually has even stricter checks...

On 4 May 2017 18:48, "Temple" notifications@github.com wrote:

Checking out 2.1.0 has "broken" my SF2 application cache. Is there any known bug or unexplicited behaviour behind 2.1.x?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Ocramius/ProxyManager/issues/371#issuecomment-299243386, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJakM2V4_mm0LneIl9Th07xdRgrnRm8ks5r2gFPgaJpZM4NQ5bI .

temple commented 7 years ago

Whenever I rebuild SF2 project cache via php app/console cache:clear it produces following constructor signature for class Gos\Bundle\WebSocketBundle\Server\Type\WebSocketServer (at version 1.8.8) when warming up app/cache/prod/appProdProjectContainer.php

class GosBundleWebSocketBundleServerTypeWebSocketServer_000000004ee6d98f0000000078e37539e77e8ca7c8cf8b427c73c477d767db6e extends \Gos\Bundle\WebSocketBundle\Server\Type\WebSocketServer implements \ProxyManager\Proxy\VirtualProxyInterface
{

. . . 
  public function __construct(\React\EventLoop\LoopInterface $loop, \Symfony\Component\EventDispatcher\EventDispatcherInterface $eventDispatcher, \Gos\Bundle\WebSocketBundle\Server\App\Registry\PeriodicRegistry $periodicRegistry, \Gos\Bundle\WebSocketBundle\Server\App\WampApplication $wampApplication, \Gos\Bundle\WebSocketBundle\Server\App\Registry\OriginRegistry $originRegistry, $originCheck, \Ratchet\Wamp\TopicManager $topicManager, \Gos\Bundle\WebSocketBundle\Pusher\ServerPushHandlerRegistry $serverPushHandlerRegistry, ?\Psr\Log\LoggerInterface $logger = null)
    {
. . . 

It produces a bad namespaced type hint (?\Psr\Log\LoggerInterface) for $logger argument (the last one), coming from this constructor signature:

// vendor/gos/web-socket-bundle/Server/Type/WebSocketServer.php
. . .
use Gos\Bundle\WebSocketBundle\Server\App\Registry\PeriodicRegistry;
use Gos\Bundle\WebSocketBundle\Server\App\WampApplication;
use Gos\Component\RatchetStack\Builder;
use ProxyManager\Proxy\ProxyInterface;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Ratchet\Wamp\TopicManager;
use React\EventLoop\Factory;
use React\EventLoop\LoopInterface;
use React\Socket\Server;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\Session\Storage\Handler\NullSessionHandler;
. . . 
 public function __construct(
        LoopInterface $loop,
        EventDispatcherInterface $eventDispatcher,
        PeriodicRegistry $periodicRegistry,
        WampApplication $wampApplication,
        OriginRegistry $originRegistry,
        $originCheck,
        TopicManager $topicManager,
        ServerPushHandlerRegistry $serverPushHandlerRegistry,
        LoggerInterface $logger = null
    ) {

This only happens when I've attached ocramius/proxy-manager* to my project via composer, but not when I remove it.

¿Any idea why?

* Reproduced using vendor versions 2.0.4 and 2.1.0

temple commented 7 years ago

I think I've found.

symfony/proxy-manager-bridge vendor has following "composer.json" requirements:

"require": {
        "php": ">=5.3.9",
        "symfony/dependency-injection": "~2.3",
        "ocramius/proxy-manager": "~0.4|~1.0|~2.0"
    },
    "require-dev": {
        "symfony/config": "~2.3"
    },
Ocramius commented 7 years ago

It produces a bad namespaced type hint (?\Psr\Log\LoggerInterface) for $logger argument (the last one), coming from this constructor signature

That signature is correct. Writing:

function foo(Bar $baz = null) {}

and

function foo(?Bar $baz = null) {}

is equivalent

temple commented 7 years ago

¿Really? Then, it is not an issue for "ocramius/proxy-manager", "symfony/proxy-manager-bridge" neither "zenframework/zend-code".

Sorry for the inconvenience

Ocramius commented 7 years ago

@temple not a problem, but what is the failure you are experiencing?

temple commented 7 years ago

Is this (kind of error), it happens at execution time

PHP Fatal error:  Declaration of GosBundleWebSocketBundleServerAppDispatcherTopicDispatcher_000000004b5242aa000000002a3c5adfebd6ee0aa660f497e7e72cab75be4d85::dispatch($calledMethod, Ratchet\ConnectionInterface $conn, Ratchet\Wamp\Topic $topic, Gos\Bundle\WebSocketBundle\Router\WampRequest $request, $payload = NULL, $exclude = NULL, $eligible = NULL, $provider = NULL) must be compatible with Gos\Bundle\WebSocketBundle\Server\App\Dispatcher\TopicDispatcherInterface::dispatch($calledMethod, ?Ratchet\ConnectionInterface $conn, Ratchet\Wamp\Topic $topic, Gos\Bundle\WebSocketBundle\Router\WampRequest $request, $payload = NULL, $exclude = NULL, $eligible = NULL) in /var/www/hotfixes/app/cache/prod/appProdProjectContainer.php

That error was produced by running the application in Apache 2.4.25 over an Ubuntu Trusty Linux (14.04 LTS) My php version is PHP Version 7.0.16-2+deb.sury.org~trusty+1

Ocramius commented 7 years ago

That is extremely weird, see https://3v4l.org/O6fKY for an example (pasted below).

<?php

interface Bar {}
interface Foo
{
    public function aaa(Bar $bar = null);
}
class Baz implements Foo
{
    public function aaa(Bar $bar = null) {}
}
class Tab implements Foo
{
    public function aaa(?Bar $bar = null) {}
}

Something else is wrong in that declaration mismatch

Ocramius commented 7 years ago

Wait, where did your $provider parameter go? :|

temple commented 7 years ago

Sorry, cause I'm dealing with two interactions facing this problem.

The error I've pasted here appears when clearing and warming up SF2 cache.

And the initial issue experienced was reported by apache like this:

[Fri May 05 10:29:02.320159 2017] [:error] [pid 1213] [client 127.0.0.1:48512] PHP Parse error:  syntax error, unexpected '?', expecting variable (T_VARIABLE) in /var/www/hotfixes/app/cache/prod/appProdProjectContainer.php on line 8078

I'm checking php-cli and apache php module versions and them mismatch.

cli is PHP 7.1.0-3+deb.sury.org~trusty+1 with Zend Engine v3.1.0-dev, while apache is PHP 7.0.16-2+deb.sury.org~trusty+1 with Zend Engine v3.0.0.

This mismatch between what is producing proxied code (php-cli) and what is interpreted (php7_module), may be the reason why.

Ocramius commented 7 years ago

Ah yes, make sure you got 7.1 straight there :-)

Let me know if I can be of any further help.

temple commented 7 years ago

Well, it ain't exactly what was told here https://github.com/Ocramius/ProxyManager/issues/371#issuecomment-299243386

BUT,

Upgrading to 2.1.x from 2.0.4, meant assuming an upgrade from Zend Engine 3.0.0 to 3.0.1 (php-cli 7.0.X to 7.1).

My mismatch between apache and cli done the rest.

temple commented 7 years ago

I confirm aligning both php interpreters to version 7.1 fixes the "error"

temple commented 7 years ago

Recovering the issue subject, now that I have the project working (with ocramius/proxy-manager set at version 2.1.1), I'm getting this error:

[2017-05-05 13:31:54] request.CRITICAL: Uncaught PHP Exception ReflectionException: "Class ProxyManagerGeneratedProxy__PM__\Itbid\AdministracionBundle\Classes\PHPExcel\TemporaryPHPExcel_Reader\Generated8a8520b9bcaeaddc8d63114d770f9140 does not exist" at /var/www/hotfixes/vendor/ocramius/proxy-manager/src/ProxyManager/Factory/AbstractBaseFactory.php line 99 {"exception":"[object] (ReflectionException(code: -1): Class ProxyManagerGeneratedProxy\PM\Itbid\AdministracionBundle\Classes\PHPExcel\TemporaryPHPExcel_Reader\Generated8a8520b9bcaeaddc8d63114d770f9140 does not exist at /var/www/hotfixes/vendor/ocramius/proxy-manager/src/ProxyManager/Factory/AbstractBaseFactory.php:99)"} []

This happens (I think) because I'm using this code to instantiate the ProxyFactory

        $configuration = new Configuration();
        $filelocator = new FileLocator(getcwd());
        $filewriterGenerator = new FileWriterGeneratorStrategy($filelocator);
        $configuration->setGeneratorStrategy($filewriterGenerator);
        $factory = new AccessInterceptorValueHolderFactory($configuration);

But this is "solved" by adding this three lines before the creation of the factory instance:

        $inflector = $configuration->getClassNameInflector();
        $autoloader = new Autoloader($filelocator,$inflector);
        $configuration->setProxyAutoloader($autoloader);

I think this doesn't make really sense to have an Autoloader that can works with a differerent FileLocator than the one used by the FileWriterGenerator. Don't you think?

Ocramius commented 7 years ago

I think this doesn't make really sense to have an Autoloader that can works with a differerent FileLocator than the one used by the FileWriterGenerator. Don't you think?

Depending on the setup, some folks may not want an autoloader at all (they generate a composer autoloader with the proxies in it, for example), but indeed, allowing diverging configs is a pain.

The idea is that if you write to your getcwd(), then you should provide a mechanism to read from getcwd() as well.

I don't have a real fix for this, as there is no assumption that a generator strategy will ever need a file locator, or that an autoloader will do, so I'm going to close it as "misconfiguration issue". There is no sane/proper way to check for symmetricity.

temple commented 7 years ago

No, it is not really easy to handle.

A possible way to face this is to replace constructors arguments with Configuration type hints, or just giving an oportunity to injections on setters. In other words, If you want your Autoloader's $fileLocator differ from what is set in Configuration, you will always be able to user a setter.

Let me explain in the "code way":

    // Here we need no dependencies, cause Configuration "knows"
        $configuration = new Configuration();
    // Here we choose how to read/write
        $filelocator = new FileLocator(getcwd());
    // Now we should be able to inject on Configuration
        $configuration->setFileLocator($filelocator);
    // We have injected $filelocator into $configuration, then we don't wanna inject to FileWriterGeneratorStrategy
        $filewriterGenerator = new FileWriterGeneratorStrategy();
    // Configuration should be able too provide the $filelocator to $filewriterGenerator in following setter
        $configuration->setGeneratorStrategy($filewriterGenerator);
    // Then the Autoloader instantiation sharing the same $filelocator as the FileWriterGeneratorStrategy won't be able, cause, as said before, Configuration "knows", and knows about hot to create a new instance of Autoloader, providing its own ClassNameInflector and FileLocator
/**        $inflector = $configuration->getClassNameInflector();
        $autoloader = new Autoloader($filelocator,$inflector);
        $configuration->setProxyAutoloader($autoloader);**/

        $factory = new AccessInterceptorValueHolderFactory($configuration);

Now we wanna a different $filelocator for Autoloading proxy classes, then, we should do:

        $configuration->getAutoloader()->setLocator(new FileLocator(realpath('./app/cache')));
Ocramius commented 7 years ago

@temple the setter approach is already working: how would the Configuration class code differ?

temple commented 7 years ago

The difference is in Autoloader and GeneratorStrategies. Them both seem to be though for building or setting up a Configuration instance.

That's why, Configuration instance can have it own FileLocator and use this when creating instances of Autoloader or receiving a GeneratorStrategy (via setter)

This lets the user define a different FileLocator for reading and writing, if wants to, but doesn't force to code an snippet of 10 lines, when just an different generator strategy (from default one) was wanted (what means 2 lines)

What is meant to be reached out, is the following:

$factory = new AccessInterceptorValueHolderFactory();
// Because Configuration knows, FileWriterGenerator won't need an specific FileLocator
$factory->getConfiguration()->setFileLocator($MyOwnFileLocatorClassExtendingFileLocator)->setGeneratorStrategy(new FileWriterGeneratorStrategy());

But it would be reached out too if Configuration had an own FileLocator property

$factory = new AccessInterceptorValueHolderFactory();

$factory->getConfiguration()->setFileLocator($MyOwnFileLocatorClassExtendingFileLocator);
$factory->getConfiguration()->setGeneratorStrategy(new FileWriterGeneratorStrategy($factory->getConfiguration()->getFileLocator()));