From 66dda072e5f4c605905b5955ad016d6f9e39406f Mon Sep 17 00:00:00 2001 From: xjm <xjm@65776.no-reply.drupal.org> Date: Sun, 4 Feb 2018 19:08:34 -0500 Subject: [PATCH] Revert "Issue #2893117 by Mile23, Berdir: Improve HTML caching of Simpletest UI test form" This reverts commit c8dbe3a93657625a9d3e1798092023804b3cb64e. --- core/modules/simpletest/simpletest.module | 4 + .../simpletest/simpletest.services.yml | 7 +- .../Context/TestDiscoveryCacheContext.php | 94 ------------------- .../src/Form/SimpletestTestForm.php | 4 - core/modules/simpletest/src/TestDiscovery.php | 24 +++-- .../Context/TestDiscoveryCacheContextTest.php | 53 ----------- 6 files changed, 20 insertions(+), 166 deletions(-) delete mode 100644 core/modules/simpletest/src/Cache/Context/TestDiscoveryCacheContext.php delete mode 100644 core/modules/simpletest/tests/src/Kernel/Cache/Context/TestDiscoveryCacheContextTest.php diff --git a/core/modules/simpletest/simpletest.module b/core/modules/simpletest/simpletest.module index 6670e5acc0be..0e0ea2afd400 100644 --- a/core/modules/simpletest/simpletest.module +++ b/core/modules/simpletest/simpletest.module @@ -661,6 +661,10 @@ function simpletest_clean_environment() { else { drupal_set_message(t('Clear results is disabled and the test results table will not be cleared.'), 'warning'); } + + // Detect test classes that have been added, renamed or deleted. + \Drupal::cache()->delete('simpletest'); + \Drupal::cache()->delete('simpletest_phpunit'); } /** diff --git a/core/modules/simpletest/simpletest.services.yml b/core/modules/simpletest/simpletest.services.yml index 326cf38f2d37..8b645deb246a 100644 --- a/core/modules/simpletest/simpletest.services.yml +++ b/core/modules/simpletest/simpletest.services.yml @@ -1,9 +1,4 @@ services: test_discovery: class: Drupal\simpletest\TestDiscovery - arguments: ['@app.root', '@class_loader', '@module_handler'] - cache_context.test_discovery: - class: Drupal\simpletest\Cache\Context\TestDiscoveryCacheContext - arguments: ['@test_discovery', '@private_key'] - tags: - - { name: cache.context} + arguments: ['@app.root', '@class_loader', '@module_handler', '@?cache.discovery'] diff --git a/core/modules/simpletest/src/Cache/Context/TestDiscoveryCacheContext.php b/core/modules/simpletest/src/Cache/Context/TestDiscoveryCacheContext.php deleted file mode 100644 index e3d5cf351d62..000000000000 --- a/core/modules/simpletest/src/Cache/Context/TestDiscoveryCacheContext.php +++ /dev/null @@ -1,94 +0,0 @@ -<?php - -namespace Drupal\simpletest\Cache\Context; - -use Drupal\Core\Cache\CacheableMetadata; -use Drupal\Core\Cache\Context\CacheContextInterface; -use Drupal\Core\PrivateKey; -use Drupal\Core\Site\Settings; -use Drupal\simpletest\TestDiscovery; - -/** - * Defines the TestDiscoveryCacheContext service. - * - * Cache context ID: 'test_discovery'. - */ -class TestDiscoveryCacheContext implements CacheContextInterface { - - /** - * The test discovery service. - * - * @var \Drupal\simpletest\TestDiscovery - */ - protected $testDiscovery; - - /** - * The private key service. - * - * @var \Drupal\Core\PrivateKey - */ - protected $privateKey; - - /** - * The hash of discovered test information. - * - * Services should not be stateful, but we only keep this information per - * request. That way we don't perform a file scan every time we need this - * hash. The test scan results are unlikely to change during the request. - * - * @var string - */ - protected $hash; - - /** - * Construct a test discovery cache context. - * - * @param \Drupal\simpletest\TestDiscovery $test_discovery - * The test discovery service. - * @param \Drupal\Core\PrivateKey $private_key - * The private key service. - */ - public function __construct(TestDiscovery $test_discovery, PrivateKey $private_key) { - $this->testDiscovery = $test_discovery; - $this->privateKey = $private_key; - } - - /** - * {@inheritdoc} - */ - public static function getLabel() { - return t('Test discovery'); - } - - /** - * {@inheritdoc} - */ - public function getContext() { - if (empty($this->hash)) { - $tests = $this->testDiscovery->getTestClasses(); - $this->hash = $this->hash(serialize($tests)); - } - return $this->hash; - } - - /** - * {@inheritdoc} - */ - public function getCacheableMetadata() { - return new CacheableMetadata(); - } - - /** - * Hashes the given string. - * - * @param string $identifier - * The string to be hashed. - * - * @return string - * The hash. - */ - protected function hash($identifier) { - return hash('sha256', $this->privateKey->get() . Settings::getHashSalt() . $identifier); - } - -} diff --git a/core/modules/simpletest/src/Form/SimpletestTestForm.php b/core/modules/simpletest/src/Form/SimpletestTestForm.php index 69f31e4572eb..0b704f90c543 100644 --- a/core/modules/simpletest/src/Form/SimpletestTestForm.php +++ b/core/modules/simpletest/src/Form/SimpletestTestForm.php @@ -110,10 +110,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { ]; $form['tests'] = [ - '#cache' => [ - 'keys' => ['simpletest_ui_table'], - 'contexts' => ['test_discovery'], - ], '#type' => 'table', '#id' => 'simpletest-form-table', '#tableselect' => TRUE, diff --git a/core/modules/simpletest/src/TestDiscovery.php b/core/modules/simpletest/src/TestDiscovery.php index da3d3af4af4f..9e6c32c327cf 100644 --- a/core/modules/simpletest/src/TestDiscovery.php +++ b/core/modules/simpletest/src/TestDiscovery.php @@ -6,6 +6,7 @@ use Doctrine\Common\Reflection\StaticReflectionParser; use Drupal\Component\Annotation\Reflection\MockFileFinder; use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Extension\ExtensionDiscovery; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\simpletest\Exception\MissingGroupException; @@ -24,11 +25,11 @@ class TestDiscovery { protected $classLoader; /** - * Statically cached list of test classes. + * Backend for caching discovery results. * - * @var array + * @var \Drupal\Core\Cache\CacheBackendInterface */ - protected $testClasses; + protected $cacheBackend; /** * Cached map of all test namespaces to respective directories. @@ -69,11 +70,14 @@ class TestDiscovery { * \Symfony\Component\ClassLoader\ApcClassLoader. * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler * The module handler. + * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend + * (optional) Backend for caching discovery results. */ - public function __construct($root, $class_loader, ModuleHandlerInterface $module_handler) { + public function __construct($root, $class_loader, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache_backend = NULL) { $this->root = $root; $this->classLoader = $class_loader; $this->moduleHandler = $module_handler; + $this->cacheBackend = $cache_backend; } /** @@ -155,9 +159,9 @@ public function getTestClasses($extension = NULL, array $types = []) { $reader = new SimpleAnnotationReader(); $reader->addNamespace('Drupal\\simpletest\\Annotation'); - if (!isset($extension) && empty($types)) { - if (!empty($this->testClasses)) { - return $this->testClasses; + if (!isset($extension)) { + if ($this->cacheBackend && $cache = $this->cacheBackend->get('simpletest:discovery:classes')) { + return $cache->data; } } $list = []; @@ -211,8 +215,10 @@ public function getTestClasses($extension = NULL, array $types = []) { // Allow modules extending core tests to disable originals. $this->moduleHandler->alter('simpletest', $list); - if (!isset($extension) && empty($types)) { - $this->testClasses = $list; + if (!isset($extension)) { + if ($this->cacheBackend) { + $this->cacheBackend->set('simpletest:discovery:classes', $list); + } } if ($types) { diff --git a/core/modules/simpletest/tests/src/Kernel/Cache/Context/TestDiscoveryCacheContextTest.php b/core/modules/simpletest/tests/src/Kernel/Cache/Context/TestDiscoveryCacheContextTest.php deleted file mode 100644 index 25b2af92e439..000000000000 --- a/core/modules/simpletest/tests/src/Kernel/Cache/Context/TestDiscoveryCacheContextTest.php +++ /dev/null @@ -1,53 +0,0 @@ -<?php - -namespace Drupal\Tests\simpletest\Kernel\Cache\Context; - -use Drupal\KernelTests\KernelTestBase; -use Drupal\simpletest\Cache\Context\TestDiscoveryCacheContext; -use Drupal\simpletest\TestDiscovery; - -/** - * @group simpletest - */ -class TestDiscoveryCacheContextTest extends KernelTestBase { - - /** - * {@inheritdoc} - */ - public static $modules = ['simpletest']; - - /** - * Tests that test context hashes are unique. - */ - public function testContext() { - // Mock test discovery. - $discovery = $this->getMockBuilder(TestDiscovery::class) - ->setMethods(['getTestClasses']) - ->disableOriginalConstructor() - ->getMock(); - // Set getTestClasses() to return different results on subsequent calls. - // This emulates changed tests in the filesystem. - $discovery->expects($this->any()) - ->method('getTestClasses') - ->willReturnOnConsecutiveCalls( - ['group1' => ['Test']], - ['group2' => ['Test2']] - ); - - // Make our cache context object. - $cache_context = new TestDiscoveryCacheContext($discovery, $this->container->get('private_key')); - - // Generate a context hash. - $context_hash = $cache_context->getContext(); - - // Since the context stores the hash, we have to reset it. - $hash_ref = new \ReflectionProperty($cache_context, 'hash'); - $hash_ref->setAccessible(TRUE); - $hash_ref->setValue($cache_context, NULL); - - // And then assert that we did not generate the same hash for different - // content. - $this->assertNotSame($context_hash, $cache_context->getContext()); - } - -} -- GitLab