Skip to content

Commit de89a50

Browse files
artongeartonge
committed
fix: Remove legacy optimization when checking encryption status
This is causing performance issues because for non encrypted nodes, we were checking all upper folders in the hierarchy. Also, this seem unnecessary as top and sub E2EE folders are marked as encrypted, so checking for the current node or its parent folder is enough to have the answer. Partially introduced by #316 which was an improvement at the time by introducing caching. Signed-off-by: Louis Chemineau <[email protected]>
1 parent 60e10e2 commit de89a50

File tree

7 files changed

+18
-171
lines changed

7 files changed

+18
-171
lines changed

‎lib/Connector/Sabre/APlugin.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
use OCA\DAV\Connector\Sabre\Directory;
1111
use OCA\DAV\Connector\Sabre\File;
12-
use OCA\EndToEndEncryption\E2EEnabledPathCache;
12+
use OCP\Files\Folder;
1313
use OCP\Files\IRootFolder;
1414
use OCP\IUserSession;
1515
use Sabre\DAV\Exception\Conflict;
@@ -22,7 +22,6 @@ abstract class APlugin extends ServerPlugin {
2222
protected ?Server $server = null;
2323
protected IRootFolder $rootFolder;
2424
protected IUserSession $userSession;
25-
protected E2EEnabledPathCache $pathCache;
2625

2726
/**
2827
* Should plugin be applied to the current node?
@@ -36,11 +35,9 @@ abstract class APlugin extends ServerPlugin {
3635
public function __construct(
3736
IRootFolder $rootFolder,
3837
IUserSession $userSession,
39-
E2EEnabledPathCache $pathCache,
4038
) {
4139
$this->rootFolder = $rootFolder;
4240
$this->userSession = $userSession;
43-
$this->pathCache = $pathCache;
4441
}
4542

4643
/**
@@ -85,7 +82,12 @@ protected function getNodeForPath(string $path): INode {
8582
*/
8683
protected function isE2EEnabledPath(INode $node): bool {
8784
if ($node instanceof \OCA\DAV\Connector\Sabre\Node) {
88-
return $this->pathCache->isE2EEnabledPath($node->getNode());
85+
$node = $node->getNode();
86+
if ($node instanceof Folder) {
87+
return $node->isEncrypted();
88+
} else {
89+
return $node->getParent()->isEncrypted();
90+
}
8991
}
9092
return false;
9193
}

‎lib/Connector/Sabre/LockPlugin.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
1515
use OCA\DAV\Connector\Sabre\File;
1616
use OCA\DAV\Upload\FutureFile;
17-
use OCA\EndToEndEncryption\E2EEnabledPathCache;
1817
use OCA\EndToEndEncryption\LockManager;
1918
use OCA\EndToEndEncryption\UserAgentManager;
2019
use OCP\AppFramework\Http;
@@ -34,8 +33,8 @@ public function __construct(IRootFolder $rootFolder,
3433
IUserSession $userSession,
3534
LockManager $lockManager,
3635
UserAgentManager $userAgentManager,
37-
E2EEnabledPathCache $pathCache) {
38-
parent::__construct($rootFolder, $userSession, $pathCache);
36+
) {
37+
parent::__construct($rootFolder, $userSession);
3938
$this->lockManager = $lockManager;
4039
$this->userAgentManager = $userAgentManager;
4140
}

‎lib/Connector/Sabre/PropFindPlugin.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use OCA\DAV\Connector\Sabre\Directory;
1313
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
1414
use OCA\DAV\Connector\Sabre\File;
15-
use OCA\EndToEndEncryption\E2EEnabledPathCache;
1615
use OCA\EndToEndEncryption\IMetaDataStorage;
1716
use OCA\EndToEndEncryption\UserAgentManager;
1817
use OCP\Files\Folder;
@@ -35,7 +34,6 @@ class PropFindPlugin extends APlugin {
3534
public function __construct(
3635
IRootFolder $rootFolder,
3736
IUserSession $userSession,
38-
E2EEnabledPathCache $pathCache,
3937
private UserAgentManager $userAgentManager,
4038
private IRequest $request,
4139
private IMetaDataStorage $metaDataStorage,

‎lib/E2EEnabledPathCache.php‎

Lines changed: 0 additions & 111 deletions
This file was deleted.

‎tests/Unit/Connector/Sabre/LockPluginTest.php‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use OCA\DAV\Connector\Sabre\File;
1616
use OCA\DAV\Upload\FutureFile;
1717
use OCA\EndToEndEncryption\Connector\Sabre\LockPlugin;
18-
use OCA\EndToEndEncryption\E2EEnabledPathCache;
1918
use OCA\EndToEndEncryption\LockManager;
2019
use OCA\EndToEndEncryption\UserAgentManager;
2120
use OCP\Files\Cache\ICache;
@@ -31,21 +30,10 @@
3130

3231
class LockPluginTest extends TestCase {
3332

34-
/** @var IRootFolder|\PHPUnit\Framework\MockObject\MockObject */
35-
private $rootFolder;
36-
37-
/** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */
38-
private $userSession;
39-
40-
/** @var LockManager|\PHPUnit\Framework\MockObject\MockObject */
41-
private $lockManager;
42-
43-
/** @var UserAgentManager|\PHPUnit\Framework\MockObject\MockObject */
44-
private $userAgentManager;
45-
46-
/** @var E2EEnabledPathCache|\PHPUnit\Framework\MockObject\MockObject */
47-
private $pathCache;
48-
33+
private IRootFolder&\PHPUnit\Framework\MockObject\MockObject $rootFolder;
34+
private IUserSession&\PHPUnit\Framework\MockObject\MockObject $userSession;
35+
private LockManager&\PHPUnit\Framework\MockObject\MockObject $lockManager;
36+
private UserAgentManager&\PHPUnit\Framework\MockObject\MockObject $userAgentManager;
4937
private LockPlugin $plugin;
5038

5139
protected function setUp(): void {
@@ -55,10 +43,8 @@ protected function setUp(): void {
5543
$this->userSession = $this->createMock(IUserSession::class);
5644
$this->lockManager = $this->createMock(LockManager::class);
5745
$this->userAgentManager = $this->createMock(UserAgentManager::class);
58-
$this->pathCache = $this->createMock(E2EEnabledPathCache::class);
5946

60-
$this->plugin = new LockPlugin($this->rootFolder, $this->userSession,
61-
$this->lockManager, $this->userAgentManager, $this->pathCache);
47+
$this->plugin = new LockPlugin($this->rootFolder, $this->userSession, $this->lockManager, $this->userAgentManager);
6248
}
6349

6450
public function testInitialize(): void {
@@ -85,7 +71,6 @@ public function testCheckLockForCalendar(): void {
8571
$this->userSession,
8672
$this->lockManager,
8773
$this->userAgentManager,
88-
$this->pathCache,
8974
])
9075
->getMock();
9176

@@ -132,7 +117,6 @@ public function testCheckLockNonCopyMoveNoE2EPath(string $method):void {
132117
$this->userSession,
133118
$this->lockManager,
134119
$this->userAgentManager,
135-
$this->pathCache,
136120
])
137121
->getMock();
138122

@@ -183,7 +167,6 @@ public function testCheckLockBlockUnsupportedClients(string $method): void {
183167
$this->userSession,
184168
$this->lockManager,
185169
$this->userAgentManager,
186-
$this->pathCache,
187170
])
188171
->getMock();
189172

@@ -267,7 +250,6 @@ public function testCheckLockForWrite(string $method,
267250
$this->userSession,
268251
$this->lockManager,
269252
$this->userAgentManager,
270-
$this->pathCache,
271253
])
272254
->getMock();
273255

@@ -396,7 +378,6 @@ public function testCheckLockForWriteCopyMove(string $method,
396378
$this->userSession,
397379
$this->lockManager,
398380
$this->userAgentManager,
399-
$this->pathCache,
400381
])
401382
->getMock();
402383

@@ -545,7 +526,6 @@ public function testIsE2EEnabledPathEncryptedFolder():void {
545526
$this->userSession,
546527
$this->lockManager,
547528
$this->userAgentManager,
548-
new E2EEnabledPathCache(),
549529
])
550530
->getMock();
551531

