From b06fc28aa32f477e1785cd998385fdb490bc5ebf Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 29 Aug 2020 11:45:08 +0200 Subject: REST API: allow override of creation and update dates Note that if they're not provided, default behaviour will apply: creation and update dates will be autogenerated, and not empty. Fixes #1223 --- application/api/ApiUtils.php | 11 ++++++++++- application/api/controllers/ApiController.php | 3 ++- application/api/controllers/Links.php | 4 ++-- tests/api/controllers/links/PostLinkTest.php | 8 ++++---- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/application/api/ApiUtils.php b/application/api/ApiUtils.php index faebb8f5..4a6326f0 100644 --- a/application/api/ApiUtils.php +++ b/application/api/ApiUtils.php @@ -94,7 +94,7 @@ class ApiUtils * * @return Bookmark instance. */ - public static function buildLinkFromRequest($input, $defaultPrivate) + public static function buildBookmarkFromRequest($input, $defaultPrivate): Bookmark { $bookmark = new Bookmark(); $url = ! empty($input['url']) ? cleanup_url($input['url']) : ''; @@ -110,6 +110,15 @@ class ApiUtils $bookmark->setTags(! empty($input['tags']) ? $input['tags'] : []); $bookmark->setPrivate($private); + $created = \DateTime::createFromFormat(\DateTime::ATOM, $input['created'] ?? ''); + if ($created instanceof \DateTimeInterface) { + $bookmark->setCreated($created); + } + $updated = \DateTime::createFromFormat(\DateTime::ATOM, $input['updated'] ?? ''); + if ($updated instanceof \DateTimeInterface) { + $bookmark->setUpdated($updated); + } + return $bookmark; } diff --git a/application/api/controllers/ApiController.php b/application/api/controllers/ApiController.php index c4b3d0c3..88a845eb 100644 --- a/application/api/controllers/ApiController.php +++ b/application/api/controllers/ApiController.php @@ -4,6 +4,7 @@ namespace Shaarli\Api\Controllers; use Shaarli\Bookmark\BookmarkServiceInterface; use Shaarli\Config\ConfigManager; +use Shaarli\History; use Slim\Container; /** @@ -31,7 +32,7 @@ abstract class ApiController protected $bookmarkService; /** - * @var HistoryController + * @var History */ protected $history; diff --git a/application/api/controllers/Links.php b/application/api/controllers/Links.php index 29247950..778097fd 100644 --- a/application/api/controllers/Links.php +++ b/application/api/controllers/Links.php @@ -116,7 +116,7 @@ class Links extends ApiController public function postLink($request, $response) { $data = $request->getParsedBody(); - $bookmark = ApiUtils::buildLinkFromRequest($data, $this->conf->get('privacy.default_private_links')); + $bookmark = ApiUtils::buildBookmarkFromRequest($data, $this->conf->get('privacy.default_private_links')); // duplicate by URL, return 409 Conflict if (! empty($bookmark->getUrl()) && ! empty($dup = $this->bookmarkService->findByUrl($bookmark->getUrl())) @@ -155,7 +155,7 @@ class Links extends ApiController $index = index_url($this->ci['environment']); $data = $request->getParsedBody(); - $requestBookmark = ApiUtils::buildLinkFromRequest($data, $this->conf->get('privacy.default_private_links')); + $requestBookmark = ApiUtils::buildBookmarkFromRequest($data, $this->conf->get('privacy.default_private_links')); // duplicate URL on a different link, return 409 Conflict if (! empty($requestBookmark->getUrl()) && ! empty($dup = $this->bookmarkService->findByUrl($requestBookmark->getUrl())) diff --git a/tests/api/controllers/links/PostLinkTest.php b/tests/api/controllers/links/PostLinkTest.php index 4e791a04..f969fe1c 100644 --- a/tests/api/controllers/links/PostLinkTest.php +++ b/tests/api/controllers/links/PostLinkTest.php @@ -160,6 +160,8 @@ class PostLinkTest extends TestCase 'description' => 'shaare description', 'tags' => ['one', 'two'], 'private' => true, + 'created' => '2015-05-05T12:30:00+03:00', + 'updated' => '2016-06-05T14:32:10+03:00', ]; $env = Environment::mock([ 'REQUEST_METHOD' => 'POST', @@ -181,10 +183,8 @@ class PostLinkTest extends TestCase $this->assertEquals($link['description'], $data['description']); $this->assertEquals($link['tags'], $data['tags']); $this->assertEquals(true, $data['private']); - $this->assertTrue( - new \DateTime('2 seconds ago') < \DateTime::createFromFormat(\DateTime::ATOM, $data['created']) - ); - $this->assertEquals('', $data['updated']); + $this->assertSame($link['created'], $data['created']); + $this->assertSame($link['updated'], $data['updated']); } /** -- cgit v1.2.3 From 2cd0509b503332b1989f06da45d569d4d2929be5 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 3 Sep 2020 17:46:26 +0200 Subject: Improve regex to extract HTML metadata (title, description, etc.) Also added a bunch of tests to cover more use cases. Fixes #1375 --- application/bookmark/LinkUtils.php | 6 ++- tests/bookmark/LinkUtilsTest.php | 89 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/application/bookmark/LinkUtils.php b/application/bookmark/LinkUtils.php index 68914fca..03e1b82a 100644 --- a/application/bookmark/LinkUtils.php +++ b/application/bookmark/LinkUtils.php @@ -66,11 +66,13 @@ function html_extract_tag($tag, $html) { $propertiesKey = ['property', 'name', 'itemprop']; $properties = implode('|', $propertiesKey); + // We need a OR here to accept either 'property=og:noquote' or 'property="og:unrelated og:my-tag"' + $orCondition = '["\']?(?:og:)?'. $tag .'["\']?|["\'][^\'"]*?(?:og:)?' . $tag . '[^\'"]*?[\'"]'; // Try to retrieve OpenGraph image. - $ogRegex = '#]+(?:'. $properties .')=["\']?(?:og:)?'. $tag .'["\'\s][^>]*content=["\']?(.*?)["\'/>]#'; + $ogRegex = '#]+(?:'. $properties .')=(?:'. $orCondition .')[^>]*content=["\'](.*?)["\'].*?>#'; // If the attributes are not in the order property => content (e.g. Github) // New regex to keep this readable... more or less. - $ogRegexReverse = '#]+content=["\']([^"\']+)[^>]+(?:'. $properties .')=["\']?(?:og)?:'. $tag .'["\'\s/>]#'; + $ogRegexReverse = '#]+content=["\'](.*?)["\'][^>]+(?:'. $properties .')=(?:'. $orCondition .').*?>#'; if (preg_match($ogRegex, $html, $matches) > 0 || preg_match($ogRegexReverse, $html, $matches) > 0 diff --git a/tests/bookmark/LinkUtilsTest.php b/tests/bookmark/LinkUtilsTest.php index 7d4a7b89..cc7819bc 100644 --- a/tests/bookmark/LinkUtilsTest.php +++ b/tests/bookmark/LinkUtilsTest.php @@ -81,8 +81,78 @@ class LinkUtilsTest extends TestCase public function testHtmlExtractExistentNameTag() { $description = 'Bob and Alice share cookies.'; + + // Simple one line $html = 'stuff2'; $this->assertEquals($description, html_extract_tag('description', $html)); + + // Simple OpenGraph + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // Simple reversed OpenGraph + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // ItemProp OpenGraph + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph without quotes + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph reversed without quotes + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph with noise + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph reversed with noise + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph multiple properties start + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph multiple properties end + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph multiple properties both end + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph multiple properties both end with noise + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph reversed multiple properties start + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph reversed multiple properties end + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph reversed multiple properties both end + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // OpenGraph reversed multiple properties both end with noise + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); + + // Suggestion from #1375 + $html = ''; + $this->assertEquals($description, html_extract_tag('description', $html)); } /** @@ -92,6 +162,25 @@ class LinkUtilsTest extends TestCase { $html = 'stuff2'; $this->assertFalse(html_extract_tag('description', $html)); + + // Partial meta tag + $html = ''; + $this->assertFalse(html_extract_tag('description', $html)); + + $html = ''; + $this->assertFalse(html_extract_tag('description', $html)); + + $html = ''; + $this->assertFalse(html_extract_tag('description', $html)); + + $html = ''; + $this->assertFalse(html_extract_tag('description', $html)); + + $html = ''; + $this->assertFalse(html_extract_tag('description', $html)); + + $html = ''; + $this->assertFalse(html_extract_tag('description', $html)); } /** -- cgit v1.2.3 From 84045ffbb10c9d312b51dad972bae973c11aad3c Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 13 Oct 2020 11:59:54 +0200 Subject: Update badge versions --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4fb0bfe0..46dda8d5 100644 --- a/README.md +++ b/README.md @@ -6,13 +6,13 @@ _Do you want to share the links you discover?_ _Shaarli is a minimalist link sharing service that you can install on your own server._ _It is designed to be personal (single-user), fast and handy._ -[![](https://img.shields.io/badge/stable-v0.10.4-blue.svg)](https://github.com/shaarli/Shaarli/releases/tag/v0.10.4) +[![](https://img.shields.io/badge/stable-v0.11.1-blue.svg)](https://github.com/shaarli/Shaarli/releases/tag/v0.11.1) [![](https://img.shields.io/travis/shaarli/Shaarli/stable.svg?label=stable)](https://travis-ci.org/shaarli/Shaarli) • -[![](https://img.shields.io/badge/latest-v0.11.1-blue.svg)](https://github.com/shaarli/Shaarli/releases/tag/v0.11.1) +[![](https://img.shields.io/badge/latest-v0.12.0-blue.svg)](https://github.com/shaarli/Shaarli/releases/tag/v0.12.0) [![](https://img.shields.io/travis/shaarli/Shaarli/latest.svg?label=latest)](https://travis-ci.org/shaarli/Shaarli) • -[![](https://img.shields.io/badge/master-v0.11.x-blue.svg)](https://github.com/shaarli/Shaarli) +[![](https://img.shields.io/badge/master-v0.12.x-blue.svg)](https://github.com/shaarli/Shaarli) [![](https://img.shields.io/travis/shaarli/Shaarli.svg?label=master)](https://travis-ci.org/shaarli/Shaarli) [![Join the chat at https://gitter.im/shaarli/Shaarli](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/shaarli/Shaarli) -- cgit v1.2.3 From 8fabcd0224b1122a48b495326854bb3562cd2e9d Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 27 Aug 2020 15:25:18 +0200 Subject: Add Markdown Extra formatter Library: [Parsedown Extra](https://github.com/erusev/parsedown-extra) Also sort dependencies alphabetically. Fixes #1169 --- .../formatter/BookmarkMarkdownExtraFormatter.php | 24 +++ .../front/controller/admin/ConfigureController.php | 2 +- composer.json | 10 +- composer.lock | 67 ++++++++- .../BookmarkMarkdownExtraFormatterTest.php | 162 +++++++++++++++++++++ .../controller/admin/ConfigureControllerTest.php | 2 +- tpl/default/includes.html | 2 +- 7 files changed, 256 insertions(+), 13 deletions(-) create mode 100644 application/formatter/BookmarkMarkdownExtraFormatter.php create mode 100644 tests/formatter/BookmarkMarkdownExtraFormatterTest.php diff --git a/application/formatter/BookmarkMarkdownExtraFormatter.php b/application/formatter/BookmarkMarkdownExtraFormatter.php new file mode 100644 index 00000000..0694b23f --- /dev/null +++ b/application/formatter/BookmarkMarkdownExtraFormatter.php @@ -0,0 +1,24 @@ +parsedown = new \ParsedownExtra(); + } +} diff --git a/application/front/controller/admin/ConfigureController.php b/application/front/controller/admin/ConfigureController.php index e675fcca..0ed7ad81 100644 --- a/application/front/controller/admin/ConfigureController.php +++ b/application/front/controller/admin/ConfigureController.php @@ -30,7 +30,7 @@ class ConfigureController extends ShaarliAdminController 'theme_available', ThemeUtils::getThemes($this->container->conf->get('resource.raintpl_tpl')) ); - $this->assignView('formatter_available', ['default', 'markdown']); + $this->assignView('formatter_available', ['default', 'markdown', 'markdownExtra']); list($continents, $cities) = generateTimeZoneData( timezone_identifiers_list(), $this->container->conf->get('general.timezone') diff --git a/composer.json b/composer.json index cd9fcf5b..7e675623 100644 --- a/composer.json +++ b/composer.json @@ -10,6 +10,7 @@ }, "keywords": ["bookmark", "link", "share", "web"], "config": { + "sort-packages": true, "platform": { "php": "7.1.29" } @@ -18,12 +19,13 @@ "php": ">=7.1", "ext-json": "*", "ext-zlib": "*", - "shaarli/netscape-bookmark-parser": "^2.1", - "erusev/parsedown": "^1.6", - "slim/slim": "^3.0", "arthurhoaro/web-thumbnailer": "^2.0", + "erusev/parsedown": "^1.6", + "erusev/parsedown-extra": "^0.8.1", + "gettext/gettext": "^4.4", "pubsubhubbub/publisher": "dev-master", - "gettext/gettext": "^4.4" + "shaarli/netscape-bookmark-parser": "^2.1", + "slim/slim": "^3.0" }, "require-dev": { "roave/security-advisories": "dev-master", diff --git a/composer.lock b/composer.lock index 2c8b0ea7..e02491ff 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "98520a05a7185503ee13d05ffaa535f6", + "content-hash": "f84918821b0dceb0cd569875c0418bb8", "packages": [ { "name": "arthurhoaro/web-thumbnailer", @@ -107,6 +107,57 @@ }, "time": "2019-12-30T22:54:17+00:00" }, + { + "name": "erusev/parsedown-extra", + "version": "0.8.1", + "source": { + "type": "git", + "url": "https://github.com/erusev/parsedown-extra.git", + "reference": "91ac3ff98f0cea243bdccc688df43810f044dcef" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/erusev/parsedown-extra/zipball/91ac3ff98f0cea243bdccc688df43810f044dcef", + "reference": "91ac3ff98f0cea243bdccc688df43810f044dcef", + "shasum": "" + }, + "require": { + "erusev/parsedown": "^1.7.4" + }, + "require-dev": { + "phpunit/phpunit": "^4.8.35" + }, + "type": "library", + "autoload": { + "psr-0": { + "ParsedownExtra": "" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Emanuil Rusev", + "email": "hello@erusev.com", + "homepage": "http://erusev.com" + } + ], + "description": "An extension of Parsedown that adds support for Markdown Extra.", + "homepage": "https://github.com/erusev/parsedown-extra", + "keywords": [ + "markdown", + "markdown extra", + "parsedown", + "parser" + ], + "support": { + "issues": "https://github.com/erusev/parsedown-extra/issues", + "source": "https://github.com/erusev/parsedown-extra/tree/0.8.x" + }, + "time": "2019-12-30T23:20:37+00:00" + }, { "name": "gettext/gettext", "version": "v4.8.2", @@ -1577,12 +1628,12 @@ "source": { "type": "git", "url": "https://github.com/Roave/SecurityAdvisories.git", - "reference": "0749ceaf15c136d085b722a5bb88141398a54142" + "reference": "ba5d234b3a1559321b816b64aafc2ce6728799ff" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/Roave/SecurityAdvisories/zipball/0749ceaf15c136d085b722a5bb88141398a54142", - "reference": "0749ceaf15c136d085b722a5bb88141398a54142", + "url": "https://api.github.com/repos/Roave/SecurityAdvisories/zipball/ba5d234b3a1559321b816b64aafc2ce6728799ff", + "reference": "ba5d234b3a1559321b816b64aafc2ce6728799ff", "shasum": "" }, "conflict": { @@ -1642,7 +1693,7 @@ "ezsystems/ezplatform-kernel": ">=1,<1.0.2.1", "ezsystems/ezplatform-user": ">=1,<1.0.1", "ezsystems/ezpublish-kernel": ">=5.3,<5.3.12.1|>=5.4,<5.4.14.2|>=6,<6.7.9.1|>=6.8,<6.13.6.3|>=7,<7.2.4.1|>=7.3,<7.3.2.1|>=7.5,<7.5.7.1", - "ezsystems/ezpublish-legacy": ">=5.3,<5.3.12.6|>=5.4,<5.4.14.1|>=2011,<2017.12.7.2|>=2018.6,<2018.6.1.4|>=2018.9,<2018.9.1.3|>=2019.3,<2019.3.4.2", + "ezsystems/ezpublish-legacy": ">=5.3,<5.3.12.6|>=5.4,<5.4.14.2|>=2011,<2017.12.7.3|>=2018.6,<2018.6.1.4|>=2018.9,<2018.9.1.3|>=2019.3,<2019.3.5.1", "ezsystems/platform-ui-assets-bundle": ">=4.2,<4.2.3", "ezsystems/repository-forms": ">=2.3,<2.3.2.1", "ezyang/htmlpurifier": "<4.1.1", @@ -1685,6 +1736,8 @@ "mittwald/typo3_forum": "<1.2.1", "monolog/monolog": ">=1.8,<1.12", "namshi/jose": "<2.2", + "nette/application": ">=2,<2.0.19|>=2.1,<2.1.13|>=2.2,<2.2.10|>=2.3,<2.3.14|>=2.4,<2.4.16|>=3,<3.0.6", + "nette/nette": ">=2,<2.0.19|>=2.1,<2.1.13", "nystudio107/craft-seomatic": "<3.3", "nzo/url-encryptor-bundle": ">=4,<4.3.2|>=5,<5.0.1", "october/backend": ">=1.0.319,<1.0.467", @@ -1720,6 +1773,7 @@ "privatebin/privatebin": "<1.2.2|>=1.3,<1.3.2", "propel/propel": ">=2-alpha.1,<=2-alpha.7", "propel/propel1": ">=1,<=1.7.1", + "pterodactyl/panel": "<0.7.19|>=1-rc.0,<=1-rc.6", "pusher/pusher-php-server": "<2.2.1", "rainlab/debugbar-plugin": "<3.1", "robrichards/xmlseclibs": "<3.0.4", @@ -1805,6 +1859,7 @@ "typo3/flow": ">=1,<1.0.4|>=1.1,<1.1.1|>=2,<2.0.1|>=2.3,<2.3.16|>=3,<3.0.10|>=3.1,<3.1.7|>=3.2,<3.2.7|>=3.3,<3.3.5", "typo3/neos": ">=1.1,<1.1.3|>=1.2,<1.2.13|>=2,<2.0.4", "typo3/phar-stream-wrapper": ">=1,<2.1.1|>=3,<3.1.1", + "typo3fluid/fluid": ">=2,<2.0.5|>=2.1,<2.1.4|>=2.2,<2.2.1|>=2.3,<2.3.5|>=2.4,<2.4.1|>=2.5,<2.5.5|>=2.6,<2.6.1", "ua-parser/uap-php": "<3.8", "usmanhalalit/pixie": "<1.0.3|>=2,<2.0.2", "verot/class.upload.php": "<=1.0.3|>=2,<=2.0.4", @@ -1878,7 +1933,7 @@ "type": "tidelift" } ], - "time": "2020-09-24T17:02:11+00:00" + "time": "2020-10-08T21:02:27+00:00" }, { "name": "sebastian/code-unit-reverse-lookup", diff --git a/tests/formatter/BookmarkMarkdownExtraFormatterTest.php b/tests/formatter/BookmarkMarkdownExtraFormatterTest.php new file mode 100644 index 00000000..d4941ef3 --- /dev/null +++ b/tests/formatter/BookmarkMarkdownExtraFormatterTest.php @@ -0,0 +1,162 @@ +conf = new ConfigManager(self::$testConf); + $this->formatter = new BookmarkMarkdownExtraFormatter($this->conf, true); + } + + /** + * Test formatting a bookmark with all its attribute filled. + */ + public function testFormatExtra(): void + { + $bookmark = new Bookmark(); + $bookmark->setId($id = 11); + $bookmark->setShortUrl($short = 'abcdef'); + $bookmark->setUrl('https://sub.domain.tld?query=here&for=real#hash'); + $bookmark->setTitle($title = 'This is a bookmark'); + $bookmark->setDescription('

Content

`Here is some content

'); + $bookmark->setTags($tags = ['tag1', 'bookmark', 'other', '']); + $bookmark->setThumbnail('http://domain2.tdl2/?type=img&name=file.png'); + $bookmark->setSticky(true); + $bookmark->setCreated($created = DateTime::createFromFormat('Ymd_His', '20190521_190412')); + $bookmark->setUpdated($updated = DateTime::createFromFormat('Ymd_His', '20190521_191213')); + $bookmark->setPrivate(true); + + $link = $this->formatter->format($bookmark); + $this->assertEquals($id, $link['id']); + $this->assertEquals($short, $link['shorturl']); + $this->assertEquals('https://sub.domain.tld?query=here&for=real#hash', $link['url']); + $this->assertEquals( + 'https://sub.domain.tld?query=here&for=real#hash', + $link['real_url'] + ); + $this->assertEquals('This is a <strong>bookmark</strong>', $link['title']); + $this->assertEquals( + '

'. + '<h2>Content</h2><p>`Here is some content</p>'. + '

', + $link['description'] + ); + $tags[3] = '<script>alert("xss");</script>'; + $this->assertEquals($tags, $link['taglist']); + $this->assertEquals(implode(' ', $tags), $link['tags']); + $this->assertEquals( + 'http://domain2.tdl2/?type=img&name=file.png', + $link['thumbnail'] + ); + $this->assertEquals($created, $link['created']); + $this->assertEquals($created->getTimestamp(), $link['timestamp']); + $this->assertEquals($updated, $link['updated']); + $this->assertEquals($updated->getTimestamp(), $link['updated_timestamp']); + $this->assertTrue($link['private']); + $this->assertTrue($link['sticky']); + $this->assertEquals('private', $link['class']); + } + + /** + * Test formatting a bookmark with all its attribute filled. + */ + public function testFormatExtraMinimal(): void + { + $bookmark = new Bookmark(); + + $link = $this->formatter->format($bookmark); + $this->assertEmpty($link['id']); + $this->assertEmpty($link['shorturl']); + $this->assertEmpty($link['url']); + $this->assertEmpty($link['real_url']); + $this->assertEmpty($link['title']); + $this->assertEmpty($link['description']); + $this->assertEmpty($link['taglist']); + $this->assertEmpty($link['tags']); + $this->assertEmpty($link['thumbnail']); + $this->assertEmpty($link['created']); + $this->assertEmpty($link['timestamp']); + $this->assertEmpty($link['updated']); + $this->assertEmpty($link['updated_timestamp']); + $this->assertFalse($link['private']); + $this->assertFalse($link['sticky']); + $this->assertEmpty($link['class']); + } + + /** + * Make sure that the description is properly formatted by the default formatter. + */ + public function testFormatExtrraDescription(): void + { + $description = 'This a description'. PHP_EOL; + $description .= 'text https://sub.domain.tld?query=here&for=real#hash more text'. PHP_EOL; + $description .= 'Also, there is an #hashtag added'. PHP_EOL; + $description .= ' A N D KEEP SPACES ! '. PHP_EOL; + $description .= '# Header {.class}'. PHP_EOL; + + $bookmark = new Bookmark(); + $bookmark->setDescription($description); + $link = $this->formatter->format($bookmark); + + $description = '

'; + $description .= 'This a <strong>description</strong>
'. PHP_EOL; + $url = 'https://sub.domain.tld?query=here&for=real#hash'; + $description .= 'text '. $url .' more text
'. PHP_EOL; + $description .= 'Also, there is an #hashtag added
'. PHP_EOL; + $description .= 'A N D KEEP SPACES !

' . PHP_EOL; + $description .= '

Header

'; + $description .= '
'; + + $this->assertEquals($description, $link['description']); + } + + /** + * Test formatting URL with an index_url set + * It should prepend relative links. + */ + public function testFormatExtraNoteWithIndexUrl(): void + { + $bookmark = new Bookmark(); + $bookmark->setUrl($short = '?abcdef'); + $description = 'Text #hashtag more text'; + $bookmark->setDescription($description); + + $this->formatter->addContextData('index_url', $root = 'https://domain.tld/hithere/'); + + $description = '

'; + $description .= 'Text #hashtag more text'; + $description .= '

'; + + $link = $this->formatter->format($bookmark); + $this->assertEquals($root . $short, $link['url']); + $this->assertEquals($root . $short, $link['real_url']); + $this->assertEquals( + $description, + $link['description'] + ); + } +} diff --git a/tests/front/controller/admin/ConfigureControllerTest.php b/tests/front/controller/admin/ConfigureControllerTest.php index aca6cff3..d82db0a7 100644 --- a/tests/front/controller/admin/ConfigureControllerTest.php +++ b/tests/front/controller/admin/ConfigureControllerTest.php @@ -51,7 +51,7 @@ class ConfigureControllerTest extends TestCase static::assertSame('general.title', $assignedVariables['title']); static::assertSame('resource.theme', $assignedVariables['theme']); static::assertEmpty($assignedVariables['theme_available']); - static::assertSame(['default', 'markdown'], $assignedVariables['formatter_available']); + static::assertSame(['default', 'markdown', 'markdownExtra'], $assignedVariables['formatter_available']); static::assertNotEmpty($assignedVariables['continents']); static::assertNotEmpty($assignedVariables['cities']); static::assertSame('general.retrieve_description', $assignedVariables['retrieve_description']); diff --git a/tpl/default/includes.html b/tpl/default/includes.html index 227f9b52..09768ac4 100644 --- a/tpl/default/includes.html +++ b/tpl/default/includes.html @@ -8,7 +8,7 @@ -{if="$formatter==='markdown'"} +{if="strpos($formatter, 'markdown') !== false"} {/if} {loop="$plugins_includes.css_files"} -- cgit v1.2.3 From fd1ddad98df45bc3c18be7980c1cbe68ce6b219c Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 26 Sep 2020 14:18:01 +0200 Subject: Add mutex on datastore I/O operations To make sure that there is no concurrent operation on the datastore file. Fixes #1132 --- application/api/ApiMiddleware.php | 2 + application/bookmark/BookmarkFileService.php | 9 ++- application/bookmark/BookmarkIO.php | 35 ++++++--- application/bookmark/BookmarkServiceInterface.php | 11 --- application/container/ContainerBuilder.php | 2 + composer.json | 1 + composer.lock | 87 ++++++++++++++++++++++- init.php | 1 + tests/api/controllers/info/InfoTest.php | 4 +- tests/api/controllers/links/DeleteLinkTest.php | 9 ++- tests/api/controllers/links/GetLinkIdTest.php | 4 +- tests/api/controllers/links/GetLinksTest.php | 4 +- tests/api/controllers/links/PostLinkTest.php | 4 +- tests/api/controllers/links/PutLinkTest.php | 4 +- tests/api/controllers/tags/DeleteTagTest.php | 11 ++- tests/api/controllers/tags/GetTagNameTest.php | 4 +- tests/api/controllers/tags/GetTagsTest.php | 4 +- tests/api/controllers/tags/PutTagTest.php | 4 +- tests/bookmark/BookmarkFileServiceTest.php | 44 +++++++----- tests/bookmark/BookmarkFilterTest.php | 4 +- tests/bookmark/BookmarkInitializerTest.php | 13 ++-- tests/bootstrap.php | 4 ++ tests/feed/FeedBuilderTest.php | 4 +- tests/netscape/BookmarkExportTest.php | 4 +- tests/netscape/BookmarkImportTest.php | 4 +- tests/updater/UpdaterTest.php | 4 +- 26 files changed, 218 insertions(+), 63 deletions(-) diff --git a/application/api/ApiMiddleware.php b/application/api/ApiMiddleware.php index f5b53b01..adc8b266 100644 --- a/application/api/ApiMiddleware.php +++ b/application/api/ApiMiddleware.php @@ -1,6 +1,7 @@ container->get('history'), + new FlockMutex(fopen(SHAARLI_MUTEX_FILE, 'r'), 2), true ); $this->container['db'] = $linkDb; diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php index c9ec2609..1ba00712 100644 --- a/application/bookmark/BookmarkFileService.php +++ b/application/bookmark/BookmarkFileService.php @@ -5,6 +5,7 @@ namespace Shaarli\Bookmark; use Exception; +use malkusch\lock\mutex\Mutex; use Shaarli\Bookmark\Exception\BookmarkNotFoundException; use Shaarli\Bookmark\Exception\DatastoreNotInitializedException; use Shaarli\Bookmark\Exception\EmptyDataStoreException; @@ -47,15 +48,19 @@ class BookmarkFileService implements BookmarkServiceInterface /** @var bool true for logged in users. Default value to retrieve private bookmarks. */ protected $isLoggedIn; + /** @var Mutex */ + protected $mutex; + /** * @inheritDoc */ - public function __construct(ConfigManager $conf, History $history, $isLoggedIn) + public function __construct(ConfigManager $conf, History $history, Mutex $mutex, $isLoggedIn) { $this->conf = $conf; $this->history = $history; + $this->mutex = $mutex; $this->pageCacheManager = new PageCacheManager($this->conf->get('resource.page_cache'), $isLoggedIn); - $this->bookmarksIO = new BookmarkIO($this->conf); + $this->bookmarksIO = new BookmarkIO($this->conf, $this->mutex); $this->isLoggedIn = $isLoggedIn; if (!$this->isLoggedIn && $this->conf->get('privacy.hide_public_links', false)) { diff --git a/application/bookmark/BookmarkIO.php b/application/bookmark/BookmarkIO.php index 6bf7f365..099653b0 100644 --- a/application/bookmark/BookmarkIO.php +++ b/application/bookmark/BookmarkIO.php @@ -2,6 +2,8 @@ namespace Shaarli\Bookmark; +use malkusch\lock\mutex\Mutex; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\Exception\DatastoreNotInitializedException; use Shaarli\Bookmark\Exception\EmptyDataStoreException; use Shaarli\Bookmark\Exception\NotWritableDataStoreException; @@ -27,11 +29,14 @@ class BookmarkIO */ protected $conf; + + /** @var Mutex */ + protected $mutex; + /** * string Datastore PHP prefix */ protected static $phpPrefix = 'conf = $conf; $this->datastore = $conf->get('resource.datastore'); + $this->mutex = $mutex; } /** @@ -67,11 +77,16 @@ class BookmarkIO throw new NotWritableDataStoreException($this->datastore); } + $content = null; + $this->mutex->synchronized(function () use (&$content) { + $content = file_get_contents($this->datastore); + }); + // Note that gzinflate is faster than gzuncompress. // See: http://www.php.net/manual/en/function.gzdeflate.php#96439 $links = unserialize(gzinflate(base64_decode( - substr(file_get_contents($this->datastore), - strlen(self::$phpPrefix), -strlen(self::$phpSuffix))))); + substr($content, strlen(self::$phpPrefix), -strlen(self::$phpSuffix)) + ))); if (empty($links)) { if (filesize($this->datastore) > 100) { @@ -100,9 +115,13 @@ class BookmarkIO throw new NotWritableDataStoreException(dirname($this->datastore)); } - file_put_contents( - $this->datastore, - self::$phpPrefix.base64_encode(gzdeflate(serialize($links))).self::$phpSuffix - ); + $data = self::$phpPrefix.base64_encode(gzdeflate(serialize($links))).self::$phpSuffix; + + $this->mutex->synchronized(function () use ($data) { + file_put_contents( + $this->datastore, + $data + ); + }); } } diff --git a/application/bookmark/BookmarkServiceInterface.php b/application/bookmark/BookmarkServiceInterface.php index b9b483eb..638cfa5f 100644 --- a/application/bookmark/BookmarkServiceInterface.php +++ b/application/bookmark/BookmarkServiceInterface.php @@ -5,8 +5,6 @@ namespace Shaarli\Bookmark; use Shaarli\Bookmark\Exception\BookmarkNotFoundException; use Shaarli\Bookmark\Exception\NotWritableDataStoreException; -use Shaarli\Config\ConfigManager; -use Shaarli\History; /** * Class BookmarksService @@ -15,15 +13,6 @@ use Shaarli\History; */ interface BookmarkServiceInterface { - /** - * BookmarksService constructor. - * - * @param ConfigManager $conf instance - * @param History $history instance - * @param bool $isLoggedIn true if the current user is logged in - */ - public function __construct(ConfigManager $conf, History $history, $isLoggedIn); - /** * Find a bookmark by hash * diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php index 55bb51b5..c21d58dd 100644 --- a/application/container/ContainerBuilder.php +++ b/application/container/ContainerBuilder.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shaarli\Container; +use malkusch\lock\mutex\FlockMutex; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Bookmark\BookmarkServiceInterface; use Shaarli\Config\ConfigManager; @@ -84,6 +85,7 @@ class ContainerBuilder return new BookmarkFileService( $container->conf, $container->history, + new FlockMutex(fopen(SHAARLI_MUTEX_FILE, 'r'), 2), $container->loginManager->isLoggedIn() ); }; diff --git a/composer.json b/composer.json index 7e675623..c0855e47 100644 --- a/composer.json +++ b/composer.json @@ -23,6 +23,7 @@ "erusev/parsedown": "^1.6", "erusev/parsedown-extra": "^0.8.1", "gettext/gettext": "^4.4", + "malkusch/lock": "^2.1", "pubsubhubbub/publisher": "dev-master", "shaarli/netscape-bookmark-parser": "^2.1", "slim/slim": "^3.0" diff --git a/composer.lock b/composer.lock index e02491ff..c379d8e7 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "f84918821b0dceb0cd569875c0418bb8", + "content-hash": "932b191006135ff8be495aa0b4ba7e09", "packages": [ { "name": "arthurhoaro/web-thumbnailer", @@ -344,6 +344,91 @@ }, "time": "2016-11-07T19:29:14+00:00" }, + { + "name": "malkusch/lock", + "version": "v2.1", + "source": { + "type": "git", + "url": "https://github.com/php-lock/lock.git", + "reference": "093f389ec2f38fc8686d2f70e23378182fce7714" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-lock/lock/zipball/093f389ec2f38fc8686d2f70e23378182fce7714", + "reference": "093f389ec2f38fc8686d2f70e23378182fce7714", + "shasum": "" + }, + "require": { + "php": ">=7.1", + "psr/log": "^1" + }, + "require-dev": { + "eloquent/liberator": "^2.0", + "ext-memcached": "*", + "ext-pcntl": "*", + "ext-pdo_mysql": "*", + "ext-pdo_sqlite": "*", + "ext-redis": "*", + "ext-sysvsem": "*", + "johnkary/phpunit-speedtrap": "^3.0", + "kriswallsmith/spork": "^0.3", + "mikey179/vfsstream": "^1.6", + "php-mock/php-mock-phpunit": "^2.1", + "phpunit/phpunit": "^7.4", + "predis/predis": "^1.1", + "squizlabs/php_codesniffer": "^3.3" + }, + "suggest": { + "ext-pnctl": "Enables locking with flock without busy waiting in CLI scripts.", + "ext-redis": "To use this library with the PHP Redis extension.", + "ext-sysvsem": "Enables locking using semaphores.", + "predis/predis": "To use this library with predis." + }, + "type": "library", + "autoload": { + "psr-4": { + "malkusch\\lock\\": "classes/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "WTFPL" + ], + "authors": [ + { + "name": "Markus Malkusch", + "email": "markus@malkusch.de", + "homepage": "http://markus.malkusch.de", + "role": "Developer" + }, + { + "name": "Willem Stuursma-Ruwen", + "email": "willem@stuursma.name", + "role": "Developer" + } + ], + "description": "Mutex library for exclusive code execution.", + "homepage": "https://github.com/malkusch/lock", + "keywords": [ + "advisory-locks", + "cas", + "flock", + "lock", + "locking", + "memcache", + "mutex", + "mysql", + "postgresql", + "redis", + "redlock", + "semaphore" + ], + "support": { + "issues": "https://github.com/php-lock/lock/issues", + "source": "https://github.com/php-lock/lock/tree/v2.1" + }, + "time": "2018-12-12T19:53:29+00:00" + }, { "name": "nikic/fast-route", "version": "v1.3.0", diff --git a/init.php b/init.php index f0b84368..ab0e4ea7 100644 --- a/init.php +++ b/init.php @@ -60,6 +60,7 @@ ini_set('session.use_only_cookies', 1); ini_set('session.use_trans_sid', false); define('SHAARLI_VERSION', ApplicationUtils::getVersion(__DIR__ .'/'. ApplicationUtils::$VERSION_FILE)); +define('SHAARLI_MUTEX_FILE', __FILE__); session_name('shaarli'); // Start session if needed (Some server auto-start sessions). diff --git a/tests/api/controllers/info/InfoTest.php b/tests/api/controllers/info/InfoTest.php index 1598e1e8..10b29ab2 100644 --- a/tests/api/controllers/info/InfoTest.php +++ b/tests/api/controllers/info/InfoTest.php @@ -1,6 +1,7 @@ conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -58,7 +60,7 @@ class InfoTest extends TestCase $this->container = new Container(); $this->container['conf'] = $this->conf; - $this->container['db'] = new BookmarkFileService($this->conf, $history, true); + $this->container['db'] = new BookmarkFileService($this->conf, $history, $mutex, true); $this->container['history'] = null; $this->controller = new Info($this->container); diff --git a/tests/api/controllers/links/DeleteLinkTest.php b/tests/api/controllers/links/DeleteLinkTest.php index cf9464f0..805c9be3 100644 --- a/tests/api/controllers/links/DeleteLinkTest.php +++ b/tests/api/controllers/links/DeleteLinkTest.php @@ -3,6 +3,7 @@ namespace Shaarli\Api\Controllers; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Config\ConfigManager; use Shaarli\History; @@ -53,11 +54,15 @@ class DeleteLinkTest extends \Shaarli\TestCase */ protected $controller; + /** @var NoMutex */ + protected $mutex; + /** * Before each test, instantiate a new Api with its config, plugins and bookmarks. */ protected function setUp(): void { + $this->mutex = new NoMutex(); $this->conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -65,7 +70,7 @@ class DeleteLinkTest extends \Shaarli\TestCase $refHistory = new \ReferenceHistory(); $refHistory->write(self::$testHistory); $this->history = new History(self::$testHistory); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->container = new Container(); $this->container['conf'] = $this->conf; @@ -100,7 +105,7 @@ class DeleteLinkTest extends \Shaarli\TestCase $this->assertEquals(204, $response->getStatusCode()); $this->assertEmpty((string) $response->getBody()); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->assertFalse($this->bookmarkService->exists($id)); $historyEntry = $this->history->getHistory()[0]; diff --git a/tests/api/controllers/links/GetLinkIdTest.php b/tests/api/controllers/links/GetLinkIdTest.php index 99dc606f..1ec56ef3 100644 --- a/tests/api/controllers/links/GetLinkIdTest.php +++ b/tests/api/controllers/links/GetLinkIdTest.php @@ -2,6 +2,7 @@ namespace Shaarli\Api\Controllers; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\Bookmark; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Config\ConfigManager; @@ -57,6 +58,7 @@ class GetLinkIdTest extends \Shaarli\TestCase */ protected function setUp(): void { + $mutex = new NoMutex(); $this->conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -65,7 +67,7 @@ class GetLinkIdTest extends \Shaarli\TestCase $this->container = new Container(); $this->container['conf'] = $this->conf; - $this->container['db'] = new BookmarkFileService($this->conf, $history, true); + $this->container['db'] = new BookmarkFileService($this->conf, $history, $mutex, true); $this->container['history'] = null; $this->controller = new Links($this->container); diff --git a/tests/api/controllers/links/GetLinksTest.php b/tests/api/controllers/links/GetLinksTest.php index ca1bfc63..0f5073b4 100644 --- a/tests/api/controllers/links/GetLinksTest.php +++ b/tests/api/controllers/links/GetLinksTest.php @@ -1,6 +1,7 @@ conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -65,7 +67,7 @@ class GetLinksTest extends \Shaarli\TestCase $this->container = new Container(); $this->container['conf'] = $this->conf; - $this->container['db'] = new BookmarkFileService($this->conf, $history, true); + $this->container['db'] = new BookmarkFileService($this->conf, $history, $mutex, true); $this->container['history'] = null; $this->controller = new Links($this->container); diff --git a/tests/api/controllers/links/PostLinkTest.php b/tests/api/controllers/links/PostLinkTest.php index 20694571..7ff92f5c 100644 --- a/tests/api/controllers/links/PostLinkTest.php +++ b/tests/api/controllers/links/PostLinkTest.php @@ -2,6 +2,7 @@ namespace Shaarli\Api\Controllers; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\Bookmark; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Config\ConfigManager; @@ -72,6 +73,7 @@ class PostLinkTest extends TestCase */ protected function setUp(): void { + $mutex = new NoMutex(); $this->conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -79,7 +81,7 @@ class PostLinkTest extends TestCase $refHistory = new \ReferenceHistory(); $refHistory->write(self::$testHistory); $this->history = new History(self::$testHistory); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $mutex, true); $this->container = new Container(); $this->container['conf'] = $this->conf; diff --git a/tests/api/controllers/links/PutLinkTest.php b/tests/api/controllers/links/PutLinkTest.php index a2e87c59..240ee323 100644 --- a/tests/api/controllers/links/PutLinkTest.php +++ b/tests/api/controllers/links/PutLinkTest.php @@ -3,6 +3,7 @@ namespace Shaarli\Api\Controllers; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\Bookmark; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Config\ConfigManager; @@ -64,6 +65,7 @@ class PutLinkTest extends \Shaarli\TestCase */ protected function setUp(): void { + $mutex = new NoMutex(); $this->conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -71,7 +73,7 @@ class PutLinkTest extends \Shaarli\TestCase $refHistory = new \ReferenceHistory(); $refHistory->write(self::$testHistory); $this->history = new History(self::$testHistory); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $mutex, true); $this->container = new Container(); $this->container['conf'] = $this->conf; diff --git a/tests/api/controllers/tags/DeleteTagTest.php b/tests/api/controllers/tags/DeleteTagTest.php index 1326eb47..37f07229 100644 --- a/tests/api/controllers/tags/DeleteTagTest.php +++ b/tests/api/controllers/tags/DeleteTagTest.php @@ -3,6 +3,7 @@ namespace Shaarli\Api\Controllers; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Bookmark\LinkDB; use Shaarli\Config\ConfigManager; @@ -54,11 +55,15 @@ class DeleteTagTest extends \Shaarli\TestCase */ protected $controller; + /** @var NoMutex */ + protected $mutex; + /** * Before each test, instantiate a new Api with its config, plugins and bookmarks. */ protected function setUp(): void { + $this->mutex = new NoMutex(); $this->conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -66,7 +71,7 @@ class DeleteTagTest extends \Shaarli\TestCase $refHistory = new \ReferenceHistory(); $refHistory->write(self::$testHistory); $this->history = new History(self::$testHistory); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->container = new Container(); $this->container['conf'] = $this->conf; @@ -102,7 +107,7 @@ class DeleteTagTest extends \Shaarli\TestCase $this->assertEquals(204, $response->getStatusCode()); $this->assertEmpty((string) $response->getBody()); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $tags = $this->bookmarkService->bookmarksCountPerTag(); $this->assertFalse(isset($tags[$tagName])); @@ -136,7 +141,7 @@ class DeleteTagTest extends \Shaarli\TestCase $this->assertEquals(204, $response->getStatusCode()); $this->assertEmpty((string) $response->getBody()); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $tags = $this->bookmarkService->bookmarksCountPerTag(); $this->assertFalse(isset($tags[$tagName])); $this->assertTrue($tags[strtolower($tagName)] > 0); diff --git a/tests/api/controllers/tags/GetTagNameTest.php b/tests/api/controllers/tags/GetTagNameTest.php index 9c05954b..878de5a4 100644 --- a/tests/api/controllers/tags/GetTagNameTest.php +++ b/tests/api/controllers/tags/GetTagNameTest.php @@ -2,6 +2,7 @@ namespace Shaarli\Api\Controllers; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Bookmark\LinkDB; use Shaarli\Config\ConfigManager; @@ -55,6 +56,7 @@ class GetTagNameTest extends \Shaarli\TestCase */ protected function setUp(): void { + $mutex = new NoMutex(); $this->conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -63,7 +65,7 @@ class GetTagNameTest extends \Shaarli\TestCase $this->container = new Container(); $this->container['conf'] = $this->conf; - $this->container['db'] = new BookmarkFileService($this->conf, $history, true); + $this->container['db'] = new BookmarkFileService($this->conf, $history, $mutex, true); $this->container['history'] = null; $this->controller = new Tags($this->container); diff --git a/tests/api/controllers/tags/GetTagsTest.php b/tests/api/controllers/tags/GetTagsTest.php index 3459fdfa..b565a8c4 100644 --- a/tests/api/controllers/tags/GetTagsTest.php +++ b/tests/api/controllers/tags/GetTagsTest.php @@ -1,6 +1,7 @@ conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); $this->refDB->write(self::$testDatastore); $history = new History('sandbox/history.php'); - $this->bookmarkService = new BookmarkFileService($this->conf, $history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $history, $mutex, true); $this->container = new Container(); $this->container['conf'] = $this->conf; diff --git a/tests/api/controllers/tags/PutTagTest.php b/tests/api/controllers/tags/PutTagTest.php index 74edde78..c73f6d3b 100644 --- a/tests/api/controllers/tags/PutTagTest.php +++ b/tests/api/controllers/tags/PutTagTest.php @@ -2,6 +2,7 @@ namespace Shaarli\Api\Controllers; +use malkusch\lock\mutex\NoMutex; use Shaarli\Api\Exceptions\ApiBadParametersException; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Bookmark\LinkDB; @@ -64,6 +65,7 @@ class PutTagTest extends \Shaarli\TestCase */ protected function setUp(): void { + $mutex = new NoMutex(); $this->conf = new ConfigManager('tests/utils/config/configJson'); $this->conf->set('resource.datastore', self::$testDatastore); $this->refDB = new \ReferenceLinkDB(); @@ -71,7 +73,7 @@ class PutTagTest extends \Shaarli\TestCase $refHistory = new \ReferenceHistory(); $refHistory->write(self::$testHistory); $this->history = new History(self::$testHistory); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $mutex, true); $this->container = new Container(); $this->container['conf'] = $this->conf; diff --git a/tests/bookmark/BookmarkFileServiceTest.php b/tests/bookmark/BookmarkFileServiceTest.php index c399822b..6c56dfaa 100644 --- a/tests/bookmark/BookmarkFileServiceTest.php +++ b/tests/bookmark/BookmarkFileServiceTest.php @@ -6,6 +6,7 @@ namespace Shaarli\Bookmark; use DateTime; +use malkusch\lock\mutex\NoMutex; use ReferenceLinkDB; use ReflectionClass; use Shaarli; @@ -52,6 +53,9 @@ class BookmarkFileServiceTest extends TestCase */ protected $privateLinkDB = null; + /** @var NoMutex */ + protected $mutex; + /** * Instantiates public and private LinkDBs with test data * @@ -68,6 +72,8 @@ class BookmarkFileServiceTest extends TestCase */ protected function setUp(): void { + $this->mutex = new NoMutex(); + if (file_exists(self::$testDatastore)) { unlink(self::$testDatastore); } @@ -87,8 +93,8 @@ class BookmarkFileServiceTest extends TestCase $this->refDB = new \ReferenceLinkDB(); $this->refDB->write(self::$testDatastore); $this->history = new History('sandbox/history.php'); - $this->publicLinkDB = new BookmarkFileService($this->conf, $this->history, false); - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->publicLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, false); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); } /** @@ -105,7 +111,7 @@ class BookmarkFileServiceTest extends TestCase $db = self::getMethod('migrate'); $db->invokeArgs($this->privateLinkDB, []); - $db = new \FakeBookmarkService($this->conf, $this->history, true); + $db = new \FakeBookmarkService($this->conf, $this->history, $this->mutex, true); $this->assertInstanceOf(BookmarkArray::class, $db->getBookmarks()); $this->assertEquals($this->refDB->countLinks(), $db->count()); } @@ -174,7 +180,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertEquals($updated, $bookmark->getUpdated()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(43); $this->assertEquals(43, $bookmark->getId()); @@ -212,7 +218,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertNull($bookmark->getUpdated()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(43); $this->assertEquals(43, $bookmark->getId()); @@ -242,7 +248,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertEquals(43, $bookmark->getId()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->privateLinkDB->get(43); } @@ -314,7 +320,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertTrue(new \DateTime('5 seconds ago') < $bookmark->getUpdated()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(42); $this->assertEquals(42, $bookmark->getId()); @@ -355,7 +361,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertTrue(new \DateTime('5 seconds ago') < $bookmark->getUpdated()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(42); $this->assertEquals(42, $bookmark->getId()); @@ -388,7 +394,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertEquals($title, $bookmark->getTitle()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(42); $this->assertEquals(42, $bookmark->getId()); @@ -452,7 +458,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertEquals(43, $bookmark->getId()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(43); $this->assertEquals(43, $bookmark->getId()); @@ -472,7 +478,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertEquals($title, $bookmark->getTitle()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(42); $this->assertEquals(42, $bookmark->getId()); @@ -515,7 +521,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertEquals($title, $bookmark->getTitle()); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $bookmark = $this->privateLinkDB->get(42); $this->assertEquals(42, $bookmark->getId()); @@ -541,7 +547,7 @@ class BookmarkFileServiceTest extends TestCase $this->assertInstanceOf(BookmarkNotFoundException::class, $exception); // reload from file - $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, true); + $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->privateLinkDB->get(42); } @@ -645,7 +651,7 @@ class BookmarkFileServiceTest extends TestCase $conf = new ConfigManager('tests/utils/config/configJson'); $conf->set('resource.datastore', 'null/store.db'); - new BookmarkFileService($conf, $this->history, true); + new BookmarkFileService($conf, $this->history, $this->mutex, true); } /** @@ -655,7 +661,7 @@ class BookmarkFileServiceTest extends TestCase { unlink(self::$testDatastore); $this->assertFileNotExists(self::$testDatastore); - new BookmarkFileService($this->conf, $this->history, true); + new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->assertFileExists(self::$testDatastore); // ensure the correct data has been written @@ -669,7 +675,7 @@ class BookmarkFileServiceTest extends TestCase { unlink(self::$testDatastore); $this->assertFileNotExists(self::$testDatastore); - $db = new \FakeBookmarkService($this->conf, $this->history, false); + $db = new \FakeBookmarkService($this->conf, $this->history, $this->mutex, false); $this->assertFileNotExists(self::$testDatastore); $this->assertInstanceOf(BookmarkArray::class, $db->getBookmarks()); $this->assertCount(0, $db->getBookmarks()); @@ -702,13 +708,13 @@ class BookmarkFileServiceTest extends TestCase */ public function testSave() { - $testDB = new BookmarkFileService($this->conf, $this->history, true); + $testDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $dbSize = $testDB->count(); $bookmark = new Bookmark(); $testDB->add($bookmark); - $testDB = new BookmarkFileService($this->conf, $this->history, true); + $testDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->assertEquals($dbSize + 1, $testDB->count()); } @@ -718,7 +724,7 @@ class BookmarkFileServiceTest extends TestCase public function testCountHiddenPublic() { $this->conf->set('privacy.hide_public_links', true); - $linkDB = new BookmarkFileService($this->conf, $this->history, false); + $linkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, false); $this->assertEquals(0, $linkDB->count()); } diff --git a/tests/bookmark/BookmarkFilterTest.php b/tests/bookmark/BookmarkFilterTest.php index 48c7f824..644abbc8 100644 --- a/tests/bookmark/BookmarkFilterTest.php +++ b/tests/bookmark/BookmarkFilterTest.php @@ -3,6 +3,7 @@ namespace Shaarli\Bookmark; use Exception; +use malkusch\lock\mutex\NoMutex; use ReferenceLinkDB; use Shaarli\Config\ConfigManager; use Shaarli\History; @@ -37,12 +38,13 @@ class BookmarkFilterTest extends TestCase */ public static function setUpBeforeClass(): void { + $mutex = new NoMutex(); $conf = new ConfigManager('tests/utils/config/configJson'); $conf->set('resource.datastore', self::$testDatastore); self::$refDB = new \ReferenceLinkDB(); self::$refDB->write(self::$testDatastore); $history = new History('sandbox/history.php'); - self::$bookmarkService = new \FakeBookmarkService($conf, $history, true); + self::$bookmarkService = new \FakeBookmarkService($conf, $history, $mutex, true); self::$linkFilter = new BookmarkFilter(self::$bookmarkService->getBookmarks()); } diff --git a/tests/bookmark/BookmarkInitializerTest.php b/tests/bookmark/BookmarkInitializerTest.php index 25704004..0c8420ce 100644 --- a/tests/bookmark/BookmarkInitializerTest.php +++ b/tests/bookmark/BookmarkInitializerTest.php @@ -2,6 +2,7 @@ namespace Shaarli\Bookmark; +use malkusch\lock\mutex\NoMutex; use Shaarli\Config\ConfigManager; use Shaarli\History; use Shaarli\TestCase; @@ -34,11 +35,15 @@ class BookmarkInitializerTest extends TestCase /** @var BookmarkInitializer instance */ protected $initializer; + /** @var NoMutex */ + protected $mutex; + /** * Initialize an empty BookmarkFileService */ public function setUp(): void { + $this->mutex = new NoMutex(); if (file_exists(self::$testDatastore)) { unlink(self::$testDatastore); } @@ -47,7 +52,7 @@ class BookmarkInitializerTest extends TestCase $this->conf = new ConfigManager(self::$testConf); $this->conf->set('resource.datastore', self::$testDatastore); $this->history = new History('sandbox/history.php'); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->initializer = new BookmarkInitializer($this->bookmarkService); } @@ -59,7 +64,7 @@ class BookmarkInitializerTest extends TestCase { $refDB = new \ReferenceLinkDB(); $refDB->write(self::$testDatastore); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->initializer = new BookmarkInitializer($this->bookmarkService); $this->initializer->initialize(); @@ -90,7 +95,7 @@ class BookmarkInitializerTest extends TestCase $this->bookmarkService->save(); // Reload from file - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->assertEquals($refDB->countLinks() + 3, $this->bookmarkService->count()); $bookmark = $this->bookmarkService->get(43); @@ -121,7 +126,7 @@ class BookmarkInitializerTest extends TestCase public function testInitializeNonExistentDataStore(): void { $this->conf->set('resource.datastore', static::$testDatastore . '_empty'); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $this->mutex, true); $this->initializer->initialize(); diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 2d675c9a..3508a7b1 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -30,3 +30,7 @@ require_once 'tests/utils/ReferenceLinkDB.php'; require_once 'tests/utils/ReferenceSessionIdHashes.php'; \ReferenceSessionIdHashes::genAllHashes(); + +if (!defined('SHAARLI_MUTEX_FILE')) { + define('SHAARLI_MUTEX_FILE', __FILE__); +} diff --git a/tests/feed/FeedBuilderTest.php b/tests/feed/FeedBuilderTest.php index c29e8ef3..6b9204eb 100644 --- a/tests/feed/FeedBuilderTest.php +++ b/tests/feed/FeedBuilderTest.php @@ -3,6 +3,7 @@ namespace Shaarli\Feed; use DateTime; +use malkusch\lock\mutex\NoMutex; use ReferenceLinkDB; use Shaarli\Bookmark\Bookmark; use Shaarli\Bookmark\BookmarkFileService; @@ -47,6 +48,7 @@ class FeedBuilderTest extends TestCase */ public static function setUpBeforeClass(): void { + $mutex = new NoMutex(); $conf = new ConfigManager('tests/utils/config/configJson'); $conf->set('resource.datastore', self::$testDatastore); $refLinkDB = new \ReferenceLinkDB(); @@ -54,7 +56,7 @@ class FeedBuilderTest extends TestCase $history = new History('sandbox/history.php'); $factory = new FormatterFactory($conf, true); self::$formatter = $factory->getFormatter(); - self::$bookmarkService = new BookmarkFileService($conf, $history, true); + self::$bookmarkService = new BookmarkFileService($conf, $history, $mutex, true); self::$serverInfo = array( 'HTTPS' => 'Off', diff --git a/tests/netscape/BookmarkExportTest.php b/tests/netscape/BookmarkExportTest.php index 9b95ccc9..ad288f78 100644 --- a/tests/netscape/BookmarkExportTest.php +++ b/tests/netscape/BookmarkExportTest.php @@ -2,6 +2,7 @@ namespace Shaarli\Netscape; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Config\ConfigManager; use Shaarli\Formatter\BookmarkFormatter; @@ -56,12 +57,13 @@ class BookmarkExportTest extends TestCase */ public static function setUpBeforeClass(): void { + $mutex = new NoMutex(); static::$conf = new ConfigManager('tests/utils/config/configJson'); static::$conf->set('resource.datastore', static::$testDatastore); static::$refDb = new \ReferenceLinkDB(); static::$refDb->write(static::$testDatastore); static::$history = new History('sandbox/history.php'); - static::$bookmarkService = new BookmarkFileService(static::$conf, static::$history, true); + static::$bookmarkService = new BookmarkFileService(static::$conf, static::$history, $mutex, true); $factory = new FormatterFactory(static::$conf, true); static::$formatter = $factory->getFormatter('raw'); } diff --git a/tests/netscape/BookmarkImportTest.php b/tests/netscape/BookmarkImportTest.php index c1e49b5f..c526d5c8 100644 --- a/tests/netscape/BookmarkImportTest.php +++ b/tests/netscape/BookmarkImportTest.php @@ -3,6 +3,7 @@ namespace Shaarli\Netscape; use DateTime; +use malkusch\lock\mutex\NoMutex; use Psr\Http\Message\UploadedFileInterface; use Shaarli\Bookmark\Bookmark; use Shaarli\Bookmark\BookmarkFileService; @@ -87,6 +88,7 @@ class BookmarkImportTest extends TestCase */ protected function setUp(): void { + $mutex = new NoMutex(); if (file_exists(self::$testDatastore)) { unlink(self::$testDatastore); } @@ -97,7 +99,7 @@ class BookmarkImportTest extends TestCase $this->conf->set('resource.page_cache', $this->pagecache); $this->conf->set('resource.datastore', self::$testDatastore); $this->history = new History(self::$historyFilePath); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, $mutex, true); $this->netscapeBookmarkUtils = new NetscapeBookmarkUtils($this->bookmarkService, $this->conf, $this->history); } diff --git a/tests/updater/UpdaterTest.php b/tests/updater/UpdaterTest.php index a6280b8c..47332544 100644 --- a/tests/updater/UpdaterTest.php +++ b/tests/updater/UpdaterTest.php @@ -2,6 +2,7 @@ namespace Shaarli\Updater; use Exception; +use malkusch\lock\mutex\NoMutex; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Bookmark\BookmarkServiceInterface; use Shaarli\Config\ConfigManager; @@ -44,12 +45,13 @@ class UpdaterTest extends TestCase */ protected function setUp(): void { + $mutex = new NoMutex(); $this->refDB = new \ReferenceLinkDB(); $this->refDB->write(self::$testDatastore); copy('tests/utils/config/configJson.json.php', self::$configFile .'.json.php'); $this->conf = new ConfigManager(self::$configFile); - $this->bookmarkService = new BookmarkFileService($this->conf, $this->createMock(History::class), true); + $this->bookmarkService = new BookmarkFileService($this->conf, $this->createMock(History::class), $mutex, true); $this->updater = new Updater([], $this->bookmarkService, $this->conf, true); } -- cgit v1.2.3 From efb7d21b52eb033530e80e5e49d175e6e3b031f4 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Fri, 2 Oct 2020 17:50:59 +0200 Subject: Add strict types for bookmarks management Parameters typing and using strict types overall increase the codebase quality by enforcing the a given parameter will have the expected type. It also removes the need to unnecessary unit tests checking methods behavior with invalid input. --- application/api/ApiUtils.php | 6 +- application/api/controllers/Links.php | 19 ++-- application/bookmark/Bookmark.php | 121 +++++++++++---------- application/bookmark/BookmarkArray.php | 14 ++- application/bookmark/BookmarkFileService.php | 66 +++++------ application/bookmark/BookmarkFilter.php | 47 ++++---- application/bookmark/BookmarkIO.php | 6 +- application/bookmark/BookmarkInitializer.php | 6 +- application/bookmark/BookmarkServiceInterface.php | 72 ++++++------ application/feed/FeedBuilder.php | 2 +- .../controller/admin/ThumbnailsController.php | 2 +- application/security/LoginManager.php | 2 +- tests/HistoryTest.php | 8 -- tests/bookmark/BookmarkArrayTest.php | 13 --- tests/bookmark/BookmarkFileServiceTest.php | 44 -------- tests/bookmark/BookmarkTest.php | 38 ------- .../DeleteBookmarkTest.php | 4 + .../SaveBookmarkTest.php | 20 +++- .../controller/admin/ThumbnailsControllerTest.php | 4 +- 19 files changed, 209 insertions(+), 285 deletions(-) diff --git a/application/api/ApiUtils.php b/application/api/ApiUtils.php index 4a6326f0..eb1ca9bc 100644 --- a/application/api/ApiUtils.php +++ b/application/api/ApiUtils.php @@ -89,12 +89,12 @@ class ApiUtils * If no URL is provided, it will generate a local note URL. * If no title is provided, it will use the URL as title. * - * @param array $input Request Link. - * @param bool $defaultPrivate Request Link. + * @param array|null $input Request Link. + * @param bool $defaultPrivate Setting defined if a bookmark is private by default. * * @return Bookmark instance. */ - public static function buildBookmarkFromRequest($input, $defaultPrivate): Bookmark + public static function buildBookmarkFromRequest(?array $input, bool $defaultPrivate): Bookmark { $bookmark = new Bookmark(); $url = ! empty($input['url']) ? cleanup_url($input['url']) : ''; diff --git a/application/api/controllers/Links.php b/application/api/controllers/Links.php index 778097fd..73a1b84e 100644 --- a/application/api/controllers/Links.php +++ b/application/api/controllers/Links.php @@ -96,11 +96,12 @@ class Links extends ApiController */ public function getLink($request, $response, $args) { - if (!$this->bookmarkService->exists($args['id'])) { + $id = is_integer_mixed($args['id']) ? (int) $args['id'] : null; + if ($id === null || ! $this->bookmarkService->exists($id)) { throw new ApiLinkNotFoundException(); } $index = index_url($this->ci['environment']); - $out = ApiUtils::formatLink($this->bookmarkService->get($args['id']), $index); + $out = ApiUtils::formatLink($this->bookmarkService->get($id), $index); return $response->withJson($out, 200, $this->jsonStyle); } @@ -115,7 +116,7 @@ class Links extends ApiController */ public function postLink($request, $response) { - $data = $request->getParsedBody(); + $data = (array) ($request->getParsedBody() ?? []); $bookmark = ApiUtils::buildBookmarkFromRequest($data, $this->conf->get('privacy.default_private_links')); // duplicate by URL, return 409 Conflict if (! empty($bookmark->getUrl()) @@ -148,7 +149,8 @@ class Links extends ApiController */ public function putLink($request, $response, $args) { - if (! $this->bookmarkService->exists($args['id'])) { + $id = is_integer_mixed($args['id']) ? (int) $args['id'] : null; + if ($id === null || !$this->bookmarkService->exists($id)) { throw new ApiLinkNotFoundException(); } @@ -159,7 +161,7 @@ class Links extends ApiController // duplicate URL on a different link, return 409 Conflict if (! empty($requestBookmark->getUrl()) && ! empty($dup = $this->bookmarkService->findByUrl($requestBookmark->getUrl())) - && $dup->getId() != $args['id'] + && $dup->getId() != $id ) { return $response->withJson( ApiUtils::formatLink($dup, $index), @@ -168,7 +170,7 @@ class Links extends ApiController ); } - $responseBookmark = $this->bookmarkService->get($args['id']); + $responseBookmark = $this->bookmarkService->get($id); $responseBookmark = ApiUtils::updateLink($responseBookmark, $requestBookmark); $this->bookmarkService->set($responseBookmark); @@ -189,10 +191,11 @@ class Links extends ApiController */ public function deleteLink($request, $response, $args) { - if (! $this->bookmarkService->exists($args['id'])) { + $id = is_integer_mixed($args['id']) ? (int) $args['id'] : null; + if ($id === null || !$this->bookmarkService->exists($id)) { throw new ApiLinkNotFoundException(); } - $bookmark = $this->bookmarkService->get($args['id']); + $bookmark = $this->bookmarkService->get($id); $this->bookmarkService->remove($bookmark); return $response->withStatus(204); diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index 1beb8be2..fa45d2fc 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -1,5 +1,7 @@ id = $data['id']; - $this->shortUrl = $data['shorturl']; - $this->url = $data['url']; - $this->title = $data['title']; - $this->description = $data['description']; - $this->thumbnail = isset($data['thumbnail']) ? $data['thumbnail'] : null; - $this->sticky = isset($data['sticky']) ? $data['sticky'] : false; - $this->created = $data['created']; + $this->id = $data['id'] ?? null; + $this->shortUrl = $data['shorturl'] ?? null; + $this->url = $data['url'] ?? null; + $this->title = $data['title'] ?? null; + $this->description = $data['description'] ?? null; + $this->thumbnail = $data['thumbnail'] ?? null; + $this->sticky = $data['sticky'] ?? false; + $this->created = $data['created'] ?? null; if (is_array($data['tags'])) { $this->tags = $data['tags']; } else { - $this->tags = preg_split('/\s+/', $data['tags'], -1, PREG_SPLIT_NO_EMPTY); + $this->tags = preg_split('/\s+/', $data['tags'] ?? '', -1, PREG_SPLIT_NO_EMPTY); } if (! empty($data['updated'])) { $this->updated = $data['updated']; } - $this->private = $data['private'] ? true : false; + $this->private = ($data['private'] ?? false) ? true : false; return $this; } @@ -95,13 +97,12 @@ class Bookmark * * @throws InvalidBookmarkException */ - public function validate() + public function validate(): void { if ($this->id === null || ! is_int($this->id) || empty($this->shortUrl) || empty($this->created) - || ! $this->created instanceof DateTimeInterface ) { throw new InvalidBookmarkException($this); } @@ -119,11 +120,11 @@ class Bookmark * - created: with the current datetime * - shortUrl: with a generated small hash from the date and the given ID * - * @param int $id + * @param int|null $id * * @return Bookmark */ - public function setId($id) + public function setId(?int $id): Bookmark { $this->id = $id; if (empty($this->created)) { @@ -139,9 +140,9 @@ class Bookmark /** * Get the Id. * - * @return int + * @return int|null */ - public function getId() + public function getId(): ?int { return $this->id; } @@ -149,9 +150,9 @@ class Bookmark /** * Get the ShortUrl. * - * @return string + * @return string|null */ - public function getShortUrl() + public function getShortUrl(): ?string { return $this->shortUrl; } @@ -159,9 +160,9 @@ class Bookmark /** * Get the Url. * - * @return string + * @return string|null */ - public function getUrl() + public function getUrl(): ?string { return $this->url; } @@ -171,7 +172,7 @@ class Bookmark * * @return string */ - public function getTitle() + public function getTitle(): ?string { return $this->title; } @@ -181,7 +182,7 @@ class Bookmark * * @return string */ - public function getDescription() + public function getDescription(): string { return ! empty($this->description) ? $this->description : ''; } @@ -191,7 +192,7 @@ class Bookmark * * @return DateTimeInterface */ - public function getCreated() + public function getCreated(): ?DateTimeInterface { return $this->created; } @@ -201,7 +202,7 @@ class Bookmark * * @return DateTimeInterface */ - public function getUpdated() + public function getUpdated(): ?DateTimeInterface { return $this->updated; } @@ -209,11 +210,11 @@ class Bookmark /** * Set the ShortUrl. * - * @param string $shortUrl + * @param string|null $shortUrl * * @return Bookmark */ - public function setShortUrl($shortUrl) + public function setShortUrl(?string $shortUrl): Bookmark { $this->shortUrl = $shortUrl; @@ -223,14 +224,14 @@ class Bookmark /** * Set the Url. * - * @param string $url - * @param array $allowedProtocols + * @param string|null $url + * @param string[] $allowedProtocols * * @return Bookmark */ - public function setUrl($url, $allowedProtocols = []) + public function setUrl(?string $url, array $allowedProtocols = []): Bookmark { - $url = trim($url); + $url = $url !== null ? trim($url) : ''; if (! empty($url)) { $url = whitelist_protocols($url, $allowedProtocols); } @@ -242,13 +243,13 @@ class Bookmark /** * Set the Title. * - * @param string $title + * @param string|null $title * * @return Bookmark */ - public function setTitle($title) + public function setTitle(?string $title): Bookmark { - $this->title = trim($title); + $this->title = $title !== null ? trim($title) : ''; return $this; } @@ -256,11 +257,11 @@ class Bookmark /** * Set the Description. * - * @param string $description + * @param string|null $description * * @return Bookmark */ - public function setDescription($description) + public function setDescription(?string $description): Bookmark { $this->description = $description; @@ -271,11 +272,11 @@ class Bookmark * Set the Created. * Note: you shouldn't set this manually except for special cases (like bookmark import) * - * @param DateTimeInterface $created + * @param DateTimeInterface|null $created * * @return Bookmark */ - public function setCreated($created) + public function setCreated(?DateTimeInterface $created): Bookmark { $this->created = $created; @@ -285,11 +286,11 @@ class Bookmark /** * Set the Updated. * - * @param DateTimeInterface $updated + * @param DateTimeInterface|null $updated * * @return Bookmark */ - public function setUpdated($updated) + public function setUpdated(?DateTimeInterface $updated): Bookmark { $this->updated = $updated; @@ -301,7 +302,7 @@ class Bookmark * * @return bool */ - public function isPrivate() + public function isPrivate(): bool { return $this->private ? true : false; } @@ -309,11 +310,11 @@ class Bookmark /** * Set the Private. * - * @param bool $private + * @param bool|null $private * * @return Bookmark */ - public function setPrivate($private) + public function setPrivate(?bool $private): Bookmark { $this->private = $private ? true : false; @@ -323,9 +324,9 @@ class Bookmark /** * Get the Tags. * - * @return array + * @return string[] */ - public function getTags() + public function getTags(): array { return is_array($this->tags) ? $this->tags : []; } @@ -333,13 +334,13 @@ class Bookmark /** * Set the Tags. * - * @param array $tags + * @param string[]|null $tags * * @return Bookmark */ - public function setTags($tags) + public function setTags(?array $tags): Bookmark { - $this->setTagsString(implode(' ', $tags)); + $this->setTagsString(implode(' ', $tags ?? [])); return $this; } @@ -357,11 +358,11 @@ class Bookmark /** * Set the Thumbnail. * - * @param string|bool $thumbnail Thumbnail's URL - false if no thumbnail could be found + * @param string|bool|null $thumbnail Thumbnail's URL - false if no thumbnail could be found * * @return Bookmark */ - public function setThumbnail($thumbnail) + public function setThumbnail($thumbnail): Bookmark { $this->thumbnail = $thumbnail; @@ -373,7 +374,7 @@ class Bookmark * * @return bool */ - public function isSticky() + public function isSticky(): bool { return $this->sticky ? true : false; } @@ -381,11 +382,11 @@ class Bookmark /** * Set the Sticky. * - * @param bool $sticky + * @param bool|null $sticky * * @return Bookmark */ - public function setSticky($sticky) + public function setSticky(?bool $sticky): Bookmark { $this->sticky = $sticky ? true : false; @@ -395,7 +396,7 @@ class Bookmark /** * @return string Bookmark's tags as a string, separated by a space */ - public function getTagsString() + public function getTagsString(): string { return implode(' ', $this->getTags()); } @@ -403,7 +404,7 @@ class Bookmark /** * @return bool */ - public function isNote() + public function isNote(): bool { // We check empty value to get a valid result if the link has not been saved yet return empty($this->url) || startsWith($this->url, '/shaare/') || $this->url[0] === '?'; @@ -416,14 +417,14 @@ class Bookmark * - multiple spaces will be removed * - trailing dash in tags will be removed * - * @param string $tags + * @param string|null $tags * * @return $this */ - public function setTagsString($tags) + public function setTagsString(?string $tags): Bookmark { // Remove first '-' char in tags. - $tags = preg_replace('/(^| )\-/', '$1', $tags); + $tags = preg_replace('/(^| )\-/', '$1', $tags ?? ''); // Explode all tags separted by spaces or commas $tags = preg_split('/[\s,]+/', $tags); // Remove eventual empty values @@ -440,7 +441,7 @@ class Bookmark * @param string $fromTag * @param string $toTag */ - public function renameTag($fromTag, $toTag) + public function renameTag(string $fromTag, string $toTag): void { if (($pos = array_search($fromTag, $this->tags)) !== false) { $this->tags[$pos] = trim($toTag); @@ -452,7 +453,7 @@ class Bookmark * * @param string $tag */ - public function deleteTag($tag) + public function deleteTag(string $tag): void { if (($pos = array_search($tag, $this->tags)) !== false) { unset($this->tags[$pos]); diff --git a/application/bookmark/BookmarkArray.php b/application/bookmark/BookmarkArray.php index 3bd5eb20..67bb3b73 100644 --- a/application/bookmark/BookmarkArray.php +++ b/application/bookmark/BookmarkArray.php @@ -1,5 +1,7 @@ ids[$id])) { + if ($id !== null && isset($this->ids[$id])) { return $this->ids[$id]; } return null; @@ -205,7 +207,7 @@ class BookmarkArray implements \Iterator, \Countable, \ArrayAccess * * @return int next ID. */ - public function getNextId() + public function getNextId(): int { if (!empty($this->ids)) { return max(array_keys($this->ids)) + 1; @@ -214,11 +216,11 @@ class BookmarkArray implements \Iterator, \Countable, \ArrayAccess } /** - * @param $url + * @param string $url * * @return Bookmark|null */ - public function getByUrl($url) + public function getByUrl(string $url): ?Bookmark { if (! empty($url) && isset($this->urls[$url]) diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php index 1ba00712..804b2520 100644 --- a/application/bookmark/BookmarkFileService.php +++ b/application/bookmark/BookmarkFileService.php @@ -1,9 +1,10 @@ conf = $conf; $this->history = $history; @@ -96,7 +97,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function findByHash($hash) + public function findByHash(string $hash): Bookmark { $bookmark = $this->bookmarkFilter->filter(BookmarkFilter::$FILTER_HASH, $hash); // PHP 7.3 introduced array_key_first() to avoid this hack @@ -111,7 +112,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function findByUrl($url) + public function findByUrl(string $url): ?Bookmark { return $this->bookmarks->getByUrl($url); } @@ -120,10 +121,10 @@ class BookmarkFileService implements BookmarkServiceInterface * @inheritDoc */ public function search( - $request = [], - $visibility = null, - $caseSensitive = false, - $untaggedOnly = false, + array $request = [], + string $visibility = null, + bool $caseSensitive = false, + bool $untaggedOnly = false, bool $ignoreSticky = false ) { if ($visibility === null) { @@ -131,8 +132,8 @@ class BookmarkFileService implements BookmarkServiceInterface } // Filter bookmark database according to parameters. - $searchtags = isset($request['searchtags']) ? $request['searchtags'] : ''; - $searchterm = isset($request['searchterm']) ? $request['searchterm'] : ''; + $searchTags = isset($request['searchtags']) ? $request['searchtags'] : ''; + $searchTerm = isset($request['searchterm']) ? $request['searchterm'] : ''; if ($ignoreSticky) { $this->bookmarks->reorder('DESC', true); @@ -140,7 +141,7 @@ class BookmarkFileService implements BookmarkServiceInterface return $this->bookmarkFilter->filter( BookmarkFilter::$FILTER_TAG | BookmarkFilter::$FILTER_TEXT, - [$searchtags, $searchterm], + [$searchTags, $searchTerm], $caseSensitive, $visibility, $untaggedOnly @@ -150,7 +151,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function get($id, $visibility = null) + public function get(int $id, string $visibility = null): Bookmark { if (! isset($this->bookmarks[$id])) { throw new BookmarkNotFoundException(); @@ -173,20 +174,17 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function set($bookmark, $save = true) + public function set(Bookmark $bookmark, bool $save = true): Bookmark { if (true !== $this->isLoggedIn) { throw new Exception(t('You\'re not authorized to alter the datastore')); } - if (! $bookmark instanceof Bookmark) { - throw new Exception(t('Provided data is invalid')); - } if (! isset($this->bookmarks[$bookmark->getId()])) { throw new BookmarkNotFoundException(); } $bookmark->validate(); - $bookmark->setUpdated(new \DateTime()); + $bookmark->setUpdated(new DateTime()); $this->bookmarks[$bookmark->getId()] = $bookmark; if ($save === true) { $this->save(); @@ -198,15 +196,12 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function add($bookmark, $save = true) + public function add(Bookmark $bookmark, bool $save = true): Bookmark { if (true !== $this->isLoggedIn) { throw new Exception(t('You\'re not authorized to alter the datastore')); } - if (! $bookmark instanceof Bookmark) { - throw new Exception(t('Provided data is invalid')); - } - if (! empty($bookmark->getId())) { + if (!empty($bookmark->getId())) { throw new Exception(t('This bookmarks already exists')); } $bookmark->setId($this->bookmarks->getNextId()); @@ -223,14 +218,11 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function addOrSet($bookmark, $save = true) + public function addOrSet(Bookmark $bookmark, bool $save = true): Bookmark { if (true !== $this->isLoggedIn) { throw new Exception(t('You\'re not authorized to alter the datastore')); } - if (! $bookmark instanceof Bookmark) { - throw new Exception('Provided data is invalid'); - } if ($bookmark->getId() === null) { return $this->add($bookmark, $save); } @@ -240,14 +232,11 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function remove($bookmark, $save = true) + public function remove(Bookmark $bookmark, bool $save = true): void { if (true !== $this->isLoggedIn) { throw new Exception(t('You\'re not authorized to alter the datastore')); } - if (! $bookmark instanceof Bookmark) { - throw new Exception(t('Provided data is invalid')); - } if (! isset($this->bookmarks[$bookmark->getId()])) { throw new BookmarkNotFoundException(); } @@ -262,7 +251,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function exists($id, $visibility = null) + public function exists(int $id, string $visibility = null): bool { if (! isset($this->bookmarks[$id])) { return false; @@ -285,7 +274,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function count($visibility = null) + public function count(string $visibility = null): int { return count($this->search([], $visibility)); } @@ -293,7 +282,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function save() + public function save(): void { if (true !== $this->isLoggedIn) { // TODO: raise an Exception instead @@ -308,7 +297,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function bookmarksCountPerTag($filteringTags = [], $visibility = null) + public function bookmarksCountPerTag(array $filteringTags = [], string $visibility = null): array { $bookmarks = $this->search(['searchtags' => $filteringTags], $visibility); $tags = []; @@ -344,13 +333,14 @@ class BookmarkFileService implements BookmarkServiceInterface $keys = array_keys($tags); $tmpTags = array_combine($keys, $keys); array_multisort($tags, SORT_DESC, $tmpTags, SORT_ASC, $tags); + return $tags; } /** * @inheritDoc */ - public function days() + public function days(): array { $bookmarkDays = []; foreach ($this->search() as $bookmark) { @@ -365,7 +355,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function filterDay($request) + public function filterDay(string $request) { $visibility = $this->isLoggedIn ? BookmarkFilter::$ALL : BookmarkFilter::$PUBLIC; @@ -375,7 +365,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function initialize() + public function initialize(): void { $initializer = new BookmarkInitializer($this); $initializer->initialize(); @@ -388,7 +378,7 @@ class BookmarkFileService implements BookmarkServiceInterface /** * Handles migration to the new database format (BookmarksArray). */ - protected function migrate() + protected function migrate(): void { $bookmarkDb = new LegacyLinkDB( $this->conf->get('resource.datastore'), diff --git a/application/bookmark/BookmarkFilter.php b/application/bookmark/BookmarkFilter.php index 6636bbfe..4232f114 100644 --- a/application/bookmark/BookmarkFilter.php +++ b/application/bookmark/BookmarkFilter.php @@ -1,5 +1,7 @@ bookmarks; @@ -151,11 +158,11 @@ class BookmarkFilter * * @param string $smallHash permalink hash. * - * @return array $filtered array containing permalink data. + * @return Bookmark[] $filtered array containing permalink data. * - * @throws \Shaarli\Bookmark\Exception\BookmarkNotFoundException if the smallhash doesn't match any link. + * @throws BookmarkNotFoundException if the smallhash doesn't match any link. */ - private function filterSmallHash($smallHash) + private function filterSmallHash(string $smallHash) { foreach ($this->bookmarks as $key => $l) { if ($smallHash == $l->getShortUrl()) { @@ -186,9 +193,9 @@ class BookmarkFilter * @param string $searchterms search query. * @param string $visibility Optional: return only all/private/public bookmarks. * - * @return array search results. + * @return Bookmark[] search results. */ - private function filterFulltext($searchterms, $visibility = 'all') + private function filterFulltext(string $searchterms, string $visibility = 'all') { if (empty($searchterms)) { return $this->noFilter($visibility); @@ -268,7 +275,7 @@ class BookmarkFilter * * @return string generated regex fragment */ - private static function tag2regex($tag) + private static function tag2regex(string $tag): string { $len = strlen($tag); if (!$len || $tag === "-" || $tag === "*") { @@ -314,13 +321,13 @@ class BookmarkFilter * You can specify one or more tags, separated by space or a comma, e.g. * print_r($mydb->filterTags('linux programming')); * - * @param string $tags list of tags separated by commas or blank spaces. - * @param bool $casesensitive ignore case if false. - * @param string $visibility Optional: return only all/private/public bookmarks. + * @param string|array $tags list of tags, separated by commas or blank spaces if passed as string. + * @param bool $casesensitive ignore case if false. + * @param string $visibility Optional: return only all/private/public bookmarks. * - * @return array filtered bookmarks. + * @return Bookmark[] filtered bookmarks. */ - public function filterTags($tags, $casesensitive = false, $visibility = 'all') + public function filterTags($tags, bool $casesensitive = false, string $visibility = 'all') { // get single tags (we may get passed an array, even though the docs say different) $inputTags = $tags; @@ -396,9 +403,9 @@ class BookmarkFilter * * @param string $visibility return only all/private/public bookmarks. * - * @return array filtered bookmarks. + * @return Bookmark[] filtered bookmarks. */ - public function filterUntagged($visibility) + public function filterUntagged(string $visibility) { $filtered = []; foreach ($this->bookmarks as $key => $link) { @@ -427,11 +434,11 @@ class BookmarkFilter * @param string $day day to filter. * @param string $visibility return only all/private/public bookmarks. - * @return array all link matching given day. + * @return Bookmark[] all link matching given day. * * @throws Exception if date format is invalid. */ - public function filterDay($day, $visibility) + public function filterDay(string $day, string $visibility) { if (!checkDateFormat('Ymd', $day)) { throw new Exception('Invalid date format'); @@ -460,9 +467,9 @@ class BookmarkFilter * @param string $tags string containing a list of tags. * @param bool $casesensitive will convert everything to lowercase if false. * - * @return array filtered tags string. + * @return string[] filtered tags string. */ - public static function tagsStrToArray($tags, $casesensitive) + public static function tagsStrToArray(string $tags, bool $casesensitive): array { // We use UTF-8 conversion to handle various graphemes (i.e. cyrillic, or greek) $tagsOut = $casesensitive ? $tags : mb_convert_case($tags, MB_CASE_LOWER, 'UTF-8'); diff --git a/application/bookmark/BookmarkIO.php b/application/bookmark/BookmarkIO.php index 099653b0..f40fa476 100644 --- a/application/bookmark/BookmarkIO.php +++ b/application/bookmark/BookmarkIO.php @@ -1,5 +1,7 @@ bookmarkService = $bookmarkService; } @@ -31,7 +33,7 @@ class BookmarkInitializer /** * Initialize the data store with default bookmarks */ - public function initialize() + public function initialize(): void { $bookmark = new Bookmark(); $bookmark->setTitle('quicksilver (loop) on Vimeo ' . t('(private bookmark with thumbnail demo)')); diff --git a/application/bookmark/BookmarkServiceInterface.php b/application/bookmark/BookmarkServiceInterface.php index 638cfa5f..37a54d03 100644 --- a/application/bookmark/BookmarkServiceInterface.php +++ b/application/bookmark/BookmarkServiceInterface.php @@ -1,7 +1,8 @@ bookmarksCount */ - public function bookmarksCountPerTag($filteringTags = [], $visibility = 'all'); + public function bookmarksCountPerTag(array $filteringTags = [], ?string $visibility = null): array; /** * Returns the list of days containing articles (oldest first) * * @return array containing days (in format YYYYMMDD). */ - public function days(); + public function days(): array; /** * Returns the list of articles for a given day. @@ -166,10 +170,10 @@ interface BookmarkServiceInterface * * @throws BookmarkNotFoundException */ - public function filterDay($request); + public function filterDay(string $request); /** * Creates the default database after a fresh install. */ - public function initialize(); + public function initialize(): void; } diff --git a/application/feed/FeedBuilder.php b/application/feed/FeedBuilder.php index f6def630..f70fce4f 100644 --- a/application/feed/FeedBuilder.php +++ b/application/feed/FeedBuilder.php @@ -102,7 +102,7 @@ class FeedBuilder } // Optionally filter the results: - $linksToDisplay = $this->linkDB->search($userInput, null, false, false, true); + $linksToDisplay = $this->linkDB->search($userInput ?? [], null, false, false, true); $nblinksToDisplay = $this->getNbLinks(count($linksToDisplay), $userInput); diff --git a/application/front/controller/admin/ThumbnailsController.php b/application/front/controller/admin/ThumbnailsController.php index 81c87ed0..4dc09d38 100644 --- a/application/front/controller/admin/ThumbnailsController.php +++ b/application/front/controller/admin/ThumbnailsController.php @@ -52,7 +52,7 @@ class ThumbnailsController extends ShaarliAdminController } try { - $bookmark = $this->container->bookmarkService->get($id); + $bookmark = $this->container->bookmarkService->get((int) $id); } catch (BookmarkNotFoundException $e) { return $response->withStatus(404); } diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index d74c3118..65048f10 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -118,7 +118,7 @@ class LoginManager * * @return true when the user is logged in, false otherwise */ - public function isLoggedIn() + public function isLoggedIn(): bool { if ($this->openShaarli) { return true; diff --git a/tests/HistoryTest.php b/tests/HistoryTest.php index 6dc0e5b7..e810104e 100644 --- a/tests/HistoryTest.php +++ b/tests/HistoryTest.php @@ -89,14 +89,6 @@ class HistoryTest extends \Shaarli\TestCase $this->assertEquals(History::CREATED, $actual['event']); $this->assertTrue(new DateTime('-2 seconds') < $actual['datetime']); $this->assertEquals(1, $actual['id']); - - $history = new History(self::$historyFilePath); - $bookmark = (new Bookmark())->setId('str'); - $history->addLink($bookmark); - $actual = $history->getHistory()[0]; - $this->assertEquals(History::CREATED, $actual['event']); - $this->assertTrue(new DateTime('-2 seconds') < $actual['datetime']); - $this->assertEquals('str', $actual['id']); } // /** diff --git a/tests/bookmark/BookmarkArrayTest.php b/tests/bookmark/BookmarkArrayTest.php index ebed9bfc..1953078c 100644 --- a/tests/bookmark/BookmarkArrayTest.php +++ b/tests/bookmark/BookmarkArrayTest.php @@ -90,19 +90,6 @@ class BookmarkArrayTest extends TestCase $array['nope'] = $bookmark; } - /** - * Test adding a bad entry: invalid ID type - */ - public function testArrayAccessAddBadEntryIdType() - { - $this->expectException(\Shaarli\Bookmark\Exception\InvalidBookmarkException::class); - - $array = new BookmarkArray(); - $bookmark = (new Bookmark())->setId('nope'); - $bookmark->validate(); - $array[] = $bookmark; - } - /** * Test adding a bad entry: ID/offset not consistent */ diff --git a/tests/bookmark/BookmarkFileServiceTest.php b/tests/bookmark/BookmarkFileServiceTest.php index 6c56dfaa..59c0608c 100644 --- a/tests/bookmark/BookmarkFileServiceTest.php +++ b/tests/bookmark/BookmarkFileServiceTest.php @@ -264,17 +264,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->add(new Bookmark()); } - /** - * Test add() method with an entry which is not a bookmark instance - */ - public function testAddNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->add(['title' => 'hi!']); - } - /** * Test add() method with a Bookmark already containing an ID */ @@ -412,17 +401,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->set(new Bookmark()); } - /** - * Test set() method with an entry which is not a bookmark instance - */ - public function testSetNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->set(['title' => 'hi!']); - } - /** * Test set() method with a Bookmark without an ID defined. */ @@ -496,17 +474,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->addOrSet(new Bookmark()); } - /** - * Test addOrSet() method with an entry which is not a bookmark instance - */ - public function testAddOrSetNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->addOrSet(['title' => 'hi!']); - } - /** * Test addOrSet() method for a bookmark without any field set and without writing the data store */ @@ -564,17 +531,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->remove($bookmark); } - /** - * Test remove() method with an entry which is not a bookmark instance - */ - public function testRemoveNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->remove(['title' => 'hi!']); - } - /** * Test remove() method with a Bookmark with an unknown ID */ diff --git a/tests/bookmark/BookmarkTest.php b/tests/bookmark/BookmarkTest.php index afec2440..4c7ae4c0 100644 --- a/tests/bookmark/BookmarkTest.php +++ b/tests/bookmark/BookmarkTest.php @@ -153,25 +153,6 @@ class BookmarkTest extends TestCase $this->assertContainsPolyfill('- ID: '. PHP_EOL, $exception->getMessage()); } - /** - * Test validate() with a a bookmark with a non integer ID. - */ - public function testValidateNotValidStringId() - { - $bookmark = new Bookmark(); - $bookmark->setId('str'); - $bookmark->setShortUrl('abc'); - $bookmark->setCreated(\DateTime::createFromFormat('Ymd_His', '20190514_200102')); - $exception = null; - try { - $bookmark->validate(); - } catch (InvalidBookmarkException $e) { - $exception = $e; - } - $this->assertNotNull($exception); - $this->assertContainsPolyfill('- ID: str'. PHP_EOL, $exception->getMessage()); - } - /** * Test validate() with a a bookmark without short url. */ @@ -210,25 +191,6 @@ class BookmarkTest extends TestCase $this->assertContainsPolyfill('- Created: '. PHP_EOL, $exception->getMessage()); } - /** - * Test validate() with a a bookmark with a bad created datetime. - */ - public function testValidateNotValidBadCreated() - { - $bookmark = new Bookmark(); - $bookmark->setId(1); - $bookmark->setShortUrl('abc'); - $bookmark->setCreated('hi!'); - $exception = null; - try { - $bookmark->validate(); - } catch (InvalidBookmarkException $e) { - $exception = $e; - } - $this->assertNotNull($exception); - $this->assertContainsPolyfill('- Created: Not a DateTime object'. PHP_EOL, $exception->getMessage()); - } - /** * Test setId() and make sure that default fields are generated. */ diff --git a/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php b/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php index ba774e21..83bbee7c 100644 --- a/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php +++ b/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php @@ -356,6 +356,10 @@ class DeleteBookmarkTest extends TestCase ; $response = new Response(); + $this->container->bookmarkService->method('get')->with('123')->willReturn( + (new Bookmark())->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123') + ); + $this->container->formatterFactory = $this->createMock(FormatterFactory::class); $this->container->formatterFactory ->expects(static::once()) diff --git a/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php b/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php index f7a68226..37542c26 100644 --- a/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php +++ b/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php @@ -66,23 +66,27 @@ class SaveBookmarkTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('addOrSet') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertFalse($save); $checkBookmark($bookmark); $bookmark->setId($id); + + return $bookmark; }) ; $this->container->bookmarkService ->expects(static::once()) ->method('set') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertTrue($save); $checkBookmark($bookmark); static::assertSame($id, $bookmark->getId()); + + return $bookmark; }) ; @@ -155,21 +159,25 @@ class SaveBookmarkTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('addOrSet') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertFalse($save); $checkBookmark($bookmark); + + return $bookmark; }) ; $this->container->bookmarkService ->expects(static::once()) ->method('set') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertTrue($save); $checkBookmark($bookmark); static::assertSame($id, $bookmark->getId()); + + return $bookmark; }) ; @@ -230,8 +238,10 @@ class SaveBookmarkTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('addOrSet') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($thumb): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($thumb): Bookmark { static::assertSame($thumb, $bookmark->getThumbnail()); + + return $bookmark; }) ; diff --git a/tests/front/controller/admin/ThumbnailsControllerTest.php b/tests/front/controller/admin/ThumbnailsControllerTest.php index f4a8acff..e5749654 100644 --- a/tests/front/controller/admin/ThumbnailsControllerTest.php +++ b/tests/front/controller/admin/ThumbnailsControllerTest.php @@ -89,8 +89,10 @@ class ThumbnailsControllerTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('set') - ->willReturnCallback(function (Bookmark $bookmark) use ($thumb) { + ->willReturnCallback(function (Bookmark $bookmark) use ($thumb): Bookmark { static::assertSame($thumb, $bookmark->getThumbnail()); + + return $bookmark; }) ; -- cgit v1.2.3 From ec457491879893c8cfcc9dd6542d1593aa5c91f5 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 15 Oct 2020 08:59:51 +0200 Subject: Doc: add PHP 7.4 and 8.0 as supported version --- doc/md/Server-configuration.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/md/Server-configuration.md b/doc/md/Server-configuration.md index 297d7c29..14070c8a 100644 --- a/doc/md/Server-configuration.md +++ b/doc/md/Server-configuration.md @@ -40,6 +40,8 @@ Supported PHP versions: Version | Status | Shaarli compatibility :---:|:---:|:---: +8.0 | Supported | Yes +7.4 | Supported | Yes 7.3 | Supported | Yes 7.2 | Supported | Yes 7.1 | Supported | Yes @@ -53,7 +55,7 @@ Required PHP extensions: Extension | Required? | Usage ---|:---:|--- -[`openssl`](http://php.net/manual/en/book.openssl.php) | requires | OpenSSL, HTTPS +[`openssl`](http://php.net/manual/en/book.openssl.php) | required | OpenSSL, HTTPS [`php-json`](http://php.net/manual/en/book.json.php) | required | configuration parsing [`php-simplexml`](https://www.php.net/manual/en/book.simplexml.php) | required | REST API (Slim framework) [`php-mbstring`](http://php.net/manual/en/book.mbstring.php) | CentOS, Fedora, RHEL, Windows, some hosting providers | multibyte (Unicode) string support @@ -421,7 +423,7 @@ By default Shaarli already disallows indexing of your local copy of the document before = common.conf [Definition] failregex = \s-\s\s-\sLogin failed for user.*$ -ignoreregex = +ignoreregex = ``` ```ini -- cgit v1.2.3 From 4cf3564d28dc8e4d08a3e64f09ad045ffbde97ae Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Fri, 25 Sep 2020 13:29:36 +0200 Subject: Add a setting to retrieve bookmark metadata asynchrounously - There is a new standalone script (metadata.js) which requests a new controller to get bookmark metadata and fill the form async - This feature is enabled with the new setting: general.enable_async_metadata (enabled by default) - general.retrieve_description is now enabled by default - A small rotating loader animation has a been added to bookmark inputs when metadata is being retrieved (default template) - Custom JS htmlentities has been removed and mathiasbynens/he library is used instead Fixes #1563 --- Makefile | 1 + application/config/ConfigManager.php | 3 +- application/container/ContainerBuilder.php | 5 + application/container/ShaarliContainer.php | 2 + .../controller/admin/ManageShaareController.php | 36 ++---- .../front/controller/admin/MetadataController.php | 29 +++++ application/http/MetadataRetriever.php | 68 ++++++++++++ assets/common/js/metadata.js | 39 +++++++ assets/default/js/base.js | 12 +- assets/default/scss/shaarli.scss | 51 +++++++++ doc/md/Shaarli-configuration.md | 1 + index.php | 2 +- package.json | 1 + tests/container/ContainerBuilderTest.php | 2 + .../DisplayCreateFormTest.php | 118 ++++++++++++++------ tests/http/MetadataRetrieverTest.php | 123 +++++++++++++++++++++ tpl/default/editlink.html | 22 +++- webpack.config.js | 2 + yarn.lock | 5 + 19 files changed, 447 insertions(+), 75 deletions(-) create mode 100644 application/front/controller/admin/MetadataController.php create mode 100644 application/http/MetadataRetriever.php create mode 100644 assets/common/js/metadata.js create mode 100644 tests/http/MetadataRetrieverTest.php diff --git a/Makefile b/Makefile index 0ff6bd3f..7415887a 100644 --- a/Makefile +++ b/Makefile @@ -175,6 +175,7 @@ translate: eslint: @yarn run eslint -c .dev/.eslintrc.js assets/vintage/js/ @yarn run eslint -c .dev/.eslintrc.js assets/default/js/ + @yarn run eslint -c .dev/.eslintrc.js assets/common/js/ ### Run CSSLint check against Shaarli's SCSS files sasslint: diff --git a/application/config/ConfigManager.php b/application/config/ConfigManager.php index 4c98be30..fb085023 100644 --- a/application/config/ConfigManager.php +++ b/application/config/ConfigManager.php @@ -366,7 +366,8 @@ class ConfigManager $this->setEmpty('general.links_per_page', 20); $this->setEmpty('general.enabled_plugins', self::$DEFAULT_PLUGINS); $this->setEmpty('general.default_note_title', 'Note: '); - $this->setEmpty('general.retrieve_description', false); + $this->setEmpty('general.retrieve_description', true); + $this->setEmpty('general.enable_async_metadata', true); $this->setEmpty('updates.check_updates', false); $this->setEmpty('updates.check_updates_branch', 'stable'); diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php index c21d58dd..fd94a1c3 100644 --- a/application/container/ContainerBuilder.php +++ b/application/container/ContainerBuilder.php @@ -14,6 +14,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; @@ -90,6 +91,10 @@ class ContainerBuilder ); }; + $container['metadataRetriever'] = function (ShaarliContainer $container): MetadataRetriever { + return new MetadataRetriever($container->conf, $container->httpAccess); + }; + $container['pageBuilder'] = function (ShaarliContainer $container): PageBuilder { return new PageBuilder( $container->conf, diff --git a/application/container/ShaarliContainer.php b/application/container/ShaarliContainer.php index 66e669aa..3a7c238f 100644 --- a/application/container/ShaarliContainer.php +++ b/application/container/ShaarliContainer.php @@ -10,6 +10,7 @@ use Shaarli\Feed\FeedBuilder; use Shaarli\Formatter\FormatterFactory; use Shaarli\History; use Shaarli\Http\HttpAccess; +use Shaarli\Http\MetadataRetriever; use Shaarli\Netscape\NetscapeBookmarkUtils; use Shaarli\Plugin\PluginManager; use Shaarli\Render\PageBuilder; @@ -35,6 +36,7 @@ use Slim\Container; * @property History $history * @property HttpAccess $httpAccess * @property LoginManager $loginManager + * @property MetadataRetriever $metadataRetriever * @property NetscapeBookmarkUtils $netscapeBookmarkUtils * @property callable $notFoundHandler Overrides default Slim exception display * @property PageBuilder $pageBuilder diff --git a/application/front/controller/admin/ManageShaareController.php b/application/front/controller/admin/ManageShaareController.php index bb083486..df2f1631 100644 --- a/application/front/controller/admin/ManageShaareController.php +++ b/application/front/controller/admin/ManageShaareController.php @@ -53,36 +53,22 @@ class ManageShaareController extends ShaarliAdminController // If this is an HTTP(S) link, we try go get the page to extract // the title (otherwise we will to straight to the edit form.) - if (empty($title) && strpos(get_url_scheme($url) ?: '', 'http') !== false) { - $retrieveDescription = $this->container->conf->get('general.retrieve_description'); - // Short timeout to keep the application responsive - // The callback will fill $charset and $title with data from the downloaded page. - $this->container->httpAccess->getHttpResponse( - $url, - $this->container->conf->get('general.download_timeout', 30), - $this->container->conf->get('general.download_max_size', 4194304), - $this->container->httpAccess->getCurlDownloadCallback( - $charset, - $title, - $description, - $tags, - $retrieveDescription - ) - ); - if (! empty($title) && strtolower($charset) !== 'utf-8' && mb_check_encoding($charset)) { - $title = mb_convert_encoding($title, 'utf-8', $charset); - } + if (true !== $this->container->conf->get('general.enable_async_metadata', true) + && empty($title) + && strpos(get_url_scheme($url) ?: '', 'http') !== false + ) { + $metadata = $this->container->metadataRetriever->retrieve($url); } - if (empty($url) && empty($title)) { - $title = $this->container->conf->get('general.default_note_title', t('Note: ')); + if (empty($url)) { + $metadata['title'] = $this->container->conf->get('general.default_note_title', t('Note: ')); } $link = [ - 'title' => $title, + 'title' => $title ?? $metadata['title'] ?? '', 'url' => $url ?? '', - 'description' => $description ?? '', - 'tags' => $tags ?? '', + 'description' => $description ?? $metadata['description'] ?? '', + 'tags' => $tags ?? $metadata['tags'] ?? '', 'private' => $private, ]; } else { @@ -352,6 +338,8 @@ class ManageShaareController extends ShaarliAdminController 'source' => $request->getParam('source') ?? '', 'tags' => $tags, 'default_private_links' => $this->container->conf->get('privacy.default_private_links', false), + 'async_metadata' => $this->container->conf->get('general.enable_async_metadata', true), + 'retrieve_description' => $this->container->conf->get('general.retrieve_description', false), ]); $this->executePageHooks('render_editlink', $data, TemplatePage::EDIT_LINK); diff --git a/application/front/controller/admin/MetadataController.php b/application/front/controller/admin/MetadataController.php new file mode 100644 index 00000000..ff845944 --- /dev/null +++ b/application/front/controller/admin/MetadataController.php @@ -0,0 +1,29 @@ +getParam('url'); + + // Only try to extract metadata from URL with HTTP(s) scheme + if (!empty($url) && strpos(get_url_scheme($url) ?: '', 'http') !== false) { + return $response->withJson($this->container->metadataRetriever->retrieve($url)); + } + + return $response->withJson([]); + } +} diff --git a/application/http/MetadataRetriever.php b/application/http/MetadataRetriever.php new file mode 100644 index 00000000..2ca982e2 --- /dev/null +++ b/application/http/MetadataRetriever.php @@ -0,0 +1,68 @@ +conf = $conf; + $this->httpAccess = $httpAccess; + } + + /** + * Retrieve metadata for given URL. + * + * @return array [ + * 'title' => , + * 'description' => , + * 'tags' => , + * ] + */ + public function retrieve(string $url): array + { + $charset = null; + $title = null; + $description = null; + $tags = null; + $retrieveDescription = $this->conf->get('general.retrieve_description'); + + // Short timeout to keep the application responsive + // The callback will fill $charset and $title with data from the downloaded page. + $this->httpAccess->getHttpResponse( + $url, + $this->conf->get('general.download_timeout', 30), + $this->conf->get('general.download_max_size', 4194304), + $this->httpAccess->getCurlDownloadCallback( + $charset, + $title, + $description, + $tags, + $retrieveDescription + ) + ); + + if (!empty($title) && strtolower($charset) !== 'utf-8') { + $title = mb_convert_encoding($title, 'utf-8', $charset); + } + + return [ + 'title' => $title, + 'description' => $description, + 'tags' => $tags, + ]; + } +} diff --git a/assets/common/js/metadata.js b/assets/common/js/metadata.js new file mode 100644 index 00000000..5200b481 --- /dev/null +++ b/assets/common/js/metadata.js @@ -0,0 +1,39 @@ +import he from 'he'; + +function clearLoaders(loaders) { + if (loaders != null && loaders.length > 0) { + [...loaders].forEach((loader) => { + loader.classList.remove('loading-input'); + }); + } +} + +(() => { + const loaders = document.querySelectorAll('.loading-input'); + const inputTitle = document.querySelector('input[name="lf_title"]'); + if (inputTitle != null && inputTitle.value.length > 0) { + clearLoaders(loaders); + return; + } + + const url = document.querySelector('input[name="lf_url"]').value; + const basePath = document.querySelector('input[name="js_base_path"]').value; + + const xhr = new XMLHttpRequest(); + xhr.open('GET', `${basePath}/admin/metadata?url=${encodeURI(url)}`, true); + xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded'); + xhr.onload = () => { + const result = JSON.parse(xhr.response); + Object.keys(result).forEach((key) => { + if (result[key] !== null && result[key].length) { + const element = document.querySelector(`input[name="lf_${key}"], textarea[name="lf_${key}"]`); + if (element != null && element.value.length === 0) { + element.value = he.decode(result[key]); + } + } + }); + clearLoaders(loaders); + }; + + xhr.send(); +})(); diff --git a/assets/default/js/base.js b/assets/default/js/base.js index be986ae0..31688815 100644 --- a/assets/default/js/base.js +++ b/assets/default/js/base.js @@ -1,4 +1,5 @@ import Awesomplete from 'awesomplete'; +import he from 'he'; /** * Find a parent element according to its tag and its attributes @@ -95,15 +96,6 @@ function updateAwesompleteList(selector, tags, instances) { return instances; } -/** - * html_entities in JS - * - * @see http://stackoverflow.com/questions/18749591/encode-html-entities-in-javascript - */ -function htmlEntities(str) { - return str.replace(/[\u00A0-\u9999<>&]/gim, (i) => `&#${i.charCodeAt(0)};`); -} - /** * Add the class 'hidden' to city options not attached to the current selected continent. * @@ -569,7 +561,7 @@ function init(description) { input.setAttribute('name', totag); input.setAttribute('value', totag); findParent(input, 'div', { class: 'rename-tag-form' }).style.display = 'none'; - block.querySelector('a.tag-link').innerHTML = htmlEntities(totag); + block.querySelector('a.tag-link').innerHTML = he.encode(totag); block .querySelector('a.tag-link') .setAttribute('href', `${basePath}/?searchtags=${encodeURIComponent(totag)}`); diff --git a/assets/default/scss/shaarli.scss b/assets/default/scss/shaarli.scss index a528adb0..df9c867b 100644 --- a/assets/default/scss/shaarli.scss +++ b/assets/default/scss/shaarli.scss @@ -1269,6 +1269,57 @@ form { } } +.loading-input { + position: relative; + + @keyframes around { + 0% { + transform: rotate(0deg); + } + + 100% { + transform: rotate(360deg); + } + } + + .icon-container { + position: absolute; + right: 60px; + top: calc(50% - 10px); + } + + .loader { + position: relative; + height: 20px; + width: 20px; + display: inline-block; + animation: around 5.4s infinite; + + &::after, + &::before { + content: ""; + background: $form-input-background; + position: absolute; + display: inline-block; + width: 100%; + height: 100%; + border-width: 2px; + border-color: #333 #333 transparent transparent; + border-style: solid; + border-radius: 20px; + box-sizing: border-box; + top: 0; + left: 0; + animation: around 0.7s ease-in-out infinite; + } + + &::after { + animation: around 0.7s ease-in-out 0.1s infinite; + background: transparent; + } + } +} + // LOGIN .login-form-container { .remember-me { diff --git a/doc/md/Shaarli-configuration.md b/doc/md/Shaarli-configuration.md index 263fb761..dbfc3da9 100644 --- a/doc/md/Shaarli-configuration.md +++ b/doc/md/Shaarli-configuration.md @@ -150,6 +150,7 @@ _These settings should not be edited_ - **timezone**: See [the list of supported timezones](http://php.net/manual/en/timezones.php). - **enabled_plugins**: List of enabled plugins. - **default_note_title**: Default title of a new note. +- **enable_async_metadata** (boolean): Retrieve external bookmark metadata asynchronously to prevent bookmark creation slowdown. - **retrieve_description** (boolean): If set to true, for every new Shaare Shaarli will try to retrieve the description and keywords from the HTML meta tags. - **root_url**: Overrides automatic discovery of Shaarli instance's URL (e.g.) `https://sub.domain.tld/shaarli-folder/`. diff --git a/index.php b/index.php index b10397dd..220847f5 100644 --- a/index.php +++ b/index.php @@ -129,7 +129,7 @@ $app->group('/admin', function () { $this->post('/plugins', '\Shaarli\Front\Controller\Admin\PluginsController:save'); $this->get('/token', '\Shaarli\Front\Controller\Admin\TokenController:getToken'); $this->get('/thumbnails', '\Shaarli\Front\Controller\Admin\ThumbnailsController:index'); - + $this->get('/metadata', '\Shaarli\Front\Controller\Admin\MetadataController:ajaxRetrieveTitle'); $this->get('/visibility/{visibility}', '\Shaarli\Front\Controller\Admin\SessionFilterController:visibility'); })->add('\Shaarli\Front\ShaarliAdminMiddleware'); diff --git a/package.json b/package.json index 8a24512a..b879b223 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "awesomplete": "^1.1.2", "blazy": "^1.8.2", "fork-awesome": "^1.1.7", + "he": "^1.2.0", "pure-extras": "^1.0.0", "purecss": "^1.0.0" }, 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..2a1838e8 --- /dev/null +++ b/tests/http/MetadataRetrieverTest.php @@ -0,0 +1,123 @@ +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'; + + $expectedResult = [ + 'title' => $remoteTitle, + 'description' => $remoteDesc, + 'tags' => $remoteTags, + ]; + + $this->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->httpAccess + ->expects(static::once()) + ->method('getHttpResponse') + ->with($url, 30, 4194304) + ->willReturnCallback(function($url, $timeout, $maxBytes, $callback): void { + $callback(); + }) + ; + + $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 (&$charset, &$title, &$description, &$tags): callable { + return function () use (&$charset, &$title, &$description, &$tags): void {}; + } + ) + ; + $this->httpAccess + ->expects(static::once()) + ->method('getHttpResponse') + ->with($url, 30, 4194304) + ->willReturnCallback(function($url, $timeout, $maxBytes, $callback): void { + $callback(); + }) + ; + + $result = $this->retriever->retrieve($url); + + static::assertSame($expectedResult, $result); + } +} diff --git a/tpl/default/editlink.html b/tpl/default/editlink.html index 568545bd..7ab7e1fe 100644 --- a/tpl/default/editlink.html +++ b/tpl/default/editlink.html @@ -12,6 +12,8 @@ action="{$base_path}/admin/shaare" class="page-form pure-u-lg-3-5 pure-u-22-24 page-form page-form-light" > + {$asyncLoadClass=$link_is_new && $async_metadata && empty($link.title) ? 'loading-input' : ''} +

{if="!$link_is_new"}{'Edit Shaare'|t}{else}{'New Shaare'|t}{/if}

@@ -28,21 +30,32 @@
-
- +
+ +
+ +
-
+
+
+ +
-
+
+
+ +
@@ -88,5 +101,6 @@
{include="page.footer"} + {if="$link_is_new && $async_metadata"}{/if} diff --git a/webpack.config.js b/webpack.config.js index a73758cc..8e3d1470 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -20,6 +20,7 @@ module.exports = [ entry: { thumbnails: './assets/common/js/thumbnails.js', thumbnails_update: './assets/common/js/thumbnails-update.js', + metadata: './assets/common/js/metadata.js', pluginsadmin: './assets/default/js/plugins-admin.js', shaarli: [ './assets/default/js/base.js', @@ -99,6 +100,7 @@ module.exports = [ ].concat(glob.sync('./assets/vintage/img/*')), markdown: './assets/common/css/markdown.css', thumbnails: './assets/common/js/thumbnails.js', + metadata: './assets/common/js/metadata.js', thumbnails_update: './assets/common/js/thumbnails-update.js', }, output: { diff --git a/yarn.lock b/yarn.lock index 0a12820c..55bd9827 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2912,6 +2912,11 @@ hash.js@^1.0.0, hash.js@^1.0.3: inherits "^2.0.3" minimalistic-assert "^1.0.1" +he@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/he/-/he-1.2.0.tgz#84ae65fa7eafb165fddb61566ae14baf05664f0f" + integrity sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw== + hmac-drbg@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/hmac-drbg/-/hmac-drbg-1.0.1.tgz#d2745701025a6c775a6c545793ed502fc0c649a1" -- cgit v1.2.3 From 5334090be04e66da5cb5c3ad487604b3733c5cac Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 15 Oct 2020 11:20:33 +0200 Subject: Improve metadata retrieval (performances and accuracy) - Use dedicated function to download headers to avoid apply multiple regexps on headers - Also try to extract title from meta tags --- application/http/HttpAccess.php | 22 ++-- application/http/HttpUtils.php | 123 +++++++++++------- application/http/MetadataRetriever.php | 1 + tests/bookmark/LinkUtilsTest.php | 223 ++++++++++++++++----------------- tests/http/MetadataRetrieverTest.php | 45 +++++-- 5 files changed, 237 insertions(+), 177 deletions(-) diff --git a/application/http/HttpAccess.php b/application/http/HttpAccess.php index 81d9e076..646a5264 100644 --- a/application/http/HttpAccess.php +++ b/application/http/HttpAccess.php @@ -14,9 +14,14 @@ namespace Shaarli\Http; */ class HttpAccess { - public function getHttpResponse($url, $timeout = 30, $maxBytes = 4194304, $curlWriteFunction = null) - { - return get_http_response($url, $timeout, $maxBytes, $curlWriteFunction); + public function getHttpResponse( + $url, + $timeout = 30, + $maxBytes = 4194304, + $curlHeaderFunction = null, + $curlWriteFunction = null + ) { + return get_http_response($url, $timeout, $maxBytes, $curlHeaderFunction, $curlWriteFunction); } public function getCurlDownloadCallback( @@ -24,16 +29,19 @@ class HttpAccess &$title, &$description, &$keywords, - $retrieveDescription, - $curlGetInfo = 'curl_getinfo' + $retrieveDescription ) { return get_curl_download_callback( $charset, $title, $description, $keywords, - $retrieveDescription, - $curlGetInfo + $retrieveDescription ); } + + public function getCurlHeaderCallback(&$charset, $curlGetInfo = 'curl_getinfo') + { + return get_curl_header_callback($charset, $curlGetInfo); + } } diff --git a/application/http/HttpUtils.php b/application/http/HttpUtils.php index 9f414073..28c12969 100644 --- a/application/http/HttpUtils.php +++ b/application/http/HttpUtils.php @@ -6,12 +6,14 @@ use Shaarli\Http\Url; * GET an HTTP URL to retrieve its content * Uses the cURL library or a fallback method * - * @param string $url URL to get (http://...) - * @param int $timeout network timeout (in seconds) - * @param int $maxBytes maximum downloaded bytes (default: 4 MiB) - * @param callable|string $curlWriteFunction Optional callback called during the download (cURL CURLOPT_WRITEFUNCTION). - * Can be used to add download conditions on the - * headers (response code, content type, etc.). + * @param string $url URL to get (http://...) + * @param int $timeout network timeout (in seconds) + * @param int $maxBytes maximum downloaded bytes (default: 4 MiB) + * @param callable|string $curlHeaderFunction Optional callback called during the download of headers + * (CURLOPT_HEADERFUNCTION) + * @param callable|string $curlWriteFunction Optional callback called during the download (cURL CURLOPT_WRITEFUNCTION). + * Can be used to add download conditions on the + * headers (response code, content type, etc.). * * @return array HTTP response headers, downloaded content * @@ -35,8 +37,13 @@ use Shaarli\Http\Url; * @see http://stackoverflow.com/q/9183178 * @see http://stackoverflow.com/q/1462720 */ -function get_http_response($url, $timeout = 30, $maxBytes = 4194304, $curlWriteFunction = null) -{ +function get_http_response( + $url, + $timeout = 30, + $maxBytes = 4194304, + $curlHeaderFunction = null, + $curlWriteFunction = null +) { $urlObj = new Url($url); $cleanUrl = $urlObj->idnToAscii(); @@ -70,7 +77,8 @@ function get_http_response($url, $timeout = 30, $maxBytes = 4194304, $curlWriteF // General cURL settings curl_setopt($ch, CURLOPT_AUTOREFERER, true); curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); - curl_setopt($ch, CURLOPT_HEADER, true); + // Default header download if the $curlHeaderFunction is not defined + curl_setopt($ch, CURLOPT_HEADER, !is_callable($curlHeaderFunction)); curl_setopt( $ch, CURLOPT_HTTPHEADER, @@ -81,25 +89,21 @@ function get_http_response($url, $timeout = 30, $maxBytes = 4194304, $curlWriteF curl_setopt($ch, CURLOPT_TIMEOUT, $timeout); curl_setopt($ch, CURLOPT_USERAGENT, $userAgent); - if (is_callable($curlWriteFunction)) { - curl_setopt($ch, CURLOPT_WRITEFUNCTION, $curlWriteFunction); - } - // Max download size management curl_setopt($ch, CURLOPT_BUFFERSIZE, 1024*16); curl_setopt($ch, CURLOPT_NOPROGRESS, false); + if (is_callable($curlHeaderFunction)) { + curl_setopt($ch, CURLOPT_HEADERFUNCTION, $curlHeaderFunction); + } + if (is_callable($curlWriteFunction)) { + curl_setopt($ch, CURLOPT_WRITEFUNCTION, $curlWriteFunction); + } curl_setopt( $ch, CURLOPT_PROGRESSFUNCTION, - function ($arg0, $arg1, $arg2, $arg3, $arg4 = 0) use ($maxBytes) { - if (version_compare(phpversion(), '5.5', '<')) { - // PHP version lower than 5.5 - // Callback has 4 arguments - $downloaded = $arg1; - } else { - // Callback has 5 arguments - $downloaded = $arg2; - } + function ($arg0, $arg1, $arg2, $arg3, $arg4) use ($maxBytes) { + $downloaded = $arg2; + // Non-zero return stops downloading return ($downloaded > $maxBytes) ? 1 : 0; } @@ -489,6 +493,46 @@ function is_https($server) return ! empty($server['HTTPS']); } +/** + * Get cURL callback function for CURLOPT_WRITEFUNCTION + * + * @param string $charset to extract from the downloaded page (reference) + * @param string $curlGetInfo Optionally overrides curl_getinfo function + * + * @return Closure + */ +function get_curl_header_callback( + &$charset, + $curlGetInfo = 'curl_getinfo' +) { + $isRedirected = false; + + return function ($ch, $data) use ($curlGetInfo, &$charset, &$isRedirected) { + $responseCode = $curlGetInfo($ch, CURLINFO_RESPONSE_CODE); + $chunkLength = strlen($data); + if (!empty($responseCode) && in_array($responseCode, [301, 302])) { + $isRedirected = true; + return $chunkLength; + } + if (!empty($responseCode) && $responseCode !== 200) { + return false; + } + // After a redirection, the content type will keep the previous request value + // until it finds the next content-type header. + if (! $isRedirected || strpos(strtolower($data), 'content-type') !== false) { + $contentType = $curlGetInfo($ch, CURLINFO_CONTENT_TYPE); + } + if (!empty($contentType) && strpos($contentType, 'text/html') === false) { + return false; + } + if (!empty($contentType) && empty($charset)) { + $charset = header_extract_charset($contentType); + } + + return $chunkLength; + }; +} + /** * Get cURL callback function for CURLOPT_WRITEFUNCTION * @@ -506,10 +550,8 @@ function get_curl_download_callback( &$title, &$description, &$keywords, - $retrieveDescription, - $curlGetInfo = 'curl_getinfo' + $retrieveDescription ) { - $isRedirected = false; $currentChunk = 0; $foundChunk = null; @@ -524,37 +566,18 @@ function get_curl_download_callback( * * @return int|bool length of $data or false if we need to stop the download */ - return function (&$ch, $data) use ( + return function ($ch, $data) use ( $retrieveDescription, - $curlGetInfo, &$charset, &$title, &$description, &$keywords, - &$isRedirected, &$currentChunk, &$foundChunk ) { + $chunkLength = strlen($data); $currentChunk++; - $responseCode = $curlGetInfo($ch, CURLINFO_RESPONSE_CODE); - if (!empty($responseCode) && in_array($responseCode, [301, 302])) { - $isRedirected = true; - return strlen($data); - } - if (!empty($responseCode) && $responseCode !== 200) { - return false; - } - // After a redirection, the content type will keep the previous request value - // until it finds the next content-type header. - if (! $isRedirected || strpos(strtolower($data), 'content-type') !== false) { - $contentType = $curlGetInfo($ch, CURLINFO_CONTENT_TYPE); - } - if (!empty($contentType) && strpos($contentType, 'text/html') === false) { - return false; - } - if (!empty($contentType) && empty($charset)) { - $charset = header_extract_charset($contentType); - } + if (empty($charset)) { $charset = html_extract_charset($data); } @@ -562,6 +585,10 @@ function get_curl_download_callback( $title = html_extract_title($data); $foundChunk = ! empty($title) ? $currentChunk : $foundChunk; } + if (empty($title)) { + $title = html_extract_tag('title', $data); + $foundChunk = ! empty($title) ? $currentChunk : $foundChunk; + } if ($retrieveDescription && empty($description)) { $description = html_extract_tag('description', $data); $foundChunk = ! empty($description) ? $currentChunk : $foundChunk; @@ -591,6 +618,6 @@ function get_curl_download_callback( return false; } - return strlen($data); + return $chunkLength; }; } diff --git a/application/http/MetadataRetriever.php b/application/http/MetadataRetriever.php index 2ca982e2..ba9bd40c 100644 --- a/application/http/MetadataRetriever.php +++ b/application/http/MetadataRetriever.php @@ -46,6 +46,7 @@ class MetadataRetriever $url, $this->conf->get('general.download_timeout', 30), $this->conf->get('general.download_max_size', 4194304), + $this->httpAccess->getCurlHeaderCallback($charset), $this->httpAccess->getCurlDownloadCallback( $charset, $title, 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 @@ -215,61 +215,92 @@ class LinkUtilsTest extends TestCase $this->assertFalse(html_extract_tag('description', $html)); } + /** + * 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">' . 'Refactoring · GitHub' . '' . '', ]; - 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">' . 'Refactoring · GitHub' . '' . '', ]; - 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', '', 'end' => 'th=device-width">' . 'Refactoring · GitHub' @@ -310,14 +342,10 @@ class LinkUtilsTest extends TestCase . '' . '', ]; - 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' . 'Refactoring · GitHub' . '' . '', ]; - 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">' . 'Refactoring · GitHub' . ' - thumbnail
diff --git a/tpl/default/includes.html b/tpl/default/includes.html index 09768ac4..3e3fb664 100644 --- a/tpl/default/includes.html +++ b/tpl/default/includes.html @@ -12,10 +12,10 @@ {/if} {loop="$plugins_includes.css_files"} - + {/loop} {if="is_file('data/user.css')"} - + {/if} diff --git a/tpl/default/linklist.html b/tpl/default/linklist.html index b08773d8..e1fb54dd 100644 --- a/tpl/default/linklist.html +++ b/tpl/default/linklist.html @@ -140,7 +140,7 @@
{ignore}RainTPL hack: put the 2 src on two different line to avoid path replace bug{/ignore} diff --git a/tpl/default/page.footer.html b/tpl/default/page.footer.html index 51bdb2f0..ea84aab9 100644 --- a/tpl/default/page.footer.html +++ b/tpl/default/page.footer.html @@ -10,7 +10,7 @@ {/if} · {'The personal, minimalist, super-fast, database free, bookmarking service'|t} {'by the Shaarli community'|t} · - {'Documentation'|t} + {'Documentation'|t} {loop="$plugins_footer.text"} {$value} {/loop} @@ -25,7 +25,7 @@ {/loop} {loop="$plugins_footer.js_files"} - + {/loop}