diff --git a/server/src/main/java/org/eclipse/openvsx/admin/ConsistencyAPI.java b/server/src/main/java/org/eclipse/openvsx/admin/ConsistencyAPI.java new file mode 100644 index 000000000..dcc1b23d0 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/admin/ConsistencyAPI.java @@ -0,0 +1,147 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.admin; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import org.eclipse.openvsx.consistency.ConsistencyCheckService; +import org.eclipse.openvsx.consistency.ConsistencyCheckSummary; +import org.eclipse.openvsx.consistency.ConsistencyFinding; +import org.eclipse.openvsx.json.ConsistencyCheckJson; +import org.eclipse.openvsx.json.ConsistencyCheckListJson; +import org.eclipse.openvsx.json.ConsistencyFindingJson; +import org.eclipse.openvsx.json.ConsistencyFindingListJson; +import org.eclipse.openvsx.json.ResultJson; +import org.eclipse.openvsx.settings.MutatingOperation; +import org.eclipse.openvsx.util.ErrorResultException; +import org.eclipse.openvsx.util.NotFoundException; + +/** + * Admin dashboard endpoints for the data consistency checks (see #1622): a live overview of every + * registered {@link org.eclipse.openvsx.consistency.ConsistencyCheck}, its findings, and actions to fix + * them - one at a time or all at once. There is no "run now" action here: findings are always + * recomputed live, and the scheduled sweep that auto-fixes what it can runs independently of this page. + */ +@RestController +@RequestMapping("/admin/consistency") +@ApiResponse( + responseCode = "403", + description = "Administration role is required", + content = @Content() +) +public class ConsistencyAPI { + + private final AdminService admins; + private final ConsistencyCheckService service; + + public ConsistencyAPI(AdminService admins, ConsistencyCheckService service) { + this.admins = admins; + this.service = service; + } + + @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) + @Operation(summary = "Get an overview of every registered consistency check") + public ResponseEntity listChecks() { + try { + admins.checkAdminUser(); + var json = new ConsistencyCheckListJson(); + json.setChecks( + service.listSummaries().stream() + .map(ConsistencyAPI::toJson) + .toList()); + return ResponseEntity.ok(json); + } catch (ErrorResultException exc) { + return exc.toResponseEntity(ConsistencyCheckListJson.class); + } + } + + @GetMapping(path = "/{checkId}/findings", produces = MediaType.APPLICATION_JSON_VALUE) + @Operation(summary = "Get the current findings of one consistency check") + public ResponseEntity findings(@PathVariable String checkId) { + try { + admins.checkAdminUser(); + var json = new ConsistencyFindingListJson(); + json.setFindings( + service.findings(checkId).stream() + .map(ConsistencyAPI::toJson) + .toList()); + return ResponseEntity.ok(json); + } catch (NotFoundException exc) { + var json = ConsistencyFindingListJson.error("Unknown consistency check: " + checkId); + return new ResponseEntity<>(json, HttpStatus.NOT_FOUND); + } catch (ErrorResultException exc) { + return exc.toResponseEntity(ConsistencyFindingListJson.class); + } + } + + @PostMapping(path = "/{checkId}/fix", produces = MediaType.APPLICATION_JSON_VALUE) + @MutatingOperation + @Operation(summary = "Fix every current finding of one consistency check") + public ResponseEntity fixAll(@PathVariable String checkId) { + try { + admins.checkAdminUser(); + var fixed = service.fixAll(checkId); + return ResponseEntity.ok(ResultJson.success("Fixed " + fixed + " finding(s) for check '" + checkId + "'.")); + } catch (NotFoundException exc) { + return new ResponseEntity<>( + ResultJson.error("Unknown consistency check: " + checkId), + HttpStatus.NOT_FOUND); + } catch (ErrorResultException exc) { + return exc.toResponseEntity(); + } + } + + @PostMapping(path = "/{checkId}/fix/{entityId}", produces = MediaType.APPLICATION_JSON_VALUE) + @MutatingOperation + @Operation(summary = "Fix a single finding of one consistency check") + public ResponseEntity fixOne(@PathVariable String checkId, @PathVariable long entityId) { + try { + admins.checkAdminUser(); + service.fixOne(checkId, entityId); + return ResponseEntity.ok(ResultJson.success("Fixed entity " + entityId + " for check '" + checkId + "'.")); + } catch (NotFoundException exc) { + return new ResponseEntity<>( + ResultJson.error("Unknown consistency check: " + checkId), + HttpStatus.NOT_FOUND); + } catch (ErrorResultException exc) { + return exc.toResponseEntity(); + } + } + + private static ConsistencyCheckJson toJson(ConsistencyCheckSummary summary) { + var json = new ConsistencyCheckJson(); + json.setId(summary.id()); + json.setName(summary.name()); + json.setDescription(summary.description()); + json.setCurrentFindingsCount(summary.currentFindingsCount()); + return json; + } + + private static ConsistencyFindingJson toJson(ConsistencyFinding finding) { + var json = new ConsistencyFindingJson(); + json.setEntityId(finding.entityId()); + json.setLabel(finding.label()); + json.setDetail(finding.detail()); + return json; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheck.java b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheck.java new file mode 100644 index 000000000..ff5e552a3 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheck.java @@ -0,0 +1,69 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.consistency; + +import java.util.List; + +/** + * A single, self-contained check for one kind of data inconsistency (see issue #1622: "Need a way to + * check the database for consistency"). Implementations are picked up automatically as Spring beans by + * {@link ConsistencyCheckService} - registering a new kind of check is exactly one new {@code @Component} + * implementing this interface, with no other wiring needed. + *

+ * Findings are always computed live from current data, never cached: a stored list of affected entities + * would go stale the moment anything about them changes, which is exactly the kind of silent drift this + * feature exists to catch. + */ +public interface ConsistencyCheck { + + /** + * A stable, unique identifier for this check (e.g. {@code "extension-active-flag"}). Used as the + * path segment in the admin API and as the key under which run history is recorded, so it must + * never change once a check has shipped. + */ + String getId(); + + /** + * A short, human-readable name shown in the admin UI. + */ + String getName(); + + /** + * Explains what this check looks for and why it matters, shown in the admin UI. + */ + String getDescription(); + + /** + * Runs the check now and returns every entity currently found inconsistent. Empty means healthy. + */ + List check(); + + /** + * Repairs the entity identified by {@code entityId} (one of {@link ConsistencyFinding#entityId()} + * from a prior {@link #check()} call). A no-op if the entity no longer exists or is no longer + * inconsistent (e.g. it was already fixed by something else in the meantime). + */ + void fix(long entityId); + + /** + * Whether the scheduled sweep (and the admin dashboard's "run now" action) should automatically fix + * this check's findings, rather than only recording them in run history for a human to fix from the + * dashboard. Defaults to {@code true}: a purely mechanical recomputation like + * {@link ExtensionActiveFlagCheck} is always safe to fix unattended. Override to return + * {@code false} only when fixing requires a judgment call a human needs to make - e.g. deciding + * which of two conflicting records is the correct one - not merely because a check is new. + */ + default boolean autoFixOnSchedule() { + return true; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckJobRequestHandler.java new file mode 100644 index 000000000..29bff33bd --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckJobRequestHandler.java @@ -0,0 +1,35 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +import org.jobrunr.jobs.annotations.Job; +import org.jobrunr.jobs.lambdas.JobRequestHandler; +import org.springframework.stereotype.Component; + +import org.eclipse.openvsx.migration.HandlerJobRequest; + +@Component +public class ConsistencyCheckJobRequestHandler implements JobRequestHandler> { + + private final ConsistencyCheckService service; + + public ConsistencyCheckJobRequestHandler(ConsistencyCheckService service) { + this.service = service; + } + + @Override + @Job(name = "Run data consistency checks", retries = 0) + public void run(HandlerJobRequest jobRequest) { + service.runAllChecks(); + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckService.java b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckService.java new file mode 100644 index 000000000..026acc937 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckService.java @@ -0,0 +1,188 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import jakarta.persistence.EntityManager; +import jakarta.transaction.Transactional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; + +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.json.ResultJson; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.util.LogService; +import org.eclipse.openvsx.util.NotFoundException; + +/** + * Orchestrates every registered {@link ConsistencyCheck}: overview for the admin dashboard, on-demand + * fixes, and the scheduled sweep's auto-fixing. Adding a new kind of check only requires a new + * {@code @Component implements ConsistencyCheck} bean - it is picked up here automatically and needs no + * further wiring. + *

+ * There is no persisted run history: findings are always computed live, and every fix - whether an + * admin clicked "Fix"/"Fix all" or the scheduled sweep did it unattended - is recorded as a normal admin + * log entry instead, attributed to a dedicated system user (the same pattern + * {@code ExtensionControlService}/{@code FixTargetPlatformsService} use), visible on the existing Admin + * Logs page rather than a bespoke report. + */ +@Service +public class ConsistencyCheckService { + + private static final Logger logger = LoggerFactory.getLogger(ConsistencyCheckService.class); + + private static final String SYSTEM_USER_LOGIN_NAME = "ConsistencyCheckUser"; + + private final Map checksById; + private final EntityManager entityManager; + private final RepositoryService repositories; + private final LogService logs; + + public ConsistencyCheckService( + List checks, + EntityManager entityManager, + RepositoryService repositories, + LogService logs + ) { + this.checksById = checks.stream().collect(Collectors.toMap(ConsistencyCheck::getId, c -> c)); + this.entityManager = entityManager; + this.repositories = repositories; + this.logs = logs; + } + + /** + * One summary per registered check, with its live findings count. + */ + public List listSummaries() { + return checksById.values().stream() + .map( + check -> new ConsistencyCheckSummary( + check.getId(), + check.getName(), + check.getDescription(), + check.check().size())) + .toList(); + } + + /** + * The live findings for one check. + */ + public List findings(String checkId) { + return getCheck(checkId).check(); + } + + /** + * Fixes every entity the check currently finds inconsistent, logging the outcome. + */ + @Transactional + public int fixAll(String checkId) { + var check = getCheck(checkId); + var fixed = fixAll(check); + if (fixed > 0) { + log("Fixed " + fixed + " finding(s) for consistency check '" + check.getId() + "'."); + } + return fixed; + } + + /** + * Fixes a single finding, logging the outcome. + */ + @Transactional + public void fixOne(String checkId, long entityId) { + var check = getCheck(checkId); + check.fix(entityId); + log("Fixed entity " + entityId + " for consistency check '" + check.getId() + "'."); + } + + /** + * Runs every registered check and auto-fixes what it found, unless the check opted out via + * {@link ConsistencyCheck#autoFixOnSchedule()} - in which case its findings are only logged as a + * warning for a human to act on from the dashboard. Used by the recurring background job; the admin + * dashboard's "Fix all" already covers triggering a fix on demand for a single check, so there is no + * separate "run all checks now" admin action. + */ + @Transactional + public void runAllChecks() { + for (var check : checksById.values()) { + var count = check.check().size(); + if (count == 0) { + continue; + } + + logger.atWarn() + .setMessage("Consistency check '{}' found {} inconsistent entit{}") + .addArgument(check.getId()) + .addArgument(count) + .addArgument(count == 1 ? "y" : "ies") + .log(); + + if (check.autoFixOnSchedule()) { + var fixed = fixAll(check); + log("Auto-fixed " + fixed + " finding(s) for consistency check '" + check.getId() + "'."); + } + } + } + + /** + * Recomputes findings once more after fixing each one, rather than fixing a stale list, in case + * fixing one finding incidentally resolves another (not expected for any current check, but not + * something to assume never happens either). + */ + private int fixAll(ConsistencyCheck check) { + var fixed = 0; + var findings = check.check(); + while (!findings.isEmpty()) { + for (var finding : findings) { + check.fix(finding.entityId()); + fixed++; + } + findings = check.check(); + } + return fixed; + } + + private void log(String message) { + logs.logAction(getSystemUser(), ResultJson.success(message)); + } + + /** + * The user every consistency-check fix is attributed to in the admin log, regardless of whether an + * admin clicked a button or the scheduled sweep did it unattended - fixing a detected inconsistency + * is a mechanical recomputation either way, so who triggered it is less relevant than what happened. + * Same convention as {@code ExtensionControlService#createExtensionControlUser()} and + * {@code FixTargetPlatformsService#getUser()}: a dedicated {@code provider = "system"} user, created + * once and reused afterward. + */ + private UserData getSystemUser() { + var user = repositories.findUserByLoginName("system", SYSTEM_USER_LOGIN_NAME); + if (user == null) { + user = new UserData(); + user.setProvider("system"); + user.setLoginName(SYSTEM_USER_LOGIN_NAME); + entityManager.persist(user); + } + return user; + } + + private ConsistencyCheck getCheck(String checkId) { + var check = checksById.get(checkId); + if (check == null) { + throw new NotFoundException(); + } + return check; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckSummary.java b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckSummary.java new file mode 100644 index 000000000..96b0820df --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyCheckSummary.java @@ -0,0 +1,22 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +/** + * One row of the admin "data consistency" overview: a check's identity and its live findings count. + * Deliberately has no historical/last-run fields - any fix, whether from the scheduled sweep or an + * admin clicking "Fix"/"Fix all", is recorded as a normal admin log entry (see + * {@link ConsistencyCheckService}) rather than in a separate table, so there is nothing here to go + * stale between one page load and the next. + */ +public record ConsistencyCheckSummary(String id, String name, String description, int currentFindingsCount) {} diff --git a/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyFinding.java b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyFinding.java new file mode 100644 index 000000000..f0c3adf85 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/consistency/ConsistencyFinding.java @@ -0,0 +1,22 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.consistency; + +/** + * One entity a {@link ConsistencyCheck} found to be inconsistent. + * + * @param entityId the id {@link ConsistencyCheck#fix(long)} needs to repair this finding + * @param label a short, human-readable identifier for the affected entity (e.g. its namespace/extension id) + * @param detail what specifically is inconsistent about it + */ +public record ConsistencyFinding(long entityId, String label, String detail) {} diff --git a/server/src/main/java/org/eclipse/openvsx/consistency/ExtensionActiveFlagCheck.java b/server/src/main/java/org/eclipse/openvsx/consistency/ExtensionActiveFlagCheck.java new file mode 100644 index 000000000..da75a93f5 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/consistency/ExtensionActiveFlagCheck.java @@ -0,0 +1,93 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +import java.util.List; + +import jakarta.persistence.EntityManager; +import jakarta.transaction.Transactional; +import org.springframework.stereotype.Component; + +import org.eclipse.openvsx.ExtensionService; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.util.NamingUtil; + +/** + * Finds and repairs any {@link Extension} whose {@code active} flag disagrees with whether it actually + * has an active version. This can only happen from a cross-transaction lost-update race: a transaction + * that never touches {@code active} (e.g. a download-count bump, a review, mirror metadata sync) loads + * the row, and a full-row {@code UPDATE} committed after a concurrent delete overwrites {@code active} + * back to its stale, pre-delete value. {@code @DynamicUpdate} on {@code Extension} prevents this going + * forward; this check repairs any row already left inconsistent by it before that fix (or by any future + * regression of the same kind). + */ +@Component +public class ExtensionActiveFlagCheck implements ConsistencyCheck { + + public static final String ID = "extension-active-flag"; + + private final EntityManager entityManager; + private final RepositoryService repositories; + private final ExtensionService extensions; + + public ExtensionActiveFlagCheck( + EntityManager entityManager, + RepositoryService repositories, + ExtensionService extensions + ) { + this.entityManager = entityManager; + this.repositories = repositories; + this.extensions = extensions; + } + + @Override + public String getId() { + return ID; + } + + @Override + public String getName() { + return "Extension active flag"; + } + + @Override + public String getDescription() { + return "Extensions whose `active` flag disagrees with whether any of their versions is active."; + } + + @Override + @Transactional + public List check() { + return repositories.findExtensionsWithInconsistentActiveFlag().stream() + .map( + extension -> new ConsistencyFinding( + extension.getId(), + NamingUtil.toExtensionId(extension), + extension.isActive() + ? "marked active, but no version of it is active" + : "marked inactive, but an active version exists")) + .toList(); + } + + @Override + @Transactional + public void fix(long entityId) { + var extension = entityManager.find(Extension.class, entityId); + if (extension == null) { + return; + } + + extensions.updateExtension(extension); + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/consistency/ScheduleConsistencyCheckJobs.java b/server/src/main/java/org/eclipse/openvsx/consistency/ScheduleConsistencyCheckJobs.java new file mode 100644 index 000000000..e2d70a309 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/consistency/ScheduleConsistencyCheckJobs.java @@ -0,0 +1,48 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +import java.time.ZoneId; + +import org.jobrunr.scheduling.JobRequestScheduler; +import org.jobrunr.scheduling.cron.Cron; +import org.springframework.boot.context.event.ApplicationStartedEvent; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Component; + +import org.eclipse.openvsx.migration.HandlerJobRequest; + +/** + * Runs every registered {@link ConsistencyCheck} once a day and records the result, so #1622's + * "on a constant basis" is covered without an admin needing to remember to open the dashboard. + */ +@Component +public class ScheduleConsistencyCheckJobs { + + private static final String JOB_ID = "consistency-check"; + + private final JobRequestScheduler scheduler; + + public ScheduleConsistencyCheckJobs(JobRequestScheduler scheduler) { + this.scheduler = scheduler; + } + + @EventListener + public void scheduleJobs(ApplicationStartedEvent event) { + scheduler.scheduleRecurrently( + JOB_ID, + Cron.daily(3), + ZoneId.of("UTC"), + new HandlerJobRequest<>(ConsistencyCheckJobRequestHandler.class)); + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/entities/Extension.java b/server/src/main/java/org/eclipse/openvsx/entities/Extension.java index 47ccaa750..75d01d9ae 100644 --- a/server/src/main/java/org/eclipse/openvsx/entities/Extension.java +++ b/server/src/main/java/org/eclipse/openvsx/entities/Extension.java @@ -18,12 +18,18 @@ import java.util.Objects; import jakarta.persistence.*; +import org.hibernate.annotations.DynamicUpdate; import org.jspecify.annotations.NonNull; import org.eclipse.openvsx.search.ExtensionSearch; import org.eclipse.openvsx.util.NamingUtil; +// Many code paths (downloads, reviews, mirror sync, deprecation checks) load and save this entity +// without ever touching `active`. Without @DynamicUpdate, Hibernate's default full-row UPDATE would +// rewrite `active` from whatever stale value that transaction happened to load, silently clobbering +// a concurrent, correct update to it (e.g. a delete that just deactivated the last active version). @Entity +@DynamicUpdate @Table(name = "extension") public class Extension implements Serializable { diff --git a/server/src/main/java/org/eclipse/openvsx/json/ConsistencyCheckJson.java b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyCheckJson.java new file mode 100644 index 000000000..79a3eb0a3 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyCheckJson.java @@ -0,0 +1,60 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.json; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonInclude.Include; + +@JsonInclude(Include.NON_NULL) +public class ConsistencyCheckJson { + + private String id; + + private String name; + + private String description; + + private int currentFindingsCount; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public int getCurrentFindingsCount() { + return currentFindingsCount; + } + + public void setCurrentFindingsCount(int currentFindingsCount) { + this.currentFindingsCount = currentFindingsCount; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/json/ConsistencyCheckListJson.java b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyCheckListJson.java new file mode 100644 index 000000000..701e4ea10 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyCheckListJson.java @@ -0,0 +1,38 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.json; + +import java.util.List; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonInclude.Include; + +@JsonInclude(Include.NON_NULL) +public class ConsistencyCheckListJson extends ResultJson { + + public static ConsistencyCheckListJson error(String message) { + var result = new ConsistencyCheckListJson(); + result.setError(message); + return result; + } + + private List checks; + + public List getChecks() { + return checks; + } + + public void setChecks(List checks) { + this.checks = checks; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/json/ConsistencyFindingJson.java b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyFindingJson.java new file mode 100644 index 000000000..991288c85 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyFindingJson.java @@ -0,0 +1,50 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.json; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonInclude.Include; + +@JsonInclude(Include.NON_NULL) +public class ConsistencyFindingJson { + + private long entityId; + + private String label; + + private String detail; + + public long getEntityId() { + return entityId; + } + + public void setEntityId(long entityId) { + this.entityId = entityId; + } + + public String getLabel() { + return label; + } + + public void setLabel(String label) { + this.label = label; + } + + public String getDetail() { + return detail; + } + + public void setDetail(String detail) { + this.detail = detail; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/json/ConsistencyFindingListJson.java b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyFindingListJson.java new file mode 100644 index 000000000..465f401f8 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/json/ConsistencyFindingListJson.java @@ -0,0 +1,38 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.json; + +import java.util.List; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonInclude.Include; + +@JsonInclude(Include.NON_NULL) +public class ConsistencyFindingListJson extends ResultJson { + + public static ConsistencyFindingListJson error(String message) { + var result = new ConsistencyFindingListJson(); + result.setError(message); + return result; + } + + private List findings; + + public List getFindings() { + return findings; + } + + public void setFindings(List findings) { + this.findings = findings; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionRepository.java index 02330e495..3d261aad5 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionRepository.java @@ -76,4 +76,14 @@ Extension findByNameIgnoreCaseAndNamespaceNameIgnoreCaseForUpdateNoWait( Streamable findAllNotMatchingByExtensionId(List extensionIds); Streamable findByReplacement(Extension replacement); + + // Extensions where `active` disagrees with whether any of their versions actually is. Repairs the + // fallout of the cross-transaction lost-update race @DynamicUpdate on Extension now prevents going + // forward; used by ExtensionActiveFlagReconciler to fix rows already affected. + @Query( + value = "select e.* from extension e where e.active <> exists" + + " (select 1 from extension_version v where v.extension_id = e.id and v.active = true)", + nativeQuery = true + ) + Streamable findExtensionsWithInconsistentActiveFlag(); } diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java index 9d7772293..36ac7b3a0 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java @@ -271,6 +271,10 @@ public Streamable findAllNotMatchingByExtensionId(List extens return extensionRepo.findAllNotMatchingByExtensionId(extensionIds); } + public Streamable findExtensionsWithInconsistentActiveFlag() { + return extensionRepo.findExtensionsWithInconsistentActiveFlag(); + } + public long countExtensions() { return extensionRepo.count(); } diff --git a/server/src/test/java/org/eclipse/openvsx/admin/ConsistencyAPITest.java b/server/src/test/java/org/eclipse/openvsx/admin/ConsistencyAPITest.java new file mode 100644 index 000000000..4e249e62c --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/admin/ConsistencyAPITest.java @@ -0,0 +1,136 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.admin; + +import java.util.List; + +import io.micrometer.core.instrument.MeterRegistry; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.security.oauth2.client.autoconfigure.servlet.OAuth2ClientWebSecurityAutoConfiguration; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; + +import org.eclipse.openvsx.consistency.ConsistencyCheckService; +import org.eclipse.openvsx.consistency.ConsistencyCheckSummary; +import org.eclipse.openvsx.consistency.ConsistencyFinding; +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.util.ErrorResultException; +import org.eclipse.openvsx.util.NotFoundException; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.*; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; + +@WebMvcTest( + value = ConsistencyAPI.class, + excludeAutoConfiguration = { OAuth2ClientWebSecurityAutoConfiguration.class } +) +@AutoConfigureMockMvc(addFilters = false) +class ConsistencyAPITest { + + @Autowired + MockMvc mockMvc; + + @MockitoBean + AdminService admins; + + @MockitoBean + ConsistencyCheckService service; + + @MockitoBean + MeterRegistry meterRegistry; + + @Test + void listChecks_requiresAdmin() throws Exception { + when(admins.checkAdminUser()) + .thenThrow(new ErrorResultException("Administration role is required.", HttpStatus.FORBIDDEN)); + + mockMvc.perform(get("/admin/consistency").accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isForbidden()); + } + + @Test + void listChecks_returnsOneSummaryPerRegisteredCheck() throws Exception { + when(admins.checkAdminUser()).thenReturn(adminUser()); + when(service.listSummaries()).thenReturn( + List.of( + new ConsistencyCheckSummary( + "extension-active-flag", + "Extension active flag", + "Extensions whose `active` flag disagrees with whether any of their versions is active.", + 2))); + + mockMvc.perform(get("/admin/consistency").accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.checks[0].id").value("extension-active-flag")) + .andExpect(jsonPath("$.checks[0].currentFindingsCount").value(2)); + } + + @Test + void findings_returnsNotFoundForUnknownCheck() throws Exception { + when(admins.checkAdminUser()).thenReturn(adminUser()); + when(service.findings("does-not-exist")).thenThrow(new NotFoundException()); + + mockMvc.perform(get("/admin/consistency/does-not-exist/findings").accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error").value("Unknown consistency check: does-not-exist")); + } + + @Test + void findings_returnsLiveFindings() throws Exception { + when(admins.checkAdminUser()).thenReturn(adminUser()); + when(service.findings("extension-active-flag")).thenReturn( + List.of( + new ConsistencyFinding(42L, "acme.foo", "marked active, but no version of it is active"))); + + mockMvc.perform(get("/admin/consistency/extension-active-flag/findings").accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.findings[0].entityId").value(42)) + .andExpect(jsonPath("$.findings[0].label").value("acme.foo")); + } + + @Test + void fixAll_fixesEveryCurrentFinding() throws Exception { + when(admins.checkAdminUser()).thenReturn(adminUser()); + when(service.fixAll("extension-active-flag")).thenReturn(3); + + mockMvc.perform(post("/admin/consistency/extension-active-flag/fix").accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.success").value("Fixed 3 finding(s) for check 'extension-active-flag'.")); + + verify(service).fixAll("extension-active-flag"); + } + + @Test + void fixOne_fixesTheGivenEntity() throws Exception { + when(admins.checkAdminUser()).thenReturn(adminUser()); + + mockMvc.perform(post("/admin/consistency/extension-active-flag/fix/42").accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + + verify(service).fixOne(eq("extension-active-flag"), eq(42L)); + } + + private static UserData adminUser() { + var user = new UserData(); + user.setRole(UserData.Role.ADMIN); + return user; + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/consistency/ConsistencyCheckServiceAutoFixTest.java b/server/src/test/java/org/eclipse/openvsx/consistency/ConsistencyCheckServiceAutoFixTest.java new file mode 100644 index 000000000..2d07c29c4 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/consistency/ConsistencyCheckServiceAutoFixTest.java @@ -0,0 +1,162 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +import java.util.ArrayList; +import java.util.List; + +import jakarta.persistence.EntityManager; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.json.ResultJson; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.util.LogService; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Covers {@link ConsistencyCheckService#runAllChecks()}'s auto-fix behavior in isolation, using a fake + * {@link ConsistencyCheck} rather than the real (always-auto-fixing) {@link ExtensionActiveFlagCheck}, so + * both the opt-in and opt-out paths of {@link ConsistencyCheck#autoFixOnSchedule()} are exercised + * without needing a database. + */ +class ConsistencyCheckServiceAutoFixTest { + + private final EntityManager entityManager = mock(EntityManager.class); + private final RepositoryService repositories = mock(RepositoryService.class); + private final LogService logs = mock(LogService.class); + private final UserData systemUser = new UserData(); + + @Test + void runAllChecks_fixesFindingsByDefault() { + var check = new FakeCheck(true); + check.findings.add(new ConsistencyFinding(1L, "thing-1", "broken")); + newService(check).runAllChecks(); + + assertThat(check.fixedIds) + .as("a check that does not opt out must be auto-fixed by the scheduled run") + .containsExactly(1L); + } + + @Test + void runAllChecks_leavesFindingsAloneWhenACheckOptsOut() { + var check = new FakeCheck(false); + check.findings.add(new ConsistencyFinding(1L, "thing-1", "broken")); + newService(check).runAllChecks(); + + assertThat(check.fixedIds) + .as("a check that opts out of autoFixOnSchedule must not be touched by the scheduled run") + .isEmpty(); + verify(logs, never()).logAction(any(), any()); + } + + /** + * Every fix - scheduled or manual - is attributed to the dedicated system user and recorded via the + * normal admin log, rather than a bespoke history table. + */ + @Test + void runAllChecks_logsTheFixUnderTheSystemUser() { + var check = new FakeCheck(true); + check.findings.add(new ConsistencyFinding(1L, "thing-1", "broken")); + when(repositories.findUserByLoginName("system", "ConsistencyCheckUser")).thenReturn(systemUser); + + newService(check).runAllChecks(); + + var resultCaptor = ArgumentCaptor.forClass(ResultJson.class); + verify(logs).logAction(eq(systemUser), resultCaptor.capture()); + assertThat(resultCaptor.getValue().getSuccess()) + .as("the log message must mention the check and how many findings it auto-fixed") + .contains("fake-check") + .contains("1"); + } + + @Test + void fixAll_logsUnderTheSystemUserToo() { + var check = new FakeCheck(true); + check.findings.add(new ConsistencyFinding(1L, "thing-1", "broken")); + when(repositories.findUserByLoginName("system", "ConsistencyCheckUser")).thenReturn(systemUser); + + var fixed = newService(check).fixAll("fake-check"); + + assertThat(fixed).isEqualTo(1); + verify(logs).logAction(eq(systemUser), any(ResultJson.class)); + } + + @Test + void fixAll_createsTheSystemUserWhenItDoesNotExistYet() { + var check = new FakeCheck(true); + check.findings.add(new ConsistencyFinding(1L, "thing-1", "broken")); + when(repositories.findUserByLoginName("system", "ConsistencyCheckUser")).thenReturn(null); + + newService(check).fixAll("fake-check"); + + var userCaptor = ArgumentCaptor.forClass(UserData.class); + verify(entityManager).persist(userCaptor.capture()); + assertThat(userCaptor.getValue().getProvider()).isEqualTo("system"); + assertThat(userCaptor.getValue().getLoginName()).isEqualTo("ConsistencyCheckUser"); + } + + private ConsistencyCheckService newService(FakeCheck check) { + return new ConsistencyCheckService(List.of(check), entityManager, repositories, logs); + } + + private static class FakeCheck implements ConsistencyCheck { + + private final boolean autoFix; + private final List findings = new ArrayList<>(); + private final List fixedIds = new ArrayList<>(); + + FakeCheck(boolean autoFix) { + this.autoFix = autoFix; + } + + @Override + public String getId() { + return "fake-check"; + } + + @Override + public String getName() { + return "Fake check"; + } + + @Override + public String getDescription() { + return "A fake check for testing ConsistencyCheckService in isolation."; + } + + @Override + public List check() { + return List.copyOf(findings); + } + + @Override + public void fix(long entityId) { + fixedIds.add(entityId); + findings.removeIf(f -> f.entityId() == entityId); + } + + @Override + public boolean autoFixOnSchedule() { + return autoFix; + } + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/consistency/ConsistencyCheckServiceTest.java b/server/src/test/java/org/eclipse/openvsx/consistency/ConsistencyCheckServiceTest.java new file mode 100644 index 000000000..e1cea80d6 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/consistency/ConsistencyCheckServiceTest.java @@ -0,0 +1,139 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +import java.util.List; + +import jakarta.persistence.EntityManager; +import org.jobrunr.scheduling.JobRequestScheduler; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +import org.eclipse.openvsx.AbstractPostgresContainerTest; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.search.SearchUtilService; +import org.eclipse.openvsx.util.NotFoundException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Covers the generic orchestration {@link ConsistencyCheckService} provides on top of whatever + * {@link ConsistencyCheck} beans are registered - using the real {@link ExtensionActiveFlagCheck} as the + * one currently wired up, rather than a fake, so this also doubles as an end-to-end check that a + * registered check is actually picked up with no further wiring. + */ +@SpringBootTest +class ConsistencyCheckServiceTest extends AbstractPostgresContainerTest { + + private static final String NAMESPACE = "consistency-service-testns"; + private static final String EXTENSION = "consistency-service-testext"; + + @Autowired + ConsistencyCheckService service; + + @Autowired + EntityManager em; + + @Autowired + PlatformTransactionManager txManager; + + @MockitoBean + SearchUtilService search; + + @MockitoBean + JobRequestScheduler scheduler; + + @BeforeEach + void setUp() { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var namespace = new Namespace(); + namespace.setName(NAMESPACE); + em.persist(namespace); + + var extension = new Extension(); + extension.setName(EXTENSION); + extension.setNamespace(namespace); + extension.setActive(true); // inconsistent on purpose: no version exists at all + em.persist(extension); + }); + } + + @Test + void listSummaries_includesEveryRegisteredCheckWithLiveCount() { + var summaries = service.listSummaries(); + + var extensionActiveFlag = summaries.stream() + .filter(s -> s.id().equals(ExtensionActiveFlagCheck.ID)) + .findFirst() + .orElseThrow(); + assertThat(extensionActiveFlag.currentFindingsCount()) + .as("the extension created in setUp is currently inconsistent") + .isGreaterThanOrEqualTo(1); + } + + @Test + void findings_throwsNotFoundForAnUnregisteredCheckId() { + assertThatThrownBy(() -> service.findings("does-not-exist")).isInstanceOf(NotFoundException.class); + } + + /** + * ExtensionActiveFlagCheck auto-fixes on schedule by default (a pure recomputation, always safe + * unattended), so a run must both leave the database actually fixed and record what it did as a + * normal admin log entry attributed to the dedicated system user - not a bespoke history table. + */ + @Test + void runAllChecks_autoFixesAndLogsUnderTheSystemUser() { + service.runAllChecks(); + + var summary = service.listSummaries().stream() + .filter(s -> s.id().equals(ExtensionActiveFlagCheck.ID)) + .findFirst() + .orElseThrow(); + assertThat(summary.currentFindingsCount()) + .as("the finding must actually be fixed by the run, not just reported") + .isZero(); + + assertThat(logMessagesForSystemUser()) + .as("the fix must be recorded as an admin log entry attributed to the system user") + .anyMatch(message -> message.contains(ExtensionActiveFlagCheck.ID)); + } + + private List logMessagesForSystemUser() { + return new TransactionTemplate(txManager).execute( + status -> em.createQuery( + "select p.message from PersistedLog p where p.user.loginName = :loginName", + String.class) + .setParameter("loginName", "ConsistencyCheckUser") + .getResultList()); + } + + @AfterEach + void tearDown() { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + em.createQuery("delete from PersistedLog p where p.user.loginName = :loginName") + .setParameter("loginName", "ConsistencyCheckUser").executeUpdate(); + em.createQuery("delete from Extension e where e.namespace.name = :namespace") + .setParameter("namespace", NAMESPACE).executeUpdate(); + em.createQuery("delete from Namespace n where n.name = :namespace") + .setParameter("namespace", NAMESPACE).executeUpdate(); + }); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/consistency/ExtensionActiveFlagCheckTest.java b/server/src/test/java/org/eclipse/openvsx/consistency/ExtensionActiveFlagCheckTest.java new file mode 100644 index 000000000..06a2fa2aa --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/consistency/ExtensionActiveFlagCheckTest.java @@ -0,0 +1,271 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.consistency; + +import java.time.LocalDateTime; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +import jakarta.persistence.EntityManager; +import org.jobrunr.scheduling.JobRequestScheduler; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +import org.eclipse.openvsx.AbstractPostgresContainerTest; +import org.eclipse.openvsx.ExtensionService; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.ExtensionVersion; +import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.entities.PersonalAccessToken; +import org.eclipse.openvsx.entities.PersonalAccessTokenType; +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.search.SearchUtilService; +import org.eclipse.openvsx.util.TargetPlatform; +import org.eclipse.openvsx.util.TargetPlatformVersion; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Regression tests for the {@code Extension.active} lost-update race: a transaction that never touches + * {@code active} (e.g. a download-count bump) could load the row before a concurrent delete commits and, + * on a later commit, blindly rewrite {@code active} back to its stale pre-delete value via Hibernate's + * default full-row {@code UPDATE}. Covers both the {@code @DynamicUpdate} fix that prevents it going + * forward, and {@link ExtensionActiveFlagCheck} (run through {@link ConsistencyCheckService}), which + * repairs rows already left inconsistent by it. + */ +@SpringBootTest +class ExtensionActiveFlagCheckTest extends AbstractPostgresContainerTest { + + private static final String NAMESPACE = "active-flag-testns"; + private static final String EXTENSION = "active-flag-testext"; + private static final String OWNER_LOGIN = "active-flag-owner"; + + @Autowired + ExtensionService extensionService; + + @Autowired + ConsistencyCheckService consistencyCheckService; + + @Autowired + RepositoryService repositories; + + @Autowired + EntityManager em; + + @Autowired + PlatformTransactionManager txManager; + + @MockitoBean + SearchUtilService search; + + @MockitoBean + JobRequestScheduler scheduler; + + private long extensionId; + private long ownerId; + private long ownerTokenId; + + @BeforeEach + void setUp() { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var owner = new UserData(); + owner.setLoginName(OWNER_LOGIN); + em.persist(owner); + + var token = new PersonalAccessToken(); + token.setUser(owner); + token.setValue(OWNER_LOGIN + "_token"); + token.setCreatedTimestamp(LocalDateTime.now()); + token.setActive(true); + token.setType(PersonalAccessTokenType.LLT); + em.persist(token); + + var namespace = new Namespace(); + namespace.setName(NAMESPACE); + em.persist(namespace); + + var extension = new Extension(); + extension.setName(EXTENSION); + extension.setNamespace(namespace); + extension.setActive(true); + em.persist(extension); + em.flush(); + + ownerId = owner.getId(); + ownerTokenId = token.getId(); + extensionId = extension.getId(); + }); + } + + private UserData owner() { + var owner = new UserData(); + owner.setId(ownerId); + owner.setLoginName(OWNER_LOGIN); + return owner; + } + + /** + * Reproduces the race directly: a writer transaction loads the extension (sees active=true) and + * only ever touches an unrelated field (download count), but doesn't commit until after a concurrent + * delete has deactivated the last active version and committed active=false. Without + * {@code @DynamicUpdate}, the writer's full-row UPDATE would blindly rewrite active back to true + * using its stale in-memory snapshot. + */ + @Test + void unrelatedConcurrentWrite_doesNotResurrectActiveFlagAfterDelete() throws InterruptedException { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + var writerLoaded = new CountDownLatch(1); + var deleteCommitted = new CountDownLatch(1); + var writerFailure = new AtomicReference(); + + var writer = new Thread(() -> { + try { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var extension = em.find(Extension.class, extensionId); + assertThat(extension.isActive()) + .as("the writer must load the pre-delete state to reproduce the race") + .isTrue(); + writerLoaded.countDown(); + await(deleteCommitted); + + // Only an unrelated field is touched here - never `active`. + extension.setDownloadCount(extension.getDownloadCount() + 1); + }); + } catch (Throwable t) { + writerFailure.set(t); + } + }); + writer.start(); + writerLoaded.await(); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); + deleteCommitted.countDown(); + writer.join(); + + assertThat(writerFailure.get()).as("the unrelated write must not fail").isNull(); + assertThat(extensionActiveInDb()) + .as( + "an unrelated concurrent write must not resurrect `active` after the last active " + + "version was deleted") + .isFalse(); + } + + /** + * Repairs a row already left inconsistent (as if by the race above, before the {@code @DynamicUpdate} + * fix existed): {@code active} is forced true directly in the database while no version is active. + */ + @Test + void consistencyCheck_fixesStaleActiveFlag() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, false, true); + forceActiveFlagInDb(true); + assertThat(extensionActiveInDb()).isTrue(); + + var fixed = consistencyCheckService.fixAll(ExtensionActiveFlagCheck.ID); + + assertThat(fixed).isEqualTo(1); + assertThat(extensionActiveInDb()) + .as("the check must recompute `active` from actual version state") + .isFalse(); + } + + /** + * A consistent row (active matches having an active version) must be left untouched. + */ + @Test + void consistencyCheck_leavesConsistentExtensionsAlone() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + assertThat( + repositories.findExtensionsWithInconsistentActiveFlag().stream() + .map(Extension::getId) + .toList()) + .doesNotContain(extensionId); + + var fixed = consistencyCheckService.fixAll(ExtensionActiveFlagCheck.ID); + + assertThat(fixed).isZero(); + assertThat(extensionActiveInDb()).isTrue(); + } + + private void await(CountDownLatch latch) { + try { + latch.await(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + private void persistVersion(String version, String targetPlatform, boolean active, boolean removed) { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var extension = em.find(Extension.class, extensionId); + var token = em.getReference(PersonalAccessToken.class, ownerTokenId); + var extVersion = new ExtensionVersion(); + extVersion.setVersion(version); + extVersion.setTargetPlatform(targetPlatform); + extVersion.setExtension(extension); + extVersion.setPublishedWith(token); + extVersion.setActive(active); + extVersion.setRemoved(removed); + if (removed) { + extVersion.setActive(false); + extVersion.setRemovedTimestamp(LocalDateTime.now()); + extVersion.setRemovedBy(em.getReference(UserData.class, ownerId)); + } + em.persist(extVersion); + }); + } + + private void forceActiveFlagInDb(boolean active) { + new TransactionTemplate(txManager).executeWithoutResult( + status -> em.createNativeQuery("update extension set active = :active where id = :id") + .setParameter("active", active) + .setParameter("id", extensionId) + .executeUpdate()); + } + + private boolean extensionActiveInDb() { + return Boolean.TRUE.equals( + new TransactionTemplate(txManager).execute( + status -> (boolean) em + .createNativeQuery("select active from extension where id = :id") + .setParameter("id", extensionId) + .getSingleResult())); + } + + @AfterEach + void tearDown() { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + em.createQuery("delete from ExtensionVersion ev where ev.extension.namespace.name = :namespace") + .setParameter("namespace", NAMESPACE).executeUpdate(); + em.createQuery("delete from Extension e where e.namespace.name = :namespace") + .setParameter("namespace", NAMESPACE).executeUpdate(); + em.createQuery("delete from PersistedLog pl where pl.user.loginName = :login") + .setParameter("login", OWNER_LOGIN).executeUpdate(); + em.createQuery("delete from PersonalAccessToken t where t.user.loginName = :login") + .setParameter("login", OWNER_LOGIN).executeUpdate(); + em.createQuery("delete from Namespace n where n.name = :namespace") + .setParameter("namespace", NAMESPACE).executeUpdate(); + em.createQuery("delete from UserData u where u.loginName = :login") + .setParameter("login", OWNER_LOGIN).executeUpdate(); + }); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java index 4dd7c266a..00fbfc78d 100644 --- a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java +++ b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java @@ -231,6 +231,7 @@ void testExecuteQueries() { () -> repositories.findExtensionForUpdate("name", "namespace"), () -> repositories.findExtensionForUpdateNoWait("name", "namespace"), () -> repositories.findExtensions(namespace), + () -> repositories.findExtensionsWithInconsistentActiveFlag(), () -> repositories.findFileByType(extVersion, "type"), () -> repositories.findFiles(extVersion), () -> repositories.findFilesByStorageType("storageType"), diff --git a/webui/CHANGELOG.md b/webui/CHANGELOG.md index 607918efa..b53fdc673 100644 --- a/webui/CHANGELOG.md +++ b/webui/CHANGELOG.md @@ -4,6 +4,10 @@ This change log covers only the frontend library (webui) of Open VSX. ## [next] (unreleased) +### Added + +- Add a "Data Consistency" page to the admin dashboard (#1622): a live overview of every registered consistency check's finding count, with actions to refresh it and to fix findings one at a time or all at once + ### Fixed - Fix a React warning ("Received `true` for a non-boolean attribute `notched`") from the admin dashboard's publisher role filter, whose custom `InputBase` doesn't consume the `notched` prop MUI's `Select` injects for the (unused) outlined variant diff --git a/webui/src/extension-registry-service.ts b/webui/src/extension-registry-service.ts index e39bbe0bd..5c4963201 100644 --- a/webui/src/extension-registry-service.ts +++ b/webui/src/extension-registry-service.ts @@ -55,7 +55,9 @@ import { TrustedPublisher, TrustedPublisherList, TrustedPublisherRequest, - TrustedPublisherStatus + TrustedPublisherStatus, + ConsistencyCheckList, + ConsistencyFindingList } from './extension-registry-types'; import { createAbsoluteURL, addQuery } from './utils'; import { sendRequest, ErrorResponse, sendNonRetriableRequest, sendStrictRequest } from './server-request'; @@ -781,6 +783,13 @@ export interface AdminService { deleteCustomerRateLimitToken(customerName: string, tokenId: number): Promise>; getSettings(abortController: AbortController): Promise>; updateSettings(settings: Settings): Promise>; + getConsistencyChecks(abortController: AbortController): Promise>; + getConsistencyFindings( + abortController: AbortController, + checkId: string + ): Promise>; + fixConsistencyFindings(checkId: string): Promise>; + fixConsistencyFinding(checkId: string, entityId: number): Promise>; } export interface AdminServiceConstructor { @@ -1583,6 +1592,62 @@ export class AdminServiceImpl implements AdminService { headers }); } + + async getConsistencyChecks(abortController: AbortController): Promise> { + return sendNonRetriableRequest({ + abortController, + credentials: true, + endpoint: createAbsoluteURL([this.registry.serverUrl, 'admin', 'consistency']) + }); + } + + async getConsistencyFindings( + abortController: AbortController, + checkId: string + ): Promise> { + return sendNonRetriableRequest({ + abortController, + credentials: true, + endpoint: createAbsoluteURL([this.registry.serverUrl, 'admin', 'consistency', checkId, 'findings']) + }); + } + + async fixConsistencyFindings(checkId: string): Promise> { + const headers = await this.csrfHeaders(); + return sendNonRetriableRequest({ + method: 'POST', + credentials: true, + endpoint: createAbsoluteURL([this.registry.serverUrl, 'admin', 'consistency', checkId, 'fix']), + headers + }); + } + + async fixConsistencyFinding(checkId: string, entityId: number): Promise> { + const headers = await this.csrfHeaders(); + return sendNonRetriableRequest({ + method: 'POST', + credentials: true, + endpoint: createAbsoluteURL([ + this.registry.serverUrl, + 'admin', + 'consistency', + checkId, + 'fix', + String(entityId) + ]), + headers + }); + } + + private async csrfHeaders(): Promise> { + const csrfResponse = await this.registry.getCsrfToken(); + const headers: Record = {}; + if (!isError(csrfResponse)) { + const csrfToken = csrfResponse as CsrfTokenJson; + headers[csrfToken.header] = csrfToken.value; + } + return headers; + } } export interface ExtensionFilter { diff --git a/webui/src/extension-registry-types.ts b/webui/src/extension-registry-types.ts index 5ff231941..6f8d8608c 100644 --- a/webui/src/extension-registry-types.ts +++ b/webui/src/extension-registry-types.ts @@ -584,3 +584,24 @@ export interface LogPageableList { export interface Settings { readOnly: boolean; } + +export interface ConsistencyCheck { + id: string; + name: string; + description: string; + currentFindingsCount: number; +} + +export interface ConsistencyCheckList { + checks: ConsistencyCheck[]; +} + +export interface ConsistencyFinding { + entityId: number; + label: string; + detail: string; +} + +export interface ConsistencyFindingList { + findings: ConsistencyFinding[]; +} diff --git a/webui/src/pages/admin-dashboard/admin-dashboard-routes.ts b/webui/src/pages/admin-dashboard/admin-dashboard-routes.ts index f8837d9d3..43625d9f5 100644 --- a/webui/src/pages/admin-dashboard/admin-dashboard-routes.ts +++ b/webui/src/pages/admin-dashboard/admin-dashboard-routes.ts @@ -22,4 +22,5 @@ export namespace AdminDashboardRoutes { export const USAGE_STATS = createRoute([ROOT, 'usage']); export const SETTINGS = createRoute([ROOT, 'settings']); export const LOGS = createRoute([ROOT, 'logs']); + export const CONSISTENCY = createRoute([ROOT, 'consistency']); } diff --git a/webui/src/pages/admin-dashboard/admin-dashboard.tsx b/webui/src/pages/admin-dashboard/admin-dashboard.tsx index cb78f7e45..e1ab33664 100644 --- a/webui/src/pages/admin-dashboard/admin-dashboard.tsx +++ b/webui/src/pages/admin-dashboard/admin-dashboard.tsx @@ -16,6 +16,7 @@ import AccountBoxIcon from '@mui/icons-material/AccountBox'; import AssignmentIndIcon from '@mui/icons-material/AssignmentInd'; import BarChartIcon from '@mui/icons-material/BarChart'; import ExtensionSharpIcon from '@mui/icons-material/ExtensionSharp'; +import FactCheckIcon from '@mui/icons-material/FactCheck'; import HistoryIcon from '@mui/icons-material/History'; import PeopleIcon from '@mui/icons-material/People'; import PersonIcon from '@mui/icons-material/Person'; @@ -42,6 +43,7 @@ import { Welcome } from './welcome'; const ExtensionAdmin = lazy(() => import('./extension-admin').then(m => ({ default: m.ExtensionAdmin }))); const UsageStatsView = lazy(() => import('./usage-stats/usage-stats').then(m => ({ default: m.UsageStatsView }))); +const DataConsistency = lazy(() => import('./consistency/consistency').then(m => ({ default: m.DataConsistency }))); const navConfig: NavEntry[] = [ { @@ -98,7 +100,13 @@ const navConfig: NavEntry[] = [ icon: , description: 'Manage runtime settings for the registry' }, - { path: AdminDashboardRoutes.LOGS, name: 'Logs', icon: , description: 'Browse admin activity logs' } + { path: AdminDashboardRoutes.LOGS, name: 'Logs', icon: , description: 'Browse admin activity logs' }, + { + path: AdminDashboardRoutes.CONSISTENCY, + name: 'Data Consistency', + icon: , + description: 'Check the database for known inconsistencies and fix them' + } ]; const routeNames: { [key: string]: string } = { @@ -179,6 +187,7 @@ export const AdminDashboard: FunctionComponent = props => { } /> } /> } /> + } /> } /> diff --git a/webui/src/pages/admin-dashboard/consistency/consistency.tsx b/webui/src/pages/admin-dashboard/consistency/consistency.tsx new file mode 100644 index 000000000..6f897e6f3 --- /dev/null +++ b/webui/src/pages/admin-dashboard/consistency/consistency.tsx @@ -0,0 +1,178 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ + +import { FC, useState } from 'react'; +import { + Accordion, + AccordionDetails, + AccordionSummary, + Alert, + Box, + Button, + Chip, + List, + ListItem, + ListItemText, + Paper, + Typography +} from '@mui/material'; +import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; +import RefreshIcon from '@mui/icons-material/Refresh'; +import { ButtonWithProgress } from '../../../components/button-with-progress'; +import type { ConsistencyCheck } from '../../../extension-registry-types'; +import { handleError } from '../../../utils'; +import { + useConsistencyChecks, + useConsistencyFindings, + useFixAllConsistencyFindings, + useFixConsistencyFinding, + useRefreshConsistency +} from './use-consistency'; + +/** + * Admin dashboard overview of every registered data-consistency check (#1622): each check's live + * finding count, and actions to fix them - one at a time or all at once. A check that does not require + * human judgment also auto-fixes itself once a day via a scheduled job; every fix, scheduled or manual, + * shows up on the Admin Logs page rather than a bespoke history view here. + */ +export const DataConsistency: FC = () => { + const { data, isLoading, error } = useConsistencyChecks(); + const refresh = useRefreshConsistency(); + const [expanded, setExpanded] = useState>(new Set()); + + const toggleExpanded = (checkId: string) => { + setExpanded(prev => { + const next = new Set(prev); + if (next.has(checkId)) { + next.delete(checkId); + } else { + next.add(checkId); + } + return next; + }); + }; + + return ( + + + + + Data Consistency + + + Checks the database for known inconsistencies and lets you fix them directly. Findings shown + below are always live; a check that does not require human judgment also auto-fixes itself once + a day via a scheduled job. + + + + + + {error && {handleError(error)}} + + {!error && + !isLoading && + data?.checks.map(check => ( + toggleExpanded(check.id)} + /> + ))} + + ); +}; + +const ConsistencyCheckCard: FC<{ check: ConsistencyCheck; expanded: boolean; onToggle: () => void }> = ({ + check, + expanded, + onToggle +}) => { + const { data, isLoading, error } = useConsistencyFindings(check.id, expanded); + const fixAll = useFixAllConsistencyFindings(check.id); + const fixOne = useFixConsistencyFinding(check.id); + + const healthy = check.currentFindingsCount === 0; + + return ( + + }> + + + + {check.name} + + {check.description} + + + {!healthy && ( + { + // Fixing must not also toggle the accordion the button sits inside of. + event.stopPropagation(); + fixAll.mutate(); + }}> + Fix all + + )} + + + + {fixAll.isError && {handleError(fixAll.error)}} + {fixOne.isError && {handleError(fixOne.error)}} + {error && {handleError(error)}} + + {!error && + !isLoading && + data && + (data.findings.length === 0 ? ( + No inconsistencies found. + ) : ( + + + {data.findings.map(finding => ( + fixOne.mutate(finding.entityId)}> + Fix + + }> + + + ))} + + + ))} + + + ); +}; diff --git a/webui/src/pages/admin-dashboard/consistency/use-consistency.ts b/webui/src/pages/admin-dashboard/consistency/use-consistency.ts new file mode 100644 index 000000000..02bc8b594 --- /dev/null +++ b/webui/src/pages/admin-dashboard/consistency/use-consistency.ts @@ -0,0 +1,86 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ + +import { useContext } from 'react'; +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { MainContext } from '../../../context'; +import { controllerFromSignal } from '../../../query-client'; + +const checksQueryKey = ['admin', 'consistency', 'checks'] as const; +const findingsQueryKey = (checkId: string) => ['admin', 'consistency', 'checks', checkId, 'findings'] as const; + +/** + * The overview of every registered consistency check: its live findings count and its last recorded + * (scheduled or manually triggered) run. + */ +export const useConsistencyChecks = () => { + const { service } = useContext(MainContext); + return useQuery({ + queryKey: checksQueryKey, + queryFn: ({ signal }) => service.admin.getConsistencyChecks(controllerFromSignal(signal)) + }); +}; + +/** + * The live findings of one check. Enabled lazily (only once its card is expanded) so opening the + * overview page never has to compute every check's full finding list up front. + */ +export const useConsistencyFindings = (checkId: string, enabled: boolean) => { + const { service } = useContext(MainContext); + return useQuery({ + queryKey: findingsQueryKey(checkId), + queryFn: ({ signal }) => service.admin.getConsistencyFindings(controllerFromSignal(signal), checkId), + enabled + }); +}; + +/** + * Invalidates both the overview and this check's findings - shared by every mutation below, since all + * of them change what both would report. + */ +const useInvalidateConsistency = (checkId: string) => { + const queryClient = useQueryClient(); + return () => { + queryClient.invalidateQueries({ queryKey: checksQueryKey }); + queryClient.invalidateQueries({ queryKey: findingsQueryKey(checkId) }); + }; +}; + +export const useFixAllConsistencyFindings = (checkId: string) => { + const { service } = useContext(MainContext); + const invalidate = useInvalidateConsistency(checkId); + return useMutation({ + mutationFn: () => service.admin.fixConsistencyFindings(checkId), + onSuccess: invalidate + }); +}; + +export const useFixConsistencyFinding = (checkId: string) => { + const { service } = useContext(MainContext); + const invalidate = useInvalidateConsistency(checkId); + return useMutation({ + mutationFn: (entityId: number) => service.admin.fixConsistencyFinding(checkId, entityId), + onSuccess: invalidate + }); +}; + +/** + * Re-fetches every consistency query currently on screen - the overview and any expanded check's + * findings. Purely a client-side cache refresh: findings are always computed live server-side on every + * request anyway, so there is no server action to trigger, just a reason to ask again right now instead + * of waiting for the next mount or mutation. + */ +export const useRefreshConsistency = () => { + const queryClient = useQueryClient(); + return () => queryClient.invalidateQueries({ queryKey: ['admin', 'consistency'] }); +}; diff --git a/webui/test/unit/pages/admin-dashboard/consistency/consistency.spec.tsx b/webui/test/unit/pages/admin-dashboard/consistency/consistency.spec.tsx new file mode 100644 index 000000000..1b563b561 --- /dev/null +++ b/webui/test/unit/pages/admin-dashboard/consistency/consistency.spec.tsx @@ -0,0 +1,110 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ + +import { describe, expect, it, vi } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { renderWithProviders } from '../../../support/test-providers'; +import { DataConsistency } from '../../../../../src/pages/admin-dashboard/consistency/consistency'; +import { ExtensionRegistryService, AdminService } from '../../../../../src/extension-registry-service'; +import { ConsistencyCheck, ConsistencyFinding } from '../../../../../src/extension-registry-types'; + +const CHECK: ConsistencyCheck = { + id: 'extension-active-flag', + name: 'Extension active flag', + description: 'Extensions whose active flag disagrees with whether any of their versions is active.', + currentFindingsCount: 1 +}; + +const FINDING: ConsistencyFinding = { + entityId: 42, + label: 'acme.foo', + detail: 'marked active, but no version of it is active' +}; + +function serviceWith(overrides: Partial = {}): ExtensionRegistryService { + const admin = { + getConsistencyChecks: vi.fn().mockResolvedValue({ checks: [CHECK] }), + getConsistencyFindings: vi.fn().mockResolvedValue({ findings: [FINDING] }), + fixConsistencyFindings: vi.fn().mockResolvedValue({ success: 'Fixed 1 finding(s).' }), + fixConsistencyFinding: vi.fn().mockResolvedValue({ success: 'Fixed entity 42.' }), + ...overrides + } as unknown as AdminService; + + return { serverUrl: 'https://open-vsx.org', admin } as ExtensionRegistryService; +} + +describe('DataConsistency', () => { + it('shows a finding count badge for each registered check', async () => { + renderWithProviders(, { mainContext: { service: serviceWith() } }); + + expect(await screen.findByText('Extension active flag')).toBeInTheDocument(); + expect(screen.getByText('1 found')).toBeInTheDocument(); + }); + + it('loads and displays findings only once the check card is expanded', async () => { + const service = serviceWith(); + const ue = userEvent.setup(); + renderWithProviders(, { mainContext: { service } }); + + await screen.findByText('Extension active flag'); + expect(service.admin.getConsistencyFindings).not.toHaveBeenCalled(); + + await ue.click(screen.getByText('Extension active flag')); + + expect(await screen.findByText('acme.foo')).toBeInTheDocument(); + expect(screen.getByText('marked active, but no version of it is active')).toBeInTheDocument(); + expect(service.admin.getConsistencyFindings).toHaveBeenCalledWith(expect.anything(), 'extension-active-flag'); + }); + + it('fixes a single finding and refreshes the list', async () => { + const service = serviceWith(); + const ue = userEvent.setup(); + renderWithProviders(, { mainContext: { service } }); + + await ue.click(await screen.findByText('Extension active flag')); + await screen.findByText('acme.foo'); + + await ue.click(screen.getByRole('button', { name: 'Fix' })); + + expect(service.admin.fixConsistencyFinding).toHaveBeenCalledWith('extension-active-flag', 42); + await waitFor(() => expect(service.admin.getConsistencyChecks).toHaveBeenCalledTimes(2)); + }); + + it('offers "Fix all" directly on the check card, without needing to expand it', async () => { + const service = serviceWith(); + const ue = userEvent.setup(); + renderWithProviders(, { mainContext: { service } }); + + await screen.findByText('Extension active flag'); + expect(service.admin.getConsistencyFindings).not.toHaveBeenCalled(); + + await ue.click(screen.getByRole('button', { name: 'Fix all' })); + + expect(service.admin.fixConsistencyFindings).toHaveBeenCalledWith('extension-active-flag'); + await waitFor(() => expect(service.admin.getConsistencyChecks).toHaveBeenCalledTimes(2)); + // Clicking "Fix all" must not also expand the card. + expect(service.admin.getConsistencyFindings).not.toHaveBeenCalled(); + }); + + it('refreshes the overview on demand without triggering any server action', async () => { + const service = serviceWith(); + const ue = userEvent.setup(); + renderWithProviders(, { mainContext: { service } }); + + await screen.findByText('Extension active flag'); + await ue.click(screen.getByRole('button', { name: 'Refresh' })); + + await waitFor(() => expect(service.admin.getConsistencyChecks).toHaveBeenCalledTimes(2)); + }); +});