magento/magento2: Fixes for the schema cache.xsd #26933

andrewbess · 2020-02-19T14:54:21Z

Description (*)

This PR fixes the schema "cache.xsd" from Magento\Framework\Cache.

The attribute "instance" has changed on required because if a developer won't be to use it new cache type will not be created;
The parameters "label" and "description" have changed on required because of the class "Magento\Framework\App\Cache\TypeList" gets theirs without checking for existing. Please take a look at this code;
Checking on unique of the type name has added;
The configuration reader class has fixed for more informative during to run CLI commands;
The class SchemaLocator has been fixed to check the cache configuration during configuration merging;
The unnecessary checking has removed from the class TypeList;
Unit tests have been covered.

Fixed Issues (if relevant)

Fixes: #26224

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)

m2-assistant · 2020-02-19T14:54:30Z

Hi @andrewbess. 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

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

magento-deployment-service · 2020-02-19T14:55:21Z

Hi @m2-assistant[bot] Here is your new Magento Instance: https://pr-26933.magento-testing-service.engineering
Admin access: https://pr-26933.magento-testing-service.engineering/admin_9c20
Login: 6a81d57b , Password: 34181a6d3f4e

swnsma

Hi @andrewbess!
Thank you for your contribution!
Could you please add unit tests to check new schema validation?
Maybe, we also should refactor the class \Magento\Framework\App\Cache\TypeList as 'instance' will become a required parameter? We may simplify some checks here.

ihor-sviziev

Please see following review details: #26933 (review)

andrewbess · 2020-03-14T09:51:51Z

Hi @andrewbess!
Thank you for your contribution!
Could you please add unit tests to check new schema validation?
Maybe, we also should refactor the class \Magento\Framework\App\Cache\TypeList as 'instance' will become a required parameter? We may simplify some checks here.

Hello @swnsma
Thank you for review.
I added the changes.
There are additional changes for schema, tests, additional changes for TypeList in the PR.
Also, I have changed description of PR.
Please recheck.
Thank you in advance.

rogyar · 2020-03-14T15:58:12Z

lib/internal/Magento/Framework/Cache/Config/Reader.php

