Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 118 additions & 1 deletion app/Audit/AbstractAuditLogFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -32,10 +34,73 @@ final public function setContext(AuditContext $ctx): void
$this->ctx = $ctx;
}

protected function handleManyToManyCollection(array $change_set): ?PersistentCollectionMetadata
{
if (!isset($change_set['collection'])) {
return null;
}

$collection = $change_set['collection'];
if (!($collection instanceof PersistentCollection)) {
return null;
}

$preloadedDeletedIds = $change_set['deleted_ids'] ?? [];
if (!is_array($preloadedDeletedIds)) {
$preloadedDeletedIds = [];
}

return PersistentCollectionMetadata::fromCollection($collection, $preloadedDeletedIds);
}

protected function processCollection(PersistentCollectionMetadata $metadata): ?array
{
$addedIds = [];
$removedIds = [];

if (!empty($metadata->preloadedDeletedIds)) {
$removedIds = array_values(array_unique(array_map('intval', $metadata->preloadedDeletedIds)));
sort($removedIds);
} else {
$addedIds = $this->extractCollectionEntityIds($metadata->collection->getInsertDiff());
$removedIds = $this->extractCollectionEntityIds($metadata->collection->getDeleteDiff());
}

return [
'field' => $metadata->fieldName,
'target_entity' => class_basename($metadata->targetEntity),
'is_deletion' => $this->event_type === Interfaces\IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE,
'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)';
Expand Down Expand Up @@ -129,5 +194,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;
}
124 changes: 118 additions & 6 deletions app/Audit/AuditEventListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

use App\Audit\Interfaces\IAuditStrategy;
use Doctrine\ORM\Event\OnFlushEventArgs;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\PersistentCollection;
use Illuminate\Support\Facades\App;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Route;
Expand All @@ -25,16 +27,16 @@
class AuditEventListener
{
private const ROUTE_METHOD_SEPARATOR = '|';

private $em;
public function onFlush(OnFlushEventArgs $eventArgs): void
{
if (app()->environment('testing')) {
return;
}
$em = $eventArgs->getObjectManager();
$uow = $em->getUnitOfWork();
$this->em = $eventArgs->getObjectManager();
$uow = $this->em->getUnitOfWork();
// Strategy selection based on environment configuration
$strategy = $this->getAuditStrategy($em);
$strategy = $this->getAuditStrategy($this->em);
if (!$strategy) {
return; // No audit strategy enabled
}
Expand All @@ -52,11 +54,23 @@ public function onFlush(OnFlushEventArgs $eventArgs): void

foreach ($uow->getScheduledEntityDeletions() as $entity) {
$strategy->audit($entity, [], IAuditStrategy::EVENT_ENTITY_DELETION, $ctx);
}
foreach ($uow->getScheduledCollectionDeletions() as $col) {
[$subject, $payload, $eventType] = $this->auditCollection($col, $uow, IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE);

if (!is_null($subject)) {
$strategy->audit($subject, $payload, $eventType, $ctx);
}
}

foreach ($uow->getScheduledCollectionUpdates() as $col) {
$strategy->audit($col, [], IAuditStrategy::EVENT_COLLECTION_UPDATE, $ctx);
[$subject, $payload, $eventType] = $this->auditCollection($col, $uow, IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE);

if (!is_null($subject)) {
$strategy->audit($subject, $payload, $eventType, $ctx);
}
}

} catch (\Exception $e) {
Log::error('Audit event listener failed', [
'error' => $e->getMessage(),
Expand Down Expand Up @@ -98,7 +112,7 @@ private function buildAuditContext(): AuditContext
$member = $memberRepo->findOneBy(["user_external_id" => $userExternalId]);
}

//$ui = app()->bound('ui.context') ? app('ui.context') : [];
$ui = [];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if $ui was previously populated from somewhere
rg -n -B5 -A5 '\$ui\s*=' app/Audit/AuditEventListener.php

# Search for any context that might provide app/flow values
rg -n "uiApp|uiFlow|ui_app|ui_flow" --type php -g '!tests/*'

Repository: OpenStackweb/summit-api

Length of output: 1022


🏁 Script executed:

cat -n app/Audit/AuditEventListener.php | sed -n '110,140p'

Repository: OpenStackweb/summit-api

Length of output: 1518


Remove unused $ui initialization or populate the array.

The $ui variable is initialized as an empty array at line 114, but it's never populated. Lines 134-135 attempt to access $ui['app'] and $ui['flow'], which will always be null due to the null coalescing operator. Either remove the initialization and use null directly, or add the logic to populate these array keys from the request/route context.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AuditEventListener.php` at line 114, The $ui array is initialized
but never populated, making later accesses to $ui['app'] and $ui['flow'] always
null; update the AuditEventListener code to either remove the unused $ui
initialization and use null/defaults directly where $ui['app']/$ui['flow'] are
read, or populate $ui from the current request/route context (e.g. pull values
from the Request or route attributes) before those reads so $ui['app'] and
$ui['flow'] contain the intended values; ensure you update the same method in
AuditEventListener where $ui is declared and where $ui['app']/$ui['flow'] are
accessed.


$req = request();
$rawRoute = null;
Expand Down Expand Up @@ -127,4 +141,102 @@ private function buildAuditContext(): AuditContext
rawRoute: $rawRoute
);
}

/**
* Audit collection changes
* Returns triple: [$subject, $payload, $eventType]
* Subject will be null if collection should not be audited
*
* @param object $subject The collection
* @param mixed $uow The UnitOfWork
* @param string $eventType The event type constant (EVENT_COLLECTION_MANYTOMANY_DELETE or EVENT_COLLECTION_MANYTOMANY_UPDATE)
* @return array [$subject, $payload, $eventType]
*/
private function auditCollection($subject, $uow, string $eventType): array
{
if (!$subject instanceof PersistentCollection) {
return [null, null, null];
}

$mapping = $subject->getMapping();

if (!$mapping->isManyToMany()) {
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];

if ($eventType === IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE
&& (
!$subject->isInitialized()
|| ($subject->isInitialized() && count($subject->getDeleteDiff()) === 0)
)) {
if ($this->em instanceof EntityManagerInterface) {
$payload['deleted_ids'] = $this->fetchManyToManyIds($subject, $this->em);
}
}

return [$owner, $payload, $eventType];
}


private function fetchManyToManyIds(PersistentCollection $collection, EntityManagerInterface $em): array
{
try {
$mapping = $collection->getMapping();
$joinTable = $mapping->joinTable;
$tableName = is_array($joinTable) ? ($joinTable['name'] ?? null) : ($joinTable->name ?? null);
$joinColumns = is_array($joinTable) ? ($joinTable['joinColumns'] ?? []) : ($joinTable->joinColumns ?? []);
$inverseJoinColumns = is_array($joinTable) ? ($joinTable['inverseJoinColumns'] ?? []) : ($joinTable->inverseJoinColumns ?? []);

$joinColumn = $joinColumns[0] ?? null;
$inverseJoinColumn = $inverseJoinColumns[0] ?? null;
$sourceColumn = is_array($joinColumn) ? ($joinColumn['name'] ?? null) : ($joinColumn->name ?? null);
$targetColumn = is_array($inverseJoinColumn) ? ($inverseJoinColumn['name'] ?? null) : ($inverseJoinColumn->name ?? null);

if (!$sourceColumn || !$targetColumn || !$tableName) {
return [];
}

$owner = $collection->getOwner();
if ($owner === null) {
return [];
}

$ownerId = method_exists($owner, 'getId') ? $owner->getId() : null;
if ($ownerId === null) {
$ownerMeta = $em->getClassMetadata(get_class($owner));
$ownerIds = $ownerMeta->getIdentifierValues($owner);
$ownerId = empty($ownerIds) ? null : reset($ownerIds);
}

if ($ownerId === null) {
return [];
}

$ids = $em->getConnection()->fetchFirstColumn(
"SELECT {$targetColumn} FROM {$tableName} WHERE {$sourceColumn} = ?",
[$ownerId]
);

return array_values(array_map('intval', $ids));

} catch (\Exception $e) {
Log::error("AuditEventListener::fetchManyToManyIds error: " . $e->getMessage(), [
'exception' => get_class($e),
'trace' => $e->getTraceAsString()
]);
return [];
}
}
}
17 changes: 14 additions & 3 deletions app/Audit/AuditLogFormatterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
use App\Audit\ConcreteFormatters\EntityCollectionUpdateAuditLogFormatter;
use App\Audit\ConcreteFormatters\EntityCreationAuditLogFormatter;
use App\Audit\ConcreteFormatters\EntityDeletionAuditLogFormatter;
use App\Audit\ConcreteFormatters\DefaultEntityManyToManyCollectionUpdateAuditLogFormatter;
use App\Audit\ConcreteFormatters\DefaultEntityManyToManyCollectionDeleteAuditLogFormatter;
use App\Audit\ConcreteFormatters\EntityUpdateAuditLogFormatter;
use App\Audit\Interfaces\IAuditStrategy;
use Doctrine\ORM\PersistentCollection;
Expand Down Expand Up @@ -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;
Expand All @@ -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)) {
Expand Down Expand Up @@ -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 DefaultEntityManyToManyCollectionUpdateAuditLogFormatter();
}
break;
case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE:
$formatter = $this->getFormatterByContext($subject, $event_type, $ctx);
if (is_null($formatter)) {
$formatter = new DefaultEntityManyToManyCollectionDeleteAuditLogFormatter();
}
break;
case IAuditStrategy::EVENT_ENTITY_CREATION:
$formatter = $this->getFormatterByContext($subject, $event_type, $ctx);
if(is_null($formatter)) {
Expand Down
Loading
Loading