Problem/Motivation

#2807785: Move global constants from *.module files into interfaces deprecated a bunch of constants but it did not actually replace their usage. We should do this. This issue handles UPDATE_NOT_SECURE, UPDATE_REVOKED, UPDATE_NOT_SUPPORTED, UPDATE_NOT_CURRENT, UPDATE_CURRENT, UPDATE_NOT_CHECKED, UPDATE_UNKNOWN, UPDATE_NOT_FETCHED and UPDATE_FETCH_PENDING. This is a bug because we've deprecated something but we've not completed the task.

Proposed resolution

Use UpdateManagerInterface::NOT_SECURE, UpdateManagerInterface::REVOKED, UpdateManagerInterface::NOT_SUPPORTED, UpdateManagerInterface::NOT_CURRENT, UpdateManagerInterface::CURRENT, UpdateFetcherInterface::NOT_CHECKED, UpdateFetcherInterface::UNKNOWN, UpdateFetcherInterface::NOT_FETCHED and UpdateFetcherInterface::FETCH_PENDING instead.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#10 interdiff-7-10.txt1.2 KBlongwave
#10 3015810-10.drupal.Properly-deprecate-UPDATE-constants.patch31.3 KBlongwave
#7 3015810-7.drupal.Properly-deprecate-UPDATE-constants.patch30.87 KBlongwave
#4 3015810-4.patch30.79 KBalexpott
#4 2-4-interdiff.txt4.83 KBalexpott
#2 3015810-2.patch30.78 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 Thunder commented
Status: Active » Needs review
FileSize
3015810-2.patch30.78 KB

With this patch the old constants are no longer used.

grep -R UPDATE_ ./core/modules/update                                                                             Fri 23 Nov 20:43:45 2018
./core/modules/update/update.module:const UPDATE_NOT_SECURE = 1;
./core/modules/update/update.module:const UPDATE_REVOKED = 2;
./core/modules/update/update.module:const UPDATE_NOT_SUPPORTED = 3;
./core/modules/update/update.module:const UPDATE_NOT_CURRENT = 4;
./core/modules/update/update.module:const UPDATE_CURRENT = 5;
./core/modules/update/update.module:const UPDATE_NOT_CHECKED = -1;
./core/modules/update/update.module:const UPDATE_UNKNOWN = -2;
./core/modules/update/update.module:const UPDATE_NOT_FETCHED = -3;
./core/modules/update/update.module:const UPDATE_FETCH_PENDING = -4;
./core/modules/update/tests/src/Functional/UpdateContribTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateContribTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateContribTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateContribTest.php:        'expected_update_message_type' => static::UPDATE_NONE,
./core/modules/update/tests/src/Functional/UpdateContribTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateContribTest.php:        'expected_update_message_type' => static::UPDATE_AVAILABLE,
./core/modules/update/tests/src/Functional/UpdateContribTest.php:        'expected_update_message_type' => static::UPDATE_NONE,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::UPDATE_AVAILABLE,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::UPDATE_NONE,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::SECURITY_UPDATE_REQUIRED,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::UPDATE_AVAILABLE,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::UPDATE_AVAILABLE,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => static::UPDATE_NONE,
./core/modules/update/tests/src/Functional/UpdateCoreTest.php:        'expected_update_message_type' => $pre_release === '2.0-rc2' ? static::UPDATE_NONE : static::UPDATE_AVAILABLE,
./core/modules/update/tests/src/Functional/UpdateTestBase.php:  const SECURITY_UPDATE_REQUIRED = 'SECURITY_UPDATE_REQUIRED';
./core/modules/update/tests/src/Functional/UpdateTestBase.php:  const UPDATE_AVAILABLE = 'UPDATE_AVAILABLE';
./core/modules/update/tests/src/Functional/UpdateTestBase.php:  const UPDATE_NONE = 'UPDATE_NONE';
./core/modules/update/tests/src/Functional/UpdateTestBase.php:      if ($expected_update_message_type === static::SECURITY_UPDATE_REQUIRED) {
./core/modules/update/tests/src/Functional/UpdateTestBase.php:      if ($expected_update_message_type === static::UPDATE_AVAILABLE) {
./core/modules/update/tests/src/Functional/UpdateTestBase.php:      elseif ($expected_update_message_type === static::UPDATE_NONE) {
./core/modules/update/update.compare.inc:  // If the project is marked as UPDATE_FETCH_PENDING, it means that the
./core/modules/update/src/UpdateFetcher.php:  const UPDATE_DEFAULT_URL = 'http://updates.drupal.org/release-history';
./core/modules/update/src/UpdateFetcher.php:        $url = static::UPDATE_DEFAULT_URL;

Status: Needs review » Needs work

The last submitted patch, 2: 3015810-2.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Needs work » Needs review
FileSize
2-4-interdiff.txt4.83 KB
3015810-4.patch30.79 KB

Fixed the fails. Needed to read https://twig.symfony.com/doc/2.x/functions/constant.html

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
FileSize
3015810-7.drupal.Properly-deprecate-UPDATE-constants.patch30.87 KB

Rerolled. This is now needed in 9.0.x for #3097423: Remove update.module BC layers

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Issue tags: +blocker
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

There is one reference to UPDATE_FETCH_PENDING left in update.compare.inc in a comment.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
FileSize
3015810-10.drupal.Properly-deprecate-UPDATE-constants.patch31.3 KB
interdiff-7-10.txt1.2 KB
Berdir’s picture

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

Thanks, that was the only thing I found. It's a constant, so we can test/enforce it.

  • catch committed ed9baee on 9.0.x 
    Issue #3015810 by alexpott, longwave, Berdir: Properly deprecate UPDATE_...
catch’s picture

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

Committed ed9baee and pushed to 9.0.x. Thanks!

  • catch committed 83fa172 on 8.9.x 
    Issue #3015810 by alexpott, longwave, Berdir: Properly deprecate UPDATE_...
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

I see it was also pushed to 8.9 now, should also be safe for a backport as it just updates usage of constants, but also not so important.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
CreditAttribution: benjifisher at Hook 42 commented

I guess I am a little too late for this issue, but

+++ b/core/modules/update/update.install
@@ -7,6 +7,8 @@
 
 use Drupal\Core\Link;
 use Drupal\Core\Url;
+use Drupal\Update\UpdateFetcherInterface;
+use Drupal\Update\UpdateManagerInterface;

The namespace should be Drupal\update instead of Drupal\Update.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented

Originally I thought that we should do #16 in a new issue because there are lots of code comments with the same bug but then I got concerned about case sensitive file systems so I made follow-up commit.

Committed and pushed 4a812fc9c7 to 9.0.x and 73b1fa5970 to 8.9.x. Thanks!

  • alexpott committed 4a812fc on 9.0.x 
    Issue #3015810 follow-up by benjifisher: Properly deprecate UPDATE_*...

  • alexpott committed 73b1fa5 on 8.9.x 
    Issue #3015810 follow-up by benjifisher: Properly deprecate UPDATE_*...

Status: Fixed » Closed (fixed)

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