diff options
author | ArthurHoaro <arthur@hoa.ro> | 2020-10-02 17:50:59 +0200 |
---|---|---|
committer | ArthurHoaro <arthur@hoa.ro> | 2020-10-13 13:50:11 +0200 |
commit | efb7d21b52eb033530e80e5e49d175e6e3b031f4 (patch) | |
tree | 4f34052788a08be1a30cb88c3339ae14e0b7c4da /application/bookmark/BookmarkFileService.php | |
parent | 29c31b7ec6ca48ba37b7eb6da650931fd0cb7164 (diff) | |
download | Shaarli-efb7d21b52eb033530e80e5e49d175e6e3b031f4.tar.gz Shaarli-efb7d21b52eb033530e80e5e49d175e6e3b031f4.tar.zst Shaarli-efb7d21b52eb033530e80e5e49d175e6e3b031f4.zip |
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.
Diffstat (limited to 'application/bookmark/BookmarkFileService.php')
-rw-r--r-- | application/bookmark/BookmarkFileService.php | 66 |
1 files changed, 28 insertions, 38 deletions
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 @@ | |||
1 | <?php | 1 | <?php |
2 | 2 | ||
3 | declare(strict_types=1); | ||
3 | 4 | ||
4 | namespace Shaarli\Bookmark; | 5 | namespace Shaarli\Bookmark; |
5 | 6 | ||
6 | 7 | use DateTime; | |
7 | use Exception; | 8 | use Exception; |
8 | use malkusch\lock\mutex\Mutex; | 9 | use malkusch\lock\mutex\Mutex; |
9 | use Shaarli\Bookmark\Exception\BookmarkNotFoundException; | 10 | use Shaarli\Bookmark\Exception\BookmarkNotFoundException; |
@@ -54,7 +55,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
54 | /** | 55 | /** |
55 | * @inheritDoc | 56 | * @inheritDoc |
56 | */ | 57 | */ |
57 | public function __construct(ConfigManager $conf, History $history, Mutex $mutex, $isLoggedIn) | 58 | public function __construct(ConfigManager $conf, History $history, Mutex $mutex, bool $isLoggedIn) |
58 | { | 59 | { |
59 | $this->conf = $conf; | 60 | $this->conf = $conf; |
60 | $this->history = $history; | 61 | $this->history = $history; |
@@ -96,7 +97,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
96 | /** | 97 | /** |
97 | * @inheritDoc | 98 | * @inheritDoc |
98 | */ | 99 | */ |
99 | public function findByHash($hash) | 100 | public function findByHash(string $hash): Bookmark |
100 | { | 101 | { |
101 | $bookmark = $this->bookmarkFilter->filter(BookmarkFilter::$FILTER_HASH, $hash); | 102 | $bookmark = $this->bookmarkFilter->filter(BookmarkFilter::$FILTER_HASH, $hash); |
102 | // PHP 7.3 introduced array_key_first() to avoid this hack | 103 | // PHP 7.3 introduced array_key_first() to avoid this hack |
@@ -111,7 +112,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
111 | /** | 112 | /** |
112 | * @inheritDoc | 113 | * @inheritDoc |
113 | */ | 114 | */ |
114 | public function findByUrl($url) | 115 | public function findByUrl(string $url): ?Bookmark |
115 | { | 116 | { |
116 | return $this->bookmarks->getByUrl($url); | 117 | return $this->bookmarks->getByUrl($url); |
117 | } | 118 | } |
@@ -120,10 +121,10 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
120 | * @inheritDoc | 121 | * @inheritDoc |
121 | */ | 122 | */ |
122 | public function search( | 123 | public function search( |
123 | $request = [], | 124 | array $request = [], |
124 | $visibility = null, | 125 | string $visibility = null, |
125 | $caseSensitive = false, | 126 | bool $caseSensitive = false, |
126 | $untaggedOnly = false, | 127 | bool $untaggedOnly = false, |
127 | bool $ignoreSticky = false | 128 | bool $ignoreSticky = false |
128 | ) { | 129 | ) { |
129 | if ($visibility === null) { | 130 | if ($visibility === null) { |
@@ -131,8 +132,8 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
131 | } | 132 | } |
132 | 133 | ||
133 | // Filter bookmark database according to parameters. | 134 | // Filter bookmark database according to parameters. |
134 | $searchtags = isset($request['searchtags']) ? $request['searchtags'] : ''; | 135 | $searchTags = isset($request['searchtags']) ? $request['searchtags'] : ''; |
135 | $searchterm = isset($request['searchterm']) ? $request['searchterm'] : ''; | 136 | $searchTerm = isset($request['searchterm']) ? $request['searchterm'] : ''; |
136 | 137 | ||
137 | if ($ignoreSticky) { | 138 | if ($ignoreSticky) { |
138 | $this->bookmarks->reorder('DESC', true); | 139 | $this->bookmarks->reorder('DESC', true); |
@@ -140,7 +141,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
140 | 141 | ||
141 | return $this->bookmarkFilter->filter( | 142 | return $this->bookmarkFilter->filter( |
142 | BookmarkFilter::$FILTER_TAG | BookmarkFilter::$FILTER_TEXT, | 143 | BookmarkFilter::$FILTER_TAG | BookmarkFilter::$FILTER_TEXT, |
143 | [$searchtags, $searchterm], | 144 | [$searchTags, $searchTerm], |
144 | $caseSensitive, | 145 | $caseSensitive, |
145 | $visibility, | 146 | $visibility, |
146 | $untaggedOnly | 147 | $untaggedOnly |
@@ -150,7 +151,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
150 | /** | 151 | /** |
151 | * @inheritDoc | 152 | * @inheritDoc |
152 | */ | 153 | */ |
153 | public function get($id, $visibility = null) | 154 | public function get(int $id, string $visibility = null): Bookmark |
154 | { | 155 | { |
155 | if (! isset($this->bookmarks[$id])) { | 156 | if (! isset($this->bookmarks[$id])) { |
156 | throw new BookmarkNotFoundException(); | 157 | throw new BookmarkNotFoundException(); |
@@ -173,20 +174,17 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
173 | /** | 174 | /** |
174 | * @inheritDoc | 175 | * @inheritDoc |
175 | */ | 176 | */ |
176 | public function set($bookmark, $save = true) | 177 | public function set(Bookmark $bookmark, bool $save = true): Bookmark |
177 | { | 178 | { |
178 | if (true !== $this->isLoggedIn) { | 179 | if (true !== $this->isLoggedIn) { |
179 | throw new Exception(t('You\'re not authorized to alter the datastore')); | 180 | throw new Exception(t('You\'re not authorized to alter the datastore')); |
180 | } | 181 | } |
181 | if (! $bookmark instanceof Bookmark) { | ||
182 | throw new Exception(t('Provided data is invalid')); | ||
183 | } | ||
184 | if (! isset($this->bookmarks[$bookmark->getId()])) { | 182 | if (! isset($this->bookmarks[$bookmark->getId()])) { |
185 | throw new BookmarkNotFoundException(); | 183 | throw new BookmarkNotFoundException(); |
186 | } | 184 | } |
187 | $bookmark->validate(); | 185 | $bookmark->validate(); |
188 | 186 | ||
189 | $bookmark->setUpdated(new \DateTime()); | 187 | $bookmark->setUpdated(new DateTime()); |
190 | $this->bookmarks[$bookmark->getId()] = $bookmark; | 188 | $this->bookmarks[$bookmark->getId()] = $bookmark; |
191 | if ($save === true) { | 189 | if ($save === true) { |
192 | $this->save(); | 190 | $this->save(); |
@@ -198,15 +196,12 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
198 | /** | 196 | /** |
199 | * @inheritDoc | 197 | * @inheritDoc |
200 | */ | 198 | */ |
201 | public function add($bookmark, $save = true) | 199 | public function add(Bookmark $bookmark, bool $save = true): Bookmark |
202 | { | 200 | { |
203 | if (true !== $this->isLoggedIn) { | 201 | if (true !== $this->isLoggedIn) { |
204 | throw new Exception(t('You\'re not authorized to alter the datastore')); | 202 | throw new Exception(t('You\'re not authorized to alter the datastore')); |
205 | } | 203 | } |
206 | if (! $bookmark instanceof Bookmark) { | 204 | if (!empty($bookmark->getId())) { |
207 | throw new Exception(t('Provided data is invalid')); | ||
208 | } | ||
209 | if (! empty($bookmark->getId())) { | ||
210 | throw new Exception(t('This bookmarks already exists')); | 205 | throw new Exception(t('This bookmarks already exists')); |
211 | } | 206 | } |
212 | $bookmark->setId($this->bookmarks->getNextId()); | 207 | $bookmark->setId($this->bookmarks->getNextId()); |
@@ -223,14 +218,11 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
223 | /** | 218 | /** |
224 | * @inheritDoc | 219 | * @inheritDoc |
225 | */ | 220 | */ |
226 | public function addOrSet($bookmark, $save = true) | 221 | public function addOrSet(Bookmark $bookmark, bool $save = true): Bookmark |
227 | { | 222 | { |
228 | if (true !== $this->isLoggedIn) { | 223 | if (true !== $this->isLoggedIn) { |
229 | throw new Exception(t('You\'re not authorized to alter the datastore')); | 224 | throw new Exception(t('You\'re not authorized to alter the datastore')); |
230 | } | 225 | } |
231 | if (! $bookmark instanceof Bookmark) { | ||
232 | throw new Exception('Provided data is invalid'); | ||
233 | } | ||
234 | if ($bookmark->getId() === null) { | 226 | if ($bookmark->getId() === null) { |
235 | return $this->add($bookmark, $save); | 227 | return $this->add($bookmark, $save); |
236 | } | 228 | } |
@@ -240,14 +232,11 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
240 | /** | 232 | /** |
241 | * @inheritDoc | 233 | * @inheritDoc |
242 | */ | 234 | */ |
243 | public function remove($bookmark, $save = true) | 235 | public function remove(Bookmark $bookmark, bool $save = true): void |
244 | { | 236 | { |
245 | if (true !== $this->isLoggedIn) { | 237 | if (true !== $this->isLoggedIn) { |
246 | throw new Exception(t('You\'re not authorized to alter the datastore')); | 238 | throw new Exception(t('You\'re not authorized to alter the datastore')); |
247 | } | 239 | } |
248 | if (! $bookmark instanceof Bookmark) { | ||
249 | throw new Exception(t('Provided data is invalid')); | ||
250 | } | ||
251 | if (! isset($this->bookmarks[$bookmark->getId()])) { | 240 | if (! isset($this->bookmarks[$bookmark->getId()])) { |
252 | throw new BookmarkNotFoundException(); | 241 | throw new BookmarkNotFoundException(); |
253 | } | 242 | } |
@@ -262,7 +251,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
262 | /** | 251 | /** |
263 | * @inheritDoc | 252 | * @inheritDoc |
264 | */ | 253 | */ |
265 | public function exists($id, $visibility = null) | 254 | public function exists(int $id, string $visibility = null): bool |
266 | { | 255 | { |
267 | if (! isset($this->bookmarks[$id])) { | 256 | if (! isset($this->bookmarks[$id])) { |
268 | return false; | 257 | return false; |
@@ -285,7 +274,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
285 | /** | 274 | /** |
286 | * @inheritDoc | 275 | * @inheritDoc |
287 | */ | 276 | */ |
288 | public function count($visibility = null) | 277 | public function count(string $visibility = null): int |
289 | { | 278 | { |
290 | return count($this->search([], $visibility)); | 279 | return count($this->search([], $visibility)); |
291 | } | 280 | } |
@@ -293,7 +282,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
293 | /** | 282 | /** |
294 | * @inheritDoc | 283 | * @inheritDoc |
295 | */ | 284 | */ |
296 | public function save() | 285 | public function save(): void |
297 | { | 286 | { |
298 | if (true !== $this->isLoggedIn) { | 287 | if (true !== $this->isLoggedIn) { |
299 | // TODO: raise an Exception instead | 288 | // TODO: raise an Exception instead |
@@ -308,7 +297,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
308 | /** | 297 | /** |
309 | * @inheritDoc | 298 | * @inheritDoc |
310 | */ | 299 | */ |
311 | public function bookmarksCountPerTag($filteringTags = [], $visibility = null) | 300 | public function bookmarksCountPerTag(array $filteringTags = [], string $visibility = null): array |
312 | { | 301 | { |
313 | $bookmarks = $this->search(['searchtags' => $filteringTags], $visibility); | 302 | $bookmarks = $this->search(['searchtags' => $filteringTags], $visibility); |
314 | $tags = []; | 303 | $tags = []; |
@@ -344,13 +333,14 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
344 | $keys = array_keys($tags); | 333 | $keys = array_keys($tags); |
345 | $tmpTags = array_combine($keys, $keys); | 334 | $tmpTags = array_combine($keys, $keys); |
346 | array_multisort($tags, SORT_DESC, $tmpTags, SORT_ASC, $tags); | 335 | array_multisort($tags, SORT_DESC, $tmpTags, SORT_ASC, $tags); |
336 | |||
347 | return $tags; | 337 | return $tags; |
348 | } | 338 | } |
349 | 339 | ||
350 | /** | 340 | /** |
351 | * @inheritDoc | 341 | * @inheritDoc |
352 | */ | 342 | */ |
353 | public function days() | 343 | public function days(): array |
354 | { | 344 | { |
355 | $bookmarkDays = []; | 345 | $bookmarkDays = []; |
356 | foreach ($this->search() as $bookmark) { | 346 | foreach ($this->search() as $bookmark) { |
@@ -365,7 +355,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
365 | /** | 355 | /** |
366 | * @inheritDoc | 356 | * @inheritDoc |
367 | */ | 357 | */ |
368 | public function filterDay($request) | 358 | public function filterDay(string $request) |
369 | { | 359 | { |
370 | $visibility = $this->isLoggedIn ? BookmarkFilter::$ALL : BookmarkFilter::$PUBLIC; | 360 | $visibility = $this->isLoggedIn ? BookmarkFilter::$ALL : BookmarkFilter::$PUBLIC; |
371 | 361 | ||
@@ -375,7 +365,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
375 | /** | 365 | /** |
376 | * @inheritDoc | 366 | * @inheritDoc |
377 | */ | 367 | */ |
378 | public function initialize() | 368 | public function initialize(): void |
379 | { | 369 | { |
380 | $initializer = new BookmarkInitializer($this); | 370 | $initializer = new BookmarkInitializer($this); |
381 | $initializer->initialize(); | 371 | $initializer->initialize(); |
@@ -388,7 +378,7 @@ class BookmarkFileService implements BookmarkServiceInterface | |||
388 | /** | 378 | /** |
389 | * Handles migration to the new database format (BookmarksArray). | 379 | * Handles migration to the new database format (BookmarksArray). |
390 | */ | 380 | */ |
391 | protected function migrate() | 381 | protected function migrate(): void |
392 | { | 382 | { |
393 | $bookmarkDb = new LegacyLinkDB( | 383 | $bookmarkDb = new LegacyLinkDB( |
394 | $this->conf->get('resource.datastore'), | 384 | $this->conf->get('resource.datastore'), |