Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
be769f4
Add collectives trash backend provider to files_trashbin
mejo- Apr 12, 2023
cecd019
Trash pages instead of deleting them when files_trashbin is enabled
mejo- May 19, 2023
e8978f4
Allow to restore/delete pages from trash via Collectives API
mejo- May 19, 2023
46874d2
Refactor: move node-related functions from PageService to NodeHelper
mejo- May 19, 2023
88042dd
Add behat tests for page trash
mejo- May 19, 2023
a19e0f7
Remove versions along with pages
mejo- May 22, 2023
d8bb2f7
Delete trash and versions when deleting a collective
mejo- May 22, 2023
8b6ac69
Add behat tests for collectives trash over webdav API
mejo- May 23, 2023
cb18b01
Revert subfolders of parent page when deleting from trash
mejo- May 24, 2023
6300d97
Don't grant access to trash of read-only collectives
mejo- May 24, 2023
bcac981
Restore attachments folder along with page if it exists
mejo- May 24, 2023
c089e16
Restore+remove attachments folder from trash along with page
mejo- May 24, 2023
e310a12
Add behat tests for page trash API in public shares
mejo- May 24, 2023
ea037d5
Add fixes detected by psalm
mejo- May 24, 2023
f935a7c
Call notifyPush when trashing a page also without files_trashbin
mejo- May 25, 2023
b894a77
Delete collective leftovers even if collective folder is already gone
mejo- May 25, 2023
235b4aa
Improve exception message when trashed page not found in database
mejo- May 25, 2023
93c8d63
Also return full PageInfo objects for page trash index
mejo- May 27, 2023
14967ef
Fix handling of deleted pages with subpages
mejo- May 30, 2023
d3a46c4
PageTrashCleanup: Different return values for different failure reasons
mejo- Jun 5, 2023
710a616
Code style fixes by php-cs-fixer
mejo- Jun 5, 2023
34b080a
Make sure long filenames are truncated in trashbin
mejo- Jun 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Trash pages instead of deleting them when files_trashbin is enabled
Add `trashed_timestamp` property to pages table in database to flag
pages as trashed.

Signed-off-by: Jonas <[email protected]>
  • Loading branch information
