Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 0 additions & 2 deletions lib/Command/Watch.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ protected function configure() {

private function getLastLogId() {
$logIterator = $this->logIteratorFactory->getLogIterator(self::ALL_LEVELS);
$logIterator->next();
if ($logIterator->current() !== null) {
return $logIterator->current()['reqId'];
}
Expand Down Expand Up @@ -74,7 +73,6 @@ public function watch(bool $raw, OutputInterface $output): int {
$id = $this->getLastLogId();
if ($id !== $lastId) {
$iterator = $this->logIteratorFactory->getLogIterator(self::ALL_LEVELS);
$iterator->next();

$lines = [];

Expand Down
3 changes: 2 additions & 1 deletion lib/Controller/LogController.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public function get($query = '', $count = 50, $offset = 0): JSONResponse {
*/
private function getLastItem() {
$iterator = $this->logIteratorFactory->getLogIterator($this->settingsService->getShownLevels());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This already does rewind on creating from class, and calling next() once - so it should be on the latest line already:

$this->logIteratorFactory->getLogIterator($this->settingsService->getShownLevels());
	...
	new LogIterator($handle, $dateFormat, $timezone);
		...
		__construct($handle, string $dateFormat, string $timezone) {
			...
			$this->rewind();
				...
				$this->next();

What does it fix? I remember investigating something with it, but didn't finish. But not sure that current code brings something to it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the fix I always got null has return from getLastItem

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is something weird going on because I agree with @Antreesy the code is supposed to return a rewinded iterator already from the factory

$iterator->rewind();
return $iterator->current();
}

Expand Down Expand Up @@ -102,7 +103,7 @@ public function poll(string $lastReqId): JSONResponse {
}

$iterator = $this->logIteratorFactory->getLogIterator($this->settingsService->getShownLevels());
$iterator->next();
$iterator->rewind();

$data = [];

Expand Down
96 changes: 96 additions & 0 deletions tests/Unit/Controller/LogControllerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\LogReader\Tests\Unit\Controller;

use OCA\LogReader\Controller\LogController;
use OCA\LogReader\Log\LogIterator;
use OCA\LogReader\Log\LogIteratorFactory;
use OCA\LogReader\Service\SettingsService;
use OCP\IRequest;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;

class LogControllerTest extends TestCase {

private LogController $logController;

/** @var LogIteratorFactory|MockObject */
private $logIteratorFactory;

/** @var SettingsService|MockObject */
private $settingsService;

/** @var LoggerInterface|MockObject */
private $logger;

/** @var IRequest|MockObject */
private $request;

protected function setUp(): void {
parent::setUp();

$this->logIteratorFactory = $this->createStub(LogIteratorFactory::class);
$this->settingsService = $this->createStub(SettingsService::class);
$this->logger = $this->createStub(LoggerInterface::class);
$this->request = $this->createStub(IRequest::class);

$this->settingsService->method('getLoggingType')->willReturn('file');
$this->settingsService->method('getShownLevels')->willReturn([0, 1, 2, 3, 4]);

$this->logController = new LogController(
'logreader',
$this->request,
$this->logIteratorFactory,
$this->settingsService,
$this->logger,
);
}

private function getLogIterator(string $log): LogIterator {
$handle = fopen('php://temp', 'r+');
fwrite($handle, $log);
rewind($handle);
return new LogIterator($handle, \DateTime::ATOM, 'UTC');
}

/**
* Every call to the factory must return a freshly rewound iterator over
* the same log content, mirroring how the real factory opens the log
* file anew for each call.
*/
private function mockLogWithEntries(string $log): void {
$this->logIteratorFactory->method('getLogIterator')
->willReturnCallback(fn () => $this->getLogIterator($log));
}

public function testPollIncludesTheNewestLogEntry(): void {
$log = '{"reqId":"1","level":3,"time":"2019-11-04T18:50:57+00:00","app":"comments"}' . "\n"
. '{"reqId":"2","level":3,"time":"2019-11-04T18:50:58+00:00","app":"gallery"}' . "\n"
. '{"reqId":"3","level":3,"time":"2019-11-04T18:50:59+00:00","app":"files"}';
$this->mockLogWithEntries($log);

$response = $this->logController->poll('1');
$data = $response->getData();

$this->assertCount(2, $data);
// sorted newest first, the last written entry (reqId 3) must be included
$this->assertEquals('3', $data[0]['reqId']);
$this->assertEquals('2', $data[1]['reqId']);
}

public function testPollReturnsEmptyWhenThereIsNoNewEntry(): void {
$log = '{"reqId":"1","level":3,"time":"2019-11-04T18:50:57+00:00","app":"comments"}';
$this->mockLogWithEntries($log);

$response = $this->logController->poll('1');

$this->assertEquals([], $response->getData());
}
}
Loading