From 3ca3015c64d8a3d77222a23693521ee4c231d85a Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 14 Sep 2026 17:13:31 -0400 Subject: [PATCH 1/5] fix(files_external): always set default fs check policy for mounts Apply the OncePerRequest filesystem-check policy when an external mount does not define filesystem_check_changes. This keeps mounts created through occ and imports consistent with GUI-created mounts while preserving explicitly configured values. Fixes #53249 Signed-off-by: Josh --- .../files_external/lib/Service/StoragesService.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php index 545c1be0da68e..9aae1d572c30a 100644 --- a/apps/files_external/lib/Service/StoragesService.php +++ b/apps/files_external/lib/Service/StoragesService.php @@ -36,6 +36,8 @@ * @psalm-import-type ExternalMountInfo from DBConfigService */ abstract class StoragesService { + private const DEFAULT_FILESYSTEM_CHECK_CHANGES = 1; + public function __construct( protected BackendService $backendService, protected DBConfigService $dbConfig, @@ -60,13 +62,18 @@ protected function getStorageConfigFromDBMount(array $mount): ?StorageConfig { => $applicable['type'] === DBConfigService::APPLICABLE_TYPE_GROUP); $applicableGroups = array_map(static fn (array $applicable) => $applicable['value'], $applicableGroups); + $options = $mount['options']; + if (!array_key_exists('filesystem_check_changes', $options)) { + $options['filesystem_check_changes'] = self::DEFAULT_FILESYSTEM_CHECK_CHANGES; + } + try { $config = $this->createStorage( $mount['mount_point'], $mount['storage_backend'], $mount['auth_backend'], $mount['config'], - $mount['options'], + $options, array_values($applicableUsers), array_values($applicableGroups), $mount['priority'] @@ -210,6 +217,11 @@ public function addStorage(StorageConfig $newStorage): StorageConfig { foreach ($newStorage->getBackendOptions() as $key => $value) { $this->dbConfig->setConfig($configId, $key, $value); } + + if (!array_key_exists('filesystem_check_changes', $newStorage->getMountOptions())) { + $newStorage->setMountOption('filesystem_check_changes', self::DEFAULT_FILESYSTEM_CHECK_CHANGES); + } + foreach ($newStorage->getMountOptions() as $key => $value) { $this->dbConfig->setOption($configId, $key, $value); } From 19ba876af7befbcaa9e6b26bd95307051fc9f491 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 14 Sep 2026 17:21:17 -0400 Subject: [PATCH 2/5] test(files_external): cover fs check defaults for external mounts Verify that mounts without an explicit filesystem_check_changes option default to checking once per request, while preserving explicit values for Never, OncePerRequest, and Always. Also cover loading legacy mounts that lack the option. Assisted-by: Copilot:gpt-5.6-luna Signed-off-by: Josh --- .../Service/GlobalStoragesServiceTest.php | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/apps/files_external/tests/Service/GlobalStoragesServiceTest.php b/apps/files_external/tests/Service/GlobalStoragesServiceTest.php index 59bab4d044487..8fca68df3b1eb 100644 --- a/apps/files_external/tests/Service/GlobalStoragesServiceTest.php +++ b/apps/files_external/tests/Service/GlobalStoragesServiceTest.php @@ -11,6 +11,7 @@ use OC\Files\Filesystem; use OCA\Files_External\MountConfig; +use OCA\Files_External\Service\DBConfigService; use OCA\Files_External\Service\GlobalStoragesService; #[\PHPUnit\Framework\Attributes\Group(name: 'DB')] @@ -126,6 +127,7 @@ public function testAddStorage($storageParams): void { $this->assertEquals($storage->getBackend(), $newStorage->getBackend()); $this->assertEquals($storage->getAuthMechanism(), $newStorage->getAuthMechanism()); $this->assertEquals($storage->getBackendOptions(), $newStorage->getBackendOptions()); + $this->assertSame(1, $newStorage->getMountOption('filesystem_check_changes')); $this->assertEquals($storage->getApplicableUsers(), $newStorage->getApplicableUsers()); $this->assertEquals($storage->getApplicableGroups(), $newStorage->getApplicableGroups()); $this->assertEquals($storage->getPriority(), $newStorage->getPriority()); @@ -135,6 +137,62 @@ public function testAddStorage($storageParams): void { $this->assertEquals($baseId + 1, $nextStorage->getId()); } + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'filesystemCheckChangesProvider')] + public function testAddStoragePreservesFilesystemCheckChanges(int $value): void { + $storage = $this->makeStorageConfig([ + 'mountPoint' => 'mountpoint-' . $value, + 'backendIdentifier' => 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'authMechanismIdentifier' => 'identifier:\Auth\Mechanism', + 'backendOptions' => [ + 'option1' => 'value1', + ], + 'applicableUsers' => [], + 'applicableGroups' => [], + 'mountOptions' => [ + 'filesystem_check_changes' => $value, + ], + ]); + + $newStorage = $this->service->addStorage($storage); + $reloadedStorage = $this->service->getStorage($newStorage->getId()); + + $this->assertSame( + $value, + $reloadedStorage->getMountOption('filesystem_check_changes') + ); + } + + public static function filesystemCheckChangesProvider(): array { + return [ + 'never' => [0], + 'once per request' => [1], + 'always' => [2], + ]; + } + + public function testLoadingMountWithoutFilesystemCheckChangesUsesCompatibilityDefault(): void { + $mountId = $this->dbConfig->addMount( + 'mountpoint', + 'identifier:\OCA\Files_External\Lib\Backend\SMB', + 'identifier:\Auth\Mechanism', + 100, + \OCA\Files_External\Service\DBConfigService::MOUNT_TYPE_ADMIN + ); + + $this->dbConfig->addApplicable( + $mountId, + DBConfigService::APPLICABLE_TYPE_GLOBAL, + null + ); + + $storage = $this->service->getStorage($mountId); + + $this->assertSame( + 1, + $storage->getMountOption('filesystem_check_changes') + ); + } + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'storageDataProvider')] public function testUpdateStorage($updatedStorageParams): void { $updatedStorage = $this->makeStorageConfig($updatedStorageParams); From 75855942802ac08136d4bdddbd1647e945e927ec Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 14 Sep 2026 17:25:41 -0400 Subject: [PATCH 3/5] chore(files_external): rename default fs check policy constant Signed-off-by: Josh --- apps/files_external/lib/Service/StoragesService.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php index 9aae1d572c30a..057c6ec1c71fc 100644 --- a/apps/files_external/lib/Service/StoragesService.php +++ b/apps/files_external/lib/Service/StoragesService.php @@ -36,7 +36,8 @@ * @psalm-import-type ExternalMountInfo from DBConfigService */ abstract class StoragesService { - private const DEFAULT_FILESYSTEM_CHECK_CHANGES = 1; + // Default policy: once per request + private const DEFAULT_FILESYSTEM_CHECK_POLICY = 1; public function __construct( protected BackendService $backendService, @@ -64,7 +65,7 @@ protected function getStorageConfigFromDBMount(array $mount): ?StorageConfig { $options = $mount['options']; if (!array_key_exists('filesystem_check_changes', $options)) { - $options['filesystem_check_changes'] = self::DEFAULT_FILESYSTEM_CHECK_CHANGES; + $options['filesystem_check_changes'] = self::DEFAULT_FILESYSTEM_CHECK_POLICY; } try { @@ -219,7 +220,7 @@ public function addStorage(StorageConfig $newStorage): StorageConfig { } if (!array_key_exists('filesystem_check_changes', $newStorage->getMountOptions())) { - $newStorage->setMountOption('filesystem_check_changes', self::DEFAULT_FILESYSTEM_CHECK_CHANGES); + $newStorage->setMountOption('filesystem_check_changes', self::DEFAULT_FILESYSTEM_CHECK_POLICY); } foreach ($newStorage->getMountOptions() as $key => $value) { From ac0fd5fa12b1a1219034d95b99b19b55dc35bec9 Mon Sep 17 00:00:00 2001 From: Josh Richards Date: Wed, 16 Sep 2026 09:38:00 -0400 Subject: [PATCH 4/5] fix(files_external): preserve mount option types in storage config Accept native mount-option values in DBConfigService and add round-trip coverage for numeric and boolean options. Also skip the inherited filesystem-check test for UserGlobalStoragesService, which is read-only. Signed-off-by: Josh Richards --- .../lib/Service/DBConfigService.php | 2 +- .../tests/Service/DBConfigServiceTest.php | 22 +++++++++++++++++++ .../Service/UserGlobalStoragesServiceTest.php | 5 +++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/Service/DBConfigService.php b/apps/files_external/lib/Service/DBConfigService.php index d6135ff88cccb..69c1b39b4d07a 100644 --- a/apps/files_external/lib/Service/DBConfigService.php +++ b/apps/files_external/lib/Service/DBConfigService.php @@ -384,7 +384,7 @@ public function setConfig(int $mountId, string $key, string $value): void { } } - public function setOption(int $mountId, string $key, string $value): void { + public function setOption(int $mountId, string $key, mixed $value): void { try { $builder = $this->connection->getQueryBuilder(); $builder->insert('external_options') diff --git a/apps/files_external/tests/Service/DBConfigServiceTest.php b/apps/files_external/tests/Service/DBConfigServiceTest.php index 989f8b918adf9..473b413e4c7a6 100644 --- a/apps/files_external/tests/Service/DBConfigServiceTest.php +++ b/apps/files_external/tests/Service/DBConfigServiceTest.php @@ -141,6 +141,28 @@ public function testSetConfigOverwrite(): void { $this->assertEquals(['foo' => 'qwerty', 'asd' => '1'], $mount['config']); } + #[DataProvider('nativeOptionValuesProvider')] + public function testSetOptionPreservesNativeTypes(mixed $value): void { + $id = $this->addMount('/test', 'foo', 'bar', 100, DBConfigService::MOUNT_TYPE_ADMIN); + + $this->dbConfig->setOption($id, 'test_option', $value); + + $mount = $this->dbConfig->getMountById($id); + + $this->assertSame($value, $mount['options']['test_option']); + } + + public static function nativeOptionValuesProvider(): array { + return [ + 'zero integer' => [0], + 'positive integer' => [1], + 'negative integer' => [-1], + 'float' => [1.5], + 'true' => [true], + 'false' => [false], + ]; + } + public function testSetOption(): void { $id = $this->addMount('/test', 'foo', 'bar', 100, DBConfigService::MOUNT_TYPE_ADMIN); $this->dbConfig->setOption($id, 'foo', 'bar'); diff --git a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php index 4a35796b97f46..886fa5ccc5ecd 100644 --- a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php +++ b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php @@ -267,6 +267,11 @@ public function testGetUniqueStorages( } } + public function testAddStoragePreservesFilesystemCheckChanges(int $value): void { + // we don't test this here + $this->addToAssertionCount(1); + } + public function testGetStoragesBackendNotVisible(): void { // we don't test this here $this->addToAssertionCount(1); From 34088d93c4da579cfbc8f9e21c2cb141882b5c7a Mon Sep 17 00:00:00 2001 From: Josh Richards Date: Wed, 16 Sep 2026 16:20:24 -0400 Subject: [PATCH 5/5] test(files_external): fix overridden data-provider test signature Signed-off-by: Josh Richards --- .../tests/Service/UserGlobalStoragesServiceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php index 886fa5ccc5ecd..a51dfa8f7c84f 100644 --- a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php +++ b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php @@ -267,7 +267,7 @@ public function testGetUniqueStorages( } } - public function testAddStoragePreservesFilesystemCheckChanges(int $value): void { + public function testAddStoragePreservesFilesystemCheckChanges(): void { // we don't test this here $this->addToAssertionCount(1); }