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/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/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 new file mode 100644 index 000000000..bbd2074a0 --- /dev/null +++ b/tests/Feature/PresentationMaterialRabbitMQIntegrationTest.php @@ -0,0 +1,176 @@ + 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; + + /** + * @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, 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', 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( + 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) + // 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 + } + // 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(); + } + + 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)); + + // 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($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'); + } +} 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'); + } +}