Thoughts on DI

[altaraven]

Kern046, first of all, thank you for this package since it looks like the only one ABAC implementation for native PHP at all :) But I found this library very NON-extendable :( One simple example. It is not possible at all to plug in your own Loader (I want to develop and use e.g. SQL or MongoDB loader for policies). And also all Managers are instantiated by class name too in Abac constructor... By using classname-dependant code you don't give a chance for developer to extend your library ((( The only way to use it in real project is to copy-paste and rewrite the code.

[Kern046]

Hello !

Thank you for your feedback, it is very nice to have some thoughts about the library !

I really understand your problem and I want php-abac to be available for any kind of usage. My primary thought about it was to implement step-by-step new loaders, new comparisons and features in the library core. I still want to do it that way.

On the other hand, I totally agree about the need for customization, as I often do in my own projects.

So I will implement a way to add your own Manager classes, loaders, comparisons, etc... But I think the best way to do it is to implement a new Abac class, something like CustomAbac, to keep other users from a dependency injection system which could be useless for them.

I want to know your thoughts about this idea, would that be a solution for your use ?

Eventually, I would like to know as well what are the different baheviours you would ahve in these new Manager classes ? If this concerns new features, perhaps I can implement it in the current classes !

Once again, thanks for your feedback ! I need it to make an useful and efficient library !

[altaraven]

But I think the best way to do it is to implement a new Abac class, something like CustomAbac, to keep other users from a dependency injection system which could be useless for them.

I think it is not a good idea. DI cannot be useless in 2016 :) According to SOLID principles: 1) Its better to provide inheritense ability for class users rather than modification of existing code (O letter) 2) Parts of the programm should be replacable without harming the logic of it (L). Considering this I would advice to check the PHP-FFMpeg/PHP-FFMpeg library implementation at github. I consider it to be near to perfect implementaton of extendable library. Check this use case:

$ffmpeg = FFMpeg::create([
            'timeout' => 0,
            'ffmpeg.threads' => 4,
        ]);  
$video = $ffmpeg->open($originalPath);
$video->addFilter(new SimpleFilter(['-metadata', 'title=']));
$video->save(new X264(), $compressedPath);

In this case you can create your own video filter which just needs to implement known interface - and library still working. The same is with format class (new X264). So, the first thing you need to do is start using interfaces. Especially you need to get rid of such things like:

    public function __construct(ComparisonManager $comparisonManager) {
        $this->comparisonManager = $comparisonManager;
    }

You should declare interfaces names, not classes:

    public function __construct(SomeManagerInterface $comparisonManager) {
        $this->comparisonManager = $comparisonManager;
    }

As for Manager classes - in my case I just needed to add new loader to ConfigurationManager's loaders. So I had to re-write Abac class and ConfigurationManager class to make it possible. So maybe a good idea will be to re-design loaders instantiation and move it to upper level - to Abac class. And then pass ready loaders to Manager. Or even make it settable like this:

$abac = new Abac([
//...
   'loader' => new MyLoader($anyParams)
]);
//or
$abac = new Abac();
$abac->setLoader(new MyLoader($anyParams)); //path to yml can be passed here. Or db connection instance

[Kern046]

I can't agree more :) !

I will work on it and keep you updated !

[Kern046]

Hello ! Sorry for the delay I'm running out of time. I am currently working on what we said.

I wanted to give precisions about the work and discuss the form it is taking.

The constructor had too many arguments if I passed the managers to it.

That's why I am creating an init method with these parameters. If the user does not provide instances of AttributeManagerInterface for example, the library one will be provided !

I hope I can create a PR for it soon !

[altaraven]

ok, thanks for update! I have tried the library with Yii2 and Laravel 5. Both projects were REST APIs, without any highload for now unfortunately:( But works well with both. With Yii2 it is even possible to map policies to sql select conditions, so the user sees only records he allowed. (they have action as a standalone class implementation). Extended some comparison classes to get more flexibility with user's fields.

[altaraven]

The constructor had too many arguments if I passed the managers to it.

I think in this case there are 2 ways to make it pretty. 1) pass an array of managers 2) make chaining call obj->setM1()->setM2()...

Or implement both

[Kern046]

Using the Factory pattern, it is now possible to use its own managers with the Abac object :) !

I'm closing this issue