refactor: replace the remaining EntityManager.merge updates with find

[netomi]

Second and final batch of the #989 audit — the 13 sites left after #2150. Between the two, every EntityManager.merge call site has been dealt with.

Stacked on #2150. The base is fix/entitymanager-merge-audit, so the diff here shows only this batch; GitHub will retarget it to main automatically once #2150 merges. Please review that one first.

Why these were subtler than the first batch

Every site here did intend an update, so none of them is dead code. The hazard is that merge writes every column of the detached copy, so any field that moved on the row since the caller loaded it is silently reverted — which is exactly how the original outage happened. These sites have long windows between load and write: a VSIX download and scan, a storage round-trip.

The clearest example is a pair that could revert each other. SetPreReleaseJobService writes preRelease/preview and CheckPotentiallyMaliciousExtensionVersionsService writes potentiallyMalicious; both loaded an ExtensionVersion, did slow VSIX work, then merged the whole row. Overlapping on the same version, whichever finished second undid the other's field — and either could revert active.

Service owns the mutation → find then set its own field

EclipseService#updateUserData, CustomerService#deactivateRateLimitToken, ExtensionVersionIntegrityService#setSignatureKeyPair, PublishExtensionVersionService#markExtensionAsPotentiallyMalicious, AccessTokenService#scheduleTokenExpirationNotification, SetPreReleaseJobService, CheckPotentiallyMaliciousExtensionVersionsService.

Worth noting that PublishExtensionVersionService#activateExtension already did exactly this, so the correct pattern was sitting next to the incorrect one in the same class; this makes the neighbours consistent with it.

Two methods renamed for the field they persist

MigrationService#updateResource → updateResourceSize, RenameDownloadsService#updateResource → updateResourceName.

Their old signatures couldn't express which field the caller had changed, which is precisely why they had to merge the whole row. Naming them for the field they write removes the hazard and stops a future caller setting something else and quietly having it dropped. Each had exactly one caller.

Merge before remove

MigrationService#deleteFileResource and the old namespace in ChangeNamespaceService now find and remove, skipping a row that has already gone rather than having merge try to resurrect it.

One merge deliberately kept

ChangeNamespaceService keeps a single merge, now documented:

} else {
    // Deliberately a merge, unlike the rest of #989's call sites: the caller mutates this
    // detached namespace before handing it over (ChangeNamespaceJobRequestHandler sets
    // logoName on it), so the update belongs to the caller and only merge can carry it.
    // find-then-set would silently drop that rename.
    newNamespace = entityManager.merge(newNamespace);
}

I nearly converted this one before reading the caller. So the conclusion of the audit isn't "merge is wrong" — it's that merge is wrong when the service owns the mutation, and right when the caller does. That distinction is what the comment is there to preserve. The same method's updatedResources loop only ever changes the name, so that one writes just the name.

CustomerService needed something to throw for the vanished-row case; it uses ErrorResultException, which the class already imports and uses for a missing row, rather than introducing a different exception type.

Testing

Full server suite green (1145 tests). Three new cases on markExtensionAsPotentiallyMalicious: the flag landing on the managed row, a concurrent activation surviving the write (the reverted-column case, which is the point of the change), and a version purged in the meantime being ignored rather than resurrected.

Fixes #989

🤖 Generated with Claude Code

[Copilot]

Copilot wasn't able to review any files in this pull request.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The changes consistently apply the safer “load managed then set” pattern (or find+remove) and include targeted tests covering the previously risky stale-merge failure mode.

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The refactors consistently replace risky detached-entity merges with managed-row updates (plus safe missing-row handling) and include targeted tests for the key concurrency regression scenario.

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Lite