Problem/Motivation

#2454649: Cache Optimization and hardening -- [PP-1] Use assert() instead of exceptions in Cache::merge(Tags|Contexts) deprecated Cache::validateTags() before deprecation testing was a thing and it never removed all the usages.

Proposed resolution

  • Add @trigger_error
  • Add assertion to test

Note there is no change record for this change. Given this was done on 8.0.x I think adding one now is only confusing.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

N/a

CommentFileSizeAuthor
#7 3068076-7.patch1.72 KBalexpott
#7 3-7-interdiff.txt2.01 KBalexpott
#3 3068076-3.patch1.63 KBalexpott
#3 2-3-interdiff.txt1.65 KBalexpott
#2 3068076-2.patch1.59 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review
FileSize
3068076-2.patch1.59 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
FileSize
2-3-interdiff.txt1.65 KB
3068076-3.patch1.63 KB

Let's add the namespace to the deprecation message.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Looks good.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Component: base system » cache system

👍

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/lib/Drupal/Core/Cache/Cache.php
@@ -96,12 +96,13 @@ public static function mergeMaxAges($a = Cache::PERMANENT, $b = Cache::PERMANENT
+    @trigger_error(__NAMESPACE__ . '\Cache::validateTags() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use assert(Inspector::assertAllStrings($tags)) instead.', E_USER_DEPRECATED);

We should include the namespace for Inspector here as well.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs work » Reviewed & tested by the community
FileSize
3-7-interdiff.txt2.01 KB
3068076-7.patch1.72 KB

Sure why not.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed c6b60cd and pushed to 8.8.x. Thanks!

  • catch committed c6b60cd on 8.8.x 
    Issue #3068076 by alexpott: Properly deprecate Cache::validateTags()
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.