Skip to content

[stable5.7] fix(ai): handle invalid JSON and possible null for smart replies #12616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
backportbot wants to merge 1 commit into
base: stable5.7
Choose a base branch
from
Open

[stable5.7] fix(ai): handle invalid JSON and possible null for smart replies #12616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/Service/AiIntegrations/AiIntegrationsService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,12 @@ public function getSmartReply(Account $account, Mailbox $mailbox, Message $messa
if (in_array(FreePromptTaskType::class, $this->textProcessingManager->getAvailableTaskTypes(), true)) {
$cachedReplies = $this->cache->getValue('smartReplies_' . $message->getId());
if ($cachedReplies) {
return json_decode($cachedReplies, true, 512);
try {
return json_decode($cachedReplies, true, 512, JSON_THROW_ON_ERROR);
} catch (JsonException $e) {
$this->cache->remove('smartReplies_' . $message->getId());
throw new ServiceException('Failed to decode smart replies JSON output', previous: $e);
}
}
$client = $this->clientFactory->getClient($account);
try {
Expand Down
8 changes: 8 additions & 0 deletions lib/Service/AiIntegrations/Cache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,13 @@ public function addValue(string $key, ?string $value): void {
$this->cache->set($key, $value ?? false, self::CACHE_TTL);
}

/**
* @param string $key
*
* @return void
*/
public function remove(string $key): void {
$this->cache->remove($key);
}

}
235 changes: 235 additions & 0 deletions tests/Unit/Controller/MessagesControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1398,4 +1398,239 @@ public function testRestrictLimit(?int $limit, int $expectedLimit): void {

$this->controller->index(100, null, null, $limit);
}

public function testSmartReplyNoUser(): void {
$controller = new MessagesController(
$this->appName,
$this->request,
$this->accountService,
$this->mailManager,
$this->mailSearch,
$this->itineraryService,
null,
$this->userFolder,
$this->logger,
$this->l10n,
$this->mimeTypeDetector,
$this->urlGenerator,
$this->nonceManager,
$this->trustedSenderService,
$this->mailTransmission,
$this->smimeService,
$this->clientFactory,
$this->dkimService,
$this->userPreferences,
$this->snoozeService,
$this->aiIntegrationsService,
$this->cacheFactory,
);

$actualResponse = $controller->smartReply(100);
$expectedResponse = new JSONResponse([], Http::STATUS_UNAUTHORIZED);
$this->assertEquals($expectedResponse, $actualResponse);
}

public function testSmartReplyNoMessage(): void {
$this->mailManager->expects($this->once())
->method('getMessage')
->with($this->userId, 100)
->willThrowException(new DoesNotExistException(''));

$actualResponse = $this->controller->smartReply(100);
$expectedResponse = new JSONResponse([], Http::STATUS_FORBIDDEN);
$this->assertEquals($expectedResponse, $actualResponse);
}

public function testSmartReplyNoMailbox(): void {
$message = new \OCA\Mail\Db\Message();
$message->setId(100);
$message->setMailboxId(1);

$this->mailManager->expects($this->once())
->method('getMessage')
->with($this->userId, 100)
->willReturn($message);

$this->mailManager->expects($this->once())
->method('getMailbox')
->with($this->userId, $message->getMailboxId())
->willThrowException(new DoesNotExistException(''));

$actualResponse = $this->controller->smartReply(100);
$expectedResponse = new JSONResponse([], Http::STATUS_FORBIDDEN);
$this->assertEquals($expectedResponse, $actualResponse);
}

public function testSmartReplyNoAccount(): void {
$message = new \OCA\Mail\Db\Message();
$message->setId(100);
$message->setMailboxId(1);
$mailbox = new Mailbox();
$mailbox->setId(1);
$mailbox->setAccountId(1);

$this->mailManager->expects($this->once())
->method('getMessage')
->with($this->userId, 100)
->willReturn($message);

$this->mailManager->expects($this->once())
->method('getMailbox')
->with($this->userId, $message->getMailboxId())
->willReturn($mailbox);

$this->accountService->expects($this->once())
->method('find')
->with($this->userId, $mailbox->getAccountId())
->willThrowException(new DoesNotExistException(''));

$actualResponse = $this->controller->smartReply(100);
$expectedResponse = new JSONResponse([], Http::STATUS_FORBIDDEN);
$this->assertEquals($expectedResponse, $actualResponse);
}

public function testSmartReplyServiceException(): void {
$message = new \OCA\Mail\Db\Message();
$message->setId(100);
$message->setMailboxId(1);
$mailbox = new Mailbox();
$mailbox->setId(1);
$mailbox->setAccountId(1);

$this->mailManager->expects($this->once())
->method('getMessage')
->with($this->userId, 100)
->willReturn($message);

$this->mailManager->expects($this->once())
->method('getMailbox')
->with($this->userId, $message->getMailboxId())
->willReturn($mailbox);

$this->accountService->expects($this->once())
->method('find')
->with($this->userId, $mailbox->getAccountId())
->willReturn(new Account(new MailAccount()));

$this->aiIntegrationsService->expects($this->once())
->method('getSmartReply')
->with($this->anything(), $this->anything(), $this->anything(), $this->userId)
->willThrowException(new ServiceException('AI service error'));

$this->logger->expects($this->once())
->method('error');

$actualResponse = $this->controller->smartReply(100);
$expectedResponse = new JSONResponse([], Http::STATUS_NO_CONTENT);
$this->assertEquals($expectedResponse, $actualResponse);
}

public function testSmartReplySuccessful(): void {
$message = new \OCA\Mail\Db\Message();
$message->setId(100);
$message->setMailboxId(1);
$mailbox = new Mailbox();
$mailbox->setId(1);
$mailbox->setAccountId(1);

$this->mailManager->expects($this->once())
->method('getMessage')
->with($this->userId, 100)
->willReturn($message);

$this->mailManager->expects($this->once())
->method('getMailbox')
->with($this->userId, $message->getMailboxId())
->willReturn($mailbox);

$this->accountService->expects($this->once())
->method('find')
->with($this->userId, $mailbox->getAccountId())
->willReturn(new Account(new MailAccount()));

$replies = ['reply1' => 'OK thanks', 'reply2' => 'Sounds good'];
$this->aiIntegrationsService->expects($this->once())
->method('getSmartReply')
->with($this->anything(), $this->anything(), $this->anything(), $this->userId)
->willReturn($replies);

$actualResponse = $this->controller->smartReply(100);
$expectedResponse = new JSONResponse(array_values($replies));
$this->assertEquals($expectedResponse, $actualResponse);
}

public function testSmartReplyEmptyReplies(): void {
$message = new \OCA\Mail\Db\Message();
$message->setId(100);
$message->setMailboxId(1);
$mailbox = new Mailbox();
$mailbox->setId(1);
$mailbox->setAccountId(1);

$this->mailManager->expects($this->once())
->method('getMessage')
->with($this->userId, 100)
->willReturn($message);

$this->mailManager->expects($this->once())
->method('getMailbox')
->with($this->userId, $message->getMailboxId())
->willReturn($mailbox);

$this->accountService->expects($this->once())
->method('find')
->with($this->userId, $mailbox->getAccountId())
->willReturn(new Account(new MailAccount()));

$this->aiIntegrationsService->expects($this->once())
->method('getSmartReply')
->with($this->anything(), $this->anything(), $this->anything(), $this->userId)
->willReturn([]);

$actualResponse = $this->controller->smartReply(100);
$expectedResponse = new JSONResponse([]);
$this->assertEquals($expectedResponse, $actualResponse);
}

public function testSmartReplyWithCachedInvalidJson(): void {
// This test verifies that when getSmartReply() encounters corrupted cache
// (which would cause json_decode to fail), it throws ServiceException
// and the controller properly handles it by returning NO_CONTENT.
// This prevents the TypeError: array_values(): Argument #1 ($array) must be of type array, null given

$message = new \OCA\Mail\Db\Message();
$message->setId(100);
$message->setMailboxId(1);
$mailbox = new Mailbox();
$mailbox->setId(1);
$mailbox->setAccountId(1);

$this->mailManager->expects($this->once())
->method('getMessage')
->with($this->userId, 100)
->willReturn($message);

$this->mailManager->expects($this->once())
->method('getMailbox')
->with($this->userId, $message->getMailboxId())
->willReturn($mailbox);

$this->accountService->expects($this->once())
->method('find')
->with($this->userId, $mailbox->getAccountId())
->willReturn(new Account(new MailAccount()));

// Simulate the AI service throwing ServiceException due to corrupted cache
$this->aiIntegrationsService->expects($this->once())
->method('getSmartReply')
->with($this->anything(), $this->anything(), $this->anything(), $this->userId)
->willThrowException(new ServiceException('Failed to decode smart replies JSON output'));

$this->logger->expects($this->once())
->method('error');

$actualResponse = $this->controller->smartReply(100);
$expectedResponse = new JSONResponse([], Http::STATUS_NO_CONTENT);
$this->assertEquals($expectedResponse, $actualResponse);
}
}
Loading