AppState emulateAreaCode was not respected by file collector

[johan-lindahl]

Description (*)

The "hack" to emulate an area code via AppState->emulateAreaCode() is not respected by the base file collector. The consequence is that *.xml file in the emulated area is not found by the file collector.

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

The problem occurs in an extension I have built that require to access the adminhtml area from the graphql area. There are likely other modules in the Magento 2 that suffers from the same problem. However I can not point one such module without spending a considerable amount of time searching for it.

The following snippet suffer from this bug,

$data = $this->app_state->emulateAreaCode(   
  \Magento\Framework\App\Area::AREA_ADMINHTML,   
  [$this, "getDataSourceData"],  
  [$field, $args]
);  

Questions or comments

I thinks someone more knowledgable should look into this issue. I have troubleshooted and my conclusions are as follows.

The base file collector is accessing the area code of the Theme with a call to getData('area').
The Theme has an override called getArea(), this method is never called by the base file collector.
This causes problems for modules that use "emulated area codes" via the
\Magento\Framework\App\State->emulateAreaCode() method.
The emulated area code is then not respected by the file collector since it access the data directly and not via the intended getArea() method in the Theme.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

Resolved issues:

1. resolves [Issue] AppState emulateAreaCode was not respected by file collector #29656: AppState emulateAreaCode was not respected by file collector

[m2-assistant]

Hi @johan-lindahl. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

[johan-lindahl]

@magento run all tests

[johan-lindahl]

@magento run all tests

[sidolov]

@magento create issue

[gwharton]

I dont know if it is related, but I when running bin/magento console apps that emulate AREA_FRONTEND, the /etc/view.xml file is not read from your theme when you attempt to do anything theme related, e.g simply getting the viewConfig item results in a merged object of all enabled modules etc/view.xml but the content from the themes view.xml is missing.

[magento-engcom-team]

Hi @lenaorobei, thank you for the review.
ENGCOM-8130 has been created to process this Pull Request
✳️ @lenaorobei, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[engcom-Bravo]

Dev experience is required for testing of this PR. Please note that Manual testing has not been performed.

[engcom-Charlie]

✔️ QA Passed
Checked by the following steps:

1. Create simple CLI command.
2. Create \Magento\Framework\View\File\Collector\Base with param ['subDir' => 'ui_component'].

$baseFileCollector = $objectManager->create(Base::class, ['subDir' => 'ui_component']);

3. Emulates callback inside adminhtml area.

$adminAreaEmulate = $objectManager->create(EmulatedAdminhtmlAreaProcessor::class);
$adminAreaEmulate->process([$baseFileCollector, 'getFiles'], [$theme, '*']);

Before:
As result, we have files only from base area.
view/base/ui_component/*

After
As result, we have files from base and adminhtml areas.
view/base/ui_component/*
view/adminhtml/ui_component/*

[m2-assistant]

Hi @johan-lindahl, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[richjoregonstate]

@magento-engcom-team @johan-lindahl @lenaorobei

It looks like this fix has a few more consequences then intended. My team just finished an upgrade from 2.3.5 -> 2.4.3 (enterprise) and we now get an "Area code not set" any time we run a cli command. Upon investigation we found that the change from $area = $theme->getData('area'); to $area = $theme->getArea(); was the root of the issue.

The large difference between the two is that $theme->getData('area'); can return null while $theme->getArea(); can't. Instead it throws "Area code not set". When $theme->getData('area'); returns null, in 2.4.2 and earlier, nothing happens as the file path it looks at doesn't exist* and the $result is unaffected. However in 2.4.3 this behavior is no longer preserved.

I do believe that the switch to $theme->getArea(); is the right move, however in order to preserve the original behavior of the function the second collectFilesWithContext block should be wrapped in a try-catch block, that throws a warning instead of erroring out completely. A-la:

try {
    $area = $theme->getArea();
    $themeFiles = $this->componentDirSearch->collectFilesWithContext(
        ComponentRegistrar::MODULE,
        "view/{$area}/{$this->subDir}{$filePath}"
    );
    foreach ($themeFiles as $file) {
        $result[] = $this->fileFactory->create($file->getFullPath(), $file->getComponentName());
    }
}
catch (\Magento\Framework\Exception\LocalizedException $e ) {
   ...  // Warning area code not set
} // Otherwise fail normally 

*The file path looks like "view//{$this->subDir}{$filePath}" which is invalid due to "//".

[tuanpt-0634]

Thanks @richjoregonstate for reporting

We are upgrading from 2.4.1 to 2.4.3 and facing this issue too.

[hostep]

@richjoregonstate: would you be so kind to send in a PR with your suggestion so it can be followed up properly, otherwise your comment might be forgotten, thanks! 🙂

[devetus]

We are experiencing the same issue "Area code is not set" while updating modules during the upgrade from 2.3.6->2.4.3-p1

[hostep]

@devetus: I'm going to guess that your problem might get fixed by: #33726

[devetus]

@hostep thanks, that worked like a charm in my case, without extra try-catch block.