From f3ca99177718b16345c9f70950bf7db957a20dc4 Mon Sep 17 00:00:00 2001 From: romanetar Date: Thu, 6 Aug 2026 18:51:37 +0200 Subject: [PATCH 1/2] fix(presentations): resolve summit_id via owning presentation for PresentationMaterial lifecycle events Signed-off-by: romanetar --- .../Materials/PresentationMaterial.php | 13 ++ ...ntationMaterialRabbitMQIntegrationTest.php | 153 ++++++++++++++++++ .../PresentationMaterialEventDispatchTest.php | 139 ++++++++++++++++ 3 files changed, 305 insertions(+) create mode 100644 tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php create mode 100644 tests/Unit/Services/PresentationMaterialEventDispatchTest.php diff --git a/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php b/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php index ec540161d..a033dcc34 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php @@ -73,6 +73,19 @@ public function getPresentationId(){ } } + /** + * @return int + */ + public function getSummitId(): int + { + try { + return $this->presentation->getSummitId(); + } + catch (\Throwable $ex){ + return 0; + } + } + /** * @return string */ diff --git a/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php b/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php new file mode 100644 index 000000000..d7965b616 --- /dev/null +++ b/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php @@ -0,0 +1,153 @@ + EventServiceProvider listener -> job payload + * (already covered by PresentationMaterialEventDispatchTest via Queue::fake()) + * -> ProcessScheduleEntityLifeCycleEvent::handle() -> ProcessScheduleEntityLifeCycleEventService::process() + * -> RabbitPublisherService::publish() -> real message on the entities-updates-broker + * exchange. + * + * Runs the job for real (no Queue::fake()) against the actual RabbitMQ broker + * from docker-compose (rabbitmq_sponsor_services, reachable on the + * summit-api-local-net network the app container is also on) and asserts the + * published AMQP payload's summit_id is non-zero for a PresentationMediaUpload. + * + * @package Tests\Feature + */ +class PresentationMaterialRabbitMQIntegrationTest extends TestCase +{ + use InsertSummitTestData; + + const EXCHANGE = 'entities-updates-broker'; + + private ?AMQPStreamConnection $consumer_connection = null; + + private ?AMQPChannel $consumer_channel = null; + + private ?string $consumer_queue = null; + + protected function setUp(): void + { + parent::setUp(); + self::insertSummitTestData(); + + // The default rabbitmq.* config (RABBITMQ_HOST=host.docker.internal:5672) + // has nothing listening in this environment. Point it at the broker this + // container can actually reach, matching the credentials docker-compose + // already configures for rabbitmq_sponsor_services. + Config::set('rabbitmq.host', 'rabbitmq_sponsor_services'); + Config::set('rabbitmq.port', 5672); + Config::set('rabbitmq.user', 'admin'); + Config::set('rabbitmq.password', '1qaz2wsx'); + Config::set('rabbitmq.vhost', '/'); + // Force IProcessScheduleEntityLifeCycleEventService (a singleton) to + // rebuild its internal RabbitPublisherService against the config above, + // in case anything already resolved it with the default (unreachable) host. + app()->forgetInstance(IProcessScheduleEntityLifeCycleEventService::class); + + $this->consumer_connection = new AMQPStreamConnection('rabbitmq_sponsor_services', 5672, 'admin', '1qaz2wsx', '/'); + $this->consumer_channel = $this->consumer_connection->channel(); + // Must match the type/durable/auto_delete the real publisher declares with + // (RabbitPublisherService defaults: fanout, durable=true, auto_delete=false) + // or RabbitMQ rejects the redeclaration. + $this->consumer_channel->exchange_declare(self::EXCHANGE, AMQPExchangeType::FANOUT, false, true, false); + [$this->consumer_queue] = $this->consumer_channel->queue_declare('', false, false, true, true); + $this->consumer_channel->queue_bind($this->consumer_queue, self::EXCHANGE); + } + + public function tearDown(): void + { + try { + $this->consumer_channel?->close(); + $this->consumer_connection?->close(); + } catch (\Throwable $ex) { + // best-effort cleanup + } + self::clearSummitTestData(); + parent::tearDown(); + } + + private function drainQueue(): void + { + while ($this->consumer_channel->basic_get($this->consumer_queue, true)) { + // discard any message left over from a previous run + } + } + + public function testMediaUploadUpdateEndToEndPublishesNonZeroSummitId(): void + { + $media_upload = null; + foreach (self::$presentations as $presentation) { + $candidate = $presentation->getMediaUploads()->first(); + if ($candidate !== false) { + $media_upload = $candidate; + break; + } + } + $this->assertNotNull($media_upload, 'Pre-condition: fixtures must include a media upload'); + + // Compute summit_id exactly the way ScheduleEntity's PostPersist/PostUpdate/ + // PreRemove hooks do (private _getSummitId(), resolved via reflection), + // instead of assuming PresentationMaterial::getSummitId() exists - so this + // test also fails meaningfully pre-fix (summit_id resolves to 0) rather + // than erroring on a missing method. + $rc = new \ReflectionClass($media_upload); + $get_summit_id = $rc->getMethod('_getSummitId'); + $get_summit_id->setAccessible(true); + $summit_id = $get_summit_id->invoke($media_upload); + $entity_id = $media_upload->getId(); + $this->assertGreaterThan(0, $summit_id, 'Pre-condition: presentation must belong to a summit, and _getSummitId() must resolve it (this is the exact bug being fixed)'); + + $this->drainQueue(); + + // Run the real job/service/publisher chain, exactly as the + // EventServiceProvider listener would when the queue worker picks it up. + $job = new ProcessScheduleEntityLifeCycleEvent( + ScheduleEntityLifeCycleEvent::Operation_Update, + $summit_id, + $entity_id, + 'PresentationMediaUpload' + ); + $job->handle(app(IProcessScheduleEntityLifeCycleEventService::class)); + + $message = null; + for ($i = 0; $i < 30 && is_null($message); $i++) { + $message = $this->consumer_channel->basic_get($this->consumer_queue, true); + if (is_null($message)) { + usleep(100000); + } + } + + $this->assertNotNull($message, 'Expected a message published to the entities-updates-broker exchange'); + $payload = json_decode($message->getBody(), true); + + $this->assertSame('PresentationMediaUpload', $payload['entity_type']); + $this->assertSame($entity_id, $payload['entity_id']); + $this->assertSame($summit_id, $payload['summit_id'], 'Published summit_id must be the real summit id, not 0'); + } +} diff --git a/tests/Unit/Services/PresentationMaterialEventDispatchTest.php b/tests/Unit/Services/PresentationMaterialEventDispatchTest.php new file mode 100644 index 000000000..0f7e790d6 --- /dev/null +++ b/tests/Unit/Services/PresentationMaterialEventDispatchTest.php @@ -0,0 +1,139 @@ + 0: ScheduleEntity::_getSummitId() resolves the + * summit id via reflection (a "summit" property, or a getSummitId() method), and + * PresentationMaterial exposed neither - it only has a "presentation" relation. + * The fix adds PresentationMaterial::getSummitId(), delegating to the owning + * Presentation. + * + * @package Tests\Unit\Services + */ +class PresentationMaterialEventDispatchTest extends TestCase +{ + use InsertSummitTestData; + + protected function setUp(): void + { + parent::setUp(); + self::insertSummitTestData(); + } + + public function tearDown(): void + { + self::clearSummitTestData(); + parent::tearDown(); + } + + private function getMediaUpload(): PresentationMediaUpload + { + foreach (self::$presentations as $presentation) { + $media_upload = $presentation->getMediaUploads()->first(); + if ($media_upload !== false) { + return $media_upload; + } + } + $this->fail('Pre-condition: no presentation with a media upload found in fixtures'); + } + + /** + * @return ProcessScheduleEntityLifeCycleEvent[] + */ + private function jobsFor(string $entity_type): array + { + return Queue::pushed(ProcessScheduleEntityLifeCycleEvent::class, function ($job) use ($entity_type) { + return $job->entity_type === $entity_type; + })->all(); + } + + public function testUpdateMediaUploadDispatchesLifeCycleEventWithSummitId(): void + { + $media_upload = $this->getMediaUpload(); + $summit_id = $media_upload->getPresentation()->getSummitId(); + $this->assertGreaterThan(0, $summit_id, 'Pre-condition: presentation must belong to a summit'); + + Queue::fake(); + + $media_upload->setName('Updated Media Upload Name'); + self::$em->persist($media_upload); + self::$em->flush(); + + $jobs = $this->jobsFor('PresentationMediaUpload'); + $this->assertCount(1, $jobs, 'Expected 1 ProcessScheduleEntityLifeCycleEvent for PresentationMediaUpload update'); + $this->assertSame($summit_id, $jobs[0]->summit_id, 'Dispatched summit_id must be the presentation summit id, not 0'); + } + + public function testInsertMediaUploadDispatchesLifeCycleEventWithSummitId(): void + { + $presentation = self::$presentations[0]; + $summit_id = $presentation->getSummitId(); + $this->assertGreaterThan(0, $summit_id, 'Pre-condition: presentation must belong to a summit'); + + Queue::fake(); + + $media_upload = new PresentationMediaUpload(); + $media_upload->setName('New Media Upload'); + $media_upload->setDescription('New Media Upload Description'); + $media_upload->setFilename('new_media_upload.png'); + $media_upload->setMediaUploadType(self::$media_uploads_types[0]); + $presentation->addMediaUpload($media_upload); + self::$em->persist($media_upload); + self::$em->flush(); + + $jobs = $this->jobsFor('PresentationMediaUpload'); + $this->assertCount(1, $jobs, 'Expected 1 ProcessScheduleEntityLifeCycleEvent for PresentationMediaUpload insert'); + $this->assertSame($summit_id, $jobs[0]->summit_id, 'Dispatched summit_id must be the presentation summit id, not 0'); + } + + /** + * Characterization test for the deleting() (PreRemove) path: the production + * delete flow (Presentation::removeMediaUpload() -> unsetPresentation(), + * relying on the materials collection's orphanRemoval to schedule the actual + * Doctrine delete) nulls the "presentation" association *before* flush() + * triggers PreRemove. So getSummitId() legitimately falls into its + * defensive catch and returns 0 here - same accepted degraded case already + * called out for getPresentationId(), and the same shape as SummitOwned's + * former_summit_id gap for entities whose owning reference is cleared ahead + * of removal. This is not a regression: the important behavior is that the + * lifecycle event still dispatches without throwing (see + * PresentationMaterial::getSummitId() catching \Throwable, not just + * \Exception, to survive exactly this null-presentation case). + */ + public function testDeleteMediaUploadDispatchesLifeCycleEventWithoutError(): void + { + $media_upload = $this->getMediaUpload(); + $presentation = $media_upload->getPresentation(); + $this->assertGreaterThan(0, $presentation->getSummitId(), 'Pre-condition: presentation must belong to a summit'); + + Queue::fake(); + + $presentation->removeMediaUpload($media_upload); + self::$em->flush(); + + $jobs = $this->jobsFor('PresentationMediaUpload'); + $this->assertCount(1, $jobs, 'Expected 1 ProcessScheduleEntityLifeCycleEvent for PresentationMediaUpload delete'); + $this->assertSame(0, $jobs[0]->summit_id, 'summit_id is 0 here because unsetPresentation() runs before PreRemove - accepted degraded case, not a regression'); + } +} From af30e55342848a5b1cfba0d413c6e477fbc0727d Mon Sep 17 00:00:00 2001 From: romanetar Date: Thu, 6 Aug 2026 19:34:36 +0200 Subject: [PATCH 2/2] fix(tests): address CodeRabbit findings in RabbitMQ integration test Restores the original rabbitmq config and forgets the IProcessScheduleEntityLifeCycleEventService singleton in tearDown() so the real-broker override doesn't leak to later tests in the same process; sources broker credentials from existing DOMAIN_EVENTS_RABBITMQ_* / RABBITMQ_* env vars instead of hardcoding them in a tracked file; and makes the message-polling loop skip unrelated messages on the shared fanout exchange instead of asserting on the first one received. Signed-off-by: romanetar --- .github/workflows/push.yml | 13 +++++ tests/Feature/ExampleTest.php | 21 -------- ...ntationMaterialRabbitMQIntegrationTest.php | 51 ++++++++++++++----- 3 files changed, 50 insertions(+), 35 deletions(-) delete mode 100644 tests/Feature/ExampleTest.php diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 2e2aec304..af3ee96db 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -69,6 +69,7 @@ jobs: # Named by path because no job in this matrix runs the tests/ root, only its # subdirectories - a file added there runs nowhere unless it is listed here. - { name: "PresentationMediaUploads", filter: "tests/PresentationMediaUploadsTest.php tests/PresentationMediaUploadsVisibilityTest.php tests/PresentationSerializerCacheKeyTest.php" } + - { name: "PresentationMaterialRabbitMQIntegration", filter: "tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php" } env: OTEL_SERVICE_ENABLED: false APP_ENV: testing @@ -118,6 +119,10 @@ jobs: REGISTRATION_VALIDATE_TICKET_TYPE_REMOVAL: false MEMCACHED_SERVER_HOST: 127.0.0.1 MEMCACHED_SERVER_PORT: 11211 + DOMAIN_EVENTS_RABBITMQ_HOST: 127.0.0.1 + DOMAIN_EVENTS_RABBITMQ_VHOST: / + DOMAIN_EVENTS_RABBITMQ_LOGIN: admin + DOMAIN_EVENTS_RABBITMQ_PASSWORD: 1qaz2wsx services: mysql_api_model: @@ -136,6 +141,14 @@ jobs: ports: - 3306:3306 options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=10 + rabbitmq: + image: rabbitmq:3-management + env: + RABBITMQ_DEFAULT_USER: ${{ env.DOMAIN_EVENTS_RABBITMQ_LOGIN }} + RABBITMQ_DEFAULT_PASS: ${{ env.DOMAIN_EVENTS_RABBITMQ_PASSWORD }} + ports: + - 5672:5672 + options: --health-cmd="rabbitmq-diagnostics check_running" --health-interval=10s --health-timeout=5s --health-retries=10 steps: - name: Start Memcached (with larger item size) diff --git a/tests/Feature/ExampleTest.php b/tests/Feature/ExampleTest.php deleted file mode 100644 index f31e495ca..000000000 --- a/tests/Feature/ExampleTest.php +++ /dev/null @@ -1,21 +0,0 @@ -get('/'); - - $response->assertStatus(200); - } -} diff --git a/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php b/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php index d7965b616..bbd2074a0 100644 --- a/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php +++ b/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php @@ -51,26 +51,39 @@ class PresentationMaterialRabbitMQIntegrationTest extends TestCase private ?string $consumer_queue = null; + /** + * @var array + */ + private array $original_rabbitmq_config = []; + protected function setUp(): void { parent::setUp(); self::insertSummitTestData(); + $this->original_rabbitmq_config = Config::get('rabbitmq'); + // The default rabbitmq.* config (RABBITMQ_HOST=host.docker.internal:5672) // has nothing listening in this environment. Point it at the broker this - // container can actually reach, matching the credentials docker-compose - // already configures for rabbitmq_sponsor_services. - Config::set('rabbitmq.host', 'rabbitmq_sponsor_services'); + // container can actually reach, reusing the same credentials docker-compose + // already configures (as plain env vars, not literals) for rabbitmq_sponsor_services. + Config::set('rabbitmq.host', env('DOMAIN_EVENTS_RABBITMQ_HOST', 'rabbitmq_sponsor_services')); Config::set('rabbitmq.port', 5672); - Config::set('rabbitmq.user', 'admin'); - Config::set('rabbitmq.password', '1qaz2wsx'); - Config::set('rabbitmq.vhost', '/'); + Config::set('rabbitmq.user', env('DOMAIN_EVENTS_RABBITMQ_LOGIN', env('RABBITMQ_LOGIN', 'guest'))); + Config::set('rabbitmq.password', env('DOMAIN_EVENTS_RABBITMQ_PASSWORD', env('RABBITMQ_PASSWORD', 'guest'))); + Config::set('rabbitmq.vhost', env('DOMAIN_EVENTS_RABBITMQ_VHOST', '/')); // Force IProcessScheduleEntityLifeCycleEventService (a singleton) to // rebuild its internal RabbitPublisherService against the config above, // in case anything already resolved it with the default (unreachable) host. app()->forgetInstance(IProcessScheduleEntityLifeCycleEventService::class); - $this->consumer_connection = new AMQPStreamConnection('rabbitmq_sponsor_services', 5672, 'admin', '1qaz2wsx', '/'); + $this->consumer_connection = new AMQPStreamConnection( + config('rabbitmq.host'), + config('rabbitmq.port'), + config('rabbitmq.user'), + config('rabbitmq.password'), + config('rabbitmq.vhost'), + ); $this->consumer_channel = $this->consumer_connection->channel(); // Must match the type/durable/auto_delete the real publisher declares with // (RabbitPublisherService defaults: fanout, durable=true, auto_delete=false) @@ -88,6 +101,11 @@ public function tearDown(): void } catch (\Throwable $ex) { // best-effort cleanup } + // Undo the setUp() overrides so later tests in this process see the + // original rabbitmq config and a fresh service singleton, not this + // test's real-broker configuration. + Config::set('rabbitmq', $this->original_rabbitmq_config); + app()->forgetInstance(IProcessScheduleEntityLifeCycleEventService::class); self::clearSummitTestData(); parent::tearDown(); } @@ -135,19 +153,24 @@ public function testMediaUploadUpdateEndToEndPublishesNonZeroSummitId(): void ); $job->handle(app(IProcessScheduleEntityLifeCycleEventService::class)); - $message = null; - for ($i = 0; $i < 30 && is_null($message); $i++) { + // The queue is bound to the whole fanout exchange, so it can receive + // unrelated messages from other activity on the same broker. Skip past + // anything that isn't this update before asserting on it. + $payload = null; + for ($i = 0; $i < 30 && is_null($payload); $i++) { $message = $this->consumer_channel->basic_get($this->consumer_queue, true); if (is_null($message)) { usleep(100000); + continue; + } + $candidate = json_decode($message->getBody(), true); + if (($candidate['entity_type'] ?? null) === 'PresentationMediaUpload' + && ($candidate['entity_id'] ?? null) === $entity_id) { + $payload = $candidate; } } - $this->assertNotNull($message, 'Expected a message published to the entities-updates-broker exchange'); - $payload = json_decode($message->getBody(), true); - - $this->assertSame('PresentationMediaUpload', $payload['entity_type']); - $this->assertSame($entity_id, $payload['entity_id']); + $this->assertNotNull($payload, 'Expected a PresentationMediaUpload message for this entity on the entities-updates-broker exchange'); $this->assertSame($summit_id, $payload['summit_id'], 'Published summit_id must be the real summit id, not 0'); } }