@@ -568,7 +548,6 @@ public function testIsE2EEnabledPathParentEncrypted():void {
568548
$this->userSession,
569549
$this->lockManager,
570550
$this->userAgentManager,
571-
new E2EEnabledPathCache(),
572551
])
573552
->getMock();
574553

@@ -624,7 +603,6 @@ public function testIsE2EEnabledPathNonEncrypted():void {
624603
$this->userSession,
625604
$this->lockManager,
626605
$this->userAgentManager,
627-
new E2EEnabledPathCache(),
628606
])
629607
->getMock();
630608

‎tests/Unit/Connector/Sabre/PropFindPluginTest.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use OCA\DAV\Connector\Sabre\Directory;
1212
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
1313
use OCA\EndToEndEncryption\Connector\Sabre\PropFindPlugin;
14-
use OCA\EndToEndEncryption\E2EEnabledPathCache;
1514
use OCA\EndToEndEncryption\IMetaDataStorage;
1615
use OCA\EndToEndEncryption\UserAgentManager;
1716
use OCP\Files\Folder;
@@ -32,7 +31,6 @@ class PropFindPluginTest extends TestCase {
3231
private UserAgentManager&\PHPUnit\Framework\MockObject\MockObject $userAgentManager;
3332
private IRequest&\PHPUnit\Framework\MockObject\MockObject $request;
3433
protected Server&\PHPUnit\Framework\MockObject\MockObject $server;
35-
protected E2EEnabledPathCache&\PHPUnit\Framework\MockObject\MockObject $pathCache;
3634
protected IMetaDataStorage&\PHPUnit\Framework\MockObject\MockObject $metaDataStorage;
3735
protected Folder&\PHPUnit\Framework\MockObject\MockObject $userFolder;
3836
private PropFindPlugin $plugin;
@@ -44,14 +42,13 @@ protected function setUp(): void {
4442
$this->userSession = $this->createMock(IUserSession::class);
4543
$this->userAgentManager = $this->createMock(UserAgentManager::class);
4644
$this->request = $this->createMock(IRequest::class);
47-
$this->pathCache = $this->createMock(E2EEnabledPathCache::class);
45+
$this->server = $this->createMock(Server::class);
4846
$this->metaDataStorage = $this->createMock(IMetaDataStorage::class);
4947
$this->userFolder = $this->createMock(Folder::class);
5048

5149
$this->plugin = new PropFindPlugin(
5250
$this->rootFolder,
5351
$this->userSession,
54-
$this->pathCache,
5552
$this->userAgentManager,
5653
$this->request,
5754
$this->metaDataStorage,

0 commit comments

Comments
 (0)