diff --git a/src/Analyser/PatternAnalyser.php b/src/Analyser/PatternAnalyser.php index a14784f..abbc8cf 100644 --- a/src/Analyser/PatternAnalyser.php +++ b/src/Analyser/PatternAnalyser.php @@ -59,7 +59,7 @@ public function addPossibleInsightClass(string $insightClass): static */ protected function findPossibleInsightClass(string $insightClass): int { - $index = array_search($insightClass, $this->possibleInsightClasses); + $index = array_search($insightClass, $this->possibleInsightClasses, true); if ($index === false) { throw new InvalidArgumentException("Class " . $insightClass . " not found in possible insight classes."); } @@ -96,7 +96,25 @@ public function overridePossibleInsightClass(string $parentInsightClass, string } /** - * Analyse a log and return an Analysis + * Get the patterns of all possible insight classes + * + * getPatterns() often builds its patterns from string fragments, so calling it + * for every entry of a log adds up. The result is only reused within one analysis, + * so changes to the possible insight classes or their patterns still take effect. + * + * @return array, string[]> The patterns of each insight class, keyed by class name + */ + protected function resolvePatterns(): array + { + $patternsByClass = []; + foreach ($this->possibleInsightClasses as $possibleInsightClass) { + $patternsByClass[$possibleInsightClass] = $possibleInsightClass::getPatterns(); + } + return $patternsByClass; + } + + /** + * Analyze a log and return an Analysis * * @return AnalysisInterface */ @@ -105,18 +123,11 @@ public function analyse(): AnalysisInterface $analysis = new Analysis(); $analysis->setLog($this->log); + $patternsByClass = $this->resolvePatterns(); + foreach ($this->log as $entry) { - foreach ($this->possibleInsightClasses as $possibleInsightClass) { - /** @var PatternInsightInterface $possibleInsightClass */ - $patterns = $possibleInsightClass::getPatterns(); - foreach ($patterns as $patternKey => $pattern) { - $insights = $this->analyseEntry($entry, $possibleInsightClass, $patternKey, $pattern); - if ($insights) { - foreach ($insights as $insight) { - $analysis->addInsight($insight); - } - } - } + foreach ($this->analyseEntry($entry, $patternsByClass) as $insight) { + $analysis->addInsight($insight); } } @@ -124,31 +135,68 @@ public function analyse(): AnalysisInterface } /** - * Compare the entry against the given pattern and create an insight object if it matches + * Compare an entry against all patterns and create an insight for every match * * @param EntryInterface $entry - * @param string $possibleInsightClass - * @param mixed $patternKey - * @param string $pattern - * @return null|PatternInsightInterface[] + * @param array, string[]> $patternsByClass The patterns from resolvePatterns() + * @return PatternInsightInterface[] */ - protected function analyseEntry(EntryInterface $entry, string $possibleInsightClass, mixed $patternKey, string $pattern): ?array + protected function analyseEntry(EntryInterface $entry, array $patternsByClass): array { - $result = preg_match_all($pattern, $entry, $matches, PREG_SET_ORDER); - if ($result === false || $result === 0) { - return null; + // Converting an entry to a string joins all of its lines, so it is done once here + // and not for every pattern, which used to be most of the time spent analysing a log. + $text = (string)$entry; + + $insights = []; + foreach ($patternsByClass as $insightClass => $patterns) { + foreach ($patterns as $patternKey => $pattern) { + foreach ($this->findMatches($pattern, $text) as $matches) { + $insights[] = $this->createInsight($insightClass, $entry, $matches, $patternKey); + } + } } - $return = []; - foreach ($matches as $match) { - /** @var PatternInsightInterface $insight */ - $insight = new $possibleInsightClass(); - $insight->setMatches($match, $patternKey); - $insight->setEntry($entry); + return $insights; + } - $return[] = $insight; + /** + * Find all matches of a pattern in a text + * + * A pattern that fails, for example because it exceeds the backtrack limit, + * triggers a warning and is treated as not matching, so the rest of the log is still analysed. + * Without the warning, such a pattern would silently never find its problem. + * + * @param string $pattern + * @param string $text + * @return array> One array of groups for every match, empty if the pattern does not match + */ + protected function findMatches(string $pattern, string $text): array + { + $result = preg_match_all($pattern, $text, $matches, PREG_SET_ORDER); + if ($result === false) { + // $matches can contain the matches found before the error, they are discarded + trigger_error("preg_match_all() failed for pattern " . $pattern . ": " . preg_last_error_msg(), E_USER_WARNING); + return []; } - return $return; + return $matches; + } + + /** + * Create an insight for a match of one of its patterns in an entry + * + * @param class-string $insightClass + * @param EntryInterface $entry The entry the pattern matched in + * @param array $matches The groups of the match + * @param mixed $patternKey The key of the matching pattern in getPatterns() + * @return PatternInsightInterface + */ + protected function createInsight(string $insightClass, EntryInterface $entry, array $matches, mixed $patternKey): PatternInsightInterface + { + $insight = new $insightClass(); + $insight->setMatches($matches, $patternKey); + $insight->setEntry($entry); + + return $insight; } -} \ No newline at end of file +} diff --git a/test/tests/Analyser/PatternAnalyserTest.php b/test/tests/Analyser/PatternAnalyserTest.php index 3076af5..fda224b 100644 --- a/test/tests/Analyser/PatternAnalyserTest.php +++ b/test/tests/Analyser/PatternAnalyserTest.php @@ -4,6 +4,8 @@ use Aternos\Codex\Analyser\PatternAnalyser; use Aternos\Codex\Analysis\Analysis; +use Aternos\Codex\Analysis\AnalysisInterface; +use Aternos\Codex\Analysis\InsightInterface; use Aternos\Codex\Analysis\PatternInsightInterface; use Aternos\Codex\Log\Entry; use Aternos\Codex\Log\File\PathLogFile; @@ -85,6 +87,228 @@ public function testAnalyseWithPossibleInsightClasses(): void $this->assertJsonStringEqualsJsonString(json_encode($this->getExpectedAnalysis()->getInsights()), json_encode($analysis->getInsights())); } + public function testAnalyseResolvesPatternsOnce(): void + { + $logFile = new PathLogFile(__DIR__ . '/../../data/problem.log'); + $log = (new TestPatternLog())->setLogFile($logFile); + $log->parse(); + + $insightClass = new class extends TestPatternProblem { + public static int $getPatternsCalls = 0; + + /** + * @inheritDoc + */ + public static function getPatterns(): array + { + static::$getPatternsCalls++; + return parent::getPatterns(); + } + }; + + $analyser = (new PatternAnalyser())->addPossibleInsightClass($insightClass::class); + $analysis = $log->analyse($analyser); + + $this->assertGreaterThan(1, count($log)); + $this->assertSame(1, $insightClass::$getPatternsCalls); + $this->assertCount(4, $analysis->getInsights()); + } + + public function testAnalyseConvertsEveryEntryToStringOnce(): void + { + $entry = new class extends Entry { + public int $stringConversions = 0; + + /** + * @inheritDoc + */ + public function __toString(): string + { + $this->stringConversions++; + return parent::__toString(); + } + }; + $entry->addLine(new Line(1, "I have a problem with ABC")); + $entry->addLine(new Line(2, "This log was generated by software v1.2.3")); + + $log = (new TestPatternLog())->addEntry($entry); + $analysis = $log->analyse(); + + $this->assertSame(1, $entry->stringConversions); + $this->assertCount(2, $analysis->getInsights()); + } + + public function testAnalysePassesMatchesAndPatternKeyToInsight(): void + { + $insightClass = new class extends TestPatternProblem { + /** + * @inheritDoc + */ + public static function getPatterns(): array + { + return [ + 'problem' => '/I have a problem with (\w+)/', + 'version' => '/generated by software (v[0-9.]+)/' + ]; + } + + /** + * @inheritDoc + */ + public function setMatches(array $matches, mixed $patternKey): void + { + $this->cause = $patternKey . " " . $matches[1]; + } + }; + + $entry = (new Entry()) + ->addLine(new Line(1, "I have a problem with ABC")) + ->addLine(new Line(2, "This log was generated by software v1.2.3")) + ->addLine(new Line(3, "I have a problem with DEF")); + $log = (new TestPatternLog())->addEntry($entry); + + $analyser = (new PatternAnalyser())->addPossibleInsightClass($insightClass::class); + $analysis = $log->analyse($analyser); + + $this->assertSame($log, $analysis->getLog()); + $this->assertSame([ + "There is a problem with problem ABC", + "There is a problem with problem DEF", + "There is a problem with version v1.2.3" + ], array_map(fn(InsightInterface $insight) => $insight->getMessage(), $analysis->getInsights())); + foreach ($analysis->getInsights() as $insight) { + $this->assertSame($entry, $insight->getEntry()); + } + } + + public function testAnalyseUsesInsightClassAddedTwiceOnlyOnce(): void + { + $logFile = new PathLogFile(__DIR__ . '/../../data/problem.log'); + $log = (new TestPatternLog())->setLogFile($logFile); + $log->parse(); + + $analyser = (new PatternAnalyser()) + ->setPossibleInsightClasses([ + TestPatternProblem::class, + TestPatternProblem::class, + TestPatternInformation::class + ]); + + $analysis = $log->analyse($analyser); + $this->assertJsonStringEqualsJsonString(json_encode($this->getExpectedAnalysis()->getInsights()), json_encode($analysis->getInsights())); + } + + public function testAnalyseIgnoresRemovedInsightClass(): void + { + $logFile = new PathLogFile(__DIR__ . '/../../data/problem.log'); + $log = (new TestPatternLog())->setLogFile($logFile); + $log->parse(); + + $analyser = (new PatternAnalyser()) + ->setPossibleInsightClasses([ + TestPatternProblem::class, + TestPatternInformation::class + ]); + $analyser->removePossibleInsightClass(TestPatternProblem::class); + + $analysis = $log->analyse($analyser); + $this->assertCount(0, $analysis->getProblems()); + $this->assertCount(1, $analysis->getInformation()); + } + + /** + * Analyse a log and collect the messages of the warnings triggered by the analyser + * + * @param TestPatternLog $log + * @param PatternAnalyser $analyser + * @param string[] $warnings + * @return AnalysisInterface + */ + protected function analyseAndCollectWarnings(TestPatternLog $log, PatternAnalyser $analyser, array &$warnings): AnalysisInterface + { + set_error_handler(function (int $level, string $message) use (&$warnings): bool { + $warnings[] = $message; + return true; + }, E_USER_WARNING); + + try { + return $log->analyse($analyser); + } finally { + restore_error_handler(); + } + } + + public function testAnalyseReportsFailingPatternAndContinues(): void + { + $insightClass = new class extends TestPatternProblem { + /** + * @inheritDoc + */ + public static function getPatterns(): array + { + return [ + '/I have a problem with (\w+)/u', + '/I have an issue with (\w+)/' + ]; + } + }; + + // The invalid UTF-8 byte makes preg_match_all() return false for patterns with the u modifier + $entry = (new Entry()) + ->addLine(new Line(1, "I have a problem with ABC \xFF")) + ->addLine(new Line(2, "I have an issue with DEF")); + $log = (new TestPatternLog()) + ->addEntry($entry) + ->addEntry((new Entry())->addLine(new Line(3, "I have a problem with GHI"))); + $analyser = (new PatternAnalyser())->addPossibleInsightClass($insightClass::class); + + $warnings = []; + $analysis = $this->analyseAndCollectWarnings($log, $analyser, $warnings); + + $this->assertSame( + ['preg_match_all() failed for pattern /I have a problem with (\w+)/u: Malformed UTF-8 characters, possibly incorrectly encoded'], + $warnings + ); + $this->assertSame( + ["There is a problem with DEF", "There is a problem with GHI"], + array_map(fn(InsightInterface $insight) => $insight->getMessage(), $analysis->getInsights()) + ); + } + + public function testAnalyseDiscardsMatchesFoundBeforePatternFailed(): void + { + $insightClass = new class extends TestPatternProblem { + /** + * @inheritDoc + */ + public static function getPatterns(): array + { + return ['/I have a problem with (\w+)|(?:a+)+b/']; + } + }; + + // The pattern matches in the first line and then exceeds the backtrack limit in the second line + $entry = (new Entry()) + ->addLine(new Line(1, "I have a problem with ABC")) + ->addLine(new Line(2, str_repeat("a", 40))); + $log = (new TestPatternLog())->addEntry($entry); + $analyser = (new PatternAnalyser())->addPossibleInsightClass($insightClass::class); + + $warnings = []; + $backtrackLimit = ini_set('pcre.backtrack_limit', '1000'); + try { + $analysis = $this->analyseAndCollectWarnings($log, $analyser, $warnings); + } finally { + ini_set('pcre.backtrack_limit', $backtrackLimit); + } + + $this->assertSame( + ['preg_match_all() failed for pattern /I have a problem with (\w+)|(?:a+)+b/: Backtrack limit exhausted'], + $warnings + ); + $this->assertSame([], $analysis->getInsights()); + } + public function testAddPossibleInsightClassThrowsExceptionIfPossibleInsightClassDoesNotImplementPatternInsightInterface(): void { $this->expectException(\InvalidArgumentException::class);