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

ControllerFactoryFilter::getController returns false #7

Closed
aydot opened this issue Apr 9, 2015 · 14 comments
Closed

ControllerFactoryFilter::getController returns false #7

aydot opened this issue Apr 9, 2015 · 14 comments
Labels

Comments

@aydot
Copy link
Contributor

aydot commented Apr 9, 2015

ControllerFactoryFilter::getController returns false when trying to use controller method injection.
I see it returns false from here :
[https://github.com/lorenzo/piping-bag/blob/master/src/Routing/Filter/ControllerFactoryFilter.php#L93]
but I am calling from a controller, and I don't understand why it's returning false when there is definitely a controller.

@aydot
Copy link
Contributor Author

aydot commented Apr 9, 2015

Also, the Injecting Dependencies in Controllers example code you have doesn't work for me. When I use the Piping::get('Classname'), it works. but when i inject the dependency, and put the annotation, it doesn't.

@lorenzo
Copy link
Owner

lorenzo commented Apr 9, 2015

I'll take a look at this soon.

@aydot
Copy link
Contributor Author

aydot commented Apr 10, 2015

Had a chance to look at this yet ?

@lorenzo
Copy link
Owner

lorenzo commented Apr 11, 2015

I can confirm the bug...

@lorenzo lorenzo added the bug label Apr 11, 2015
@lorenzo
Copy link
Owner

lorenzo commented Apr 11, 2015

@aydot due to some changes I made earlier for performance, I had to change the way the plugin is installed. Check the new instructions https://github.com/lorenzo/piping-bag#configuration

In particular, you need to change

DispatcherFactory::add('ControllerFactory');

With the following:

use PipingBag\Di\PipingBag;
DispatcherFactory::add(PipingBag::get('PipingBag\Routing\Filter\ControllerFactoryFilter'));

@aydot
Copy link
Contributor Author

aydot commented Apr 11, 2015

the controller action injection still doesn't work for me

@lorenzo
Copy link
Owner

lorenzo commented Apr 11, 2015

Tell me what you did, it works for me fine

@lorenzo
Copy link
Owner

lorenzo commented Apr 11, 2015

@aydot actually you are right, it does not work. I need to investigate further.

@lorenzo lorenzo reopened this Apr 11, 2015
@aydot
Copy link
Contributor Author

aydot commented Apr 13, 2015

have an idea when you'd be able to look into this? quick question, will this work for a Controller constructor injection just like Laravel does?

@lorenzo
Copy link
Owner

lorenzo commented Apr 13, 2015

Hey, I'm sorry about this issue. I'm currently sorting it out with Ray.Di creator, since I need a feature that is not currently supported in the library.

CakePHP already reserves the constructor of the controller, so adding more parameters to it will only feel awkward. Instead you can use use any other method to get your dependencies injected:

/**
 * @Inject
 */
public function postConstruct(Dep1 $dep, Dep2 $dep2 ....) {
}

I'll try to solve this issue as soon as possible. If you need any workaround please let me know. There are way to workaround this, but need some changes in Ray.Di to make this work in a generic way.

@aydot
Copy link
Contributor Author

aydot commented Apr 13, 2015

How do I write test for the dependencies? Will I have to swap out the instance in the container?
I tried this code here but didn't work.

$container = new Container;
$browser = $this->getMock('Ikimea\Browser\Browser');
(new Bind($container, 'Ikimea\Browser\Browser'))->toInstance($browser);

@lorenzo
Copy link
Owner

lorenzo commented Apr 13, 2015

The best way is to create a new container with extra modules that you can use in the test. In general, this technique is used for integration testing and makes no much sense for unit testing since you should be passing the dependencies manually:

PipingBag::container($anotherInjectorInstance);

You can create the injector instance manually or by passing the result of calling PipingBag::create($allModulesNeededForTest)

@aydot
Copy link
Contributor Author

aydot commented Apr 14, 2015

How would you test a code like this?

<?php
namespace App\View\Cell;

use Cake\View\Cell;
use PipingBag\Di\PipingBag as Di;
use Ikimea\Browser\Browser;

class BrowserCompatibilityCell extends Cell
{
    protected $_validCellOptions = [];

    public function display()
    {
        $browser = Di::get('Ikimea\Browser\Browser');
        $this->set('browser', $browser->getBrowser().' '.$browser->getVersion());
    }
}

Since I can't inject it's dependency via the constructor

@lorenzo
Copy link
Owner

lorenzo commented May 3, 2015

Closing this as it is working now

@lorenzo lorenzo closed this as completed May 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants