refactor: #3468921 Convert trigger_error in VariationCache into a LogicException (c7dc1485) · Commits · project / drupal

core/lib/Drupal/Core/Cache/VariationCache.php

+2 −2

Original line number	Diff line number	Diff line
		@@ -235,13 +235,13 @@ public function set(array $keys, $data, CacheableDependencyInterface $cacheabili
		// adding the cacheability from previous access checks that did not
		// lead to a value being returned.
		if (!array_diff($common_contexts, $previous_step_contexts)) {
		trigger_error(sprintf(
		throw new \LogicException(sprintf(
		'Trying to overwrite a cache redirect for "%s" with one that has nothing in common, old one at address "%s" was pointing to "%s", new one points to "%s".',
		$chain_cid,
		implode(', ', $previous_step_contexts),
		implode(', ', array_diff($result_contexts, $previous_step_contexts)),
		implode(', ', array_diff($contexts, $previous_step_contexts)),
		), E_USER_WARNING);
		));
		}

		// != is the most appropriate comparison operator here, since we

core/tests/Drupal/Tests/Core/Cache/VariationCacheTest.php

+4 −48

Original line number	Diff line number	Diff line
		@@ -432,12 +432,6 @@ public function testIncompleteVariationsException(): void {
		* @legacy-covers ::set
		*/
		public function testIncompleteRedirectException(): void {
		// @todo Remove in Drupal 12.0.0. For more information, see:
		// https://www.drupal.org/project/drupal/issues/3468921
		set_error_handler(static function (int $errno, string $errstr): never {
		throw new \LogicException($errstr, $errno);
		}, E_USER_WARNING);

		// This should never happen. When we have a cache redirect at address A,
		// pointing to 'A,B:foo' and then someone tries to store a cache redirect at
		// A pointing to 'A,B', something is wrong. The cache contexts leading up to
		@@ -460,13 +454,8 @@ public function testIncompleteRedirectException(): void {
		$garden_cacheability = (new CacheableMetadata())
		->setCacheContexts(['house.type', 'garden.type']);

		try {
		$this->setVariationCacheItem('You have a house with a baroque garden!', $garden_cacheability, $house_cacheability);
		}
		finally {
		restore_error_handler();
		}
		}

		/**
		* Tests exception for a cache item that has incompatible cache redirects.
		@@ -475,12 +464,6 @@ public function testIncompleteRedirectException(): void {
		* @legacy-covers ::set
		*/
		public function testIncompatibleRedirectsException(): void {
		// @todo Remove in Drupal 12.0.0. For more information, see:
		// https://www.drupal.org/project/drupal/issues/3468921
		set_error_handler(static function (int $errno, string $errstr): never {
		throw new \LogicException($errstr, $errno);
		}, E_USER_WARNING);

		// This should never happen. When someone first triggers the storing of a
		// redirect using context A and then tries to store another redirect in the
		// same spot using context B, something is wrong. The cache contexts of all
		@@ -502,13 +485,8 @@ public function testIncompatibleRedirectsException(): void {
		->setCacheContexts(['house.type', 'house.orientation']);

		$this->setVariationCacheItem('You have a nice house with a garden!', $garden_cacheability, $house_cacheability);
		try {
		$this->setVariationCacheItem('You have a nice north-facing house!', $orientation_cacheability, $house_cacheability);
		}
		finally {
		restore_error_handler();
		}
		}

		/**
		* Tests the same as above, but with more redirects.
		@@ -518,12 +496,6 @@ public function testIncompatibleRedirectsException(): void {
		*/
		#[Depends('testIncompatibleRedirectsException')]
		public function testIncompatibleChainedRedirectsException(): void {
		// @todo Remove in Drupal 12.0.0. For more information, see:
		// https://www.drupal.org/project/drupal/issues/3468921
		set_error_handler(static function (int $errno, string $errstr): never {
		throw new \LogicException($errstr, $errno);
		}, E_USER_WARNING);

		$this->expectException(\LogicException::class);
		$this->expectExceptionMessage('Trying to overwrite a cache redirect for "your:housing:situation:gt.garden:ht.house" with one that has nothing in common, old one at address "house.type, garden.type" was pointing to "house.orientation", new one points to "solar.type".');

		@@ -550,13 +522,8 @@ public function testIncompatibleChainedRedirectsException(): void {
		->setCacheContexts(['house.type', 'garden.type', 'solar.type']);

		$this->setVariationCacheItem('You have a nice north-facing house with a garden!', $orientation_cacheability, $house_cacheability);
		try {
		$this->setVariationCacheItem('You have a nice house with solar panels and a garden!', $solar_cacheability, $house_cacheability);
		}
		finally {
		restore_error_handler();
		}
		}

		/**
		* Tests the same as above, but even more complex.
		@@ -566,12 +533,6 @@ public function testIncompatibleChainedRedirectsException(): void {
		*/
		#[Depends('testIncompatibleChainedRedirectsException')]
		public function testIncompatibleChainedRedirectsComplexException(): void {
		// @todo Remove in Drupal 12.0.0. For more information, see:
		// https://www.drupal.org/project/drupal/issues/3468921
		set_error_handler(static function (int $errno, string $errstr): never {
		throw new \LogicException($errstr, $errno);
		}, E_USER_WARNING);

		$this->expectException(\LogicException::class);
		$this->expectExceptionMessage('Trying to overwrite a cache redirect for "your:housing:situation:gt.garden:ht.house" with one that has nothing in common, old one at address "house.type, garden.type" was pointing to "house.orientation", new one points to "solar.type".');

		@@ -600,13 +561,8 @@ public function testIncompatibleChainedRedirectsComplexException(): void {
		// Now we arrive at the same scenario as the test above. We have a redirect
		// chain at house.type of garden.type and finally house.orientation, but are
		// trying to set solar.type at that last address.
		try {
		$this->setVariationCacheItem('You have a nice house with solar panels and a garden!', $solar_cacheability, $house_cacheability);
		}
		finally {
		restore_error_handler();
		}
		}

		/**
		* Creates the sorted cache ID from cache ID parts.