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

Extend WordPress.tv embed handler to also handle VideoPress #4755

Merged
merged 9 commits into from
May 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
lib/optimizer/resources/local_fallback/* linguist-generated=true
lib/optimizer/tests/spec/* linguist-generated=true
12 changes: 4 additions & 8 deletions includes/embeds/class-amp-wordpress-tv-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,12 @@
/**
* Class AMP_WordPress_TV_Embed_Handler
*
* This sanitizes embeds both for WordPress.tv and for VideoPress (which use the same underlying infrastructure).
*
* @since 1.4
*/
class AMP_WordPress_TV_Embed_Handler extends AMP_Base_Embed_Handler {

/**
* The URL pattern to determine if an embed URL is for this type, copied from WP_oEmbed.
*
* @see https://github.com/WordPress/wordpress-develop/blob/e13480/src/wp-includes/class-wp-oembed.php#L64
*/
const URL_PATTERN = '#https?://wordpress\.tv/.*#i';

/**
* Register embed.
*/
Expand All @@ -42,7 +37,8 @@ public function unregister_embed() {
* @return string The filtered embed markup.
*/
public function filter_oembed_html( $cache, $url ) {
if ( ! preg_match( self::URL_PATTERN, $url ) ) {
$host = wp_parse_url( $url, PHP_URL_HOST );
if ( ! in_array( $host, [ 'wordpress.tv', 'videopress.com' ], true ) ) {
return $cache;
}

Expand Down
7 changes: 4 additions & 3 deletions lib/common/src/RemoteRequest/FilesystemRemoteGetRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@ final class FilesystemRemoteGetRequest implements RemoteGetRequest
{

/**
* Associative array of data for mapping between arguments and returned results.
* Associative array of data for mapping between arguments and filepaths pointing to the results to return.
*
* @var array
*/
private $argumentMap;

/**
* Instantiate a StubbedRemoteGetRequest object.
* Instantiate a FilesystemRemoteGetRequest object.
*
* @param array $argumentMap Associative array of data for mapping between arguments and returned results.
* @param array $argumentMap Associative array of data for mapping between arguments and filepaths pointing to the
* results to return.
*/
public function __construct($argumentMap)
{
Expand Down
2 changes: 1 addition & 1 deletion lib/optimizer/resources/local_fallback/rtv/metadata
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"ampRuntimeVersion":"012004030010070","ampCssUrl":"https://cdn.ampproject.org/rtv/012004030010070/v0.css","canaryPercentage":"0.005","diversions":["002004012111560","002004012158290","022004030010070","032004012111560","032004012158290","042004030033570"],"ltsRuntimeVersion":"012002251816300","ltsCssUrl":"https://cdn.ampproject.org/rtv/012002251816300/v0.css"}
{"ampRuntimeVersion":"012004252135000","ampCssUrl":"https://cdn.ampproject.org/rtv/012004252135000/v0.css","canaryPercentage":"0.005","diversions":["002004252135000","002005050322000","022004252135000","032004252135000","032005050322000","042005051629000","052004252135000","102004252135000"],"ltsRuntimeVersion":"012004030010070","ltsCssUrl":"https://cdn.ampproject.org/rtv/012004030010070/v0.css"}
4 changes: 2 additions & 2 deletions lib/optimizer/resources/local_fallback/v0.css

Large diffs are not rendered by default.

13 changes: 0 additions & 13 deletions lib/optimizer/src/Configurable.php

This file was deleted.

19 changes: 19 additions & 0 deletions lib/optimizer/src/Configuration/AmpRuntimeCssConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,21 @@ final class AmpRuntimeCssConfiguration extends BaseTransformerConfiguration
/**
* Configuration key that holds the version number to use.
*
* If the version is not provided, the latest runtime version is fetched from cdn.ampproject.org.
*
* @var string
*/
const VERSION = 'version';

/**
* Configuration key that holds the actual runtime CSS styles to use.
*
* If the styles are not provided, the latest runtime styles are fetched from cdn.ampproject.org.
*
* @var string
*/
const STYLES = 'styles';

/**
* Configuration key that holds the flag for the canary version of the runtime style.
*
Expand All @@ -42,6 +53,7 @@ protected function getAllowedKeys()
{
return [
self::VERSION => '',
self::STYLES => '',
self::CANARY => false,
];
}
Expand All @@ -63,6 +75,13 @@ protected function validate($key, $value)
$value = trim($value);
break;

case self::STYLES:
if (! is_string($value)) {
throw InvalidConfigurationValue::forInvalidSubValueType(self::class, self::STYLES, 'string', gettype($value));
}
$value = trim($value);
break;

case self::CANARY:
if (! is_bool($value)) {
throw InvalidConfigurationValue::forInvalidSubValueType(self::class, self::CANARY, 'boolean', gettype($value));
Expand Down
13 changes: 0 additions & 13 deletions lib/optimizer/src/MakesRemoteRequests.php

This file was deleted.

80 changes: 59 additions & 21 deletions lib/optimizer/src/TransformationEngine.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
use AmpProject\Dom\Document;
use AmpProject\RemoteGetRequest;
use AmpProject\RemoteRequest\CurlRemoteGetRequest;
use ReflectionClass;
use ReflectionException;

/**
* Transformation engine that accepts HTML and returns optimized HTML.
Expand All @@ -28,17 +30,26 @@ final class TransformationEngine
*/
private $remoteRequest;

/**
* Collection of transformers that were initialized.
*
* @var Transformer[]
*/
private $transformers;

/**
* Instantiate a TransformationEngine object.
*
* @param Configuration $configuration Configuration data to use for setting up the transformers.
* @param RemoteGetRequest $remoteRequest Optional. Transport to use for remote requests. Defaults to the
* @param Configuration|null $configuration Optional. Configuration data to use for setting up the transformers.
* @param RemoteGetRequest|null $remoteRequest Optional. Transport to use for remote requests. Defaults to the
* CurlRemoteGetRequest implementation shipped with the library.
*/
public function __construct(Configuration $configuration, RemoteGetRequest $remoteRequest = null)
public function __construct(Configuration $configuration = null, RemoteGetRequest $remoteRequest = null)
{
$this->configuration = $configuration;
$this->configuration = isset($configuration) ? $configuration : new Configuration();
$this->remoteRequest = isset($remoteRequest) ? $remoteRequest : new CurlRemoteGetRequest();

$this->initializeTransformers();
}

/**
Expand All @@ -50,7 +61,7 @@ public function __construct(Configuration $configuration, RemoteGetRequest $remo
*/
public function optimizeDom(Document $document, ErrorCollection $errors)
{
foreach ($this->getTransformers() as $transformer) {
foreach ($this->transformers as $transformer) {
$transformer->transform($document, $errors);
}
}
Expand All @@ -71,31 +82,58 @@ public function optimizeHtml($html, ErrorCollection $errors)
}

/**
* Get the array of transformers to use.
* Initialize the array of transformers to use.
*/
private function initializeTransformers()
{
$this->transformers = [];

foreach ($this->configuration->get(Configuration::KEY_TRANSFORMERS) as $transformerClass) {
$this->transformers[$transformerClass] = new $transformerClass(
...$this->getTransformerDependencies($transformerClass)
);
}
}

/**
* Get the dependencies of a transformer and put them in the correct order.
*
* @return Transformer[] Array of transformers to use.
* @param string $transformerClass Class of the transformer to get the dependencies for.
* @return array Array of dependencies in the order as they appear in the transformer's constructor.
* @throws ReflectionException If the transformer could not be reflected upon.
*/
private function getTransformers()
private function getTransformerDependencies($transformerClass)
{
static $transformers = null;
$constructor = (new ReflectionClass($transformerClass))->getConstructor();

if ($constructor === null) {
return [];
}

if (null === $transformers) {
$transformers = [];
foreach ($this->configuration->get(Configuration::KEY_TRANSFORMERS) as $transformerClass) {
$arguments = [];
$dependencies = [];
foreach ($constructor->getParameters() as $parameter) {
$dependencyType = $parameter->getClass();

if (is_a($transformerClass, MakesRemoteRequests::class, true)) {
$arguments[] = $this->remoteRequest;
}
if ($dependencyType === null) {
// No type provided, so we pass `null` in the hopes that the argument is optional.
$dependencies[] = null;
continue;
}

if (is_a($transformerClass, Configurable::class, true)) {
$arguments[] = $this->configuration->getTransformerConfiguration($transformerClass);
}
if (is_a($dependencyType->name, TransformerConfiguration::class, true)) {
$dependencies[] = $this->configuration->getTransformerConfiguration($transformerClass);
continue;
}

$transformers[$transformerClass] = new $transformerClass(...$arguments);
if (is_a($dependencyType->name, RemoteGetRequest::class, true)) {
$dependencies[] = $this->remoteRequest;
continue;
}

// Unknown dependency type, so we pass `null` in the hopes that the argument is optional.
$dependencies[] = null;
}

return $transformers;
return $dependencies;
}
}
40 changes: 23 additions & 17 deletions lib/optimizer/src/Transformer/AmpRuntimeCss.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
use AmpProject\Amp;
use AmpProject\Attribute;
use AmpProject\Dom\Document;
use AmpProject\Optimizer\Configurable;
use AmpProject\Optimizer\Configuration\AmpRuntimeCssConfiguration;
use AmpProject\Optimizer\Error;
use AmpProject\Optimizer\ErrorCollection;
use AmpProject\Optimizer\MakesRemoteRequests;
use AmpProject\Optimizer\TransformerConfiguration;
use AmpProject\RemoteGetRequest;
use AmpProject\Optimizer\Transformer;
Expand All @@ -36,7 +34,7 @@
*
* @package ampproject/optimizer
*/
final class AmpRuntimeCss implements Transformer, Configurable, MakesRemoteRequests
final class AmpRuntimeCss implements Transformer
{

/**
Expand All @@ -61,29 +59,29 @@ final class AmpRuntimeCss implements Transformer, Configurable, MakesRemoteReque
const V0_CSS_URL = Amp::CACHE_HOST . '/' . self::V0_CSS;

/**
* Transport to use for remote requests.
* Configuration store to use.
*
* @var RemoteGetRequest
* @var TransformerConfiguration
*/
private $remoteRequest;
private $configuration;

/**
* Configuration store to use.
* Transport to use for remote requests.
*
* @var TransformerConfiguration
* @var RemoteGetRequest
*/
private $configuration;
private $remoteRequest;

/**
* Instantiate an AmpRuntimeCss object.
*
* @param RemoteGetRequest $remoteRequest Transport to use for remote requests.
* @param TransformerConfiguration $configuration Configuration store to use.
* @param RemoteGetRequest $remoteRequest Transport to use for remote requests.
*/
public function __construct(RemoteGetRequest $remoteRequest, TransformerConfiguration $configuration)
public function __construct(TransformerConfiguration $configuration, RemoteGetRequest $remoteRequest)
{
$this->remoteRequest = $remoteRequest;
$this->configuration = $configuration;
$this->remoteRequest = $remoteRequest;
}

/**
Expand Down Expand Up @@ -143,6 +141,7 @@ private function addStaticCss(Document $document, DOMElement $ampRuntimeStyle, E
} catch (Exception $exception) {
$errors->add(Error\CannotInlineRuntimeCss::fromException($exception, $ampRuntimeStyle, $version));
$this->linkCss($document, $ampRuntimeStyle);
$ampRuntimeStyle->parentNode->removeChild($ampRuntimeStyle);
}
}

Expand All @@ -164,14 +163,21 @@ private function inlineCss(DOMElement $ampRuntimeStyle, $version)
}

$ampRuntimeStyle->setAttribute(Attribute::I_AMPHTML_VERSION, $version);
$response = $this->remoteRequest->get($v0CssUrl);
$statusCode = $response->getStatusCode();

if ($statusCode < 200 || $statusCode >= 300) {
return;
$styles = $this->configuration->get(AmpRuntimeCssConfiguration::STYLES);

if (empty($styles)) {
$response = $this->remoteRequest->get($v0CssUrl);
$statusCode = $response->getStatusCode();

if (200 < $statusCode || $statusCode >= 300) {
return;
}

$styles = $response->getBody();
}

$ampRuntimeStyle->textContent = $response->getBody();
$ampRuntimeStyle->textContent = $styles;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/optimizer/src/Transformer/ReorderHead.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* ReorderHead reorders the children of <head>. Specifically, it
* orders the <head> like so:
* (0) <meta charset> tag
* (1) <style amp-runtime> (inserted by ampruntimecss.go)
* (1) <style amp-runtime> (inserted by AmpRuntimeCss transformer)
* (2) remaining <meta> tags (those other than <meta charset>)
* (3) AMP runtime .js <script> tag
* (4) AMP viewer runtime .js <script>
Expand Down
3 changes: 1 addition & 2 deletions lib/optimizer/src/Transformer/TransformedIdentifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace AmpProject\Optimizer\Transformer;

use AmpProject\Dom\Document;
use AmpProject\Optimizer\Configurable;
use AmpProject\Optimizer\Configuration\TransformedIdentifierConfiguration;
use AmpProject\Optimizer\ErrorCollection;
use AmpProject\Optimizer\Transformer;
Expand All @@ -25,7 +24,7 @@
*
* @package ampproject/optimizer
*/
final class TransformedIdentifier implements Transformer, Configurable
final class TransformedIdentifier implements Transformer
{

/**
Expand Down
Loading