Problem/Motivation

A site names the nodes behind its special pages — the front page, the services page, the blog — in the settings of this module. The service that reads them returns NULL when a page has not been named, which is the normal state of a fresh install, of a site being set up, and of the site every test runs on. Most callers pass that NULL to the entity loader of this module, which is built for it and answers NULL. Three do not, and two other kinds of caller do something worse than fail.

  • Three calls hand the NULL to the node storage directly - the front page resource, the services list resource and the helper. Core asserts that a storage is never asked to load a NULL ID, so with assertions enabled - a development site, or a test run - the endpoint answers 500 with Cannot load the "node" entity with NULL ID. In production, where assertions are compiled out, the same call returns NULL and the endpoint answers the empty response it meant to answer all along. A defect that shows only where developers work is the kind that survives longest.
  • About twenty calls build a path as '/node/' . $id and ask the alias manager about it. With no ID that is a question about /node/, and the answer - the string itself - is handed to the front end as the address of a page.
  • Two build cache tags as 'node:' . $id, which with no ID is the tag node:: a tag no invalidation will ever match, so the response is cached and never refreshed.

Proposed resolution

  • Read the ID first and answer with the empty, cacheable response when there is none, instead of asking the storage about it - the three direct loads.
  • Leave out the alias and the cache tag of a page that has not been named, rather than deriving them from nothing.
  • Cover it with a kernel test: a site that has not named a front page node reads the endpoint and gets the empty answer rather than an error. It fails against the current code, since the test environment is exactly where the assertion is active.
  • Add the check that would have caught it without anybody looking: a test that walks every endpoint this module registers on a site with nothing configured and asserts that none of them answers with a server error. A 404 or an empty payload is a legitimate answer for an unconfigured site; a 500 never is.

Remaining tasks

Everything.

Issue fork myrest-3619452

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 6723a776 on 1.0.x
    Issue #3619452: Keep the ID of an unnamed special page out of the...
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)