diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index af8cc31be5..901a424d77 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -66,6 +66,7 @@ use OCA\Mail\Service\Search\MailSearch; use OCA\Mail\Service\TrustedSenderService; use OCA\Mail\Service\UserPreferenceService; +use OCA\Mail\SetupChecks\MailConnectionPerformance; use OCA\Mail\SetupChecks\MailTransport; use OCA\Mail\Vendor\Favicon\Favicon; use OCP\AppFramework\App; @@ -160,6 +161,7 @@ public function register(IRegistrationContext $context): void { $context->registerNotifierService(Notifier::class); $context->registerSetupCheck(MailTransport::class); + $context->registerSetupCheck(MailConnectionPerformance::class); // bypass Horde Translation system Horde_Translation::setHandler('Horde_Imap_Client', new HordeTranslationHandler()); diff --git a/lib/Db/MailAccountMapper.php b/lib/Db/MailAccountMapper.php index 0e41bbe976..ae87fdb692 100644 --- a/lib/Db/MailAccountMapper.php +++ b/lib/Db/MailAccountMapper.php @@ -234,4 +234,24 @@ public function getAllUserIdsWithAccounts(): array { return $this->findEntities($query); } + + public function getRandomAccountIdsByImapHost(string $host, int $limit = 3): array { + $query = $this->db->getQueryBuilder(); + $query->select('id') + ->from($this->getTableName()) + ->where($query->expr()->eq('inbound_host', $query->createNamedParameter($host), IQueryBuilder::PARAM_STR)) + ->setMaxResults(1000); + $result = $query->executeQuery(); + $ids = $result->fetchAll(\PDO::FETCH_COLUMN); + $result->closeCursor(); + // Pick 3 random accounts or any available + if ($ids !== [] && count($ids) >= $limit) { + $rids = array_rand($ids, $limit); + if (!is_array($rids)) { + $rids = [$rids]; + } + return array_intersect_key($ids, array_values($rids)); + } + return $ids; + } } diff --git a/lib/Db/ProvisioningMapper.php b/lib/Db/ProvisioningMapper.php index 0626f18cf8..0486b84a30 100644 --- a/lib/Db/ProvisioningMapper.php +++ b/lib/Db/ProvisioningMapper.php @@ -138,4 +138,19 @@ public function get(int $id): ?Provisioning { return null; } } + + /** + * @since 4.2.0 + * + * @return array + */ + public function findUniqueImapHosts(): array { + $query = $this->db->getQueryBuilder(); + $query->selectDistinct('imap_host') + ->from('mail_provisionings'); + $result = $query->executeQuery(); + $data = $result->fetchAll(\PDO::FETCH_COLUMN); + $result->closeCursor(); + return $data; + } } diff --git a/lib/SetupChecks/MailConnectionPerformance.php b/lib/SetupChecks/MailConnectionPerformance.php new file mode 100644 index 0000000000..7f05f7d966 --- /dev/null +++ b/lib/SetupChecks/MailConnectionPerformance.php @@ -0,0 +1,108 @@ +l10n->t('Mail connection performance'); + } + + public function getCategory(): string { + return 'mail'; + } + + public function run(): SetupResult { + // retrieve unique imap hosts for provisionings and abort if none exists + $hosts = $this->provisioningMapper->findUniqueImapHosts(); + if (empty($hosts)) { + return SetupResult::success(); + } + // retrieve random account ids for each host + $accounts = []; + foreach ($hosts as $host) { + $accounts[$host] = $this->accountMapper->getRandomAccountIdsByImapHost($host); + } + // test accounts + $tests = []; + foreach ($accounts as $host => $collection) { + foreach ($collection as $accountId) { + $account = new Account($this->accountMapper->findById((int)$accountId)); + $client = $this->clientFactory->getClient($account); + try { + $tStart = $this->microtime->getNumeric(); + // time login + $client->login(); + $tLogin = $this->microtime->getNumeric(); + // time operation + $list = $client->listMailboxes('*'); + $status = $client->status(key($list)); + $tOperation = $this->microtime->getNumeric(); + + $tests[$host][$accountId] = ['start' => $tStart, 'login' => $tLogin, 'operation' => $tOperation]; + } catch (Throwable $e) { + $this->logger->warning('Error occurred while performing system check on mail account: ' . $account->getId()); + } finally { + $client->close(); + } + } + } + // calculate performance + $performance = []; + foreach ($tests as $host => $test) { + $tLogin = 0; + $tOperation = 0; + foreach ($test as $entry) { + [$start, $login, $operation] = array_values($entry); + $tLogin += ($login - $start); + $tOperation += ($operation - $login); + } + $performance[$host]['login'] = $tLogin / count($tests[$host]); + $performance[$host]['operation'] = $tOperation / count($tests[$host]); + } + // display performance test outcome + foreach ($performance as $host => $entry) { + [$login, $operation] = array_values($entry); + if ($login > 1) { + return SetupResult::warning( + $this->l10n->t('Slow mail service detected (%1$s) an attempt to connect to several accounts took an average of %2$s seconds per account', [$host, round($login, 3)]) + ); + } + if ($operation > 1) { + return SetupResult::warning( + $this->l10n->t('Slow mail service detected (%1$s) an attempt to perform a mail box list operation on several accounts took an average of %2$s seconds per account', [$host, round($operation, 3)]) + ); + } + } + return SetupResult::success(); + } + +} diff --git a/lib/SetupChecks/MicroTime.php b/lib/SetupChecks/MicroTime.php new file mode 100644 index 0000000000..96dc48cf73 --- /dev/null +++ b/lib/SetupChecks/MicroTime.php @@ -0,0 +1,18 @@ +setProvisioningDomain($this->data['provisioningDomain']); + $provisioning->setEmailTemplate($this->data['emailTemplate']); + $provisioning->setImapUser($this->data['imapUser']); + $provisioning->setImapHost($this->data['imapHost']); + $provisioning->setImapPort(42); + $provisioning->setImapSslMode($this->data['imapSslMode']); + $provisioning->setSmtpUser($this->data['smtpUser']); + $provisioning->setSmtpHost($this->data['smtpHost']); + $provisioning->setSmtpPort(24); + $provisioning->setSmtpSslMode($this->data['smtpSslMode']); + $provisioning->setSieveEnabled($this->data['sieveEnabled']); + $provisioning = $this->mapper->insert($provisioning); + + $hosts = $this->mapper->findUniqueImapHosts(); + + $this->assertIsArray($hosts); + $this->assertNotEmpty($hosts); + $this->assertEquals($this->data['imapHost'], $hosts[0]); + } } diff --git a/tests/Unit/SetupChecks/MailConnectionPerformanceTest.php b/tests/Unit/SetupChecks/MailConnectionPerformanceTest.php new file mode 100644 index 0000000000..04dd25451a --- /dev/null +++ b/tests/Unit/SetupChecks/MailConnectionPerformanceTest.php @@ -0,0 +1,227 @@ +l10n = $this->createMock(L10N::class); + $this->logger = new TestLogger(); + $this->provisioningMapper = $this->createMock(ProvisioningMapper::class); + $this->accountMapper = $this->createMock(MailAccountMapper::class); + $this->clientFactory = $this->createMock(IMAPClientFactory::class); + $this->folderMapper = $this->createMock(FolderMapper::class); + $this->microtime = $this->createMock(MicroTime::class); + + $this->check = new MailConnectionPerformance( + $this->l10n, + $this->logger, + $this->provisioningMapper, + $this->accountMapper, + $this->clientFactory, + $this->folderMapper, + $this->microtime + ); + } + + public function testNoHosts(): void { + $this->provisioningMapper->expects($this->once()) + ->method('findUniqueImapHosts') + ->willReturn([]); + + $result = $this->check->run(); + + $this->assertEquals(SetupResult::SUCCESS, $result->getSeverity()); + } + + public function testNoAccounts(): void { + $this->provisioningMapper->expects($this->once()) + ->method('findUniqueImapHosts') + ->willReturn(['imap.example.com']); + + $this->accountMapper->expects($this->once()) + ->method('getRandomAccountIdsByImapHost') + ->with('imap.example.com') + ->willReturn([]); + + $result = $this->check->run(); + + $this->assertEquals(SetupResult::SUCCESS, $result->getSeverity()); + } + + public function testConnectionSuccess(): void { + $account = new MailAccount( + [ + 'accountId' => 42, + 'imapHost' => 'imap.example.com', + 'imapUser' => 'user', + 'imapPassword' => 'password', + 'imapPort' => 143, + 'imapSslMode' => 'none', + ] + ); + + $client = $this->createMock(Horde_Imap_Client_Socket::class); + + $this->provisioningMapper->expects($this->once()) + ->method('findUniqueImapHosts') + ->willReturn(['imap.example.com']); + + $this->accountMapper->expects($this->once()) + ->method('getRandomAccountIdsByImapHost') + ->with('imap.example.com') + ->willReturn([42]); + $this->accountMapper->expects($this->once()) + ->method('findById') + ->with(42) + ->willReturn($account); + + $client->expects($this->once()) + ->method('listMailboxes') + ->with('*') + ->willReturn(['INBOX' => []]); + $client->expects($this->once()) + ->method('status') + ->with('INBOX') + ->willReturn([]); + + $this->clientFactory->expects($this->once()) + ->method('getClient') + ->with(new Account($account)) + ->willReturn($client); + + $this->microtime->method('getNumeric') + ->willReturnOnConsecutiveCalls(0, .2, .2); + + $result = $this->check->run(); + + $this->assertEquals(SetupResult::SUCCESS, $result->getSeverity()); + + } + + public function testConnectionWarning(): void { + $account = new MailAccount( + [ + 'accountId' => 42, + 'imapHost' => 'imap.example.com', + 'imapUser' => 'user', + 'imapPassword' => 'password', + 'imapPort' => 143, + 'imapSslMode' => 'none', + ] + ); + + $client = $this->createMock(Horde_Imap_Client_Socket::class); + + $this->provisioningMapper->expects($this->once()) + ->method('findUniqueImapHosts') + ->willReturn(['imap.example.com']); + + $this->accountMapper->expects($this->once()) + ->method('getRandomAccountIdsByImapHost') + ->with('imap.example.com') + ->willReturn([42]); + $this->accountMapper->expects($this->once()) + ->method('findById') + ->with(42) + ->willReturn($account); + + $client->expects($this->once()) + ->method('listMailboxes') + ->with('*') + ->willReturn(['INBOX' => []]); + $client->expects($this->once()) + ->method('status') + ->with('INBOX') + ->willReturn([]); + + $this->clientFactory->expects($this->once()) + ->method('getClient') + ->with(new Account($account)) + ->willReturn($client); + + $this->microtime->method('getNumeric') + ->willReturnOnConsecutiveCalls(0, 2, 3); + + $result = $this->check->run(); + + $this->assertEquals(SetupResult::WARNING, $result->getSeverity()); + + } + + public function testConnectionFailure(): void { + $account = new MailAccount( + [ + 'accountId' => 42, + 'imapHost' => 'imap.example.com', + 'imapUser' => 'user', + 'imapPassword' => 'password', + 'imapPort' => 143, + 'imapSslMode' => 'none', + ] + ); + + $client = $this->createMock(Horde_Imap_Client_Socket::class); + + $this->provisioningMapper->expects($this->once()) + ->method('findUniqueImapHosts') + ->willReturn(['imap.example.com']); + + $this->accountMapper->expects($this->once()) + ->method('getRandomAccountIdsByImapHost') + ->with('imap.example.com') + ->willReturn([42]); + $this->accountMapper->expects($this->once()) + ->method('findById') + ->with(42) + ->willReturn($account); + + $this->clientFactory->expects($this->once()) + ->method('getClient') + ->with(new Account($account)) + ->willReturn($client); + + $client->expects($this->once()) + ->method('login') + ->willThrowException(new \Exception('Login failed')); + + $result = $this->check->run(); + + $this->assertEquals(SetupResult::SUCCESS, $result->getSeverity()); + $this->assertTrue($this->logger->hasWarningThatContains('Error occurred while performing system check on mail account: 42')); + } + +}