Skip to content

Update to Doctrine ORM 3.2 #10257 #23

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

Merged
merged 20 commits into from
Apr 27, 2025
Merged

Update to Doctrine ORM 3.2 #10257 #23

merged 20 commits into from
Apr 27, 2025

Conversation

PowerKiKi
Copy link
Member

No description provided.

@PowerKiKi
Copy link
Member Author

PowerKiKi commented Jul 11, 2024

Blocked by:

The whole PR must be reviewed again before merging. And requiresSQLCommentHint might need to be re-introduced depending on how the blockers are solved.

Without this, the record would not exist anymore in DB when we try to
get the file path in `deleteFile()`. And then we could not delete the
file on disk after the record deletion.
We don't need our custom implementation since ORM now supports string
backed enum mapped to ENUM columns natively.
Because most enums use native ORM implementation, and the few enums left
as pure string also don't need the comments to work anymore.
Because Doctrine will write to log before connecting, and thus will
re-create a second connection to log to DB. Except that we don't log low
priority log to DB, so nothing is ever written, but a second connection
is still created for nothing. And that second connection leads to
painful DB deadlock in unit tests, because we may log some things on one
 connection, while the other, main, connection is inside a transaction.

To prevent all of this mess, the DB connection for logging is
lazy-evaluated, and only retrieved when we actually write the first log
to DB.
Because laminas-log and laminas-mail are both discontinued. And we do it
now, because the migration to doctrine 3 exposed a cyclic dependency
issue in Epicerio. The end result was we were trying to log
something to DB before a connection to DB was established, and so we
established a second connection to DB, wrecking havoc in our apps.

So instead of deep diving in laminas-log to sort out all the details,
I'd rather deep dive in monolog directly and clear the technical debt at
the same time. And since monolog has an integration with symfony/mailer,
it seemed natural to do that too, and clear even more technical debt.

And to avoid the double-connection issue, the DB logger is only active,
and only retrieves its dependencies, after we are certain that a
connection to DB was established.

The overall concept of our logs remains unchanged though. We have 3 log
destinations: File, Mail and DB. Mail and DB have extra information
through `RecordCompleter`. File does not.
@PowerKiKi PowerKiKi marked this pull request as ready for review April 27, 2025 11:38
@PowerKiKi PowerKiKi merged commit daf37e6 into master Apr 27, 2025
10 checks passed
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

Successfully merging this pull request may close these issues.

1 participant