Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/Policy/MapResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,20 +89,29 @@ public function map(string $resourceClass, callable|object|string $policy)
/**
* {@inheritDoc}
*
* @throws \InvalidArgumentException When a resource is not an object.
* Accepts either an object instance or a class FQCN string registered in
* the map. Strings that are not valid class names continue to raise
* InvalidArgumentException.
*
* @throws \InvalidArgumentException When a resource is neither an object nor a class FQCN string.
* @throws \Authorization\Policy\Exception\MissingPolicyException When a policy for a resource has not been defined.
*/
public function getPolicy($resource): mixed
{
if (!is_object($resource)) {
$message = sprintf('Resource must be an object, `%s` given.', gettype($resource));
if (is_object($resource)) {
$class = $resource::class;
} elseif (is_string($resource) && class_exists($resource)) {
$class = $resource;
} else {
$message = sprintf(
'Resource must be an object or fully-qualified class name, `%s` given.',
is_string($resource) ? $resource : gettype($resource),
);
throw new InvalidArgumentException($message);
}

$class = $resource::class;

if (!isset($this->map[$class])) {
throw new MissingPolicyException($resource);
throw new MissingPolicyException(is_object($resource) ? $resource : [$class]);
}

$policy = $this->map[$class];
Expand Down
35 changes: 34 additions & 1 deletion src/Policy/OrmResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ public function __construct(
}

/**
* Get a policy for an ORM Table, Entity or Query.
* Get a policy for an ORM Table, Entity, Query or class name string.
*
* Accepting an entity/table fully-qualified class name as the resource allows checks
* like `$user->can('add', Article::class)` where no instance is on hand
* (e.g. menu rendering before a `newEmptyEntity()`).
*
* @param mixed $resource The resource.
* @return mixed
Expand All @@ -88,10 +92,39 @@ public function getPolicy(mixed $resource): mixed

return $this->getRepositoryPolicy($repo);
}
if (is_string($resource) && class_exists($resource)) {
return $this->getPolicyByClassName($resource);
}

throw new MissingPolicyException([get_debug_type($resource)]);
}

/**
* Locate a policy from a class name string by matching the standard
* entity/table namespace markers.
*
* @param string $class The fully qualified class name.
* @return mixed
* @throws \Authorization\Policy\Exception\MissingPolicyException When the
* string does not match an entity/table namespace pattern or no policy
* exists at the conventional location.
*/
protected function getPolicyByClassName(string $class): mixed
{
foreach (['\Model\Entity\\', '\Model\Table\\'] as $marker) {
$pos = strpos($class, $marker);
if ($pos === false) {
continue;
}
$namespace = str_replace('\\', '/', substr($class, 0, $pos));
$name = str_replace('\\', '/', substr($class, $pos + strlen($marker)));
Comment on lines +114 to +120

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to be doing string searches? Can't we use the interfaces instead? What do we gain from substring matching?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two different jobs, conflated:

  1. Discrimination — "is this an entity or a table?" New code decides via strpos($class, '\Model\Entity') vs '\Model\Table'.
  2. Extraction — pull namespace + name segments to feed findPolicy().

The # 1:
Should probably use interfaces ideally

For # 2:
getEntityPolicy()/getRepositoryPolicy() already substring-search \Model\Entity\ to derive namespace+name for findPolicy() — even for instances. That's OrmResolver's existing convention. So interface approach removes substring from the decision, not from the lookup. A non-conventional namespace still fails at findPolicy() either way.


return $this->findPolicy($class, $name, $namespace);
}

throw new MissingPolicyException([$class]);
}

/**
* Get a policy for an entity
*
Expand Down
21 changes: 20 additions & 1 deletion tests/TestCase/Policy/MapResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,30 @@ public function testGetPolicyPrimitive(): void
$resolver = new MapResolver();

$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Resource must be an object, `string` given.');
$this->expectExceptionMessage('Resource must be an object or class FQCN string, `Foo` given.');

$resolver->getPolicy('Foo');
}

public function testGetPolicyClassNameAsResource(): void
{
$resolver = new MapResolver();
$resolver->map(Article::class, ArticlePolicy::class);

$result = $resolver->getPolicy(Article::class);
$this->assertInstanceOf(ArticlePolicy::class, $result);
}

public function testGetPolicyUnregisteredClassString(): void
{
$resolver = new MapResolver();

$this->expectException(MissingPolicyException::class);
$this->expectExceptionMessage('Policy for `TestApp\Model\Entity\Article` has not been defined.');

$resolver->getPolicy(Article::class);
}

public function testGetPolicyMissing(): void
{
$resolver = new MapResolver();
Expand Down
31 changes: 31 additions & 0 deletions tests/TestCase/Policy/OrmResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use stdClass;
use TestApp\Model\Entity\Article;
use TestApp\Model\Entity\SubDir\Widget;
use TestApp\Model\Table\ArticlesTable;
use TestApp\Model\Table\SubDir\WidgetsTable;
use TestApp\Policy\ArticlePolicy;
use TestApp\Policy\ArticlesTablePolicy;
Expand Down Expand Up @@ -147,6 +148,36 @@ public function testGetPolicyUnknownTable(): void
$resolver->getPolicy($articles);
}

public function testGetPolicyFromEntityClassString(): void
{
$resolver = new OrmResolver('TestApp');
$policy = $resolver->getPolicy(Article::class);
$this->assertInstanceOf(ArticlePolicy::class, $policy);
}

public function testGetPolicyFromTableClassString(): void
{
$resolver = new OrmResolver('TestApp');
$policy = $resolver->getPolicy(ArticlesTable::class);
$this->assertInstanceOf(ArticlesTablePolicy::class, $policy);
}

public function testGetPolicyFromUnrelatedClassString(): void
{
$resolver = new OrmResolver('TestApp');

$this->expectException(MissingPolicyException::class);
$resolver->getPolicy(TestService::class);
}

public function testGetPolicyFromNonClassString(): void
{
$resolver = new OrmResolver('TestApp');

$this->expectException(MissingPolicyException::class);
$resolver->getPolicy('NotAClassName');
}

public function testGetPolicyViaDIC(): void
{
$container = new Container();
Expand Down
Loading