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

Discovery: Architectural design v2 #3080

Closed
schlessera opened this issue Aug 23, 2019 · 5 comments
Closed

Discovery: Architectural design v2 #3080

schlessera opened this issue Aug 23, 2019 · 5 comments
Labels
Discussion For issues that are high-level and not yet ready to implement. Epic Groomed P2 Low priority WS:Perf Work stream for Metrics, Performance and Optimizer

Comments

@schlessera
Copy link
Collaborator

schlessera commented Aug 23, 2019

The organically grown architecture of the plugin currently has a few limitations within its WordPress application, and more-so renders most of the code useless outside of the WordPress context.

Given the long-term goal of providing official AMP support (through libraries or extensions) to more frameworks/applications than just WordPress (#2315), it makes sense to rethink the current architecture and overhaul it to better fit the goals of this long-term plan.

First of all, there should be a clear separation between:
a.) Server-side PHP code to deal with AMP (validation, sanitization, embed handling, ...)
b.) WordPress server-side integration (hooking up to WP routing, integrating into WP admin interface, ...)
c.) WordPress client-side integration (AMP Stories, ...)

The code for a.) should be generic PHP code (not using any WP functionality) pulled in as a Composer package.

The code for b.) and c.) should be a WordPress plugin. While they can certainly be housed within the same plugin, it might at one point make sense to separate them for diverging release cycles.

Some requirements we need to keep in mind:

  • The code needs to stay either fully backwards compatible, or provide a sane migration path with ample time for the ecosystem to adapt. The specifics of this depend on the subsystem in question, as not all of them are equally used by third-party code.
  • The strategy should not rely on an "all-or-nothing" approach, as we still need to focus on providing regular improvements to maintain the momentum.
  • The focus should not be 100% on creating a pure abstraction, but rather progressing towards a clean separation of concerns that is easy to maintain and adapt. The abstraction itself should only be finalized with more targets (like Drupal or Joomla) actively involved. Starting too soon with finalizing the abstraction could still yield an unusable end result for other CMSes.
  • A proper OOP approach should be used, to allow for easy changes down the road that can be completely transparent in terms of BC.

Considering the above, I suggest progressing in the following way:
1.) Reserve the PHP namespace AMP as the target for housing the refactored code. This gives us a clean slate in terms of naming.
2.) Start the redesign & refactoring process with a single subsystem at a time. Use the "Strangler Pattern" to move more and more subsystems over from the old codebase to the new codebase.
3.) Use an "anticorruption layer" between the old code and the new code, so that we don't need to compromise the new code to interact with old subsystems, but rather have a translation layer in-between. Otherwise, the new code would immediately start off on the wrong foot just to interoperate with the old one.
4.) Subsystems should slowly move from "old code as source of truth <=> new code as stubs executing old code" to "old code as stubs executing new code <=> new code as source of truth". So while we start with the new code getting a performance hit for overhead, we will at one point reverse this and have the performance hit in the old code.
5.) The old subsystem will receive deprecation notices as soon as the new subsystem is ready for prime time.

This approach lets us move the code over to a new design one subsystem at a time, while keeping things running smoothly and without (unmanaged) breaks in compatibility.

@schlessera schlessera added the Discussion For issues that are high-level and not yet ready to implement. label Aug 23, 2019
@schlessera
Copy link
Collaborator Author

Quick note regarding the generic PHP library:

At its most basic, the sanitization subsystem of the library should accept a string of HTML and return a string of valid AMP-HTML.

Due to the way most modern PHP frameworks are currently designed, we can then build a bridge package to provide a PSR-15 middleware that can be hooked up to most recent frameworks' controllers without additional code. So, providing an AMP endpoint in Laravel, for example, would just be a matter of pulling in said package and mapping an endpoint URL to use the provided middleware.

@schlessera
Copy link
Collaborator Author

schlessera commented Aug 23, 2019

Related:

@amedina
Copy link
Member

amedina commented Apr 1, 2020

@schlessera can we start by creating an Architecture/Design PRD doc to move towards the making the changes in a concerted way?

@amedina
Copy link
Member

amedina commented Apr 1, 2020

Related to: #3077

@westonruter
Copy link
Member

Also related #1478, a PR which I just closed that addresses a need for plugins to be able to filter default args that are passed into the sanitizers. Either that PR should be resurrected, or it should be addressed as part of the re-architecture of the sanitizers.

@kmyram kmyram added the WS:Perf Work stream for Metrics, Performance and Optimizer label Aug 5, 2020
@kmyram kmyram added the Epic label Feb 2, 2021
@kmyram kmyram added this to the v2.2 milestone Feb 2, 2021
@kmyram kmyram added P2 Low priority Groomed labels Feb 2, 2021
@westonruter westonruter modified the milestones: v2.2, v2.3 Jun 18, 2021
@westonruter westonruter modified the milestones: v2.3, v2.4 Dec 23, 2021
@westonruter westonruter removed this from the v2.4 milestone Apr 14, 2022
@westonruter westonruter closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2022
@github-project-automation github-project-automation bot moved this to Backlog in Ongoing Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion For issues that are high-level and not yet ready to implement. Epic Groomed P2 Low priority WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
Archived in project
Development

No branches or pull requests

4 participants