diff --git a/lib/Controller/GlobalSettingsController.php b/lib/Controller/GlobalSettingsController.php index 89223373..e9fe9fed 100644 --- a/lib/Controller/GlobalSettingsController.php +++ b/lib/Controller/GlobalSettingsController.php @@ -59,8 +59,8 @@ public function getGlobalSettings() : JSONResponse { public function setGlobalSettings(array $globalSettings) : JSONResponse { return $this->tryExecute(function () use ($globalSettings) { $globalSettingsObject = new GlobalSettings(); - $globalSettingsObject->processorCount = $globalSettings['processorCount']; - $globalSettingsObject->timeout = $globalSettings['timeout'] ?? null; + $globalSettingsObject->processorCount = isset($globalSettings['processorCount']) ? (int)$globalSettings['processorCount'] : null; + $globalSettingsObject->timeout = isset($globalSettings['timeout']) ? (int)$globalSettings['timeout'] : null; $this->globalSettingsService->setGlobalSettings($globalSettingsObject); return $this->globalSettingsService->getGlobalSettings(); diff --git a/lib/Migration/Version1034Date20260331194630.php b/lib/Migration/Version1034Date20260331194630.php new file mode 100644 index 00000000..555af25a --- /dev/null +++ b/lib/Migration/Version1034Date20260331194630.php @@ -0,0 +1,107 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\WorkflowOcr\Migration; + +use Closure; +use OCA\WorkflowOcr\AppInfo\Application; +use OCP\DB\ISchemaWrapper; +use OCP\IAppConfig; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Converts the 'processorCount' and 'timeout' app-config values from the + * legacy string type (VALUE_STRING = 4) to the correct integer type + * (VALUE_INT = 8). Nextcloud's IAppConfig refuses to call setValueInt() on + * a key that was previously stored via setValueString(), so we must delete the + * old rows and re-insert them with the right type. + */ +class Version1034Date20260331194630 extends SimpleMigrationStep { + /** Integer keys in GlobalSettings that must be migrated. */ + private const INT_KEYS = ['processorCount', 'timeout']; + + public function __construct( + private IDBConnection $db, + private IAppConfig $appConfig, + ) { + } + + public function name(): string { + return 'convert GlobalSettings integer config values from string to int type'; + } + + public function description(): string { + return 'Deletes processorCount and timeout app-config entries that were ' + . 'stored as VALUE_STRING and re-inserts them as VALUE_INT so that ' + . 'IAppConfig::setValueInt() no longer throws a type-conflict error.'; + } + + /** + * {@inheritDoc} + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + foreach (self::INT_KEYS as $key) { + $this->migrateKey($key, $output); + } + return null; + } + + private function migrateKey(string $key, IOutput $output): void { + // Read the raw string value directly from the DB so we can preserve it. + $qb = $this->db->getQueryBuilder(); + $result = $qb->select('configvalue', 'type') + ->from('appconfig') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_NAME))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter($key))) + ->executeQuery(); + + $row = $result->fetch(); + $result->closeCursor(); + + if ($row === false) { + // Key not yet stored – nothing to migrate. + return; + } + + if ((int)$row['type'] === IAppConfig::VALUE_INT) { + // Already the right type, nothing to do. + return; + } + + $intValue = is_numeric($row['configvalue']) ? (int)$row['configvalue'] : 0; + + // IAppConfig throws a type-conflict if the stored type differs from the + // requested one, so we must remove the old entry before re-inserting. + $this->appConfig->deleteKey(Application::APP_NAME, $key); + $this->appConfig->setValueInt(Application::APP_NAME, $key, $intValue); + + $output->info(sprintf( + "Migrated config key '%s' for app '%s' from string to int (value: %d).", + $key, + Application::APP_NAME, + $intValue, + )); + } +} diff --git a/lib/Model/GlobalSettings.php b/lib/Model/GlobalSettings.php index 917983eb..29c8135c 100644 --- a/lib/Model/GlobalSettings.php +++ b/lib/Model/GlobalSettings.php @@ -27,8 +27,6 @@ namespace OCA\WorkflowOcr\Model; class GlobalSettings { - /** @var string? */ - public $processorCount; - /** @var string? */ - public $timeout; + public ?int $processorCount = null; + public ?int $timeout = null; } diff --git a/lib/OcrProcessors/CommandLineUtils.php b/lib/OcrProcessors/CommandLineUtils.php index 5e4fa8f8..5e82090d 100644 --- a/lib/OcrProcessors/CommandLineUtils.php +++ b/lib/OcrProcessors/CommandLineUtils.php @@ -68,7 +68,7 @@ public function getCommandlineArgs(WorkflowSettings $settings, GlobalSettings $g } // Number of CPU's to be used - $processorCount = intval($globalSettings->processorCount); + $processorCount = $globalSettings->processorCount ?? 0; if ($processorCount > 0) { $args[] = '--jobs ' . $processorCount; } diff --git a/lib/OcrProcessors/Remote/Client/ApiClient.php b/lib/OcrProcessors/Remote/Client/ApiClient.php index ec1c8c69..4caa7e0f 100644 --- a/lib/OcrProcessors/Remote/Client/ApiClient.php +++ b/lib/OcrProcessors/Remote/Client/ApiClient.php @@ -111,11 +111,8 @@ private function exAppRequest(string $path, ?array $options, string $method, boo */ private function getTimeout(object $settings): int { $timeout = self::DEFAULT_TIMEOUT; - if (isset($settings->timeout) && $settings->timeout !== null && $settings->timeout !== '') { - $timeoutInt = (int)$settings->timeout; - if ($timeoutInt > 0) { - $timeout = $timeoutInt; - } + if ($settings->timeout !== null && $settings->timeout > 0) { + $timeout = $settings->timeout; } return $timeout; } diff --git a/lib/Service/GlobalSettingsService.php b/lib/Service/GlobalSettingsService.php index 81b11443..3bea5625 100644 --- a/lib/Service/GlobalSettingsService.php +++ b/lib/Service/GlobalSettingsService.php @@ -53,8 +53,15 @@ public function getGlobalSettings() : GlobalSettings { foreach ($this->getProperties($settings) as $prop) { $key = $prop->getName(); - $configValue = $this->config->getValueString(Application::APP_NAME, $key); - $settings->$key = $configValue; + $type = $prop->getType(); + if ($type instanceof \ReflectionNamedType && $type->getName() === 'int') { + $intValue = $this->config->getValueInt(Application::APP_NAME, $key, 0); + // 0 is used as the "not configured" sentinel: none of the integer settings + // have a meaningful zero value, so 0 → null (not set). + $settings->$key = $intValue > 0 ? $intValue : null; + } else { + $settings->$key = $this->config->getValueString(Application::APP_NAME, $key); + } } return $settings; @@ -69,7 +76,16 @@ public function setGlobalSettings(GlobalSettings $settings) : void { foreach ($this->getProperties($settings) as $prop) { $key = $prop->getName(); $value = $settings->$key; - $this->config->setValueString(Application::APP_NAME, $key, $value); + $type = $prop->getType(); + $isNullable = $type !== null && $type->allowsNull(); + + if ($isNullable && $value === null) { + $this->config->deleteKey(Application::APP_NAME, $key); + } elseif ($type instanceof \ReflectionNamedType && $type->getName() === 'int') { + $this->config->setValueInt(Application::APP_NAME, $key, (int)$value); + } else { + $this->config->setValueString(Application::APP_NAME, $key, (string)($value ?? '')); + } } } diff --git a/psalm.xml b/psalm.xml index 448e34b8..c67bf98d 100644 --- a/psalm.xml +++ b/psalm.xml @@ -12,6 +12,7 @@ + diff --git a/src/components/GlobalSettings.vue b/src/components/GlobalSettings.vue index b63f3039..de399a1f 100644 --- a/src/components/GlobalSettings.vue +++ b/src/components/GlobalSettings.vue @@ -34,10 +34,10 @@
+ @input="settings.processorCount = Number($event.target.value) || null; save()">
@@ -50,11 +50,11 @@
+ @input="settings.timeout = Number($event.target.value) || null; save()">
diff --git a/src/test/components/GlobalSettings.spec.js b/src/test/components/GlobalSettings.spec.js index 0737df69..fa1c6a2f 100644 --- a/src/test/components/GlobalSettings.spec.js +++ b/src/test/components/GlobalSettings.spec.js @@ -49,7 +49,7 @@ describe('Init tests', () => { describe('Interaction tests', () => { test('Should update settings when processorCount is changed', async () => { - const initialMockSettings = { processorCount: '2' } + const initialMockSettings = { processorCount: 2 } getGlobalSettings.mockResolvedValueOnce(initialMockSettings) const afterSaveMockSettings = { processorCount: 42 } @@ -62,7 +62,7 @@ describe('Interaction tests', () => { processorCount.element.value = '42' await processorCount.trigger('input') - // v-model on type="number" input converts to number + // Inline @input handler converts to Number and falls back to null expect(wrapper.vm.settings.processorCount).toBe(42) expect(setGlobalSettings).toHaveBeenCalledTimes(1) expect(setGlobalSettings).toHaveBeenCalledWith(expect.objectContaining({ @@ -70,8 +70,29 @@ describe('Interaction tests', () => { })) }) + test('Should convert empty input to null', async () => { + const initialMockSettings = { processorCount: 2 } + getGlobalSettings.mockResolvedValueOnce(initialMockSettings) + + const afterSaveMockSettings = { processorCount: null } + setGlobalSettings.mockResolvedValueOnce(afterSaveMockSettings) + + const wrapper = mount(GlobalSettings, mountOptions) + await new Promise(process.nextTick) + + const processorCount = wrapper.find('input[name="processorCount"]') + processorCount.element.value = '' + await processorCount.trigger('input') + + expect(wrapper.vm.settings.processorCount).toBeNull() + expect(setGlobalSettings).toHaveBeenCalledTimes(1) + expect(setGlobalSettings).toHaveBeenCalledWith(expect.objectContaining({ + processorCount: null, + })) + }) + test('Should show error when save fails', async () => { - const initialMockSettings = { processorCount: '2' } + const initialMockSettings = { processorCount: 2 } getGlobalSettings.mockResolvedValueOnce(initialMockSettings) setGlobalSettings.mockRejectedValueOnce(new Error('save-failed')) diff --git a/src/test/service/globalSettingsService.spec.js b/src/test/service/globalSettingsService.spec.js index 09f7a682..13a407cb 100644 --- a/src/test/service/globalSettingsService.spec.js +++ b/src/test/service/globalSettingsService.spec.js @@ -19,14 +19,14 @@ test('getGlobalSettings returns correct data from server', async () => { }) test('setGlobalSettings sends correct data to server', async () => { - const request = { data: { processorCount: 42 } } - const mockedResponse = { data: { processorCount: '42' } } + const request = { processorCount: 42 } + const mockedResponse = { data: { processorCount: 42 } } axios.put.mockResolvedValueOnce(mockedResponse) const result = await setGlobalSettings(request) - expect(result.processorCount).toBe('42') + expect(result.processorCount).toBe(42) expect(axios.put).toHaveBeenCalledTimes(1) expect(axios.put).toHaveBeenCalledWith('/apps/workflow_ocr/globalSettings', { globalSettings: request }) }) diff --git a/tests/Unit/Controller/GlobalSettingsControllerTest.php b/tests/Unit/Controller/GlobalSettingsControllerTest.php index 1bc2534e..51a8d4d2 100644 --- a/tests/Unit/Controller/GlobalSettingsControllerTest.php +++ b/tests/Unit/Controller/GlobalSettingsControllerTest.php @@ -120,4 +120,19 @@ public function testSetSettingsWithoutTimeout() { $this->controller->setGlobalSettings($settings); } + + public function testSetSettingsPassesNullValues() { + $settings = [ + 'processorCount' => null, + 'timeout' => null, + ]; + + $this->globalSettingsService->expects($this->once()) + ->method('setGlobalSettings') + ->with($this->callback(function (GlobalSettings $settings) { + return $settings->processorCount === null && $settings->timeout === null; + })); + + $this->controller->setGlobalSettings($settings); + } } diff --git a/tests/Unit/OcrProcessors/Remote/Client/ApiClientTest.php b/tests/Unit/OcrProcessors/Remote/Client/ApiClientTest.php index 1b51ca92..df470c93 100644 --- a/tests/Unit/OcrProcessors/Remote/Client/ApiClientTest.php +++ b/tests/Unit/OcrProcessors/Remote/Client/ApiClientTest.php @@ -125,7 +125,7 @@ public function testHeartbeatFailure(): void { public function testProcessOcrUsesConfiguredTimeout(): void { $settings = new GlobalSettings(); - $settings->timeout = '120'; + $settings->timeout = 120; $globalSettingsService = $this->createMock(IGlobalSettingsService::class); $globalSettingsService->method('getGlobalSettings')->willReturn($settings); @@ -212,19 +212,19 @@ public function testGetTimeoutParsesValuesCorrectly(): void { $method->setAccessible(true); $settings = new GlobalSettings(); - $settings->timeout = ''; + $settings->timeout = null; $this->assertEquals(60, $method->invoke($realClient, $settings)); - $settings->timeout = '120'; + $settings->timeout = 120; $this->assertEquals(120, $method->invoke($realClient, $settings)); - $settings->timeout = '0'; + $settings->timeout = 0; $this->assertEquals(60, $method->invoke($realClient, $settings)); - $settings->timeout = '-10'; + $settings->timeout = -10; $this->assertEquals(60, $method->invoke($realClient, $settings)); - $settings->timeout = '3600'; + $settings->timeout = 3600; $this->assertEquals(3600, $method->invoke($realClient, $settings)); } } diff --git a/tests/Unit/Service/GlobalSettingsServiceTest.php b/tests/Unit/Service/GlobalSettingsServiceTest.php index a26cc273..41014fd6 100644 --- a/tests/Unit/Service/GlobalSettingsServiceTest.php +++ b/tests/Unit/Service/GlobalSettingsServiceTest.php @@ -45,10 +45,10 @@ public function setUp() : void { public function testGetSettings_ReturnsCorrectSettings() { $this->config->expects($this->any()) - ->method('getValueString') + ->method('getValueInt') ->willReturnMap([ - [Application::APP_NAME, 'processorCount', '2'], - [Application::APP_NAME, 'timeout', '30'], + [Application::APP_NAME, 'processorCount', 0, 2], + [Application::APP_NAME, 'timeout', 0, 30], ]); $settings = $this->globalSettingsService->getGlobalSettings(); @@ -58,15 +58,30 @@ public function testGetSettings_ReturnsCorrectSettings() { $this->assertEquals(30, $settings->timeout); } - public function testSetSettings_CallsConfigSetAppValue() { + public function testGetSettings_ReturnsNullForUnsetValues() { + // getValueInt returns 0 for keys that have never been stored. + // 0 is not a valid value for processorCount or timeout, so it is + // treated as "not configured" and mapped to null. + $this->config->expects($this->any()) + ->method('getValueInt') + ->willReturn(0); + + $settings = $this->globalSettingsService->getGlobalSettings(); + + $this->assertInstanceOf(GlobalSettings::class, $settings); + $this->assertNull($settings->processorCount); + $this->assertNull($settings->timeout); + } + + public function testSetSettings_CallsConfigSetValueInt() { $settings = new GlobalSettings(); - $settings->processorCount = '2'; - $settings->timeout = '30'; + $settings->processorCount = 2; + $settings->timeout = 30; $this->config->expects($this->any()) - ->method('setValueString') + ->method('setValueInt') ->willReturnCallback( - function (string $appName, string $key, string $value) use ($settings) { + function (string $appName, string $key, int $value) use ($settings) { if ($key === 'processorCount') { $this->assertEquals($settings->processorCount, $value); } elseif ($key === 'timeout') { @@ -80,4 +95,24 @@ function (string $appName, string $key, string $value) use ($settings) { $this->globalSettingsService->setGlobalSettings($settings); } + + public function testSetSettings_DeletesKeyForNullValues() { + $settings = new GlobalSettings(); + $settings->processorCount = null; + $settings->timeout = null; + + $this->config->expects($this->never()) + ->method('setValueInt'); + + $this->config->expects($this->exactly(2)) + ->method('deleteKey') + ->willReturnCallback( + function (string $appName, string $key) { + $this->assertEquals(Application::APP_NAME, $appName); + $this->assertContains($key, ['processorCount', 'timeout']); + } + ); + + $this->globalSettingsService->setGlobalSettings($settings); + } } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 0797d114..84367c6b 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -36,16 +36,6 @@ - - - - - - - - - - rootFolder]]>