Merge pull request #1567 from ArthurHoaro/feature/async-title-retrieval

4 files changed, 347 insertions, 150 deletions

diff --git a/tests/bookmark/LinkUtilsTest.php b/tests/bookmark/LinkUtilsTest.php
index 29941c8c..3321242f 100644
--- a/tests/bookmark/LinkUtilsTest.php
+++ b/tests/bookmark/LinkUtilsTest.php
@@ -216,60 +216,91 @@ class LinkUtilsTest extends TestCase
     }
 
     /**
+     * Test the header callback with valid value
+     */
+    public function testCurlHeaderCallbackOk(): void
+    {
+        $callback = get_curl_header_callback($charset, 'ut_curl_getinfo_ok');
+        $data = [
+            'HTTP/1.1 200 OK',
+            'Server: GitHub.com',
+            'Date: Sat, 28 Oct 2017 12:01:33 GMT',
+            'Content-Type: text/html; charset=utf-8',
+            'Status: 200 OK',
+        ];
+
+        foreach ($data as $chunk) {
+            static::assertIsInt($callback(null, $chunk));
+        }
+
+        static::assertSame('utf-8', $charset);
+    }
+
+    /**
      * Test the download callback with valid value
      */
-    public function testCurlDownloadCallbackOk()
+    public function testCurlDownloadCallbackOk(): void
     {
+        $charset = 'utf-8';
         $callback = get_curl_download_callback(
             $charset,
             $title,
             $desc,
             $keywords,
-            false,
-            'ut_curl_getinfo_ok'
+            false
         );
+
         $data = [
-            'HTTP/1.1 200 OK',
-            'Server: GitHub.com',
-            'Date: Sat, 28 Oct 2017 12:01:33 GMT',
-            'Content-Type: text/html; charset=utf-8',
-            'Status: 200 OK',
-            'end' => 'th=device-width">'
+            'th=device-width">'
                 . '<title>Refactoring · GitHub</title>'
                 . '<link rel="search" type="application/opensea',
             '<title>ignored</title>'
                 . '<meta name="description" content="desc" />'
                 . '<meta name="keywords" content="key1,key2" />',
         ];
-        foreach ($data as $key => $line) {
-            $ignore = null;
-            $expected = $key !== 'end' ? strlen($line) : false;
-            $this->assertEquals($expected, $callback($ignore, $line));
-            if ($expected === false) {
-                break;
-            }
+
+        foreach ($data as $chunk) {
+            static::assertSame(strlen($chunk), $callback(null, $chunk));
         }
-        $this->assertEquals('utf-8', $charset);
-        $this->assertEquals('Refactoring · GitHub', $title);
-        $this->assertEmpty($desc);
-        $this->assertEmpty($keywords);
+
+        static::assertSame('utf-8', $charset);
+        static::assertSame('Refactoring · GitHub', $title);
+        static::assertEmpty($desc);
+        static::assertEmpty($keywords);
+    }
+
+    /**
+     * Test the header callback with valid value
+     */
+    public function testCurlHeaderCallbackNoCharset(): void
+    {
+        $callback = get_curl_header_callback($charset, 'ut_curl_getinfo_no_charset');
+        $data = [
+            'HTTP/1.1 200 OK',
+        ];
+
+        foreach ($data as $chunk) {
+            static::assertSame(strlen($chunk), $callback(null, $chunk));
+        }
+
+        static::assertFalse($charset);
     }
 
     /**
      * Test the download callback with valid values and no charset
      */
-    public function testCurlDownloadCallbackOkNoCharset()
+    public function testCurlDownloadCallbackOkNoCharset(): void
     {
+        $charset = null;
         $callback = get_curl_download_callback(
             $charset,
             $title,
             $desc,
             $keywords,
-            false,
-            'ut_curl_getinfo_no_charset'
+            false
         );
+
         $data = [
-            'HTTP/1.1 200 OK',
             'end' => 'th=device-width">'
                 . '<title>Refactoring · GitHub</title>'
                 . '<link rel="search" type="application/opensea',
@@ -277,10 +308,11 @@ class LinkUtilsTest extends TestCase
             . '<meta name="description" content="desc" />'
             . '<meta name="keywords" content="key1,key2" />',
         ];
-        foreach ($data as $key => $line) {
-            $ignore = null;
-            $this->assertEquals(strlen($line), $callback($ignore, $line));
+
+        foreach ($data as $chunk) {
+            static::assertSame(strlen($chunk), $callback(null, $chunk));
         }
+
         $this->assertEmpty($charset);
         $this->assertEquals('Refactoring · GitHub', $title);
         $this->assertEmpty($desc);
@@ -290,18 +322,18 @@ class LinkUtilsTest extends TestCase
     /**
      * Test the download callback with valid values and no charset
      */
-    public function testCurlDownloadCallbackOkHtmlCharset()
+    public function testCurlDownloadCallbackOkHtmlCharset(): void
     {
+        $charset = null;
         $callback = get_curl_download_callback(
             $charset,
             $title,
             $desc,
             $keywords,
-            false,
-            'ut_curl_getinfo_no_charset'
+            false
         );
+
         $data = [
-            'HTTP/1.1 200 OK',
             '<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />',
             'end' => 'th=device-width">'
                 . '<title>Refactoring · GitHub</title>'
@@ -310,14 +342,10 @@ class LinkUtilsTest extends TestCase
             . '<meta name="description" content="desc" />'
             . '<meta name="keywords" content="key1,key2" />',
         ];
-        foreach ($data as $key => $line) {
-            $ignore = null;
-            $expected = $key !== 'end' ? strlen($line) : false;
-            $this->assertEquals($expected, $callback($ignore, $line));
-            if ($expected === false) {
-                break;
-            }
+        foreach ($data as $chunk) {
+            static::assertSame(strlen($chunk), $callback(null, $chunk));
         }
+
         $this->assertEquals('utf-8', $charset);
         $this->assertEquals('Refactoring · GitHub', $title);
         $this->assertEmpty($desc);
@@ -327,25 +355,26 @@ class LinkUtilsTest extends TestCase
     /**
      * Test the download callback with valid values and no title
      */
-    public function testCurlDownloadCallbackOkNoTitle()
+    public function testCurlDownloadCallbackOkNoTitle(): void
     {
+        $charset = 'utf-8';
         $callback = get_curl_download_callback(
             $charset,
             $title,
             $desc,
             $keywords,
-            false,
-            'ut_curl_getinfo_ok'
+            false
         );
+
         $data = [
-            'HTTP/1.1 200 OK',
             'end' => 'th=device-width">Refactoring · GitHub<link rel="search" type="application/opensea',
             'ignored',
         ];
-        foreach ($data as $key => $line) {
-            $ignore = null;
-            $this->assertEquals(strlen($line), $callback($ignore, $line));
+
+        foreach ($data as $chunk) {
+            static::assertSame(strlen($chunk), $callback(null, $chunk));
         }
+
         $this->assertEquals('utf-8', $charset);
         $this->assertEmpty($title);
         $this->assertEmpty($desc);
@@ -353,81 +382,55 @@ class LinkUtilsTest extends TestCase
     }
 
     /**
-     * Test the download callback with an invalid content type.
+     * Test the header callback with an invalid content type.
      */
-    public function testCurlDownloadCallbackInvalidContentType()
+    public function testCurlHeaderCallbackInvalidContentType(): void
     {
-        $callback = get_curl_download_callback(
-            $charset,
-            $title,
-            $desc,
-            $keywords,
-            false,
-            'ut_curl_getinfo_ct_ko'
-        );
-        $ignore = null;
-        $this->assertFalse($callback($ignore, ''));
-        $this->assertEmpty($charset);
-        $this->assertEmpty($title);
+        $callback = get_curl_header_callback($charset, 'ut_curl_getinfo_ct_ko');
+        $data = [
+            'HTTP/1.1 200 OK',
+        ];
+
+        static::assertFalse($callback(null, $data[0]));
+        static::assertNull($charset);
     }
 
     /**
-     * Test the download callback with an invalid response code.
+     * Test the header callback with an invalid response code.
      */
-    public function testCurlDownloadCallbackInvalidResponseCode()
+    public function testCurlHeaderCallbackInvalidResponseCode(): void
     {
-        $callback = $callback = get_curl_download_callback(
-            $charset,
-            $title,
-            $desc,
-            $keywords,
-            false,
-            'ut_curl_getinfo_rc_ko'
-        );
-        $ignore = null;
-        $this->assertFalse($callback($ignore, ''));
-        $this->assertEmpty($charset);
-        $this->assertEmpty($title);
+        $callback = get_curl_header_callback($charset, 'ut_curl_getinfo_rc_ko');
+
+        static::assertFalse($callback(null, ''));
+        static::assertNull($charset);
     }
 
     /**
-     * Test the download callback with an invalid content type and response code.
+     * Test the header callback with an invalid content type and response code.
      */
-    public function testCurlDownloadCallbackInvalidContentTypeAndResponseCode()
+    public function testCurlHeaderCallbackInvalidContentTypeAndResponseCode(): void
     {
-        $callback = $callback = get_curl_download_callback(
-            $charset,
-            $title,
-            $desc,
-            $keywords,
-            false,
-            'ut_curl_getinfo_rs_ct_ko'
-        );
-        $ignore = null;
-        $this->assertFalse($callback($ignore, ''));
-        $this->assertEmpty($charset);
-        $this->assertEmpty($title);
+        $callback = get_curl_header_callback($charset, 'ut_curl_getinfo_rs_ct_ko');
+
+        static::assertFalse($callback(null, ''));
+        static::assertNull($charset);
     }
 
     /**
      * Test the download callback with valid value, and retrieve_description option enabled.
      */
-    public function testCurlDownloadCallbackOkWithDesc()
+    public function testCurlDownloadCallbackOkWithDesc(): void
     {
+        $charset = 'utf-8';
         $callback = get_curl_download_callback(
             $charset,
             $title,
             $desc,
             $keywords,
-            true,
-            'ut_curl_getinfo_ok'
+            true
         );
         $data = [
-            'HTTP/1.1 200 OK',
-            'Server: GitHub.com',
-            'Date: Sat, 28 Oct 2017 12:01:33 GMT',
-            'Content-Type: text/html; charset=utf-8',
-            'Status: 200 OK',
             'th=device-width">'
                 . '<title>Refactoring · GitHub</title>'
                 . '<link rel="search" type="application/opensea',
@@ -435,14 +438,11 @@ class LinkUtilsTest extends TestCase
             . '<meta name="description" content="link desc" />'
             . '<meta name="keywords" content="key1,key2" />',
         ];
-        foreach ($data as $key => $line) {
-            $ignore = null;
-            $expected = $key !== 'end' ? strlen($line) : false;
-            $this->assertEquals($expected, $callback($ignore, $line));
-            if ($expected === false) {
-                break;
-            }
+
+        foreach ($data as $chunk) {
+            static::assertSame(strlen($chunk), $callback(null, $chunk));
         }
+
         $this->assertEquals('utf-8', $charset);
         $this->assertEquals('Refactoring · GitHub', $title);
         $this->assertEquals('link desc', $desc);
@@ -453,8 +453,9 @@ class LinkUtilsTest extends TestCase
      * Test the download callback with valid value, and retrieve_description option enabled,
      * but no desc or keyword defined in the page.
      */
-    public function testCurlDownloadCallbackOkWithDescNotFound()
+    public function testCurlDownloadCallbackOkWithDescNotFound(): void
     {
+        $charset = 'utf-8';
         $callback = get_curl_download_callback(
             $charset,
             $title,
@@ -464,24 +465,16 @@ class LinkUtilsTest extends TestCase
             'ut_curl_getinfo_ok'
         );
         $data = [
-            'HTTP/1.1 200 OK',
-            'Server: GitHub.com',
-            'Date: Sat, 28 Oct 2017 12:01:33 GMT',
-            'Content-Type: text/html; charset=utf-8',
-            'Status: 200 OK',
             'th=device-width">'
                 . '<title>Refactoring · GitHub</title>'
                 . '<link rel="search" type="application/opensea',
             'end' => '<title>ignored</title>',
         ];
-        foreach ($data as $key => $line) {
-            $ignore = null;
-            $expected = $key !== 'end' ? strlen($line) : false;
-            $this->assertEquals($expected, $callback($ignore, $line));
-            if ($expected === false) {
-                break;
-            }
+
+        foreach ($data as $chunk) {
+            static::assertSame(strlen($chunk), $callback(null, $chunk));
         }
+
         $this->assertEquals('utf-8', $charset);
         $this->assertEquals('Refactoring · GitHub', $title);
         $this->assertEmpty($desc);
diff --git a/tests/container/ContainerBuilderTest.php b/tests/container/ContainerBuilderTest.php
index 5d52daef..3dadc0b9 100644
--- a/tests/container/ContainerBuilderTest.php
+++ b/tests/container/ContainerBuilderTest.php
@@ -12,6 +12,7 @@ use Shaarli\Front\Controller\Visitor\ErrorController;
 use Shaarli\Front\Controller\Visitor\ErrorNotFoundController;
 use Shaarli\History;
 use Shaarli\Http\HttpAccess;
+use Shaarli\Http\MetadataRetriever;
 use Shaarli\Netscape\NetscapeBookmarkUtils;
 use Shaarli\Plugin\PluginManager;
 use Shaarli\Render\PageBuilder;
@@ -72,6 +73,7 @@ class ContainerBuilderTest extends TestCase
         static::assertInstanceOf(History::class, $container->history);
         static::assertInstanceOf(HttpAccess::class, $container->httpAccess);
         static::assertInstanceOf(LoginManager::class, $container->loginManager);
+        static::assertInstanceOf(MetadataRetriever::class, $container->metadataRetriever);
         static::assertInstanceOf(NetscapeBookmarkUtils::class, $container->netscapeBookmarkUtils);
         static::assertInstanceOf(PageBuilder::class, $container->pageBuilder);
         static::assertInstanceOf(PageCacheManager::class, $container->pageCacheManager);
diff --git a/tests/front/controller/admin/ManageShaareControllerTest/DisplayCreateFormTest.php b/tests/front/controller/admin/ManageShaareControllerTest/DisplayCreateFormTest.php
index 2eb95251..4fd88480 100644
--- a/tests/front/controller/admin/ManageShaareControllerTest/DisplayCreateFormTest.php
+++ b/tests/front/controller/admin/ManageShaareControllerTest/DisplayCreateFormTest.php
@@ -9,6 +9,7 @@ use Shaarli\Config\ConfigManager;
 use Shaarli\Front\Controller\Admin\FrontAdminControllerMockHelper;
 use Shaarli\Front\Controller\Admin\ManageShaareController;
 use Shaarli\Http\HttpAccess;
+use Shaarli\Http\MetadataRetriever;
 use Shaarli\TestCase;
 use Slim\Http\Request;
 use Slim\Http\Response;
@@ -25,6 +26,7 @@ class DisplayCreateFormTest extends TestCase
         $this->createContainer();
 
         $this->container->httpAccess = $this->createMock(HttpAccess::class);
+        $this->container->metadataRetriever = $this->createMock(MetadataRetriever::class);
         $this->controller = new ManageShaareController($this->container);
     }
 
@@ -32,7 +34,7 @@ class DisplayCreateFormTest extends TestCase
      * Test displaying bookmark create form
      * Ensure that every step of the standard workflow works properly.
      */
-    public function testDisplayCreateFormWithUrl(): void
+    public function testDisplayCreateFormWithUrlAndWithMetadataRetrieval(): void
     {
         $this->container->environment = [
             'HTTP_REFERER' => $referer = 'http://shaarli/subfolder/controller/?searchtag=abc'
@@ -53,40 +55,20 @@ class DisplayCreateFormTest extends TestCase
         });
         $response = new Response();
 
-        $this->container->httpAccess
-            ->expects(static::once())
-            ->method('getCurlDownloadCallback')
-            ->willReturnCallback(
-                function (&$charset, &$title, &$description, &$tags) use (
-                    $remoteTitle,
-                    $remoteDesc,
-                    $remoteTags
-                ): callable {
-                    return function () use (
-                        &$charset,
-                        &$title,
-                        &$description,
-                        &$tags,
-                        $remoteTitle,
-                        $remoteDesc,
-                        $remoteTags
-                    ): void {
-                        $charset = 'ISO-8859-1';
-                        $title = $remoteTitle;
-                        $description = $remoteDesc;
-                        $tags = $remoteTags;
-                    };
-                }
-            )
-        ;
-        $this->container->httpAccess
-            ->expects(static::once())
-            ->method('getHttpResponse')
-            ->with($expectedUrl, 30, 4194304)
-            ->willReturnCallback(function($url, $timeout, $maxBytes, $callback): void {
-                $callback();
-            })
-        ;
+        $this->container->conf = $this->createMock(ConfigManager::class);
+        $this->container->conf->method('get')->willReturnCallback(function (string $param, $default) {
+            if ($param === 'general.enable_async_metadata') {
+                return false;
+            }
+
+            return $default;
+        });
+
+        $this->container->metadataRetriever->expects(static::once())->method('retrieve')->willReturn([
+            'title' => $remoteTitle,
+            'description' => $remoteDesc,
+            'tags' => $remoteTags,
+        ]);
 
         $this->container->bookmarkService
             ->expects(static::once())
@@ -127,6 +109,72 @@ class DisplayCreateFormTest extends TestCase
         static::assertSame($tags, $assignedVariables['tags']);
         static::assertArrayHasKey('source', $assignedVariables);
         static::assertArrayHasKey('default_private_links', $assignedVariables);
+        static::assertArrayHasKey('async_metadata', $assignedVariables);
+        static::assertArrayHasKey('retrieve_description', $assignedVariables);
+    }
+
+    /**
+     * Test displaying bookmark create form without any external metadata retrieval attempt
+     */
+    public function testDisplayCreateFormWithUrlAndWithoutMetadata(): void
+    {
+        $this->container->environment = [
+            'HTTP_REFERER' => $referer = 'http://shaarli/subfolder/controller/?searchtag=abc'
+        ];
+
+        $assignedVariables = [];
+        $this->assignTemplateVars($assignedVariables);
+
+        $url = 'http://url.tld/other?part=3&utm_ad=pay#hash';
+        $expectedUrl = str_replace('&utm_ad=pay', '', $url);
+
+        $request = $this->createMock(Request::class);
+        $request->method('getParam')->willReturnCallback(function (string $key) use ($url): ?string {
+            return $key === 'post' ? $url : null;
+        });
+        $response = new Response();
+
+        $this->container->metadataRetriever->expects(static::never())->method('retrieve');
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('bookmarksCountPerTag')
+            ->willReturn($tags = ['tag1' => 2, 'tag2' => 1])
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::at(0))
+            ->method('executeHooks')
+            ->willReturnCallback(function (string $hook, array $data): array {
+                static::assertSame('render_editlink', $hook);
+                static::assertSame('', $data['link']['title']);
+                static::assertSame('', $data['link']['description']);
+
+                return $data;
+            })
+        ;
+
+        $result = $this->controller->displayCreateForm($request, $response);
+
+        static::assertSame(200, $result->getStatusCode());
+        static::assertSame('editlink', (string) $result->getBody());
+
+        static::assertSame('Shaare - Shaarli', $assignedVariables['pagetitle']);
+
+        static::assertSame($expectedUrl, $assignedVariables['link']['url']);
+        static::assertSame('', $assignedVariables['link']['title']);
+        static::assertSame('', $assignedVariables['link']['description']);
+        static::assertSame('', $assignedVariables['link']['tags']);
+        static::assertFalse($assignedVariables['link']['private']);
+
+        static::assertTrue($assignedVariables['link_is_new']);
+        static::assertSame($referer, $assignedVariables['http_referer']);
+        static::assertSame($tags, $assignedVariables['tags']);
+        static::assertArrayHasKey('source', $assignedVariables);
+        static::assertArrayHasKey('default_private_links', $assignedVariables);
+        static::assertArrayHasKey('async_metadata', $assignedVariables);
+        static::assertArrayHasKey('retrieve_description', $assignedVariables);
     }
 
     /**
diff --git a/tests/http/MetadataRetrieverTest.php b/tests/http/MetadataRetrieverTest.php
new file mode 100644
index 00000000..3c9eaa0e
--- /dev/null
+++ b/tests/http/MetadataRetrieverTest.php
@@ -0,0 +1,154 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Http;
+
+use PHPUnit\Framework\TestCase;
+use Shaarli\Config\ConfigManager;
+
+class MetadataRetrieverTest extends TestCase
+{
+    /** @var MetadataRetriever */
+    protected $retriever;
+
+    /** @var ConfigManager */
+    protected $conf;
+
+    /** @var HttpAccess */
+    protected $httpAccess;
+
+    public function setUp(): void
+    {
+        $this->conf = $this->createMock(ConfigManager::class);
+        $this->httpAccess = $this->createMock(HttpAccess::class);
+        $this->retriever = new MetadataRetriever($this->conf, $this->httpAccess);
+
+        $this->conf->method('get')->willReturnCallback(function (string $param, $default) {
+            return $default === null ? $param : $default;
+        });
+    }
+
+    /**
+     * Test metadata retrieve() with values returned
+     */
+    public function testFullRetrieval(): void
+    {
+        $url = 'https://domain.tld/link';
+        $remoteTitle = 'Remote Title ';
+        $remoteDesc = 'Sometimes the meta description is relevant.';
+        $remoteTags = 'abc def';
+        $remoteCharset = 'utf-8';
+
+        $expectedResult = [
+            'title' => $remoteTitle,
+            'description' => $remoteDesc,
+            'tags' => $remoteTags,
+        ];
+
+        $this->httpAccess
+            ->expects(static::once())
+            ->method('getCurlHeaderCallback')
+            ->willReturnCallback(
+                function (&$charset) use (
+                    $remoteCharset
+                ): callable {
+                    return function () use (
+                        &$charset,
+                        $remoteCharset
+                    ): void {
+                        $charset = $remoteCharset;
+                    };
+                }
+            )
+        ;
+        $this->httpAccess
+            ->expects(static::once())
+            ->method('getCurlDownloadCallback')
+            ->willReturnCallback(
+                function (&$charset, &$title, &$description, &$tags) use (
+                    $remoteCharset,
+                    $remoteTitle,
+                    $remoteDesc,
+                    $remoteTags
+                ): callable {
+                    return function () use (
+                        &$charset,
+                        &$title,
+                        &$description,
+                        &$tags,
+                        $remoteCharset,
+                        $remoteTitle,
+                        $remoteDesc,
+                        $remoteTags
+                    ): void {
+                        static::assertSame($remoteCharset, $charset);
+
+                        $title = $remoteTitle;
+                        $description = $remoteDesc;
+                        $tags = $remoteTags;
+                    };
+                }
+            )
+        ;
+        $this->httpAccess
+            ->expects(static::once())
+            ->method('getHttpResponse')
+            ->with($url, 30, 4194304)
+            ->willReturnCallback(function($url, $timeout, $maxBytes, $headerCallback, $dlCallback): void {
+                $headerCallback();
+                $dlCallback();
+            })
+        ;
+
+        $result = $this->retriever->retrieve($url);
+
+        static::assertSame($expectedResult, $result);
+    }
+
+    /**
+     * Test metadata retrieve() without any value
+     */
+    public function testEmptyRetrieval(): void
+    {
+        $url = 'https://domain.tld/link';
+
+        $expectedResult = [
+            'title' => null,
+            'description' => null,
+            'tags' => null,
+        ];
+
+        $this->httpAccess
+            ->expects(static::once())
+            ->method('getCurlDownloadCallback')
+            ->willReturnCallback(
+                function (): callable {
+                    return function (): void {};
+                }
+            )
+        ;
+        $this->httpAccess
+            ->expects(static::once())
+            ->method('getCurlHeaderCallback')
+            ->willReturnCallback(
+                function (): callable {
+                    return function (): void {};
+                }
+            )
+        ;
+        $this->httpAccess
+            ->expects(static::once())
+            ->method('getHttpResponse')
+            ->with($url, 30, 4194304)
+            ->willReturnCallback(function($url, $timeout, $maxBytes, $headerCallback, $dlCallback): void {
+                $headerCallback();
+                $dlCallback();
+            })
+        ;
+
+        $result = $this->retriever->retrieve($url);
+
+        static::assertSame($expectedResult, $result);
+    }
+}