+    {
+        try {
+            return parent::read($scope);
+        } catch (\Magento\Framework\Exception\LocalizedException $e) {


May I ask you why do we need to transform this localized exception and throw once again?

Thank you

Hello @rogyar.
Thank you for your remark.
Previously, I have added the render-method that prepares an error message to output error messages correctly.
But it caused other problems.
I have removed new methods.
Please recheck.
Thank you in advance.

swnsma

Please check my comment.
Would you be so kind to check strict types declaration, type hints and add 'declare(strict_types=1);' to changed files.
Thank you!

swnsma · 2020-03-14T20:44:52Z

lib/internal/Magento/Framework/Cache/Test/Unit/XsdTest.php

+ */
+namespace Magento\Framework\Cache\Test\Unit;
+
+class XsdTest extends \PHPUnit\Framework\TestCase


Hi @andrewbess,

I have discovered that we already have bunch of tests which test *.xsd files.
They are located in Magento\Test\Integrity namespace.
Could I ask you to move your your tests under this namespace to keep consistency in tests structure?
Please check \Magento\Test\Integrity\Magento\Widget\WidgetConfigTest for example.

Hello @swnsma.
I have moved tests by the suggested path.
Also, I added declaration strict types where it is required.
Please recheck.
Thank you in advance.

swnsma

Hi @andrewbess!
Thank you for your contribution!
Could you please check my notices?

dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/ConfigTest.php

swnsma · 2020-03-16T09:02:05Z

dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/ConfigTest.php

+    /**
+     * @var \Magento\Framework\TestFramework\Unit\Utility\XsdValidator
+     */
+    protected $_xsdValidator;


Please
protected -> private
$_xsdValidator -> $xsdValidator

dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/ConfigTest.php

lib/internal/Magento/Framework/Cache/Exception/WrongConfigurationException.php

lib/internal/Magento/Framework/App/Test/Unit/Cache/TypeListTest/Type.php

…ess/magento2 into improvement/cache-schema � Conflicts: � lib/internal/Magento/Framework/App/Test/Unit/Cache/TypeListTest.php

andrewbess · 2020-03-17T19:49:04Z

Closed in favor of #27307

m2-assistant · 2020-03-17T19:49:07Z

Hi @andrewbess, 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.

magento/magento2: Fixes for the schema cache.xsd

6114fa1

magento-engcom-team added Component: Cache Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Feb 19, 2020

andrewbess requested a review from rogyar February 20, 2020 16:51

swnsma self-requested a review February 20, 2020 18:46

swnsma requested changes Feb 20, 2020

View reviewed changes

swnsma self-assigned this Feb 20, 2020

eduard13 mentioned this pull request Feb 28, 2020

[Framework] Fixing the caches that don't have no instance configured #26236

Closed

4 tasks

ghost added the Progress: pending review label Mar 4, 2020

ihor-sviziev requested changes Mar 4, 2020

View reviewed changes

ghost assigned ihor-sviziev Mar 4, 2020

ghost added Progress: needs update and removed Progress: pending review labels Mar 4, 2020

ihor-sviziev removed their assignment Mar 4, 2020

andrewbess force-pushed the improvement/cache-schema branch 4 times, most recently from 641b94e to 1b7d5f9 Compare March 13, 2020 14:49

andrewbess requested review from swnsma and ihor-sviziev March 14, 2020 09:52

rogyar reviewed Mar 14, 2020

View reviewed changes

ghost assigned rogyar Mar 14, 2020

swnsma requested changes Mar 14, 2020

View reviewed changes

andrewbess force-pushed the improvement/cache-schema branch from 1b7d5f9 to 56b1391 Compare March 15, 2020 09:32

andrewbess requested a review from rogyar March 15, 2020 11:22

andrewbess requested a review from swnsma March 15, 2020 11:22

andrewbess force-pushed the improvement/cache-schema branch 2 times, most recently from dfe05e5 to dcf42e9 Compare March 16, 2020 08:10

swnsma requested changes Mar 16, 2020

View reviewed changes

magento/magento2: Additional fixes for the cache configuration

1f8f2a0

andrewbess force-pushed the improvement/cache-schema branch from dcf42e9 to 1f8f2a0 Compare March 16, 2020 09:25

swnsma and others added 4 commits March 16, 2020 11:49

Merge branch '2.4-develop' into improvement/cache-schema

4e1555d

Merge branch '2.4-develop' into improvement/cache-schema

9b55ead

magento/magento2: Additional fixes for the cache configuration

677fe49

Merge branch 'improvement/cache-schema' of https://github.com/andrewb…

3f76bdd

…ess/magento2 into improvement/cache-schema � Conflicts: � lib/internal/Magento/Framework/App/Test/Unit/Cache/TypeListTest.php

andrewbess closed this Mar 17, 2020

andrewbess deleted the improvement/cache-schema branch March 17, 2020 19:49

magento/magento2: Fixes for the schema cache.xsd #26933

magento/magento2: Fixes for the schema cache.xsd #26933

Uh oh!

Conversation

andrewbess commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Feb 19, 2020

Uh oh!

magento-deployment-service bot commented Feb 19, 2020

Uh oh!

swnsma left a comment

Choose a reason for hiding this comment

Uh oh!

ihor-sviziev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewbess commented Mar 14, 2020

Uh oh!

rogyar Mar 14, 2020

Choose a reason for hiding this comment

Uh oh!

andrewbess Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swnsma left a comment

Choose a reason for hiding this comment

Uh oh!

swnsma Mar 14, 2020

Choose a reason for hiding this comment

Uh oh!

andrewbess Mar 15, 2020

Choose a reason for hiding this comment

Uh oh!

swnsma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swnsma Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewbess commented Mar 17, 2020

Uh oh!

m2-assistant bot commented Mar 17, 2020

Uh oh!

Uh oh!

andrewbess commented Feb 19, 2020 •

edited

Loading

ihor-sviziev left a comment •

edited

Loading

andrewbess Mar 15, 2020 •

edited

Loading