Skip to content

Commit 87655da

Browse files
janepiejanepie
authored
Merge pull request #256 from nextcloud/fix/loading-view
feat: show loading screen in task area only
2 parents d038247 + d03bead commit 87655da

11 files changed

+690
-305
lines changed

‎appinfo/routes.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
['name' => 'assistantApi#getAvailableTaskTypes', 'url' => '/api/{apiVersion}/task-types', 'verb' => 'GET', 'requirements' => $requirements],
2525
['name' => 'assistantApi#getUserTasks', 'url' => '/api/{apiVersion}/tasks', 'verb' => 'GET', 'requirements' => $requirements],
2626
['name' => 'assistantApi#parseTextFromFile', 'url' => '/api/{apiVersion}/parse-file', 'verb' => 'POST', 'requirements' => $requirements],
27+
['name' => 'assistantApi#getNotifyWhenReady', 'url' => '/api/{apiVersion}/task/{ocpTaskId}/notify', 'verb' => 'GET', 'requirements' => $requirements],
2728
['name' => 'assistantApi#notifyWhenReady', 'url' => '/api/{apiVersion}/task/{ocpTaskId}/notify', 'verb' => 'POST', 'requirements' => $requirements],
29+
['name' => 'assistantApi#cancelNotifyWhenReady', 'url' => '/api/{apiVersion}/task/{ocpTaskId}/notify', 'verb' => 'DELETE', 'requirements' => $requirements],
2830
['name' => 'assistantApi#uploadInputFile', 'url' => '/api/{apiVersion}/input-file', 'verb' => 'POST', 'requirements' => $requirements],
2931
['name' => 'assistantApi#displayUserFile', 'url' => '/api/{apiVersion}/file/{fileId}/display', 'verb' => 'GET', 'requirements' => $requirements],
3032
['name' => 'assistantApi#getUserFileInfo', 'url' => '/api/{apiVersion}/file/{fileId}/info', 'verb' => 'GET', 'requirements' => $requirements],

‎lib/Controller/AssistantApiController.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,32 @@ public function __construct(
4747
parent::__construct($appName, $request);
4848
}
4949

