[netomi]

Summary

• LocalRegistryService.postReview checks whether a user already has an active review for an extension, then inserts a new one — but those two steps aren't serialized against each other. Two requests from the same user submitted close together can both pass the check before either commits, leaving the user with more than one active review for the same extension and skewing its average rating and review count.
• Added a partial unique index (unique_active_review on extension_review(extension_id, user_id) WHERE active) as the actual enforcement point, and @Retryable around postReview to retry once on the resulting constraint violation — the same pattern already used for concurrent extension publications (PublishExtensionVersionHandler.DuplicateExtensionPredicate). On a losing race, the retry re-runs in a fresh transaction, sees the now-committed review, and returns the normal "already reviewed" message instead of a raw database error.
• The migration is partial (WHERE active) rather than a plain unique constraint because deleteReview soft-deletes (sets active = false) rather than removing rows, so a user can legitimately hold multiple inactive reviews over time — just never more than one active one.
• The migration also cleans up any duplicates that may already exist in a given deployment: it deletes all but the most recent active review per user/extension, and recomputes the affected extensions' average_rating/review_count before adding the index. Ranked by timestamp, not id (review finding) — ExtensionReview's sequence generator has no explicit allocationSize, so JPA defaults it to 50, matching the sequence's INCREMENT 50; that's a pooled allocator, so separate application instances can hold different id blocks and a later insert can end up with a lower id than an earlier one on another instance.
• CI caught a real bug in the first version of this PR: @EnableResilientMethods defaults to interface-based (JDK dynamic) proxying, and LocalRegistryService implements IExtensionRegistry. Adding @Retryable to postReview made Spring proxy the bean against that interface instead of the concrete class, breaking every other bean (RegistryAPI, UserAPI, AdminAPI) wired to the concrete LocalRegistryService type — a BeanNotOfRequiredTypeException at context startup that failed essentially the whole @WebMvcTest suite. Fixed by switching to @EnableResilientMethods(proxyTargetClass = true), matching how @EnableCaching is already configured in this codebase for the same reason. Checked every other @Retryable user in the codebase (PublishExtensionVersionHandler, MigrationService, PublishExtensionVersionService, ElasticSearchService) — this is backward-compatible for all of them.
• The migration's cleanup wasn't serialized against live writers (review finding): the Kubernetes manifest uses the default rolling-update strategy and Flyway runs at each instance's own startup (no separate migration job), so an old-version pod can still be serving postReview() while a new pod runs this migration. A plain INSERT landing between the DELETE and CREATE UNIQUE INDEX would reintroduce a duplicate and fail the whole migration. Fixed with LOCK TABLE extension_review IN SHARE MODE as the first statement — it conflicts with the ROW EXCLUSIVE every writer needs (blocking them until this migration's transaction commits) without blocking readers, and Flyway wraps the whole script in one transaction so the lock holds through index creation.
• src/main/jooq-gen/ needed regenerating for the new index (review finding, ./gradlew jooqCodegen per server/AGENTS.md) — I couldn't run that myself (needs a live Postgres, unavailable in this sandbox), so this was run against a local dev Postgres and applied here: one new Indexes.java entry plus adding it to ExtensionReview.java's getIndexes(), matching the unique_active_signature_key_pair_idx precedent exactly as predicted.

Test plan

• Added LocalRegistryServicePostReviewConcurrencyTest, modeled directly on the existing PublishExtensionVersionConcurrencyTest: a @MockitoSpyBean on RepositoryService intercepts the duplicate-review check and injects a genuinely concurrent second postReview call in that exact window. Asserts exactly one call succeeds, the other gets the polite rejection, and only one active review row exists afterward.
• ./gradlew spotlessCheck and ./gradlew compileTestJava pass.
• Confirmed on CI (this environment has no working Docker, so I couldn't run the Testcontainers-backed test myself): LocalRegistryServicePostReviewConcurrencyTest passed against a real PostgreSQL instance, and the full server test suite is green after the proxyTargetClass fix.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

The migration can retain an older review because pooled sequence IDs are not chronological across instances.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

• Ensure review cleanup retains the most recent review · New

What changed in this PR

Prevents concurrent requests from creating duplicate active extension reviews.

Changes:

• Adds duplicate cleanup and a partial unique index.
• Retries review creation after matching constraint violations.
• Enables class-based resilience proxies and adds concurrency coverage.

File	Description
LocalRegistryServicePostReviewConcurrencyTest.java	Tests concurrent review submission.
V1_76__Unique_Active_Review.sql	Deduplicates reviews and adds uniqueness enforcement.
RegistryApplication.java	Enables CGLIB resilience proxies.
LocalRegistryService.java	Retries duplicate-review races.

---

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

[Copilot]

Copilot review overview

🟡 Changes recommended

The migration needs serialization against live writers and regenerated jOOQ metadata.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Serialize cleanup with writes before creating the unique index · New
• Regenerate and commit jOOQ models for the new schema index · New

Resolved since last review (1)

• Ensure review cleanup retains the most recent review

[Copilot]

Copilot review overview

🟢 Approval recommended

The database constraint, transactional retry, migration safety, generated metadata, and concurrency test consistently address the race.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Serialize cleanup with writes before creating the unique index
• Regenerate and commit jOOQ models for the new schema index

Files not reviewed (2)

• server/src/main/jooq-gen/org/eclipse/openvsx/jooq/Indexes.java: Generated file
• server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/ExtensionReview.java: Generated file