Drupal 9 Deprecated Code Report (Messenger)

I replaced drupal_set_message() with Drupal Messenger service.
Here is the patch.

Personally, I think using the MessengerTrait would be a less disruptive change, though in this case it is probably safe to change the parameters for this class as I doubt someone else is extending it.

Fixed the messenger once more.

Also, added the required core_version_requirement strings and fixed a typo.

Changes check ok.

Hello, after running drupal-check 02/04/2020

 27/27 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ---------------------------------------------------------------------- 
  Line   memcache_admin/src/Controller/MemcacheStatisticsController.php        
 ------ ---------------------------------------------------------------------- 
  55     Call to deprecated function drupal_set_message():                     
         in drupal:8.5.0 and is removed from drupal:9.0.0.                     
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.  
  79     Call to deprecated function drupal_set_message():                     
         in drupal:8.5.0 and is removed from drupal:9.0.0.                     
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.  
  84     Call to deprecated function drupal_set_message():                     
         in drupal:8.5.0 and is removed from drupal:9.0.0.                     
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.  
  224    Call to deprecated function drupal_set_message():                     
         in drupal:8.5.0 and is removed from drupal:9.0.0.                     
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.  
 ------ ---------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------- 
  Line   src/Tests/MemcacheLockFunctionalTest.php                                                             
 ------ ----------------------------------------------------------------------------------------------------- 
         Class Drupal\Tests\system\Functional\Lock\LockFunctionalTest not found and could not be autoloaded.  
 ------ ----------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------ 
  Line   tests/src/Kernel/MemcacheBackendTest.php                                                                    
 ------ ------------------------------------------------------------------------------------------------------------ 
         Class Drupal\KernelTests\Core\Cache\GenericCacheBackendUnitTestBase not found and could not be autoloaded.  
 ------ ------------------------------------------------------------------------------------------------------------ 

 ------ ------------------------------------------------------------------------ 
  Line   tests/src/Unit/MemcacheSettingsTest.php                                 
 ------ ------------------------------------------------------------------------ 
         Class Drupal\Tests\UnitTestCase not found and could not be autoloaded.  
 ------ ------------------------------------------------------------------------ 

                                                                                                                        
 [ERROR] Found 7 errors

Commit: 3a623de259a2482d4a23015051f5172c634c6abb

Drupal version: 8.8.2
PHP: 7.2

Re #6 - There are no drupal_set_message() uses in the module after applying the patch on #4.

RTBC++

Hi all,
I've updated solution in comment #4,
"Memcache 8.x-2.0-dev" Scanned on Sat, 02/08/2020 - 19:03.
No known issues found.

Please review.

Thanks @nikolas.tatianenko for working on this :)

I do have one concern with the latest patch:

+++ b/src/MemcacheBackend.php
@@ -58,12 +66,15 @@ class MemcacheBackend implements CacheBackendInterface {
-  public function __construct($bin, DrupalMemcacheInterface $memcache, CacheTagsChecksumInterface $checksum_provider, TimestampInvalidatorInterface $timestamp_invalidator) {
+  public function __construct($bin, DrupalMemcacheInterface $memcache, CacheTagsChecksumInterface $checksum_provider, TimestampInvalidatorInterface $timestamp_invalidator, TimeInterface $time) {

This is a BC breaking change, and any custom cache backends that people may have implemented extending this class would need to be updated or they will break. Could easily be avioded by not injecting the service and just using \Drupal::time()->getRequestTime();

Up to the maintainers on how to deal with this, just wanted to point it out :)

Good point by @Manuel Garcia. In Drupal 8 core though, constructors are considered internal and are not guaranteed to change: https://www.drupal.org/core/d8-bc-policy while not using dependency injection is strongly discouraged.

Some discussion on alternative approach to constructor overrides here: https://www.previousnext.com.au/blog/safely-extending-drupal-8-plugin-cl...

But yes, maintainer call in the end.

Ran drupal-check on #8, all clear, RTBCing.

Ran drupal-check on #8, all clear

Same here.

+1 on rtbc. Although seeing MemcacheLockFunctionalTest in src/Tests looked very much like it was a Simpletest. And that isn't true. So we could move it. But that isn't strictly necessary for 9.x compatibility.

Just added #8 to a running Drupal 9.0-beta2 site that without Memcache configuration runs fine. When I enable the config that I use on the production (running 8.8.5) I get a WSOD and the following stack trace:

PHP Fatal error:  Uncaught ArgumentCountError: Too few arguments to function Drupal\memcache\MemcacheBackend::__construct(), 4 passed in /app/web/core/lib/Drupal/Component/DependencyInjection/PhpArrayContainer.php on line 83 and exactly 5 expected in /app/web/modules/contrib/memcache/src/MemcacheBackend.php:72
Stack trace:
#0 /app/web/core/lib/Drupal/Component/DependencyInjection/PhpArrayContainer.php(83): Drupal\memcache\MemcacheBackend->__construct('container', Object(Drupal\memcache\Driver\MemcachedDriver), Object(Drupal\Core\Cache\DatabaseCacheTagsChecksum), Object(Drupal\memcache\Invalidator\MemcacheTimestampInvalidator))
#1 /app/web/core/lib/Drupal/Component/DependencyInjection/Container.php(173): Drupal\Component\DependencyInjection\PhpArrayContainer->createService(Array, 'cache.container')
#2 /app/web/core/lib/Drupal/Core/DrupalKernel.php(546): Drupal\Component\DependencyInjection\Container->get('cache.container')
#3 /app/web/core/lib/Drupal/Core/DrupalKernel.php(891): Drupal\Core\DrupalKernel->getCachedContainerDefini in /app/web/modules/contrib/memcache/src/MemcacheBackend.php on line 72

In the REAME.txt file on how to install this module, you have instructed people how to use "Cache Container on bootstrap (with cache tags on database)", which I followed. I don't believe it is possible to use the cache container on bootstrap and to pass along the @datetime.time service to MemcacheBackend, as this service doesn't exist yet at that time.

I'll provide a patch that removes this dependency injection and seems to work fine in my case. I suggest that if the maintainers wish to decide on having this dependency injection, this MUST be a BC break, and the module must switch to version 3.x.

Instead of removing the services, I think it would be better to use the pattern that is outlined in https://www.previousnext.com.au/blog/safely-extending-drupal-8-plugin-cl..., as that does not break on parent constructor changes.

Fixed for the messenger. Removing the Time service for now, it is handled in this issue: #2999614: Update REQUEST_TIME call