From 37c82f68bcbafe27f43cf42c4628693c0f3e0911 Mon Sep 17 00:00:00 2001
From: Arunas Skirius <arukomp@gmail.com>
Date: Tue, 11 Jul 2023 20:12:24 +0300
Subject: [PATCH] more improvements, bug fixes

---
 src/HttpLogReader.php                       | 27 ++++++-----
 tests/Pest.php                              |  2 +-
 tests/Unit/AccessLogs/HttpLogReaderTest.php | 54 ++++++++++++++++++---
 3 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/src/HttpLogReader.php b/src/HttpLogReader.php
index b1a2481a..8cd05afa 100644
--- a/src/HttpLogReader.php
+++ b/src/HttpLogReader.php
@@ -177,13 +177,13 @@ public function next(): ?HttpLog
         }
 
         // get the next log line
-        $line = match ($this->direction) {
+        list($text, $position) = match ($this->direction) {
             Direction::Forward => $this->readLineForward(),
             Direction::Backward => $this->readLineBackward(),
             default => throw new \Exception('Unknown direction: '.$this->direction),
         };
 
-        if ($line === false) {
+        if ($text === false) {
             return null;
         }
 
@@ -193,38 +193,41 @@ public function next(): ?HttpLog
             return $this->next();
         }
 
-        $position = ftell($this->fileHandle);
-
-        return $this->makeLog($line, $position);
+        return $this->makeLog($text, $position);
     }
 
-    protected function readLineForward(): string|bool
+    protected function readLineForward(): array
     {
-        return fgets($this->fileHandle);
+        $position = ftell($this->fileHandle);
+        $line = fgets($this->fileHandle);
+
+        return [$line, $position];
     }
 
-    protected function readLineBackward(): string|bool
+    protected function readLineBackward(): array
     {
-        $line = '';
+        $line = false;
+        $position = null;
 
         while (true) {
             if (ftell($this->fileHandle) <= 0) {
-                return false;
+                return [$line, 0];
             }
 
             fseek($this->fileHandle, -1, SEEK_CUR);
             $char = fgetc($this->fileHandle);
 
             if ($char === "\n") {
+                $position = ftell($this->fileHandle);
                 fseek($this->fileHandle, -1, SEEK_CUR);
                 break;
             }
 
-            $line = $char.$line;
+            $line = $char.($line ?: '');
             fseek($this->fileHandle, -1, SEEK_CUR);
         }
 
-        return $line;
+        return [$line, $position];
     }
 
     protected function makeLog(string $text, int $filePosition): HttpLog
diff --git a/tests/Pest.php b/tests/Pest.php
index ba428324..c1f7df16 100644
--- a/tests/Pest.php
+++ b/tests/Pest.php
@@ -90,7 +90,7 @@ function makeHttpAccessLogEntry(CarbonInterface $date = null, string $method = '
     $contentLength ??= rand(1, 9999);
 
     return <<<EOF
-$randomIp - - [$dateFormatted] "$method /$path HTTP/2.0" $statusCode $contentLength "http://www.example.com/post.php" "Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; AS; rv:11.0) like Gecko"
+$randomIp - - [$dateFormatted] "$method $path HTTP/2.0" $statusCode $contentLength "http://www.example.com/post.php" "Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; AS; rv:11.0) like Gecko"
 EOF;
 }
 
diff --git a/tests/Unit/AccessLogs/HttpLogReaderTest.php b/tests/Unit/AccessLogs/HttpLogReaderTest.php
index 736e3e32..e2f480b8 100644
--- a/tests/Unit/AccessLogs/HttpLogReaderTest.php
+++ b/tests/Unit/AccessLogs/HttpLogReaderTest.php
@@ -47,6 +47,49 @@
     }
 });
 
+it('provides the correct file position in the log entry', function () {
+    $lines = [
+        makeHttpAccessLogEntry(),
+        makeHttpAccessLogEntry(),
+        makeHttpAccessLogEntry(),
+    ];
+
+    $file = generateLogFile('http.log', implode("\n", $lines), type: LogFile::TYPE_HTTP_ACCESS);
+
+    $httpLogReader = new HttpLogReader($file);
+
+    $expectedPosition = 0;
+    foreach ($lines as $expectedLine) {
+        $actualLine = $httpLogReader->next();
+        expect($actualLine->filePosition)->toBe($expectedPosition);
+        $expectedPosition += strlen($expectedLine) + strlen("\n");
+    }
+});
+
+it('provides the correct file position when reading backwards', function () {
+    $lines = [
+        makeHttpAccessLogEntry(),
+        makeHttpAccessLogEntry(),
+        makeHttpAccessLogEntry(),
+    ];
+
+    $file = generateLogFile('http.log', implode("\n", $lines), type: LogFile::TYPE_HTTP_ACCESS);
+
+    $httpLogReader = (new HttpLogReader($file))->reverse();
+
+    // expected positions are reversed, because we're reading in reverse
+    $expectedPositions = [
+        strlen($lines[0]) + strlen("\n") + strlen($lines[1]) + strlen("\n"),
+        strlen($lines[0]) + strlen("\n"),
+        0,
+    ];
+
+    foreach ($expectedPositions as $expectedPosition) {
+        $actualLine = $httpLogReader->next();
+        expect($actualLine->filePosition)->toBe($expectedPosition);
+    }
+});
+
 it('can skip a number of logs', function () {
     $lines = [
         makeHttpAccessLogEntry(),
@@ -69,13 +112,12 @@
     ];
     $file = generateLogFile('http.log', implode("\n", $lines), type: LogFile::TYPE_HTTP_ACCESS);
 
-    $httpLogReader = new HttpLogReader($file);
+    $httpLogReader = (new HttpLogReader($file))->reverse();
 
-    $entry = $httpLogReader->reverse()->next();
-    expect($entry->text)->toBe($lines[2]);
-
-    $entry = $httpLogReader->next();
-    expect($entry->text)->toBe($lines[1]);
+    foreach (array_reverse($lines) as $expectedLine) {
+        $actualLine = $httpLogReader->next();
+        expect($actualLine->text)->toBe($expectedLine);
+    }
 });
 
 it('can limit the number of lots to get', function () {