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

Proposals for the dependency injection container #330

Closed
ghost opened this issue Jul 31, 2014 · 19 comments
Closed

Proposals for the dependency injection container #330

ghost opened this issue Jul 31, 2014 · 19 comments
Assignees

Comments

@ghost
Copy link

ghost commented Jul 31, 2014

Hi Arul,

I noticed your Scope class (and a thread on Stackoverflow where you introduce it). While it's a great idea to enable DI in RESTler, but in fact, the current solution has some issues.

So it would be great to change the way RESTler uses DI. My "action plan" is the following:

  • First, enable the usage of external containers, so that controllers/services could be instantiated by them. In my opinion, only container-interop compliant containers should be regarded.
  • Then the Scope class could support the aforementioned standard, so it would become optional to use it or not.
  • Finally it could be worth refactoring RESTler to use real DI (not service location).

If you are ok with my opinion, I could make a PR soon for the first step. But the second and the third ones need more time and effort to be developed, so I don't promise anything.

@ghost
Copy link
Author

ghost commented Aug 1, 2014

A slight clarification: maybe I missed the point about Scope: you have never stated that Restler internally uses DI. You only told that Scope can ALSO be used as a DI container. Sorry for my misunderstanding. That is why the first issue becomes only a notice. :)

@Arul-
Copy link
Member

Arul- commented Aug 1, 2014

I will go through the above mentioned article and get back to you!

@Arul- Arul- self-assigned this Aug 1, 2014
@ghost ghost mentioned this issue Aug 4, 2014
@Arul-
Copy link
Member

Arul- commented Aug 18, 2014

I will definitely consider this on RC6

@ghost
Copy link
Author

ghost commented Aug 28, 2014

I sent a new pull request for this issue which is far better than the last one. Basically, I created a Container Interop-compliant adapter for the Scope class which can be used for instanting the API classes.

@matak
Copy link

matak commented Nov 16, 2014

i am still not sure how DI in restler works, can you help to understand? bcs how can i inject for example database connection to apiclass constructor when it is instantiated? also it would be nice to set dependencies in constructors of api classes, when this class will be instantiated it looks to contructor and inject the service which i need from my DI container, maybe you should look on this implementation

http://doc.nette.org/en/2.2/dependency-injection

@matak
Copy link

matak commented Nov 16, 2014

i think the problem is here
https://github.com/Luracast/Restler/blob/master/vendor/Luracast/Restler/Scope.php#L94
so it means that is imposible to inject any objects/services into constructor of created api class?

@Arul-
Copy link
Member

Arul- commented Nov 17, 2014

It is possible to do dependency injection with Scope, here is how it is done!

Scope::register('My\Api\Something', function () {
    return new My\Api\Something(
        Scope::get('My\Dependency1'),
        Scope::get('My\Dependency2')
    );
});

After the above code somewhere before $restler->handle(). When class Something is requested it will be created with the above function.

@Arul- Arul- closed this as completed Nov 17, 2014
@matak
Copy link

matak commented Nov 17, 2014

i understand your meaning but it is not solution for me, all dependencies i have to keep in some configuration outside of the classes, in nette/di you can do this

class XY {
   public function __construct(\DibiConnection $sql, \CacheStorage $cache) { ...
}

and the services sql and cache are automaticaly injected, also the deep injection is provided.

bcs the \CacheStorage also need injection of \FileStorage and \DibiConnection and etc. so my structure what i need is based on constructors of the services nowhere else, and if i change it so it is all i need to do

what do you think about that? just let the nette/di do all DI and all will be served automatically, what do you think?

@matak
Copy link

matak commented Nov 17, 2014

i wanted to say that in your example how i set dependencies of My\Dependency1 ?

@Arul-
Copy link
Member

Arul- commented Nov 17, 2014

Scope::register('My\DependencyX', function () {
    return My\DependencyX::instance();
}
Scope::register('My\Dependency1', function () {
    return new My\Dependency1(
        Scope::get('My\DependencyX'),
    );
});
Scope::register('My\Api\Something', function () {
    return new My\Api\Something(
        Scope::get('My\Dependency1'),
        Scope::get('My\Dependency2')
    );
});

@Arul-
Copy link
Member

Arul- commented Nov 17, 2014

You can use any other DI along with Scope, see the following pseudo code

Scope::register('My\Api\Something', function ()  {
    return nette/di['something'];
});

@matak
Copy link

matak commented Nov 17, 2014

nice, what about implement nette/di from scratch? this means i need configure all api classes

@matak
Copy link

matak commented Nov 17, 2014

as a workaround done

$apis = array('Api\Endpoints', 'Api\Documents');
foreach($apis as $apiClass) {
    Luracast\Restler\Scope::register($apiClass, function () use ($context, $apiClass)  {
        return $context->getByType($apiClass);
    }); 
}

$r = new Restler(Debugger::DETECT); // return true if you are in production mode
$r->setSupportedFormats('JsonFormat', 'XmlFormat'); //from restler framework for API Explorer
$r->addAPIClass('Luracast\Restler\Resources');

//setup apis
foreach($apis as $apiClass) {
    $r->addAPIClass($apiClass);
}

but i think the whole project appreciate to use nette/di i can help with understanding of functions

@Arul-
Copy link
Member

Arul- commented Nov 20, 2014

With the above commit that will soon be part of RC6 all you need is

Scope::$resolver = function ($className) use ($context) {
    return $context->getByType($className);
};

@Arul-
Copy link
Member

Arul- commented Nov 26, 2014

@matak did you try the above solution?

@matak
Copy link

matak commented Nov 26, 2014

didnt have so much time, but this settings of composer "restler/framework": "@rc",

still doesnt work

Access to undeclared static property: Luracast\Restler\Scope::$resolver

?

@soda-2005
Copy link

With the above commit that will soon be part of RC6 all you need is
Scope::$resolver = function ($className) use ($context) {
return $context->getByType($className);
};

I tried to use the dependency-Injection (of my MVC-framework), but it did not work in Explorer (when i want to see the online-documentation for my API's). When i call the online-documentation of my API's, than this PHP-Fatal-Error appears:
PHP Fatal error: Call to a member function getRequestedApiVersion() on a non-object in Luracast/Restler/Explorer.php on line 167

The problem/bug is in class 'Scope' in Method 'get':
When Scope::$resolver is defined, than the 'logic' does not take place, where some object-properties were set. This PHP-code will not be called:
Scope:get:

       if (class_exists($fullName)) {
           $shortName = Util::getShortName($name);
           $r = new $fullName();
           static::$instances[$name] = (object)array('instance' => $r);
           if ($name != 'Restler') {
               $r->restler = static::get('Restler');
               $m = Util::nestedValue($r->restler, 'apiMethodInfo', 'metadata');
               if ($m) {
                   $properties = Util::nestedValue(
                       $m, 'class', $fullName,
                       CommentParser::$embeddedDataName
                   ) ?: (Util::nestedValue(
                       $m, 'class', $shortName,
                       CommentParser::$embeddedDataName
                   ) ?: array());
               } else {
                   static::$instances[$name]->initPending = true;
               }
           }
       }

I would be very cool, when you can fix that bug, so i could really use my own dependency-Injection :-)

@soda-2005
Copy link

oh, i forgot, i use restler in version 3.0.0-RC6

@soda-2005
Copy link

I created a new Issue (#423) for that bug:
#423

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