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/bookmark/Bookmark.php | 121 +++++++++++++++++++------------------- 1 file changed, 61 insertions(+), 60 deletions(-) (limited to 'application/bookmark/Bookmark.php') 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]); -- cgit v1.2.3 From 4e3875c0ce7f3b17e3d358dc5ecb1f8bed64546b Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Mon, 12 Oct 2020 11:35:55 +0200 Subject: Feature: highlight fulltext search results How it works: 1. when a fulltext search is made, Shaarli looks for the first occurence position of every term matching the search. No change here, but we store these positions in an array, in Bookmark's additionalContent. 2. when formatting bookmarks (through BookmarkFormatter implementation): 1. first we insert specific tokens at every search result positions 2. we format the content (escape HTML, apply markdown, etc.) 3. as a last step, we replace our token with displayable span elements Cons: this tightens coupling between search filters and formatters Pros: it was absolutely necessary not to perform the search twice. this solution has close to no impact on performances. Fixes #205 --- application/bookmark/Bookmark.php | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) (limited to 'application/bookmark/Bookmark.php') diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index fa45d2fc..ea565d1f 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -54,6 +54,9 @@ class Bookmark /** @var bool True if the bookmark can only be seen while logged in */ protected $private; + /** @var mixed[] Available to store any additional content for a bookmark. Currently used for search highlight. */ + protected $additionalContent = []; + /** * Initialize a link from array data. Especially useful to create a Bookmark from former link storage format. * @@ -95,6 +98,8 @@ class Bookmark * - the URL with the permalink * - the title with the URL * + * Also make sure that we do not save search highlights in the datastore. + * * @throws InvalidBookmarkException */ public function validate(): void @@ -112,6 +117,9 @@ class Bookmark if (empty($this->title)) { $this->title = $this->url; } + if (array_key_exists('search_highlight', $this->additionalContent)) { + unset($this->additionalContent['search_highlight']); + } } /** @@ -435,6 +443,44 @@ class Bookmark return $this; } + /** + * Get entire additionalContent array. + * + * @return mixed[] + */ + public function getAdditionalContent(): array + { + return $this->additionalContent; + } + + /** + * Set a single entry in additionalContent, by key. + * + * @param string $key + * @param mixed|null $value Any type of value can be set. + * + * @return $this + */ + public function addAdditionalContentEntry(string $key, $value): self + { + $this->additionalContent[$key] = $value; + + return $this; + } + + /** + * Get a single entry in additionalContent, by key. + * + * @param string $key + * @param mixed|null $default + * + * @return mixed|null can be any type or even null. + */ + public function getAdditionalContentEntry(string $key, $default = null) + { + return array_key_exists($key, $this->additionalContent) ? $this->additionalContent[$key] : $default; + } + /** * Rename a tag in tags list. * -- cgit v1.2.3 From 21e72da9ee34cec56b10c83ae0c75b4bf320dfcb Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 15 Oct 2020 11:46:24 +0200 Subject: Asynchronous retrieval of bookmark's thumbnails This feature is based general.enable_async_metadata setting and works with existing metadata.js file. The script is compatible with any template: - the thumbnail div bloc must have attribute - the bookmark bloc must have attribute with the bookmark ID as value Fixes #1564 --- application/bookmark/Bookmark.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'application/bookmark/Bookmark.php') diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index ea565d1f..4810c5e6 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -377,6 +377,24 @@ class Bookmark return $this; } + /** + * Return true if: + * - the bookmark's thumbnail is not already set to false (= not found) + * - it's not a note + * - it's an HTTP(S) link + * - the thumbnail has not yet be retrieved (null) or its associated cache file doesn't exist anymore + * + * @return bool True if the bookmark's thumbnail needs to be retrieved. + */ + public function shouldUpdateThumbnail(): bool + { + return $this->thumbnail !== false + && !$this->isNote() + && startsWith(strtolower($this->url), 'http') + && (null === $this->thumbnail || !is_file($this->thumbnail)) + ; + } + /** * Get the Sticky. * -- cgit v1.2.3 From b3bd8c3e8d367975980043e772f7cd78b7f96bc6 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 22 Oct 2020 16:21:03 +0200 Subject: Feature: support any tag separator So it allows to have multiple words tags. Breaking change: commas ',' are no longer a default separator. Fixes #594 --- application/bookmark/Bookmark.php | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) (limited to 'application/bookmark/Bookmark.php') diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index 4810c5e6..8aaeb9d8 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -60,11 +60,13 @@ class Bookmark /** * Initialize a link from array data. Especially useful to create a Bookmark from former link storage format. * - * @param array $data + * @param array $data + * @param string $tagsSeparator Tags separator loaded from the config file. + * This is a context data, and it should *never* be stored in the Bookmark object. * * @return $this */ - public function fromArray(array $data): Bookmark + public function fromArray(array $data, string $tagsSeparator = ' '): Bookmark { $this->id = $data['id'] ?? null; $this->shortUrl = $data['shorturl'] ?? null; @@ -77,7 +79,7 @@ class Bookmark if (is_array($data['tags'])) { $this->tags = $data['tags']; } else { - $this->tags = preg_split('/\s+/', $data['tags'] ?? '', -1, PREG_SPLIT_NO_EMPTY); + $this->tags = tags_str2array($data['tags'] ?? '', $tagsSeparator); } if (! empty($data['updated'])) { $this->updated = $data['updated']; @@ -348,7 +350,12 @@ class Bookmark */ public function setTags(?array $tags): Bookmark { - $this->setTagsString(implode(' ', $tags ?? [])); + $this->tags = array_map( + function (string $tag): string { + return $tag[0] === '-' ? substr($tag, 1) : $tag; + }, + tags_filter($tags, ' ') + ); return $this; } @@ -420,11 +427,13 @@ class Bookmark } /** - * @return string Bookmark's tags as a string, separated by a space + * @param string $separator Tags separator loaded from the config file. + * + * @return string Bookmark's tags as a string, separated by a separator */ - public function getTagsString(): string + public function getTagsString(string $separator = ' '): string { - return implode(' ', $this->getTags()); + return tags_array2str($this->getTags(), $separator); } /** @@ -444,19 +453,13 @@ class Bookmark * - trailing dash in tags will be removed * * @param string|null $tags + * @param string $separator Tags separator loaded from the config file. * * @return $this */ - public function setTagsString(?string $tags): Bookmark + public function setTagsString(?string $tags, string $separator = ' '): Bookmark { - // Remove first '-' char in tags. - $tags = preg_replace('/(^| )\-/', '$1', $tags ?? ''); - // Explode all tags separted by spaces or commas - $tags = preg_split('/[\s,]+/', $tags); - // Remove eventual empty values - $tags = array_values(array_filter($tags)); - - $this->tags = $tags; + $this->setTags(tags_str2array($tags, $separator)); return $this; } @@ -507,7 +510,7 @@ class Bookmark */ public function renameTag(string $fromTag, string $toTag): void { - if (($pos = array_search($fromTag, $this->tags)) !== false) { + if (($pos = array_search($fromTag, $this->tags ?? [])) !== false) { $this->tags[$pos] = trim($toTag); } } @@ -519,7 +522,7 @@ class Bookmark */ public function deleteTag(string $tag): void { - if (($pos = array_search($tag, $this->tags)) !== false) { + if (($pos = array_search($tag, $this->tags ?? [])) !== false) { unset($this->tags[$pos]); $this->tags = array_values($this->tags); } -- cgit v1.2.3 From 53054b2bf6a919fd4ff9b44b6ad1986f21f488b6 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 22 Sep 2020 20:25:47 +0200 Subject: Apply PHP Code Beautifier on source code for linter automatic fixes --- application/bookmark/Bookmark.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'application/bookmark/Bookmark.php') diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index 8aaeb9d8..b592722f 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -106,7 +106,8 @@ class Bookmark */ public function validate(): void { - if ($this->id === null + if ( + $this->id === null || ! is_int($this->id) || empty($this->shortUrl) || empty($this->created) @@ -114,7 +115,7 @@ class Bookmark throw new InvalidBookmarkException($this); } if (empty($this->url)) { - $this->url = '/shaare/'. $this->shortUrl; + $this->url = '/shaare/' . $this->shortUrl; } if (empty($this->title)) { $this->title = $this->url; -- cgit v1.2.3 From b99e00f7cd5f7e2090f44cd97bfb426db55340c2 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sun, 8 Nov 2020 15:02:45 +0100 Subject: Manually fix remaining PHPCS errors --- application/bookmark/Bookmark.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'application/bookmark/Bookmark.php') diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index b592722f..4238ef25 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -19,7 +19,7 @@ use Shaarli\Bookmark\Exception\InvalidBookmarkException; class Bookmark { /** @var string Date format used in string (former ID format) */ - const LINK_DATE_FORMAT = 'Ymd_His'; + public const LINK_DATE_FORMAT = 'Ymd_His'; /** @var int Bookmark ID */ protected $id; -- cgit v1.2.3