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

clean_returnvalue removes keys from external_multiple_structure

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Won't Fix
    • Affects Version/s: 3.1, 3.2
    • Fix Version/s: None
    • Component/s: Web Services
    • Labels:
      None

      Description

      During MDL-53772, validation for return values was added. It used code from 2010 to complete that validation. There is a bug where external_single_structure keeps the key, but external_multiple_structure does not.

      When upgrading webservices that expected array keys to be kept they fail. They are replaced with numeric index keys based on the number of returned elements.

      As an example of the behaviour;

      • lib/externallib.php:438,439 does not use $result[$key] in the foreach loop of external_multiple_structure.
      • Annotating that code shows it was introduced in MDL-21580 in 2010.
      • However until 3.1, lib/externallib.php:call_external_function() did not call clean_returnvalue. * So that code was not used and in most cases did not affect the output.
      • Looking at get_forums_by_courses() external call as further detail and example of what's happening in code;
      • When constructing mod/forum/externallib.php:102 specifically uses the forum id as the index key. This code was added in MDL-49921 for 3.1 as well. And it sets the return keys to forum values.
      • Looking at testing code surrounding the forum web service;
        mod/forum/tests/externallib_test.php:162 was also updated in MDL-49921 to use $forums[0], even though the code on the same change was to return keys as $forum->id

      Altering lib/externallib.php:438,439 and running

      vendor/bin/phpunit mod_forum_external_testcase mod/forum/tests/externallib_test.php
      

      Produces a failure because of the id's returns. I do not think the test is correct and the externallib.php code is incorrect.

      All of these changes happened in the 3.1 cycle.
      Previously the return values weren't cleaned in the same way and didn’t do that, so indexes on multiple structures were not lost.

      This is a behaviour change from 3.0.
      This behaviour is inconsistent between external_single_structure and external_multiple_structure

      I believe this behaviour should be updated to behave as it did in 3.0 and prior.

      What if any impact will this have on core applications like moodle mobile?

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: