A whole-module pass over method names, against the rule core follows: a method is an action, so it starts with a verb, and the name answers what it does at the call site without the reader opening the signature. Thirty do not. One issue rather than thirty. Nothing changes behaviour. Every rename is mechanical, the existing suite is the proof, and pre-1.0 the API is free to break — which is the reason to do it now rather than after. ## 1. The write API `AuditTrailInterface::event()` is the module's one write method, and it is named for its object instead of its action. Renamed to **`record()`**: ```php $this->auditTrail->record('user', 'login', $subject, $context); ``` `record` is the verb, and the field agrees: Drupal's own `audit_log` names it `AuditLogLogger::log()`, utopia-php/audit and PHPAuditLogger both use `log()`, Audit.NET uses `Log()`. `log()` is the one name this module cannot take — `AuditTrailLogger` is already its PSR-3 logger, and the docs turn on the difference between the two paths. `write()` and `append()` were both rejected on inspection: neither is true. Inside an open transaction the write is parked in a buffer and `AuditTrailChainWriter::write()` returns `['id' => 0]` because no row exists yet, and a filter or an unresolved channel can end the call with nothing written at all. `record()` names what the caller asks for, which a policy may decline; `append()` would assert a position in the chain that often does not exist. Two neighbours follow from it: - `AuditTrail::dispatchEvent()`, the private inner half of the public call, becomes **`doRecord()`**. In core `dispatchEvent()` means dispatching a Symfony event, which this is not, and core spells the inner half of a public method `do*` — 223 distinct names, `doSave()`, `doCount()`, `doLoadMultiple()`. - The two bridges' private `event()` and `anonymousEvent()` helpers become **`record()`** and **`recordAnonymousEvent()`**. ## 2. The archive-file guard `ArchiveEnvelope::narrowFileMode()` sets an archive file to 0640 and throws when it cannot, so an NDJSON carrying actor uids, IP addresses, request URIs and full before/after entity snapshots is never left readable by every account on the host. `narrow` is not a Drupal word: methods beginning `narrow` in core, **0**. And `FileMode` names a noun with no predicate — a mode always exists, so the name cannot say what about it, and 0640 is the mechanism rather than the guarantee. Renamed to **`ensureFileIsNotWorldReadable()`**, `ensure*` being the family this module already uses for make-it-true-or-throw (`ensureTempDirectory()`, `ensureSegmentDirectory()`, `ensureSegmentCoverage()`), as against `assert*` for check-and-throw. ## 3. The rest A method that returns something is `get*` when it reads, `build*` when it assembles, `find*` when it may find nothing, `compute*` when it derives: | Class | Was | Is | |---|---|---| | `SecretUsage` | `rowSignature()`, `checkpointSignature()`, `segmentSignature()` | `getRowSignatures()`, `getCheckpointSignatures()`, `getSegmentSignatures()` — plural: each returns the recomputed signature *and* the stored one | | `SegmentSignature` | `archiveContentHmac()` | `computeArchiveContentHmac()`, joining `computeIdentityHmac()` and `computeLifecycleHmac()` | | `SegmentReader` | `stateOf(bool $matches)` | `getSignatureCheckState(string $expected, string $stored)` — it now runs the `hash_equals` itself, which also removes the same wrapper from its three call sites | | `ContextJsonText` | `expression()` | `buildSqlExpression()` — a SQL expression, which is core's own word for it (`Select::addExpression()`) | | `AuditTrailEntriesController` | `contextTransientText()` | `buildContextTransientExpression()` | | `TsaProviderUsage` | `tsaIdExpression()`, `baseQuery()` | `buildTsaIdExpression()`, `buildBaseQuery()` | | `DirectoryChecker` | `docrootForms()` | `getDocrootPaths()` — "forms" reads as Form API in a Drupal codebase | | `ForensicStamp` | `keptSegmentsOfPath()` | `findKeptPathSegmentCount()` — nullable, and "segment" alone means an archive range everywhere else in this module | | `ForensicStamp` | `queryKeys()`, `pathPatterns()`, `queryKeysToRedact()`, `pathPrefixesToRedact()` | `getQueryKeys()`, `getPathPatterns()`, `getQueryKeysToRedact()`, `getPathPrefixesToRedact()` | | `AuditTrailSettingsForm` | `trimmedTarget()`, `optionalTrimmedTarget()`, `lineListTarget()` | `buildTrimmedTarget()`, `buildOptionalTrimmedTarget()`, `buildLineListTarget()` | A predicate is `is*` or `has*`, and names its subject: | Class | Was | Is | |---|---|---| | `ForensicStamp` | `redacting()` | `isRedacting()` | | `AuditTrailFilterInterface` (+ base, + two plugins) | `rejectsSilently()` | `isFilteredFromOtherLoggers()` — core has no verb-phrase questions, and see the section below | | `AuditTrailCronArchiveHooks` | `holdsTransientNotYetPurged()` | `segmentHasTransientContentToPurge()` — the receiver is the hooks class, not the segment, so the subject goes first | And three test helpers, where `On()` as a suffix appears nowhere in core: `stampOn()` → `logEntryDuringRequest()`, `writerOn()` → `buildWriterForConnection()`, `onRequest()` → `runWithRequestOnStack()`, the last because an `on*` prefix reads as an event handler. ## `reject` retired from the filter feature A filter dropping a `notice` is the operator's configuration working, not something going wrong, and "reject" said otherwise. The module was already inconsistent with itself about it: both shipped plugins describe themselves as *suppressing* events, while the setting beside them said *Reject silently*. Filtering has exactly two levels, and the flag picks between them: - the **chain only** — the row is skipped, dblog and syslog still see the event, so operators can see what is being dropped; - **the other loggers as well** — no chain row and no dblog row. The chain row is skipped either way, so the flag is only ever about the others, and the name says so rather than saying "everywhere". "The other loggers" is the module's own phrase for that set: 52 uses against 4 for "every logger". So the flag is about reach, not volume: | | Was | Is | |---|---|---| | method | `rejectsSilently()` | `isFilteredFromOtherLoggers()` | | config key | `reject_silently` | `filter_other_loggers` | | schema label | *Reject silently* | *Keep the event out of the other loggers too* | | checkbox | *Drop rejected events everywhere* | *Keep the event out of dblog and syslog too* | | verdict key | `rejects_silently` | `filtered_from_other_loggers` | The plugin descriptions, `audit_trail.api.php`, `docs/configuration.md`, the interface contract and the test scaffolding (`RejectingBrokenTestFilter` → `FilteringBrokenTestFilter`) follow. "Reject" stays where something genuinely is refused: a malformed archive envelope, a duplicate `(chain, previous_hash)`, a form's own validation. The config key is the one thing here that is not internal. Pre-1.0 that is free — `README.md` already states there are no update hooks before 1.0.0 and that reinstalling is the supported way to move between them — and after 1.0 it would be frozen, which is why it goes now. ## What keeps its name, deliberately - **Hook implementations** — `fileDownload()`, `cron()`, `runtime()`, `chainInsert()`, `userLogin()` and the rest are named after the hook they implement. - **Core overrides and callbacks** — `actions()`, `applies()`, `defaultConfiguration()`, `onDependencyRemoval()`, and `exists()`, which is the `#machine_name` callback core spells that way six times. - **Converters and builders** — `toOutboxRow()`, `fromOutboxRow()`, `toMicroseconds()`, `withPermanentKey()`. - **`chainNow()`, `chainBatch()`, `chainBuffered()`** — here `chain` really is the verb, and `postQuery()` really does POST the query over HTTP. - **`should*`** — `shouldEmit()`, `shouldChain()`, `shouldIncludeNonce()`. Core has ten, and `RevisionableEntityBundleInterface::shouldCreateNewRevision()` is the same shape as a provider entity's nonce flag. - **Actions that return a bool** — `mintCheckpoint()`, `purgeSegmentArchiveFile()`. The bool is an outcome, not a state; core's `LockBackendInterface::acquire()` is the same. ## Tests No new test. A rename has no behaviour to pin, and a test that asserted a method's *name* would be a lint rule in the wrong tool. What proves it is the existing suite still passing unchanged, and `phpstan` — which is what caught the one declaration this sweep first missed. ## Folded in One word from the same audit: `docs/roadmap.md` named the JSON-derived filters as "`uid` or client IP", mixing a JSON key with prose for the key beside it. It reads `uid` or `ip` now. --- AI-Generated: Yes (Claude Code was used to sweep the module's method names and to draft this issue and the merge request against it. I reviewed both.)