mejo- committed Jun 5, 2023
commit cecd0196b8fcfa67568d9299ca00490c718b71f9
4 changes: 2 additions & 2 deletions appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#setSubpageOrder', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}/subpageOrder',
'verb' => 'PUT', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#delete', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}',
['name' => 'page#trash', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}',
'verb' => 'DELETE', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
['name' => 'page#getBacklinks', 'url' => '/_api/{collectiveId}/_pages/parent/{parentId}/page/{id}/backlinks',
'verb' => 'GET', 'requirements' => ['collectiveId' => '\d+', 'parentId' => '\d+', 'id' => '\d+']],
Expand All @@ -78,7 +78,7 @@
'verb' => 'PUT', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#setSubpageOrder', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}/subpageOrder',
'verb' => 'PUT', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#delete', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}',
['name' => 'publicPage#trash', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}',
'verb' => 'DELETE', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
['name' => 'publicPage#getAttachments', 'url' => '/_api/p/{token}/_pages/parent/{parentId}/page/{id}/attachments',
'verb' => 'GET', 'requirements' => ['parentId' => '\d+', 'id' => '\d+']],
Expand Down
2 changes: 2 additions & 0 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use OCA\Circles\Events\CircleDestroyedEvent;
use OCA\Collectives\CacheListener;
use OCA\Collectives\Db\CollectiveMapper;
use OCA\Collectives\Db\PageMapper;
use OCA\Collectives\Fs\UserFolderHelper;
use OCA\Collectives\Listeners\BeforeTemplateRenderedListener;
use OCA\Collectives\Listeners\CircleDestroyedListener;
Expand Down Expand Up @@ -74,6 +75,7 @@ public function register(IRegistrationContext $context): void {
$c->get(PageTrashManager::class),
$c->get(MountProvider::class),
$c->get(CollectiveMapper::class),
$c->get(PageMapper::class),
$c->get(LoggerInterface::class)
);
$hasVersionApp = interface_exists(IVersionBackend::class);
Expand Down
4 changes: 2 additions & 2 deletions lib/Controller/PageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ public function setSubpageOrder(int $collectiveId, int $parentId, int $id, ?stri
*
* @return DataResponse
*/
public function delete(int $collectiveId, int $parentId, int $id): DataResponse {
public function trash(int $collectiveId, int $parentId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($collectiveId, $parentId, $id): array {
$userId = $this->getUserId();
$pageInfo = $this->service->delete($collectiveId, $parentId, $id, $userId);
$pageInfo = $this->service->trash($collectiveId, $parentId, $id, $userId);
return [
"data" => $pageInfo
];
Expand Down
4 changes: 2 additions & 2 deletions lib/Controller/PublicPageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,12 @@ public function setSubpageOrder(int $parentId, int $id, ?string $subpageOrder =
*
* @return DataResponse
*/
public function delete(int $parentId, int $id): DataResponse {
public function trash(int $parentId, int $id): DataResponse {
return $this->handleErrorResponse(function () use ($parentId, $id): array {
$this->checkEditPermissions();
$owner = $this->getShare()->getOwner();
$collectiveId = $this->getShare()->getCollectiveId();
$pageInfo = $this->service->delete($collectiveId, $parentId, $id, $owner);
$pageInfo = $this->service->trash($collectiveId, $parentId, $id, $owner);
// Shares don't have a collective path
$pageInfo->setCollectivePath('');
$pageInfo->setShareToken($this->getToken());
Expand Down
4 changes: 4 additions & 0 deletions lib/Db/Page.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@
* @method void setEmoji(string $value)
* @method string getSubpageOrder()
* @method void setSubpageOrder(string $value)
* @method int|null getTrashTimestamp()
* @method void setTrashTimestamp(?int $trashTimestamp)
*/
class Page extends Entity implements JsonSerializable {
protected ?int $fileId = null;
protected ?string $lastUserId = null;
protected ?string $emoji = null;
protected ?string $subpageOrder = null;
protected ?int $trashTimestamp = null;

public function jsonSerialize(): array {
return [
Expand All @@ -32,6 +35,7 @@ public function jsonSerialize(): array {
'lastUserId' => $this->lastUserId,
'emoji' => $this->emoji,
'subpageOrder' => json_decode($this->getSubpageOrder() ?? '[]', true, 512, JSON_THROW_ON_ERROR),
'trashTimestamp' => $this->trashTimestamp,
];
}
}
44 changes: 38 additions & 6 deletions lib/Db/PageMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,23 @@ public function updateOrInsert(Page $page): Page {
*
* @return Page|null
*/
public function deleteByFileId(int $fileId): ?Page {
public function trashByFileId(int $fileId): ?Page {
if (null !== $page = $this->findByFileId($fileId)) {
return $this->delete($page);
$page->setTrashTimestamp(time());
return $this->update($page);
}
return null;
}

/**
* @param int $fileId
*
* @return Page|null
*/
public function restoreByFileId(int $fileId): ?Page {
if (null !== $page = $this->findByFileId($fileId, true)) {
$page->setTrashTimestamp(null);
return $this->update($page);
}
return null;
}
Expand All @@ -59,13 +73,31 @@ public function deleteByFileId(int $fileId): ?Page {
*
* @return Page|null
*/
public function findByFileId(int $fileId): ?Page {
public function deleteByFileId(int $fileId): ?Page {
if (null !== $page = $this->findByFileId($fileId, true)) {
return $this->delete($page);
}
return null;
}

/**
* @param int $fileId
* @param bool $trashed
*
* @return Page|null
*/
public function findByFileId(int $fileId, bool $trashed = false): ?Page {
$qb = $this->db->getQueryBuilder();
$where = $qb->expr()->andX();
$where->add($qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)));
if ($trashed) {
$where->add($qb->expr()->isNotNull('trash_timestamp'));
} else {
$where->add($qb->expr()->isNull('trash_timestamp'));
}
$qb->select('*')
->from($this->tableName)
->where(
$qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))
);
->where($where);
try {
return $this->findEntity($qb);
} catch (DoesNotExistException | MultipleObjectsReturnedException $e) {
Expand Down
3 changes: 3 additions & 0 deletions lib/Migration/Version000200Date20200822000000.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
'notnull' => false,
'length' => 8,
]);
$table->addColumn('trash_timestamp', Types::INTEGER, [
'notnull' => false,
]);

$table->setPrimaryKey(['id']);
$table->addUniqueIndex(['file_id'], 'collectives_pages_file_index');
Expand Down
35 changes: 35 additions & 0 deletions lib/Migration/Version020600Date20230519000000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace OCA\Collectives\Migration;

use Closure;
use Doctrine\DBAL\Types\Types;
use OCP\DB\ISchemaWrapper;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

class Version020600Date20230519000000 extends SimpleMigrationStep {
/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
*
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('collectives_pages');
if (!$table->hasColumn('trash_timestamp')) {
$table->addColumn('trash_timestamp', Types::INTEGER, [
'notnull' => false,
]);
return $schema;
}

return null;
}
}
4 changes: 4 additions & 0 deletions lib/Model/PageInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
* @method string getTitle()
* @method void setSubpageOrder(string $value)
* @method string getSubpageOrder()
* @method int|null getTrashTimestamp()
* @method void setTrashTimestamp(?int $trashTimestamp)
* @method void setTitle(string $value)
* @method int getTimestamp()
* @method void setTimestamp(int $value)
Expand All @@ -44,6 +46,7 @@ class PageInfo extends Entity implements JsonSerializable {
protected ?string $lastUserDisplayName = null;
protected ?string $emoji = null;
protected ?string $subpageOrder = null;
protected ?int $trashTimestamp = null;
protected ?string $title = null;
protected ?int $timestamp = null;
protected ?int $size = null;
Expand All @@ -63,6 +66,7 @@ public function jsonSerialize(): array {
'lastUserDisplayName' => $this->lastUserDisplayName,
'emoji' => $this->emoji,
'subpageOrder' => json_decode($this->subpageOrder ?? '[]', true, 512, JSON_THROW_ON_ERROR),
'trashTimestamp' => $this->trashTimestamp,
'title' => $this->title,
'timestamp' => $this->timestamp,
'size' => $this->size,
Expand Down
33 changes: 25 additions & 8 deletions lib/Service/PageService.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace OCA\Collectives\Service;

use OC\App\AppManager;
use OCA\Collectives\Db\Page;
use OCA\Collectives\Db\PageMapper;
use OCA\Collectives\Fs\NodeHelper;
Expand Down Expand Up @@ -29,11 +30,12 @@ class PageService {
private UserFolderHelper $userFolderHelper;
private IUserManager $userManager;
private IConfig $config;
private ContainerInterface $container;
private ?IQueue $pushQueue = null;
private ?CollectiveInfo $collectiveInfo = null;
private bool $trashEnabled;

/**
* @param AppManager $appManager
* @param PageMapper $pageMapper
* @param NodeHelper $nodeHelper
* @param CollectiveServiceBase $collectiveService
Expand All @@ -42,7 +44,8 @@ class PageService {
* @param IConfig $config
* @param ContainerInterface $container
*/
public function __construct(PageMapper $pageMapper,
public function __construct(AppManager $appManager,
PageMapper $pageMapper,
NodeHelper $nodeHelper,
CollectiveServiceBase $collectiveService,
UserFolderHelper $userFolderHelper,
Expand All @@ -55,6 +58,7 @@ public function __construct(PageMapper $pageMapper,
$this->userFolderHelper = $userFolderHelper;
$this->userManager = $userManager;
$this->config = $config;
$this->trashEnabled = $appManager->isEnabledForUser('files_trashbin');
try {
$this->pushQueue = $container->get(IQueue::class);
} catch (\Exception $e) {
Expand Down Expand Up @@ -188,8 +192,10 @@ private function getParentPageId(File $file, ?Node $parent = null): int {
}

/**
* @param File $file
* @param File $file
* @param Node|null $parent
* @param bool $includeTrash
*
* @return PageInfo
* @throws NotFoundException
*/
Expand Down Expand Up @@ -878,7 +884,7 @@ private function removeFromSubpageOrder(int $collectiveId, int $pageId, int $rem
* @throws NotFoundException
* @throws NotPermittedException
*/
public function delete(int $collectiveId, int $parentId, int $id, string $userId): PageInfo {
public function trash(int $collectiveId, int $parentId, int $id, string $userId): PageInfo {
$this->verifyEditPermissions($collectiveId, $userId);
$folder = $this->getFolder($collectiveId, $parentId, $userId);
$file = $this->nodeHelper->getFileById($folder, $id);
Expand All @@ -902,14 +908,25 @@ public function delete(int $collectiveId, int $parentId, int $id, string $userId
} catch (FilesNotPermittedException $e) {
throw new NotPermittedException($e->getMessage(), 0, $e);
}
$this->pageMapper->deleteByFileId($pageInfo->getId());
$this->removeFromSubpageOrder($collectiveId, $parentId, $id, $userId);

$this->revertSubFolders($folder);
// Delete directly if trash is not available
if (!$this->trashEnabled) {
$this->pageMapper->deleteByFileId($id);
$this->removeFromSubpageOrder($collectiveId, $parentId, $id, $userId);
$this->revertSubFolders($folder);
}

$this->notifyPush($collectiveId, $userId);

$trashedPage = $this->pageMapper->findByFileId($id, true);
if (!$trashedPage) {
throw new NotFoundException('Failed to find trashed page in page trash: ' . $id);
}

$pageInfo->setTrashTimestamp($trashedPage->getTrashTimestamp());
return $pageInfo;
}


/**
* @param string $collectiveName
* @param PageInfo $pageInfo
Expand Down
11 changes: 11 additions & 0 deletions lib/Trash/PageTrashBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use OC\Files\Storage\Wrapper\Jail;
use OCA\Collectives\Db\CollectiveMapper;
use OCA\Collectives\Db\PageMapper;
use OCA\Collectives\Mount\CollectiveFolderManager;
use OCA\Collectives\Mount\CollectiveStorage;
use OCA\Collectives\Mount\MountProvider;
Expand All @@ -26,18 +27,21 @@ class PageTrashBackend implements ITrashBackend {
private PageTrashManager $trashManager;
private MountProvider $mountProvider;
private CollectiveMapper $collectiveMapper;
private PageMapper $pageMapper;
private LoggerInterface $logger;
private ?VersionsBackend $versionsBackend = null;

public function __construct(CollectiveFolderManager $collectiveFolderManager,
PageTrashManager $trashManager,
MountProvider $mountProvider,
CollectiveMapper $collectiveMapper,
PageMapper $pageMapper,
LoggerInterface $logger) {
$this->collectiveFolderManager = $collectiveFolderManager;
$this->trashManager = $trashManager;
$this->mountProvider = $mountProvider;
$this->collectiveMapper = $collectiveMapper;
$this->pageMapper = $pageMapper;
$this->logger = $logger;
}

Expand Down Expand Up @@ -161,6 +165,8 @@ public function restoreItem(ITrashItem $item): void {
$targetFolder->getStorage()->moveFromStorage($trashStorage, $node->getInternalPath(), $targetLocation);
$targetFolder->getStorage()->getUpdater()->renameFromStorage($trashStorage, $node->getInternalPath(), $targetLocation);
$this->trashManager->removeItem((int)$collectiveId, $item->getName(), $item->getDeletedTime());

$this->pageMapper->restoreByFileId($item->getId());
}

/**
Expand Down Expand Up @@ -188,6 +194,8 @@ public function removeItem(ITrashItem $item): void {
if ($item->isRootItem()) {
$this->trashManager->removeItem((int)$collectiveId, $item->getName(), $item->getDeletedTime());
}

$this->pageMapper->deleteByFileId($item->getId());
}

/**
Expand Down Expand Up @@ -217,6 +225,9 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool {
} else {
throw new \Exception('Failed to move collective item to trash');
}

$this->pageMapper->trashByFileId($fileEntry->getId());

return true;
}
return false;
Expand Down