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

Exception running AclExtras aco_sync with abstract controller classes, v0.2.2 #110

Closed
mikeu opened this issue Dec 2, 2016 · 1 comment
Closed
Milestone

Comments

@mikeu
Copy link
Contributor

mikeu commented Dec 2, 2016

I have an abstract controller class (named other than "AppController"), and this leads to an exception when running bin/cake acl_extras aco_sync:

Exception: Cannot instantiate abstract class App\Controller\MyAbstractController in
[/apppath/vendor/cakephp/acl/src/AclExtras.php, line 433]

It looks as a closely related issue was addressed almost two years ago in 6106e51, but then that fix was reverted several days later, in 655b1e3.

I'm not clear if there's a good reason the original fix was reverted, such as undesirable behaviour elsewhere in the plugin, or if it was a regression. Or am I just using the plugin incorrectly, e.g. missing a configuration option or something, and my abstract controller shouldn't be a problem?

It looks as if the problem, at least as I'm experiencing it, can be solved by ignoring abstract controller classes entirely. For example, by checking whether $controller is abstract in _updateControllers and skipping over it when it is, at the same point that a $controllerName of 'App' is skipped. Something like this in:

// existing check
if ($controllerName == 'App') {
    continue;
}
// new check
$namespace = $this->_getNamespace($controller, $pluginPath, $prefix);
if ((new \ReflectionClass($namespace))->isAbstract()) {
    continue;
}

If this is a bug and that fix seems appropriate, then I can submit a PR for it.

@markstory markstory added this to the 1.0 milestone Dec 3, 2016
@markstory
Copy link
Member

A pull request to ignore abstract controllers sounds like a good idea. I don't remember why that commit was reverted, to be honest.

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