Unexpected behavior when using ResolverCollection and mapping a Table class to a Policy

[michielkeijts]

Problem Description

For ease of use, I wanted to minimize the amount of policies needed. In this case I have a CoinPolicy which is the policy for everything what is related to Coins (Coin (Entity) and CoinTable (Table)).
This CoinPolicy implements can() methods and scopes.

While using solely ORM resolver, the plugin expects me to have a CoinPolicy (for the can() methods) and a CoinsTablePolicy for the scopes.

However, I wanted to map the CoinsTable::class to the CoinPolicy::class in order to use one class.

As suggested here, I implemented the Resolvercollection like:

$orm = new OrmResolver();
$map = new MapResolver();
$resolver = new ResolverCollection([$map, $orm]);

In the map resolver I defined the mapping (E.g. \App\Model\Table\CoinsTable::class => \App\Policy\CoinsPolicy::class). So I expect:

1. The Map resolver checks first for an existing mapped policy,
2. In case not found, the ORM resolver is the fallback

However when I call in the controller $this->Authorization->applyScope($query, 'adminIndex'); where $query comes from the CoinsTable repository, it results in an error, while it sees the resource $query as instance of Cake\ORM\Query and starts looking for the QueryPolicy instead of the CoinsPolicy
However, I would expect the MapResolver to find my mapped CoinsTable::class and return the CoinsPolicy. This does not happen.

Where in Code

The OrmResolver::getPolicy($resource) checks if the $resource is instance of EntityInterface, RepositoryInterface or QueryInterface and searches for the corresponding EntityPolicy or a RepositoryPolicy.
The MapResolver does not map the Entity/Repository Interfaces to corresponding Policies before trying to find a mapped policy.

Proposed solution

I suggest to update the Mapresolver::getPolicy() and replace $class = get_class($resource) in order that $class is the same type what the getPolicy method is searching for in the OrmResolver

[markstory]

The map resolver is meant to cover simple mapping scenarios. Have you considered building your own resolver that applies the conventions you want to have? The ResolverInterface is intentionally small to make this kind of customisation possible.

[michielkeijts]

Yes I have. Indeed, implementing a resolver yourself is an option. But my point here is that I was expect the ORM resolver and the Map resolver to search for the same policy, in order to map them differently. This is not happening as the ORM resolver translates the Query resource to a policy for Table objects.

[markstory]

But the MapResolver is a simple map, and you're applying a policy to to a Query not a table.

[michielkeijts]

Well, the point is that I implemented the ORM resolver without having policies defined. I got a missing policy exception, for a CoinsTablePolicy. However, by mapping this policy onto the CoinsPolicy, it resulted in the behaviour I am describing.

I still think the Mapresolver should be able to map a policy like a TablePolicy to any other custom policy. This is also not possible at the moment, as the Mapresolver simply not looks for a TablePolicy when a Query $resource is supplied.

[markstory]

I still think the Mapresolver should be able to map a policy like a TablePolicy to any other custom policy.

But this isn't a 'map' anymore. It is a map with ORM specific features added in. I'm not against having a different Resolver for that use case, but I disagree that the behaviour you're describing belongs in MapResolver.

[michielkeijts]

I see your point. I think I agree with you. The mapresolver can be left as it is and a different 'ORMMapResolver' or so can be added for this use case. I can make a suggestion (merge request) if you like.

However, I think the reason I thought this was an issue, is because the documentation writes about the combination of the Map and ORM resolver. This combination does not work as fluent as it is suggested over there, hence this issue. Should we mention this more explicit?

[markstory]

Both of those changes sound good to me.

[dereuromark]

ping @michielkeijts

[othercorey]

Closing since the solution is a custom resolver.