Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symfony uuid type support #230

Open
CalinBolea opened this issue Oct 31, 2024 · 0 comments
Open

Symfony uuid type support #230

CalinBolea opened this issue Oct 31, 2024 · 0 comments

Comments

@CalinBolea
Copy link

Hello, not sure if this is a bug or it's intended so I'm adding a support question.

First of all, thank you to all the contributors and to @DamienHarper especially for all the work done here! A lot of people appreciate what you are doing!

A bit of context:
A relatively simple symfony7.1 app, using php8.3, doctrine as ORM and MySql8.0.
Relatively standard entities. We are using uuids, they're binary(16) in the db.
Example config for ids:

#[ORM\Id]
#[ORM\Column(type: 'uuid', unique: true)]
private Uuid $id;

From what I can see, up until version 2.4.8, this type was cast into string in the AuditTrait
https://github.com/DamienHarper/auditor/blob/2.4.8/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php#L106-L111

From version 3 onward, this type is not being cast anymore, only ulid:
https://github.com/DamienHarper/auditor/blob/3.0.0/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php#L109

I see this change was made in a PR that refers to setting up docker for testing so I figure it might not be intended.
#206

The issue is that uuid binary representations are not valid strings to be persisted in the audit tables as object_ids (which are varchar(255)).
The exceptions also fail silently https://github.com/DamienHarper/auditor/blob/master/src/EventSubscriber/AuditEventSubscriber.php#L35
So nothing gets stored anymore and you don't get any feedback.

The "naive" fix I came up with is to listen to the event just before the AuditEventSubscriber kicks in and just transform the object_id representation from binary to rfc4122.

    public static function getSubscribedEvents(): array
    {
        return [
            LifecycleEvent::class => [
                ['fixUuidRepresentation', -999_999],  // should be fired just before the audit event subscriber
            ],
        ];
    }

    public function fixUuidRepresentation(LifecycleEvent $event): LifecycleEvent
    {
        $payload = $event->getPayload();

        try {
            $uuid = Uuid::fromString((string) $payload['object_id']);
        } catch (InvalidArgumentException) {
            return $event;
        }

        $payload['object_id'] = $uuid->toRfc4122();
        $event->setPayload($payload);

        return $event;
    }

My questions in the end are: Is this intended? Am I missing something? Would it make sense to revert the change for the AuditorTrait and cast the uuid type as string again (I would gladly contribute if this is the case) or is there a better solution anyone could recommend?

Thank you again for the tool you've built and all the time invested!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant