>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3619737. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !82 >>> <p>A duplication audit over the whole module, tests included. It reports 39 byte-identical body groups (96 methods, 89 removable lines), 37 near-duplicate clusters, 214 windowed clone sites, and one concept with thirty implementations. A previous audit on this subject, <span class="drupalorg-gitlab-issue-link drupalorg-gitlab-link-wrapper"><a href="https://git.drupalcode.org/project/pdv/-/work_items/3619101" class="drupalorg-gitlab-link">https://git.drupalcode.org/project/pdv/-/work_items/3619101</a></span>, reported one case. That number was not wrong for the question it asked; the question was badly scoped, and this issue records why so the next audit does not repeat it.</p> <h3>Why the earlier audit found one case</h3> <p>Its detector excluded test directories by construction, so 601 test functions were never scanned and the exclusion was never stated in the result. It skipped every body under six normalized lines, which is precisely where accessor duplication lives: 493 bodies. It suppressed nineteen method names outright, among them label, id and getInfo. And it compared whole methods only, so a duplicated block inside two larger methods was invisible. Run at those settings this tree yields nought to two groups, which is what was reported.</p> <p>The replacement extracts bodies by brace matching rather than by regular expression, which also fixes a fault worth naming: a regular expression anchored on the closing brace lets the last method in a class absorb the class's own brace, so a body identical to its siblings hashes one line longer and files itself under a group of one. Verified on this tree: 1581 bodies, zero brace imbalances. The detector is development tooling and stays outside the repository: what merges is the code it found, not the thing that found it.</p> <h3>What the passes report</h3> <p>1581 functions across 259 files, 980 production and 601 test, with 108 excluded as interface or core contracts. Byte-identical bodies: 39 groups, 96 methods, 89 removable lines, of which 13 groups have three or more members. Same-shape bodies with literals and variables blinded: 10 groups. Near-duplicate bodies at 80 per cent similarity but not identical: 49 pairs in 37 clusters. Windowed six-line blocks: 214 distinct clone sites. Entity queries grouped by condition shape: 29 shapes, 8 of them used at two or more sites.</p> <h3>Recording a vault event has no owner</h3> <p>Thirty sites construct an AuditableEvent. Twenty-two of them pair the dispatch with a near-identical logger call carrying an overlapping context array, and seven classes each inject EventDispatcherInterface to do it: ConsumerApi, GrantManager, StoreRequestManager, TrustManager, Vault, DormancyCollector and ConsumerFloodGuard, plus two forms and one controller. The six-line dispatch block is the most repeated fragment in the module. No existing service owns "record a vault event", so this is the one finding here where a new service is the right answer rather than reusing one.</p> <h3>Entity row accessors</h3> <p>Thirteen identical groups have three or more members and nearly all are entity field accessors: getCreatedTime() five times, then getOwnerId(), getConsumerId(), getScope() and getCipherSuite() three times each. Every one of the five entities declares the matching base field, so a trait per shared accessor is available. The module already ships ten traits, all of them for forms and controllers, so an entity trait is a new home for an established pattern.</p> <h3>The rest, in descending value</h3> <p>generateRandomToken() is byte-identical in three classes. GrantManager::revoke(), GrantManager::denyRequest() and TrustManager::revoke() form a three-way near-duplicate cluster of about thirty lines each. Three batch forms duplicate a seven-line load helper. getCancelUrl() is duplicated three ways, twice over, with two distinct bodies. The two list builders share getEntityIds() byte-for-byte, buildHeader() near-identically and one query shape. The crypto wrappers duplicate their envelope handling. An owner-scoped entity query is written five times. A submit-actions block appears at eight sites differing only in the button label. And <span class="drupalorg-gitlab-issue-link drupalorg-gitlab-link-wrapper"><a href="https://git.drupalcode.org/project/pdv/-/work_items/3619101" class="drupalorg-gitlab-link">https://git.drupalcode.org/project/pdv/-/work_items/3619101</a></span> folded two expired-row reapers while leaving reapOrphanRequests() and reapOrphanTrusts() standing, which is the same shape with the same fix left unfinished.</p> <p>In tests: eight identical groups and twenty near-duplicate clusters. Key creation is duplicated five ways, uid() four ways, and there are pairs for fileWebform(), dispatch(), getSubjectKey(), startRequest() and three count helpers, plus four return-url rejection tests built the same way.</p> <h3>Two behaviour changes ride with this</h3> <p>Ten log keys change. With the convention in one place instead of restated at each site, the nine of eighteen keys that had drifted from it now follow it: pdv.read and pdv.write become pdv.read.allowed and pdv.write.allowed, the four pdv.offer keys become pdv.consent, and four more lose the spaces they carried. No test asserted any of them and the audit chain is untouched, because the event channel and action never changed; anything filtering dblog on the old strings needs updating.</p> <p>The item-kind and tenant collection screens paginate again. Both list builders overrode getEntityIds() to sort by weight, but core builds the query in getEntityListQuery() and adds the pager there, so overriding the caller dropped it and both screens listed every row. Both entity types now declare a weight entity key, as five core config entity types do, and each list builder names it through core's own SORT_KEY. On a site with more than fifty item kinds this replaces one long list with a pager.</p> <h3>Two findings withdrawn before filing</h3> <p>The detector grouped four exception constructors and three API controller constructors as identical bodies. Neither is removable: each subclass adds its own promoted properties, so the parent::__construct() call is mandatory delegation. They are recorded here because they look like strong findings and are not, and a future run will flag them again.</p> <h3>A clean run is a floor, not a verdict</h3> <p>The 37 near-duplicate clusters exist only because byte-identical hashing cannot see them, and a layer below that, the same computation written a different way, is what the reused query shapes are evidence of. After each pattern is fixed the detector is re-run to confirm the pattern is gone rather than reduced, which is the specific failure mode of the previous round.</p> <p>AI-Generated: Yes (Claude Code was used to build the detector, run the audit, draft this issue summary, and write the fixes on the merge request. I reviewed and ran the work myself before posting it.)</p>