Refactor

[bakura10]

Hi everyone,

This PR is nothing more than the Rbac component I have written with @danizord and @arekkas (https://github.com/zf-fr/rbac) that is used for ZfcRbac since a lot of time.

It is therefore mature, tested. This PR only has renamed every classes to the Zend namespaces. All tests were updated too.

However for ZF3, I'd like:

• To remove the various traversal strategies to keep only the generator one (the most efficient). While it made sense before to have different strategies and select the best one, now that we are PHP 5.5+ I think it's not necessary to make the architecture this complex with traversal strategies (I'm not sure anyone has already provided a custom traversal strategy... and I don't think of a use case of that). I'll wait @danizord feedback on this.
• Maybe include Abac feature into this (or should we create a zend-permissions-abac module)?

[aeneasr]

:+1:

[Maks3w]

We had to rewrite master branch for match release-2.4.2. Could you rebase your work again?

Sorry for the inconvenience

[weierophinney]

@bakura10 Before I can do any real review here, can you detail:

• any additions to the current API
• any removals from the current API
• use cases and examples

That will make it easier for me to understand what the refactor accomplishes, and what the migration issues might entail.

Thanks!

[bakura10]

Hi @weierophinney ,

Sorry, I didn't have the time to answer to other PR (I'm not doing programming those times). But for this proposal this is quite simple:

any additions to the current API

Everything.

any removals from the current API

Everything.

There is nothing in common (so definitely no migration path). This refactor is just the basis for Rbac model. We've used this library for ZfcRbac still v2 for at least one year and a half. I suspect not many people still use RBAC from ZF2 (it's only used by ZfcRbac < v2 and from Packagist not many people still use this versions).

This proposal is actually very stabled. No bugs have been found in the core library. The only thing is removing some code that was needed for 5.4 compatibility.

[bakura10]

If @danizord or @arekkas want to tell you more about it as they have been major contributors of it too.

[weierophinney]

But for this proposal this is quite simple:

any additions to the current API Everything.

any removals from the current API Everything.

So, I need to see use cases: how does a developer consume the library after the refactor? How do they build the RBAC, how do they query it, etc. Again, as I noted in some of the other components, this PR will often be the first line of documentation for users interested in what changed, as it will be in the changelog; I want them to have enough information at their fingertips to get started. :)

/cc @danizord @arekkas

[aeneasr]

I can not provide good use cases at the moment, because it has been over a year since I last actively worked with ZfcRbac. However, I remember that we had a lot of issues with the native RBAC implementation and basically simpled down the architecture whilst keeping the functionality.

Regarding the strategies: I'm in favor of removing them and just use the generator. It will simplify things even more :)

Regarding abac: I would probably not introduce it to RBAC and rather create a new module for that functionality. Keep things simple :)

[bakura10]

Exactly.

I'm not sure neither what I could say by features. ZF2 RBAC was painfully to work with, it used a complicated architecture based on RecursiveIteratorIterator that proved to be highly inefficient in most cases.

The core idea of the component has not changed, it has just been re-architectured to be efficient.

[aeneasr]

Ah right, the RecursiveIteratorIterator. That thing sucked :D

[weierophinney]

@bakura10 and @arekkas — My point is: documentation. How does a user consume the component after the changes? What are the differences between how it was consumed previously and how it is now? Will existing code need to change?

Saying, "I'm not sure what I could say by features" could mean that the API is exactly the same, and this just refactors the internals. If that's the case, fine: say that. I need to know what I'm reviewing, and that's quite enough, when it comes down to it.

Next: the PR currently is unmergeable, which means me reviewing is likely pointless currently. Could one of you rebase and re-push?

[bakura10]

Hi,

No, the API is not the same, the code is not backward compatible, the architecture is different. In other words: nothing is the same, because ZF2 Rbac had a lot of architectural flaws that basically made it wrong.

Actually, ZF2 Rbac was already unusable on its own (it was jsut the basic logic of Rbac, so nobody would actually use ZF2 Rbac alone). ZfcRbac was the module that made it work.

However since a long time ago, ZfcRbac does not rely on ZF2 Rbac anymore, but on zfr/rbac (which is this proposal). I just checked and approximately 90% of downlaods for ZfcRbac comes with a version that uses zfr/rbac (so basically 10% people are still using a very old, unmaintained version of ZfcRbac that relies on ZF2 Rbac).

I cannot really say more actually. ZF2 Rbac is bascialy dead and I highly doubt any people are still using it, except people using very old ZfcRbac.

[danizord]

Heya! Sorry for the delay, I completely forgot to reply this.

As @bakura10 said, this refactor changed the whole thing. So we definitely need a complete migration guide.

You guys know my english is not very good, but this weekend I'll try to write down some migration notes for this one. Also I'm planning to add some changes to this proposal, like a stateless service to allow standalone usage of the component without ZfcRbac.

@bakura10, :+1: for removing RecursiveRoleIteratorStrategy. But I'd like to keep the traversal strategy concept because it allow us to provide strategies for specific use-cases (remember the intent here https://github.com/zf-fr/rbac/pull/5). A simple strategy for non-hierarchical implementations that iterates over the roles without visiting the hierarchy may be a good example.

[bakura10]

Sounds very good to me, thanks @danizord :)

[bakura10]

Any news from your work @danizord? I'd like to start writing doc for this :).

[exptom]

Is this likely to be implemented? What is the recommended solution for RBAC in a zf-mvc v3 app?

[weierophinney]

@exptom We need a rebase, documentation, and migration guides before we can consider this, since it is such a large change from the current version. @bakura10 and/or @danizord — are you still considering completing this, by any chance?

[ezimuel]

I'm closing this PR for inactivity.