Skip to content

Commit 4ea8977

Browse files
CopilotR0Wi
Copilot
and
R0Wi
authored
Add configurable timeout for OCR processing requests (#333)
* Add configurable REST client timeout for OCR processing Signed-off-by: GitHub <[email protected]> * Add PSALM baseline exceptions for getTimeout method with object type Add baseline exceptions for MixedPropertyFetch, MixedAssignment, and MixedArgument that occur when accessing the timeout property on the generic object parameter in the getTimeout method. Co-authored-by: R0Wi <[email protected]> * Fix PSALM baseline: remove unused entries and add PossiblyUnusedProperty for timeout The previous baseline entries for MixedArgument, MixedAssignment, and MixedPropertyFetch were unused because isset() guards the property access properly. Added the correct baseline entry for PossiblyUnusedProperty on GlobalSettings::$timeout instead. Co-authored-by: R0Wi <[email protected]> --------- Signed-off-by: GitHub <[email protected]> Co-authored-by: Robin Windey <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: R0Wi <[email protected]>
1 parent ad30eee commit 4ea8977

File tree

9 files changed

+197
-9
lines changed

9 files changed

+197
-9
lines changed

‎lib/Controller/GlobalSettingsController.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public function setGlobalSettings(array $globalSettings) : JSONResponse {
6060
return $this->tryExecute(function () use ($globalSettings) {
6161
$globalSettingsObject = new GlobalSettings();
6262
$globalSettingsObject->processorCount = $globalSettings['processorCount'];
63+
$globalSettingsObject->timeout = $globalSettings['timeout'] ?? null;
6364

6465
$this->globalSettingsService->setGlobalSettings($globalSettingsObject);
6566
return $this->globalSettingsService->getGlobalSettings();

‎lib/Model/GlobalSettings.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,6 @@
2929
class GlobalSettings {
3030
/** @var string? */
3131
public $processorCount;
32+
/** @var string? */
33+
public $timeout;
3234
}

‎lib/OcrProcessors/Remote/Client/ApiClient.php‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,24 @@
2626
use OCA\WorkflowOcr\AppInfo\Application;
2727
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\Model\ErrorResult;
2828
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\Model\OcrResult;
29+
use OCA\WorkflowOcr\Service\IGlobalSettingsService;
2930
use OCA\WorkflowOcr\Wrapper\IAppApiWrapper;
3031
use OCP\Http\Client\IResponse;
3132
use Psr\Log\LoggerInterface;
3233
use RuntimeException;
3334

3435
class ApiClient implements IApiClient {
36+
private const DEFAULT_TIMEOUT = 60;
3537
public function __construct(
3638
private IAppApiWrapper $appApiWrapper,
3739
private LoggerInterface $logger,
40+
private IGlobalSettingsService $globalSettingsService,
3841
) {
3942
}
4043

4144
public function processOcr($file, string $fileName, string $ocrMyPdfParameters): OcrResult|ErrorResult {
45+
$settings = $this->globalSettingsService->getGlobalSettings();
46+
$timeout = $this->getTimeout($settings);
4247
$options = [
4348
'multipart' => [
4449
[
@@ -51,7 +56,7 @@ public function processOcr($file, string $fileName, string $ocrMyPdfParameters):
5156
'contents' => $ocrMyPdfParameters
5257
]
5358
],
54-
'timeout' => 60
59+
'timeout' => $timeout
5560
];
5661

5762
$response = $this->exAppRequest('/process_ocr', $options, 'POST');
@@ -100,4 +105,18 @@ private function exAppRequest(string $path, ?array $options, string $method, boo
100105

101106
return $response;
102107
}
108+
109+
/**
110+
* Parse timeout from GlobalSettings and return the effective timeout
111+
*/
112+
private function getTimeout(object $settings): int {
113+
$timeout = self::DEFAULT_TIMEOUT;
114+
if (isset($settings->timeout) && $settings->timeout !== null && $settings->timeout !== '') {
115+
$timeoutInt = (int)$settings->timeout;
116+
if ($timeoutInt > 0) {
117+
$timeout = $timeoutInt;
118+
}
119+
}
120+
return $timeout;
121+
}
103122
}

‎src/components/GlobalSettings.vue‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,22 @@
3838
@input="save">
3939
</div>
4040
</div>
41+
<div class="div-table-row">
42+
<div class="div-table-col div-table-col-left">
43+
<span class="leftcol">{{ translate('Request timeout (seconds):') }}</span>
44+
<br>
45+
<em>{{ translate('Maximum time in seconds to wait for OCR processing to complete. Default is 60 seconds. Increase this value for slow systems or large files.') }}</em>
46+
<br>
47+
<em><strong>{{ translate('Note:') }}</strong> {{ translate('This setting only applies when using the workflow_ocr_backend app.') }}</em>
48+
</div>
49+
<div class="div-table-col">
50+
<input v-model="settings.timeout"
51+
name="timeout"
52+
type="number"
53+
min="1"
54+
@input="save">
55+
</div>
56+
</div>
4157
</NcSettingsSection>
4258
</div>
4359
</template>

‎tests/Integration/IntegrationTestApiClient.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\ApiClient;
2727
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\Model\ErrorResult;
2828
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\Model\OcrResult;
29+
use OCA\WorkflowOcr\Service\IGlobalSettingsService;
2930
use OCA\WorkflowOcr\Wrapper\IAppApiWrapper;
3031
use Psr\Log\LoggerInterface;
3132

@@ -39,8 +40,9 @@ class IntegrationTestApiClient extends ApiClient {
3940
public function __construct(
4041
private IAppApiWrapper $appApiWrapper,
4142
private LoggerInterface $logger,
43+
private IGlobalSettingsService $globalSettingsService,
4244
) {
43-
parent::__construct($appApiWrapper, $logger);
45+
parent::__construct($appApiWrapper, $logger, $globalSettingsService);
4446
}
4547

4648
public function processOcr($file, string $fileName, string $ocrMyPdfParameters): OcrResult|ErrorResult {

‎tests/Unit/Controller/GlobalSettingsControllerTest.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,13 @@ public function testGetSettingsCatchesExceptions() {
7878
public function testSetSettingsCallsService() {
7979
$settings = [
8080
'processorCount' => 42,
81+
'timeout' => 120,
8182
];
8283

8384
$this->globalSettingsService->expects($this->once())
8485
->method('setGlobalSettings')
8586
->with($this->callback(function (GlobalSettings $settings) {
86-
return $settings->processorCount === 42;
87+
return $settings->processorCount === 42 && $settings->timeout === 120;
8788
}));
8889

8990
$this->controller->setGlobalSettings($settings);
@@ -105,4 +106,18 @@ public function testSetSettingsCatchesExceptions() {
105106
$this->assertEquals(500, $result->getStatus());
106107
$this->assertEquals(['error' => 'test'], $result->getData());
107108
}
109+
110+
public function testSetSettingsWithoutTimeout() {
111+
$settings = [
112+
'processorCount' => 42,
113+
];
114+
115+
$this->globalSettingsService->expects($this->once())
116+
->method('setGlobalSettings')
117+
->with($this->callback(function (GlobalSettings $settings) {
118+
return $settings->processorCount === 42 && $settings->timeout === null;
119+
}));
120+
121+
$this->controller->setGlobalSettings($settings);
122+
}
108123
}

‎tests/Unit/OcrProcessors/Remote/Client/ApiClientTest.php‎

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
*/
2323
namespace OCA\WorkflowOcr\Tests\Unit\OcrProcessors\Remote\Client;
2424

25+
use OCA\WorkflowOcr\Model\GlobalSettings;
2526
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\ApiClient;
2627
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\Model\ErrorResult;
2728
use OCA\WorkflowOcr\OcrProcessors\Remote\Client\Model\OcrResult;
29+
use OCA\WorkflowOcr\Service\IGlobalSettingsService;
2830
use OCA\WorkflowOcr\Wrapper\IAppApiWrapper;
2931
use OCP\Http\Client\IResponse;
3032
use PHPUnit\Framework\TestCase;
@@ -35,11 +37,17 @@ class ApiClientTest extends TestCase {
3537
private ApiClient $apiClient;
3638
private $appApiWrapper;
3739
private $logger;
40+
private $globalSettingsService;
3841

3942
protected function setUp(): void {
4043
$this->appApiWrapper = $this->createMock(IAppApiWrapper::class);
4144
$this->logger = $this->createMock(LoggerInterface::class);
42-
$this->apiClient = new ApiClient($this->appApiWrapper, $this->logger);
45+
$this->globalSettingsService = $this->createMock(IGlobalSettingsService::class);
46+
47+
$settings = new GlobalSettings();
48+
$this->globalSettingsService->method('getGlobalSettings')->willReturn($settings);
49+
50+
$this->apiClient = new ApiClient($this->appApiWrapper, $this->logger, $this->globalSettingsService);
4351
}
4452

4553
public function testProcessOcrSuccess(): void {
@@ -114,4 +122,109 @@ public function testHeartbeatFailure(): void {
114122

115123
$this->assertFalse($this->apiClient->heartbeat());
116124
}
125+
126+
public function testProcessOcrUsesConfiguredTimeout(): void {
127+
$settings = new GlobalSettings();
128+
$settings->timeout = '120';
129+
130+
$globalSettingsService = $this->createMock(IGlobalSettingsService::class);
131+
$globalSettingsService->method('getGlobalSettings')->willReturn($settings);
132+
133+
$apiClient = new ApiClient($this->appApiWrapper, $this->logger, $globalSettingsService);
134+
135+
$response = $this->createMock(IResponse::class);
136+
$response->method('getStatusCode')->willReturn(200);
137+
$response->method('getBody')->willReturn(json_encode(['result' => 'success']));
138+
139+
$this->appApiWrapper->expects($this->once())
140+
->method('exAppRequest')
141+
->with(
142+
$this->anything(),
143+
$this->anything(),
144+
$this->anything(),
145+
$this->anything(),
146+
$this->anything(),
147+
$this->callback(function ($options) {
148+
return isset($options['timeout']) && $options['timeout'] === 120;
149+
})
150+
)
151+
->willReturn($response);
152+
153+
$apiClient->processOcr('file_content', 'file.pdf', 'parameters');
154+
}
155+
156+
public function testProcessOcrUsesDefaultTimeoutWhenNotConfigured(): void {
157+
$settings = new GlobalSettings();
158+
159+
$globalSettingsService = $this->createMock(IGlobalSettingsService::class);
160+
$globalSettingsService->method('getGlobalSettings')->willReturn($settings);
161+
162+
$apiClient = new ApiClient($this->appApiWrapper, $this->logger, $globalSettingsService);
163+
164+
$response = $this->createMock(IResponse::class);
165+
$response->method('getStatusCode')->willReturn(200);
166+
$response->method('getBody')->willReturn(json_encode(['result' => 'success']));
167+
168+
$this->appApiWrapper->expects($this->once())
169+
->method('exAppRequest')
170+
->with(
171+
$this->anything(),
172+
$this->anything(),
173+
$this->anything(),
174+
$this->anything(),
175+
$this->anything(),
176+
$this->callback(function ($options) {
177+
return isset($options['timeout']) && $options['timeout'] === 60;
178+
})
179+
)
180+
->willReturn($response);
181+
182+
$apiClient->processOcr('file_content', 'file.pdf', 'parameters');
183+
}
184+
185+
public function testGetTimeoutUsesDefaultWhenNotSet(): void {
186+
$settings = new GlobalSettings();
187+
188+
$apiClient = $this->createMock(ApiClient::class);
189+
190+
// instantiate a real ApiClient to test private method via reflection
191+
$appApiWrapper = $this->getMockBuilder('OCA\\WorkflowOcr\\Wrapper\\IAppApiWrapper')->disableOriginalConstructor()->getMock();
192+
$logger = $this->getMockBuilder('Psr\\Log\\LoggerInterface')->getMock();
193+
$globalSettingsService = $this->getMockBuilder('OCA\\WorkflowOcr\\Service\\IGlobalSettingsService')->getMock();
194+
195+
$realClient = new ApiClient($appApiWrapper, $logger, $globalSettingsService);
196+
197+
$ref = new \ReflectionClass($realClient);
198+
$method = $ref->getMethod('getTimeout');
199+
$method->setAccessible(true);
200+
201+
$this->assertEquals(60, $method->invoke($realClient, $settings));
202+
}
203+
204+
public function testGetTimeoutParsesValuesCorrectly(): void {
205+
$appApiWrapper = $this->getMockBuilder('OCA\\WorkflowOcr\\Wrapper\\IAppApiWrapper')->disableOriginalConstructor()->getMock();
206+
$logger = $this->getMockBuilder('Psr\\Log\\LoggerInterface')->getMock();
207+
$globalSettingsService = $this->getMockBuilder('OCA\\WorkflowOcr\\Service\\IGlobalSettingsService')->getMock();
208+
209+
$realClient = new ApiClient($appApiWrapper, $logger, $globalSettingsService);
210+
$ref = new \ReflectionClass($realClient);
211+
$method = $ref->getMethod('getTimeout');
212+
$method->setAccessible(true);
213+
214+
$settings = new GlobalSettings();
215+
$settings->timeout = '';
216+
$this->assertEquals(60, $method->invoke($realClient, $settings));
217+
218+
$settings->timeout = '120';
219+
$this->assertEquals(120, $method->invoke($realClient, $settings));
220+
221+
$settings->timeout = '0';
222+
$this->assertEquals(60, $method->invoke($realClient, $settings));
223+
224+
$settings->timeout = '-10';
225+
$this->assertEquals(60, $method->invoke($realClient, $settings));
226+
227+
$settings->timeout = '3600';
228+
$this->assertEquals(3600, $method->invoke($realClient, $settings));
229+
}
117230
}

‎tests/Unit/Service/GlobalSettingsServiceTest.php‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,39 @@ public function setUp() : void {
4444
}
4545

4646
public function testGetSettings_ReturnsCorrectSettings() {
47-
$this->config->expects($this->once())
47+
$this->config->expects($this->any())
4848
->method('getValueString')
49-
->with(Application::APP_NAME, 'processorCount')
50-
->willReturn('2');
49+
->willReturnMap([
50+
[Application::APP_NAME, 'processorCount', '2'],
51+
[Application::APP_NAME, 'timeout', '30'],
52+
]);
5153

5254
$settings = $this->globalSettingsService->getGlobalSettings();
5355

5456
$this->assertInstanceOf(GlobalSettings::class, $settings);
5557
$this->assertEquals(2, $settings->processorCount);
58+
$this->assertEquals(30, $settings->timeout);
5659
}
5760

5861
public function testSetSettings_CallsConfigSetAppValue() {
5962
$settings = new GlobalSettings();
6063
$settings->processorCount = '2';
64+
$settings->timeout = '30';
6165

62-
$this->config->expects($this->once())
66+
$this->config->expects($this->any())
6367
->method('setValueString')
64-
->with(Application::APP_NAME, 'processorCount', '2');
68+
->willReturnCallback(
69+
function (string $appName, string $key, string $value) use ($settings) {
70+
if ($key === 'processorCount') {
71+
$this->assertEquals($settings->processorCount, $value);
72+
} elseif ($key === 'timeout') {
73+
$this->assertEquals($settings->timeout, $value);
74+
} else {
75+
$this->fail("Unexpected key: $key");
76+
}
77+
return true;
78+
}
79+
);
6580

6681
$this->globalSettingsService->setGlobalSettings($settings);
6782
}

‎tests/psalm-baseline.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@
7272
<code><![CDATA[__construct]]></code>
7373
</PossiblyUnusedMethod>
7474
</file>
75+
<file src="lib/Model/GlobalSettings.php">
76+
<PossiblyUnusedProperty>
77+
<code><![CDATA[$timeout]]></code>
78+
</PossiblyUnusedProperty>
79+
</file>
7580
<file src="lib/OcrProcessors/Remote/Client/ApiClient.php">
7681
<InvalidReturnStatement>
7782
<code><![CDATA[ObjectSerializer::deserialize(json_decode($response->getBody(), false, 512, JSON_THROW_ON_ERROR), $class)]]></code>

0 commit comments

Comments
 (0)