From 8bcf93676bac84c22e22af42be539c3b88a9a742 Mon Sep 17 00:00:00 2001 From: Jose Andres Tejerina Date: Tue, 28 Oct 2025 12:25:21 -0300 Subject: [PATCH] fix: change method for log collection changes --- app/Audit/AuditLogOtlpStrategy.php | 38 ++++- tests/OpenTelemetry/AuditOtlpStrategyTest.php | 134 +++++++++++++++--- 2 files changed, 146 insertions(+), 26 deletions(-) diff --git a/app/Audit/AuditLogOtlpStrategy.php b/app/Audit/AuditLogOtlpStrategy.php index ee57b56f0..b44f49702 100644 --- a/app/Audit/AuditLogOtlpStrategy.php +++ b/app/Audit/AuditLogOtlpStrategy.php @@ -197,11 +197,39 @@ private function getCollectionType(PersistentCollection $collection): string private function getCollectionChanges(PersistentCollection $collection, array $change_set): array { - return [ - 'current_count' => count($collection), - 'snapshot_count' => count($collection->getSnapshot()), - 'is_dirty' => $collection->isDirty(), - ]; + try { + $isInitialized = $collection->isInitialized(); + + if (!$isInitialized) { + $snapshotCount = count($collection->getSnapshot()); + $currentCount = $snapshotCount; + $isDirty = !empty($collection->getSnapshot()) || count($change_set) > 0; + } else { + $currentCount = count($collection); + $snapshotCount = count($collection->getSnapshot()); + $isDirty = $collection->isDirty(); + } + + return [ + 'current_count' => $currentCount, + 'snapshot_count' => $snapshotCount, + 'is_dirty' => $isDirty, + 'is_initialized' => $isInitialized + ]; + + } catch (\Exception $ex) { + Log::warning('Error getting collection changes', [ + 'error' => $ex->getMessage() + ]); + + return [ + 'current_count' => 0, + 'snapshot_count' => 0, + 'is_dirty' => false, + 'is_initialized' => false, + 'error' => 'Failed to get collection changes' + ]; + } } private function mapEventTypeToAction(string $event_type): string diff --git a/tests/OpenTelemetry/AuditOtlpStrategyTest.php b/tests/OpenTelemetry/AuditOtlpStrategyTest.php index 9d6d98041..7d39297a1 100644 --- a/tests/OpenTelemetry/AuditOtlpStrategyTest.php +++ b/tests/OpenTelemetry/AuditOtlpStrategyTest.php @@ -17,11 +17,13 @@ use Tests\InsertSummitTestData; use OpenTelemetry\API\Trace\StatusCode; use OpenTelemetry\API\Trace\TracerInterface; +use Illuminate\Support\Facades\DB; +use Doctrine\ORM\PersistentCollection; class AuditOtlpStrategyTest extends OpenTelemetryTestCase { - use InsertSummitTestData; - use InsertMemberTestData; + public static $summit; + public static $em; private AuditLogOtlpStrategy $auditStrategy; @@ -29,17 +31,20 @@ protected function setUp(): void { parent::setUp(); - self::insertMemberTestData(IGroup::TrackChairs); - self::$defaultMember = self::$member; - self::insertSummitTestData(); - $this->auditStrategy = $this->app->make(AuditLogOtlpStrategy::class); + + if (!self::$em) { + self::$em = \LaravelDoctrine\ORM\Facades\EntityManager::getFacadeRoot(); + } + + if (!self::$summit) { + $summitRepo = self::$em->getRepository(\models\summit\Summit::class); + self::$summit = $summitRepo->findOneBy([]); + } } protected function tearDown(): void { - self::clearSummitTestData(); - self::clearMemberTestData(); parent::tearDown(); } @@ -65,13 +70,15 @@ public function testAuditSummitChangeWithOtlp(): void AuditLogOtlpStrategy::EVENT_ENTITY_UPDATE ); + $this->assertNotNull($span, 'Span should be created'); + $this->assertNotEmpty($simulatedChangeSet, 'ChangeSet should not be empty'); + $span->setStatus(StatusCode::STATUS_OK, 'Summit audit completed'); - $this->assertTrue(true); } catch (\Exception $e) { $span->recordException($e); $span->setStatus(StatusCode::STATUS_ERROR, $e->getMessage()); - throw $e; + $this->fail('Audit failed: ' . $e->getMessage()); } finally { $span->end(); $spanScope->detach(); @@ -87,7 +94,12 @@ public function testAuditSummitEventChangeWithOtlp(): void $spanScope = $span->activate(); try { - $summitEvent = self::$summit->getEvents()[0]; + $events = self::$summit->getEvents(); + $this->assertNotEmpty($events, 'Summit must have events'); + + $summitEvent = $events[0]; + $this->assertNotNull($summitEvent, 'Summit event should exist'); + $simulatedChangeSet = $this->createSummitEventChangeSet($summitEvent); $this->auditStrategy->audit( @@ -96,13 +108,14 @@ public function testAuditSummitEventChangeWithOtlp(): void AuditLogOtlpStrategy::EVENT_ENTITY_UPDATE ); + $this->assertNotEmpty($simulatedChangeSet, 'Event changeSet should not be empty'); + $span->setStatus(StatusCode::STATUS_OK, 'SummitEvent audit completed'); - $this->assertTrue(true); } catch (\Exception $e) { $span->recordException($e); $span->setStatus(StatusCode::STATUS_ERROR, $e->getMessage()); - throw $e; + $this->fail('Event audit failed: ' . $e->getMessage()); } finally { $span->end(); $spanScope->detach(); @@ -115,13 +128,18 @@ public function testAuditStrategyWithoutActiveSpan(): void $simulatedChangeSet = ['name' => ['Old Name', 'New Name']]; - $this->auditStrategy->audit( - self::$summit, - $simulatedChangeSet, - AuditLogOtlpStrategy::EVENT_ENTITY_UPDATE - ); + try { + $this->auditStrategy->audit( + self::$summit, + $simulatedChangeSet, + AuditLogOtlpStrategy::EVENT_ENTITY_UPDATE + ); + $this->assertTrue(true, 'Audit works without active span'); + } catch (\Exception $e) { + $this->fail('Audit should work without active span: ' . $e->getMessage()); + } - $this->assertTrue(true); + $this->assertNotNull(self::$summit, 'Summit should exist'); } public function testAuditStrategyWithEmptyChangeSet(): void @@ -140,18 +158,92 @@ public function testAuditStrategyWithEmptyChangeSet(): void ); $span->setStatus(StatusCode::STATUS_OK, 'Empty changeset audit completed'); - $this->assertTrue(true); + $this->assertNotNull(self::$summit, 'Summit should still exist after empty audit'); } catch (\Exception $e) { $span->recordException($e); $span->setStatus(StatusCode::STATUS_ERROR, $e->getMessage()); - throw $e; + $this->fail('Audit should handle empty changeSet: ' . $e->getMessage()); } finally { $span->end(); $spanScope->detach(); } } + public function testGetCollectionTypeDoesNotTriggerLazyLoading(): void + { + $this->skipIfOpenTelemetryDisabled(); + + DB::enableQueryLog(); + DB::flushQueryLog(); + + $eventsCollection = self::$summit->getEvents(); + + $this->assertInstanceOf( + PersistentCollection::class, + $eventsCollection, + 'Events should be a PersistentCollection' + ); + + DB::flushQueryLog(); + + $this->auditStrategy->audit( + $eventsCollection, + ['events' => [[], [1, 2, 3]]], + AuditLogOtlpStrategy::EVENT_COLLECTION_UPDATE + ); + + $queries = DB::getQueryLog(); + $queryCount = count($queries); + + DB::disableQueryLog(); + + $this->assertLessThanOrEqual( + 2, + $queryCount, + "getCollectionType() ejecutó $queryCount queries, máximo 2 permitidas" + ); + } + + public function testAuditSummitEventTagsCollectionWithOtlp(): void + { + $this->skipIfOpenTelemetryDisabled(); + + \DB::enableQueryLog(); + \DB::flushQueryLog(); + + // Usa el summit ya gestionado + $events = self::$summit->getEvents(); + $this->assertNotEmpty($events, 'Summit must have events'); + $event = $events[0]; + + $tagsCollection = $event->getTags(); + $this->assertInstanceOf(\Doctrine\ORM\PersistentCollection::class, $tagsCollection); + + $this->assertGreaterThanOrEqual(2, count($tagsCollection), "El evento debe tener al menos 2 tags para el test"); + + $tags = []; + foreach ($tagsCollection as $tag) { + $tags[] = $tag; + if (count($tags) === 2) break; + } + + \DB::flushQueryLog(); + + $this->auditStrategy->audit( + $tagsCollection, + ['tags' => [[], [$tags[0]->getId(), $tags[1]->getId()]]], + AuditLogOtlpStrategy::EVENT_COLLECTION_UPDATE + ); + + $queries = \DB::getQueryLog(); + $this->assertLessThanOrEqual( + 2, + count($queries), + "getCollectionType() ejecutó demasiadas queries" + ); + } + private function skipIfOpenTelemetryDisabled(): void { if (!$this->isOpenTelemetryEnabled()) {