Build abstraction layer

[schlessera]

An abstraction layer of some sorts would make sense for this plugin for three reasons:

• Deduplication of code across plugin logic, REST API controllers and CLI commands.
• Making the interface provided to third-party plugins/themes as explicit and controlled as possible.
• Enabling changes in technological decisions while maintaining backward compatibility.

This issue is meant as an umbrella ticket to collect initial discussion, with the actual execution later split out into separate issues as needed.

I have still to dive deeper into the code base, but my initial feeling is that we should tackle this in two different steps:
1.) Provide an abstraction for the pure data model (what is referred to as entities and repositories in DDD).
2.) Model the remaining problem domain as a superset on top of that data model abstraction.

This two-step approach allows to get a first abstraction layer implemented in a relatively short amount of time and already reap a large part of the benefits. The second step will entail a lot of discussion and planning, and might even be premature given the critical design work that is still being done.

The data model we might cover in this first iteration:

• Custom post type 'amp_validated_url'
• Custom taxonomy 'amp_validation_error'
• Custom post type 'amp_story'
• Custom taxonomy 'amp_template'

There are already some forms of abstractions in place, but they seem to mix multiple concerns. As an example, the AMP_Validation_Error_Taxonomy object does the following:

• Register the storage mechanism for the persisting validation errors.
• Provides CRUD mechanisms to manage validation errors (it returns WP_Term objects, though).
• Provides SQL helpers.
• Adds filters to hook the processing into WP.
• Adds admin backend integration.
• Adds rendered output, like admin notices and error pages.

While the above is perfectly fine for a regular WordPress plugin, it causes a lot of issues if you need to use the code in multiple contexts. That's probably one of the reasons why you can find actual SQL queries in the CLI commands, which should actually do nothing more than mapping STDIN to function arguments and function results to STDOUT.

The way I understand the purpose of the AMP plugin is that we want to provide a developer tool and an end-user-facing tool in one product. I think that the way to go here is not to mix the two into one code base, but rather to build one on top of the other. Ideally, the end goal should be to allow generic PHP code to interface with the AMP validation library to retrieve reports, irregardless of whether these validation errors were triggered on a WordPress site or a Drupal site. That is surely a lofty goal, but we should at least strive to work towards such a clean abstraction, it will yield countless maintenance benefits later down the road.

Therefore, given that we already have Composer support set up, it might even make sense to start building the data model abstraction in a separate package. Such a strict separation makes it easier to detect leaky abstractions, and is a joy to reuse in third-party packages that want to join the AMP validation fun.

All of the above is just my first thoughts based on the very limited context I have about the code so far, so please freely share any observations you have.

[westonruter]

Closely related to the efforts in #2316 to redesign the compatibility tool (where REST API endpoints for validation are needed) and #2315 (where sanitizer/embed logic should be extracted into CMS-agnostic package).

[amedina]

@schlessera This is related to this: #3080

We should capture both in a single design doc.