Skip to content

Commit cdb07f2

Browse files
marcelklehrmarcelklehr
authored
Merge pull request #2305 from nextcloud/fix/generic-url-previewer
fix(BookmarksPreviewer): Add generic previewer to previewers list
2 parents c2074f0 + d4e70f9 commit cdb07f2

10 files changed

Lines changed: 54 additions & 66 deletions

‎.github/workflows/psalm.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ jobs:
7575
7676
7777
- name: Install dependencies # zizmor: ignore[template-injection]
78-
run: composer require --dev 'nextcloud/ocp:${{ matrix.ocp-version }}' --ignore-platform-reqs --with-dependencies
78+
run: composer require --dev 'nextcloud/ocp:${{ matrix.ocp-version }}' --ignore-platform-reqs --with-all-dependencies
7979

8080
- name: Run coding standards check
8181
run: composer run psalm -- --threads=1 --monochrome --no-progress --output-format=github

‎lib/Controller/FoldersController.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,8 @@ public function getFolder($folderId): JSONResponse {
221221
* @throws UnauthenticatedError
222222
*/
223223
public function addToFolder($folderId, $bookmarkId): JSONResponse {
224-
if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request)) ||
225-
!Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForBookmark($bookmarkId, $this->request))) {
224+
if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request))
225+
|| !Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForBookmark($bookmarkId, $this->request))) {
226226
$res = new JSONResponse(['status' => 'error', 'data' => ['Not found']], Http::STATUS_NOT_FOUND);
227227
$res->throttle();
228228
return $res;
@@ -259,8 +259,8 @@ public function addToFolder($folderId, $bookmarkId): JSONResponse {
259259
* @throws UnauthenticatedError
260260
*/
261261
public function removeFromFolder($folderId, $bookmarkId, bool $hardDelete = false): JSONResponse {
262-
if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request)) ||
263-
!Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForBookmark($bookmarkId, $this->request))) {
262+
if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request))
263+
|| !Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForBookmark($bookmarkId, $this->request))) {
264264
$res = new JSONResponse(['status' => 'error', 'data' => ['Not found']], Http::STATUS_BAD_REQUEST);
265265
$res->throttle();
266266
return $res;
@@ -293,8 +293,8 @@ public function removeFromFolder($folderId, $bookmarkId, bool $hardDelete = fals
293293
* @throws UnauthenticatedError
294294
*/
295295
public function undeleteFromFolder(int $folderId, int $bookmarkId): JSONResponse {
296-
if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request)) ||
297-
!Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForBookmark($bookmarkId, $this->request))) {
296+
if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request))
297+
|| !Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForBookmark($bookmarkId, $this->request))) {
298298
$res = new JSONResponse(['status' => 'error', 'data' => ['Unauthorized']], Http::STATUS_FORBIDDEN);
299299
$res->throttle();
300300
return $res;

‎lib/Controller/WebViewController.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ public function manifest(): JSONResponse {
172172
'name' => $this->l->t('Bookmarks'),
173173
'short_name' => $this->l->t('Bookmarks'),
174174
'start_url' => $this->urlGenerator->linkToRouteAbsolute('bookmarks.web_view.index'),
175-
'icons' =>
176-
[
175+
'icons'
176+
=> [
177177
[
178178
'src' => $this->urlGenerator->linkToRoute('theming.Icon.getTouchIcon',
179179
['app' => 'bookmarks']),

‎lib/Db/BookmarkMapper.php‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,8 @@ public function findAll(string $userId, QueryParameters $queryParams, bool $with
220220

221221
$qb
222222
->from('*PREFIX*bookmarks', 'b')
223-
->innerJoin('b', 'folder_tree', 'tree', 'tree.item_id = b.id AND tree.type = ' . $qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK) .
224-
($queryParams->getSoftDeleted() ? ' AND tree.soft_deleted_at is NOT NULL' : ' AND tree.soft_deleted_at is NULL'));
223+
->innerJoin('b', 'folder_tree', 'tree', 'tree.item_id = b.id AND tree.type = ' . $qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK)
224+
. ($queryParams->getSoftDeleted() ? ' AND tree.soft_deleted_at is NOT NULL' : ' AND tree.soft_deleted_at is NULL'));
225225

226226
$this->_filterUrl($qb, $queryParams);
227227
$this->_filterArchived($qb, $queryParams);
@@ -298,9 +298,9 @@ private function _generateCTE(int $folderId, bool $withSoftDeleted) : array {
298298

299299
if ($this->getDbType() === 'mysql') {
300300
// For mysql we can just throw these three queries together in a CTE
301-
$withRecursiveQuery = 'WITH RECURSIVE folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS ( ' .
302-
$baseCase->getSQL() . ' UNION ALL ' . $recursiveCase->getSQL() .
303-
' UNION ALL ' . $recursiveCaseShares->getSQL() . ')';
301+
$withRecursiveQuery = 'WITH RECURSIVE folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS ( '
302+
. $baseCase->getSQL() . ' UNION ALL ' . $recursiveCase->getSQL()
303+
. ' UNION ALL ' . $recursiveCaseShares->getSQL() . ')';
304304
} else {
305305
// Postgres loves us dearly and doesn't allow two recursive references in one CTE, aaah.
306306
// So we nest them:
@@ -333,12 +333,12 @@ private function _generateCTE(int $folderId, bool $withSoftDeleted) : array {
333333
// then we need another instance of the first recursive case, duplicated here as secondRecursive case
334334
// to recurse into child folders of shared folders
335335
// Note: This doesn't cover cases where a shared folder is inside a shared folder.
336-
$withRecursiveQuery = 'WITH RECURSIVE folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS ( ' .
337-
'WITH RECURSIVE second_folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS (' .
338-
'WITH RECURSIVE inner_folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS ( ' .
339-
$baseCase->getSQL() . ' UNION ALL ' . $recursiveCase->getSQL() . ')' .
340-
' ' . $secondBaseCase->getSQL() . ' UNION ALL ' . $recursiveCaseShares->getSQL() . ')' .
341-
' ' . $thirdBaseCase->getSQL() . ' UNION ALL ' . $secondRecursiveCase->getSQL() . ')';
336+
$withRecursiveQuery = 'WITH RECURSIVE folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS ( '
337+
. 'WITH RECURSIVE second_folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS ('
338+
. 'WITH RECURSIVE inner_folder_tree(item_id, parent_folder, type, idx, soft_deleted_at) AS ( '
339+
. $baseCase->getSQL() . ' UNION ALL ' . $recursiveCase->getSQL() . ')'
340+
. ' ' . $secondBaseCase->getSQL() . ' UNION ALL ' . $recursiveCaseShares->getSQL() . ')'
341+
. ' ' . $thirdBaseCase->getSQL() . ' UNION ALL ' . $secondRecursiveCase->getSQL() . ')';
342342
}
343343

344344
// Now we need to concatenate the params of all these queries for downstream assembly of the greater query

‎lib/Migration/Version000014000Date20181029094721.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
9090

9191
$children = array_merge($childFolders, $childBookmarks);
9292
$children = array_map(static function ($child) {
93-
return $child['bookmark_id'] ?
94-
['type' => 'bookmark', 'id' => $child['bookmark_id']]
93+
return $child['bookmark_id']
94+
? ['type' => 'bookmark', 'id' => $child['bookmark_id']]
9595
: ['type' => 'folder', 'id' => $child['id']];
9696
}, $children);
9797
if (count($children) > 0) {

‎lib/Service/BookmarkPreviewer.php‎

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OCA\Bookmarks\Db\Bookmark;
1414
use OCA\Bookmarks\Image;
1515
use OCA\Bookmarks\Service\Previewers\DefaultBookmarkPreviewer;
16+
use OCA\Bookmarks\Service\Previewers\GenericUrlBookmarkPreviewer;
1617
use OCA\Bookmarks\Service\Previewers\PageresBookmarkPreviewer;
1718
use OCA\Bookmarks\Service\Previewers\ScreeenlyBookmarkPreviewer;
1819
use OCA\Bookmarks\Service\Previewers\ScreenshotMachineBookmarkPreviewer;
@@ -23,46 +24,28 @@
2324
class BookmarkPreviewer implements IBookmarkPreviewer {
2425
// Cache for one month
2526
public const CACHE_TTL = 4 * 4 * 7 * 24 * 60 * 60;
27+
private string $enabled;
28+
private FileCache $cache;
29+
private DefaultBookmarkPreviewer $defaultPreviewer;
30+
private ScreeenlyBookmarkPreviewer $screeenlyPreviewer;
31+
private ScreenshotMachineBookmarkPreviewer $screenshotMachinePreviewer;
32+
private PageresBookmarkPreviewer $pageresPreviewer;
33+
private Previewers\GenericUrlBookmarkPreviewer $genericUrlPreviewer;
2634

27-
/**
28-
* @var string
29-
*/
30-
private $enabled;
31-
/**
32-
* @var DefaultBookmarkPreviewer
33-
*/
34-
private $defaultPreviewer;
35-
/**
36-
* @var ScreeenlyBookmarkPreviewer
37-
*/
38-
private $screeenlyPreviewer;
39-
40-
/**
41-
* @var FileCache
42-
*/
43-
private $cache;
44-
/**
45-
* @var Previewers\ScreenshotMachineBookmarkPreviewer
46-
*/
47-
private $screenshotMachinePreviewer;
48-
/**
49-
* @var Previewers\PageresBookmarkPreviewer
50-
*/
51-
private $pageresPreviewer;
52-
53-
/**
54-
* @param IConfig $config
55-
* @param ScreeenlyBookmarkPreviewer $screeenlyPreviewer
56-
* @param DefaultBookmarkPreviewer $defaultPreviewer
57-
* @param FileCache $cache
58-
* @param Previewers\ScreenshotMachineBookmarkPreviewer $screenshotMachinePreviewer
59-
* @param Previewers\PageresBookmarkPreviewer $pageresPreviewer
60-
*/
61-
public function __construct(IConfig $config, ScreeenlyBookmarkPreviewer $screeenlyPreviewer, DefaultBookmarkPreviewer $defaultPreviewer, FileCache $cache, ScreenshotMachineBookmarkPreviewer $screenshotMachinePreviewer, PageresBookmarkPreviewer $pageresPreviewer) {
35+
public function __construct(
36+
IConfig $config,
37+
ScreeenlyBookmarkPreviewer $screeenlyPreviewer,
38+
DefaultBookmarkPreviewer $defaultPreviewer,
39+
FileCache $cache,
40+
ScreenshotMachineBookmarkPreviewer $screenshotMachinePreviewer,
41+
PageresBookmarkPreviewer $pageresPreviewer,
42+
GenericUrlBookmarkPreviewer $genericUrlPreviewer,
43+
) {
6244
$this->screeenlyPreviewer = $screeenlyPreviewer;
6345
$this->defaultPreviewer = $defaultPreviewer;
6446
$this->screenshotMachinePreviewer = $screenshotMachinePreviewer;
6547
$this->pageresPreviewer = $pageresPreviewer;
48+
$this->genericUrlPreviewer = $genericUrlPreviewer;
6649

6750
$this->enabled = $config->getAppValue('bookmarks', 'privacy.enableScraping', 'false');
6851
$this->cache = $cache;
@@ -85,7 +68,13 @@ public function getImage($bookmark, $cacheOnly = false): ?IImage {
8568
return null;
8669
}
8770

88-
$previewers = [$this->screeenlyPreviewer, $this->screenshotMachinePreviewer, $this->pageresPreviewer, $this->defaultPreviewer];
71+
$previewers = [
72+
$this->screeenlyPreviewer,
73+
$this->screenshotMachinePreviewer,
74+
$this->pageresPreviewer,
75+
$this->genericUrlPreviewer,
76+
$this->defaultPreviewer,
77+
];
8978
foreach ($previewers as $previewer) {
9079
$key = $previewer::CACHE_PREFIX . '-' . md5($bookmark->getUrl());
9180
// Try cache first

‎lib/Service/FileCache.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ private function getStorage(): ISimpleFolder {
5858
if (!$this->storage->fileExists('CACHEDIR.TAG')) {
5959
try {
6060
$this->storage->newFile('CACHEDIR.TAG',
61-
'Signature: 8a477f597d28d172789f06886806bc55' . "\r\n" .
62-
'# This file is a cache directory tag created by the nextcloud bookmarks app.' . "\r\n" .
63-
'# For information about cache directory tags, see:' . "\r\n" .
64-
'# http://www.brynosaurus.com/cachedir/)' . "\r\n"
61+
'Signature: 8a477f597d28d172789f06886806bc55' . "\r\n"
62+
. '# This file is a cache directory tag created by the nextcloud bookmarks app.' . "\r\n"
63+
. '# For information about cache directory tags, see:' . "\r\n"
64+
. '# http://www.brynosaurus.com/cachedir/)' . "\r\n"
6565
);
6666
} catch (NotPermittedException $e) {
6767
// No op

‎lib/Service/Previewers/GenericUrlBookmarkPreviewer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function getImage($bookmark, $cacheOnly = false): ?IImage {
6767
if (!isset($bookmark)) {
6868
return null;
6969
}
70-
if ($this->apiKey === '' || $cacheOnly) {
70+
if ($this->apiUrl === '' || $cacheOnly) {
7171
return null;
7272
}
7373
$url = $bookmark->getUrl();

‎lib/Service/Previewers/PageresBookmarkPreviewer.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ protected function fetchImage(string $serverPath, string $url): Image {
7676
$escapedUrl = escapeshellarg($url);
7777
$env = $this->config->getAppValue('bookmarks', 'previews.pageres.env');
7878

79-
$cmd = "cd {$tempDir} && {$env} {$command} {$escapedUrl} 1024x768" .
80-
' --delay=4 --filename=' . escapeshellarg($tempFile) . ' --crop --overwrite 2>&1';
79+
$cmd = "cd {$tempDir} && {$env} {$command} {$escapedUrl} 1024x768"
80+
. ' --delay=4 --filename=' . escapeshellarg($tempFile) . ' --crop --overwrite 2>&1';
8181

8282
$retries = 0;
8383
$output = [];

‎tests/FolderControllerTest.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace OCA\Bookmarks\Tests;
44

55
use OC;
6-
use OCA\Bookmarks\Controller\BookmarkController;
76
use OCA\Bookmarks\Controller\FoldersController;
87
use OCA\Bookmarks\Db\Bookmark;
98
use OCA\Bookmarks\Db\BookmarkMapper;

0 commit comments

Comments
 (0)