Problem/Motivation

The module is about to be offered as something a site owner installs to get a REST API of their content model. That promise only holds if installing it changes nothing except the appearance of new endpoints, and today it does not hold. Some of it has already been fixed: issue 61 and issue 3616452 scoped the inbound path processor and the 404 handler to this module's own API, the normalizer answers only for routes carrying the module's route requirement, and issue 70 put the node redirect behind a setting. What is left was found by reading the code rather than by a systematic pass, which is the problem this issue exists to solve — the list is not known to be complete.

Known offenders at the time of writing, each of which is a change to a site that installed this module for its API:

  • myrest_mail_alter() rewrites the body of every contact page_mail message the site sends, whether or not it came through this module.
  • myrest_toolbar_alter() writes into $items['admin_toolbar_tools'] without checking that the item exists.
  • MyrestResourceResponseSubscriber::onRequest() runs on every request of the site and, when the load counter is enabled, writes to state on each one — HTML pages included.
  • onRedirectToDefaultLanguage() redirects any entity add form in any non-default language, with no setting and no scope.
  • The token authentication provider applies to any request on the site carrying the configured header.
  • The alias path processor resolves aliases through a switch over one site's bundle names.
  • The normalizer names a foreign route requirement and six satellite resource IDs it cannot know about.

Proposed resolution

  • Enumerate every service the module registers that participates in a site-wide pipeline: event subscribers, path processors inbound and outbound, normalizers and encoders, route subscribers, authentication providers, cache contexts and policies, and every hook implementation in myrest.module.
  • For each one, record what it touches, on which requests it runs, and what a site that never calls this module's endpoints would notice.
  • Classify each as: scoped correctly, needs scoping, needs a setting, or must go.
  • Write the result into this issue as the work list for the issues that follow, and into MODERNIZATION.md §1.1 as the audit trail.

Remaining tasks

The audit itself. The fixes are separate issues — 72, 73, 74, 156, 157, 158 — and the test that keeps the promise honest afterwards is issue 83; nothing of that belongs here. If the audit finds side effects those issues do not cover, they get issues of their own rather than being folded into this one.

Issue fork myrest-3618238

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 0ddae837 on 1.0.x
    Issue #3618238: Document every global side effect and cover the scoped...
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

Status: Fixed » Closed (fixed)