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

Stabilize public API #7

Open
Ocramius opened this issue Apr 12, 2012 · 4 comments
Open

Stabilize public API #7

Ocramius opened this issue Apr 12, 2012 · 4 comments

Comments

@Ocramius
Copy link
Contributor

Here's a short list of interfaces and ideas I came up with (could be splitted into multiple issues later)

  • ZfcAcl\Service\AclServiceInterface - new interface, let's place what the Acl service should actually do in here
  • ZfcAcl\Service\Context - do we need this? It looks more like a behavior to be set into a custom RoleProvider, and I would also move it out of the ZfcAcl\Service namespace
  • ZfcAcl\Service\Acl\RoleProvider - could this not just simply be replaced by a Zend\Acl\Role\RoleInterface? I think it adds an unnecessary layer of complexity... Eventual usage of associated entities/user objects could be hidden behind its implementation. I think it could be more a matter of educating users about it's usage.
  • ZfcAcl\View\Helper\ZfcAcl should be type hinted.
  • There is a LOT of confusion and abbreviations within ZfcAcl\Model in my opinion (still have to figure out how to solve that)
  • ZfcAcl\Model contains a lot of configuration classes and maps that consume them that aren't really easy nor intuitive, and require a lot of configuration that in most cases doesn't fit (see following point).
  • ZfcAcl\Service\ResourceMapperInterface - could add this interface as simple bi-directional mapping of $resourceName <-> $requestedItem. That would probably simplify the clutter of ZfcAcl\Model, whose aim was to map routes, events and controllers to resources. The entire configuration mapping thing could somehow be abstracted and made optional (or at least not necessary) by introducing sane defaults (see the mapper for the Dispatch guard)
  • ZfcAcl\Guard\GuardInterface (to rename) - Problematic to add a public method here, but I think it could be done assuming the mapper interface will be flexible enough.
  • ZfcAcl\Exception\ExceptionInterface - simply to comply with the new ZendFramework 2 exception concept :)
  • ZfcAcl\Guard\ActionControllerGuard - because we don't handle "actions" right now...
  • ZfcAcl\ZfcAclAwareInterface - for setter injection of the service itself
@matuszeman
Copy link
Contributor

some examples plz

@Ocramius
Copy link
Contributor Author

@matuszemi updated issue :)

@matuszeman
Copy link
Contributor

(please comment below my comments to maintain the thread and post it as a new comment)

Here's a short list of interfaces and ideas I came up with (could be splitted into multiple issues later)

  • ZfcAcl\Service\AclServiceInterface - new interface, let's place what the Acl service should actually do in here
    @matuszemi: do we need this? it's more obvious for mappers but do we need this for (each) service?
  • ZfcAcl\Service\Context - do we need this? It looks more like a behavior to be set into a custom RoleProvider, and I would also move it out of the ZfcAcl\Service namespace
    @matuszemi: Context is probably not best way of describing the service purpose. I did not want to put this into Acl service itself, but I found nice usecase where runAs(role, callable) might be useful. See: https://github.com/kapitchi/KapitchiIdentity/blob/master/src/KapitchiIdentity/Service/Registration.php
  • ZfcAcl\Service\Acl\RoleProvider - could this not just simply be replaced by a Zend\Acl\Role\RoleInterface? I think it adds an unnecessary layer of complexity... Eventual usage of associated entities/user objects could be hidden behind its implementation. I think it could be more a matter of educating users about it's usage.
    @matuszemi: as there is RowObjectInterface in Zend\Db to define a "contract", I believe we should keep this interface to do the same. RoleInterface should be implemented by classes representing role itself only and RoleProvider by classes providing current role to Acl service.
  • ZfcAcl\View\Helper\ZfcAcl should be type hinted
    @matuszemi: agree.
    .
  • There is a LOT of confusion and abbreviations within ZfcAcl\Model in my opinion (still have to figure out how to solve that)
    @matuszemi: TBD
  • ZfcAcl\Model contains a lot of configuration classes and maps that consume them that aren't really easy nor intuitive, and require a lot of configuration that in most cases doesn't fit (see following point).
  • ZfcAcl\Service\ResourceMapperInterface - could add this interface as simple bi-directional mapping of $resourceName <-> $requestedItem. That would probably simplify the clutter of ZfcAcl\Model, whose aim was to map routes, events and controllers to resources. The entire configuration mapping thing could somehow be abstracted and made optional (or at least not necessary) by introducing sane defaults (see the mapper for the Dispatch guard)
    @matuszemi: TBD ---- it's not always this easy $resourceName <-> $requestedItem .... or let me rephrase that...
    I agree that in dispatchable guard it works this way - but taking the example of Route guard - you want to protect admin area - it is easier to say route 'admin' (and all child routes) -> resource 'Route/Admin'. Hmmmmm.. I'm just thinking... can we use Acl resource hierarchy for this case?
  • ZfcAcl\Guard\GuardInterface (to rename) - Problematic to add a public method here, but I think it could be done assuming the mapper interface will be flexible enough.
    @matuszemi: I still haven't identified common API for guards... it might come by generalizing model namespace?
  • ZfcAcl\Exception\ExceptionInterface - simply to comply with the new ZendFramework 2 exception concept :)
    @matuszemi: TBD - but agree in general - as long as it becomes "better" and will follow conventions.
  • ZfcAcl\Guard\ActionControllerGuard - because we don't handle "actions" right now...
    @matuszemi: are you talking about implementing new guard?
  • ZfcAcl\ZfcAclAwareInterface - for setter injection of the service itself
    @matuszemi: use-case please? is it really needed? again ... as per my comment on first point - then you could easily end up having 'aware' interfaces for every single service/mapper/model/mywhatever.... do we really need this?

@Ocramius
Copy link
Contributor Author

@matuszemi just added a couple of PRs that already fix a couple of things :) I'll comment the rest tomorrow :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants