mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Paul LütznerandClaude Opus 5
2790c13a82
CalendarState was the string "fixed", and its docblock explained that this
was a confession rather than a design: events are written from the sync
engine, from extraction, from the web editor and from CalendarEvent/set,
and a log recording a quarter of the writes "is not a weaker version of
it, it is a lie with a number on it". So canCalculateChanges was false,
there was no CalendarEvent/changes, and every client re-ran its query.
calendar_change_log closes that. The rows are written by a Doctrine
listener rather than by a recorder the writers call, and that is the whole
point — CalendarEvent exposes public mutable properties, so fields move
wherever somebody assigns one and lets the flush carry it. An RSVP in
InviteResponder, a SEQUENCE bump in EventReconciler, the recurrence flags
in RecurrenceMaterialiser: none pass through CalendarEventWriter, so a
recorder wired into write() would have missed all three and every deletion
besides. Doctrine already knows what changed; asking it is the only way to
be complete without asking every future caller to remember.
CalendarChangeLog the log. Its own table rather than new
JmapObjectType cases on jmap_change_log, which
CalendarState suggested: that table is keyed by
account_id, and CalendarAccountResolver says a
user can delete their last account and keep a
calendar. CalDAV also counts per collection, and
there is nowhere to put the collection.
the listener onFlush collects, postFlush writes. An ignore
list, not an allow list, so a field added later
defaults to being logged — over-recording costs a
wasted conditional GET, under-recording is the
silent failure.
CalendarChangeReader tokens and deltas, protocol-neutral, in both
scopes: per user for JMAP, per calendar for the
CalDAV server this is the groundwork for.
Two bugs fell out while building it.
The first was mine and is why postFlush writes SQL rather than persisting
entities. Flushing from inside postFlush re-runs the whole unit of work,
and the second pass sees what the first left behind: EventDismisser
removes an event while an EventSourceLink still points at it, which is
legal once and throws "a new entity was found through the relationship"
twice. It broke every calendar deletion in the suite and five demo-flow
tests. A plain INSERT stays out of the UnitOfWork entirely, so this
listener cannot make somebody else's flush fail.
The second is subtler: a flush that throws never reaches postFlush, so the
queued rows survived to be written by the next flush that succeeded —
logging changes the database had rolled back. onFlush clears the queue as
it starts. A worker whose EntityManager is reset and carries on is exactly
where that would have bitten.
CalendarEvent/get, /query and /set now return real sequences. Calendar/get
still does not: the log records events, not the collections holding them,
so a rename or a recolour moves nothing. That is a smaller gap than it
sounds — a user has a handful of calendars, Calendar/get returns all of
them by default, and CalDAV discovers the collection set with a PROPFIND
that holds no token. CalendarState now says so instead of describing a
world that no longer exists.
Also marks config/reference.php optional in phpstan.dist.neon. It is
gitignored and generated, every sibling entry in that exclude list already
carries (?), and without it PHPStan refuses to start on a clean checkout.
No deployment change. One new table, created on boot.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
109 lines
4.9 KiB
Plaintext
109 lines
4.9 KiB
Plaintext
# Static analysis.
|
|
#
|
|
# Level 5 with everything already failing captured in a baseline. The
|
|
# alternative — level 0 with no baseline — reports nothing interesting, and
|
|
# level 9 buys a fortnight of triage before it finds its first real bug. Five is
|
|
# where the class of defect this codebase actually has starts showing up:
|
|
# nullable entity getters (`?int getId()`, `?Account getAccount()`) flowing into
|
|
# parameters and return types that are not nullable.
|
|
#
|
|
# The baseline is a debt ledger, not a licence. It shrinks; raise the level when
|
|
# it is small enough that doing so is a day's work rather than a fortnight's.
|
|
includes:
|
|
- phpstan-baseline.neon
|
|
- vendor/phpstan/phpstan-symfony/extension.neon
|
|
- vendor/phpstan/phpstan-symfony/rules.neon
|
|
- vendor/phpstan/phpstan-doctrine/extension.neon
|
|
- vendor/phpstan/phpstan-doctrine/rules.neon
|
|
|
|
parameters:
|
|
level: 5
|
|
|
|
# The whole project rather than just src/ and tests/: bin/, config/ and
|
|
# public/ hold real PHP that can break the same way anything else does —
|
|
# config/bootstrap_generated_secrets.php is loaded into every process
|
|
# through composer's autoload.files.
|
|
#
|
|
# Listed explicitly rather than as `./`, and that is not a style
|
|
# preference. PHPStan walks the tree with a RecursiveDirectoryIterator
|
|
# *before* it applies excludePaths, so a directory it cannot read is a hard
|
|
# crash rather than a skip — and no exclusion can prevent it, because the
|
|
# crash happens during enumeration. `.claude/worktrees` holds entire git
|
|
# worktrees of this repository (37 of them at the time of writing), several
|
|
# containing a root-owned `var/secrets` left by a container. With `./` the
|
|
# gate could not be run at all from a working checkout: it died on a
|
|
# directory belonging to a different checkout of itself.
|
|
#
|
|
# Everything with PHP in it is named below. A new top-level directory that
|
|
# holds code has to be added here, which is the cost of this; the previous
|
|
# arrangement's cost was that the command did not run.
|
|
paths:
|
|
- bin
|
|
- config
|
|
- public
|
|
- src
|
|
- tests
|
|
- importmap.php
|
|
|
|
excludePaths:
|
|
analyseAndScan:
|
|
# Not PHP.
|
|
- assets
|
|
- docs
|
|
- frankenphp
|
|
- templates
|
|
- translations
|
|
# Third-party code, and its symbols come from the autoloader
|
|
# anyway — analysing it reports other people's debt.
|
|
- vendor
|
|
- node_modules (?)
|
|
# Build and run artefacts. public/assets is compiled output and
|
|
# var/ holds the container dumps this very config reads.
|
|
#
|
|
# The `(?)` marks a path that may legitimately be absent: PHPStan
|
|
# treats an excludePath that matches nothing as a config error, and
|
|
# none of these exist on a fresh clone or in a CI job that has not
|
|
# built assets yet.
|
|
- public/assets (?)
|
|
- public/bundles (?)
|
|
- var (?)
|
|
- playwright (?)
|
|
- playwright-report (?)
|
|
- test-results (?)
|
|
# Generated: 54 migrations nobody edits by hand, and a ~1000-line
|
|
# dump of the framework config tree that exists to be read.
|
|
- migrations
|
|
- config/reference.php (?)
|
|
# Twig extensions lifted from upstream rather than written here.
|
|
# NOTE: only Vendor/ — the rest of src/Twig is application code
|
|
# (SidebarCounts and friends) and must stay analysed.
|
|
- src/Twig/Vendor
|
|
|
|
# ignoreErrors is deliberately empty.
|
|
#
|
|
# The rule usually wanted here — silencing "Property …::$id (int|null) is
|
|
# never assigned int" on Doctrine identifiers — is not needed: the
|
|
# phpstan-doctrine extension included above already understands that the
|
|
# ORM assigns identifiers by reflection, so the error never fires. Adding
|
|
# the pattern anyway would fail the build, because PHPStan treats an ignore
|
|
# that matches nothing as a config error.
|
|
#
|
|
# What property.onlyWritten does catch in this project is a different and
|
|
# genuine thing: controllers injecting repositories they never use
|
|
# (MailController::$mailboxRepository, ComposeController::$mailboxRepository
|
|
# and about fifteen more). Those are in the baseline as debt to delete, not
|
|
# noise to suppress — which is why there is no blanket ignore for them.
|
|
ignoreErrors: []
|
|
|
|
# Docblocks here are prose as much as types — plenty predate the signatures
|
|
# beside them. Trusting them as certain turns a stale @var into a stream of
|
|
# "condition is always false" on code that is fine.
|
|
treatPhpDocTypesAsCertain: false
|
|
|
|
symfony:
|
|
containerXmlPath: var/cache/test/App_KernelTestDebugContainer.xml
|
|
consoleApplicationLoader: tests/phpstan-console-loader.php
|
|
|
|
doctrine:
|
|
objectManagerLoader: tests/phpstan-object-manager.php
|