From 2c5e482de0f809165cc38d6511d0dce7d7ac3749 Mon Sep 17 00:00:00 2001 From: Christopher Joe Date: Mon, 20 Mar 2017 14:02:43 +1300 Subject: [PATCH 1/3] Add LabelField component definition --- src/Forms/LabelField.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Forms/LabelField.php b/src/Forms/LabelField.php index 5d870e9c6..5b963f8c4 100644 --- a/src/Forms/LabelField.php +++ b/src/Forms/LabelField.php @@ -9,6 +9,10 @@ namespace SilverStripe\Forms; */ class LabelField extends DatalessField { + protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_STRUCTURAL; + + protected $schemaComponent = 'LabelField'; + /** * @param string $name * @param null|string $title From 9be22701fd31123e4cb63102c04c6eb4b82b8792 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 3 Apr 2017 16:29:48 +1200 Subject: [PATCH 2/3] API exists() no longer true for nullifyIfEmpty if empty string Optimise DBString::exists() to skip shortcodes --- src/ORM/FieldType/DBHTMLText.php | 8 ++++++++ src/ORM/FieldType/DBHTMLVarchar.php | 8 ++++++++ src/ORM/FieldType/DBString.php | 5 ++--- tests/php/ORM/DBFieldTest.php | 4 ++-- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/ORM/FieldType/DBHTMLText.php b/src/ORM/FieldType/DBHTMLText.php index bc7c83fb4..c3624a701 100644 --- a/src/ORM/FieldType/DBHTMLText.php +++ b/src/ORM/FieldType/DBHTMLText.php @@ -242,4 +242,12 @@ class DBHTMLText extends DBText } return null; } + + public function exists() + { + // Optimisation: don't process shortcode just for ->exists() + $value = $this->getValue(); + // All truthy values and non-empty strings exist ('0' but not (int)0) + return $value || (is_string($value) && strlen($value)); + } } diff --git a/src/ORM/FieldType/DBHTMLVarchar.php b/src/ORM/FieldType/DBHTMLVarchar.php index 4ce7ebc4a..561ad4c4c 100644 --- a/src/ORM/FieldType/DBHTMLVarchar.php +++ b/src/ORM/FieldType/DBHTMLVarchar.php @@ -141,4 +141,12 @@ class DBHTMLVarchar extends DBVarchar } return null; } + + public function exists() + { + // Optimisation: don't process shortcode just for ->exists() + $value = $this->getValue(); + // All truthy values and non-empty strings exist ('0' but not (int)0) + return $value || (is_string($value) && strlen($value)); + } } diff --git a/src/ORM/FieldType/DBString.php b/src/ORM/FieldType/DBString.php index cdfeb9b8f..de5e4730b 100644 --- a/src/ORM/FieldType/DBString.php +++ b/src/ORM/FieldType/DBString.php @@ -96,9 +96,8 @@ abstract class DBString extends DBField public function exists() { $value = $this->RAW(); - return $value // All truthy values exist - || (is_string($value) && strlen($value)) // non-empty strings exist ('0' but not (int)0) - || (!$this->getNullifyEmpty() && $value === ''); // Remove this stupid exemption in 4.0 + // All truthy values and non-empty strings exist ('0' but not (int)0) + return $value || (is_string($value) && strlen($value)); } public function prepValueForDB($value) diff --git a/tests/php/ORM/DBFieldTest.php b/tests/php/ORM/DBFieldTest.php index 2d9c88f28..8946d72c9 100644 --- a/tests/php/ORM/DBFieldTest.php +++ b/tests/php/ORM/DBFieldTest.php @@ -205,7 +205,7 @@ class DBFieldTest extends SapphireTest $varcharField->setValue('abc'); $this->assertTrue($varcharField->exists()); $varcharField->setValue(''); - $this->assertTrue($varcharField->exists()); + $this->assertFalse($varcharField->exists()); $varcharField->setValue(null); $this->assertFalse($varcharField->exists()); @@ -223,7 +223,7 @@ class DBFieldTest extends SapphireTest $textField->setValue('abc'); $this->assertTrue($textField->exists()); $textField->setValue(''); - $this->assertTrue($textField->exists()); + $this->assertFalse($textField->exists()); $textField->setValue(null); $this->assertFalse($textField->exists()); } From e61257c27b42a29bf45acaf8e5a15a13a1e9ed34 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 3 Apr 2017 16:30:01 +1200 Subject: [PATCH 3/3] API Update embed/embed to 3.0 API Better shortcode generation for embed shortcodes --- composer.json | 2 +- .../HTMLEditor/EmbedShortcodeProvider.php | 138 +++++++++++++++--- .../HTMLEditor/HTMLEditorField_Embed.php | 9 +- .../php/Forms/EmbedShortcodeProviderTest.php | 103 ++++++++++--- .../MockResolver.php | 63 ++++++++ 5 files changed, 269 insertions(+), 46 deletions(-) create mode 100644 tests/php/Forms/EmbedShortcodeProviderTest/MockResolver.php diff --git a/composer.json b/composer.json index 603756f00..62116a004 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=5.6.0", "composer/installers": "~1.0", - "embed/embed": "^2.6", + "embed/embed": "^3.0", "league/flysystem": "~1.0.12", "monolog/monolog": "~1.11", "nikic/php-parser": "^2 || ^3", diff --git a/src/Forms/HTMLEditor/EmbedShortcodeProvider.php b/src/Forms/HTMLEditor/EmbedShortcodeProvider.php index 05df0f5fe..78cee9c28 100644 --- a/src/Forms/HTMLEditor/EmbedShortcodeProvider.php +++ b/src/Forms/HTMLEditor/EmbedShortcodeProvider.php @@ -2,9 +2,13 @@ namespace SilverStripe\Forms\HtmlEditor; +use SilverStripe\Core\Convert; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Forms\FormField; use SilverStripe\View\Parsers\ShortcodeHandler; use Embed\Adapters\Adapter; use Embed\Embed; +use SilverStripe\View\Parsers\ShortcodeParser; /** * Provider for the [embed] shortcode tag used by the embedding service @@ -28,47 +32,139 @@ class EmbedShortcodeProvider implements ShortcodeHandler * Embed shortcode parser from Oembed. This is a temporary workaround. * Oembed class has been replaced with the Embed external service. * - * @param $arguments - * @param $content - * @param $parser - * @param $shortcode + * @param array $arguments + * @param string $content + * @param ShortcodeParser $parser + * @param string $shortcode * @param array $extra * * @return string */ public static function handle_shortcode($arguments, $content, $parser, $shortcode, $extra = array()) { - $embed = Embed::create($content, $arguments); - if ($embed && $embed instanceof Adapter) { - return self::embedForTemplate($embed); + // Get service URL + if (!empty($content)) { + $serviceURL = $content; + } elseif (!empty($arguments['url'])) { + $serviceURL = $arguments['url']; } else { - return '' . $content . ''; + return ''; } + + // See https://github.com/oscarotero/Embed#example-with-all-options for service arguments + $serviceArguments = []; + if (!empty($arguments['width'])) { + $serviceArguments['min_image_width'] = $arguments['width']; + } + if (!empty($arguments['height'])) { + $serviceArguments['min_image_height'] = $arguments['height']; + } + + // Allow resolver to be mocked + $dispatcher = null; + if (isset($extra['resolver'])) { + $dispatcher = Injector::inst()->create( + $extra['resolver']['class'], + $serviceURL, + $extra['resolver']['config'] + ); + } + + // Process embed + $embed = Embed::create($serviceURL, $serviceArguments, $dispatcher); + + // Convert embed object into HTML + if ($embed && $embed instanceof Adapter) { + $result = static::embedForTemplate($embed, $arguments); + if ($result) { + return $result; + } + } + + // Fallback to link to service + return static::linkEmbed($arguments, $serviceURL, $serviceURL); } /** * @param Adapter $embed - * + * @param array $arguments Additional shortcode params * @return string */ - public static function embedForTemplate($embed) + public static function embedForTemplate($embed, $arguments) { - switch ($embed->type) { + switch ($embed->getType()) { case 'video': case 'rich': - if ($embed->extraClass) { - return "
$embed->code
"; - } else { - return "
$embed->code
"; + // Attempt to inherit width (but leave height auto) + if (empty($arguments['width']) && $embed->getWidth()) { + $arguments['width'] = $embed->getWidth(); } - break; + return self::videoEmbed($arguments, $embed->getCode()); case 'link': - return '' . $embed->title . ''; - break; + return self::linkEmbed($arguments, $embed->getUrl(), $embed->getTitle()); case 'photo': - return ""; - break; + return self::photoEmbed($arguments, $embed->getUrl()); + default: + return null; } - return null; + } + + /** + * Build video embed tag + * + * @param array $arguments + * @param string $content Raw HTML content + * @return string + */ + protected static function videoEmbed($arguments, $content) + { + // Ensure outer div has given width (but leave height auto) + if (!empty($arguments['width'])) { + $arguments['style'] = 'width: ' . intval($arguments['width']) . 'px;'; + } + + // Convert caption to

+ if (!empty($arguments['caption'])) { + $xmlCaption = Convert::raw2xml($arguments['caption']); + $content .= "\n

{$xmlCaption}

"; + } + unset($arguments['width']); + unset($arguments['height']); + unset($arguments['url']); + unset($arguments['caption']); + return FormField::create_tag('div', $arguments, $content); + } + + /** + * Build embed tag + * + * @param array $arguments + * @param string $href + * @param string $title Default title + * @return string + */ + protected static function linkEmbed($arguments, $href, $title) + { + $title = !empty($arguments['caption']) ? ($arguments['caption']) : $title; + unset($arguments['caption']); + unset($arguments['width']); + unset($arguments['height']); + unset($arguments['url']); + $arguments['href'] = $href; + return Formfield::create_tag('a', $arguments, Convert::raw2xml($title)); + } + + /** + * Build img embed tag + * + * @param array $arguments + * @param string $src + * @return string + */ + protected static function photoEmbed($arguments, $src) + { + $arguments['src'] = $src; + unset($arguments['url']); + return FormField::create_tag('img', $arguments); } } diff --git a/src/Forms/HTMLEditor/HTMLEditorField_Embed.php b/src/Forms/HTMLEditor/HTMLEditorField_Embed.php index 31b801986..2ab09e922 100644 --- a/src/Forms/HTMLEditor/HTMLEditorField_Embed.php +++ b/src/Forms/HTMLEditor/HTMLEditorField_Embed.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms\HTMLEditor; +use Embed\Adapters\Adapter; use SilverStripe\Assets\File; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPResponse_Exception; @@ -26,7 +27,7 @@ class HTMLEditorField_Embed extends HTMLEditorField_File /** * Embed result * - * @var Embed + * @var Adapter */ protected $embed; @@ -60,7 +61,7 @@ class HTMLEditorField_Embed extends HTMLEditorField_File public function getFields() { $fields = parent::getFields(); - if ($this->Type === 'photo') { + if ($this->getType() === 'photo') { $fields->insertBefore('CaptionText', new TextField( 'AltText', _t('HTMLEditorField.IMAGEALTTEXT', 'Alternative text (alt) - shown if image can\'t be displayed'), @@ -82,7 +83,7 @@ class HTMLEditorField_Embed extends HTMLEditorField_File */ public function getWidth() { - return $this->embed->width ?: 100; + return $this->embed->getWidth() ?: 100; } /** @@ -92,7 +93,7 @@ class HTMLEditorField_Embed extends HTMLEditorField_File */ public function getHeight() { - return $this->embed->height ?: 100; + return $this->embed->getHeight() ?: 100; } public function getPreviewURL() diff --git a/tests/php/Forms/EmbedShortcodeProviderTest.php b/tests/php/Forms/EmbedShortcodeProviderTest.php index 67d11e511..8d37ee09c 100644 --- a/tests/php/Forms/EmbedShortcodeProviderTest.php +++ b/tests/php/Forms/EmbedShortcodeProviderTest.php @@ -2,10 +2,9 @@ namespace SilverStripe\Forms\Tests; -use Embed\Adapters\Webpage; -use Embed\Embed; use SilverStripe\Forms\HtmlEditor\EmbedShortcodeProvider; use SilverStripe\Dev\SapphireTest; +use SilverStripe\framework\tests\php\Forms\EmbedShortcodeProviderTest\MockResolver; /** * Class EmbedShortcodeProviderTest @@ -29,27 +28,91 @@ class EmbedShortcodeProviderTest extends SapphireTest public function testYoutube() { - /** @var Webpage $result */ - $result = Embed::create(self::$test_youtube, array()); - self::assertEquals($result->providerName, 'YouTube'); - $embedded = EmbedShortcodeProvider::embedForTemplate($result); - self::assertContains("
mockRequest( + [ + 'url' => static::$test_youtube, + 'caption' => 'A nice video', + 'width' => 480, + 'height' => 360, + ], + [ + 'version' => '1.0', + 'provider_url' => 'https://www.youtube.com/', + 'title' => 'SilverStripe Platform 2 min introduction', + 'html' => '', + 'provider_name' => 'YouTube', + 'thumbnail_width' => 480, + 'type' => 'video', + 'thumbnail_url' => 'https://i.ytimg.com/vi/dM15HfUYwF0/hqdefault.jpg', + 'thumbnail_height' => 360, + 'width' => 480, + 'author_url' => 'https://www.youtube.com/user/SilverStripe', + 'author_name' => 'SilverStripe', + 'height' => 270, + ] + ); + $this->assertEquals( + << +

A nice video

+EOS + , + $result + ); } public function testSoundcloud() { - /** @var Webpage $result */ - $result = Embed::create(self::$test_soundcloud, array()); - self::assertEquals($result->providerName, 'SoundCloud'); - $embedded = EmbedShortcodeProvider::embedForTemplate($result); - self::assertContains("
mockRequest( + ['url' => static::$test_soundcloud], + [ + 'version' => 1, + 'type' => 'rich', + 'provider_name' => 'SoundCloud', + 'provider_url' => 'http://soundcloud.com', + 'height' => 400, + 'width' => '100%', + 'title' => 'DELAIN - Suckerpunch by Napalm Records', + 'description' => 'Taken from the EP "Lunar Prelude": http://shop.napalmrecords.com/delain', + 'thumbnail_url' => 'http://i1.sndcdn.com/artworks-000143578557-af0v6l-t500x500.jpg', + 'html' => '', + 'author_name' => 'Napalm Records', + 'author_url' => 'http://soundcloud.com/napalmrecords', + ] + ); + $this->assertEquals( + <<
+EOS + , + $result + ); + } + + /** + * Mock an oembed request + * + * @param array $arguments Input arguments + * @param array $response JSON response body + * @return string + */ + protected function mockRequest($arguments, $response) + { + return EmbedShortcodeProvider::handle_shortcode( + $arguments, + '', + null, + 'embed', + [ + 'resolver' => [ + 'class' => MockResolver::class, + 'config' => [ + 'expectedContent' => json_encode($response), + ], + ], + ] + ); } } diff --git a/tests/php/Forms/EmbedShortcodeProviderTest/MockResolver.php b/tests/php/Forms/EmbedShortcodeProviderTest/MockResolver.php new file mode 100644 index 000000000..bfe9dfe42 --- /dev/null +++ b/tests/php/Forms/EmbedShortcodeProviderTest/MockResolver.php @@ -0,0 +1,63 @@ +url = $url; + if (empty($config['expectedContent'])) { + throw new InvalidArgumentException("Mock resolvers need expectedContent"); + } + $this->expectedContent = $config['expectedContent']; + } + + /** + * Dispatch an url. + * + * @param Url $url + * + * @return Response + */ + public function dispatch(Url $url) + { + return new Response( + $url, + $url, + 200, + 'application/json', + $this->expectedContent, + [], + [] + ); + } + + /** + * Resolve multiple image urls at once. + * + * @param Url[] $urls + * + * @return ImageResponse[] + */ + public function dispatchImages(array $urls) + { + return []; + } +}