Problem/Motivation

The module has roughly 300 lines of tests across three files, against more than 40 resource plugins and a 6,600-line helper. Effective coverage is close to zero, so refactoring carries a high regression risk.

Proposed resolution

  • Add ResourceTestBase-style functional tests covering authentication, response shape and status codes for each resource group.
  • Add kernel tests for the helper's structure builders using generated test content.
  • Target a meaningful coverage threshold and enforce it in CI.

Remaining tasks

DONE for the shared layer. The functional test exists and the module is testable at all, which it was not. The group-by-group suites and the coverage threshold need content fixtures and are split out into issue 67.

  • MyrestResourceAccessTest covers the rule that every resource of the module shares, because it is written once in MyrestResourceBase::getBaseRouteRequirements(): anonymous gets 403, an authenticated account without the permission gets 403, and access myrest api alone opens the endpoint. The resources declare no per-resource restful get <id> permission - permissions() returns an empty array on purpose - so that one permission is the entire gate, and the test is what pins that down.
  • The same test asserts the response shape of global_variables_get_resource: the body parses as JSON, carries current_user and is_session_open, the uid and email belong to the account that made the request, and all seven keys the front end reads are present. That resource was chosen because it needs no content fixtures - the fixtures the other groups need are the reason this issue is not finished.
  • Three requests live in one test method deliberately. A functional test installs a site per method, and none of the three needs a fresh one.

Writing the test found five defects that made the module unusable outside this one site, and each had to be fixed before the test could reach an assertion. They are listed because they say something about the state of the code, not because the issue asked for them:

  • MyrestResourceBase::create() asked the container for myfacet.manager and myrest_metatag.helper unconditionally, although both belong to suggested modules. On a site without them every resource plugin threw ServiceNotFoundException at instantiation - not only the ones that use facets. Both are now optional, the properties are nullable, and the eight resources that query facets check the manager before using it, as do the three places in MyrestHelper.
  • drupal:file was missing from the dependencies while file.repository is a hard constructor argument. Installing the module without the file module produced the same failure.
  • $currentRequest was typed ?Request and $systemFileConfig was typed ImmutableConfig, but neither class was imported, so both resolved to the plugin's own namespace and every construction ended in a TypeError. $mailManager was typed against \Drupal\symfony_mailer\MailManagerReplacementInterface while the constructor receives core's MailManagerInterface; the property now says what is actually injected.
  • $request_query was assigned without being declared - a dynamic property, deprecated since PHP 8.2.
  • MyrestNormalizer::getLocales() passed the front page setting to getAliasByPath() without checking it, and that setting is absent on a site that does not provide it.

Worth stating plainly: before this, no resource plugin of the module could be instantiated on this checkout at all. plugin.manager.rest threw on the first of these five, and the endpoints were answering with an error rather than data.

Testing

The new functional test passes: 1 test, 22 assertions. The existing suite is unchanged at 44 tests and 362 assertions, which is what says the fixes above did not disturb the unit and kernel coverage. PHPCS reports nothing on the new test. The 57 deprecations the run reports are the @RestResource annotations of the resource plugins, which core removes in 13.0 - a separate piece of work, untouched here.

User interface changes

None.

API changes

None. drupal:file becomes an explicit dependency, which reflects what the code already required.

Issue fork myrest-3616415

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

sergeydruua created an issue. See original summary.

  • sergeydruua committed 7f719fd5 on 1.0.x
    Issue #3616415: Add functional test coverage for the API access rule and...
sergeydruua’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

sergeydruua’s picture

Issue summary: View changes
Status: Fixed » Closed (fixed)