Lesterhuis-Training-en-Consultancy / moodle-block-user_favorites

Moodle plugin - marking pages as favorites
GNU General Public License v3.0
1 stars 4 forks source link

Debugging: External function parameters: invalid OPTIONAL value specified. #8

Closed ewallah closed 4 years ago

ewallah commented 4 years ago

When running phpunit tests 1 error is detected:

vendor/bin/phpunit --colors "core_externallib_testcase" lib/tests/externallib_test.php
Moodle 3.8.3+ (Build: 20200605), 6f0115e399a5523f1572e859dcf89936cce3a28a
Php: 7.3.18.1.18.04.1.1, pgsql: 10.12 (Ubuntu 10.12-0ubuntu0.18.04.1), OS: Linux 4.15.0-101-generic x86_64
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

.................E.............................................  63 / 651 (  9%)
............................................................... 126 / 651 ( 19%)
............................................................... 189 / 651 ( 29%)
............................................................... 252 / 651 ( 38%)
............................................................... 315 / 651 ( 48%)
............................................................... 378 / 651 ( 58%)
............................................................... 441 / 651 ( 67%)
............................................................... 504 / 651 ( 77%)
............................................................... 567 / 651 ( 87%)
............................................................... 630 / 651 ( 96%)
.....................                                           651 / 651 (100%)

Time: 19.86 seconds, Memory: 101.00 MB

There was 1 error:

1) core_externallib_testcase::test_all_external_info with data set "block_user_favorites_set_url" (stdClass Object (...))
Unexpected debugging() call detected.
Debugging: External function parameters: invalid OPTIONAL value specified.
* line 712 of /lib/externallib.php: call to debugging()
* line 85 of /blocks/user_favorites/externallib.php: call to external_function_parameters->__construct()
To re-run:
 vendor/bin/phpunit --colors "core_externallib_testcase" lib/tests/externallib_test.php

ERRORS!
Tests: 651, Assertions: 3887, Errors: 1.

When I modify the externallib.php file and change the url from optional to required,

-'url' => new external_value(PARAM_URL, 'URL', VALUE_OPTIONAL),
+'url' => new external_value(PARAM_URL, 'URL', VALUE_REQUIRED),

the error is gone, and the block continue to work as expected.

Any reason why you need an optional external value?

luukverhoeven commented 4 years ago

Thanks for checking.

Can you check if it's passing the test if you change the line like below: 'url' => new external_value(PARAM_URL, 'URL', VALUE_OPTIONAL, ''),

ewallah commented 4 years ago

Sorry, but this does not help.

Moodle simply does not like optional values: line 712 of /lib/externallib.php is throwing an exception for all optional values:

        if ($CFG->debugdeveloper) {
            foreach ($keys as $key => $value) {
                if ($value instanceof external_value) {
                    if ($value->required == VALUE_OPTIONAL) {
                        debugging('External function parameters: invalid OPTIONAL value specified.', DEBUG_DEVELOPER);
                        break;
                    }
                }
            }
        }

Could this be security related?

luukverhoeven commented 4 years ago

Strange I think it's only a notice. The documentation doesn't advise to not use VALUE_OPTIONAL

https://docs.moodle.org/dev/Adding_a_web_service_to_a_plugin#Required.2C_Optional_or_Default_value

ewallah commented 4 years ago

The note in your link explains everything: Because some web service protocols are strict about the number and types of arguments - it is not possible to specify an optional parameter as one of the top-most parameters for a function.

The cool sample get_biscuit_parameters() provides a solution.

luukverhoeven commented 4 years ago

Thanks for checking the Moodle webservice page. I will implement this in the next version.

luukverhoeven commented 4 years ago

@ewallah Can you check if you still see this problem when running phpunit? Should be solved now.

ewallah commented 4 years ago
#vendor/bin/phpunit --colors "core_externallib_testcase" lib/tests/externallib_test.php
Moodle 3.8.3+ (Build: 20200711), a6227d56752c3af728f09dc863e50062f1bac773
Php: 7.3.20.1.18.04.1.1, pgsql: 10.12 (Ubuntu 10.12-0ubuntu0.18.04.1), OS: Linux 4.15.0-109-generic x86_64
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

...............................................................  63 / 649 (  9%)
............................................................... 126 / 649 ( 19%)
............................................................... 189 / 649 ( 29%)
............................................................... 252 / 649 ( 38%)
............................................................... 315 / 649 ( 48%)
............................................................... 378 / 649 ( 58%)
............................................................... 441 / 649 ( 67%)
............................................................... 504 / 649 ( 77%)
............................................................... 567 / 649 ( 87%)
............................................................... 630 / 649 ( 97%)
...................                                             649 / 649 (100%)

Time: 6.71 seconds, Memory: 103.00 MB

OK (649 tests, 3875 assertions)

Fixed!

luukverhoeven commented 4 years ago

👌thanks for checking