From 3c3694b59f5cba5b81c835965439998410945ad1 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 18 Dec 2024 10:34:39 +0100 Subject: [PATCH 1/6] enh: Integrate with ContextChat Signed-off-by: Marcel Klehr --- lib/AppInfo/Application.php | 6 ++ lib/ContextChat/ContentProvider.php | 97 ++++++++++++++++++++++++++ lib/Controller/ZammadAPIController.php | 11 +++ lib/Service/ZammadAPIService.php | 10 +++ 4 files changed, 124 insertions(+) create mode 100644 lib/ContextChat/ContentProvider.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index a4babaa..e1e5333 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -33,6 +33,8 @@ class Application extends App implements IBootstrap { public const APP_ID = 'integration_zammad'; private IUserConfig $userConfig; + public static $contextChatEnabled = false; + public function __construct(array $urlParams = []) { parent::__construct(self::APP_ID, $urlParams); @@ -49,6 +51,10 @@ public function register(IRegistrationContext $context): void { $context->registerReferenceProvider(ZammadReferenceProvider::class); $context->registerEventListener(RenderReferenceEvent::class, ZammadReferenceListener::class); + if (class_exists('\OCA\ContextChat\Public\IContentProvider')) { + self::$contextChatEnabled = true; + $context->registerEventListener(\OCA\ContextChat\Event\ContentProviderRegisterEvent::class, \OCA\Zammad\ContextChat\ContentProvider::class); + } } public function boot(IBootContext $context): void { diff --git a/lib/ContextChat/ContentProvider.php b/lib/ContextChat/ContentProvider.php new file mode 100644 index 0000000..fdeae48 --- /dev/null +++ b/lib/ContextChat/ContentProvider.php @@ -0,0 +1,97 @@ +registerContentProvider(Application::APP_ID, self::ID, self::class); +} + + /** + * The ID of the provider + * + * @return string + * @since 1.1.0 + */ + public function getId(): string { + return self::ID; + } + + /** + * The ID of the app making the provider avaialble + * + * @return string + * @since 1.1.0 + */ + public function getAppId(): string { + return Application::APP_ID; + } + + /** + * The absolute URL to the content item + * + * @param string $id + * @return string + * @since 1.1.0 + */ + public function getItemUrl(string $id): string { + $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $zammadUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url') ?: $adminZammadOauthUrl; + return $zammadUrl . '/#ticket/zoom/' . $id; + } + + /** + * Starts the initial import of content items into content chat + * + * @return void + * @since 1.1.0 + */ + public function triggerInitialImport(): void { + } + + public function importTicket($id) { + $ticketInfo = $this->zammadAPIService->getTicketInfo($this->userId, (int)$id); + $item = new ContentItem( + (string)$id, + $this->getId(), + $ticketInfo['title'], + $this->getContentOfTicket($id), + 'Ticket', + new \DateTime($ticketInfo['updated_at']), + [$this->userId] + ); + $this->contentManager->updateAccess(Application::APP_ID, self::ID, $id, UpdateAccessOp::ALLOW, [$this->userId]); + $this->contentManager->updateAccessProvider(Application::APP_ID, self::ID, UpdateAccessOp::ALLOW, [$this->userId]); + $this->contentManager->submitContent(Application::APP_ID, [$item]); + } + + public function getContentOfTicket($id): string { + return array_reduce($this->zammadAPIService->getArticlesByTicket($this->userId, (int)$id), fn($agg, array $article) => $agg . $article['from'] . ":\n\n" . $article['body'] . "\n\n", ''); + } + +} \ No newline at end of file diff --git a/lib/Controller/ZammadAPIController.php b/lib/Controller/ZammadAPIController.php index ad9c622..c7075a8 100644 --- a/lib/Controller/ZammadAPIController.php +++ b/lib/Controller/ZammadAPIController.php @@ -85,6 +85,7 @@ public function getNotifications(?string $since = null): DataResponse { return new DataResponse('connection_impossible', Http::STATUS_BAD_REQUEST); } $result = $this->zammadAPIService->getNotifications($this->userId, $since, 7); + $this->importTicketsToContextChat($result); if (!isset($result['error'])) { $response = new DataResponse($result); } else { @@ -93,4 +94,14 @@ public function getNotifications(?string $since = null): DataResponse { return $response; } + private function importTicketsToContextChat(array $notifications): void { + if (!Application::$contextChatEnabled) { + return; + } + $contentProvider = \OCP\Server::get('OCA\Zammad\ContextChat\ContentProvider'); + foreach($notifications as $notification) { + $contentProvider->importTicket($notification['o_id']); + } + } + } diff --git a/lib/Service/ZammadAPIService.php b/lib/Service/ZammadAPIService.php index 1658da0..4d96e7a 100644 --- a/lib/Service/ZammadAPIService.php +++ b/lib/Service/ZammadAPIService.php @@ -441,6 +441,16 @@ public function getOrganizationInfo(string $userId, int $zammadOrgId): array { return $this->request($userId, 'organizations/' . $zammadOrgId); } + /** + * @param string|null $userId + * @param int $ticketId + * @return array + * @throws Exception + */ + public function getArticlesByTicket(?string $userId, int $ticketId): array { + return $this->request($userId, 'ticket_articles/by_ticket/' . $ticketId); + } + /** * @param string $userId * @param string $endPoint From a1c7fd44ea1053f2b5b5b58723219920acba552e Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 18 Dec 2024 10:44:46 +0100 Subject: [PATCH 2/6] fix: run cs:fix Signed-off-by: Marcel Klehr --- lib/ContextChat/ContentProvider.php | 6 +++--- lib/Controller/ZammadAPIController.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ContextChat/ContentProvider.php b/lib/ContextChat/ContentProvider.php index fdeae48..4ddb78e 100644 --- a/lib/ContextChat/ContentProvider.php +++ b/lib/ContextChat/ContentProvider.php @@ -30,7 +30,7 @@ public function handle(Event $event): void { return; } $event->registerContentProvider(Application::APP_ID, self::ID, self::class); -} + } /** * The ID of the provider @@ -91,7 +91,7 @@ public function importTicket($id) { } public function getContentOfTicket($id): string { - return array_reduce($this->zammadAPIService->getArticlesByTicket($this->userId, (int)$id), fn($agg, array $article) => $agg . $article['from'] . ":\n\n" . $article['body'] . "\n\n", ''); + return array_reduce($this->zammadAPIService->getArticlesByTicket($this->userId, (int)$id), fn ($agg, array $article) => $agg . $article['from'] . ":\n\n" . $article['body'] . "\n\n", ''); } -} \ No newline at end of file +} diff --git a/lib/Controller/ZammadAPIController.php b/lib/Controller/ZammadAPIController.php index c7075a8..22560b1 100644 --- a/lib/Controller/ZammadAPIController.php +++ b/lib/Controller/ZammadAPIController.php @@ -99,7 +99,7 @@ private function importTicketsToContextChat(array $notifications): void { return; } $contentProvider = \OCP\Server::get('OCA\Zammad\ContextChat\ContentProvider'); - foreach($notifications as $notification) { + foreach ($notifications as $notification) { $contentProvider->importTicket($notification['o_id']); } } From 544c3fb16478a614b9744449e70422d85a2ad34c Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 18 Dec 2024 10:47:39 +0100 Subject: [PATCH 3/6] fix: update baseline Signed-off-by: Marcel Klehr --- tests/psalm-baseline.xml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index a9b7140..be34566 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,2 +1,16 @@ - + + + + + + + + + + + + + + + From 4d412ea383078bee650317c11b5cb9ee294a4c51 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 15 Sep 2026 13:07:43 +0200 Subject: [PATCH 4/6] fixes Signed-off-by: Marcel Klehr --- lib/AppInfo/Application.php | 16 ++++-- lib/ContextChat/ContentProvider.php | 70 ++++++++++++-------------- lib/Controller/ConfigController.php | 34 +++++++++++++ lib/Controller/ZammadAPIController.php | 42 ++++++++++++++-- lib/Service/ZammadAPIService.php | 28 ++++++++++- tests/psalm-baseline.xml | 16 +----- 6 files changed, 143 insertions(+), 63 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index e1e5333..6fd155a 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -11,6 +11,7 @@ namespace OCA\Zammad\AppInfo; use Closure; +use OCA\Zammad\ContextChat\ContentProvider; use OCA\Zammad\Dashboard\ZammadWidget; use OCA\Zammad\Listener\ZammadReferenceListener; use OCA\Zammad\Notification\Notifier; @@ -22,6 +23,8 @@ use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\Collaboration\Reference\RenderReferenceEvent; use OCP\Config\IUserConfig; +use OCP\ContextChat\Events\ContentProviderRegisterEvent; +use OCP\ContextChat\IContentProvider; use OCP\IL10N; use OCP\INavigationManager; use OCP\IURLGenerator; @@ -33,7 +36,10 @@ class Application extends App implements IBootstrap { public const APP_ID = 'integration_zammad'; private IUserConfig $userConfig; - public static $contextChatEnabled = false; + /** + * Whether the server provides the ContextChat API, see register() + */ + public static bool $contextChatEnabled = false; public function __construct(array $urlParams = []) { parent::__construct(self::APP_ID, $urlParams); @@ -51,9 +57,13 @@ public function register(IRegistrationContext $context): void { $context->registerReferenceProvider(ZammadReferenceProvider::class); $context->registerEventListener(RenderReferenceEvent::class, ZammadReferenceListener::class); - if (class_exists('\OCA\ContextChat\Public\IContentProvider')) { + // the ContextChat API in OCP only exists since Nextcloud 32 + if (interface_exists(IContentProvider::class)) { self::$contextChatEnabled = true; - $context->registerEventListener(\OCA\ContextChat\Event\ContentProviderRegisterEvent::class, \OCA\Zammad\ContextChat\ContentProvider::class); + $context->registerEventListener(ContentProviderRegisterEvent::class, ContentProvider::class); + // context_chat dispatches its own subclass of the event and the dispatcher matches + // the exact class name, so the app specific event has to be listened for as well + $context->registerEventListener('OCA\ContextChat\Event\ContentProviderRegisterEvent', ContentProvider::class); } } diff --git a/lib/ContextChat/ContentProvider.php b/lib/ContextChat/ContentProvider.php index 4ddb78e..cbdd1ef 100644 --- a/lib/ContextChat/ContentProvider.php +++ b/lib/ContextChat/ContentProvider.php @@ -1,30 +1,38 @@ + */ +class ContentProvider implements IContentProvider, IEventListener { + + /** + * Provider IDs must not contain colons, double underscores or spaces + */ + public const ID = 'tickets'; public function __construct( private IConfig $config, - private ZammadAPIService $zammadAPIService, + private TicketImportService $importService, private ?string $userId, - private ContentManager $contentManager, ) { - } - public const ID = 'integration_zammad:tickets'; - public function handle(Event $event): void { if (!$event instanceof ContentProviderRegisterEvent) { return; @@ -36,7 +44,7 @@ public function handle(Event $event): void { * The ID of the provider * * @return string - * @since 1.1.0 + * @since 32.0.0 */ public function getId(): string { return self::ID; @@ -46,7 +54,7 @@ public function getId(): string { * The ID of the app making the provider avaialble * * @return string - * @since 1.1.0 + * @since 32.0.0 */ public function getAppId(): string { return Application::APP_ID; @@ -57,11 +65,15 @@ public function getAppId(): string { * * @param string $id * @return string - * @since 1.1.0 + * @since 32.0.0 */ public function getItemUrl(string $id): string { - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); - $zammadUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url') ?: $adminZammadOauthUrl; + // this is called outside of a user session as well, + // the admin configured instance is the only thing available then + $zammadUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + if ($this->userId !== null) { + $zammadUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url') ?: $zammadUrl; + } return $zammadUrl . '/#ticket/zoom/' . $id; } @@ -69,29 +81,9 @@ public function getItemUrl(string $id): string { * Starts the initial import of content items into content chat * * @return void - * @since 1.1.0 + * @since 32.0.0 */ public function triggerInitialImport(): void { + $this->importService->scheduleForAllUsers(); } - - public function importTicket($id) { - $ticketInfo = $this->zammadAPIService->getTicketInfo($this->userId, (int)$id); - $item = new ContentItem( - (string)$id, - $this->getId(), - $ticketInfo['title'], - $this->getContentOfTicket($id), - 'Ticket', - new \DateTime($ticketInfo['updated_at']), - [$this->userId] - ); - $this->contentManager->updateAccess(Application::APP_ID, self::ID, $id, UpdateAccessOp::ALLOW, [$this->userId]); - $this->contentManager->updateAccessProvider(Application::APP_ID, self::ID, UpdateAccessOp::ALLOW, [$this->userId]); - $this->contentManager->submitContent(Application::APP_ID, [$item]); - } - - public function getContentOfTicket($id): string { - return array_reduce($this->zammadAPIService->getArticlesByTicket($this->userId, (int)$id), fn ($agg, array $article) => $agg . $article['from'] . ":\n\n" . $article['body'] . "\n\n", ''); - } - } diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index bebe7fb..f928ed8 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -14,6 +14,7 @@ use DateTime; use OCA\Zammad\AppInfo\Application; +use OCA\Zammad\ContextChat\TicketImportService; use OCA\Zammad\Reference\ZammadReferenceProvider; use OCA\Zammad\Service\ZammadAPIService; use OCP\AppFramework\Controller; @@ -30,6 +31,9 @@ use OCP\IURLGenerator; use OCP\PreConditionNotMetException; use OCP\Security\ICrypto; +use OCP\Server; +use Psr\Log\LoggerInterface; +use Throwable; class ConfigController extends Controller { @@ -43,6 +47,7 @@ public function __construct( private ICrypto $crypto, private ZammadAPIService $zammadAPIService, private ZammadReferenceProvider $zammadReferenceProvider, + private LoggerInterface $logger, private ?string $userId, ) { parent::__construct($appName, $request); @@ -91,7 +96,9 @@ public function setSensitiveConfig(array $values): DataResponse { if (isset($values['token'])) { if ($values['token'] && $values['token'] !== '') { $result = $this->storeUserInfo(); + $this->updateContextChatSchedule(true); } else { + $this->updateContextChatSchedule(false); $this->userConfig->deleteUserConfig($this->userId, Application::APP_ID, 'user_id'); $this->userConfig->deleteUserConfig($this->userId, Application::APP_ID, 'user_name'); $this->userConfig->deleteUserConfig($this->userId, Application::APP_ID, 'last_open_check'); @@ -189,6 +196,7 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe } // get user info $this->storeUserInfo(); + $this->updateContextChatSchedule(true); return new RedirectResponse( $this->urlGenerator->linkToRoute('settings.PersonalSettings.index', ['section' => 'connected-accounts']) . '?zammadToken=success' @@ -204,6 +212,32 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe ); } + /** + * Schedule or unschedule the ContextChat ticket import for the current user. + * + * @param bool $connected whether the user just connected or disconnected their Zammad account + * @return void + */ + private function updateContextChatSchedule(bool $connected): void { + if (!Application::$contextChatEnabled || $this->userId === null) { + return; + } + try { + // resolved lazily, the service depends on classes shipped by context_chat + $importService = Server::get(TicketImportService::class); + if ($connected) { + $importService->scheduleForUser($this->userId); + } else { + $importService->unscheduleForUser($this->userId); + } + } catch (Throwable $e) { + $this->logger->warning('Could not update the Zammad ContextChat import schedule: ' . $e->getMessage(), [ + 'app' => Application::APP_ID, + 'exception' => $e, + ]); + } + } + /** * @return array * @throws PreConditionNotMetException diff --git a/lib/Controller/ZammadAPIController.php b/lib/Controller/ZammadAPIController.php index 22560b1..df5968c 100644 --- a/lib/Controller/ZammadAPIController.php +++ b/lib/Controller/ZammadAPIController.php @@ -13,6 +13,7 @@ namespace OCA\Zammad\Controller; use OCA\Zammad\AppInfo\Application; +use OCA\Zammad\ContextChat\TicketImportService; use OCA\Zammad\Service\ZammadAPIService; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -23,6 +24,9 @@ use OCP\Config\IUserConfig; use OCP\IRequest; use OCP\PreConditionNotMetException; +use OCP\Server; +use Psr\Log\LoggerInterface; +use Throwable; class ZammadAPIController extends Controller { @@ -31,6 +35,7 @@ public function __construct( IRequest $request, private IUserConfig $userConfig, private ZammadAPIService $zammadAPIService, + private LoggerInterface $logger, private ?string $userId, ) { parent::__construct($appName, $request); @@ -85,8 +90,8 @@ public function getNotifications(?string $since = null): DataResponse { return new DataResponse('connection_impossible', Http::STATUS_BAD_REQUEST); } $result = $this->zammadAPIService->getNotifications($this->userId, $since, 7); - $this->importTicketsToContextChat($result); if (!isset($result['error'])) { + $this->importTicketsToContextChat($result); $response = new DataResponse($result); } else { $response = new DataResponse($result, Http::STATUS_UNAUTHORIZED); @@ -94,13 +99,42 @@ public function getNotifications(?string $since = null): DataResponse { return $response; } + /** + * Push the tickets the user was just notified about to ContextChat. + * The background job imports them as well, this only makes them available sooner. + * + * @param array $notifications a successful result of ZammadAPIService::getNotifications() + * @return void + */ private function importTicketsToContextChat(array $notifications): void { - if (!Application::$contextChatEnabled) { + if (!Application::$contextChatEnabled || $this->userId === null) { + return; + } + try { + $importService = Server::get(TicketImportService::class); + } catch (Throwable $e) { + $this->logger->warning('Could not load the Zammad ContextChat import service: ' . $e->getMessage(), [ + 'app' => Application::APP_ID, + 'exception' => $e, + ]); + return; + } + if (!$importService->isAvailable()) { return; } - $contentProvider = \OCP\Server::get('OCA\Zammad\ContextChat\ContentProvider'); foreach ($notifications as $notification) { - $contentProvider->importTicket($notification['o_id']); + if (!isset($notification['o_id'])) { + continue; + } + try { + $importService->importTicketById($this->userId, (int)$notification['o_id']); + } catch (Throwable $e) { + // never let the ContextChat import break the dashboard widget + $this->logger->warning('Could not import Zammad ticket ' . $notification['o_id'] . ' into ContextChat: ' . $e->getMessage(), [ + 'app' => Application::APP_ID, + 'exception' => $e, + ]); + } } } diff --git a/lib/Service/ZammadAPIService.php b/lib/Service/ZammadAPIService.php index 4d96e7a..211b436 100644 --- a/lib/Service/ZammadAPIService.php +++ b/lib/Service/ZammadAPIService.php @@ -34,6 +34,11 @@ use Psr\Log\LoggerInterface; class ZammadAPIService { + /** + * Hard limit Zammad enforces on the per_page parameter of the ticket list endpoint + */ + public const TICKET_PAGE_MAX_SIZE = 100; + private ICache $cache; private IClient $client; @@ -442,15 +447,34 @@ public function getOrganizationInfo(string $userId, int $zammadOrgId): array { } /** - * @param string|null $userId + * @param string $userId * @param int $ticketId * @return array * @throws Exception */ - public function getArticlesByTicket(?string $userId, int $ticketId): array { + public function getArticlesByTicket(string $userId, int $ticketId): array { return $this->request($userId, 'ticket_articles/by_ticket/' . $ticketId); } + /** + * List the tickets the user has access to, one page at a time. + * Zammad scopes this endpoint to the tickets the token owner may read and + * always orders it by ticket ID ascending, which makes paging through it stable. + * Its hard limit is 100 tickets per page. + * + * @param string $userId + * @param int $page page number, starting at 1 + * @param int $perPage number of tickets per page, at most 100 + * @return array the list of tickets or an array with an 'error' key + * @throws Exception + */ + public function getTickets(string $userId, int $page, int $perPage): array { + return $this->request($userId, 'tickets', [ + 'page' => $page, + 'per_page' => min($perPage, self::TICKET_PAGE_MAX_SIZE), + ]); + } + /** * @param string $userId * @param string $endPoint diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index be34566..a9b7140 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,16 +1,2 @@ - - - - - - - - - - - - - - - + From 8a9d7d04f6762fd6b012205eedc37e26990af45a Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 15 Sep 2026 13:16:29 +0200 Subject: [PATCH 5/6] fixes Signed-off-by: Marcel Klehr --- lib/AppInfo/Application.php | 18 +- lib/BackgroundJob/ImportTicketsJob.php | 74 ++++++ lib/ContextChat/ContentProvider.php | 10 +- lib/ContextChat/TicketImportService.php | 298 ++++++++++++++++++++++++ lib/Controller/ConfigController.php | 12 +- lib/Controller/ZammadAPIController.php | 17 +- 6 files changed, 391 insertions(+), 38 deletions(-) create mode 100644 lib/BackgroundJob/ImportTicketsJob.php create mode 100644 lib/ContextChat/TicketImportService.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 6fd155a..7f43739 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -24,7 +24,6 @@ use OCP\Collaboration\Reference\RenderReferenceEvent; use OCP\Config\IUserConfig; use OCP\ContextChat\Events\ContentProviderRegisterEvent; -use OCP\ContextChat\IContentProvider; use OCP\IL10N; use OCP\INavigationManager; use OCP\IURLGenerator; @@ -36,11 +35,6 @@ class Application extends App implements IBootstrap { public const APP_ID = 'integration_zammad'; private IUserConfig $userConfig; - /** - * Whether the server provides the ContextChat API, see register() - */ - public static bool $contextChatEnabled = false; - public function __construct(array $urlParams = []) { parent::__construct(self::APP_ID, $urlParams); @@ -57,14 +51,10 @@ public function register(IRegistrationContext $context): void { $context->registerReferenceProvider(ZammadReferenceProvider::class); $context->registerEventListener(RenderReferenceEvent::class, ZammadReferenceListener::class); - // the ContextChat API in OCP only exists since Nextcloud 32 - if (interface_exists(IContentProvider::class)) { - self::$contextChatEnabled = true; - $context->registerEventListener(ContentProviderRegisterEvent::class, ContentProvider::class); - // context_chat dispatches its own subclass of the event and the dispatcher matches - // the exact class name, so the app specific event has to be listened for as well - $context->registerEventListener('OCA\ContextChat\Event\ContentProviderRegisterEvent', ContentProvider::class); - } + $context->registerEventListener(ContentProviderRegisterEvent::class, ContentProvider::class); + // context_chat dispatches its own subclass of the event and the dispatcher matches + // the exact class name, so the app specific event has to be listened for as well + $context->registerEventListener('OCA\ContextChat\Event\ContentProviderRegisterEvent', ContentProvider::class); } public function boot(IBootContext $context): void { diff --git a/lib/BackgroundJob/ImportTicketsJob.php b/lib/BackgroundJob/ImportTicketsJob.php new file mode 100644 index 0000000..22b5907 --- /dev/null +++ b/lib/BackgroundJob/ImportTicketsJob.php @@ -0,0 +1,74 @@ +setInterval(60 * 5); + $this->setTimeSensitivity(self::TIME_INSENSITIVE); + } + + protected function run($argument): void { + if (!is_array($argument) || !isset($argument['user_id']) || !is_string($argument['user_id'])) { + $this->logger->warning('Zammad ticket import job started without a user, removing it.', ['app' => Application::APP_ID]); + $this->jobList->remove(self::class, $argument); + return; + } + $userId = $argument['user_id']; + + if (!$this->importService->isAvailable()) { + // context_chat is not installed, keep the job around for when it is + return; + } + + if ($this->userManager->get($userId) === null || !$this->importService->hasToken($userId)) { + $this->logger->debug('Zammad account of ' . $userId . ' is gone, unscheduling the ticket import.', ['app' => Application::APP_ID]); + $this->importService->unscheduleForUser($userId); + return; + } + + try { + $this->importService->importChunk($userId); + } catch (Throwable $e) { + // keep the job scheduled, the chunk is retried on the next run + $this->logger->warning('Zammad ticket import failed: ' . $e->getMessage(), [ + 'app' => Application::APP_ID, + 'userId' => $userId, + 'exception' => $e, + ]); + } + } +} diff --git a/lib/ContextChat/ContentProvider.php b/lib/ContextChat/ContentProvider.php index cbdd1ef..154d353 100644 --- a/lib/ContextChat/ContentProvider.php +++ b/lib/ContextChat/ContentProvider.php @@ -10,11 +10,12 @@ namespace OCA\Zammad\ContextChat; use OCA\Zammad\AppInfo\Application; +use OCP\Config\IUserConfig; use OCP\ContextChat\Events\ContentProviderRegisterEvent; use OCP\ContextChat\IContentProvider; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -use OCP\IConfig; +use OCP\IAppConfig; /** * @template-implements IEventListener @@ -27,7 +28,8 @@ class ContentProvider implements IContentProvider, IEventListener { public const ID = 'tickets'; public function __construct( - private IConfig $config, + private IAppConfig $appConfig, + private IUserConfig $userConfig, private TicketImportService $importService, private ?string $userId, ) { @@ -70,9 +72,9 @@ public function getAppId(): string { public function getItemUrl(string $id): string { // this is called outside of a user session as well, // the admin configured instance is the only thing available then - $zammadUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $zammadUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url'); if ($this->userId !== null) { - $zammadUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url') ?: $zammadUrl; + $zammadUrl = $this->userConfig->getValueString($this->userId, Application::APP_ID, 'url') ?: $zammadUrl; } return $zammadUrl . '/#ticket/zoom/' . $id; } diff --git a/lib/ContextChat/TicketImportService.php b/lib/ContextChat/TicketImportService.php new file mode 100644 index 0000000..cc36598 --- /dev/null +++ b/lib/ContextChat/TicketImportService.php @@ -0,0 +1,298 @@ +userManager->callForSeenUsers(function (IUser $user): void { + if ($this->hasToken($user->getUID())) { + $this->scheduleForUser($user->getUID()); + } + }); + } + + /** + * @param string $userId + * @return void + */ + public function scheduleForUser(string $userId): void { + $argument = self::jobArgument($userId); + if (!$this->jobList->has(ImportTicketsJob::class, $argument)) { + $this->jobList->add(ImportTicketsJob::class, $argument); + } + } + + /** + * @param string $userId + * @return void + */ + public function unscheduleForUser(string $userId): void { + $this->jobList->remove(ImportTicketsJob::class, self::jobArgument($userId)); + foreach ([self::CONFIG_PAGE, self::CONFIG_SINCE, self::CONFIG_MAX, self::CONFIG_FAILED] as $key) { + $this->userConfig->deleteUserConfig($userId, Application::APP_ID, $key); + } + } + + /** + * @param string $userId + * @return array{user_id: string} + */ + public static function jobArgument(string $userId): array { + return ['user_id' => $userId]; + } + + /** + * Whether context_chat is installed and can take content. + * + * @return bool + */ + public function isAvailable(): bool { + return $this->contentManager->isContextChatAvailable(); + } + + /** + * @param string $userId + * @return bool + */ + public function hasToken(string $userId): bool { + return $this->userConfig->getValueString($userId, Application::APP_ID, 'token', lazy: true) !== ''; + } + + /** + * Import the next chunk of the user's tickets. + * + * Only tickets that were modified since the last completed sweep are sent to + * ContextChat, so repeated sweeps cost one ticket list request per chunk as + * long as nothing changed. + * + * @param string $userId + * @return void + * @throws Exception + */ + public function importChunk(string $userId): void { + $page = max(1, $this->userConfig->getValueInt($userId, Application::APP_ID, self::CONFIG_PAGE, 1, lazy: true)); + $tickets = $this->zammadAPIService->getTickets($userId, $page, self::CHUNK_SIZE); + if (isset($tickets['error'])) { + // leave the sweep state untouched, the same page is retried on the next run + $this->logger->warning( + 'Zammad API error: could not list tickets for the ContextChat import. ' . $tickets['error'], + ['app' => Application::APP_ID, 'userId' => $userId] + ); + return; + } + + $sinceTs = $this->getTimestamp($userId, self::CONFIG_SINCE); + $maxTs = $this->getTimestamp($userId, self::CONFIG_MAX); + $failedTs = $this->getTimestamp($userId, self::CONFIG_FAILED); + + foreach ($tickets as $ticket) { + if (!is_array($ticket) || !isset($ticket['id'], $ticket['title'], $ticket['updated_at'])) { + continue; + } + $ticketTs = $this->parseTimestamp((string)$ticket['updated_at']); + $maxTs = max($maxTs, $ticketTs); + if ($ticketTs > 0 && $ticketTs <= $sinceTs) { + // unchanged since the last completed sweep + continue; + } + try { + $this->importTicket($userId, $ticket); + } catch (Throwable $e) { + $this->logger->warning( + 'Could not import Zammad ticket ' . $ticket['id'] . ' into ContextChat: ' . $e->getMessage(), + ['app' => Application::APP_ID, 'userId' => $userId, 'exception' => $e] + ); + $failedTs = $failedTs === 0 ? $ticketTs : min($failedTs, $ticketTs); + } + } + + if (count($tickets) >= self::CHUNK_SIZE) { + $this->setTimestamp($userId, self::CONFIG_MAX, $maxTs); + $this->setTimestamp($userId, self::CONFIG_FAILED, $failedTs); + $this->userConfig->setValueInt($userId, Application::APP_ID, self::CONFIG_PAGE, $page + 1, lazy: true); + return; + } + + // the sweep is done, start over from the first page on the next run. + // tickets that failed to import must be picked up again, so the watermark + // never moves past the oldest failure of this sweep. + $watermark = $failedTs > 0 ? min($maxTs, $failedTs - 1) : $maxTs; + $this->setTimestamp($userId, self::CONFIG_SINCE, $watermark); + $this->userConfig->deleteUserConfig($userId, Application::APP_ID, self::CONFIG_MAX); + $this->userConfig->deleteUserConfig($userId, Application::APP_ID, self::CONFIG_FAILED); + $this->userConfig->setValueInt($userId, Application::APP_ID, self::CONFIG_PAGE, 1, lazy: true); + } + + /** + * @param string $userId + * @param int $ticketId + * @return void + * @throws Exception + */ + public function importTicketById(string $userId, int $ticketId): void { + $ticket = $this->zammadAPIService->getTicketInfo($userId, $ticketId); + if (isset($ticket['error'])) { + throw new RuntimeException('Could not get ticket information: ' . $ticket['error']); + } + if (!isset($ticket['id'], $ticket['title'], $ticket['updated_at'])) { + throw new RuntimeException('Unexpected ticket information for ticket ' . $ticketId); + } + $this->importTicket($userId, $ticket); + } + + /** + * @param string $userId + * @param array $ticket a ticket as returned by the Zammad API + * @return void + * @throws Exception + */ + public function importTicket(string $userId, array $ticket): void { + $itemId = (string)$ticket['id']; + $item = new ContentItem( + $itemId, + ContentProvider::ID, + (string)$ticket['title'], + $this->getTicketContent($userId, (int)$ticket['id']), + 'Ticket', + $this->parseDateTime((string)$ticket['updated_at']), + [$userId], + ); + $this->contentManager->submitContent(Application::APP_ID, [$item]); + // a ticket can be visible to several Nextcloud users and the item ID is the + // same for all of them, so grant access additively instead of replacing it + $this->contentManager->updateAccess( + Application::APP_ID, ContentProvider::ID, $itemId, UpdateAccessOp::ALLOW, [$userId] + ); + } + + /** + * @param string $userId + * @param int $ticketId + * @return string + * @throws Exception + */ + public function getTicketContent(string $userId, int $ticketId): string { + $articles = $this->zammadAPIService->getArticlesByTicket($userId, $ticketId); + if (isset($articles['error'])) { + throw new RuntimeException('Could not get ticket articles: ' . $articles['error']); + } + $content = ''; + foreach ($articles as $article) { + if (!is_array($article)) { + continue; + } + $body = (string)($article['body'] ?? ''); + if (($article['content_type'] ?? '') === 'text/html') { + $body = html_entity_decode(strip_tags($body), ENT_QUOTES | ENT_HTML5, 'UTF-8'); + } + $from = trim((string)($article['from'] ?? '')); + $content .= ($from === '' ? '' : $from . ":\n\n") . $body . "\n\n"; + } + return $content; + } + + /** + * @param string $date + * @return DateTime + * @throws Exception + */ + private function parseDateTime(string $date): DateTime { + try { + return new DateTime($date); + } catch (Exception $e) { + return new DateTime('@0'); + } + } + + /** + * @param string $date + * @return int the unix timestamp or 0 if the date could not be parsed + */ + private function parseTimestamp(string $date): int { + try { + return (new DateTime($date))->getTimestamp(); + } catch (Exception $e) { + return 0; + } + } + + /** + * @param string $userId + * @param string $key + * @return int + */ + private function getTimestamp(string $userId, string $key): int { + return max(0, $this->userConfig->getValueInt($userId, Application::APP_ID, $key, 0, lazy: true)); + } + + /** + * @param string $userId + * @param string $key + * @param int $timestamp + * @return void + */ + private function setTimestamp(string $userId, string $key, int $timestamp): void { + $this->userConfig->setValueInt($userId, Application::APP_ID, $key, $timestamp, lazy: true); + } +} diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index f928ed8..a132b0b 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -31,7 +31,6 @@ use OCP\IURLGenerator; use OCP\PreConditionNotMetException; use OCP\Security\ICrypto; -use OCP\Server; use Psr\Log\LoggerInterface; use Throwable; @@ -47,6 +46,7 @@ public function __construct( private ICrypto $crypto, private ZammadAPIService $zammadAPIService, private ZammadReferenceProvider $zammadReferenceProvider, + private TicketImportService $importService, private LoggerInterface $logger, private ?string $userId, ) { @@ -190,7 +190,7 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe $refreshToken = $result['refresh_token']; $this->userConfig->setValueString($this->userId, Application::APP_ID, 'refresh_token', $refreshToken, lazy: true, flags: IUserConfig::FLAG_SENSITIVE); if (isset($result['expires_in'])) { - $nowTs = (new Datetime())->getTimestamp(); + $nowTs = (new DateTime())->getTimestamp(); $expiresAt = $nowTs + (int)$result['expires_in']; $this->userConfig->setValueString($this->userId, Application::APP_ID, 'token_expires_at', (string)$expiresAt, lazy: true); } @@ -219,16 +219,14 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe * @return void */ private function updateContextChatSchedule(bool $connected): void { - if (!Application::$contextChatEnabled || $this->userId === null) { + if ($this->userId === null) { return; } try { - // resolved lazily, the service depends on classes shipped by context_chat - $importService = Server::get(TicketImportService::class); if ($connected) { - $importService->scheduleForUser($this->userId); + $this->importService->scheduleForUser($this->userId); } else { - $importService->unscheduleForUser($this->userId); + $this->importService->unscheduleForUser($this->userId); } } catch (Throwable $e) { $this->logger->warning('Could not update the Zammad ContextChat import schedule: ' . $e->getMessage(), [ diff --git a/lib/Controller/ZammadAPIController.php b/lib/Controller/ZammadAPIController.php index df5968c..b7060d1 100644 --- a/lib/Controller/ZammadAPIController.php +++ b/lib/Controller/ZammadAPIController.php @@ -24,7 +24,6 @@ use OCP\Config\IUserConfig; use OCP\IRequest; use OCP\PreConditionNotMetException; -use OCP\Server; use Psr\Log\LoggerInterface; use Throwable; @@ -35,6 +34,7 @@ public function __construct( IRequest $request, private IUserConfig $userConfig, private ZammadAPIService $zammadAPIService, + private TicketImportService $importService, private LoggerInterface $logger, private ?string $userId, ) { @@ -107,19 +107,10 @@ public function getNotifications(?string $since = null): DataResponse { * @return void */ private function importTicketsToContextChat(array $notifications): void { - if (!Application::$contextChatEnabled || $this->userId === null) { + if ($this->userId === null) { return; } - try { - $importService = Server::get(TicketImportService::class); - } catch (Throwable $e) { - $this->logger->warning('Could not load the Zammad ContextChat import service: ' . $e->getMessage(), [ - 'app' => Application::APP_ID, - 'exception' => $e, - ]); - return; - } - if (!$importService->isAvailable()) { + if (!$this->importService->isAvailable()) { return; } foreach ($notifications as $notification) { @@ -127,7 +118,7 @@ private function importTicketsToContextChat(array $notifications): void { continue; } try { - $importService->importTicketById($this->userId, (int)$notification['o_id']); + $this->importService->importTicketById($this->userId, (int)$notification['o_id']); } catch (Throwable $e) { // never let the ContextChat import break the dashboard widget $this->logger->warning('Could not import Zammad ticket ' . $notification['o_id'] . ' into ContextChat: ' . $e->getMessage(), [ From 1c59901a4c6bd71d8cad21a630bfcc93c0afb524 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 15 Sep 2026 14:20:03 +0200 Subject: [PATCH 6/6] fix: Make sure deletions are caught Signed-off-by: Marcel Klehr --- appinfo/info.xml | 2 +- lib/ContextChat/TicketImportService.php | 192 +++++++++++++++++- lib/Db/ImportedTicketMapper.php | 184 +++++++++++++++++ .../Version040200Date20260915094500.php | 69 +++++++ lib/Service/ZammadAPIService.php | 6 +- tests/stub.phpstub | 22 ++ 6 files changed, 468 insertions(+), 7 deletions(-) create mode 100644 lib/Db/ImportedTicketMapper.php create mode 100644 lib/Migration/Version040200Date20260915094500.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 5af4179..199cbeb 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,7 +5,7 @@ Integration of Zammad user support/ticketing solution - 4.1.1 + 4.2.0-beta.1 agpl Julien Veyssier Zammad diff --git a/lib/ContextChat/TicketImportService.php b/lib/ContextChat/TicketImportService.php index cc36598..9f7edca 100644 --- a/lib/ContextChat/TicketImportService.php +++ b/lib/ContextChat/TicketImportService.php @@ -13,7 +13,9 @@ use Exception; use OCA\Zammad\AppInfo\Application; use OCA\Zammad\BackgroundJob\ImportTicketsJob; +use OCA\Zammad\Db\ImportedTicketMapper; use OCA\Zammad\Service\ZammadAPIService; +use OCP\AppFramework\Http; use OCP\BackgroundJob\IJobList; use OCP\Config\IUserConfig; use OCP\ContextChat\ContentItem; @@ -31,6 +33,12 @@ * Every connected user gets their own background job, which walks through the * tickets that user can see one chunk per run and starts over once it reaches * the end, so that ticket updates keep flowing into ContextChat. + * + * Zammad has no way of telling us that a ticket was deleted or that a user lost + * access to it, but its ticket list only ever contains the tickets a user may + * read. So once a sweep has walked the whole list, every ticket we imported for + * that user that the sweep did not come across has disappeared. Those tickets are + * checked one by one and then revoked, see {@see self::cleanupChunk()}. */ class TicketImportService { @@ -40,6 +48,12 @@ class TicketImportService { */ public const CHUNK_SIZE = 50; + /** + * Number of disappeared tickets verified per background job run. + * Each one costs a Zammad API request. + */ + public const CLEANUP_CHUNK_SIZE = 20; + /** Next page of the ticket list to fetch */ private const CONFIG_PAGE = 'cc_sweep_page'; /** Tickets modified at or before this timestamp are in sync (last completed sweep) */ @@ -48,6 +62,22 @@ class TicketImportService { private const CONFIG_MAX = 'cc_sweep_max'; /** Lowest ticket modification timestamp that failed to import during the current sweep */ private const CONFIG_FAILED = 'cc_sweep_failed'; + /** Number of the sweep that is currently running, used to tell apart the tickets it has seen */ + private const CONFIG_GENERATION = 'cc_sweep_generation'; + /** Whether the sweep is done and the tickets it did not see still have to be looked at */ + private const CONFIG_CLEANUP = 'cc_cleanup'; + /** Highest ticket ID the cleanup of the current sweep has already looked at */ + private const CONFIG_CLEANUP_CURSOR = 'cc_cleanup_cursor'; + + private const CONFIG_KEYS = [ + self::CONFIG_PAGE, + self::CONFIG_SINCE, + self::CONFIG_MAX, + self::CONFIG_FAILED, + self::CONFIG_GENERATION, + self::CONFIG_CLEANUP, + self::CONFIG_CLEANUP_CURSOR, + ]; public function __construct( private IUserConfig $userConfig, @@ -55,6 +85,7 @@ public function __construct( private IJobList $jobList, private ZammadAPIService $zammadAPIService, private IContentManager $contentManager, + private ImportedTicketMapper $importedTicketMapper, private LoggerInterface $logger, ) { } @@ -84,14 +115,18 @@ public function scheduleForUser(string $userId): void { } /** + * Stop importing for a user and take away everything we imported for them, + * their tickets are not accessible to them through Nextcloud any more. + * * @param string $userId * @return void */ public function unscheduleForUser(string $userId): void { $this->jobList->remove(ImportTicketsJob::class, self::jobArgument($userId)); - foreach ([self::CONFIG_PAGE, self::CONFIG_SINCE, self::CONFIG_MAX, self::CONFIG_FAILED] as $key) { + foreach (self::CONFIG_KEYS as $key) { $this->userConfig->deleteUserConfig($userId, Application::APP_ID, $key); } + $this->revokeAllAccess($userId); } /** @@ -120,7 +155,8 @@ public function hasToken(string $userId): bool { } /** - * Import the next chunk of the user's tickets. + * Import the next chunk of the user's tickets, or, once the sweep has walked + * the whole ticket list, look at the next chunk of the tickets it did not see. * * Only tickets that were modified since the last completed sweep are sent to * ContextChat, so repeated sweeps cost one ticket list request per chunk as @@ -131,6 +167,11 @@ public function hasToken(string $userId): bool { * @throws Exception */ public function importChunk(string $userId): void { + if ($this->userConfig->getValueBool($userId, Application::APP_ID, self::CONFIG_CLEANUP, false, lazy: true)) { + $this->cleanupChunk($userId); + return; + } + $page = max(1, $this->userConfig->getValueInt($userId, Application::APP_ID, self::CONFIG_PAGE, 1, lazy: true)); $tickets = $this->zammadAPIService->getTickets($userId, $page, self::CHUNK_SIZE); if (isset($tickets['error'])) { @@ -142,10 +183,21 @@ public function importChunk(string $userId): void { return; } + $generation = $this->getGeneration($userId); $sinceTs = $this->getTimestamp($userId, self::CONFIG_SINCE); $maxTs = $this->getTimestamp($userId, self::CONFIG_MAX); $failedTs = $this->getTimestamp($userId, self::CONFIG_FAILED); + // record the whole page as still accessible before importing any of it, a + // ticket we fail to import must not end up looking like it has disappeared + $seenIds = []; + foreach ($tickets as $ticket) { + if (is_array($ticket) && isset($ticket['id'])) { + $seenIds[] = (int)$ticket['id']; + } + } + $this->importedTicketMapper->markSeen($userId, $seenIds, $generation); + foreach ($tickets as $ticket) { if (!is_array($ticket) || !isset($ticket['id'], $ticket['title'], $ticket['updated_at'])) { continue; @@ -174,7 +226,7 @@ public function importChunk(string $userId): void { return; } - // the sweep is done, start over from the first page on the next run. + // the sweep has reached the end of the ticket list. // tickets that failed to import must be picked up again, so the watermark // never moves past the oldest failure of this sweep. $watermark = $failedTs > 0 ? min($maxTs, $failedTs - 1) : $maxTs; @@ -182,6 +234,129 @@ public function importChunk(string $userId): void { $this->userConfig->deleteUserConfig($userId, Application::APP_ID, self::CONFIG_MAX); $this->userConfig->deleteUserConfig($userId, Application::APP_ID, self::CONFIG_FAILED); $this->userConfig->setValueInt($userId, Application::APP_ID, self::CONFIG_PAGE, 1, lazy: true); + // everything the sweep did not see is gone, the next runs take care of it + // before the following sweep starts over from the first page + $this->userConfig->deleteUserConfig($userId, Application::APP_ID, self::CONFIG_CLEANUP_CURSOR); + $this->userConfig->setValueBool($userId, Application::APP_ID, self::CONFIG_CLEANUP, true, lazy: true); + } + + /** + * Look at the next chunk of tickets that the completed sweep did not come + * across and revoke the ones that really are gone. + * + * @param string $userId + * @return void + * @throws Exception + */ + private function cleanupChunk(string $userId): void { + $generation = $this->getGeneration($userId); + $cursor = max(0, $this->userConfig->getValueInt($userId, Application::APP_ID, self::CONFIG_CLEANUP_CURSOR, 0, lazy: true)); + $candidates = $this->importedTicketMapper->findStale($userId, $generation, $cursor, self::CLEANUP_CHUNK_SIZE); + + foreach ($candidates as $ticketId) { + $cursor = max($cursor, $ticketId); + try { + $this->reconcileTicket($userId, $ticketId, $generation); + } catch (Throwable $e) { + // we could not tell whether the ticket is gone, keep it and look at it + // again after the next sweep. Keeping a ticket that is gone is a lot + // less harmful than dropping one that is not. + $this->logger->warning( + 'Could not check whether Zammad ticket ' . $ticketId . ' is still accessible: ' . $e->getMessage(), + ['app' => Application::APP_ID, 'userId' => $userId, 'exception' => $e] + ); + } + } + + if (count($candidates) < self::CLEANUP_CHUNK_SIZE) { + // the cleanup has reached the end of the candidates, the next sweep starts + $this->userConfig->setValueInt($userId, Application::APP_ID, self::CONFIG_GENERATION, $generation + 1, lazy: true); + $this->userConfig->deleteUserConfig($userId, Application::APP_ID, self::CONFIG_CLEANUP); + $this->userConfig->deleteUserConfig($userId, Application::APP_ID, self::CONFIG_CLEANUP_CURSOR); + return; + } + $this->userConfig->setValueInt($userId, Application::APP_ID, self::CONFIG_CLEANUP_CURSOR, $cursor, lazy: true); + } + + /** + * Ask Zammad about a single ticket that the sweep did not see and revoke it if + * it really is gone. + * + * The sweep pages through the ticket list over many runs, so a ticket that was + * deleted while the sweep was running can shift the pages enough for another + * ticket to be skipped. Asking Zammad about every candidate keeps those from + * being thrown out. + * + * @param string $userId + * @param int $ticketId + * @param int $generation + * @return void + * @throws Exception if it could not be determined whether the ticket is gone + */ + private function reconcileTicket(string $userId, int $ticketId, int $generation): void { + $ticket = $this->zammadAPIService->getTicketInfo($userId, $ticketId); + if (!isset($ticket['error'])) { + // still accessible, the sweep just missed it + $this->importedTicketMapper->markSeen($userId, [$ticketId], $generation); + return; + } + $errorCode = (int)($ticket['error-code'] ?? 0); + if ($errorCode !== Http::STATUS_FORBIDDEN && $errorCode !== Http::STATUS_NOT_FOUND) { + // a bad token, a network problem or a broken Zammad tells us nothing about + // the ticket itself + throw new RuntimeException('Zammad API error: ' . $ticket['error']); + } + $this->revokeAccess($userId, $ticketId); + } + + /** + * Take a ticket away from a user, and drop its content once no user is left + * that can see it. + * + * Zammad answers with the same status for a deleted ticket and for one the user + * may not read any more, so we do not try to tell the two apart: the content is + * only deleted when the last user has lost access to it. + * + * @param string $userId + * @param int $ticketId + * @return void + * @throws Exception + */ + public function revokeAccess(string $userId, int $ticketId): void { + $itemId = (string)$ticketId; + $this->contentManager->updateAccess( + Application::APP_ID, ContentProvider::ID, $itemId, UpdateAccessOp::DENY, [$userId] + ); + $this->importedTicketMapper->delete($userId, $ticketId); + if ($this->importedTicketMapper->filterUnreferenced([$ticketId]) !== []) { + $this->contentManager->deleteContent(Application::APP_ID, ContentProvider::ID, [$itemId]); + } + } + + /** + * Take away every ticket that was imported for a user. + * + * @param string $userId + * @return void + */ + public function revokeAllAccess(string $userId): void { + try { + $ticketIds = $this->importedTicketMapper->findForUser($userId); + $this->importedTicketMapper->deleteForUser($userId); + $this->contentManager->updateAccessProvider( + Application::APP_ID, ContentProvider::ID, UpdateAccessOp::DENY, [$userId] + ); + foreach (array_chunk($this->importedTicketMapper->filterUnreferenced($ticketIds), 500) as $orphans) { + $this->contentManager->deleteContent( + Application::APP_ID, ContentProvider::ID, array_map('strval', $orphans) + ); + } + } catch (Throwable $e) { + $this->logger->warning( + 'Could not revoke the ContextChat access of ' . $userId . ' to the Zammad tickets: ' . $e->getMessage(), + ['app' => Application::APP_ID, 'userId' => $userId, 'exception' => $e] + ); + } } /** @@ -224,6 +399,7 @@ public function importTicket(string $userId, array $ticket): void { $this->contentManager->updateAccess( Application::APP_ID, ContentProvider::ID, $itemId, UpdateAccessOp::ALLOW, [$userId] ); + $this->importedTicketMapper->markSeen($userId, [(int)$ticket['id']], $this->getGeneration($userId)); } /** @@ -277,6 +453,16 @@ private function parseTimestamp(string $date): int { } } + /** + * The number of the sweep that is currently running. + * + * @param string $userId + * @return int + */ + private function getGeneration(string $userId): int { + return max(1, $this->userConfig->getValueInt($userId, Application::APP_ID, self::CONFIG_GENERATION, 1, lazy: true)); + } + /** * @param string $userId * @param string $key diff --git a/lib/Db/ImportedTicketMapper.php b/lib/Db/ImportedTicketMapper.php new file mode 100644 index 0000000..3150a4f --- /dev/null +++ b/lib/Db/ImportedTicketMapper.php @@ -0,0 +1,184 @@ +filterKnown($userId, $chunk); + if ($known !== []) { + $qb = $this->db->getQueryBuilder(); + $qb->update(self::TABLE_NAME) + ->set('last_seen', $qb->createNamedParameter($generation, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))) + ->andWhere($qb->expr()->in('ticket_id', $qb->createNamedParameter($known, IQueryBuilder::PARAM_INT_ARRAY))); + $qb->executeStatement(); + } + foreach (array_diff($chunk, $known) as $ticketId) { + // a concurrent import of the same ticket may have inserted the row in + // the meantime, which is exactly what we would have written ourselves + $this->db->insertIgnoreConflict(self::TABLE_NAME, [ + 'user_id' => $userId, + 'ticket_id' => $ticketId, + 'last_seen' => $generation, + ]); + } + } + } + + /** + * The tickets of a user that the sweep $generation did not come across, in + * ascending ticket ID order. + * + * @param string $userId + * @param int $generation + * @param int $afterTicketId only return tickets with a higher ID, to page through the candidates + * @param int $limit + * @return int[] + * @throws Exception + */ + public function findStale(string $userId, int $generation, int $afterTicketId, int $limit): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('ticket_id') + ->from(self::TABLE_NAME) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))) + ->andWhere($qb->expr()->lt('last_seen', $qb->createNamedParameter($generation, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->gt('ticket_id', $qb->createNamedParameter($afterTicketId, IQueryBuilder::PARAM_INT))) + ->orderBy('ticket_id', 'ASC') + ->setMaxResults($limit); + return $this->fetchTicketIds($qb); + } + + /** + * @param string $userId + * @return int[] every ticket that has been imported for this user + * @throws Exception + */ + public function findForUser(string $userId): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('ticket_id') + ->from(self::TABLE_NAME) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))); + return $this->fetchTicketIds($qb); + } + + /** + * @param int[] $ticketIds + * @return int[] those of $ticketIds that no user has access to any more + * @throws Exception + */ + public function filterUnreferenced(array $ticketIds): array { + $ticketIds = array_values(array_unique(array_map('intval', $ticketIds))); + $unreferenced = []; + foreach (array_chunk($ticketIds, self::ID_CHUNK_SIZE) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->selectDistinct('ticket_id') + ->from(self::TABLE_NAME) + ->where($qb->expr()->in('ticket_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $referenced = $this->fetchTicketIds($qb); + $unreferenced = array_merge($unreferenced, array_values(array_diff($chunk, $referenced))); + } + return $unreferenced; + } + + /** + * @param string $userId + * @param int $ticketId + * @return void + * @throws Exception + */ + public function delete(string $userId, int $ticketId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::TABLE_NAME) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))) + ->andWhere($qb->expr()->eq('ticket_id', $qb->createNamedParameter($ticketId, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); + } + + /** + * @param string $userId + * @return void + * @throws Exception + */ + public function deleteForUser(string $userId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::TABLE_NAME) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))); + $qb->executeStatement(); + } + + /** + * @param string $userId + * @param int[] $ticketIds + * @return int[] those of $ticketIds that already have a row for this user + * @throws Exception + */ + private function filterKnown(string $userId, array $ticketIds): array { + if ($ticketIds === []) { + return []; + } + $qb = $this->db->getQueryBuilder(); + $qb->select('ticket_id') + ->from(self::TABLE_NAME) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))) + ->andWhere($qb->expr()->in('ticket_id', $qb->createNamedParameter($ticketIds, IQueryBuilder::PARAM_INT_ARRAY))); + return $this->fetchTicketIds($qb); + } + + /** + * @param IQueryBuilder $qb a query selecting the ticket_id column + * @return int[] + * @throws Exception + */ + private function fetchTicketIds(IQueryBuilder $qb): array { + $result = $qb->executeQuery(); + $ticketIds = []; + while (($row = $result->fetch()) !== false) { + $ticketIds[] = (int)$row['ticket_id']; + } + $result->closeCursor(); + return $ticketIds; + } +} diff --git a/lib/Migration/Version040200Date20260915094500.php b/lib/Migration/Version040200Date20260915094500.php new file mode 100644 index 0000000..2c34e8c --- /dev/null +++ b/lib/Migration/Version040200Date20260915094500.php @@ -0,0 +1,69 @@ +hasTable(ImportedTicketMapper::TABLE_NAME)) { + return null; + } + + $table = $schema->createTable(ImportedTicketMapper::TABLE_NAME); + $table->addColumn('id', Types::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 20, + ]); + $table->addColumn('user_id', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('ticket_id', Types::BIGINT, [ + 'notnull' => true, + 'length' => 20, + ]); + // the sweep during which this ticket was last seen in the user's ticket list + $table->addColumn('last_seen', Types::BIGINT, [ + 'notnull' => true, + 'default' => 0, + 'length' => 20, + ]); + $table->setPrimaryKey(['id']); + $table->addUniqueIndex(['user_id', 'ticket_id'], 'zammad_cc_user_ticket'); + // the lookup of the tickets a sweep did not see + $table->addIndex(['user_id', 'last_seen'], 'zammad_cc_user_seen'); + // the reference count of a ticket across users + $table->addIndex(['ticket_id'], 'zammad_cc_ticket'); + + return $schema; + } +} diff --git a/lib/Service/ZammadAPIService.php b/lib/Service/ZammadAPIService.php index 211b436..abad049 100644 --- a/lib/Service/ZammadAPIService.php +++ b/lib/Service/ZammadAPIService.php @@ -548,11 +548,11 @@ public function request( if ($statusCode === Http::STATUS_UNAUTHORIZED) { return ['error' => $this->l10n->t('Bad credentials'), 'error-code' => $statusCode]; } elseif ($statusCode === Http::STATUS_FORBIDDEN) { - return ['error' => 'Forbidden']; + return ['error' => 'Forbidden', 'error-code' => $statusCode]; } elseif ($statusCode === Http::STATUS_NOT_FOUND) { - return ['error' => 'Not found']; + return ['error' => 'Not found', 'error-code' => $statusCode]; } - return ['error' => $e->getMessage()]; + return ['error' => $e->getMessage(), 'error-code' => $statusCode]; } catch (ConnectException $e) { return ['error' => $e->getMessage()]; } diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 35d9dc8..b4a1a62 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -103,6 +103,28 @@ namespace Doctrine\DBAL { } } +namespace Doctrine\DBAL\Schema { + class Column + { + public function setNotnull(bool $notnull): self; + public function setDefault($default): self; + public function setLength(?int $length): self; + } + + class Table + { + public function addColumn(string $name, string $typeName, array $options = []): Column; + public function getColumn(string $name): Column; + public function hasColumn(string $name): bool; + public function dropColumn(string $name): self; + public function setPrimaryKey(array $columnNames, $indexName = false): self; + public function addIndex(array $columnNames, ?string $indexName = null, array $flags = [], array $options = []): self; + public function addUniqueIndex(array $columnNames, ?string $indexName = null, array $options = []): self; + public function hasIndex(string $indexName): bool; + public function dropIndex(string $indexName): self; + } +} + namespace OCA\DAV\CalDAV { /**