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

Bug: Commands discovery in custom namespaces #2840

Closed
michalsn opened this issue Apr 18, 2020 · 3 comments
Closed

Bug: Commands discovery in custom namespaces #2840

michalsn opened this issue Apr 18, 2020 · 3 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@michalsn
Copy link
Member

michalsn commented Apr 18, 2020

Direction
The order of namespace declaration in $psr4 array has an impact on a proper discovery of Commands when our package is located somewhere in APPPPATH.

Initially, I asked this question on the forum but it didn't gain any attention, so decided to post it here as a bug.

Describe the bug
The problem occurs when our module/package is placed somewhere in APPPATH with a custom namespace. In that case, we have to keep a "special" order in the $psr4 array in Autoload class. Otherwise, our Commands won't be discovered with a proper namespace and as a result - they won't be accessible at all.

So, here is an example with myth-auth package. This won't work:

$psr4 = [
    'App'         => APPPATH,                // To ensure filters, etc still found,
    APP_NAMESPACE => APPPATH,                // For custom namespace
    'Config'      => APPPATH . 'Config',
    'Myth\Auth'   => APPPATH . 'ThirdParty/myth-auth/src',
];

but this one will:

$psr4 = [
    'Myth\Auth'   => APPPATH . 'ThirdParty/myth-auth/src',
    'App'         => APPPATH,                // To ensure filters, etc still found,
    APP_NAMESPACE => APPPATH,                // For custom namespace
    'Config'      => APPPATH . 'Config',
];

CodeIgniter 4 version
4.0.2 and dev

Affected module(s)
It affects only Commands. Probably it has something to do with the way FileLocator::findQualifiedNameFromPath() is working.

Expected behavior, and steps to reproduce if appropriate
To a quick reproduction of this bug I used a myth-auth package. I declared a $psr4 array as follow:

$psr4 = [
    'App'         => APPPATH,                // To ensure filters, etc still found,
    APP_NAMESPACE => APPPATH,                // For custom namespace
    'Config'      => APPPATH . 'Config',
    'Myth\Auth'   => APPPATH . 'ThirdParty/myth-auth/src',
];

Then I tried to run a command: php spark list - but no new commands appeared. Also calling any command that supposes to be there directly fails.

Commands for this package should be correctly discovered regardless of the order in the $psr4 array.

@michalsn michalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Apr 18, 2020
@paulbalandan
Copy link
Member

I have encountered this problem before with FileLocator::findQualifiedNameFromPath(). As you have described, discovery of commands with custom app namespace failed, so I have decided to extend the class method.

The offending line is line 356:

return $className

The documentation for the method says that "Find the qualified name of a file according to the namespace of the first matched namespace path.". Since the path APPPATH has two defined namespace in the $psr4 array, App and APP_NAMESPACE, and App namespace is listed first, the class method will return the command name with namespace of App, even if the namespaced class does not exist.

In CodeIgniter\CLI\CommandRunner::createCommandList() line 167:

if (empty($className) || ! class_exists($className))
{
    continue;
}

since the class namespaced with App does not exist, the loop will not discover any of your custom namespaced commands.

My solution was to change line 356 of FileLocator by extending the method and changing the offending line to the following:

// check if the class exists
if (class_exists($className)) {
       return $className;
}

@michalsn
Copy link
Member Author

Yes, I think that would do it. Could you send a PR to fix this?

@paulbalandan
Copy link
Member

paulbalandan commented Apr 18, 2020

Okay I'll fork the repo and issue a PR.

lonnieezell added a commit that referenced this issue Apr 19, 2020
…aced-class-finder

Fixed Issue #2840 on discovery of classes by FileLocator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

3 participants