Refactor subject conversion APIs and simplify usage in audit-related components

- Removed `convertEntityToSubjects` method from `SubjectConverterManager` and updated all references to use `getSubjectsForEntity`.
- Ensured `getSubjectsForEntity` always returns an array by wrapping single `Subject` instances.
- Replaced manual type handling in converters with `array_push` and improved code clarity in `AuditEvent2Trail` and related tests.
- Updated and fixed tests for the modified subject conversion logic to maintain coverage.
This commit is contained in:
2026-01-28 22:34:55 +01:00
parent 412aa213c8
commit c1a0234358
7 changed files with 25 additions and 55 deletions

View File

@@ -34,10 +34,10 @@ final readonly class AuditEvent2Trail implements AuditEvent2TrailInterface
$event->description->trans($this->translator)
: $event->description;
$targets = array_reduce(
$subjects = array_reduce(
$event->subjects,
function (array $carry, mixed $item): array {
return array_merge($carry, $this->subjectConverterManager->convertEntityToSubjects($item));
return array_merge($carry, $this->subjectConverterManager->getSubjectsForEntity($item));
},
[]
);
@@ -50,7 +50,7 @@ final readonly class AuditEvent2Trail implements AuditEvent2TrailInterface
$this->clock->now(),
$user instanceof User ? $user : null,
$description,
$targets,
array_map(fn (Subject $subject) => $subject->asArray(), $subjects),
$event->metadata,
);
}

View File

@@ -22,23 +22,7 @@ final readonly class SubjectConverterManager implements SubjectConverterManagerI
private iterable $converters,
) {}
/**
* @return list<array>
*
* @throws ConvertSubjectException
*/
public function convertEntityToSubjects(mixed $subject): array
{
$subjects = $this->getSubjectsForEntity($subject);
if ($subjects instanceof Subject) {
return [$subjects->asArray()];
}
return array_map(static fn (Subject $subject) => $subject->asArray(), $subjects);
}
public function getSubjectsForEntity(mixed $subject): Subject|array
public function getSubjectsForEntity(mixed $subject): array
{
foreach ($this->converters as $converter) {
if ($converter instanceof SubjectConverterManagerAwareInterface) {
@@ -46,7 +30,9 @@ final readonly class SubjectConverterManager implements SubjectConverterManagerI
}
if ($converter->supportsConvert($subject)) {
return $converter->convert($subject);
$subjects = $converter->convert($subject);
return $subjects instanceof Subject ? [$subjects] : $subjects;
}
}

View File

@@ -14,12 +14,7 @@ namespace Chill\MainBundle\Audit;
interface SubjectConverterManagerInterface
{
/**
* @return list<array>
* @return list<Subject>
*/
public function convertEntityToSubjects(mixed $subject): array;
/**
* @return Subject|list<Subject>
*/
public function getSubjectsForEntity(mixed $subject): Subject|array;
public function getSubjectsForEntity(mixed $subject): array;
}

View File

@@ -13,6 +13,7 @@ namespace Chill\MainBundle\Tests\Audit;
use Chill\MainBundle\Audit\AuditEvent;
use Chill\MainBundle\Audit\AuditEvent2Trail;
use Chill\MainBundle\Audit\Subject;
use Chill\MainBundle\Audit\SubjectConverterManagerInterface;
use Chill\MainBundle\Entity\AuditTrail;
use PHPUnit\Framework\TestCase;
@@ -48,11 +49,11 @@ class AuditEvent2TrailTest extends TestCase
metadata: ['foo' => 'bar']
);
$subjectConverterManager->convertEntityToSubjects($subject)
->willReturn([['type' => 'stdClass', 'id' => '1']]);
$security->getUser()->willReturn(null);
$subjectConverterManager->getSubjectsForEntity($subject)
->willReturn([new Subject('stdClass', ['id' => '1'])]);
$service = new AuditEvent2Trail(
$translator->reveal(),
$subjectConverterManager->reveal(),
@@ -65,7 +66,7 @@ class AuditEvent2TrailTest extends TestCase
$this->assertInstanceOf(AuditTrail::class, $trail);
$this->assertSame('test_action', $trail->getAction());
$this->assertSame('translated description', $trail->getDescription());
$this->assertSame([[['type' => 'stdClass', 'id' => '1']]], $trail->getTargets());
$this->assertSame([['id' => '1', 't' => 'stdClass']], $trail->getTargets());
$this->assertSame(['foo' => 'bar'], $trail->getMetadata());
$this->assertEquals($clock->now(), $trail->getOccurredAt());
}

