diff --git a/adr/0002-flow1.png b/adr/0002-flow1.png new file mode 100644 index 000000000..35025f0b7 Binary files /dev/null and b/adr/0002-flow1.png differ diff --git a/adr/0002-flow2.png b/adr/0002-flow2.png new file mode 100644 index 000000000..a1500b024 Binary files /dev/null and b/adr/0002-flow2.png differ diff --git a/adr/0002-m2m-audit-logging-lightweight-join-table-query.md b/adr/0002-m2m-audit-logging-lightweight-join-table-query.md new file mode 100644 index 000000000..f90f90469 --- /dev/null +++ b/adr/0002-m2m-audit-logging-lightweight-join-table-query.md @@ -0,0 +1,584 @@ +# ADR-0002: M2M Audit Logging via Lightweight Join Table Query + +**Date:** 2026-03-05 +**Updated:** 2026-03-06 +**Author:** Sebastian Marcet (sebastian@tipit.net) +**Status:** Accepted +**Context:** PR #498 — Audit logging for many-to-many relationship changes; subsequent refactoring via `PersistentCollectionMetadata` DTO + +## Context + +The audit logging system covers Doctrine ManyToMany collection changes (e.g., `PresentationCategoryGroup::$categories`, `SummitAttendee::$tags`). It works correctly for **updates** (add/remove while the owning entity lives), but has a gap on **deletions of the owning entity**. + +When Doctrine's `getScheduledCollectionDeletions()` fires for a collection that is never loaded into memory (the owning entity is being deleted without its M2M relations ever accessed), `PersistentCollection::getDeleteDiff()` returns `[]` because no snapshot exists. The deleted IDs are silently lost from the audit trail. + +### Rejected Alternative + +Refactoring M2M relationships into intermediate entities (making the join table a first-class Doctrine entity) is rejected — it requires schema changes, migration of every M2M relationship, and changes to all service code that manages these collections. The cost vastly exceeds the benefit. + +### Constraint + +Commit `728ae6791` (Dec 2025) demonstrates that calling `count()`, `getSnapshot()`, or accessing collection elements during `onFlush` triggers Doctrine hydration and causes production memory blowup on large collections. Any solution **must not initialize or hydrate the collection**. + +## Decision + +Query the join table directly via `$em->getConnection()->fetchFirstColumn()` to retrieve target entity IDs as a flat integer array. The Doctrine mapping metadata (`$mapping->joinTable->name`, `joinColumns[0]->name`, `inverseJoinColumns[0]->name`) provides all the SQL information needed — no entity hydration, minimal memory. + +### Implementation Detail: `fetchManyToManyIds()` + +The core of the solution is a private method on `AuditEventListener` that extracts join table metadata from the Doctrine `ManyToManyOwningSideMapping` and issues a single SQL query: + +```php +private function fetchManyToManyIds(PersistentCollection $collection, EntityManagerInterface $em): array +{ + $mapping = $collection->getMapping(); + $joinTable = $mapping->joinTable; + + // Extract column names from Doctrine's JoinTableMapping / JoinColumnMapping objects + $sourceColumn = $joinTable->joinColumns[0]->name; // e.g. "PresentationCategoryGroupID" + $targetColumn = $joinTable->inverseJoinColumns[0]->name; // e.g. "PresentationCategoryID" + $tableName = $joinTable->name; // e.g. "PresentationCategoryGroup_Categories" + + $ownerId = $collection->getOwner()->getId(); + + return $em->getConnection()->fetchFirstColumn( + "SELECT {$targetColumn} FROM {$tableName} WHERE {$sourceColumn} = ?", + [$ownerId] + ); +} +``` + +**Why this works without hydration:** `$em->getConnection()` returns the raw DBAL `Connection`, and `fetchFirstColumn()` returns a flat `array` of scalar values — no entity objects, no identity map interaction, no `PersistentCollection` initialization. + +#### Mapping metadata structure (Doctrine ORM 3.x) + +``` +ManyToManyOwningSideMapping + ├── fieldName : string ("categories") + ├── targetEntity : string ("models\\summit\\PresentationCategory") + ├── isOwningSide() : bool (true) + ├── isManyToMany() : bool (true) + └── joinTable : JoinTableMapping + ├── name : string ("PresentationCategoryGroup_Categories") + ├── joinColumns[] : JoinColumnMapping[] + │ └── [0].name : string ("PresentationCategoryGroupID") + └── inverseJoinColumns[]: JoinColumnMapping[] + └── [0].name : string ("PresentationCategoryID") +``` + +For each audited entity, the generated SQL maps to the physical join table: + +| Entity | Join Table | Source Column | Target Column | +|--------|-----------|---------------|---------------| +| `PresentationCategoryGroup::$categories` | `PresentationCategoryGroup_Categories` | `PresentationCategoryGroupID` | `PresentationCategoryID` | +| `SummitAttendee::$tags` | `SummitAttendee_Tags` | `SummitAttendeeID` | `TagID` | + +### Implementation Detail: `auditCollection()` + +`auditCollection()` is a pure routing/payload-resolution method — it determines what to audit and builds the payload, but does **not** call the audit strategy. Instead it returns a triple `[$subject, $payload, $eventType]` so the caller (`onFlush()`) handles dispatch: + +```php +foreach ($uow->getScheduledCollectionDeletions() as $col) { + list($subject, $payload, $eventType) = $this->auditCollection($col, $uow, IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE); + if (is_null($subject)) continue; + $strategy->audit($subject, $payload, $eventType, $ctx); +} +foreach ($uow->getScheduledCollectionUpdates() as $col) { + list($subject, $payload, $eventType) = $this->auditCollection($col, $uow, IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE); + if (is_null($subject)) continue; + $strategy->audit($subject, $payload, $eventType, $ctx); +} +``` + +The method receives `string $eventType` instead of `bool $isDeletion`, routes by collection type, and builds a minimal payload: + +```php +private function auditCollection($subject, $uow, string $eventType): array +{ + if (!$subject instanceof PersistentCollection) { + return [null, null, null]; + } + + $mapping = $subject->getMapping(); + if (!$mapping->isManyToMany()) { + // Non-M2M collections use EVENT_COLLECTION_UPDATE regardless of caller intent + return [$subject, [], IAuditStrategy::EVENT_COLLECTION_UPDATE]; + } + + if (!$mapping->isOwningSide()) { + Log::debug("AuditEventListener::Skipping audit for non-owning side of many-to-many collection"); + return [null, null, null]; + } + + $owner = $subject->getOwner(); + if ($owner === null) { + return [null, null, null]; + } + + $payload = [ + 'collection' => $subject, + ]; + + // For deletions with uninitialized collections, query the join table directly + // to get target IDs without hydrating entities (avoids memory blowup) + if ($eventType === IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE + && !$subject->isInitialized()) { + $em = $uow->getEntityManager(); + $payload['deleted_ids'] = $this->fetchManyToManyIds($subject, $em); + } + + return [$owner, $payload, $eventType]; +} +``` + +**Routing logic:** + +| Condition | Return | +|-----------|--------| +| Not a `PersistentCollection` | `[null, null, null]` — nothing to audit | +| Non-ManyToMany (OneToMany) | `[$subject, [], EVENT_COLLECTION_UPDATE]` — existing path, ignores passed `$eventType` | +| ManyToMany inverse side | `[null, null, null]` — only the owning side is audited to avoid duplicates | +| ManyToMany owning side, no owner | `[null, null, null]` — orphaned collection | +| ManyToMany owning side, deletion + uninitialized | `[$owner, {collection, deleted_ids}, $eventType]` — queries join table via `fetchManyToManyIds()` | +| ManyToMany owning side, otherwise | `[$owner, {collection}, $eventType]` — `getInsertDiff()`/`getDeleteDiff()` work on initialized collections | + +**Design rationale:** + +- **Pure routing, no dispatch** — the method returns a triple `[$subject, $payload, $eventType]` instead of calling `$strategy->audit()`. The caller (`onFlush()`) handles dispatch, keeping strategy and context dependencies out of the routing logic. Skip conditions return `[null, null, null]`; the caller checks `is_null($subject)` before dispatching. +- **No boolean-to-event-type conversion** — the caller expresses intent directly via the event type string. No `$isDeletion` parameter. +- **`$uow` stays scoped** — only accessed inside this method when the join table query is needed (uninitialized deletion). Never passed downstream in the payload. +- **Minimal payload** — only `['collection' => $subject]` for updates, or `['collection' => $subject, 'deleted_ids' => [...]]` for uninitialized deletions. No `is_deletion` flag, no `uow` reference. + +### Sequence Diagrams + +#### Flow 1: Owning entity deleted — collection uninitialized (the fix) + +This is the case ADR-0002 solves. The user deletes a `PresentationCategoryGroup` that has associated categories, but the `$categories` collection was never loaded into memory. + +![](0002-flow1.png) + +#### Flow 2: Collection updated — collection initialized (existing path) + +When a user adds or removes items from a M2M collection (e.g., `associateTrack2TrackGroup`), the collection is already initialized in memory. The existing `getInsertDiff()`/`getDeleteDiff()` mechanism works correctly. + +![](0002-flow2.png) + +### How it works (step by step) + +1. **`AuditEventListener::onFlush()`** iterates Doctrine's scheduled collection deletions and updates, calling `auditCollection()` with the appropriate event type for each (`EVENT_COLLECTION_MANYTOMANY_DELETE` for deletions, `EVENT_COLLECTION_MANYTOMANY_UPDATE` for updates). It destructures the returned triple `[$subject, $payload, $eventType]` and dispatches to `$strategy->audit()` — skip conditions return `[null, null, null]` and the caller checks `is_null($subject)` before dispatching. + +2. **`AuditEventListener::auditCollection($subject, $uow, string $eventType): array`** receives the event type directly and routes the collection: + - Non-`PersistentCollection` → skip + - Non-ManyToMany → emit `EVENT_COLLECTION_UPDATE` (existing path, ignores passed event type) + - ManyToMany inverse side → skip (only owning side is audited) + - ManyToMany owning side → uses the passed `$eventType`, continue to step 3 + +3. **For ManyToMany owning-side collections**, the event type is used directly — no boolean-to-event-type conversion. The event type is the single source of truth — no redundant `is_deletion` flag anywhere in the pipeline. + +4. **Uninitialized collection on deletion** (`$eventType === EVENT_COLLECTION_MANYTOMANY_DELETE && !$collection->isInitialized()`): + - Calls `fetchManyToManyIds()` to query the join table directly + - Passes the ID array in the raw payload as `deleted_ids` + +5. **Initialized collection** (update or deletion where collection was loaded): + - No join table query needed — `getInsertDiff()`/`getDeleteDiff()` work correctly + - `deleted_ids` is not set in the payload + +6. **`AuditLogOtlpStrategy::buildAuditLogData()`** collects telemetry metadata (counts, snapshot size, dirty flag) for the OpenTelemetry log entry. It uses a three-way conditional for M2M events: + - `!empty($change_set['deleted_ids'])` → uses `count($change_set['deleted_ids'])` as collection count, sets safe defaults for the rest (the collection is uninitialized — no `count()`, `getSnapshot()`, or `isDirty()` calls) + - `$collection->isInitialized()` → safe to call `count()`, `getSnapshot()`, `isDirty()` on the collection + - `else` → safe defaults (uninitialized collection without pre-queried IDs) + - The `EVENT_COLLECTION_UPDATE` branch uses the same `isInitialized()` guard for OneToMany collections + +7. **`AbstractAuditLogFormatter::handleManyToManyCollection()`** converts the raw payload into a typed DTO: + ```php + protected function handleManyToManyCollection(array $change_set): ?PersistentCollectionMetadata + ``` + - Reads `$change_set['collection']` and `$change_set['deleted_ids']` from the raw payload + - Validates the collection is a `PersistentCollection` instance + - Delegates to `PersistentCollectionMetadata::fromCollection()` which extracts `fieldName`, `targetEntity`, and `isInitialized` from the Doctrine mapping metadata + - Returns `null` if the payload is missing or invalid — all formatters share this validation logic + +8. **`AbstractAuditLogFormatter::processCollection()`** handles both ID sources via the DTO: + ```php + protected function processCollection(PersistentCollectionMetadata $metadata): ?array + ``` + - If `$metadata->preloadedDeletedIds` is non-empty → use those IDs directly, skip `getDeleteDiff()` + - If empty → use `$metadata->collection->getInsertDiff()` / `getDeleteDiff()` (the standard Doctrine diff mechanism) + - Returns a uniform array: `['field', 'target_entity', 'is_deletion', 'added_ids', 'removed_ids']` + +9. **All formatters** (generic and entity-specific) call `$this->handleManyToManyCollection($change_set)` to get the DTO, then pass it to `$this->processCollection($metadata)`. Entity-specific formatters add contextual information (entity name, summit name) to the formatted message. + +### Scope of changes + +| File | Change | +|------|--------| +| `app/Audit/PersistentCollectionMetadata.php` | **NEW** — Readonly DTO with `fromCollection()` factory; centralizes metadata extraction | +| `app/Audit/AuditEventListener.php` | Contains `fetchManyToManyIds()`, detection of uninitialized collections; `auditCollection()` returns `[$subject, $payload, $eventType]` triple, takes `string $eventType` instead of `bool $isDeletion`; caller (`onFlush()`) handles strategy dispatch; no `uow` or `is_deletion` in payload | +| `app/Audit/AbstractAuditLogFormatter.php` | Provides `handleManyToManyCollection()` generic base method (raw payload → DTO); `processCollection()` accepts `PersistentCollectionMetadata` instead of `(object, bool, array)` | +| `app/Audit/ConcreteFormatters/EntityManyToManyCollectionDeleteAuditLogFormatter.php` | Calls `handleManyToManyCollection()` to get DTO, passes to `processCollection()` | +| `app/Audit/ConcreteFormatters/EntityManyToManyCollectionUpdateAuditLogFormatter.php` | Same pattern — uses base `handleManyToManyCollection()` + `processCollection()` | +| `app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php` | Collapses M2M UPDATE/DELETE switch cases into one; calls base `handleManyToManyCollection()` for DTO, routes via `$this->event_type` | +| `app/Audit/ConcreteFormatters/PresentationCategoryGroupAuditLogFormatter.php` | Same pattern as SummitAttendee — base `handleManyToManyCollection()`, routing via `$this->event_type` | +| `app/Audit/AuditLogOtlpStrategy.php` | `buildAuditLogData()` guards all collection access with `isInitialized()` checks and `!empty($change_set['deleted_ids'])` three-way conditional for M2M; `getCollectionType()` error string fix | +| `config/audit_log.php` | Registers `PresentationCategoryGroup` formatter | + +### `PersistentCollectionMetadata` DTO + +The `PersistentCollectionMetadata` class (PHP 8.1 readonly constructor promotion) captures collection metadata at a single point, replacing ad-hoc extraction duplicated across formatters: + +```php +class PersistentCollectionMetadata +{ + public function __construct( + public readonly string $fieldName, // e.g. "categories" + public readonly string $targetEntity, // e.g. "models\\summit\\PresentationCategory" + public readonly bool $isInitialized, // collection initialization state at capture time + public readonly array $preloadedDeletedIds, // IDs from fetchManyToManyIds() (empty on updates) + public readonly PersistentCollection $collection, + ) {} + + public static function fromCollection( + PersistentCollection $collection, + array $preloadedDeletedIds = [] + ): self { /* extracts fieldName, targetEntity from $collection->getMapping() */ } +} +``` + +**Key design decisions:** + +- The DTO is created inside `AbstractAuditLogFormatter::handleManyToManyCollection()`, NOT in the listener. The listener emits a minimal raw payload (`['collection', 'deleted_ids']`) — no `is_deletion` flag. +- The DTO does **not** carry `isDeletion`. The event type (`EVENT_COLLECTION_MANYTOMANY_DELETE` vs `_UPDATE`) is the single source of truth — formatters derive deletion state from `$this->event_type`. This eliminates a redundant boolean that would duplicate knowledge already encoded in the event type. +- The DTO follows the same pattern as the existing `AuditContext` class — readonly constructor promotion, no getter methods. +- `isInitialized` is captured at construction time, so the value reflects the collection's state when the DTO is built, not when it is later consumed. + +### `AbstractAuditLogFormatter` refactoring + +The base formatter class provides two protected methods that centralize M2M collection handling for all formatters: + +#### `handleManyToManyCollection(array $change_set): ?PersistentCollectionMetadata` + +This method is the **entry point** for M2M processing in any formatter. It bridges the raw listener payload to the typed DTO: + +```php +protected function handleManyToManyCollection(array $change_set): ?PersistentCollectionMetadata +{ + if (!isset($change_set['collection'])) { + return null; + } + + $col = $change_set['collection']; + if (!($col instanceof PersistentCollection)) { + return null; + } + + $preloadedDeletedIds = $change_set['deleted_ids'] ?? []; + + return PersistentCollectionMetadata::fromCollection($col, $preloadedDeletedIds); +} +``` + +**Design rationale:** +- Validates that `$change_set['collection']` exists and is a `PersistentCollection` — rejects invalid payloads early with a `null` return. +- Extracts `$change_set['deleted_ids']` (defaults to `[]` when absent — the update path). +- Delegates metadata extraction (fieldName, targetEntity, isInitialized) to the DTO's `fromCollection()` factory — no duplication across formatters. +- Does **not** determine deletion vs update — that responsibility stays with `$this->event_type` (set by the `AuditLogFormatterFactory` at construction time). + +#### `processCollection(PersistentCollectionMetadata $metadata): ?array` + +This method computes the added/removed ID arrays from the DTO, handling both sources of IDs: + +```php +protected function processCollection(PersistentCollectionMetadata $metadata): ?array +``` + +**Two paths based on `$metadata->preloadedDeletedIds`:** + +1. **Non-empty `preloadedDeletedIds`** (uninitialized collection on deletion): Uses the pre-queried IDs from `fetchManyToManyIds()` directly. Does **not** call `getDeleteDiff()` or `getInsertDiff()` on the collection — the collection is uninitialized and those methods would return empty arrays or trigger hydration. + +2. **Empty `preloadedDeletedIds`** (initialized collection on update or deletion): Uses `$metadata->collection->getInsertDiff()` and `$metadata->collection->getDeleteDiff()` — the standard Doctrine diff mechanism that compares current items against the snapshot. + +**Both paths return the same structure:** +```php +[ + 'field' => $metadata->fieldName, // e.g. "categories" + 'target_entity' => $metadata->targetEntity, // e.g. "models\\summit\\PresentationCategory" + 'is_deletion' => $this->event_type === IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, + 'added_ids' => [...], // sorted, unique integer IDs + 'removed_ids' => [...], // sorted, unique integer IDs +] +``` + +Note that `is_deletion` in the return array is derived from `$this->event_type` — it is a computed output field for downstream formatting, not an input parameter. + +**Existing helper methods** remain unchanged: +- `extractCollectionEntityIds(array $entities): array` — extracts IDs from entity objects via `getId()`, returns sorted unique values. +- `formatManyToManyDetailedMessage(array $details, ...)` — formats the human-readable audit message from the array above. +- `buildChangeDetails(array $change_set)` — formats field-by-field diffs (used by `EVENT_ENTITY_UPDATE`, not M2M). + +### Concrete formatter refactoring + +All concrete formatters connect to the base class through the same two-step pattern: + +1. Call `$this->handleManyToManyCollection($change_set)` → get a typed `PersistentCollectionMetadata` DTO (or `null` if payload is invalid). +2. Pass the DTO to `$this->processCollection($metadata)` → get the uniform `['field', 'target_entity', 'is_deletion', 'added_ids', 'removed_ids']` array. + +The refactoring over PR #498 replaces direct `$change_set['collection']` access and manual `instanceof` checks with the base class methods. Each formatter no longer extracts mapping metadata or determines deletion semantics independently — that logic is centralized. + +#### Generic formatters + +These formatters handle M2M events for entities that do not have entity-specific formatting. They are registered in `config/audit_log.php` via the `strategies` array with optional `IChildEntityAuditLogFormatter` support. + +**`EntityManyToManyCollectionDeleteAuditLogFormatter`** + +- Constructor hard-codes `event_type = EVENT_COLLECTION_MANYTOMANY_DELETE` and accepts an optional `IChildEntityAuditLogFormatter`. +- Calls `$this->handleManyToManyCollection($change_set)` to get the DTO. +- **With child formatter** (and empty `preloadedDeletedIds`): accesses `$metadata->collection->getDeleteDiff()` directly and delegates per-entity formatting to the child formatter. This path handles entities that need rich per-item audit messages. +- **Without child formatter** (or with `preloadedDeletedIds`): calls `$this->processCollection($metadata)` and uses `formatManyToManyDetailedMessage()` to produce a summary with removed IDs. +- The `preloadedDeletedIds` check ensures the child formatter path is skipped when the collection is uninitialized (pre-queried IDs cannot produce entity objects for child formatting). + +```php +public function format($subject, array $change_set): ?string +{ + $metadata = $this->handleManyToManyCollection($change_set); + if (!$metadata) return null; + + if ($this->child_entity_formatter != null && empty($metadata->preloadedDeletedIds)) { + // Child formatter path — rich per-entity messages + $deleteDiff = $metadata->collection->getDeleteDiff(); + // ... delegate each entity to child formatter + } else { + // Generic path — summary with IDs + $collectionData = $this->processCollection($metadata); + // ... use formatManyToManyDetailedMessage() + } +} +``` + +**`EntityManyToManyCollectionUpdateAuditLogFormatter`** + +- Constructor hard-codes `event_type = EVENT_COLLECTION_MANYTOMANY_UPDATE` and accepts an optional `IChildEntityAuditLogFormatter`. +- Same pattern as the delete formatter but handles both `getInsertDiff()` and `getDeleteDiff()` (a single update event can contain both additions and removals). +- **With child formatter**: iterates insert diff and delete diff separately, delegating each to the child formatter with `CHILD_ENTITY_CREATION` or `CHILD_ENTITY_DELETION`. +- **Without child formatter**: calls `$this->processCollection($metadata)` and uses `formatManyToManyDetailedMessage()`. +- No `preloadedDeletedIds` check needed — update events always have initialized collections (the user interacted with the collection in memory). + +**Refactoring over PR #498:** Both generic formatters replace manual `$change_set['collection']` extraction, `instanceof PersistentCollection` checks, and direct `$collection->getMapping()` calls with a single `$this->handleManyToManyCollection($change_set)` call. The DTO provides typed access to `fieldName`, `targetEntity`, `preloadedDeletedIds`, and the collection itself. + +#### Entity-specific formatters + +These formatters handle all event types for a specific entity (creation, update, deletion, M2M update, M2M delete) within a single `format()` method using a `switch` on `$this->event_type`. + +**`SummitAttendeeAuditLogFormatter`** + +- Handles `SummitAttendee` entities — audits `$tags` M2M collection changes. +- The `EVENT_COLLECTION_MANYTOMANY_UPDATE` and `EVENT_COLLECTION_MANYTOMANY_DELETE` cases collapse into a single handler: + +```php +case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE: +case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE: + return $this->handleAttendeeManyToManyCollection($change_set, $id, $name); +``` + +- `handleAttendeeManyToManyCollection()` follows the two-step base class pattern: + 1. `$metadata = $this->handleManyToManyCollection($change_set)` — raw payload → DTO + 2. `$collectionData = $this->processCollection($metadata)` — DTO → ID arrays + 3. Routes to `formatManyToManyUpdate()` or `formatManyToManyDelete()` based on `$this->event_type === IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE` + +- The private `formatManyToManyUpdate()` and `formatManyToManyDelete()` methods produce attendee-specific messages with entity context (`$id`, `$name`, field, target entity, IDs). + +**`PresentationCategoryGroupAuditLogFormatter`** + +- Handles `PresentationCategoryGroup` entities — audits `$categories` M2M collection changes. +- Same collapsed switch pattern as `SummitAttendee`: + +```php +case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE: +case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE: + return $this->handleCategoryGroupManyToManyCollection($change_set, $id, $name, $summit_name); +``` + +- `handleCategoryGroupManyToManyCollection()` follows the identical two-step pattern, adding `$summit_name` as additional context to the formatted messages. +- Routes to `formatManyToManyUpdate()` or `formatManyToManyDelete()` based on `$this->event_type`. + +**Refactoring over PR #498:** Entity-specific formatters replace separate `EVENT_COLLECTION_MANYTOMANY_UPDATE` and `EVENT_COLLECTION_MANYTOMANY_DELETE` switch cases (each with its own inline collection extraction logic) with a single collapsed case that delegates to the base class `handleManyToManyCollection()` + `processCollection()` chain. Deletion vs update routing uses `$this->event_type` — no `is_deletion` boolean from the payload or DTO. + +#### Adding M2M audit to a new entity + +To add M2M audit support for a new entity: + +1. Add a combined `EVENT_COLLECTION_MANYTOMANY_UPDATE`/`DELETE` case to the entity's formatter's `switch`: + ```php + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE: + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE: + return $this->handleEntityManyToManyCollection($change_set, $id, $name); + ``` +2. Implement the private handler following the two-step pattern: + ```php + private function handleEntityManyToManyCollection(array $change_set, $id, $name): ?string + { + $metadata = $this->handleManyToManyCollection($change_set); + if (!$metadata) return null; + $collectionData = $this->processCollection($metadata); + if (!$collectionData) return null; + return $this->event_type === IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE + ? $this->formatManyToManyDelete($collectionData, $id, $name) + : $this->formatManyToManyUpdate($collectionData, $id, $name); + } + ``` +3. Implement private `formatManyToManyUpdate()` and `formatManyToManyDelete()` with entity-specific message formatting. +4. Register the entity in `config/audit_log.php` if not already present. + +Current entities with M2M audit support: +- `SummitAttendee` — `$tags` collection (`SummitAttendee_Tags`) +- `PresentationCategoryGroup` — `$categories` collection (`PresentationCategoryGroup_Categories`) + +### Test infrastructure + +`PersistentCollection` is `final` in Doctrine ORM 3.x and cannot be mocked with Mockery. A `PersistentCollectionFactory` test helper (`tests/OpenTelemetry/Formatters/Support/PersistentCollectionFactory.php`) constructs real `PersistentCollection` instances with proper `ManyToManyOwningSideMapping` objects, configurable snapshots, and initialization state. The factory also provides a `createMetadata()` convenience method that wraps `create()` + `PersistentCollectionMetadata::fromCollection()` for tests that work with the DTO directly. + +A dedicated unit test (`tests/OpenTelemetry/PersistentCollectionMetadataTest.php`) validates the DTO's `fromCollection()` factory: correct extraction of `fieldName` and `targetEntity` from mapping metadata, `isInitialized` state capture, `preloadedDeletedIds` pass-through, and collection reference retention. + +An integration test (`tests/OpenTelemetry/FetchManyToManyIdsIntegrationTest.php`) verifies that `fetchManyToManyIds()` produces correct results against a real MySQL database using real Doctrine entity mappings. It asserts both the returned IDs and the underlying Doctrine mapping metadata field names (join table, source column, target column) for each audited entity. + +A listener unit test (`tests/OpenTelemetry/AuditEventListenerTest.php`) exercises `auditCollection()` via reflection, asserting on the returned `[$subject, $payload, $eventType]` triple. It verifies routing logic (skip non-`PersistentCollection`, skip non-owning side, route non-M2M to `EVENT_COLLECTION_UPDATE`), correct event type propagation for M2M collections, payload structure (presence/absence of `deleted_ids`), and the conditional `fetchManyToManyIds()` call for uninitialized deletions. + +### Implementation Detail: `AuditLogOtlpStrategy::buildAuditLogData()` + +`AuditLogOtlpStrategy::buildAuditLogData()` collects telemetry metadata (counts, snapshot size, dirty flag) from `PersistentCollection` instances for OpenTelemetry log entries. The collection-related metadata fields are: + +| Field | Description | +|-------|-------------| +| `audit.collection_type` | Short class name of the target entity (e.g. `"PresentationCategory"`) | +| `audit.collection_count` | Total number of items in the collection (or count of deleted IDs) | +| `audit.collection_current_count` | Current item count from `count($collection)` | +| `audit.collection_snapshot_count` | Snapshot size from `count($collection->getSnapshot())` | +| `audit.collection_is_dirty` | Whether the collection has pending changes from `$collection->isDirty()` | + +#### Safety rule: never access uninitialized collections + +**`buildAuditLogData()` must never call `count()`, `getSnapshot()`, or `isDirty()` on an uninitialized `PersistentCollection`** — all three trigger Doctrine lazy loading, which hydrates every related entity into memory and causes the same production memory blowup this ADR addresses (see Constraint section above). + +The private helper `getCollectionChanges()` calls all three dangerous methods: + +```php +private function getCollectionChanges(PersistentCollection $collection, array $change_set): array +{ + return [ + 'current_count' => count($collection), // triggers hydration if uninitialized + 'snapshot_count' => count($collection->getSnapshot()), // triggers hydration if uninitialized + 'is_dirty' => $collection->isDirty(), // triggers hydration if uninitialized + ]; +} +``` + +Every call site that invokes `count($collection)` or `getCollectionChanges()` **must** first confirm the collection is safe to access. The guard strategy differs by event type. + +#### `EVENT_COLLECTION_UPDATE` branch (OneToMany collections) + +For non-M2M collections, the `$subject` itself is the `PersistentCollection`. The guard checks `$subject->isInitialized()` before any access: + +```php +case IAuditStrategy::EVENT_COLLECTION_UPDATE: + if ($subject instanceof PersistentCollection) { + $data['audit.collection_type'] = $this->getCollectionType($subject); + + // Guard against uninitialized collections — calling count() + // or getSnapshot() would trigger Doctrine lazy loading. + if ($subject->isInitialized()) { + $data['audit.collection_count'] = count($subject); + $changes = $this->getCollectionChanges($subject, $change_set); + $data['audit.collection_current_count'] = $changes['current_count']; + $data['audit.collection_snapshot_count'] = $changes['snapshot_count']; + $data['audit.collection_is_dirty'] = $changes['is_dirty'] ? 'true' : 'false'; + } else { + // Safe defaults — no hydration + $data['audit.collection_count'] = 0; + $data['audit.collection_current_count'] = 0; + $data['audit.collection_snapshot_count'] = 0; + $data['audit.collection_is_dirty'] = 'true'; + } + } + break; +``` + +**Key points:** +- `$subject->isInitialized()` is the only safe method to call on a potentially uninitialized collection — it reads an internal boolean flag without triggering any database query. +- `getCollectionType($subject)` is safe regardless of initialization state — it reads mapping metadata (`$collection->getMapping()->targetEntity`), which is always available without hydration. +- When the collection is not initialized, the method sets safe defaults: zero counts and `is_dirty = 'true'` (conservative assumption). + +#### `EVENT_COLLECTION_MANYTOMANY_UPDATE` / `_DELETE` branch (M2M collections) + +For M2M events, the collection is inside the payload at `$change_set['collection']` (the `$subject` passed to `audit()` is the owner entity, not the collection). This branch uses a **three-way conditional** because there are three distinct states: + +```php +case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE: +case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE: + if (isset($change_set['collection']) && $change_set['collection'] instanceof PersistentCollection) { + $collection = $change_set['collection']; + $data['audit.collection_type'] = $this->getCollectionType($collection); + + // When deleted_ids is present the collection is uninitialized — + // calling count() or getSnapshot() would hydrate all entities + // and cause the memory blowup documented in ADR-0002. + if (!empty($change_set['deleted_ids'])) { + $data['audit.collection_count'] = count($change_set['deleted_ids']); + $data['audit.collection_current_count'] = 0; + $data['audit.collection_snapshot_count'] = 0; + $data['audit.collection_is_dirty'] = 'true'; + } elseif ($collection->isInitialized()) { + $data['audit.collection_count'] = count($collection); + $changes = $this->getCollectionChanges($collection, $change_set); + $data['audit.collection_current_count'] = $changes['current_count']; + $data['audit.collection_snapshot_count'] = $changes['snapshot_count']; + $data['audit.collection_is_dirty'] = $changes['is_dirty'] ? 'true' : 'false'; + } else { + // Uninitialized collection without deleted_ids — safe defaults + $data['audit.collection_count'] = 0; + $data['audit.collection_current_count'] = 0; + $data['audit.collection_snapshot_count'] = 0; + $data['audit.collection_is_dirty'] = 'true'; + } + } + break; +``` + +**The three conditions in detail:** + +1. **`!empty($change_set['deleted_ids'])`** — The `deleted_ids` key is present and non-empty. This means `auditCollection()` detected an uninitialized collection during a deletion event and queried the join table via `fetchManyToManyIds()` to pre-load the target IDs. The collection itself is **uninitialized** — it is unsafe to call `count()`, `getSnapshot()`, or `isDirty()` on it. Instead, `count($change_set['deleted_ids'])` provides the collection count from the pre-queried ID array. Current count and snapshot count are set to `0` (the collection is about to be deleted), and `is_dirty` is `'true'` (conservative — it is being deleted). + +2. **`$collection->isInitialized()`** — No `deleted_ids` in the payload, and the collection **is** initialized. This is the normal update path — the user added or removed items from the collection, so Doctrine loaded it into memory. It is safe to call `count($collection)`, `getCollectionChanges()` (which internally calls `count()`, `getSnapshot()`, and `isDirty()`). This is also the path for initialized deletions (the collection was accessed before the owning entity was removed). + +3. **`else`** — No `deleted_ids` in the payload, and the collection is **not** initialized. This is a fallback safety net — it should not happen in normal flow (uninitialized deletions always produce `deleted_ids`, and updates always have initialized collections). But if it does happen due to an edge case, the method sets safe defaults rather than risking hydration. Zero counts and `is_dirty = 'true'`. + +**Why `!empty($change_set['deleted_ids'])` is checked first (before `isInitialized()`):** + +The `deleted_ids` check comes first because it is the **most specific** condition — when present, it definitively identifies the uninitialized deletion path and provides a known-safe data source (`count($change_set['deleted_ids'])`) without needing to touch the collection at all. Checking `isInitialized()` first would still work (the collection is not initialized when `deleted_ids` is present), but would fall into the `else` branch and lose the pre-queried ID count information. + +#### Summary of guards + +| Event type | Condition | Collection access | Data source | +|-----------|-----------|-------------------|-------------| +| `EVENT_COLLECTION_UPDATE` | `$subject->isInitialized() === true` | `count()`, `getSnapshot()`, `isDirty()` | Collection directly | +| `EVENT_COLLECTION_UPDATE` | `$subject->isInitialized() === false` | **None** | Safe defaults (0, 0, 0, 'true') | +| `EVENT_COLLECTION_MANYTOMANY_*` | `!empty($change_set['deleted_ids'])` | **None** | `count($change_set['deleted_ids'])` | +| `EVENT_COLLECTION_MANYTOMANY_*` | `$collection->isInitialized() === true` | `count()`, `getSnapshot()`, `isDirty()` | Collection directly | +| `EVENT_COLLECTION_MANYTOMANY_*` | `$collection->isInitialized() === false` | **None** | Safe defaults (0, 0, 0, 'true') | + +**Invariant:** No code path reaches `count($collection)`, `$collection->getSnapshot()`, or `$collection->isDirty()` without first confirming `$collection->isInitialized() === true`. + +## Consequences + +- **Uninitialized collection deletions produce audit entries** with the correct target IDs, retrieved via a single lightweight SQL query per collection. +- **No entity hydration** — the solution reads only integer IDs from the join table, avoiding the memory blowup that invalidated previous approaches. `AuditLogOtlpStrategy::buildAuditLogData()` enforces `isInitialized()` guards and the `!empty($change_set['deleted_ids'])` three-way conditional (see "Implementation Detail: `AuditLogOtlpStrategy::buildAuditLogData()`") so the strategy layer also never triggers hydration. +- **Existing update path unchanged** — when a collection is initialized (user adds/removes items), `getInsertDiff()`/`getDeleteDiff()` continue to work as before. +- **Boolean flag anti-patterns eliminated** — `processCollection()` does not take `bool $isDeletion` and `array $preloadedDeletedIds` as separate parameters (encapsulated in the DTO). The listener's `auditCollection()` method receives the event type string directly instead of `bool $isDeletion` — callers pass `EVENT_COLLECTION_MANYTOMANY_DELETE` or `_UPDATE` explicitly. The listener payload does not carry `is_deletion`. Formatters derive deletion semantics from `$this->event_type`. +- **Centralized payload validation** — `handleManyToManyCollection()` in the base class validates and extracts the raw payload into a DTO once, eliminating duplicate `isset($change_set['collection'])` / `instanceof` checks across all formatters. +- **Extensible** — adding M2M audit to a new entity follows a documented pattern (see "Adding M2M audit to a new entity" under Concrete formatter refactoring). +- **The `uow` is not leaked** into formatter payloads — the `EntityManager` is accessed only inside the listener when needed. + +## References + +- `app/Audit/PersistentCollectionMetadata.php` — readonly DTO with `fromCollection()` factory +- `app/Audit/AuditEventListener.php` — listener with `fetchManyToManyIds()` +- `app/Audit/AbstractAuditLogFormatter.php` — `handleManyToManyCollection()` (raw payload → DTO) and `processCollection(PersistentCollectionMetadata)` +- `app/Audit/AuditLogOtlpStrategy.php` — `buildAuditLogData()` with `isInitialized()` guards and `deleted_ids` three-way conditional diff --git a/app/Audit/AbstractAuditLogFormatter.php b/app/Audit/AbstractAuditLogFormatter.php index 94f614e84..932cf5874 100644 --- a/app/Audit/AbstractAuditLogFormatter.php +++ b/app/Audit/AbstractAuditLogFormatter.php @@ -3,6 +3,8 @@ namespace App\Audit; use App\Audit\Utils\DateFormatter; +use Doctrine\ORM\PersistentCollection; +use Illuminate\Support\Facades\Log; /** * Copyright 2025 OpenStack Foundation @@ -32,10 +34,59 @@ final public function setContext(AuditContext $ctx): void $this->ctx = $ctx; } + protected function processCollection( + object $col, + bool $isDeletion = false + ): ?array + { + if (!($col instanceof PersistentCollection)) { + return null; + } + + $mapping = $col->getMapping(); + + $addedEntities = $col->getInsertDiff(); + $removedEntities = $col->getDeleteDiff(); + + $addedIds = $this->extractCollectionEntityIds($addedEntities); + $removedIds = $this->extractCollectionEntityIds($removedEntities); + + + return [ + 'field' => $mapping->fieldName ?? 'unknown', + 'target_entity' => $mapping->targetEntity ?? 'unknown', + 'is_deletion' => $isDeletion, + 'added_ids' => $addedIds, + 'removed_ids' => $removedIds, + ]; + } + + + /** + * Extract IDs from entity objects in collection + */ + protected function extractCollectionEntityIds(array $entities): array + { + $ids = []; + foreach ($entities as $entity) { + if (method_exists($entity, 'getId')) { + $id = $entity->getId(); + if ($id !== null) { + $ids[] = $id; + } + } + } + + $uniqueIds = array_unique($ids); + sort($uniqueIds); + + return array_values($uniqueIds); + } + protected function getUserInfo(): string { if (app()->runningInConsole()) { - return 'Worker Job'; + return 'Worker Job'; } if (!$this->ctx) { return 'Unknown (unknown)'; @@ -129,5 +180,57 @@ protected function formatFieldChange(string $prop_name, $old_value, $new_value): return sprintf("Property \"%s\" has changed from \"%s\" to \"%s\"", $prop_name, $old_display, $new_display); } + /** + * Build detailed message for many-to-many collection changes + */ + protected function buildManyToManyDetailedMessage(PersistentCollection $collection, array $insertDiff, array $deleteDiff): array + { + $fieldName = 'unknown'; + $targetEntity = 'unknown'; + + try { + $mapping = $collection->getMapping(); + $fieldName = $mapping->fieldName ?? 'unknown'; + $targetEntity = $mapping->targetEntity ?? 'unknown'; + if ($targetEntity) { + $targetEntity = class_basename($targetEntity); + } + } catch (\Exception $e) { + Log::debug("AbstractAuditLogFormatter::Could not extract collection metadata: " . $e->getMessage()); + } + + $addedIds = $this->extractCollectionEntityIds($insertDiff); + $removedIds = $this->extractCollectionEntityIds($deleteDiff); + + return [ + 'field' => $fieldName, + 'target_entity' => $targetEntity, + 'added_ids' => $addedIds, + 'removed_ids' => $removedIds, + ]; + } + + /** + * Format detailed message for many-to-many collection changes + */ + protected static function formatManyToManyDetailedMessage(array $details, int $addCount, int $removeCount, string $action): string + { + $field = $details['field'] ?? 'unknown'; + $target = $details['target_entity'] ?? 'unknown'; + $addedIds = $details['added_ids'] ?? []; + $removedIds = $details['removed_ids'] ?? []; + + $parts = []; + if (!empty($addedIds)) { + $parts[] = sprintf("Added %d %s(s): %s", $addCount, $target, implode(', ', $addedIds)); + } + if (!empty($removedIds)) { + $parts[] = sprintf("Removed %d %s(s): %s", $removeCount, $target, implode(', ', $removedIds)); + } + + $detailStr = implode(' | ', $parts); + return sprintf("Many-to-Many collection '%s' %s: %s", $field, $action, $detailStr); + } + abstract public function format(mixed $subject, array $change_set): ?string; } diff --git a/app/Audit/AuditEventListener.php b/app/Audit/AuditEventListener.php index b6cf8baf2..607cf2c98 100644 --- a/app/Audit/AuditEventListener.php +++ b/app/Audit/AuditEventListener.php @@ -14,6 +14,8 @@ use App\Audit\Interfaces\IAuditStrategy; use Doctrine\ORM\Event\OnFlushEventArgs; +use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\PersistentCollection; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Route; @@ -53,10 +55,13 @@ public function onFlush(OnFlushEventArgs $eventArgs): void foreach ($uow->getScheduledEntityDeletions() as $entity) { $strategy->audit($entity, [], IAuditStrategy::EVENT_ENTITY_DELETION, $ctx); } - + foreach ($uow->getScheduledCollectionDeletions() as $col) { + $this->auditCollection($col, $strategy, $ctx, $uow, true); + } foreach ($uow->getScheduledCollectionUpdates() as $col) { - $strategy->audit($col, [], IAuditStrategy::EVENT_COLLECTION_UPDATE, $ctx); + $this->auditCollection($col, $strategy, $ctx, $uow, false); } + } catch (\Exception $e) { Log::error('Audit event listener failed', [ 'error' => $e->getMessage(), @@ -98,7 +103,7 @@ private function buildAuditContext(): AuditContext $member = $memberRepo->findOneBy(["user_external_id" => $userExternalId]); } - //$ui = app()->bound('ui.context') ? app('ui.context') : []; + $ui = []; $req = request(); $rawRoute = null; @@ -127,4 +132,43 @@ private function buildAuditContext(): AuditContext rawRoute: $rawRoute ); } + + /** + * Audit collection changes + * Only determines if it's ManyToMany and emits appropriate event + */ + private function auditCollection($subject, IAuditStrategy $strategy, AuditContext $ctx, $uow, bool $isDeletion = false): void + { + if (!$subject instanceof PersistentCollection) { + return; + } + + $mapping = $subject->getMapping(); + if (!$mapping->isManyToMany()) { + $strategy->audit($subject, [], IAuditStrategy::EVENT_COLLECTION_UPDATE, $ctx); + return; + } + + $isOwningSide = $mapping->isOwningSide(); + if (!$isOwningSide) { + Log::debug("AuditEventListerner::Skipping audit for non-owning side of many-to-many collection"); + return; + } + + $owner = $subject->getOwner(); + if ($owner === null) { + return; + } + + $payload = [ + 'collection' => $subject, + 'uow' => $uow, + 'is_deletion' => $isDeletion, + ]; + $eventType = $isDeletion + ? IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE + : IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE; + + $strategy->audit($owner, $payload, $eventType, $ctx); + } } diff --git a/app/Audit/AuditLogFormatterFactory.php b/app/Audit/AuditLogFormatterFactory.php index a66d2032b..155ebce56 100644 --- a/app/Audit/AuditLogFormatterFactory.php +++ b/app/Audit/AuditLogFormatterFactory.php @@ -18,6 +18,8 @@ use App\Audit\ConcreteFormatters\EntityCollectionUpdateAuditLogFormatter; use App\Audit\ConcreteFormatters\EntityCreationAuditLogFormatter; use App\Audit\ConcreteFormatters\EntityDeletionAuditLogFormatter; +use App\Audit\ConcreteFormatters\EntityManyToManyCollectionUpdateAuditLogFormatter; +use App\Audit\ConcreteFormatters\EntityManyToManyCollectionDeleteAuditLogFormatter; use App\Audit\ConcreteFormatters\EntityUpdateAuditLogFormatter; use App\Audit\Interfaces\IAuditStrategy; use Doctrine\ORM\PersistentCollection; @@ -57,9 +59,7 @@ public function make(AuditContext $ctx, $subject, string $event_type): ?IAuditLo ); if (method_exists($subject, 'getTypeClass')) { $type = $subject->getTypeClass(); - // Your log shows this is ClassMetadata if ($type instanceof ClassMetadata) { - // Doctrine supports either getName() or public $name $targetEntity = method_exists($type, 'getName') ? $type->getName() : ($type->name ?? null); } elseif (is_string($type)) { $targetEntity = $type; @@ -71,7 +71,6 @@ public function make(AuditContext $ctx, $subject, string $event_type): ?IAuditLo $targetEntity = $mapping['targetEntity'] ?? null; Log::debug("AuditLogFormatterFactory::make getMapping targetEntity {$targetEntity}"); } else { - // last-resort: read private association metadata (still no hydration) $ref = new \ReflectionObject($subject); foreach (['association', 'mapping', 'associationMapping'] as $propName) { if ($ref->hasProperty($propName)) { @@ -107,6 +106,18 @@ public function make(AuditContext $ctx, $subject, string $event_type): ?IAuditLo $formatter = new EntityCollectionUpdateAuditLogFormatter($child_entity_formatter); break; + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE: + $formatter = $this->getFormatterByContext($subject, $event_type, $ctx); + if (is_null($formatter)) { + $formatter = new EntityManyToManyCollectionUpdateAuditLogFormatter(); + } + break; + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE: + $formatter = $this->getFormatterByContext($subject, $event_type, $ctx); + if (is_null($formatter)) { + $formatter = new EntityManyToManyCollectionDeleteAuditLogFormatter(); + } + break; case IAuditStrategy::EVENT_ENTITY_CREATION: $formatter = $this->getFormatterByContext($subject, $event_type, $ctx); if(is_null($formatter)) { diff --git a/app/Audit/AuditLogOtlpStrategy.php b/app/Audit/AuditLogOtlpStrategy.php index e5881b6e4..8e8da30af 100644 --- a/app/Audit/AuditLogOtlpStrategy.php +++ b/app/Audit/AuditLogOtlpStrategy.php @@ -176,6 +176,19 @@ private function buildAuditLogData($entity, $subject, array $change_set, string $data['audit.collection_is_dirty'] = $changes['is_dirty'] ? 'true' : 'false'; } break; + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE: + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE: + if (isset($change_set['collection']) && $change_set['collection'] instanceof PersistentCollection) { + $collection = $change_set['collection']; + $data['audit.collection_type'] = $this->getCollectionType($collection); + $data['audit.collection_count'] = count($collection); + + $changes = $this->getCollectionChanges($collection, $change_set); + $data['audit.collection_current_count'] = $changes['current_count']; + $data['audit.collection_snapshot_count'] = $changes['snapshot_count']; + $data['audit.collection_is_dirty'] = $changes['is_dirty'] ? 'true' : 'false'; + } + break; } return $data; @@ -184,19 +197,16 @@ private function buildAuditLogData($entity, $subject, array $change_set, string private function getCollectionType(PersistentCollection $collection): string { try { - if (!method_exists($collection, 'getMapping')) { - return 'unknown'; - } - + $mapping = $collection->getMapping(); + $targetEntity = $mapping->targetEntity ?? null; - if (!isset($mapping['targetEntity']) || empty($mapping['targetEntity'])) { + if (!$targetEntity) { return 'unknown'; } - - return class_basename($mapping['targetEntity']); + return class_basename($targetEntity); } catch (\Exception $ex) { - return 'unknown'; + return 'AuditLogOtlpStrategy:: unknown targetEntity'; } } @@ -216,6 +226,8 @@ private function mapEventTypeToAction(string $event_type): string IAuditStrategy::EVENT_ENTITY_UPDATE => IAuditStrategy::ACTION_UPDATE, IAuditStrategy::EVENT_ENTITY_DELETION => IAuditStrategy::ACTION_DELETE, IAuditStrategy::EVENT_COLLECTION_UPDATE => IAuditStrategy::ACTION_COLLECTION_UPDATE, + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE => IAuditStrategy::ACTION_COLLECTION_MANYTOMANY_UPDATE, + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE => IAuditStrategy::ACTION_COLLECTION_MANYTOMANY_DELETE, default => IAuditStrategy::ACTION_UNKNOWN }; } @@ -227,6 +239,8 @@ private function getLogMessage(string $event_type): string IAuditStrategy::EVENT_ENTITY_UPDATE => IAuditStrategy::LOG_MESSAGE_UPDATED, IAuditStrategy::EVENT_ENTITY_DELETION => IAuditStrategy::LOG_MESSAGE_DELETED, IAuditStrategy::EVENT_COLLECTION_UPDATE => IAuditStrategy::LOG_MESSAGE_COLLECTION_UPDATED, + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE => IAuditStrategy::LOG_MESSAGE_COLLECTION_UPDATED, + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE => IAuditStrategy::LOG_MESSAGE_DELETED, default => IAuditStrategy::LOG_MESSAGE_CHANGED }; } diff --git a/app/Audit/ConcreteFormatters/EntityManyToManyCollectionDeleteAuditLogFormatter.php b/app/Audit/ConcreteFormatters/EntityManyToManyCollectionDeleteAuditLogFormatter.php new file mode 100644 index 000000000..e25f0f711 --- /dev/null +++ b/app/Audit/ConcreteFormatters/EntityManyToManyCollectionDeleteAuditLogFormatter.php @@ -0,0 +1,85 @@ +child_entity_formatter = $child_entity_formatter; + } + + /** + * @inheritDoc + */ + public function format($subject, array $change_set): ?string + { + try { + + + $collection = is_array($change_set) && isset($change_set['collection']) + ? $change_set['collection'] + : null; + + if ($collection === null) { + return null; + } + + $changes = []; + $deleteDiff = $collection->getDeleteDiff(); + + if ($this->child_entity_formatter != null) { + foreach ($deleteDiff as $child_changed_entity) { + $changes[] = $this->child_entity_formatter + ->format($child_changed_entity, IChildEntityAuditLogFormatter::CHILD_ENTITY_DELETION); + } + + if (!empty($changes)) { + return implode("|", $changes); + } + } else { + $deleted_count = count($deleteDiff); + + if ($deleted_count > 0) { + $details = $this->buildManyToManyDetailedMessage($collection, [], $deleteDiff); + return self::formatManyToManyDetailedMessage($details, 0, $deleted_count, 'deleted'); + } + } + + return null; + } catch (\Throwable $e) { + Log::error(get_class($this) . " error: " . $e->getMessage()); + return null; + } + } + + +} diff --git a/app/Audit/ConcreteFormatters/EntityManyToManyCollectionUpdateAuditLogFormatter.php b/app/Audit/ConcreteFormatters/EntityManyToManyCollectionUpdateAuditLogFormatter.php new file mode 100644 index 000000000..2a79a9e4c --- /dev/null +++ b/app/Audit/ConcreteFormatters/EntityManyToManyCollectionUpdateAuditLogFormatter.php @@ -0,0 +1,91 @@ +child_entity_formatter = $child_entity_formatter; + } + + /** + * @inheritDoc + */ + public function format($subject, array $change_set): ?string + { + try { + + $collection = is_array($change_set) && isset($change_set['collection']) + ? $change_set['collection'] + : null; + + if ($collection === null) { + return null; + } + + $changes = []; + $insertDiff = $collection->getInsertDiff(); + $deleteDiff = $collection->getDeleteDiff(); + + if ($this->child_entity_formatter != null) { + foreach ($insertDiff as $child_changed_entity) { + $changes[] = $this->child_entity_formatter + ->format($child_changed_entity, IChildEntityAuditLogFormatter::CHILD_ENTITY_CREATION); + } + + foreach ($deleteDiff as $child_changed_entity) { + $changes[] = $this->child_entity_formatter + ->format($child_changed_entity, IChildEntityAuditLogFormatter::CHILD_ENTITY_DELETION); + } + + if (!empty($changes)) { + return implode("|", $changes); + } + } else { + $inserted_count = count($insertDiff); + $deleted_count = count($deleteDiff); + + if ($inserted_count > 0 || $deleted_count > 0) { + $details = $this->buildManyToManyDetailedMessage($collection, $insertDiff, $deleteDiff); + return self::formatManyToManyDetailedMessage($details, $inserted_count, $deleted_count, 'updated'); + } + } + + return null; + } catch (\Throwable $e) { + Log::error(get_class($this) . " error: " . $e->getMessage()); + return null; + } + } + + +} diff --git a/app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php b/app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php index 1e092a621..c5359d47e 100644 --- a/app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php +++ b/app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php @@ -42,11 +42,100 @@ public function format($subject, array $change_set): ?string case IAuditStrategy::EVENT_ENTITY_DELETION: return sprintf("Attendee (%s) '%s' deleted by user %s", $id, $name, $this->getUserInfo()); + + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE: + return $this->handleManyToManyCollection($subject, $change_set, $id, $name, false); + + case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE: + return $this->handleManyToManyCollection($subject, $change_set, $id, $name, true); } - } catch (\Exception $ex) { + } catch (\Throwable $ex) { Log::warning("SummitAttendeeAuditLogFormatter error: " . $ex->getMessage()); } return null; } + + private function handleManyToManyCollection(SummitAttendee $subject, array $change_set, $id, $name, bool $isDeletion): ?string + { + if (!isset($change_set['collection'])) { + return null; + } + + $col = $change_set['collection']; + + $collectionData = $this->processCollection($col, $isDeletion); + if (!$collectionData) { + return null; + } + + return $isDeletion + ? $this->formatManyToManyDelete($collectionData, $id, $name) + : $this->formatManyToManyUpdate($collectionData, $id, $name); + } + + private function formatManyToManyUpdate(array $collectionData, $id, $name): ?string + { + try { + $field = $collectionData['field'] ?? 'unknown'; + $targetEntity = $collectionData['target_entity'] ?? 'unknown'; + $added_ids = $collectionData['added_ids'] ?? []; + $removed_ids = $collectionData['removed_ids'] ?? []; + + $idsPart = ''; + if (!empty($added_ids)) { + $idsPart .= 'Added IDs: ' . json_encode($added_ids); + } + if (!empty($removed_ids)) { + $idsPart .= (!empty($added_ids) ? ', ' : '') . 'Removed IDs: ' . json_encode($removed_ids); + } + if (empty($idsPart)) { + $idsPart = 'No changes'; + } + + $description = sprintf( + "Attendee (%s) '%s', Field: %s, Target: %s, %s, by user %s", + $id, + $name, + $field, + class_basename($targetEntity), + $idsPart, + $this->getUserInfo() + ); + + return $description; + + } catch (\Throwable $ex) { + Log::warning("SummitAttendeeAuditLogFormatter::formatManyToManyUpdate error: " . $ex->getMessage()); + return sprintf("Attendee (%s) '%s' association updated by user %s", $id, $name, $this->getUserInfo()); + } + } + + private function formatManyToManyDelete(array $collectionData, $id, $name): ?string + { + try { + $removed_ids = $collectionData['removed_ids'] ?? []; + + + + $field = $collectionData['field'] ?? 'unknown'; + $targetEntity = $collectionData['target_entity'] ?? 'unknown'; + + $description = sprintf( + "Attendee (%s) '%s' association deleted: Field: %s, Target: %s, Removed IDs: %s, by user %s", + $id, + $name, + $field, + class_basename($targetEntity), + json_encode($removed_ids), + $this->getUserInfo() + ); + + return $description; + + } catch (\Throwable $ex) { + Log::warning("SummitAttendeeAuditLogFormatter::formatManyToManyDelete error: " . $ex->getMessage()); + return null; + } + } } diff --git a/app/Audit/Interfaces/IAuditStrategy.php b/app/Audit/Interfaces/IAuditStrategy.php index 4ed0a1a9c..02971e5c9 100644 --- a/app/Audit/Interfaces/IAuditStrategy.php +++ b/app/Audit/Interfaces/IAuditStrategy.php @@ -33,12 +33,16 @@ public function audit($subject, array $change_set, string $event_type, AuditCont public const EVENT_ENTITY_CREATION = 'event_entity_creation'; public const EVENT_ENTITY_DELETION = 'event_entity_deletion'; public const EVENT_ENTITY_UPDATE = 'event_entity_update'; + public const EVENT_COLLECTION_MANYTOMANY_UPDATE = 'event_collection_manytomany_update'; + public const EVENT_COLLECTION_MANYTOMANY_DELETE = 'event_collection_manytomany_delete'; public const ACTION_CREATE = 'create'; public const ACTION_UPDATE = 'update'; public const ACTION_DELETE = 'delete'; public const ACTION_COLLECTION_UPDATE = 'collection_update'; public const ACTION_UNKNOWN = 'unknown'; + public const ACTION_COLLECTION_MANYTOMANY_UPDATE = 'collection_manytomany_update'; + public const ACTION_COLLECTION_MANYTOMANY_DELETE = 'collection_manytomany_delete'; public const LOG_MESSAGE_CREATED = 'audit.entity.created'; public const LOG_MESSAGE_UPDATED = 'audit.entity.updated'; diff --git a/tests/OpenTelemetry/Formatters/AllFormattersIntegrationTest.php b/tests/OpenTelemetry/Formatters/AllFormattersIntegrationTest.php index 80b6267ef..728865ac6 100644 --- a/tests/OpenTelemetry/Formatters/AllFormattersIntegrationTest.php +++ b/tests/OpenTelemetry/Formatters/AllFormattersIntegrationTest.php @@ -86,6 +86,8 @@ private function isMainFormatter(string $className): bool 'EntityDeletionAuditLogFormatter', 'EntityUpdateAuditLogFormatter', 'EntityCollectionUpdateAuditLogFormatter', + 'EntityManyToManyCollectionDeleteAuditLogFormatter', + 'EntityManyToManyCollectionUpdateAuditLogFormatter', ]; return !in_array($reflection->getShortName(), $genericFormatters) && @@ -138,6 +140,8 @@ public function testAllFormattersHandleAllEventTypes(): void IAuditStrategy::EVENT_ENTITY_UPDATE, IAuditStrategy::EVENT_ENTITY_DELETION, IAuditStrategy::EVENT_COLLECTION_UPDATE, + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, ]; $errors = []; diff --git a/tests/OpenTelemetry/Formatters/EntityManyToManyCollectionDeleteAuditLogFormatterTest.php b/tests/OpenTelemetry/Formatters/EntityManyToManyCollectionDeleteAuditLogFormatterTest.php new file mode 100644 index 000000000..d356763ef --- /dev/null +++ b/tests/OpenTelemetry/Formatters/EntityManyToManyCollectionDeleteAuditLogFormatterTest.php @@ -0,0 +1,196 @@ +formatter = new EntityManyToManyCollectionDeleteAuditLogFormatter(); + + $this->childFormatterMock = Mockery::mock(IChildEntityAuditLogFormatter::class); + $this->formatterWithChildFormatter = new EntityManyToManyCollectionDeleteAuditLogFormatter($this->childFormatterMock); + } + + protected function tearDown(): void + { + Mockery::close(); + parent::tearDown(); + } + + public function testFormatWithoutContextReturnsNull(): void + { + $changeSet = ['collection' => $this->createMockCollection([], [1, 2, 3])]; + $result = $this->formatter->format(new \stdClass(), $changeSet); + $this->assertNull($result); + } + + public function testFormatWithoutCollectionReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $result = $this->formatter->format(new \stdClass(), []); + $this->assertNull($result); + } + + public function testFormatWithNullCollectionReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $result = $this->formatter->format(new \stdClass(), ['collection' => null]); + $this->assertNull($result); + } + + public function testFormatWithNonPersistentCollectionReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + + $badCollection = new class { + public function getInsertDiff(): array + { + return []; + } + public function getDeleteDiff(): array + { + return []; + } + }; + + $result = $this->formatter->format(new \stdClass(), ['collection' => $badCollection]); + $this->assertNull($result); + } + + public function testFormatterInitialization(): void + { + $this->assertNotNull($this->formatter); + $this->assertNotNull($this->formatterWithChildFormatter); + } + + public function testFormatWithOnlyAddedItemsReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $changeSet = ['collection' => $this->createMockCollection([1, 2], [])]; + $result = $this->formatter->format(new \stdClass(), $changeSet); + $this->assertNull($result); + } + + public function testFormatWithDeletedItemsAndChildFormatterMocksBehavior(): void + { + $this->childFormatterMock->shouldReceive('format') + ->andReturn(self::CHILD_FORMATTED_MESSAGE); + + $deletedItems = $this->createMultipleMockEntities(self::DELETED_ITEMS_COUNT); + $changeSet = ['collection' => $this->createMockCollection([], $deletedItems)]; + $result = $this->formatterWithChildFormatter->format(new \stdClass(), $changeSet); + + $this->assertNotNull($result); + $this->assertStringContainsString(self::CHILD_FORMATTED_MESSAGE, $result); + } + + public function testFormatWithChildFormatterButNoDeletedItems(): void + { + $this->childFormatterMock->shouldNotReceive('format'); + + $changeSet = ['collection' => $this->createMockCollection([], [])]; + $result = $this->formatterWithChildFormatter->format(new \stdClass(), $changeSet); + $this->assertNull($result); + } + + public function testFormatterExceptionHandlingReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $testErrorMessage = self::TEST_ERROR_MESSAGE; + + $badCollection = new class($testErrorMessage) { + public function __construct(private string $errorMessage) {} + + public function getDeleteDiff(): array + { + throw new \Exception($this->errorMessage); + } + }; + + $result = $this->formatter->format(new \stdClass(), ['collection' => $badCollection]); + $this->assertNull($result); + } + + private function createMockCollection(array $inserted = [], array $deleted = []) + { + $fieldNameMock = self::FIELD_NAME_MOCK; + $targetEntityMock = self::TARGET_ENTITY_MOCK; + + return new class($inserted, $deleted, $fieldNameMock, $targetEntityMock) { + public function __construct( + private array $inserted, + private array $deleted, + private string $fieldName, + private string $targetEntity + ) {} + + public function getInsertDiff(): array + { + return $this->inserted; + } + + public function getDeleteDiff(): array + { + return $this->deleted; + } + + public function getMapping(): array + { + return [ + 'fieldName' => $this->fieldName, + 'targetEntity' => $this->targetEntity, + ]; + } + }; + } + + private function createMultipleMockEntities(int $count): array + { + $entities = []; + for ($i = 0; $i < $count; $i++) { + $entities[] = $this->createMockEntity($i + 1); + } + return $entities; + } + + private function createMockEntity($id) + { + return new class($id) { + public function __construct(private int $id) {} + + public function getId() + { + return $this->id; + } + }; + } +} diff --git a/tests/OpenTelemetry/Formatters/EntityManyToManyCollectionUpdateAuditLogFormatterTest.php b/tests/OpenTelemetry/Formatters/EntityManyToManyCollectionUpdateAuditLogFormatterTest.php new file mode 100644 index 000000000..235a6954e --- /dev/null +++ b/tests/OpenTelemetry/Formatters/EntityManyToManyCollectionUpdateAuditLogFormatterTest.php @@ -0,0 +1,140 @@ +formatter = new EntityManyToManyCollectionUpdateAuditLogFormatter(); + + $this->childFormatterMock = Mockery::mock(IChildEntityAuditLogFormatter::class); + $this->formatterWithChildFormatter = new EntityManyToManyCollectionUpdateAuditLogFormatter($this->childFormatterMock); + } + + protected function tearDown(): void + { + Mockery::close(); + parent::tearDown(); + } + + public function testFormatWithoutContextReturnsNull(): void + { + $changeSet = ['collection' => $this->createMockCollection([], [])]; + $result = $this->formatter->format(new \stdClass(), $changeSet); + $this->assertNull($result); + } + + public function testFormatWithEmptyChangesetReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $result = $this->formatter->format(new \stdClass(), []); + $this->assertNull($result); + } + + public function testFormatWithNullCollectionReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $result = $this->formatter->format(new \stdClass(), ['collection' => null]); + $this->assertNull($result); + } + + public function testFormatWithCollectionNoChangesReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $collection = $this->createMockCollection([], []); + $result = $this->formatter->format(new \stdClass(), ['collection' => $collection]); + $this->assertNull($result); + } + + public function testFormatterExceptionHandlingReturnsNull(): void + { + $this->formatter->setContext(AuditContextBuilder::default()->build()); + $testErrorMessage = self::TEST_ERROR_MESSAGE; + + $badCollection = new class($testErrorMessage) { + public function __construct(private string $errorMessage) {} + + public function getInsertDiff(): array + { + throw new \Exception($this->errorMessage); + } + public function getDeleteDiff(): array + { + return []; + } + }; + + $result = $this->formatter->format(new \stdClass(), ['collection' => $badCollection]); + $this->assertNull($result); + } + + public function testFormatterInitialization(): void + { + $this->assertNotNull($this->formatter); + $this->assertNotNull($this->formatterWithChildFormatter); + } + + private function createMockCollection(array $inserted = [], array $deleted = []) + { + $fieldNameMock = self::FIELD_NAME_MOCK; + $targetEntityMock = self::TARGET_ENTITY_MOCK; + + return new class($inserted, $deleted, $fieldNameMock, $targetEntityMock) { + public function __construct( + private array $inserted, + private array $deleted, + private string $fieldName, + private string $targetEntity + ) {} + + public function getInsertDiff(): array + { + return $this->inserted; + } + + public function getDeleteDiff(): array + { + return $this->deleted; + } + + public function getMapping(): array + { + return [ + 'fieldName' => $this->fieldName, + 'targetEntity' => $this->targetEntity, + ]; + } + }; + } +} + + diff --git a/tests/OpenTelemetry/Formatters/SummitAttendeeAuditLogFormatterManyToManyTest.php b/tests/OpenTelemetry/Formatters/SummitAttendeeAuditLogFormatterManyToManyTest.php new file mode 100644 index 000000000..0e6a7c08c --- /dev/null +++ b/tests/OpenTelemetry/Formatters/SummitAttendeeAuditLogFormatterManyToManyTest.php @@ -0,0 +1,159 @@ +mockSubject = $this->createMockSubject(); + } + + protected function tearDown(): void + { + Mockery::close(); + parent::tearDown(); + } + + private function createMockSubject(): mixed + { + $mock = Mockery::mock('models\summit\SummitAttendee'); + $mock->shouldReceive('getId')->andReturn(self::ATTENDEE_ID); + $mock->shouldReceive('getFirstName')->andReturn(self::ATTENDEE_FIRST_NAME); + $mock->shouldReceive('getSurname')->andReturn(self::ATTENDEE_LAST_NAME); + return $mock; + } + + public function testManyToManyUpdateReturnsNullWithoutContext(): void + { + $formatter = new SummitAttendeeAuditLogFormatter( + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE + ); + + $result = $formatter->format($this->mockSubject, []); + $this->assertNull($result); + } + + public function testManyToManyDeleteReturnsNullWithoutContext(): void + { + $formatter = new SummitAttendeeAuditLogFormatter( + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE + ); + + $result = $formatter->format($this->mockSubject, []); + $this->assertNull($result); + } + + public function testManyToManyUpdateReturnsNullWithoutCollection(): void + { + $formatter = new SummitAttendeeAuditLogFormatter( + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE + ); + $formatter->setContext(AuditContextBuilder::default()->build()); + + $result = $formatter->format($this->mockSubject, []); + $this->assertNull($result); + } + + public function testManyToManyDeleteReturnsNullWithoutCollection(): void + { + $formatter = new SummitAttendeeAuditLogFormatter( + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE + ); + $formatter->setContext(AuditContextBuilder::default()->build()); + + $result = $formatter->format($this->mockSubject, []); + $this->assertNull($result); + } + + public function testFormatterReturnsNullForInvalidSubject(): void + { + $formatter = new SummitAttendeeAuditLogFormatter( + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE + ); + $formatter->setContext(AuditContextBuilder::default()->build()); + + $result = $formatter->format(new \stdClass(), []); + + $this->assertNull($result); + } + + public function testManyToManyDeleteReturnsNullWithoutRemovedIds(): void + { + $formatter = new SummitAttendeeAuditLogFormatter( + IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE + ); + $formatter->setContext(AuditContextBuilder::default()->build()); + + $collection = $this->createMockCollection([], []); + $uow = Mockery::mock(UnitOfWork::class); + + $result = $formatter->format($this->mockSubject, [ + 'collection' => $collection, + 'uow' => $uow + ]); + + $this->assertNull($result); + } + + + + + + + private function createMockCollection(array $inserted = [], array $deleted = []) + { + return new class($inserted, $deleted) { + public function __construct( + private array $inserted, + private array $deleted + ) {} + + public function getInsertDiff(): array + { + return $this->inserted; + } + + public function getDeleteDiff(): array + { + return $this->deleted; + } + + public function getMapping() + { + $meta = Mockery::mock(ClassMetadata::class); + $meta->fieldName = 'testField'; + $meta->targetEntity = 'TestEntity'; + return $meta; + } + }; + } +}