Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider eliminating loopback requests when performing AMP validation #4979

Closed
westonruter opened this issue Jul 3, 2020 · 0 comments
Closed
Labels
Developer Tools Validation WS:Core Work stream for Plugin core

Comments

@westonruter
Copy link
Member

westonruter commented Jul 3, 2020

Feature description

A common issue in the support forums is failing to validate a URL due to loopback requests not working. It's true that this exposes a critical error with a WordPress site that needs to be fixed anyway, but we should consider whether we absolutely need to do loopback requests.

Currently validation is performed by submitting a validation request to WordPress, including the URL needing to be validated (see AMP_Validation_Manager::validate_url()). WordPress then does a loopback request to fetch that URL with an amp_validate query parameter supplied which tells the AMP plugin to return validation results as JSON, as opposed to rendering an AMP page normally.

As opposed to this, consider this alternative:

  1. The user initiates a request to validate a URL.
  2. In the client, a fetch request is made to the URL needing to be validated with the aforementioned amp_validate query parameter, resulting in the validation results being returned as JSON.
  3. The client then takes that data and does a POST request to some new endpoint which then stores the validation results.

This would eliminate the need to do loopback requests. It has the downside, however, of adding more complexity to the client. It would also introduce the opportunity where a site could store bogus validation results, which I don't anticipate to be a real concern.

👉 An alternative to this would would simplify the client-side logic while also avoiding the need to allow the client direct access to store validation results would be to just go ahead and store the validation results in the amp_validate request that was made to obtain the results in the first place. If there were a new amp_validate_store query parameter, for example, the results could be stored in an amp_validate_url post right away without requiring some other previous server-side request to store the results.

This would assist with implementing the Site Scan step of the wizard (#4795 and #4719). Consider a new REST API endpoint which returns an array of URLs for key representative templates and content types on a site (for example what is currently implemented in WP-CLI via AMP_CLI_Validation_Command::crawl_site()). The client can then do an amp_validate request for each returned URL, in succession. The wizard just needs the results to be able to determine whether theme/plugins are causing validation errors. (The results wouldn't even need to be stored here, so in reality nothing is blocking this from being implemented today. Although it would be useful to have the results stored with validation having been performed with Standard mode forced so as to be able to browse the results.)


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Tools Validation WS:Core Work stream for Plugin core
Projects
None yet
Development

No branches or pull requests

3 participants