Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-67594

Deprecate supports_recursion() & extend_lock() in the Lock API

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      Using the terminal go to the root folder of your moodle instance.

      Test 1 (supports_recursion())

      1. Do a grep for 'supports_recursion' ()

      grep -R supports_recursion
      

       Confirm that the only results are in:

      • lib/upgrade.txt (notes about the deprecated supports_recursion())
      • lib/classes/lock/lock_factory.php (declaration of the interface method)
      • lib/classes/lock/installation_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/db_record_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/postgres_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/file_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/mysql_lock_factory.php (implementation method and debug message)

      Test 2 (extend_lock())

      1. Using the terminal go to the root folder of your moodle instance.
      2. Do a grep for 'extend_lock'

      grep -R extend_lock
      

       Confirm that the only results are in:

      • lib/upgrade.txt (notes about the deprecated extend_lock())
      • lib/classes/lock/lock_factory.php (declaration of the interface method)
      • lib/classes/lock/installation_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/db_record_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/postgres_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/file_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/mysql_lock_factory.php (implementation method and debug message)
      • lib/classes/lock/lock.php (used in extend() which will be also deprecated)

      Test 3 (extend())

      1. Using the terminal go to the root folder of your moodle instance.
      2. Do a grep for 'extend'

      grep -R -e "[[:blank:]]extend(" -e ">extend(" --include \*.php --include \*.txt
      

       Confirm that the only results are in:

      • lib/upgrade.txt (notes about the deprecated extend())
      • lib/classes/lock/lock.php (method declaration and debug message)
      Show
      Using the terminal go to the root folder of your moodle instance. Test 1 (supports_recursion()) Do a grep for 'supports_recursion' () grep -R supports_recursion   Confirm that the only results are in: lib/upgrade.txt (notes about the deprecated supports_recursion()) lib/classes/lock/lock_factory.php (declaration of the interface method) lib/classes/lock/installation_lock_factory.php (implementation method and debug message) lib/classes/lock/db_record_lock_factory.php  (implementation method and debug message) lib/classes/lock/postgres_lock_factory.php  (implementation method and debug message) lib/classes/lock/file_lock_factory.php  (implementation method and debug message) lib/classes/lock/mysql_lock_factory.php  (implementation method and debug message) Test 2 ( extend_lock() ) Using the terminal go to the root folder of your moodle instance. Do a grep for ' extend_lock ' grep -R extend_lock   Confirm that the only results are in: lib/upgrade.txt (notes about the deprecated extend_lock()) lib/classes/lock/lock_factory.php (declaration of the interface method) lib/classes/lock/installation_lock_factory.php (implementation method and debug message) lib/classes/lock/db_record_lock_factory.php  (implementation method and debug message) lib/classes/lock/postgres_lock_factory.php  (implementation method and debug message) lib/classes/lock/file_lock_factory.php  (implementation method and debug message) lib/classes/lock/mysql_lock_factory.php  (implementation method and debug message) lib/classes/lock/lock.php  (used in extend() which will be also deprecated) Test 3 (extend()) Using the terminal go to the root folder of your moodle instance. Do a grep for 'extend' grep -R -e "[[:blank:]]extend(" -e ">extend(" --include \*.php --include \*.txt   Confirm that the only results are in: lib/upgrade.txt (notes about the deprecated extend()) lib/classes/lock/lock.php  (method declaration and debug message)
    • Affected Branches:
      MOODLE_39_STABLE
    • Fixed Branches:
      MOODLE_40_STABLE
    • Pull Master Branch:
      MDL-67594-master-2
    • Story Points:
      3
    • Sprint:
      Activity Sprint 10, 4.0 holding pattern, 4.0 holding pattern 2

      Description

      If a lock type supports_recursion() then the locks behave very differently to when they don't.

      Yet despite this not a single place in core actually calls for supports_recursion(), and if it did then it would have to do a code fork to work around the different behaviors. This feels like an architectural mistake in the design of the lock api, all lock implementations should behave identically so that calling code doesn't need to worry about things like this.

      Given that the only place that leverages this is the unit tests, it is clear no one wants or needs it or is using this. I've had a bit of a quick search on github and in the wild for 3rd party uses and couldn't find any.

      It's also trivial to workaround in the lock types that do support recursion to bring their behavior back in line to the locks which don't, as they already internally store a list of open locks for the current request and could just reject the second lock attempt.

        Attachments

          Activity

            People

            Assignee:
            Geshoski Mihail Geshoski
            Reporter:
            brendanheywood Brendan Heywood
            Peer reviewer:
            Mathew May
            Integrator:
            Andrew Nicols
            Tester:
            Janelle Barcega
            Participants:
            Component watchers:
            Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Sara Arjona (@sarjona), Víctor Déniz Falcón
            Votes:
            2 Vote for this issue
            Watchers:
            10 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 0 minutes
                0m
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 6 hours, 10 minutes
                6h 10m