50+
/**
51+
* Get the notification request for a task when it has finished
52+
*
53+
* Does not need bruteforce protection since we respond with success anyways
54+
* as we don't want to keep the front-end waiting.
55+
* However, we still use rate limiting to prevent timing attacks.
56+
*
57+
* @param int $ocpTaskId ID of the target task
58+
* @return DataResponse<Http::STATUS_OK, array{id: int, ocp_task_id: int, timestamp: int}, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR, array{error: string}, array{}>
59+
* @throws MultipleObjectsReturnedException
60+
*
61+
* 200: Task notification request retrieved successfully
62+
*/
63+
#[NoAdminRequired]
64+
#[NoCSRFRequired]
65+
#[AnonRateLimit(limit: 10, period: 60)]
66+
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT, tags: ['assistant_api'])]
67+
public function getNotifyWhenReady(int $ocpTaskId): DataResponse {
68+
if ($this->userId === null) {
69+
return new DataResponse(['error' => $this->l10n->t('Failed to notify when ready; unknown user')], Http::STATUS_INTERNAL_SERVER_ERROR);
70+
}
71+
72+
$notification = $this->assistantService->getNotifyWhenReady($ocpTaskId, $this->userId);
73+
return new DataResponse($notification, Http::STATUS_OK);
74+
}
75+
5076
/**
5177
* Notify when the task has finished
5278
*
@@ -77,6 +103,36 @@ public function notifyWhenReady(int $ocpTaskId): DataResponse {
77103
return new DataResponse('', Http::STATUS_OK);
78104
}
79105

106+
/**
107+
* Cancel an existing notification when a task has finished
108+
*
109+
* Does not need bruteforce protection since we respond with success anyways
110+
* as we don't want to keep the front-end waiting.
111+
* However, we still use rate limiting to prevent timing attacks.
112+
*
113+
* @param int $ocpTaskId ID of the target task
114+
* @return DataResponse<Http::STATUS_OK, '', array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR, array{error: string}, array{}>
115+
* @throws MultipleObjectsReturnedException
116+
*
117+
* 200: Ready notification deleted successfully
118+
*/
119+
#[NoAdminRequired]
120+
#[NoCSRFRequired]
121+
#[AnonRateLimit(limit: 10, period: 60)]
122+
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT, tags: ['assistant_api'])]
123+
public function cancelNotifyWhenReady(int $ocpTaskId): DataResponse {
124+
if ($this->userId === null) {
125+
return new DataResponse(['error' => $this->l10n->t('Failed to cancel notification; unknown user')], Http::STATUS_INTERNAL_SERVER_ERROR);
126+
}
127+
128+
try {
129+
$this->assistantService->cancelNotifyWhenReady($ocpTaskId, $this->userId);
130+
} catch (Exception $e) {
131+
// Ignore
132+
}
133+
return new DataResponse('', Http::STATUS_OK);
134+
}
135+
80136
/**
81137
* Get available task types
82138
*

‎lib/Service/AssistantService.php‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Html2Text\Html2Text;
1212
use OC\User\NoUserException;
1313
use OCA\Assistant\AppInfo\Application;
14+
use OCA\Assistant\Db\TaskNotification;
1415
use OCA\Assistant\Db\TaskNotificationMapper;
1516
use OCA\Assistant\ResponseDefinitions;
1617
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
@@ -123,6 +124,34 @@ public function __construct(
123124
];
124125
}
125126

127+
/**
128+
* Get notification request for a task
129+
*
130+
* @param int $taskId
131+
* @param string $userId
132+
* @throws Exception
133+
* @throws MultipleObjectsReturnedException
134+
*/
135+
public function getNotifyWhenReady(int $taskId, string $userId): array {
136+
try {
137+
$task = $this->taskProcessingManager->getTask($taskId);
138+
} catch (NotFoundException $e) {
139+
// task may already be deleted, return an empty notification
140+
return (new TaskNotification())->jsonSerialize();
141+
} catch (TaskProcessingException $e) {
142+
$this->logger->debug('Task request error : ' . $e->getMessage());
143+
throw new Exception('Internal server error.', Http::STATUS_INTERNAL_SERVER_ERROR);
144+
}
145+
146+
if ($task->getUserId() !== $userId) {
147+
$this->logger->info('A user attempted viewing notifications of another user\'s task');
148+
throw new Exception('Unauthorized', Http::STATUS_UNAUTHORIZED);
149+
}
150+
151+
$notification = $this->taskNotificationMapper->getByTaskId($taskId) ?: new TaskNotification();
152+
return $notification->jsonSerialize();
153+
}
154+
126155
/**
127156
* Notify when image generation is ready
128157
*
@@ -155,6 +184,34 @@ public function notifyWhenReady(int $taskId, string $userId): void {
155184
}
156185
}
157186

187+
/**
188+
* Cancel notification when task is finished
189+
*
190+
* @param int $taskId
191+
* @param string $userId
192+
* @throws Exception
193+
* @throws MultipleObjectsReturnedException
194+
*/
195+
public function cancelNotifyWhenReady(int $taskId, string $userId): void {
196+
try {
197+
$task = $this->taskProcessingManager->getTask($taskId);
198+
} catch (NotFoundException $e) {
199+
// task may be already deleted, so delete any dangling notifications
200+
$this->taskNotificationMapper->deleteByTaskId($taskId);
201+
return;
202+
} catch (TaskProcessingException $e) {
203+
$this->logger->debug('Task request error : ' . $e->getMessage());
204+
throw new Exception('Internal server error.', Http::STATUS_INTERNAL_SERVER_ERROR);
205+
}
206+
207+
if ($task->getUserId() !== $userId) {
208+
$this->logger->info('A user attempted deleting notifications of another user\'s task');
209+
throw new Exception('Unauthorized', Http::STATUS_UNAUTHORIZED);
210+
}
211+
212+
$this->taskNotificationMapper->deleteByTaskId($taskId);
213+
}
214+
158215
/**
159216
* @return array<AssistantTaskProcessingTaskType>
160217
*/

0 commit comments

Comments
 (0)