From 97bd66e984895f12c8f17b7f401ebf352a28805c Mon Sep 17 00:00:00 2001 From: Andrii Beziazychnyi Date: Tue, 17 Mar 2020 10:22:44 +0200 Subject: [PATCH] magento/magento2: fixes for the cache configuration schema --- .../Magento/Framework/Cache/ConfigTest.php | 89 ++++++++++++++ .../_files/invalidCacheConfigXmlArray.php | 52 ++++++++ .../Cache/_files/valid_cache_config.xml | 17 +++ .../Magento/Framework/App/Cache/TypeList.php | 9 +- .../App/Test/Unit/Cache/TypeListTest.php | 116 ++++++++++-------- .../Magento/Framework/Cache/Config/Reader.php | 31 +++-- .../Framework/Cache/Config/SchemaLocator.php | 12 +- .../Cache/Test/Unit/Config/ConverterTest.php | 30 ----- .../Test/Unit/Config/_files/cache_config.php | 23 ---- .../Test/Unit/Config/_files/cache_config.xml | 17 --- .../Magento/Framework/Cache/etc/cache.xsd | 40 +++--- 11 files changed, 286 insertions(+), 150 deletions(-) create mode 100644 dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/ConfigTest.php create mode 100644 dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/invalidCacheConfigXmlArray.php create mode 100644 dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/valid_cache_config.xml delete mode 100644 lib/internal/Magento/Framework/Cache/Test/Unit/Config/ConverterTest.php delete mode 100644 lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.php delete mode 100644 lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.xml diff --git a/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/ConfigTest.php b/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/ConfigTest.php new file mode 100644 index 0000000000000..2d4a1dc0c2ce3 --- /dev/null +++ b/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/ConfigTest.php @@ -0,0 +1,89 @@ +markTestSkipped('Skipped on HHVM. Will be fixed in MAGETWO-45033'); + } + $this->urnResolver = new UrnResolver(); + $this->xsdSchema = $this->urnResolver->getRealPath( + 'urn:magento:framework:Cache/etc/cache.xsd' + ); + $this->xsdValidator = new XsdValidator(); + } + + /** + * Tests invalid configurations + * + * @param string $xmlString + * @param array $expectedError + * @dataProvider schemaCorrectlyIdentifiesInvalidXmlDataProvider + */ + public function testSchemaCorrectlyIdentifiesInvalidXml( + string $xmlString, + array $expectedError + ): void { + $actualError = $this->xsdValidator->validate( + $this->xsdSchema, + $xmlString + ); + $this->assertEquals($expectedError, $actualError); + } + + /** + * Tests valid configurations + */ + public function testSchemaCorrectlyIdentifiesValidXml(): void + { + $xmlString = file_get_contents(__DIR__ . '/_files/valid_cache_config.xml'); + $actualResult = $this->xsdValidator->validate( + $this->xsdSchema, + $xmlString + ); + + $this->assertEmpty($actualResult); + } + + /** + * Data provider with invalid xml array according to cache.xsd + */ + public function schemaCorrectlyIdentifiesInvalidXmlDataProvider(): array + { + return include __DIR__ . '/_files/invalidCacheConfigXmlArray.php'; + } +} diff --git a/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/invalidCacheConfigXmlArray.php b/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/invalidCacheConfigXmlArray.php new file mode 100644 index 0000000000000..8d2d631334a9b --- /dev/null +++ b/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/invalidCacheConfigXmlArray.php @@ -0,0 +1,52 @@ + [ + '', + ["Element 'config': Missing child element(s). Expected is ( type ).\nLine: 1\n"], + ], + 'cache_config_with_notallowed_attribute' => [ + '' . + '' . + 'Test', + ["Element 'type', attribute 'notallowed': The attribute 'notallowed' is not allowed.\nLine: 1\n"], + ], + 'cache_config_without_name_attribute' => [ + '' . + 'Test', + ["Element 'type': The attribute 'name' is required but missing.\nLine: 1\n"], + ], + 'cache_config_without_instance_attribute' => [ + '' . + 'Test', + ["Element 'type': The attribute 'instance' is required but missing.\nLine: 1\n"], + ], + 'cache_config_without_label_element' => [ + '' . + 'Test', + ["Element 'type': Missing child element(s). Expected is ( label ).\nLine: 1\n"], + ], + 'cache_config_without_description_element' => [ + '' . + '', + ["Element 'type': Missing child element(s). Expected is ( description ).\nLine: 1\n"], + ], + 'cache_config_without_child_elements' => [ + '' . + '', + ["Element 'type': Missing child element(s). Expected is one of ( label, description ).\nLine: 1\n"], + ], + 'cache_config_cache_name_not_unique' => [ + '' . + 'Test1' . + '' . + 'Test2', + [ + "Element 'type': Duplicate key-sequence ['test'] in unique identity-constraint" + . " 'uniqueCacheName'.\nLine: 1\n" + ], + ], +]; diff --git a/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/valid_cache_config.xml b/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/valid_cache_config.xml new file mode 100644 index 0000000000000..ef45c083daf0d --- /dev/null +++ b/dev/tests/static/testsuite/Magento/Test/Integrity/Magento/Framework/Cache/_files/valid_cache_config.xml @@ -0,0 +1,17 @@ + + + + + + Type1 + + + + Type2 + + diff --git a/lib/internal/Magento/Framework/App/Cache/TypeList.php b/lib/internal/Magento/Framework/App/Cache/TypeList.php index b695ee3a37fa8..c0790c4d40ad4 100644 --- a/lib/internal/Magento/Framework/App/Cache/TypeList.php +++ b/lib/internal/Magento/Framework/App/Cache/TypeList.php @@ -8,6 +8,9 @@ use Magento\Framework\App\ObjectManager; use Magento\Framework\Serialize\SerializerInterface; +/** + * Application cache type list + */ class TypeList implements TypeListInterface { const INVALIDATED_TYPES = 'core_cache_invalidate'; @@ -68,9 +71,7 @@ public function __construct( protected function _getTypeInstance($type) { $config = $this->_config->getType($type); - if (!isset($config['instance'])) { - return null; - } + return $this->_factory->get($config['instance']); } @@ -132,7 +133,7 @@ public function getTypes() } /** - * {@inheritdoc} + * @inheritdoc */ public function getTypeLabels() { diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Cache/TypeListTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Cache/TypeListTest.php index 8d9b297d7dded..02c9a872fe97f 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Cache/TypeListTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Cache/TypeListTest.php @@ -6,21 +6,30 @@ namespace Magento\Framework\App\Test\Unit\Cache; -use \Magento\Framework\App\Cache\TypeList; +use Magento\Framework\App\Cache\InstanceFactory; +use Magento\Framework\App\Cache\StateInterface; +use Magento\Framework\App\Cache\TypeList; +use Magento\Framework\App\CacheInterface; +use Magento\Framework\Cache\Frontend\Decorator\TagScope; +use Magento\Framework\Cache\ConfigInterface; +use Magento\Framework\DataObject; use Magento\Framework\Serialize\SerializerInterface; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use PHPUnit\Framework\TestCase; +use PHPUnit\Framework\MockObject\MockObject; /** * Test class for \Magento\Framework\App\Cache\TypeList */ -class TypeListTest extends \PHPUnit\Framework\TestCase +class TypeListTest extends TestCase { /** - * @var \Magento\Framework\App\Cache\TypeList + * @var TypeList */ protected $_typeList; /** - * @var \Magento\Framework\App\CacheInterface|\PHPUnit_Framework_MockObject_MockObject + * @var CacheInterface|MockObject */ protected $_cache; @@ -30,7 +39,7 @@ class TypeListTest extends \PHPUnit\Framework\TestCase protected $_typesArray; /** - * @var \Magento\Framework\Cache\ConfigInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ConfigInterface|MockObject */ protected $_config; @@ -47,10 +56,10 @@ class TypeListTest extends \PHPUnit\Framework\TestCase /** * Expected cache type */ - const CACHE_TYPE = \Magento\Framework\Cache\FrontendInterface::class; + const CACHE_TYPE = TagScope::class; /** - * @var SerializerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var SerializerInterface|MockObject */ private $serializerMock; @@ -58,33 +67,44 @@ protected function setUp() { $this->_typesArray = [ self::TYPE_KEY => [ + 'name' => self::TYPE_KEY, + 'instance' => self::CACHE_TYPE, 'label' => 'Type Label', 'description' => 'Type Description', ], ]; - $this->_config = - $this->createPartialMock(\Magento\Framework\Cache\ConfigInterface::class, ['getTypes', 'getType']); - $this->_config->expects($this->any())->method('getTypes')->will($this->returnValue($this->_typesArray)); + $this->_config = $this->createPartialMock( + ConfigInterface::class, + ['getTypes', 'getType'] + ); + $this->_config->expects($this->any())->method('getTypes') + ->will($this->returnValue($this->_typesArray)); + $this->_config->expects($this->any())->method('getType') + ->with(self::TYPE_KEY) + ->will($this->returnValue($this->_typesArray[self::TYPE_KEY])); $cacheState = $this->createPartialMock( - \Magento\Framework\App\Cache\StateInterface::class, + StateInterface::class, ['isEnabled', 'setEnabled', 'persist'] ); - $cacheState->expects($this->any())->method('isEnabled')->will($this->returnValue(self::IS_CACHE_ENABLED)); - $cacheBlockMock = $this->createMock(self::CACHE_TYPE); - $factory = $this->createPartialMock(\Magento\Framework\App\Cache\InstanceFactory::class, ['get']); - $factory->expects($this->any())->method('get')->with(self::CACHE_TYPE)->will( - $this->returnValue($cacheBlockMock) - ); + $cacheState->expects($this->any())->method('isEnabled') + ->will($this->returnValue(self::IS_CACHE_ENABLED)); + $cacheTypeMock = $this->createMock(self::CACHE_TYPE); + $cacheTypeMock->expects($this->any())->method('getTag') + ->will($this->returnValue('TEST')); + $factory = $this->createPartialMock(InstanceFactory::class, ['get']); + $factory->expects($this->any())->method('get') + ->with(self::CACHE_TYPE) + ->will($this->returnValue($cacheTypeMock)); $this->_cache = $this->createPartialMock( - \Magento\Framework\App\CacheInterface::class, + CacheInterface::class, ['load', 'getFrontend', 'save', 'remove', 'clean'] ); $this->serializerMock = $this->createMock(SerializerInterface::class); - $objectHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $objectHelper = new ObjectManager($this); $this->_typeList = $objectHelper->getObject( - \Magento\Framework\App\Cache\TypeList::class, + TypeList::class, [ 'config' => $this->_config, 'cacheState' => $cacheState, @@ -114,9 +134,9 @@ public function testGetTypeLabels() public function testGetInvalidated() { $expectation = [self::TYPE_KEY => $this->_getPreparedType()]; - $this->_cache->expects($this->once())->method('load')->with(TypeList::INVALIDATED_TYPES)->will( - $this->returnValue('serializedData') - ); + $this->_cache->expects($this->once())->method('load') + ->with(TypeList::INVALIDATED_TYPES) + ->will($this->returnValue('serializedData')); $this->serializerMock->expects($this->once()) ->method('unserialize') ->with('serializedData') @@ -127,9 +147,9 @@ public function testGetInvalidated() public function testInvalidate() { // there are no invalidated types - $this->_cache->expects($this->once())->method('load')->with(TypeList::INVALIDATED_TYPES)->will( - $this->returnValue([]) - ); + $this->_cache->expects($this->once())->method('load') + ->with(TypeList::INVALIDATED_TYPES) + ->will($this->returnValue([])); $expectedInvalidated = [ self::TYPE_KEY => 1, ]; @@ -137,18 +157,16 @@ public function testInvalidate() ->method('serialize') ->with($expectedInvalidated) ->willReturn('serializedData'); - $this->_cache->expects($this->once())->method('save')->with( - 'serializedData', - TypeList::INVALIDATED_TYPES - ); + $this->_cache->expects($this->once())->method('save') + ->with('serializedData', TypeList::INVALIDATED_TYPES); $this->_typeList->invalidate(self::TYPE_KEY); } public function testInvalidateList() { - $this->_cache->expects($this->once())->method('load')->with(TypeList::INVALIDATED_TYPES)->will( - $this->returnValue([]) - ); + $this->_cache->expects($this->once())->method('load') + ->with(TypeList::INVALIDATED_TYPES) + ->will($this->returnValue([])); $expectedInvalidated = [ self::TYPE_KEY => 1, ]; @@ -156,10 +174,8 @@ public function testInvalidateList() ->method('serialize') ->with($expectedInvalidated) ->willReturn('serializedData'); - $this->_cache->expects($this->once())->method('save')->with( - 'serializedData', - TypeList::INVALIDATED_TYPES - ); + $this->_cache->expects($this->once())->method('save') + ->with('serializedData', TypeList::INVALIDATED_TYPES); $this->_typeList->invalidate([self::TYPE_KEY]); } @@ -169,38 +185,36 @@ public function testCleanType() ->method('unserialize') ->with('serializedData') ->willReturn($this->_typesArray); - $this->_cache->expects($this->once())->method('load')->with(TypeList::INVALIDATED_TYPES)->will( - $this->returnValue('serializedData') - ); - $this->_config->expects($this->once())->method('getType')->with(self::TYPE_KEY)->will( - $this->returnValue(['instance' => self::CACHE_TYPE]) - ); + $this->_cache->expects($this->once())->method('load') + ->with(TypeList::INVALIDATED_TYPES) + ->will($this->returnValue('serializedData')); + $this->_config->expects($this->once())->method('getType') + ->with(self::TYPE_KEY) + ->will($this->returnValue(['instance' => self::CACHE_TYPE])); unset($this->_typesArray[self::TYPE_KEY]); $this->serializerMock->expects($this->once()) ->method('serialize') ->with($this->_typesArray) ->willReturn('serializedData'); - $this->_cache->expects($this->once())->method('save')->with( - 'serializedData', - TypeList::INVALIDATED_TYPES - ); + $this->_cache->expects($this->once())->method('save') + ->with('serializedData', TypeList::INVALIDATED_TYPES); $this->_typeList->cleanType(self::TYPE_KEY); } /** * Returns prepared type * - * @return \Magento\Framework\DataObject + * @return DataObject */ private function _getPreparedType() { - return new \Magento\Framework\DataObject( + return new DataObject( [ 'id' => self::TYPE_KEY, 'cache_type' => $this->_typesArray[self::TYPE_KEY]['label'], 'description' => $this->_typesArray[self::TYPE_KEY]['description'], - 'tags' => '', - 'status' => self::IS_CACHE_ENABLED, + 'tags' => 'TEST', + 'status' => (int)self::IS_CACHE_ENABLED, ] ); } diff --git a/lib/internal/Magento/Framework/Cache/Config/Reader.php b/lib/internal/Magento/Framework/Cache/Config/Reader.php index 445e91240e7e5..942a3931e0173 100644 --- a/lib/internal/Magento/Framework/Cache/Config/Reader.php +++ b/lib/internal/Magento/Framework/Cache/Config/Reader.php @@ -3,9 +3,19 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Framework\Cache\Config; -class Reader extends \Magento\Framework\Config\Reader\Filesystem +use Magento\Framework\App\Area; +use Magento\Framework\Config\Dom; +use Magento\Framework\Config\FileResolverInterface; +use Magento\Framework\Config\Reader\Filesystem; +use Magento\Framework\Config\ValidationStateInterface; + +/** + * Cache configuration reader + */ +class Reader extends Filesystem { /** * List of id attributes for merge @@ -15,24 +25,27 @@ class Reader extends \Magento\Framework\Config\Reader\Filesystem protected $_idAttributes = ['/config/type' => 'name']; /** - * @param \Magento\Framework\Config\FileResolverInterface $fileResolver + * Initialize dependencies. + * + * @param FileResolverInterface $fileResolver * @param Converter $converter * @param SchemaLocator $schemaLocator - * @param \Magento\Framework\Config\ValidationStateInterface $validationState + * @param ValidationStateInterface $validationState * @param string $fileName * @param array $idAttributes * @param string $domDocumentClass * @param string $defaultScope + * phpcs:disable Generic.CodeAnalysis.UselessOverridingMethod */ public function __construct( - \Magento\Framework\Config\FileResolverInterface $fileResolver, - \Magento\Framework\Cache\Config\Converter $converter, - \Magento\Framework\Cache\Config\SchemaLocator $schemaLocator, - \Magento\Framework\Config\ValidationStateInterface $validationState, + FileResolverInterface $fileResolver, + Converter $converter, + SchemaLocator $schemaLocator, + ValidationStateInterface $validationState, $fileName = 'cache.xml', $idAttributes = [], - $domDocumentClass = \Magento\Framework\Config\Dom::class, - $defaultScope = 'global' + $domDocumentClass = Dom::class, + $defaultScope = Area::AREA_GLOBAL ) { parent::__construct( $fileResolver, diff --git a/lib/internal/Magento/Framework/Cache/Config/SchemaLocator.php b/lib/internal/Magento/Framework/Cache/Config/SchemaLocator.php index 5471dbcfb6c62..2d3be1b1a4067 100644 --- a/lib/internal/Magento/Framework/Cache/Config/SchemaLocator.php +++ b/lib/internal/Magento/Framework/Cache/Config/SchemaLocator.php @@ -7,6 +7,9 @@ */ namespace Magento\Framework\Cache\Config; +/** + * Cache configuration schema locator + */ class SchemaLocator implements \Magento\Framework\Config\SchemaLocatorInterface { /** @@ -15,6 +18,9 @@ class SchemaLocator implements \Magento\Framework\Config\SchemaLocatorInterface protected $urnResolver; /** + * Initialize dependencies. + * + * @param \Magento\Framework\Config\Dom\UrnResolver $urnResolver */ public function __construct(\Magento\Framework\Config\Dom\UrnResolver $urnResolver) { @@ -25,6 +31,7 @@ public function __construct(\Magento\Framework\Config\Dom\UrnResolver $urnResolv * Get path to merged config schema * * @return string|null + * @throws \Magento\Framework\Exception\NotFoundException */ public function getSchema() { @@ -34,10 +41,11 @@ public function getSchema() /** * Get path to pre file validation schema * - * @return null + * @return string|null + * @throws \Magento\Framework\Exception\NotFoundException */ public function getPerFileSchema() { - return null; + return $this->urnResolver->getRealPath('urn:magento:framework:Cache/etc/cache.xsd'); } } diff --git a/lib/internal/Magento/Framework/Cache/Test/Unit/Config/ConverterTest.php b/lib/internal/Magento/Framework/Cache/Test/Unit/Config/ConverterTest.php deleted file mode 100644 index 7f86e162311c8..0000000000000 --- a/lib/internal/Magento/Framework/Cache/Test/Unit/Config/ConverterTest.php +++ /dev/null @@ -1,30 +0,0 @@ -_model = new \Magento\Framework\Cache\Config\Converter(); - } - - public function testConvert() - { - $dom = new \DOMDocument(); - $xmlFile = __DIR__ . '/_files/cache_config.xml'; - $dom->loadXML(file_get_contents($xmlFile)); - - $convertedFile = __DIR__ . '/_files/cache_config.php'; - $expectedResult = include $convertedFile; - $this->assertEquals($expectedResult, $this->_model->convert($dom)); - } -} diff --git a/lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.php b/lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.php deleted file mode 100644 index 0a45e50bbe198..0000000000000 --- a/lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.php +++ /dev/null @@ -1,23 +0,0 @@ - [ - 'config' => [ - 'name' => 'config', - 'translate' => 'label,description', - 'instance' => \Magento\Framework\App\Cache\Type\Config::class, - 'label' => 'Configuration', - 'description' => 'Cache Description', - ], - 'layout' => [ - 'name' => 'layout', - 'translate' => 'label,description', - 'instance' => \Magento\Framework\App\Cache\Type\Layout::class, - 'label' => 'Layouts', - 'description' => 'Layout building instructions', - ], - ] -]; diff --git a/lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.xml b/lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.xml deleted file mode 100644 index 315ddd9cec67c..0000000000000 --- a/lib/internal/Magento/Framework/Cache/Test/Unit/Config/_files/cache_config.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - Cache Description - - - - Layout building instructions - - diff --git a/lib/internal/Magento/Framework/Cache/etc/cache.xsd b/lib/internal/Magento/Framework/Cache/etc/cache.xsd index 74b831bb6ac03..d997e295140f5 100644 --- a/lib/internal/Magento/Framework/Cache/etc/cache.xsd +++ b/lib/internal/Magento/Framework/Cache/etc/cache.xsd @@ -6,24 +6,36 @@ */ --> - - - - - - - - - Cache type declaration + + Cache type declaration + - - - - + + + + - + + + + + + + + Cache name must be unique. + + + + + + + + + + +