View File

@@ -32,7 +32,7 @@ class SubjectConverterManagerTest extends TestCase
$manager = new SubjectConverterManager([]);
$this->expectException(ConvertSubjectException::class);
$manager->convertEntityToSubjects(new \stdClass());
$manager->getSubjectsForEntity(new \stdClass());
}
public function testReturnsSingleSubjectWhenConverterReturnsSubject(): void
@@ -46,10 +46,10 @@ class SubjectConverterManagerTest extends TestCase
$manager = new SubjectConverterManager([$converter->reveal()]);
$result = $manager->convertEntityToSubjects($subject);
$result = $manager->getSubjectsForEntity($subject);
$this->assertCount(1, $result);
$this->assertSame(['id' => '123', 't' => 'type'], $result[0]);
$this->assertSame(['id' => '123', 't' => 'type'], $result[0]->asArray());
}
public function testReturnsListOfSubjectsWhenConverterReturnsArray(): void
@@ -64,11 +64,11 @@ class SubjectConverterManagerTest extends TestCase
$manager = new SubjectConverterManager([$converter->reveal()]);
$result = $manager->convertEntityToSubjects($subject);
$result = $manager->getSubjectsForEntity($subject);
$this->assertCount(2, $result);
$this->assertSame(['id' => '123', 't' => 'type1'], $result[0]);
$this->assertSame(['id' => '456', 't' => 'type2'], $result[1]);
$this->assertSame(['id' => '123', 't' => 'type1'], $result[0]->asArray());
$this->assertSame(['id' => '456', 't' => 'type2'], $result[1]->asArray());
}
public function testSkipsConverterThatDoesNotSupportSubject(): void
@@ -86,9 +86,9 @@ class SubjectConverterManagerTest extends TestCase
$manager = new SubjectConverterManager([$converter1->reveal(), $converter2->reveal()]);
$result = $manager->convertEntityToSubjects($subject);
$result = $manager->getSubjectsForEntity($subject);
$this->assertCount(1, $result);
$this->assertSame(['id' => '123', 't' => 'type'], $result[0]);
$this->assertSame(['id' => '123', 't' => 'type'], $result[0]->asArray());
}
}

View File

@@ -24,19 +24,12 @@ class AccompanyingPeriodSubjectConverter implements SubjectConverterInterface, S
{
use SubjectConverterManagerAwareTrait;
public function convert(mixed $subject): array
public function convert(mixed $subject): Subject|array
{
$data = [new Subject('accompanying_period', ['id' => $subject->getId()])];
foreach ($subject->getCurrentParticipations() as $participation) {
$subjects = $this->subjectConverterManager->getSubjectsForEntity($participation->getPerson());
if ($subjects instanceof Subject) {
$data[] = $subjects;
} else {
foreach ($subjects as $s) {
$data[] = $s;
}
}
array_push($data, ...$this->subjectConverterManager->getSubjectsForEntity($participation->getPerson()));
}
return $data;

View File

@@ -29,12 +29,7 @@ class AccompanyingPeriodWorkSubjectConverter implements SubjectConverterInterfac
$data = [new Subject('accompanying_period_work', ['id' => $subject->getId()])];
foreach ($subject->getPersons() as $person) {
$personSubject = $this->subjectConverterManager->getSubjectsForEntity($person);
if ($personSubject instanceof Subject) {
$data[] = $personSubject;
} else {
array_push($data, ...$personSubject);
}
array_push($data, ...$this->subjectConverterManager->getSubjectsForEntity($person));
}
return $data;