Convert hook requirements that do not interact with install time

Created on 18 January 2025, 4 days ago

Problem/Motivation

hook_update_requirements and hook_runtime_requirements are now available.
Let's convert the core ones that only interact with update or runtime.

Runtime phase only

  • image
  • jsonapi
  • locale
  • mysql
  • navigation
  • node
  • search
  • update_test_schema
  • update
  • user
  • demo_umami

Update phase only

  • update_script_test

Steps to reproduce

N/A

Proposed resolution

Convert them to OOP equivalents

Remaining tasks

Convert

User interface changes

N/A

Introduced terminology

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component

base system

Created by

πŸ‡ΊπŸ‡ΈUnited States nicxvan

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @nicxvan
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I'm working on this.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan
  • Merge request !10949Resolve #3500632 "Convert hook requirements" β†’ (Open) created by nicxvan
  • Pipeline finished with Failed
    4 days ago
    Total: 126s
    #399250
  • Pipeline finished with Failed
    4 days ago
    Total: 139s
    #399251
  • Pipeline finished with Failed
    4 days ago
    Total: 137s
    #399254
  • Pipeline finished with Failed
    4 days ago
    Total: 624s
    #399257
  • Pipeline finished with Failed
    4 days ago
    Total: 2004s
    #399266
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Migrate tests are failing due to update I think.

  • Pipeline finished with Failed
    4 days ago
    Total: 189s
    #399484
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan
  • Pipeline finished with Failed
    4 days ago
    Total: 3722s
    #399491
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    We need the constants!

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Found a way forward from @berdir with modulehandler loadInclude.

  • Pipeline finished with Failed
    3 days ago
    Total: 594s
    #399676
  • Pipeline finished with Running
    3 days ago
    #399688
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I am getting navigation performance test failures in this, I'm fairly sure this is unrelated:

    PHPUnit 10.5.38 by Sebastian Bergmann and contributors.
    
    Runtime:       PHP 8.3.10
    Configuration: /var/www/html/phpunit.xml
    
    F                                                                   1 / 1 (100%)
    
    Time: 00:23.234, Memory: 4.00 MB
    
    There was 1 failure:
    
    1) Drupal\Tests\navigation\FunctionalJavascript\PerformanceTest::testLogin
    Failed asserting that two arrays are identical.
    --- Expected
    +++ Actual
    @@ @@
         0 => 'SELECT "session" FROM "sessions" WHERE "sid" = "SESSION_ID" LIMIT 0, 1',
         1 => 'SELECT * FROM "users_field_data" "u" WHERE "u"."uid" = "2" AND "u"."default_langcode" = 1',
         2 => 'SELECT "roles_target_id" FROM "user__roles" WHERE "entity_id" = "2"',
    -    3 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"',
    +    3 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.maintenance_mode" ) AND "collection" = "state"',
    +    4 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "routing.non_admin_routes" ) AND "collection" = "state"',
    +    5 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"',
    +    6 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "twig_extension_hash_prefix" ) AND "collection" = "state"',
    +    7 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.private_key" ) AND "collection" = "state"',
    +    8 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "asset.css_js_query_string" ) AND "collection" = "state"',
    +    9 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "drupal.test_wait_terminate" ) AND "collection" = "state"',
    +    10 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.cron_last" ) AND "collection" = "state"',
    +    11 => 'INSERT INTO "semaphore" ("name", "value", "expire") VALUES ("state:Drupal\Core\Cache\CacheCollector", "LOCK_ID", "EXPIRE")',
    +    12 => 'DELETE FROM "semaphore"  WHERE ("name" = "state:Drupal\Core\Cache\CacheCollector") AND ("value" = "LOCK_ID")',
     ]
    
    /var/www/html/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php:73
    
    FAILURES!
    Tests: 1, Assertions: 4, Failures: 1.
    
    
  • Pipeline finished with Success
    3 days ago
    Total: 579s
    #399805
  • Pipeline finished with Success
    2 days ago
    Total: 849s
    #400298
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Performance test was fixed in πŸ› Performance tests need to run cron Active

  • πŸ‡ΊπŸ‡ΈUnited States dww

    Started reviewing by responding to (and resolving) the open threads. Haven't yet otherwise really reviewed the MR changes, although an initial glance looks good. Hope to review this in more depth on Tuesday.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    So from what I can tell it's majority moving things around and adding dependency injection and string translation stuff (nice!).

    I did have 2 questions on the MR that @dww addressed.

    Don't see anything wrong here. +1 for RTBC but will refers to @dww review in #12

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    I agree that this looks good based on earlier reviews I did.

    My only uncertainty is in which order we want to proceed. We already completely break BC on modules *calling* requirements hooks. This is somewhat rare but it happens, I of course have a module that does that ( https://www.drupal.org/project/monitoring β†’ , see also https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22modul..., without moduleHandler it finds a lot more, but I assume that's all D7 code).

    That's OK I think, this isn't meant to be an API, and I do plan to update monitoring.

    But as I mentioned in πŸ“Œ Add value objects to represent the return of hook_requirements Needs work , the new hooks gives us a chance to considerably simplify the BC layer that the new value object for requirements definitions needs to support. we could say that the new hooks must return value objects. The it might make sense to do the other issue issue first. But that isn't close yet and there are more BC things to consider (for example usage of #type status_report, which is also used in a few projects, including again one of mine: https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22%27%2...).

    Long story short, it's probably OK to get this in now and first. Might want to think again about it before we do system module though.

Production build 0.71.5 2024