From 5f5f7377685261cbaa9735fe9e55d3abd13ac4af Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 13:24:39 +0200 Subject: [PATCH 01/17] change: make extension version immutable, add purge mechanism, overhaul namespace ui --- .gitignore | 1 + server/config/eclipse-java-formatter.xml | 4 + .../org/eclipse/openvsx/ExtensionService.java | 234 ++++++++++++++---- .../eclipse/openvsx/LocalRegistryService.java | 15 +- .../java/org/eclipse/openvsx/UserAPI.java | 22 +- .../org/eclipse/openvsx/admin/AdminAPI.java | 96 ++++++- .../eclipse/openvsx/admin/AdminService.java | 25 +- .../openvsx/entities/ExtensionVersion.java | 48 ++++ .../eclipse/openvsx/json/ExtensionJson.java | 13 + .../eclipse/openvsx/json/NamespaceJson.java | 18 -- .../json/TargetPlatformActiveJson.java | 6 +- .../FixTargetPlatformsJobRequestHandler.java | 4 +- .../mirror/DataMirrorJobRequestHandler.java | 2 +- .../openvsx/mirror/DataMirrorService.java | 4 +- .../PublishExtensionVersionHandler.java | 10 +- .../repositories/ExtensionJooqRepository.java | 8 + .../ExtensionVersionJooqRepository.java | 49 +++- .../repositories/RepositoryService.java | 4 + .../org/eclipse/openvsx/jooq/Keys.java | 1 + .../openvsx/jooq/tables/ExtensionVersion.java | 17 +- .../records/ExtensionVersionRecord.java | 47 +++- .../V1_70__ExtensionVersion_Removed.sql | 12 + .../eclipse/openvsx/ExtensionDeleteTest.java | 21 +- .../user => components}/add-user-dialog.tsx | 4 +- .../extension/extension-card-list-item.tsx} | 35 ++- .../extension/extension-card-list.tsx} | 16 +- .../extension-delete-all-versions-dialog.tsx | 27 +- .../extension/extension-detail-view.tsx | 51 +++- .../extension/extension-status-chips.tsx | 1 + .../extension-version-delete-dialog.tsx | 29 ++- .../extension/extension-version-table.tsx | 88 +++++-- .../add-namespace-member-dialog.tsx | 4 +- .../namespace/namespace-detail-view.tsx | 141 +++++++++++ .../namespace/namespace-details.tsx} | 14 +- .../namespace/namespace-extension-list.tsx | 108 ++++++++ .../namespace/namespace-member-component.tsx} | 4 +- .../namespace/namespace-member-list.tsx} | 8 +- webui/src/extension-registry-service.ts | 35 +++ webui/src/extension-registry-types.ts | 4 + .../customers/customer-member-list.tsx | 2 +- .../pages/admin-dashboard/extension-admin.tsx | 10 +- .../pages/admin-dashboard/namespace-admin.tsx | 52 +++- .../admin-dashboard/publisher-details.tsx | 8 +- .../admin-dashboard/use-extension-admin.ts | 11 + .../user/user-namespace-extension-list.tsx | 76 ------ .../pages/user/user-settings-extensions.tsx | 10 +- .../user/user-settings-namespace-detail.tsx | 181 -------------- .../pages/user/user-settings-namespaces.tsx | 6 +- 48 files changed, 1147 insertions(+), 439 deletions(-) create mode 100644 server/src/main/resources/db/migration/V1_70__ExtensionVersion_Removed.sql rename webui/src/{pages/user => components}/add-user-dialog.tsx (97%) rename webui/src/{pages/user/user-namespace-extension-list-item.tsx => components/extension/extension-card-list-item.tsx} (74%) rename webui/src/{pages/user/user-extension-list.tsx => components/extension/extension-card-list.tsx} (73%) rename webui/src/{pages/user => components/namespace}/add-namespace-member-dialog.tsx (94%) create mode 100644 webui/src/components/namespace/namespace-detail-view.tsx rename webui/src/{pages/user/user-namespace-details.tsx => components/namespace/namespace-details.tsx} (98%) create mode 100644 webui/src/components/namespace/namespace-extension-list.tsx rename webui/src/{pages/user/user-namespace-member-component.tsx => components/namespace/namespace-member-component.tsx} (96%) rename webui/src/{pages/user/user-namespace-member-list.tsx => components/namespace/namespace-member-list.tsx} (94%) delete mode 100644 webui/src/pages/user/user-namespace-extension-list.tsx delete mode 100644 webui/src/pages/user/user-settings-namespace-detail.tsx diff --git a/.gitignore b/.gitignore index 298836542..39b97e884 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .idea/ .project/ .settings/ +.claude/ data/ diff --git a/server/config/eclipse-java-formatter.xml b/server/config/eclipse-java-formatter.xml index 67c1f0741..47a759610 100644 --- a/server/config/eclipse-java-formatter.xml +++ b/server/config/eclipse-java-formatter.xml @@ -84,5 +84,9 @@ this (49); parameters default to "never split" (0), so only that one needs overriding. --> + + + + diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index 0d29e1935..bf752d6b5 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -242,6 +242,11 @@ public void reactivateExtensions(UserData user) { } private boolean canBeReactivated(ExtensionVersion extVersion) { + // A soft-deleted version is a permanent tombstone and must never be reactivated. + if (extVersion.isRemoved()) { + return false; + } + var scan = repositories.findLatestExtensionScan(extVersion); // if no scan could be found, scanning is disabled, so allow reactivation if (scan == null) { @@ -307,33 +312,71 @@ public ResultJson deleteExtension( return deleteExtensionVersions( user, - Arrays.stream(targetVersions) - .map(target -> { - var extVersion = restrictedToUser - ? repositories.findVersionPublishedWithUser( - user, - target.version(), - target.targetPlatform(), - extensionName, - namespaceName) - : repositories.findVersion( - target.version(), - target.targetPlatform(), - extensionName, - namespaceName); - - if (extVersion == null) { - throw new ErrorResultException( - "Extension not found: " + NamingUtil.toLogFormat( - namespaceName, - extensionName, - target.targetPlatform(), - target.version()), - HttpStatus.NOT_FOUND); - } - return extVersion; - }) - .toList()); + resolveVersions(user, restrictedToUser, namespaceName, extensionName, targetVersions)); + } + + /** + * Purges (permanently deletes) the given extension or extension versions from the database and storage. + *

+ * Unlike {@link #deleteExtension(UserData, boolean, String, String, TargetPlatformVersion...)}, which + * soft-deletes versions (keeping the row so the version identity stays reserved), this physically removes + * the rows and frees the version identity for republishing. It is intended for administrative purge and + * automated cleanup (mirror, extension control) and performs no user-ownership check beyond the optional + * {@code restrictedToUser} version lookup. + */ + @Transactional(rollbackOn = ErrorResultException.class) + public ResultJson purgeExtension( + UserData user, + boolean restrictedToUser, + String namespaceName, + String extensionName, + TargetPlatformVersion... targetVersions + ) throws ErrorResultException { + var extension = lockExtensionNoWait(namespaceName, extensionName); + if (repositories + .isDeleteAllVersions(restrictedToUser ? user : null, namespaceName, extensionName, targetVersions)) { + return purgeExtension(user, extension, true); + } + + return purgeExtensionVersions( + user, + resolveVersions(user, restrictedToUser, namespaceName, extensionName, targetVersions)); + } + + private List resolveVersions( + UserData user, + boolean restrictedToUser, + String namespaceName, + String extensionName, + TargetPlatformVersion... targetVersions + ) { + return Arrays.stream(targetVersions) + .map(target -> { + var extVersion = restrictedToUser + ? repositories.findVersionPublishedWithUser( + user, + target.version(), + target.targetPlatform(), + extensionName, + namespaceName) + : repositories.findVersion( + target.version(), + target.targetPlatform(), + extensionName, + namespaceName); + + if (extVersion == null) { + throw new ErrorResultException( + "Extension not found: " + NamingUtil.toLogFormat( + namespaceName, + extensionName, + target.targetPlatform(), + target.version()), + HttpStatus.NOT_FOUND); + } + return extVersion; + }) + .toList(); } /** @@ -350,6 +393,19 @@ public ResultJson deleteExtensionVersions(UserData user, List return combineResults(results); } + /** + * Purges (permanently deletes) the given pre-resolved extension versions without any ownership check. + * Callers are responsible for authorisation. + */ + @Transactional(rollbackOn = ErrorResultException.class) + public ResultJson purgeExtensionVersions(UserData user, List versions) { + var results = new ArrayList(); + for (var version : versions) { + results.add(purgeExtensionVersion(user, version)); + } + return combineResults(results); + } + /** * Locks and return the {@code Extension} identified by {@code namespaceName} and {@code extensionName}. * @@ -402,38 +458,88 @@ private ResultJson combineResults(List results) { } /** - * Delete the given extension and evict caches. + * Soft-delete the given extension: mark all its versions as removed and hide the extension. + *

+ * The extension and version rows are kept so their identities stay reserved and can never be + * republished; only the version files are stripped from storage. Use + * {@link #purgeExtension(UserData, Extension, boolean)} to physically remove the extension. *

* If {@code checkDependencies} is {@code true} and this extension is referenced by a bundle or used - * as a dependency, the delete operation will fail. + * as a dependency, the operation will fail. * * @param user the user that will be used for logging the operation - * @param extension the extension to delete - * @param checkDependencies whether to check if this extension is still references by bundles or as a dependency + * @param extension the extension to soft-delete + * @param checkDependencies whether to check if this extension is still referenced by bundles or as a dependency */ @Transactional(rollbackOn = ErrorResultException.class) public ResultJson deleteExtension(UserData user, Extension extension, boolean checkDependencies) throws ErrorResultException { if (checkDependencies) { - var bundledRefs = repositories.findBundledExtensionsReference(extension); - if (!bundledRefs.isEmpty()) { - throw new ErrorResultException( - "Extension " + NamingUtil.toExtensionId(extension) - + " is bundled by the following extension packs: " - + bundledRefs.stream() - .map(NamingUtil::toFileFormat) - .collect(Collectors.joining(", "))); - } - var dependRefs = repositories.findDependenciesReference(extension); - if (!dependRefs.isEmpty()) { - throw new ErrorResultException( - "The following extensions have a dependency on " + NamingUtil.toExtensionId(extension) + ": " - + dependRefs.stream() - .map(NamingUtil::toFileFormat) - .collect(Collectors.joining(", "))); + checkNoDependencies(extension); + } + + for (var extVersion : repositories.findVersions(extension)) { + if (!extVersion.isRemoved()) { + softDeleteExtensionVersion(user, extVersion); } } + updateExtension(extension); + + var result = ResultJson.success("Deleted " + NamingUtil.toExtensionId(extension)); + logs.logAction(user, result); + return result; + } + + /** + * Soft-delete a single extension version: strip its files from storage and mark it as removed, + * but keep the row so the version identity stays reserved. Does not touch the parent extension; + * callers are responsible for calling {@link #updateExtension(Extension)} afterwards. + */ + private void softDeleteExtensionVersion(UserData user, ExtensionVersion extVersion) { + deleteFiles(extVersion); + extVersion.setActive(false); + extVersion.setRemoved(true); + extVersion.setRemovedTimestamp(TimeUtil.getCurrentUTC()); + extVersion.setRemovedBy(user); + } + + @Transactional(rollbackOn = ErrorResultException.class) + public ResultJson deleteExtensionVersion(UserData user, ExtensionVersion extVersion) { + if (extVersion.isRemoved()) { + return ResultJson.success("Already removed " + NamingUtil.toLogFormat(extVersion)); + } + + var extension = extVersion.getExtension(); + softDeleteExtensionVersion(user, extVersion); + updateExtension(extension); + + var result = ResultJson.success("Deleted " + NamingUtil.toLogFormat(extVersion)); + logs.logAction(user, result); + return result; + } + + /** + * Purge (permanently delete) the given extension and evict caches. + *

+ * This physically removes the extension and all its versions from the database and storage, freeing + * the extension and version identities for republishing. Intended for administrative purge and automated + * cleanup (mirror, extension control). + *

+ * If {@code checkDependencies} is {@code true} and this extension is referenced by a bundle or used + * as a dependency, the operation will fail. + * + * @param user the user that will be used for logging the operation + * @param extension the extension to purge + * @param checkDependencies whether to check if this extension is still referenced by bundles or as a dependency + */ + @Transactional(rollbackOn = ErrorResultException.class) + public ResultJson purgeExtension(UserData user, Extension extension, boolean checkDependencies) + throws ErrorResultException { + if (checkDependencies) { + checkNoDependencies(extension); + } + for (var extVersion : repositories.findVersions(extension)) { removeExtensionVersion(extVersion); } @@ -463,7 +569,7 @@ public ResultJson deleteExtension(UserData user, Extension extension, boolean ch } @Transactional(rollbackOn = ErrorResultException.class) - public ResultJson deleteExtensionVersion(UserData user, ExtensionVersion extVersion) { + public ResultJson purgeExtensionVersion(UserData user, ExtensionVersion extVersion) { var extension = extVersion.getExtension(); removeExtensionVersion(extVersion); extension.getVersions().remove(extVersion); @@ -474,14 +580,42 @@ public ResultJson deleteExtensionVersion(UserData user, ExtensionVersion extVers return result; } - @Transactional(rollbackOn = ErrorResultException.class) - public void removeExtensionVersion(ExtensionVersion extVersion) { + private void checkNoDependencies(Extension extension) throws ErrorResultException { + var bundledRefs = repositories.findBundledExtensionsReference(extension); + if (!bundledRefs.isEmpty()) { + throw new ErrorResultException( + "Extension " + NamingUtil.toExtensionId(extension) + + " is bundled by the following extension packs: " + + bundledRefs.stream() + .map(NamingUtil::toFileFormat) + .collect(Collectors.joining(", "))); + } + var dependRefs = repositories.findDependenciesReference(extension); + if (!dependRefs.isEmpty()) { + throw new ErrorResultException( + "The following extensions have a dependency on " + NamingUtil.toExtensionId(extension) + ": " + + dependRefs.stream() + .map(NamingUtil::toFileFormat) + .collect(Collectors.joining(", "))); + } + } + + private void deleteFiles(ExtensionVersion extVersion) { // Clean up any pending scan jobs for this extension version // to prevent "file not found" errors after deletion scanPersistenceService.deleteScansForExtensionVersion(extVersion.getId()); repositories.findFiles(extVersion).map(RemoveFileJobRequest::new).forEach(scheduler::enqueue); repositories.deleteFiles(extVersion); + } + + /** + * Physically remove an extension version: strip its files from storage and delete the row. + * This is the low-level hard-delete primitive used by the purge paths. + */ + @Transactional(rollbackOn = ErrorResultException.class) + public void removeExtensionVersion(ExtensionVersion extVersion) { + deleteFiles(extVersion); entityManager.remove(extVersion); } } diff --git a/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java b/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java index 29299cc73..c7918fea4 100644 --- a/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java +++ b/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java @@ -119,6 +119,15 @@ public LocalRegistryService( @Override public NamespaceJson getNamespace(String namespaceName) { + return getNamespace(namespaceName, false); + } + + /** + * Build the namespace JSON. When {@code includeInactive} is {@code true}, all extensions of the + * namespace are listed, including inactive/soft-deleted ones; otherwise only active extensions are + * listed. Inactive extensions must only be exposed on admin surfaces. + */ + public NamespaceJson getNamespace(String namespaceName, boolean includeInactive) { var namespace = repositories.findNamespace(namespaceName); if (namespace == null) { throw new NotFoundException(); @@ -127,13 +136,15 @@ public NamespaceJson getNamespace(String namespaceName) { json.setName(namespace.getName()); var extensionsMap = new LinkedHashMap(); var serverUrl = UrlUtil.getBaseUrl(); - for (var name : repositories.findActiveExtensionNames(namespace)) { + var extensionNames = includeInactive + ? repositories.findAllExtensionNames(namespace) + : repositories.findActiveExtensionNames(namespace); + for (var name : extensionNames) { String url = createApiUrl(serverUrl, "api", namespace.getName(), name); extensionsMap.put(name, url); } json.setExtensions(extensionsMap); json.setVerified(repositories.hasMemberships(namespace, NamespaceMembership.ROLE_OWNER)); - json.setAccess(RESTRICTED_ACCESS); return json; } diff --git a/server/src/main/java/org/eclipse/openvsx/UserAPI.java b/server/src/main/java/org/eclipse/openvsx/UserAPI.java index d958e4ed5..47f1cd538 100644 --- a/server/src/main/java/org/eclipse/openvsx/UserAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/UserAPI.java @@ -146,9 +146,9 @@ public ErrorJson getAuthError(HttpServletRequest request) { } /** - * This endpoint is used to check whether there is a logged-in user. For this reason, it - * does not return a 403 status, but an OK status with JSON body when no user data is - * available. This is to avoid unnecessary network error logging in the browser console. + * This endpoint is used to check whether there is a logged-in user. For this reason, it does not return a 403 + * status, but an OK status with JSON body when no user data is available. This is to avoid unnecessary network + * error logging in the browser console. */ @GetMapping( path = "/user", @@ -255,6 +255,7 @@ public List getOwnExtensions() { var json = latest.toExtensionJson(); json.setPreview(latest.isPreview()); json.setActive(latest.getExtension().isActive()); + json.setRemoved(latest.isRemoved()); json.setFiles(fileUrls.get(latest.getId())); // Add scan/review status information @@ -269,9 +270,11 @@ public List getOwnExtensions() { * Add review/scan status information to the extension JSON. *

* This shows users the current state of their extension in simple terms: - * - "published" - Extension is active and publicly available - * - "under_review" - Extension is being reviewed (validation, scanning, etc.) - * - "rejected" - Extension was blocked (quarantined or rejected) + *

    + *
  • "published" - Extension is active and publicly available
    • + *
  • "under_review" - Extension is being reviewed (validation, scanning, etc.)
    • + *
  • "rejected" - Extension was blocked (quarantined or rejected)
    • + *
*/ private void enrichWithReviewStatus(ExtensionJson json, ExtensionVersion extVersion) { // Look up scan by extension metadata (namespace, name, version, platform) @@ -284,13 +287,18 @@ private void enrichWithReviewStatus(ExtensionJson json, ExtensionVersion extVers extVersion.getTargetPlatform()); if (Boolean.TRUE.equals(json.getActive())) { - // Only mark published if scan result indicates PASSED or no scan result exists (scanning disabled / manual activation) + // Only mark published if scan result indicates PASSED or no scan result exists (scanning disabled / manual + // activation) if (scanResult == null || scanResult.getStatus() == ScanStatus.PASSED) { json.setReviewStatus("published"); return; } } + if (extVersion.isRemoved()) { + return; + } + if (scanResult == null) { // No scan result found - show as under review json.setReviewStatus("under_review"); diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java index 48cb30a2e..fa383c5a0 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java @@ -389,6 +389,7 @@ public ResponseEntity getExtension( json.setAllTargetPlatformVersions( repositories.findTargetPlatformsGroupedByVersion(latest.getExtension())); json.setActive(latest.getExtension().isActive()); + json.setRemoved(latest.isRemoved()); } else { var extension = repositories.findExtension(extensionName, namespaceName); if (extension == null) { @@ -498,6 +499,98 @@ public ResponseEntity deleteExtension( } } + @PostMapping( + path = "/api/extension/{namespaceName}/{extensionName}/purge", + produces = MediaType.APPLICATION_JSON_VALUE + ) + @CrossOrigin + @Operation( + summary = "Permanently purge an extension or one or multiple extension versions", + description = "Unlike delete, this physically removes the versions from the database and storage, " + + "freeing their identities for republishing. Extension versions are otherwise immutable." + ) + @MutatingOperation + @ApiResponse( + responseCode = "200", + description = "A success message is returned in JSON format", + content = @Content(schema = @Schema(implementation = ResultJson.class)) + ) + @ApiResponse( + responseCode = "400", + description = "An error message is returned in JSON format", + content = @Content(schema = @Schema(implementation = ResultJson.class)) + ) + @ApiResponse( + responseCode = "404", + description = "Extension not found", + content = @Content() + ) + public ResponseEntity purgeExtension( + @PathVariable + @Parameter(description = "Namespace name", example = "julialang") String namespaceName, + @PathVariable + @Parameter(description = "Extension name", example = "language-julia") String extensionName, + @RequestParam(value = "token") + @Parameter(description = "A personal access token") String tokenValue, + @RequestBody(required = false) List targetVersions + ) { + try { + var adminUser = admins.checkAdminUser(tokenValue); + var targets = CollectionUtil.toArray( + targetVersions, + TargetPlatformVersionJson::toTargetPlatformVersion, + TargetPlatformVersion[]::new); + var result = admins.purgeExtensionNoWait(adminUser, namespaceName, extensionName, targets); + return ResponseEntity.ok(result); + } catch (ErrorResultException exc) { + return exc.toResponseEntity(); + } + } + + @PostMapping( + path = "/extension/{namespaceName}/{extensionName}/purge", + produces = MediaType.APPLICATION_JSON_VALUE + ) + @Operation(hidden = true, summary = "Permanently purge an extension or one or multiple extension versions") + @MutatingOperation + @ApiResponse( + responseCode = "200", + description = "A success message is returned in JSON format", + content = @Content( + mediaType = MediaType.APPLICATION_JSON_VALUE, + schema = @Schema(implementation = ResultJson.class) + ) + ) + @ApiResponse( + responseCode = "400", + description = "An error message is returned in JSON format", + content = @Content(schema = @Schema(implementation = ResultJson.class)) + ) + @ApiResponse( + responseCode = "404", + description = "Extension not found", + content = @Content() + ) + public ResponseEntity purgeExtension( + @PathVariable + @Parameter(description = "Namespace name", example = "julialang") String namespaceName, + @PathVariable + @Parameter(description = "Extension name", example = "language-julia") String extensionName, + @RequestBody List targetVersions + ) { + try { + var adminUser = admins.checkAdminUser(); + var targets = CollectionUtil.toArray( + targetVersions, + TargetPlatformVersionJson::toTargetPlatformVersion, + TargetPlatformVersion[]::new); + var result = admins.purgeExtensionNoWait(adminUser, namespaceName, extensionName, targets); + return ResponseEntity.ok(result); + } catch (ErrorResultException exc) { + return exc.toResponseEntity(); + } + } + @PostMapping( path = "/extension/{namespace}/{extension}/review/{provider}/{loginName}/delete", produces = MediaType.APPLICATION_JSON_VALUE @@ -602,7 +695,8 @@ public ResponseEntity getNamespace( try { admins.checkAdminUser(); - var namespace = local.getNamespace(namespaceName); + // Admins see all extensions of the namespace, including inactive/soft-deleted ones. + var namespace = local.getNamespace(namespaceName, true); var adminNamespaceUrl = createAdminNamespaceUrl(namespace); namespace.setMembersUrl(UrlUtil.createApiUrl(adminNamespaceUrl, "members")); namespace.setRoleUrl(UrlUtil.createApiUrl(adminNamespaceUrl, "change-member")); diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 825ed52c2..336d3147c 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -145,9 +145,9 @@ private void deleteExtensionAndDependencies(UserData admin, Extension extension, deleteExtensionAndDependencies(admin, dependRef, depth); } - // We unconditionally delete the extension, + // We unconditionally purge the extension, // not checking if there are dependencies on this extension. - extensions.deleteExtension(admin, extension, false); + extensions.purgeExtension(admin, extension, false); } private void deleteExtensionAndDependencies(UserData admin, ExtensionVersion extVersion, int depth) { @@ -173,7 +173,7 @@ private void deleteExtensionAndDependencies(UserData admin, ExtensionVersion ext public void deleteExtension(UserData admin, String namespaceName, String extensionName) throws ErrorResultException { var extension = extensions.lockExtension(namespaceName, extensionName); - extensions.deleteExtension(admin, extension, false); + extensions.purgeExtension(admin, extension, false); } /** @@ -194,6 +194,25 @@ public ResultJson deleteExtensionNoWait( return extensions.deleteExtension(user, false, namespaceName, extensionName, targetVersions); } + /** + * Purge (permanently delete) the provided versions of an extension. If all versions shall be purged, the + * extension as a whole will be removed unless it is referenced by bundles or used as a dependency. + *

+ * Unlike {@link #deleteExtensionNoWait}, this physically removes the rows from the database and storage, + * freeing the version identity for republishing. Intended for administrative purge and automated cleanup. + *

+ * The method will try to lock the extension and fail with an {@code ErrorResultException} if it can't acquire it. + */ + @Transactional(rollbackOn = ErrorResultException.class) + public ResultJson purgeExtensionNoWait( + UserData user, + String namespaceName, + String extensionName, + TargetPlatformVersion... targetVersions + ) throws ErrorResultException { + return extensions.purgeExtension(user, false, namespaceName, extensionName, targetVersions); + } + @Transactional(rollbackOn = ErrorResultException.class) public ResultJson deleteNamespace(String namespaceName, UserData admin) throws ErrorResultException { var namespace = repositories.findNamespace(namespaceName); diff --git a/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java b/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java index e7a06315b..c957d5f88 100644 --- a/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java +++ b/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java @@ -87,6 +87,20 @@ public enum Type { private boolean potentiallyMalicious; + /** + * Sticky tombstone marker. A removed version has been soft-deleted: it is hidden + * (also {@code active == false}) and its files have been stripped from storage, but the row is + * kept so its identity stays permanently reserved and can never be republished. Only an admin + * purge physically removes the row. Unlike {@code active}, this flag is never cleared by + * reactivation or scan processing. + */ + private boolean removed; + + private LocalDateTime removedTimestamp; + + @ManyToOne + private UserData removedBy; + private String displayName; @Column(length = 2048) @@ -338,6 +352,30 @@ public void setPotentiallyMalicious(boolean potentiallyMalicious) { this.potentiallyMalicious = potentiallyMalicious; } + public boolean isRemoved() { + return removed; + } + + public void setRemoved(boolean removed) { + this.removed = removed; + } + + public LocalDateTime getRemovedTimestamp() { + return removedTimestamp; + } + + public void setRemovedTimestamp(LocalDateTime removedTimestamp) { + this.removedTimestamp = removedTimestamp; + } + + public UserData getRemovedBy() { + return removedBy; + } + + public void setRemovedBy(UserData removedBy) { + this.removedBy = removedBy; + } + public String getDisplayName() { return displayName; } @@ -512,6 +550,9 @@ public boolean equals(Object o) { && preview == that.preview && active == that.active && potentiallyMalicious == that.potentiallyMalicious + && removed == that.removed + && Objects.equals(removedTimestamp, that.removedTimestamp) + && Objects.equals(getId(removedBy), getId(that.removedBy)) // use id to prevent infinite recursion && Objects.equals(getId(extension), getId(that.extension)) // use id to prevent infinite recursion && Objects.equals(version, that.version) && Objects.equals(targetPlatform, that.targetPlatform) @@ -552,6 +593,9 @@ public int hashCode() { getId(publishedWith), active, potentiallyMalicious, + removed, + removedTimestamp, + getId(removedBy), displayName, description, engines, @@ -581,4 +625,8 @@ private Long getId(Extension extension) { private Long getId(PersonalAccessToken token) { return Optional.ofNullable(token).map(PersonalAccessToken::getId).orElse(null); } + + private Long getId(UserData user) { + return Optional.ofNullable(user).map(UserData::getId).orElse(null); + } } diff --git a/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java b/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java index 684d9a6b2..5420925a5 100644 --- a/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java +++ b/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java @@ -89,6 +89,9 @@ public static ExtensionJson error(String message) { @Schema(hidden = true) private Boolean active; + @Schema(hidden = true) + private Boolean removed; + @Schema( description = "Review/publishing status: published (active and visible to all), under_review (being reviewed), rejected (blocked)", allowableValues = { "published", "under_review", "rejected" } @@ -319,6 +322,14 @@ public void setActive(Boolean active) { this.active = active; } + public Boolean getRemoved() { + return removed; + } + + public void setRemoved(Boolean removed) { + this.removed = removed; + } + public String getReviewStatus() { return reviewStatus; } @@ -650,6 +661,7 @@ public boolean equals(Object o) { && Objects.equals(preRelease, that.preRelease) && Objects.equals(publishedBy, that.publishedBy) && Objects.equals(active, that.active) + && Objects.equals(removed, that.removed) && Objects.equals(reviewStatus, that.reviewStatus) && Objects.equals(reviewMessage, that.reviewMessage) && Objects.equals(verified, that.verified) @@ -701,6 +713,7 @@ public int hashCode() { preRelease, publishedBy, active, + removed, reviewStatus, reviewMessage, verified, diff --git a/server/src/main/java/org/eclipse/openvsx/json/NamespaceJson.java b/server/src/main/java/org/eclipse/openvsx/json/NamespaceJson.java index 8450e3e2b..b1c0f986e 100644 --- a/server/src/main/java/org/eclipse/openvsx/json/NamespaceJson.java +++ b/server/src/main/java/org/eclipse/openvsx/json/NamespaceJson.java @@ -40,16 +40,6 @@ public static NamespaceJson error(String message) { @NotNull private Boolean verified; - /** - * @deprecated - */ - @Schema( - description = "Access level of the namespace. Deprecated: namespaces are now always restricted", - allowableValues = { "public", "restricted" } - ) - @Deprecated - private String access; - @Schema(hidden = true) private String membersUrl; @@ -83,14 +73,6 @@ public void setVerified(Boolean verified) { this.verified = verified; } - public String getAccess() { - return access; - } - - public void setAccess(String access) { - this.access = access; - } - public String getMembersUrl() { return membersUrl; } diff --git a/server/src/main/java/org/eclipse/openvsx/json/TargetPlatformActiveJson.java b/server/src/main/java/org/eclipse/openvsx/json/TargetPlatformActiveJson.java index 52d9ff6c0..7472982af 100644 --- a/server/src/main/java/org/eclipse/openvsx/json/TargetPlatformActiveJson.java +++ b/server/src/main/java/org/eclipse/openvsx/json/TargetPlatformActiveJson.java @@ -20,6 +20,7 @@ * * @param targetPlatform Name of the target platform * @param active Whether this target platform version is active + * @param removed Whether this target platform version has been removed (soft-deleted) */ @Schema( name = "TargetPlatformActive", @@ -43,5 +44,8 @@ public record TargetPlatformActiveJson( NAME_UNIVERSAL } ) String targetPlatform, - @Schema(description = "Whether this extension version for this target platform is active") boolean active + @Schema(description = "Whether this extension version for this target platform is active") boolean active, + @Schema( + description = "Whether this extension version for this target platform has been removed (soft-deleted)" + ) boolean removed ){} diff --git a/server/src/main/java/org/eclipse/openvsx/migration/FixTargetPlatformsJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/migration/FixTargetPlatformsJobRequestHandler.java index d6c1099dd..92652bf45 100644 --- a/server/src/main/java/org/eclipse/openvsx/migration/FixTargetPlatformsJobRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/migration/FixTargetPlatformsJobRequestHandler.java @@ -66,7 +66,9 @@ public void run(MigrationJobRequest jobRequest) throws Exception { .addArgument(() -> NamingUtil.toLogFormat(extVersion)) .log(); - extensions.deleteExtensionVersion(service.getUser(), extVersion); + // Purge (hard delete) rather than soft-delete: the version is immediately republished + // below with the corrected target platform, so its identity must be freed, not reserved. + extensions.purgeExtensionVersion(service.getUser(), extVersion); try (var input = Files.newInputStream(extensionFile.getPath())) { extensions.publishVersion(input, extVersion.getPublishedWith()); } diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java index 9eabd8b14..0e0972872 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java @@ -139,7 +139,7 @@ private void deleteOtherExtensions(List extensionIds, UserData mirrorUse ThreadLocalJobContext.getJobContext().logger().info("deleting " + extensionId); try { var namespace = extension.getNamespace(); - admin.deleteExtensionNoWait(mirrorUser, namespace.getName(), extension.getName()); + admin.purgeExtensionNoWait(mirrorUser, namespace.getName(), extension.getName()); } catch (ErrorResultException e) { if (e.getStatus() != HttpStatus.NOT_FOUND) { logger.warn("mirror: failed to delete extension {}", extensionId, e); diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java index 87bb36532..804dfb0bf 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java @@ -245,7 +245,9 @@ private void addReview(ReviewJson json, Extension extension) { public void deleteExtensionVersion(ExtensionVersion extVersion, UserData user) { var extension = extVersion.getExtension(); - admin.deleteExtensionNoWait( + // The mirror must not keep tombstones: versions dropped upstream are purged so the mirror can + // re-track upstream state (and re-publish a version that later reappears upstream). + admin.purgeExtensionNoWait( user, extension.getNamespace().getName(), extension.getName(), diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java index 231c156f6..2e6653b29 100644 --- a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java @@ -190,7 +190,15 @@ private ExtensionVersion createExtensionVersion( extVersion.getTargetPlatform(), extVersion.getVersion()); var message = "Extension " + extVersionId + " is already published"; - message += existingVersion.isActive() ? "." : ", but currently isn't active and therefore not visible."; + if (existingVersion.isRemoved()) { + message += " and was removed. Extension versions are immutable, so this version's identity" + + " stays permanently reserved and cannot be republished." + + " Ask an administrator to purge it if it must be republished."; + } else { + message += existingVersion.isActive() + ? "." + : ", but currently isn't active and therefore not visible."; + } throw new ErrorResultException(message); } } diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java index ed519bb71..bcc8fa1f9 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java @@ -228,6 +228,14 @@ public List findActiveExtensionNames(Namespace namespace) { .fetch(EXTENSION.NAME); } + public List findAllExtensionNames(Namespace namespace) { + return dsl.select(EXTENSION.NAME) + .from(EXTENSION) + .where(EXTENSION.NAMESPACE_ID.eq(namespace.getId())) + .orderBy(EXTENSION.NAME.asc()) + .fetch(EXTENSION.NAME); + } + public String findFirstUnresolvedDependency(List dependencies) { if (dependencies.isEmpty()) { return null; diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java index 08a51b5e9..4720bbe35 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java @@ -54,6 +54,7 @@ public List findAllActiveByExtensionIdAndTargetPlatform( EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -408,6 +409,7 @@ private SelectQuery findAllActive() { EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -541,6 +543,14 @@ private ExtensionVersion toExtensionVersionCommon( extVersion .setPotentiallyMalicious(row.get(extensionVersionMapper.map(EXTENSION_VERSION.POTENTIALLY_MALICIOUS))); + // The `removed` column is only selected by queries that may include non-active versions; when it + // is present, carry it through so callers observe the correct tombstone state. (Active versions + // are never removed, so leaving it false for active-only queries is correct.) + var removedField = extensionVersionMapper.map(EXTENSION_VERSION.REMOVED); + if (row.field(removedField) != null) { + extVersion.setRemoved(row.get(removedField)); + } + if (extension == null) { var namespace = new Namespace(); namespace.setId(row.get(NAMESPACE.ID)); @@ -573,6 +583,10 @@ public List findTargetPlatformsGroupedByVersion(Exte .orderBy( EXTENSION_VERSION.UNIVERSAL_TARGET_PLATFORM.desc(), EXTENSION_VERSION.TARGET_PLATFORM.asc()); + var targetPlatformsRemoved = DSL.arrayAgg(EXTENSION_VERSION.REMOVED) + .orderBy( + EXTENSION_VERSION.UNIVERSAL_TARGET_PLATFORM.desc(), + EXTENSION_VERSION.TARGET_PLATFORM.asc()); return dsl.select( EXTENSION_VERSION.SEMVER_MAJOR, @@ -581,7 +595,8 @@ public List findTargetPlatformsGroupedByVersion(Exte EXTENSION_VERSION.SEMVER_IS_PRE_RELEASE, EXTENSION_VERSION.VERSION, targetPlatforms, - targetPlatformsActive) + targetPlatformsActive, + targetPlatformsRemoved) .from(EXTENSION_VERSION) .where(EXTENSION_VERSION.EXTENSION_ID.eq(extension.getId())) .groupBy( @@ -601,7 +616,8 @@ public List findTargetPlatformsGroupedByVersion(Exte row -> toVersionTargetPlatformsJson( row.get(EXTENSION_VERSION.VERSION), row.get(targetPlatforms), - row.get(targetPlatformsActive))); + row.get(targetPlatformsActive), + row.get(targetPlatformsRemoved))); } public List findTargetPlatformsGroupedByVersion(Extension extension, UserData user) { @@ -613,6 +629,10 @@ public List findTargetPlatformsGroupedByVersion(Exte .orderBy( EXTENSION_VERSION.UNIVERSAL_TARGET_PLATFORM.desc(), EXTENSION_VERSION.TARGET_PLATFORM.asc()); + var targetPlatformsRemoved = DSL.arrayAgg(EXTENSION_VERSION.REMOVED) + .orderBy( + EXTENSION_VERSION.UNIVERSAL_TARGET_PLATFORM.desc(), + EXTENSION_VERSION.TARGET_PLATFORM.asc()); return dsl.select( EXTENSION_VERSION.SEMVER_MAJOR, @@ -621,7 +641,8 @@ public List findTargetPlatformsGroupedByVersion(Exte EXTENSION_VERSION.SEMVER_IS_PRE_RELEASE, EXTENSION_VERSION.VERSION, targetPlatforms, - targetPlatformsActive) + targetPlatformsActive, + targetPlatformsRemoved) .from(EXTENSION_VERSION) .join(PERSONAL_ACCESS_TOKEN).on(PERSONAL_ACCESS_TOKEN.ID.eq(EXTENSION_VERSION.PUBLISHED_WITH_ID)) .where(EXTENSION_VERSION.EXTENSION_ID.eq(extension.getId())) @@ -643,17 +664,23 @@ public List findTargetPlatformsGroupedByVersion(Exte row -> toVersionTargetPlatformsJson( row.get(EXTENSION_VERSION.VERSION), row.get(targetPlatforms), - row.get(targetPlatformsActive))); + row.get(targetPlatformsActive), + row.get(targetPlatformsRemoved))); } private VersionTargetPlatformsJson toVersionTargetPlatformsJson( String version, String[] targetPlatforms, - Boolean[] active + Boolean[] active, + Boolean[] removed ) { var platforms = new ArrayList(targetPlatforms.length); for (int i = 0; i < targetPlatforms.length; i++) { - platforms.add(new TargetPlatformActiveJson(targetPlatforms[i], active[i])); + platforms.add( + new TargetPlatformActiveJson( + targetPlatforms[i], + Boolean.TRUE.equals(active[i]), + Boolean.TRUE.equals(removed[i]))); } return new VersionTargetPlatformsJson(version, platforms); @@ -738,6 +765,7 @@ public ExtensionVersion findLatest( EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -807,6 +835,7 @@ public ExtensionVersion findLatest( EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -881,6 +910,7 @@ public List findLatest(Collection extensionIds) { EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -924,6 +954,7 @@ public List findLatest(Collection extensionIds) { EXTENSION.DEPRECATED, latest.field(EXTENSION_VERSION.ID), latest.field(EXTENSION_VERSION.POTENTIALLY_MALICIOUS), + latest.field(EXTENSION_VERSION.REMOVED), latest.field(EXTENSION_VERSION.VERSION), latest.field(EXTENSION_VERSION.TARGET_PLATFORM), latest.field(EXTENSION_VERSION.PREVIEW), @@ -1046,6 +1077,7 @@ public List findLatest(UserData user) { EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -1092,6 +1124,7 @@ public List findLatest(UserData user) { EXTENSION.DOWNLOADABLE, latest.field(EXTENSION_VERSION.ID), latest.field(EXTENSION_VERSION.POTENTIALLY_MALICIOUS), + latest.field(EXTENSION_VERSION.REMOVED), latest.field(EXTENSION_VERSION.VERSION), latest.field(EXTENSION_VERSION.TARGET_PLATFORM), latest.field(EXTENSION_VERSION.PREVIEW), @@ -1152,6 +1185,7 @@ public ExtensionVersion findLatest(UserData user, String namespace, String exten EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -1198,6 +1232,7 @@ public ExtensionVersion findLatest(UserData user, String namespace, String exten EXTENSION.DOWNLOADABLE, latest.field(EXTENSION_VERSION.ID), latest.field(EXTENSION_VERSION.POTENTIALLY_MALICIOUS), + latest.field(EXTENSION_VERSION.REMOVED), latest.field(EXTENSION_VERSION.VERSION), latest.field(EXTENSION_VERSION.TARGET_PLATFORM), latest.field(EXTENSION_VERSION.PREVIEW), @@ -1294,6 +1329,7 @@ public List findLatestVersionByTargetPlatform( EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, @@ -1425,6 +1461,7 @@ private ExtensionVersion findInternal( EXTENSION_VERSION.ID, EXTENSION_VERSION.VERSION, EXTENSION_VERSION.POTENTIALLY_MALICIOUS, + EXTENSION_VERSION.REMOVED, EXTENSION_VERSION.TARGET_PLATFORM, EXTENSION_VERSION.PREVIEW, EXTENSION_VERSION.PRE_RELEASE, 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 027c8bfb3..2d1187171 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java @@ -822,6 +822,10 @@ public List findActiveExtensionNames(Namespace namespace) { return extensionJooqRepo.findActiveExtensionNames(namespace); } + public List findAllExtensionNames(Namespace namespace) { + return extensionJooqRepo.findAllExtensionNames(namespace); + } + public List findMembershipsForOwner(UserData user, String namespaceName) { return membershipJooqRepo.findMembershipsForOwner(user, namespaceName); } diff --git a/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/Keys.java b/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/Keys.java index be3b66fc5..d0cbe6084 100644 --- a/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/Keys.java +++ b/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/Keys.java @@ -164,6 +164,7 @@ public class Keys { public static final ForeignKey EXTENSION_REVIEW__FKINJBN9GRK135Y6IK0UT4UJP0W = Internal.createForeignKey(ExtensionReview.EXTENSION_REVIEW, DSL.name("fkinjbn9grk135y6ik0ut4ujp0w"), new TableField[] { ExtensionReview.EXTENSION_REVIEW.USER_ID }, Keys.USER_DATA_PKEY, new TableField[] { UserData.USER_DATA.ID }, true); public static final ForeignKey EXTENSION_THREAT__FK_THREAT_SCAN = Internal.createForeignKey(ExtensionThreat.EXTENSION_THREAT, DSL.name("fk_threat_scan"), new TableField[] { ExtensionThreat.EXTENSION_THREAT.SCAN_ID }, Keys.EXTENSION_SCAN_PKEY, new TableField[] { ExtensionScan.EXTENSION_SCAN.ID }, true); public static final ForeignKey EXTENSION_VALIDATION_FAILURE__FK_VALIDATION_FAILURE_SCAN = Internal.createForeignKey(ExtensionValidationFailure.EXTENSION_VALIDATION_FAILURE, DSL.name("fk_validation_failure_scan"), new TableField[] { ExtensionValidationFailure.EXTENSION_VALIDATION_FAILURE.SCAN_ID }, Keys.EXTENSION_SCAN_PKEY, new TableField[] { ExtensionScan.EXTENSION_SCAN.ID }, true); + public static final ForeignKey EXTENSION_VERSION__EXTENSION_VERSION_REMOVED_BY_ID_FKEY = Internal.createForeignKey(ExtensionVersion.EXTENSION_VERSION, DSL.name("extension_version_removed_by_id_fkey"), new TableField[] { ExtensionVersion.EXTENSION_VERSION.REMOVED_BY_ID }, Keys.USER_DATA_PKEY, new TableField[] { UserData.USER_DATA.ID }, true); public static final ForeignKey EXTENSION_VERSION__EXTENSION_VERSION_SIGNATURE_KEY_PAIR_FKEY = Internal.createForeignKey(ExtensionVersion.EXTENSION_VERSION, DSL.name("extension_version_signature_key_pair_fkey"), new TableField[] { ExtensionVersion.EXTENSION_VERSION.SIGNATURE_KEY_PAIR_ID }, Keys.SIGNATURE_KEY_PAIR_PKEY, new TableField[] { SignatureKeyPair.SIGNATURE_KEY_PAIR.ID }, true); public static final ForeignKey EXTENSION_VERSION__FK70KHJ8PM0VACASUIIAQ0W0R80 = Internal.createForeignKey(ExtensionVersion.EXTENSION_VERSION, DSL.name("fk70khj8pm0vacasuiiaq0w0r80"), new TableField[] { ExtensionVersion.EXTENSION_VERSION.PUBLISHED_WITH_ID }, Keys.PERSONAL_ACCESS_TOKEN_PKEY, new TableField[] { PersonalAccessToken.PERSONAL_ACCESS_TOKEN.ID }, true); public static final ForeignKey EXTENSION_VERSION__FKKHS1EC9S9J08FGICQ9PMWU6BT = Internal.createForeignKey(ExtensionVersion.EXTENSION_VERSION, DSL.name("fkkhs1ec9s9j08fgicq9pmwu6bt"), new TableField[] { ExtensionVersion.EXTENSION_VERSION.EXTENSION_ID }, Keys.EXTENSION_PKEY, new TableField[] { Extension.EXTENSION.ID }, true); diff --git a/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/ExtensionVersion.java b/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/ExtensionVersion.java index 83a84e43a..a990853ce 100644 --- a/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/ExtensionVersion.java +++ b/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/ExtensionVersion.java @@ -235,6 +235,21 @@ public Class getRecordType() { */ public final TableField POTENTIALLY_MALICIOUS = createField(DSL.name("potentially_malicious"), SQLDataType.BOOLEAN, this, ""); + /** + * The column public.extension_version.removed. + */ + public final TableField REMOVED = createField(DSL.name("removed"), SQLDataType.BOOLEAN.nullable(false), this, ""); + + /** + * The column public.extension_version.removed_timestamp. + */ + public final TableField REMOVED_TIMESTAMP = createField(DSL.name("removed_timestamp"), SQLDataType.LOCALDATETIME(6), this, ""); + + /** + * The column public.extension_version.removed_by_id. + */ + public final TableField REMOVED_BY_ID = createField(DSL.name("removed_by_id"), SQLDataType.BIGINT, this, ""); + private ExtensionVersion(Name alias, Table aliased) { this(alias, aliased, (Field[]) null, null); } @@ -286,7 +301,7 @@ public List> getUniqueKeys() { @Override public List> getReferences() { - return Arrays.asList(Keys.EXTENSION_VERSION__EXTENSION_VERSION_SIGNATURE_KEY_PAIR_FKEY, Keys.EXTENSION_VERSION__FK70KHJ8PM0VACASUIIAQ0W0R80, Keys.EXTENSION_VERSION__FKKHS1EC9S9J08FGICQ9PMWU6BT); + return Arrays.asList(Keys.EXTENSION_VERSION__EXTENSION_VERSION_REMOVED_BY_ID_FKEY, Keys.EXTENSION_VERSION__EXTENSION_VERSION_SIGNATURE_KEY_PAIR_FKEY, Keys.EXTENSION_VERSION__FK70KHJ8PM0VACASUIIAQ0W0R80, Keys.EXTENSION_VERSION__FKKHS1EC9S9J08FGICQ9PMWU6BT); } @Override diff --git a/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/records/ExtensionVersionRecord.java b/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/records/ExtensionVersionRecord.java index d184b5756..68a2c2eaf 100644 --- a/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/records/ExtensionVersionRecord.java +++ b/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/records/ExtensionVersionRecord.java @@ -525,6 +525,48 @@ public Boolean getPotentiallyMalicious() { return (Boolean) get(35); } + /** + * Setter for public.extension_version.removed. + */ + public void setRemoved(Boolean value) { + set(36, value); + } + + /** + * Getter for public.extension_version.removed. + */ + public Boolean getRemoved() { + return (Boolean) get(36); + } + + /** + * Setter for public.extension_version.removed_timestamp. + */ + public void setRemovedTimestamp(LocalDateTime value) { + set(37, value); + } + + /** + * Getter for public.extension_version.removed_timestamp. + */ + public LocalDateTime getRemovedTimestamp() { + return (LocalDateTime) get(37); + } + + /** + * Setter for public.extension_version.removed_by_id. + */ + public void setRemovedById(Long value) { + set(38, value); + } + + /** + * Getter for public.extension_version.removed_by_id. + */ + public Long getRemovedById() { + return (Long) get(38); + } + // ------------------------------------------------------------------------- // Primary key information // ------------------------------------------------------------------------- @@ -548,7 +590,7 @@ public ExtensionVersionRecord() { /** * Create a detached, initialised ExtensionVersionRecord */ - public ExtensionVersionRecord(Long id, String bugs, String description, String displayName, String galleryColor, String galleryTheme, String homepage, String license, String markdown, Boolean preview, String qna, String repository, LocalDateTime timestamp, String version, Long extensionId, Long publishedWithId, Boolean active, String dependencies, String bundledExtensions, String engines, String categories, String tags, String extensionKind, Boolean preRelease, String targetPlatform, String localizedLanguages, String sponsorLink, Long signatureKeyPairId, Integer semverMajor, Integer semverMinor, Integer semverPatch, String semverPreRelease, Boolean semverIsPreRelease, String semverBuildMetadata, Boolean universalTargetPlatform, Boolean potentiallyMalicious) { + public ExtensionVersionRecord(Long id, String bugs, String description, String displayName, String galleryColor, String galleryTheme, String homepage, String license, String markdown, Boolean preview, String qna, String repository, LocalDateTime timestamp, String version, Long extensionId, Long publishedWithId, Boolean active, String dependencies, String bundledExtensions, String engines, String categories, String tags, String extensionKind, Boolean preRelease, String targetPlatform, String localizedLanguages, String sponsorLink, Long signatureKeyPairId, Integer semverMajor, Integer semverMinor, Integer semverPatch, String semverPreRelease, Boolean semverIsPreRelease, String semverBuildMetadata, Boolean universalTargetPlatform, Boolean potentiallyMalicious, Boolean removed, LocalDateTime removedTimestamp, Long removedById) { super(ExtensionVersion.EXTENSION_VERSION); setId(id); @@ -587,6 +629,9 @@ public ExtensionVersionRecord(Long id, String bugs, String description, String d setSemverBuildMetadata(semverBuildMetadata); setUniversalTargetPlatform(universalTargetPlatform); setPotentiallyMalicious(potentiallyMalicious); + setRemoved(removed); + setRemovedTimestamp(removedTimestamp); + setRemovedById(removedById); resetChangedOnNotNull(); } } diff --git a/server/src/main/resources/db/migration/V1_70__ExtensionVersion_Removed.sql b/server/src/main/resources/db/migration/V1_70__ExtensionVersion_Removed.sql new file mode 100644 index 000000000..6ab60b838 --- /dev/null +++ b/server/src/main/resources/db/migration/V1_70__ExtensionVersion_Removed.sql @@ -0,0 +1,12 @@ +-- Extension versions are immutable: a "deleted" version is soft-deleted (marked removed) instead of +-- being physically removed, so its identity stays reserved and can never be republished. +ALTER TABLE public.extension_version ADD COLUMN removed BOOLEAN; +ALTER TABLE public.extension_version ADD COLUMN removed_timestamp TIMESTAMP; +ALTER TABLE public.extension_version ADD COLUMN removed_by_id BIGINT; + +UPDATE public.extension_version SET removed = FALSE; + +ALTER TABLE public.extension_version ALTER COLUMN removed SET NOT NULL; + +ALTER TABLE public.extension_version ADD CONSTRAINT extension_version_removed_by_id_fkey +FOREIGN KEY (removed_by_id) REFERENCES public.user_data(id); diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java index d1cf78c5d..45733edcd 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java @@ -196,8 +196,11 @@ void deleteExtension_keepsExtensionAndOtherUsersVersionWhenDeletingOwnVersion() .as("another publisher's version must not be removed or orphaned") .isTrue(); assertThat(versionExists("1.0.0")) - .as("the owner's deleted version must be gone") - .isFalse(); + .as("the owner's deleted version row must be kept as an immutable tombstone") + .isTrue(); + assertThat(versionRemoved("1.0.0")) + .as("the owner's deleted version must be marked removed and inactive") + .isTrue(); } /** @@ -285,6 +288,20 @@ private boolean versionExists(String version) { .isEmpty())); } + private boolean versionRemoved(String version) { + return Boolean.TRUE.equals( + new TransactionTemplate(txManager).execute( + status -> !em.createQuery( + "select ev.id from ExtensionVersion ev " + + "where ev.version = :version " + + "and ev.extension.namespace.name = :namespace " + + "and ev.removed = true and ev.active = false") + .setParameter("version", version) + .setParameter("namespace", NAMESPACE) + .getResultList() + .isEmpty())); + } + private boolean extensionExists() { return Boolean.TRUE.equals( new TransactionTemplate(txManager).execute( diff --git a/webui/src/pages/user/add-user-dialog.tsx b/webui/src/components/add-user-dialog.tsx similarity index 97% rename from webui/src/pages/user/add-user-dialog.tsx rename to webui/src/components/add-user-dialog.tsx index fe5478118..cd75832bb 100644 --- a/webui/src/pages/user/add-user-dialog.tsx +++ b/webui/src/components/add-user-dialog.tsx @@ -24,8 +24,8 @@ import { Box, Avatar } from '@mui/material'; -import type { UserData } from '../../extension-registry-types'; -import { MainContext } from '../../context'; +import type { UserData } from '../extension-registry-types'; +import { MainContext } from '../context'; export interface AddUserDialogProps { open: boolean; diff --git a/webui/src/pages/user/user-namespace-extension-list-item.tsx b/webui/src/components/extension/extension-card-list-item.tsx similarity index 74% rename from webui/src/pages/user/user-namespace-extension-list-item.tsx rename to webui/src/components/extension/extension-card-list-item.tsx index 79ab92466..56407b9dd 100644 --- a/webui/src/pages/user/user-namespace-extension-list-item.tsx +++ b/webui/src/components/extension/extension-card-list-item.tsx @@ -14,9 +14,8 @@ import { Link as RouteLink } from 'react-router-dom'; import { Paper, Typography, Box, styled } from '@mui/material'; import { Extension } from '../../extension-registry-types'; import { createRoute } from '../../utils'; -import { ExtensionIcon } from '../../components/extension/extension-icon'; -import { Timestamp } from '../../components/timestamp'; -import { UserSettingsRoutes } from './user-settings-routes'; +import { ExtensionIcon } from './extension-icon'; +import { Timestamp } from '../timestamp'; const getOpacity = (extension: Extension) => { if (extension.deprecated) { @@ -40,12 +39,25 @@ const Paragraph = styled(Box)({ justifyContent: 'space-between' }); -export const UserNamespaceExtensionListItem: FunctionComponent = props => { +export const ExtensionCardListItem: FunctionComponent = props => { const { extension } = props; - const route = createRoute([UserSettingsRoutes.EXTENSIONS, extension.namespace, extension.name]); + const route = createRoute([props.routePrefix, extension.namespace, extension.name]); const inactive = extension.active === false; const renderStatus = (): ReactNode => { + if (extension?.removed) { + return ( + + + Deleted + + + {extension.reviewMessage ?? 'The extension has been deleted.'} + + + ); + } + if (extension.reviewStatus === 'under_review') { return ( @@ -106,11 +118,15 @@ export const UserNamespaceExtensionListItem: FunctionComponent @@ -132,7 +148,10 @@ export const UserNamespaceExtensionListItem: FunctionComponent = props => { +export const ExtensionCardList: FunctionComponent = props => { return ( = prop display: 'grid', gridTemplateColumns: `repeat(auto-fit, minmax(300px, 1fr))`, gap: '.5rem', - mt: '1rem' + mt: '1rem', + mb: '1rem' }}> {props.extensions && props.extensions.length > 0 ? props.extensions.map((extension: Extension) => ( - )) : null} diff --git a/webui/src/components/extension/extension-delete-all-versions-dialog.tsx b/webui/src/components/extension/extension-delete-all-versions-dialog.tsx index cede6ced5..1f9815972 100644 --- a/webui/src/components/extension/extension-delete-all-versions-dialog.tsx +++ b/webui/src/components/extension/extension-delete-all-versions-dialog.tsx @@ -23,11 +23,16 @@ export const DeleteAllVersionsDialog: FunctionComponent { try { setWorking(true); + // Delete (soft) only applies to versions still present; purge can also target already-removed ones. const targets: VersionDeleteTarget[] = props.versions.flatMap(v => - v.targetPlatforms.map(({ targetPlatform }) => ({ version: v.version, targetPlatform })) + v.targetPlatforms + .filter(tp => isPurge || !tp.removed) + .map(({ targetPlatform }) => ({ version: v.version, targetPlatform })) ); await props.onRemove(targets); props.onDeleted(); @@ -52,12 +57,20 @@ export const DeleteAllVersionsDialog: FunctionComponent - Delete all versions of {props.extension.displayName ?? props.extension.name}? + + {isPurge ? 'Purge' : 'Delete'} all versions of {props.extension.displayName ?? props.extension.name}? + - This will permanently remove {props.versions.length} version - {props.versions.length === 1 ? '' : 's'} of this extension across all target platforms. This action - cannot be undone. + {isPurge + ? `This permanently removes ${props.versions.length} version` + + `${props.versions.length === 1 ? '' : 's'} of this extension from the database and storage ` + + 'across all target platforms, freeing the version numbers so they can be published again. ' + + 'This cannot be undone.' + : `This removes ${props.versions.length} version` + + `${props.versions.length === 1 ? '' : 's'} of this extension across all target platforms and ` + + 'deletes their files. Extension versions are immutable, so removed versions stay ' + + 'permanently reserved and can never be republished. This cannot be undone.'} @@ -65,7 +78,7 @@ export const DeleteAllVersionsDialog: FunctionComponent - Delete All Versions + {isPurge ? 'Purge All Versions' : 'Delete All Versions'} @@ -79,4 +92,6 @@ export interface DeleteAllVersionsDialogProps { versions: VersionTargetPlatforms[]; onRemove: (targets: VersionDeleteTarget[]) => Promise; onDeleted: () => void; + // 'delete' (default) soft-deletes; 'purge' permanently removes (admin only). + mode?: 'delete' | 'purge'; } diff --git a/webui/src/components/extension/extension-detail-view.tsx b/webui/src/components/extension/extension-detail-view.tsx index 7bf2cf7b8..5f8deda07 100644 --- a/webui/src/components/extension/extension-detail-view.tsx +++ b/webui/src/components/extension/extension-detail-view.tsx @@ -24,11 +24,14 @@ import { ExtensionDetailRoutes } from '../../pages/extension-detail/extension-de import { createRoute } from '../../utils'; export const ExtensionDetailView: FunctionComponent = props => { - const { extension, actions, onRemoveVersion, onVersionDeleted } = props; + const { extension, actions, onRemoveVersion, onVersionDeleted, onPurgeVersion } = props; + const canPurge = !!onPurgeVersion; const [page, setPage] = useState(0); const [deleteDialogVersion, setDeleteDialogVersion] = useState(null); + const [purgeDialogVersion, setPurgeDialogVersion] = useState(null); const [deleteAllOpen, setDeleteAllOpen] = useState(false); + const [purgeAllOpen, setPurgeAllOpen] = useState(false); useEffect(() => { setPage(0); @@ -36,6 +39,8 @@ export const ExtensionDetailView: FunctionComponent = const publicRoute = createRoute([ExtensionDetailRoutes.ROOT, extension.namespace, extension.name]); const allVersions = (extension.allTargetPlatformVersions ?? []).filter(v => !VERSION_ALIASES.includes(v.version)); + // A version can still be (soft-)deleted while it has at least one target platform that is not removed. + const hasDeletableVersions = allVersions.some(v => v.targetPlatforms.some(tp => !tp.removed)); return ( @@ -48,16 +53,27 @@ export const ExtensionDetailView: FunctionComponent = - + {extension.active ?? ( + + )} + {canPurge && ( + + )} {actions} @@ -68,6 +84,7 @@ export const ExtensionDetailView: FunctionComponent = page={page} onPageChange={setPage} onDeleteVersion={setDeleteDialogVersion} + onPurgeVersion={canPurge ? setPurgeDialogVersion : undefined} /> {deleteDialogVersion && ( = onDeleted={onVersionDeleted} /> )} + {purgeDialogVersion && onPurgeVersion && ( + setPurgeDialogVersion(null)} + extension={extension} + version={purgeDialogVersion} + onRemove={onPurgeVersion} + onDeleted={onVersionDeleted} + /> + )} {deleteAllOpen && ( = onDeleted={onVersionDeleted} /> )} + {purgeAllOpen && onPurgeVersion && ( + setPurgeAllOpen(false)} + extension={extension} + versions={allVersions} + onRemove={onPurgeVersion} + onDeleted={onVersionDeleted} + /> + )} ); }; @@ -98,4 +137,6 @@ export interface ExtensionDetailViewProps { actions?: ReactNode; onRemoveVersion: (targets: VersionDeleteTarget[]) => Promise; onVersionDeleted: () => void; + // When provided (admin only), enables the permanent-purge affordances. + onPurgeVersion?: (targets: VersionDeleteTarget[]) => Promise; } diff --git a/webui/src/components/extension/extension-status-chips.tsx b/webui/src/components/extension/extension-status-chips.tsx index c5acc6c91..bca8e24b8 100644 --- a/webui/src/components/extension/extension-status-chips.tsx +++ b/webui/src/components/extension/extension-status-chips.tsx @@ -18,6 +18,7 @@ import { Extension } from '../../extension-registry-types'; export const ExtensionStatusChips: FunctionComponent = ({ extension }) => ( {extension.deprecated && } + {extension?.removed && } {extension.active === false && } {extension.reviewStatus === 'under_review' && } {extension.reviewStatus === 'rejected' && } diff --git a/webui/src/components/extension/extension-version-delete-dialog.tsx b/webui/src/components/extension/extension-version-delete-dialog.tsx index 5fc8cf438..49e586c2e 100644 --- a/webui/src/components/extension/extension-version-delete-dialog.tsx +++ b/webui/src/components/extension/extension-version-delete-dialog.tsx @@ -20,6 +20,7 @@ import { Dialog, DialogActions, DialogContent, + DialogContentText, DialogTitle, FormControlLabel, FormGroup @@ -44,9 +45,17 @@ export const DeleteVersionDialog: FunctionComponent = const [items, setItems] = useState([]); const [working, setWorking] = useState(false); + const mode = props.mode ?? 'delete'; + const isPurge = mode === 'purge'; + useEffect(() => { - setItems(buildVersionDialogItems(props.version.targetPlatforms.map(tp => tp.targetPlatform))); - }, [props.version]); + // Delete (soft) only applies to versions that are still present; purge can also target + // versions that were already removed (to free their identity for republishing). + const targetPlatforms = props.version.targetPlatforms + .filter(tp => isPurge || !tp.removed) + .map(tp => tp.targetPlatform); + setItems(buildVersionDialogItems(targetPlatforms)); + }, [props.version, isPurge]); const handleChange = (event: ChangeEvent) => { setItems(prev => handleVersionDialogChange(event, prev)); @@ -86,9 +95,17 @@ export const DeleteVersionDialog: FunctionComponent = return (

- Delete version {props.version.version} of {props.extension.displayName ?? props.extension.name} + {isPurge ? 'Purge' : 'Delete'} version {props.version.version} of{' '} + {props.extension.displayName ?? props.extension.name} + + {isPurge + ? 'Purging permanently removes the selected version(s) from the database and storage, ' + + 'freeing the version identity so it can be published again. This cannot be undone.' + : 'The selected version(s) will be removed and their files deleted. Extension versions are ' + + 'immutable, so a removed version stays permanently reserved and can never be republished.'} + {allItem && ( = {working && ( Promise; onDeleted: () => void; + // 'delete' (default) soft-deletes; 'purge' permanently removes (admin only). + mode?: 'delete' | 'purge'; } diff --git a/webui/src/components/extension/extension-version-table.tsx b/webui/src/components/extension/extension-version-table.tsx index 5c07cbd27..4b327d94a 100644 --- a/webui/src/components/extension/extension-version-table.tsx +++ b/webui/src/components/extension/extension-version-table.tsx @@ -13,8 +13,10 @@ import { FunctionComponent } from 'react'; import { + Chip, IconButton, Paper, + Stack, Table, TableBody, TableCell, @@ -22,11 +24,12 @@ import { TableFooter, TableHead, TablePagination, - TableRow, - Typography + TableRow } from '@mui/material'; import DeleteIcon from '@mui/icons-material/Delete'; +import DeleteForeverIcon from '@mui/icons-material/DeleteForever'; import { VersionTargetPlatforms } from '../../extension-registry-types'; +import { getTargetPlatformDisplayName } from '../../utils'; const PAGE_SIZE = 20; @@ -34,9 +37,11 @@ export const ExtensionVersionTable: FunctionComponent { const pagedVersions = versions.slice(page * PAGE_SIZE, page * PAGE_SIZE + PAGE_SIZE); + const columnCount = onPurgeVersion ? 4 : 3; return ( @@ -51,31 +56,60 @@ export const ExtensionVersionTable: FunctionComponent - {pagedVersions.map(v => ( - - {v.version} - - {v.targetPlatforms.map((tp, index) => ( - - - {tp.targetPlatform} - - {index < v.targetPlatforms.length - 1 ? ', ' : ''} - - ))} - - - onDeleteVersion(v)}> - - - - - ))} + {pagedVersions.map(v => { + const allRemoved = v.targetPlatforms.length > 0 && v.targetPlatforms.every(tp => tp.removed); + return ( + + + + {v.version} + {allRemoved && ( + + )} + + + + + {v.targetPlatforms.map(tp => ( + + ))} + + + + onDeleteVersion(v)}> + + + + {onPurgeVersion && ( + + onPurgeVersion(v)}> + + + + )} + + ); + })} {versions.length === 0 && ( - + No version information available. @@ -102,4 +136,6 @@ export interface ExtensionVersionTableProps { page: number; onPageChange: (page: number) => void; onDeleteVersion: (version: VersionTargetPlatforms) => void; + // When provided (admin only), each row shows a permanent-purge action. + onPurgeVersion?: (version: VersionTargetPlatforms) => void; } diff --git a/webui/src/pages/user/add-namespace-member-dialog.tsx b/webui/src/components/namespace/add-namespace-member-dialog.tsx similarity index 94% rename from webui/src/pages/user/add-namespace-member-dialog.tsx rename to webui/src/components/namespace/add-namespace-member-dialog.tsx index 86a91ad83..68a10e9e7 100644 --- a/webui/src/pages/user/add-namespace-member-dialog.tsx +++ b/webui/src/components/namespace/add-namespace-member-dialog.tsx @@ -11,9 +11,9 @@ import { FunctionComponent, useContext, useRef } from 'react'; import { UserData } from '../..'; import { Namespace, NamespaceMembership, isError } from '../../extension-registry-types'; -import { NamespaceDetailConfigContext } from './user-settings-namespace-detail'; +import { NamespaceDetailConfigContext } from './namespace-detail-view'; import { MainContext } from '../../context'; -import { AddUserDialog } from './add-user-dialog'; +import { AddUserDialog } from '../add-user-dialog'; export interface AddMemberDialogProps { open: boolean; diff --git a/webui/src/components/namespace/namespace-detail-view.tsx b/webui/src/components/namespace/namespace-detail-view.tsx new file mode 100644 index 000000000..abaac4a09 --- /dev/null +++ b/webui/src/components/namespace/namespace-detail-view.tsx @@ -0,0 +1,141 @@ +/******************************************************************************** + * Copyright (c) 2020 TypeFox and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ + +import { FunctionComponent, ReactNode, createContext } from 'react'; +import { Box, Link, Paper, Grid, Typography } from '@mui/material'; +import { styled, Theme } from '@mui/material/styles'; +import WarningIcon from '@mui/icons-material/Warning'; +import { NamespaceExtensionList, FetchNamespaceExtension } from './namespace-extension-list'; +import { NamespaceMemberList } from './namespace-member-list'; +import { NamespaceDetails } from './namespace-details'; +import { Namespace, UserData } from '../../extension-registry-types'; + +export interface NamespaceDetailConfig { + defaultMemberRole?: 'contributor' | 'owner'; +} + +// eslint-disable-next-line react-refresh/only-export-components +export const NamespaceDetailConfigContext = createContext({}); + +const NamespaceDetailContainer = styled(Grid)(({ theme }: { theme: Theme }) => ({ + flex: 5, + padding: theme.spacing(0, 1), + [theme.breakpoints.only('md')]: { + width: '80%' + }, + [theme.breakpoints.down('sm')]: { + width: '100%' + } +})); + +const WarningPaper = styled(Paper)(({ theme }: { theme: Theme }) => ({ + maxWidth: '800px', + margin: `0 ${theme.spacing(6)} ${theme.spacing(4)} ${theme.spacing(6)}`, + padding: theme.spacing(2), + display: 'flex', + [theme.breakpoints.down('sm')]: { + margin: `0 0 ${theme.spacing(2)} 0` + } +})); + +const NamespaceHeader = styled(Box)(({ theme }: { theme: Theme }) => ({ + display: 'flex', + justifyContent: 'space-between', + alignItems: 'center', + marginBottom: theme.spacing(1), + [theme.breakpoints.down('sm')]: { + flexDirection: 'column', + alignItems: 'center' + } +})); + +/** + * Reusable view of a namespace: its optional not-verified warning, header, member list, details and + * extension list. It is page-agnostic — page-specific concerns are injected: + * - `headerActions` lets a host render extra buttons in the header (e.g. the admin + * "Change Namespace"/"Delete" actions together with their dialogs). + * - `fetchExtension` selects the endpoint used to load each extension (public vs. admin). + */ +export const NamespaceDetailView: FunctionComponent = props => { + const warningColor = props.theme === 'dark' ? '#fff' : '#151515'; + return ( + + {!props.namespace.verified && props.namespaceAccessUrl ? ( + + + + + This namespace is not verified.{' '} + + See the documentation + {' '} + to learn about claiming namespaces. + + + + ) : null} + + + {props.namespace.name} + {props.headerActions} + + + {props.namespace.membersUrl ? ( + + + + ) : null} + {props.namespace.detailsUrl ? ( + + + + ) : null} + + + + + ); +}; + +export interface NamespaceDetailViewProps { + namespace: Namespace; + filterUsers: (user: UserData) => boolean; + fixSelf: boolean; + setLoadingState: (loading: boolean) => void; + namespaceAccessUrl?: string; + theme?: string; + // Extra actions rendered in the header (e.g. the admin "Change Namespace"/"Delete" buttons and + // their dialogs). Supplied by the host page so this view stays free of page-specific concerns. + headerActions?: ReactNode; + // Endpoint used to retrieve each extension's detail; forwarded to the extension list. Defaults to + // the public registry API when omitted (e.g. the user surface). The admin surface passes the admin + // endpoint so inactive/soft-deleted extensions are shown too. + fetchExtension?: FetchNamespaceExtension; + // Base route each extension card links to. Supplied by the host: the user surface passes the user + // settings extension route, the admin surface passes the admin extension route. + extensionRoutePrefix: string; +} diff --git a/webui/src/pages/user/user-namespace-details.tsx b/webui/src/components/namespace/namespace-details.tsx similarity index 98% rename from webui/src/pages/user/user-namespace-details.tsx rename to webui/src/components/namespace/namespace-details.tsx index 4d3e8c72d..380717883 100644 --- a/webui/src/pages/user/user-namespace-details.tsx +++ b/webui/src/components/namespace/namespace-details.tsx @@ -42,8 +42,8 @@ import ZoomInIcon from '@mui/icons-material/ZoomIn'; import ZoomOutIcon from '@mui/icons-material/ZoomOut'; import CloseIcon from '@mui/icons-material/Close'; import { MainContext } from '../../context'; -import { DelayedLoadIndicator } from '../../components/delayed-load-indicator'; -import { Namespace, NamespaceDetails, isError } from '../../extension-registry-types'; +import { DelayedLoadIndicator } from '../delayed-load-indicator'; +import { Namespace, NamespaceDetails as NamespaceDetailsData, isError } from '../../extension-registry-types'; import Dropzone from 'react-dropzone'; import AvatarEditor, { Position, type AvatarEditorRef } from 'react-avatar-editor'; import _ from 'lodash'; @@ -86,7 +86,7 @@ const GridIconItem = styled(Grid)({ alignItems: 'center' }); -export const UserNamespaceDetails: FunctionComponent = props => { +export const NamespaceDetails: FunctionComponent = props => { const INPUT_DISPLAY_NAME = 'display-name'; const INPUT_DESCRIPTION = 'description'; const INPUT_WEBSITE = 'website'; @@ -101,8 +101,8 @@ export const UserNamespaceDetails: FunctionComponent const editor = useRef(null); const context = useContext(MainContext); - const [currentDetails, setCurrentDetails] = useState(); - const [newDetails, setNewDetails] = useState(); + const [currentDetails, setCurrentDetails] = useState(); + const [newDetails, setNewDetails] = useState(); const [detailsUpdated, setDetailsUpdated] = useState(false); const [bannerNamespaceName, setBannerNamespaceName] = useState(''); const [loading, setLoading] = useState(true); @@ -186,7 +186,7 @@ export const UserNamespaceDetails: FunctionComponent } }; - const copy = (arg: NamespaceDetails): NamespaceDetails => { + const copy = (arg: NamespaceDetailsData): NamespaceDetailsData => { return JSON.parse(JSON.stringify(arg)); }; @@ -700,6 +700,6 @@ export const UserNamespaceDetails: FunctionComponent ); }; -export interface UserNamespaceDetailsProps { +export interface NamespaceDetailsProps { namespace: Namespace; } diff --git a/webui/src/components/namespace/namespace-extension-list.tsx b/webui/src/components/namespace/namespace-extension-list.tsx new file mode 100644 index 000000000..7a6f8d754 --- /dev/null +++ b/webui/src/components/namespace/namespace-extension-list.tsx @@ -0,0 +1,108 @@ +/****************************************************************************** + * 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 { FunctionComponent, useContext, useEffect, useRef, useState } from 'react'; +import { Typography } from '@mui/material'; +import { Namespace, isError, Extension, ErrorResult } from '../../extension-registry-types'; +import { MainContext } from '../../context'; +import { ExtensionCardList } from '../extension/extension-card-list'; + +/** + * Retrieves the full detail for a single extension of a namespace. The caller decides which endpoint + * to use, e.g. the public registry API (active extensions only) or the admin API (also returns + * inactive/soft-deleted extensions). Receives both the extension `name` and its metadata `url` so the + * implementation can use whichever it needs. + */ +export type FetchNamespaceExtension = ( + abortController: AbortController, + extension: { name: string; url: string } +) => Promise>; + +/** + * Generic list of the extensions published under a namespace. It renders each extension as a card + * (inactive extensions are rendered greyed out by the card itself). The extension detail lookup is + * injected via {@link FetchNamespaceExtension} so the same component can be reused with different + * endpoints on the user and admin surfaces. + */ +export const NamespaceExtensionList: FunctionComponent = props => { + const [extensions, setExtensions] = useState(); + const [loading, setLoading] = useState(true); + const context = useContext(MainContext); + + const fetchExtension: FetchNamespaceExtension = + props.fetchExtension ?? + ((abortController, extension) => context.service.getExtensionDetail(abortController, extension.url)); + + const abortController = useRef(new AbortController()); + useEffect(() => { + updateExtensions(); + return () => abortController.current.abort(); + }, []); + + useEffect(() => { + setExtensions(undefined); + setLoading(true); + updateExtensions(); + }, [props.namespace.name]); + + const updateExtensions = async (): Promise => { + const entries = Object.keys(props.namespace.extensions).map((name: string) => ({ + name, + url: props.namespace.extensions[name] + })); + + const getExtension = async (entry: { name: string; url: string }) => { + try { + const result = await fetchExtension(abortController.current, entry); + if (isError(result)) { + throw result; + } + return result; + } catch (error) { + context.handleError(error); + return undefined; + } + }; + + const extensionUnfiltered = await Promise.all(entries.map(getExtension)); + const extensions = extensionUnfiltered.filter(e => e != null) as Extension[]; + + setExtensions(extensions); + setLoading(false); + }; + + return ( + <> + Extensions + {extensions && extensions.length > 0 ? ( + + ) : ( + No extensions published under this namespace yet. + )} + + ); +}; + +export interface NamespaceExtensionListProps { + namespace: Namespace; + // Endpoint used to retrieve each extension's detail. Defaults to the public registry API. + fetchExtension?: FetchNamespaceExtension; + canDelete?: boolean; + // Base route each extension card links to. Supplied by the caller. + routePrefix: string; +} diff --git a/webui/src/pages/user/user-namespace-member-component.tsx b/webui/src/components/namespace/namespace-member-component.tsx similarity index 96% rename from webui/src/pages/user/user-namespace-member-component.tsx rename to webui/src/components/namespace/namespace-member-component.tsx index 32a30f62e..c78d69581 100644 --- a/webui/src/pages/user/user-namespace-member-component.tsx +++ b/webui/src/components/namespace/namespace-member-component.tsx @@ -13,7 +13,7 @@ import { Box, Typography, Avatar, Select, MenuItem, Button, SelectChangeEvent } import { NamespaceMembership, MembershipRole, Namespace, UserData } from '../../extension-registry-types'; import { MainContext } from '../../context'; -export const UserNamespaceMember: FunctionComponent = props => { +export const NamespaceMember: FunctionComponent = props => { const equalUser = (user1: UserData | undefined, user2: UserData | undefined) => { return user1?.loginName === user2?.loginName && user1?.provider === user2?.provider; }; @@ -85,7 +85,7 @@ export const UserNamespaceMember: FunctionComponent = ); }; -export interface UserNamespaceMemberProps { +export interface NamespaceMemberProps { namespace: Namespace; member: NamespaceMembership; fixSelf: boolean; diff --git a/webui/src/pages/user/user-namespace-member-list.tsx b/webui/src/components/namespace/namespace-member-list.tsx similarity index 94% rename from webui/src/pages/user/user-namespace-member-list.tsx rename to webui/src/components/namespace/namespace-member-list.tsx index a727c935f..afeac06ed 100644 --- a/webui/src/pages/user/user-namespace-member-list.tsx +++ b/webui/src/components/namespace/namespace-member-list.tsx @@ -10,12 +10,12 @@ import { FunctionComponent, useEffect, useState, useContext, useRef } from 'react'; import { Box, Typography, Button, Paper } from '@mui/material'; -import { UserNamespaceMember } from './user-namespace-member-component'; +import { NamespaceMember } from './namespace-member-component'; import { Namespace, NamespaceMembership, MembershipRole, isError, UserData } from '../../extension-registry-types'; import { AddMemberDialog } from './add-namespace-member-dialog'; import { MainContext } from '../../context'; -export const UserNamespaceMemberList: FunctionComponent = props => { +export const NamespaceMemberList: FunctionComponent = props => { const { service, user, handleError } = useContext(MainContext); const [members, setMembers] = useState([]); const [addDialogIsOpen, setAddDialogIsOpen] = useState(false); @@ -89,7 +89,7 @@ export const UserNamespaceMemberList: FunctionComponent {members.map(member => ( - void; filterUsers: (user: UserData) => boolean; diff --git a/webui/src/extension-registry-service.ts b/webui/src/extension-registry-service.ts index 8191333ab..97c9e0272 100644 --- a/webui/src/extension-registry-service.ts +++ b/webui/src/extension-registry-service.ts @@ -603,6 +603,11 @@ export interface AdminService { extension: string; targetPlatformVersions?: object[]; }): Promise>; + purgeExtensions(req: { + namespace: string; + extension: string; + targetPlatformVersions?: object[]; + }): Promise>; getNamespace(abortController: AbortController, name: string): Promise>; createNamespace(namespace: { name: string }): Promise>; deleteNamespace(namespace: { name: string }): Promise>; @@ -763,6 +768,36 @@ export class AdminServiceImpl implements AdminService { }); } + async purgeExtensions(req: { + namespace: string; + extension: string; + targetPlatformVersions?: object[]; + }): Promise> { + const csrfResponse = await this.registry.getCsrfToken(); + const headers: Record = { + 'Content-Type': 'application/json;charset=UTF-8' + }; + if (!isError(csrfResponse)) { + const csrfToken = csrfResponse as CsrfTokenJson; + headers[csrfToken.header] = csrfToken.value; + } + + return sendNonRetriableRequest({ + method: 'POST', + credentials: true, + endpoint: createAbsoluteURL([ + this.registry.serverUrl, + 'admin', + 'extension', + req.namespace, + req.extension, + 'purge' + ]), + headers, + payload: req.targetPlatformVersions + }); + } + async getNamespace(abortController: AbortController, name: string): Promise> { return sendNonRetriableRequest({ abortController, diff --git a/webui/src/extension-registry-types.ts b/webui/src/extension-registry-types.ts index 4e0243ee4..c9aaddf77 100644 --- a/webui/src/extension-registry-types.ts +++ b/webui/src/extension-registry-types.ts @@ -78,6 +78,7 @@ export interface Extension { // key: version, value: url allVersions: { [version: string]: UrlString }; active?: boolean; + removed?: boolean; reviewStatus?: 'published' | 'under_review' | 'rejected'; reviewMessage?: string; @@ -135,6 +136,9 @@ export interface ExtensionReference { export interface TargetPlatformActive { targetPlatform: string; active: boolean; + // Whether this target platform version has been removed (soft-deleted). A removed version is a + // permanent tombstone: hidden and non-republishable. Only an admin purge frees it. + removed: boolean; } export interface VersionTargetPlatforms { diff --git a/webui/src/pages/admin-dashboard/customers/customer-member-list.tsx b/webui/src/pages/admin-dashboard/customers/customer-member-list.tsx index ed362c2ad..c7ce3627f 100644 --- a/webui/src/pages/admin-dashboard/customers/customer-member-list.tsx +++ b/webui/src/pages/admin-dashboard/customers/customer-member-list.tsx @@ -30,7 +30,7 @@ import { Link as RouterLink } from 'react-router-dom'; import { AdminDashboardRoutes } from '../admin-dashboard-routes'; import { MainContext } from '../../../context'; import { Customer, UserData } from '../../../extension-registry-types'; -import { AddUserDialog } from '../../user/add-user-dialog'; +import { AddUserDialog } from '../../../components/add-user-dialog'; import DeleteIcon from '@mui/icons-material/Delete'; import PersonAddIcon from '@mui/icons-material/PersonAdd'; import { createRoute } from '../../../utils'; diff --git a/webui/src/pages/admin-dashboard/extension-admin.tsx b/webui/src/pages/admin-dashboard/extension-admin.tsx index 20fefd974..6ca97038b 100644 --- a/webui/src/pages/admin-dashboard/extension-admin.tsx +++ b/webui/src/pages/admin-dashboard/extension-admin.tsx @@ -15,7 +15,7 @@ import { Button, Typography } from '@mui/material'; import { MainContext } from '../../context'; import { SearchListContainer } from './search-list-container'; import { StyledInput } from './namespace-input'; -import { useAdminExtension, useDeleteExtension } from './use-extension-admin'; +import { useAdminExtension, useDeleteExtension, usePurgeExtension } from './use-extension-admin'; import { ExtensionDetailView } from '../../components/extension/extension-detail-view'; import { AdminDashboardRoutes } from './admin-dashboard-routes'; import { createRoute } from '../../utils'; @@ -42,6 +42,7 @@ export const ExtensionAdmin: FunctionComponent = () => { const { data, isFetching: loading, error: queryError, refetch } = useAdminExtension(target); const { mutateAsync: deleteExtension } = useDeleteExtension(); + const { mutateAsync: purgeExtension } = usePurgeExtension(); const is404 = !!queryError && (queryError as { status?: number }).status === 404; const extension = queryError ? undefined : data; @@ -113,6 +114,13 @@ export const ExtensionAdmin: FunctionComponent = () => { targetPlatformVersions: targets }) } + onPurgeVersion={targets => + purgeExtension({ + namespace: extension.namespace, + extension: extension.name, + targetPlatformVersions: targets + }) + } onVersionDeleted={refetch} /> ) : ( diff --git a/webui/src/pages/admin-dashboard/namespace-admin.tsx b/webui/src/pages/admin-dashboard/namespace-admin.tsx index 1a108382d..861523ddb 100644 --- a/webui/src/pages/admin-dashboard/namespace-admin.tsx +++ b/webui/src/pages/admin-dashboard/namespace-admin.tsx @@ -9,18 +9,20 @@ ********************************************************************************/ import { FunctionComponent, useState, useContext, useEffect, ReactNode } from 'react'; -import { Typography, Box } from '@mui/material'; +import { Typography, Box, Button } from '@mui/material'; import { useParams, useNavigate } from 'react-router-dom'; -import { NamespaceDetail, NamespaceDetailConfigContext } from '../user/user-settings-namespace-detail'; +import { NamespaceDetailView, NamespaceDetailConfigContext } from '../../components/namespace/namespace-detail-view'; import { ButtonWithProgress } from '../../components/button-with-progress'; import { MainContext } from '../../context'; import { StyledInput } from './namespace-input'; import { SearchListContainer } from './search-list-container'; import { AdminDashboardRoutes } from './admin-dashboard-routes'; +import { NamespaceChangeDialog } from './namespace-change-dialog'; +import { NamespaceDeleteDialog } from './namespace-delete-dialog'; import { useAdminNamespace, useClearAdminNamespace, useCreateNamespace } from './use-namespace-admin'; export const NamespaceAdmin: FunctionComponent = () => { - const { pageSettings, user, handleError } = useContext(MainContext); + const { service, pageSettings, user, handleError } = useContext(MainContext); const { namespace: nsParam } = useParams<{ namespace?: string }>(); const navigate = useNavigate(); @@ -28,6 +30,8 @@ export const NamespaceAdmin: FunctionComponent = () => { const [inputValue, setInputValue] = useState(nsParam ?? ''); // The namespace detail view can drive the loading indicator while it performs its own work. const [detailLoading, setDetailLoading] = useState(false); + const [changeDialogIsOpen, setChangeDialogIsOpen] = useState(false); + const [deleteDialogIsOpen, setDeleteDialogIsOpen] = useState(false); const { data: currentNamespace, isFetching, error, refetch } = useAdminNamespace(searchName); const { mutateAsync: createNamespace, isPending: isCreatingNamespace } = useCreateNamespace(); @@ -82,14 +86,52 @@ export const NamespaceAdmin: FunctionComponent = () => { let listContainer: ReactNode = ''; if (currentNamespace && pageSettings && user) { + const headerActions = ( + + + {Object.keys(currentNamespace.extensions).length === 0 && ( + + )} + + ); listContainer = ( - true} fixSelf={false} + headerActions={headerActions} + extensionRoutePrefix={AdminDashboardRoutes.EXTENSION_ADMIN} + fetchExtension={(abortController, extension) => + service.admin.getExtension(abortController, currentNamespace.name, extension.name) + } + /> + setChangeDialogIsOpen(false)} + namespace={currentNamespace} + setLoadingState={setDetailLoading} + /> + setDeleteDialogIsOpen(false)} + onDelete={() => { + setDeleteDialogIsOpen(false); + handleDeleteNamespace(); + }} + namespace={currentNamespace} + setLoadingState={setDetailLoading} /> ); diff --git a/webui/src/pages/admin-dashboard/publisher-details.tsx b/webui/src/pages/admin-dashboard/publisher-details.tsx index 9ce38099e..c4f03c2d2 100644 --- a/webui/src/pages/admin-dashboard/publisher-details.tsx +++ b/webui/src/pages/admin-dashboard/publisher-details.tsx @@ -34,7 +34,7 @@ import GavelIcon from '@mui/icons-material/Gavel'; import { UserRelationships, SuccessResult } from '../../extension-registry-types'; import { ErrorResponse } from '../../server-request'; import { MainContext } from '../../context'; -import { UserExtensionList } from '../user/user-extension-list'; +import { ExtensionCardList } from '../../components/extension/extension-card-list'; import { handleError as formatError, toLocalTime } from '../../utils'; import { AdminDashboardRoutes } from './admin-dashboard-routes'; import { PublisherRevokeContributionsButton } from './publisher-revoke-dialog'; @@ -256,7 +256,11 @@ export const PublisherDetails: FunctionComponent<{ entry: UserRelationships }> = title='Published extensions' count={publisherInfo.extensions.length}> {publisherInfo.extensions.length > 0 ? ( - + ) : ( This user has not published any extensions. diff --git a/webui/src/pages/admin-dashboard/use-extension-admin.ts b/webui/src/pages/admin-dashboard/use-extension-admin.ts index 61d09be91..c71b3bb72 100644 --- a/webui/src/pages/admin-dashboard/use-extension-admin.ts +++ b/webui/src/pages/admin-dashboard/use-extension-admin.ts @@ -65,3 +65,14 @@ export const useDeleteExtension = () => { mutationFn: (req: DeleteExtensionRequest) => service.admin.deleteExtensions(req) }); }; + +/** + * Permanently purges extension versions (admin only). Unlike delete, this physically removes the + * versions from the database and storage, freeing their identities for republishing. + */ +export const usePurgeExtension = () => { + const { service } = useContext(MainContext); + return useMutation({ + mutationFn: (req: DeleteExtensionRequest) => service.admin.purgeExtensions(req) + }); +}; diff --git a/webui/src/pages/user/user-namespace-extension-list.tsx b/webui/src/pages/user/user-namespace-extension-list.tsx deleted file mode 100644 index 335493c6f..000000000 --- a/webui/src/pages/user/user-namespace-extension-list.tsx +++ /dev/null @@ -1,76 +0,0 @@ -/******************************************************************************** - * Copyright (c) 2020 TypeFox and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v. 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0. - * - * SPDX-License-Identifier: EPL-2.0 - ********************************************************************************/ - -import { FunctionComponent, useContext, useEffect, useState, useRef } from 'react'; -import { Namespace, isError, Extension, ErrorResult } from '../../extension-registry-types'; -import { MainContext } from '../../context'; -import { UserExtensionList } from './user-extension-list'; -import { Typography } from '@mui/material'; - -export const UserNamespaceExtensionListContainer: FunctionComponent< - UserNamespaceExtensionListContainerProps -> = props => { - const [extensions, setExtensions] = useState(); - const [loading, setLoading] = useState(true); - const context = useContext(MainContext); - - const abortController = useRef(new AbortController()); - useEffect(() => { - updateExtensions(); - return () => abortController.current.abort(); - }, []); - - useEffect(() => { - setExtensions(undefined); - setLoading(true); - updateExtensions(); - }, [props.namespace.name]); - - const updateExtensions = async (): Promise => { - const extensionsURLs: string[] = Object.keys(props.namespace.extensions).map( - (key: string) => props.namespace.extensions[key] - ); - - const getExtension = async (url: string) => { - let result: Extension | ErrorResult; - try { - result = await context.service.getExtensionDetail(abortController.current, url); - if (isError(result)) { - throw result; - } - return result; - } catch (error) { - context.handleError(error); - return undefined; - } - }; - - const extensionUnfiltered = await Promise.all(extensionsURLs.map((url: string) => getExtension(url))); - const extensions = extensionUnfiltered.filter(e => e != null) as Extension[]; - - setExtensions(extensions); - setLoading(false); - }; - - return ( - <> - Extensions - {extensions && extensions.length > 0 ? ( - - ) : ( - No extensions published under this namespace yet. - )} - - ); -}; - -export interface UserNamespaceExtensionListContainerProps { - namespace: Namespace; -} diff --git a/webui/src/pages/user/user-settings-extensions.tsx b/webui/src/pages/user/user-settings-extensions.tsx index 6e6976234..016a363ca 100644 --- a/webui/src/pages/user/user-settings-extensions.tsx +++ b/webui/src/pages/user/user-settings-extensions.tsx @@ -12,7 +12,8 @@ import { FunctionComponent, useContext, useEffect, useState, useRef } from 'reac import { Extension } from '../../extension-registry-types'; import { Box, Typography } from '@mui/material'; import { PublishExtensionDialog } from './publish-extension-dialog'; -import { UserExtensionList } from './user-extension-list'; +import { ExtensionCardList } from '../../components/extension/extension-card-list'; +import { UserSettingsRoutes } from './user-settings-routes'; import { isError } from '../../extension-registry-types'; import { DelayedLoadIndicator } from '../../components/delayed-load-indicator'; import { MainContext } from '../../context'; @@ -82,7 +83,12 @@ export const UserSettingsExtensions: FunctionComponent = () => { {extensions && extensions.length > 0 ? ( - + ) : ( You haven't published any extensions yet. )} diff --git a/webui/src/pages/user/user-settings-namespace-detail.tsx b/webui/src/pages/user/user-settings-namespace-detail.tsx deleted file mode 100644 index 5c1fab944..000000000 --- a/webui/src/pages/user/user-settings-namespace-detail.tsx +++ /dev/null @@ -1,181 +0,0 @@ -/******************************************************************************** - * Copyright (c) 2020 TypeFox and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v. 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0. - * - * SPDX-License-Identifier: EPL-2.0 - ********************************************************************************/ - -import { FunctionComponent, createContext, useState } from 'react'; -import { useLocation } from 'react-router-dom'; -import { Box, Button, Link, Paper, Grid, Typography } from '@mui/material'; -import { styled, Theme } from '@mui/material/styles'; -import WarningIcon from '@mui/icons-material/Warning'; -import { UserNamespaceExtensionListContainer } from './user-namespace-extension-list'; -import { AdminDashboardRoutes } from '../admin-dashboard/admin-dashboard-routes'; -import { Namespace, UserData } from '../../extension-registry-types'; -import { NamespaceChangeDialog } from '../admin-dashboard/namespace-change-dialog'; -import { NamespaceDeleteDialog } from '../admin-dashboard/namespace-delete-dialog'; -import { UserNamespaceMemberList } from './user-namespace-member-list'; -import { UserNamespaceDetails } from './user-namespace-details'; - -export interface NamespaceDetailConfig { - defaultMemberRole?: 'contributor' | 'owner'; -} - -// eslint-disable-next-line react-refresh/only-export-components -export const NamespaceDetailConfigContext = createContext({}); - -const NamespaceDetailContainer = styled(Grid)(({ theme }: { theme: Theme }) => ({ - flex: 5, - padding: theme.spacing(0, 1), - [theme.breakpoints.only('md')]: { - width: '80%' - }, - [theme.breakpoints.down('sm')]: { - width: '100%' - } -})); - -const WarningPaper = styled(Paper)(({ theme }: { theme: Theme }) => ({ - maxWidth: '800px', - margin: `0 ${theme.spacing(6)} ${theme.spacing(4)} ${theme.spacing(6)}`, - padding: theme.spacing(2), - display: 'flex', - [theme.breakpoints.down('sm')]: { - margin: `0 0 ${theme.spacing(2)} 0` - } -})); - -const NamespaceHeader = styled(Box)(({ theme }: { theme: Theme }) => ({ - display: 'flex', - justifyContent: 'space-between', - alignItems: 'center', - marginBottom: theme.spacing(1), - [theme.breakpoints.down('sm')]: { - flexDirection: 'column', - alignItems: 'center' - } -})); - -export const NamespaceDetail: FunctionComponent = props => { - const [changeDialogIsOpen, setChangeDialogIsOpen] = useState(false); - const [deleteDialogIsOpen, setDeleteDialogIsOpen] = useState(false); - const { pathname } = useLocation(); - - const handleCloseChangeDialog = async () => { - setChangeDialogIsOpen(false); - }; - const handleOpenChangeDialog = () => { - setChangeDialogIsOpen(true); - }; - - const handleCloseDeleteDialog = async () => { - setDeleteDialogIsOpen(false); - }; - const handleDeletedNamespace = async () => { - setDeleteDialogIsOpen(false); - if (props.onDelete !== undefined) { - props.onDelete(); - } - }; - - const handleOpenDeleteDialog = () => { - setDeleteDialogIsOpen(true); - }; - const warningColor = props.theme === 'dark' ? '#fff' : '#151515'; - return ( - <> - - {!props.namespace.verified && props.namespaceAccessUrl ? ( - - - - - This namespace is not verified.{' '} - - See the documentation - {' '} - to learn about claiming namespaces. - - - - ) : null} - - - {props.namespace.name} - {pathname.startsWith(AdminDashboardRoutes.NAMESPACE_ADMIN) ? ( - - - {Object.keys(props.namespace.extensions).length === 0 && ( - - )} - - ) : null} - - - {props.namespace.membersUrl ? ( - - - - ) : null} - {props.namespace.detailsUrl ? ( - - - - ) : null} - - - - - - - - ); -}; - -export interface NamespaceDetailProps { - namespace: Namespace; - filterUsers: (user: UserData) => boolean; - fixSelf: boolean; - setLoadingState: (loading: boolean) => void; - namespaceAccessUrl?: string; - theme?: string; - onDelete?: () => void; -} diff --git a/webui/src/pages/user/user-settings-namespaces.tsx b/webui/src/pages/user/user-settings-namespaces.tsx index 6a8221fc4..88b8c1bb6 100644 --- a/webui/src/pages/user/user-settings-namespaces.tsx +++ b/webui/src/pages/user/user-settings-namespaces.tsx @@ -13,8 +13,9 @@ import { Box, Typography, Tabs, Tab, useTheme, useMediaQuery, Link } from '@mui/ import { Namespace, UserData } from '../../extension-registry-types'; import { DelayedLoadIndicator } from '../../components/delayed-load-indicator'; import { MainContext } from '../../context'; -import { NamespaceDetail } from './user-settings-namespace-detail'; +import { NamespaceDetailView } from '../../components/namespace/namespace-detail-view'; import { CreateNamespaceDialog } from './create-namespace-dialog'; +import { UserSettingsRoutes } from './user-settings-routes'; interface NamespaceTabProps { chosenNamespace: Namespace; @@ -111,7 +112,7 @@ export const UserSettingsNamespaces: FunctionComponent = () => { namespaces={namespaces} onChange={handleChangeNamespace} /> - setLoading(loading)} filterUsers={(foundUser: UserData) => @@ -120,6 +121,7 @@ export const UserSettingsNamespaces: FunctionComponent = () => { fixSelf={true} namespaceAccessUrl={namespaceAccessUrl} theme={pageSettings.themeType} + extensionRoutePrefix={UserSettingsRoutes.EXTENSIONS} /> ); From 32d4bce288b8e810dd7bc8c52c3d3363cc6df9ec Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 13:48:19 +0200 Subject: [PATCH 02/17] fix smoke tests --- .../eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java | 1 + 1 file changed, 1 insertion(+) 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 60c5a0dd1..210bf071e 100644 --- a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java +++ b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java @@ -202,6 +202,7 @@ void testExecuteQueries() { () -> repositories.findActiveVersions(extension), () -> repositories.findAdminStatisticsByYearAndMonth(1997, 1), () -> repositories.findAllActiveExtensions(), + () -> repositories.findAllExtensionNames(namespace), () -> repositories.findAllPersistedLogs(), () -> repositories.findPersistedLogsAfter(NOW), () -> repositories.findPersistedLogsPaginated(page), From 3dc72f368ac101a74335bb70b2c5fd42188c4c39 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 14:32:29 +0200 Subject: [PATCH 03/17] apply suggestions from review to safe-guard against soft-deleted versions --- .../eclipse/openvsx/admin/AdminService.java | 4 +++ .../GenerateKeyPairJobRequestHandler.java | 6 ++++ .../PublishExtensionVersionService.java | 28 +++++++++++++++++-- .../ExtensionVersionRepository.java | 4 +-- .../repositories/RepositoryService.java | 13 +++++---- 5 files changed, 44 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 336d3147c..7d24f92a3 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -403,6 +403,10 @@ public UserPublishInfoJson getUserPublishInfo(String provider, String loginName) var json = latest.toExtensionJson(); json.setPreview(latest.isPreview()); json.setActive(latest.getExtension().isActive()); + // findLatestVersions(user) includes inactive versions, which may be soft-deleted + // tombstones; surface that so the admin UI can distinguish removed from merely + // deactivated (mirrors UserAPI.getOwnExtensions and AdminAPI.getExtension). + json.setRemoved(latest.isRemoved()); json.setFiles(fileUrls.get(latest.getId())); return json; diff --git a/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobRequestHandler.java index 41c7bf45a..df6b00226 100644 --- a/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobRequestHandler.java @@ -108,6 +108,12 @@ private void deleteKeyPairs() { } private void enqueueCreateSignatureJob(ExtensionVersion extVersion) { + // Soft-deleted versions are tombstones whose files have been stripped from storage, so there is + // nothing to sign. Skip them here rather than enqueuing a job that would find no download and no-op. + if (extVersion.isRemoved()) { + return; + } + var handler = ExtensionVersionSignatureJobRequestHandler.class; scheduler.enqueue(new MigrationJobRequest<>(handler, extVersion.getId())); } diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java index f93acd417..7ff01e77c 100644 --- a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java @@ -11,6 +11,8 @@ import jakarta.persistence.EntityManager; import jakarta.transaction.Transactional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.cache.annotation.CacheEvict; import org.springframework.resilience.annotation.Retryable; import org.springframework.stereotype.Component; @@ -20,6 +22,7 @@ import org.eclipse.openvsx.entities.FileResource; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.storage.StorageUtilService; +import org.eclipse.openvsx.util.NamingUtil; import org.eclipse.openvsx.util.TempFile; import static org.eclipse.openvsx.cache.CacheService.CACHE_SITEMAP; @@ -27,6 +30,8 @@ @Component public class PublishExtensionVersionService { + private static final Logger logger = LoggerFactory.getLogger(PublishExtensionVersionService.class); + private final RepositoryService repositories; private final EntityManager entityManager; private final StorageUtilService storageUtil; @@ -76,8 +81,25 @@ public void markExtensionAsPotentiallyMalicious(ExtensionVersion extVersion) { @Transactional @CacheEvict(value = CACHE_SITEMAP, allEntries = true) public void activateExtension(ExtensionVersion extVersion, ExtensionService extensions) { - extVersion.setActive(true); - extVersion = entityManager.merge(extVersion); - extensions.updateExtension(extVersion.getExtension()); + // Reload the current row before mutating: the passed-in entity may carry a stale snapshot (e.g. it + // was fetched before a concurrent soft-delete committed), so we must not trust its flags. This + // matters when a version is soft-deleted while an asynchronous scan for it is still in flight: the + // scan completing (or being allowed by an admin) must not resurrect the removed version, whose files + // have already been stripped from storage. + var current = entityManager.find(ExtensionVersion.class, extVersion.getId()); + if (current == null) { + // The row was purged (hard-deleted) in the meantime; nothing to activate. + logger.warn("Refusing to activate missing extension version: {}", NamingUtil.toLogFormat(extVersion)); + return; + } + + // A soft-deleted version is a permanent tombstone and must never be reactivated. + if (current.isRemoved()) { + logger.warn("Refusing to activate removed extension version: {}", NamingUtil.toLogFormat(current)); + return; + } + + current.setActive(true); + extensions.updateExtension(current.getExtension()); } } diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionRepository.java index 8bbe592e1..dffe71870 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionRepository.java @@ -76,13 +76,13 @@ Streamable findByVersionAndExtensionNameIgnoreCaseAndExtension @Query("update ExtensionVersion ev set ev.signatureKeyPair = null") void setKeyPairsNull(); - Page findByExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( + Page findByActiveTrueAndExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( String extension, String namespace, Pageable page ); - Page findByTargetPlatformAndExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( + Page findByActiveTrueAndTargetPlatformAndExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( String targetPlatform, String extension, String namespace, 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 2d1187171..c14b07ada 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java @@ -286,7 +286,7 @@ public Streamable findActiveVersions(Extension extension) { } public Page findActiveVersionsSorted(String namespace, String extension, PageRequest page) { - return extensionVersionRepo.findByExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( + return extensionVersionRepo.findByActiveTrueAndExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( extension, namespace, page.withSort(VERSIONS_SORT)); @@ -298,11 +298,12 @@ public Page findActiveVersionsSorted( String targetPlatform, PageRequest page ) { - return extensionVersionRepo.findByTargetPlatformAndExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( - targetPlatform, - extension, - namespace, - page.withSort(VERSIONS_SORT)); + return extensionVersionRepo + .findByActiveTrueAndTargetPlatformAndExtensionNameIgnoreCaseAndExtensionNamespaceNameIgnoreCase( + targetPlatform, + extension, + namespace, + page.withSort(VERSIONS_SORT)); } public Page findActiveVersionStringsSorted( From c867ab9c848dfc7d109c5ea4f09a39a848d6dfef Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 14:48:33 +0200 Subject: [PATCH 04/17] only return file urls for active versions --- .../repositories/ExtensionVersionJooqRepository.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java index 4720bbe35..5b3f1beed 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java @@ -695,7 +695,11 @@ public List findVersionsForUrls(Extension extension, String ta query.addFrom(EXTENSION_VERSION); query.addConditions( EXTENSION_VERSION.EXTENSION_ID.eq(extension.getId()), - EXTENSION_VERSION.VERSION.eq(version)); + EXTENSION_VERSION.VERSION.eq(version), + // Only active versions may be offered for download. A soft-deleted (removed) version is always + // inactive and has had its files stripped from storage, so it must never appear in the public + // download URL map (this also avoids spurious "Could not find download" warnings for tombstones). + EXTENSION_VERSION.ACTIVE.eq(true)); if (targetPlatform != null) { query.addConditions(EXTENSION_VERSION.TARGET_PLATFORM.eq(targetPlatform)); } From 6f0b67b41ff384642ef62d05f7a2d7b66b28b2ca Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 15:42:12 +0200 Subject: [PATCH 05/17] fix view in marketplace button --- webui/src/components/extension/extension-detail-view.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webui/src/components/extension/extension-detail-view.tsx b/webui/src/components/extension/extension-detail-view.tsx index 5f8deda07..5b3cc94c4 100644 --- a/webui/src/components/extension/extension-detail-view.tsx +++ b/webui/src/components/extension/extension-detail-view.tsx @@ -53,7 +53,7 @@ export const ExtensionDetailView: FunctionComponent = - {extension.active ?? ( + {extension.active && ( From 346fd59aed3615e70dfccb9da51046bc06d9ca17 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 15:49:10 +0200 Subject: [PATCH 06/17] fix duplicate effect on namespace-extension-list --- .../namespace/namespace-extension-list.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/webui/src/components/namespace/namespace-extension-list.tsx b/webui/src/components/namespace/namespace-extension-list.tsx index 7a6f8d754..23d214157 100644 --- a/webui/src/components/namespace/namespace-extension-list.tsx +++ b/webui/src/components/namespace/namespace-extension-list.tsx @@ -11,7 +11,7 @@ * SPDX-License-Identifier: EPL-2.0 *****************************************************************************/ -import { FunctionComponent, useContext, useEffect, useRef, useState } from 'react'; +import { FunctionComponent, useContext, useEffect, useState } from 'react'; import { Typography } from '@mui/material'; import { Namespace, isError, Extension, ErrorResult } from '../../extension-registry-types'; import { MainContext } from '../../context'; @@ -43,19 +43,19 @@ export const NamespaceExtensionList: FunctionComponent context.service.getExtensionDetail(abortController, extension.url)); - const abortController = useRef(new AbortController()); - useEffect(() => { - updateExtensions(); - return () => abortController.current.abort(); - }, []); - + // A single effect keyed on the namespace name owns the fetch lifecycle: it resets state, loads the + // extensions, and creates a fresh AbortController each run so that switching namespaces (or unmounting) + // aborts the previous namespace's in-flight requests instead of letting a stale response overwrite the + // current one. (Aborts are ignored by the error handler, so this never surfaces a spurious error.) useEffect(() => { + const abortController = new AbortController(); setExtensions(undefined); setLoading(true); - updateExtensions(); + updateExtensions(abortController); + return () => abortController.abort(); }, [props.namespace.name]); - const updateExtensions = async (): Promise => { + const updateExtensions = async (abortController: AbortController): Promise => { const entries = Object.keys(props.namespace.extensions).map((name: string) => ({ name, url: props.namespace.extensions[name] @@ -63,7 +63,7 @@ export const NamespaceExtensionList: FunctionComponent { try { - const result = await fetchExtension(abortController.current, entry); + const result = await fetchExtension(abortController, entry); if (isError(result)) { throw result; } From 803a1ad4835e89f51c4ed570d60cf82f478d3f39 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 20:04:39 +0200 Subject: [PATCH 07/17] add more tests --- .../eclipse/openvsx/ExtensionServiceTest.java | 26 ++ .../openvsx/ExtensionSoftDeleteTest.java | 326 ++++++++++++++++++ .../PublishExtensionVersionServiceTest.java | 115 ++++++ 3 files changed, 467 insertions(+) create mode 100644 server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java create mode 100644 server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionServiceTest.java diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java index 8e360e0bc..a1ac366db 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java @@ -137,6 +137,32 @@ void shouldReactivateExtensionsWithQuarantinedScansAndAllowed() { assertThat(extVersion.isActive()).isTrue(); } + @Test + void shouldNotReactivateRemovedVersions() { + var user = mockUser(); + var ext = mockExtension(); + + // A soft-deleted (removed) version is a permanent tombstone: even though it is inactive (and would + // otherwise be a reactivation candidate), it must never be brought back to life. + var extVersion = new ExtensionVersion(); + extVersion.setId(1L); + extVersion.setVersion("1.1.0"); + extVersion.setTargetPlatform("linux"); + extVersion.setActive(false); + extVersion.setRemoved(true); + extVersion.setExtension(ext); + ext.getVersions().add(extVersion); + + Mockito.when(repositories.findVersionsByUser(user, false)).thenReturn(Streamable.of(extVersion)); + + svc.reactivateExtensions(user); + + assertThat(extVersion.isActive()).isFalse(); + assertThat(ext.isActive()).isFalse(); + // A tombstone is rejected up front, so its scan state is never even consulted. + Mockito.verify(repositories, Mockito.never()).findLatestExtensionScan(Mockito.any()); + } + // ---------- UTILITY ----------// private Extension mockExtension() { diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java new file mode 100644 index 000000000..e4882057b --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java @@ -0,0 +1,326 @@ +/******************************************************************************** + * Copyright (c) 2026 Eclipse Foundation and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx; + +import java.time.LocalDateTime; +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.data.domain.PageRequest; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +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.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; + +/** + * Integration tests for the soft-delete (immutable version) feature in {@link ExtensionService}. + *

+ * A "deleted" extension version is soft-deleted: its row is kept as a permanent tombstone (marked + * {@code removed} and inactive, with its files stripped from storage) so the version identity stays + * reserved and can never be republished. Only a purge physically removes the row and frees the identity. + * These tests exercise the end-to-end behaviour against a real database, as well as the query paths that + * must exclude tombstones from public surfaces. + */ +@SpringBootTest( + properties = { + "ovsx.elasticsearch.enabled=false" + } +) +@ActiveProfiles("test_db") +class ExtensionSoftDeleteTest { + + private static final String NAMESPACE = "soft-delete-testns"; + private static final String EXTENSION = "soft-delete-testext"; + private static final String OWNER_LOGIN = "soft-delete-owner"; + + @Autowired + ExtensionService extensionService; + + @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); + 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; + } + + /** + * Deleting a version while another exists soft-deletes it: the row survives as a tombstone that is + * marked removed, inactive, and stamped with the deleting user and a timestamp. + */ + @Test + void deleteExtensionVersion_softDeletesKeepingTombstone() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); + + assertThat(versionExists("1.0.0")) + .as("a soft-deleted version's row must be kept as an immutable tombstone") + .isTrue(); + assertThat(versionRemoved("1.0.0")) + .as("a soft-deleted version must be marked removed and inactive") + .isTrue(); + assertThat(removedByOf("1.0.0")) + .as("a soft-deleted version records who removed it") + .isEqualTo(ownerId); + assertThat(removedTimestampOf("1.0.0")) + .as("a soft-deleted version records when it was removed") + .isNotNull(); + assertThat(versionRemoved("2.0.0")) + .as("an untouched version must stay live") + .isFalse(); + } + + /** + * Deleting an already-removed version is an idempotent no-op: it neither fails nor re-stamps the + * tombstone (the original removal metadata is preserved). + */ + @Test + void deleteExtensionVersion_isIdempotentForAlreadyRemoved() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); + var firstTimestamp = removedTimestampOf("1.0.0"); + + // A second delete of the same version must not throw and must not touch the tombstone. + extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); + + assertThat(versionRemoved("1.0.0")).isTrue(); + assertThat(removedTimestampOf("1.0.0")) + .as("re-deleting a tombstone must not re-stamp its removal timestamp") + .isEqualTo(firstTimestamp); + } + + /** + * Purging permanently removes the version row (unlike soft-delete), freeing the identity. + */ + @Test + void purgeExtensionVersion_physicallyRemovesRow() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + extensionService.purgeExtension(owner(), false, NAMESPACE, EXTENSION, targets); + + assertThat(versionExists("1.0.0")) + .as("a purged version's row must be physically removed") + .isFalse(); + assertThat(versionExists("2.0.0")) + .as("an untouched version must survive the purge") + .isTrue(); + } + + /** + * Soft-deleting then purging the same version leaves no row behind, freeing the identity. + */ + @Test + void softDeleteThenPurge_removesTombstone() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); + assertThat(versionRemoved("1.0.0")).isTrue(); + + extensionService.purgeExtension(owner(), false, NAMESPACE, EXTENSION, targets); + assertThat(versionExists("1.0.0")) + .as("purging a tombstone must physically remove its row") + .isFalse(); + } + + /** + * Fix #2: the version listing used by the public {@code /versions} API must never surface tombstones. + */ + @Test + void findActiveVersionsSorted_excludesRemovedVersions() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, false, true); + + var page = repositories.findActiveVersionsSorted(NAMESPACE, EXTENSION, PageRequest.of(0, 10)); + var versions = page.getContent().stream().map(ExtensionVersion::getVersion).toList(); + + assertThat(versions) + .as("the public versions listing must exclude soft-deleted versions") + .contains("1.0.0") + .doesNotContain("2.0.0"); + } + + /** + * Fix #3: the download URL map must never surface tombstones (whose files are gone anyway). + */ + @Test + void findVersionsForUrls_excludesRemovedVersions() { + // Same version string, two target platforms: the universal one is live, the linux one is removed. + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("1.0.0", TargetPlatform.NAME_LINUX_X64, false, true); + + var extension = repositories.findExtension(EXTENSION, NAMESPACE); + var forUrls = repositories.findVersionsForUrls(extension, null, "1.0.0"); + var platforms = forUrls.stream().map(ExtensionVersion::getTargetPlatform).toList(); + + assertThat(platforms) + .as("the download map must only include active target platforms, never tombstones") + .containsExactly(TargetPlatform.NAME_UNIVERSAL); + } + + 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 boolean versionExists(String version) { + return count( + "select ev.id from ExtensionVersion ev where ev.version = :version " + + "and ev.extension.namespace.name = :namespace", + version) > 0; + } + + private boolean versionRemoved(String version) { + return count( + "select ev.id from ExtensionVersion ev where ev.version = :version " + + "and ev.extension.namespace.name = :namespace and ev.removed = true and ev.active = false", + version) > 0; + } + + private Long removedByOf(String version) { + return new TransactionTemplate(txManager).execute( + status -> em.createQuery( + "select ev.removedBy.id from ExtensionVersion ev where ev.version = :version " + + "and ev.extension.namespace.name = :namespace", + Long.class) + .setParameter("version", version) + .setParameter("namespace", NAMESPACE) + .getSingleResult()); + } + + private LocalDateTime removedTimestampOf(String version) { + return new TransactionTemplate(txManager).execute( + status -> em.createQuery( + "select ev.removedTimestamp from ExtensionVersion ev where ev.version = :version " + + "and ev.extension.namespace.name = :namespace", + LocalDateTime.class) + .setParameter("version", version) + .setParameter("namespace", NAMESPACE) + .getSingleResult()); + } + + private long count(String jpql, String version) { + return new TransactionTemplate(txManager).execute( + status -> (long) em.createQuery(jpql) + .setParameter("version", version) + .setParameter("namespace", NAMESPACE) + .getResultList() + .size()); + } + + @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/publish/PublishExtensionVersionServiceTest.java b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionServiceTest.java new file mode 100644 index 000000000..553654b88 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionServiceTest.java @@ -0,0 +1,115 @@ +/****************************************************************************** + * 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.publish; + +import jakarta.persistence.EntityManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +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.repositories.RepositoryService; +import org.eclipse.openvsx.storage.StorageUtilService; +import org.eclipse.openvsx.util.TargetPlatform; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link PublishExtensionVersionService#activateExtension}, focusing on the soft-delete + * guard: a removed version is an immutable tombstone and must never be reactivated, even though it is + * inactive and could otherwise be a reactivation candidate. This can happen when a version is soft-deleted + * while an asynchronous scan for it is still in flight and the scan later completes (or is allowed by an + * admin) and tries to activate it. + */ +@ExtendWith(MockitoExtension.class) +class PublishExtensionVersionServiceTest { + + @Mock + RepositoryService repositories; + @Mock + EntityManager entityManager; + @Mock + StorageUtilService storageUtil; + @Mock + ExtensionService extensions; + + private PublishExtensionVersionService svc; + + @BeforeEach + void setUp() { + svc = new PublishExtensionVersionService(repositories, entityManager, storageUtil); + } + + @Test + void activateExtension_activatesLiveVersion() { + var extVersion = version(1L, false); + when(entityManager.find(ExtensionVersion.class, 1L)).thenReturn(extVersion); + + svc.activateExtension(extVersion, extensions); + + assertThat(extVersion.isActive()).isTrue(); + verify(extensions).updateExtension(extVersion.getExtension()); + } + + @Test + void activateExtension_refusesRemovedVersion() { + var extVersion = version(1L, true); + when(entityManager.find(ExtensionVersion.class, 1L)).thenReturn(extVersion); + + svc.activateExtension(extVersion, extensions); + + assertThat(extVersion.isActive()) + .as("a soft-deleted tombstone must never be reactivated") + .isFalse(); + verify(extensions, never()).updateExtension(any()); + } + + @Test + void activateExtension_refusesMissingVersion() { + // The row was purged (hard-deleted) between fetch and activation. + var extVersion = version(1L, false); + when(entityManager.find(ExtensionVersion.class, 1L)).thenReturn(null); + + svc.activateExtension(extVersion, extensions); + + assertThat(extVersion.isActive()).isFalse(); + verify(extensions, never()).updateExtension(any()); + } + + private ExtensionVersion version(long id, boolean removed) { + var namespace = new Namespace(); + namespace.setName("redhat"); + + var extension = new Extension(); + extension.setName("vscode-yaml"); + extension.setNamespace(namespace); + + var extVersion = new ExtensionVersion(); + extVersion.setId(id); + extVersion.setVersion("1.0.0"); + extVersion.setTargetPlatform(TargetPlatform.NAME_UNIVERSAL); + extVersion.setActive(false); + extVersion.setRemoved(removed); + extVersion.setExtension(extension); + return extVersion; + } +} From 227b6e660050f81c2cb678c8ab7dc44798484cfd Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 22 Jul 2026 21:50:45 +0200 Subject: [PATCH 08/17] integration test improvements: share db container, use tags, replace deprecated test container --- server/build.gradle | 23 ++++----- .../AbstractPostgresContainerTest.java | 49 +++++++++++++++++++ .../eclipse/openvsx/ExtensionDeleteTest.java | 10 +--- .../openvsx/ExtensionSoftDeleteTest.java | 11 +---- .../org/eclipse/openvsx/IntegrationTest.java | 4 +- .../eclipse/openvsx/TestDatabaseConfig.java | 30 ------------ .../openvsx/cache/CacheServiceTest.java | 10 ++-- .../ratelimit/RateLimitIntegrationTest.java | 9 ++-- .../repositories/NamespaceRepositoryTest.java | 11 ++--- .../RepositoryServiceSmokeTest.java | 11 ++--- .../AwsStorageServiceIntegrationTest.java | 17 ++++--- .../resources/application-test_search.yml | 5 ++ server/src/test/resources/application.yml | 5 ++ 13 files changed, 101 insertions(+), 94 deletions(-) create mode 100644 server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java delete mode 100644 server/src/test/java/org/eclipse/openvsx/TestDatabaseConfig.java create mode 100644 server/src/test/resources/application-test_search.yml diff --git a/server/build.gradle b/server/build.gradle index cf47a6c72..8c150dff8 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -265,18 +265,16 @@ test { } tasks.register('unitTests', Test) { - description = 'Runs unit tests (excluding integration tests).' + description = 'Runs unit tests (excluding container-backed integration tests).' group = 'verification' testClassesDirs = sourceSets.test.output.classesDirs classpath = sourceSets.test.runtimeClasspath - useJUnitPlatform() - exclude 'org/eclipse/openvsx/ExtensionDeleteTest.class' - exclude 'org/eclipse/openvsx/IntegrationTest.class' - exclude 'org/eclipse/openvsx/cache/CacheServiceTest.class' - exclude 'org/eclipse/openvsx/ratelimit/RateLimitIntegrationTest.class' - exclude 'org/eclipse/openvsx/repositories/NamespaceRepositoryTest.class' - exclude 'org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.class' - exclude 'org/eclipse/openvsx/storage/AwsStorageServiceIntegrationTest.class' + // Exclude container-backed tests by tag rather than a hand-maintained class list: any test that + // extends AbstractPostgresContainerTest (or is otherwise tagged "integration") is skipped here, so + // this fast task never spins up a Docker container. New integration tests are covered automatically. + useJUnitPlatform { + excludeTags 'integration' + } } tasks.register('s3IntegrationTests', Test) { @@ -284,8 +282,11 @@ tasks.register('s3IntegrationTests', Test) { group = 'verification' testClassesDirs = sourceSets.test.output.classesDirs classpath = sourceSets.test.runtimeClasspath - useJUnitPlatform() - include 'org/eclipse/openvsx/storage/AwsStorageServiceIntegrationTest.class' + // Select S3 integration tests by tag rather than a hardcoded class name, so new tests tagged "s3" + // are picked up automatically. + useJUnitPlatform { + includeTags 's3' + } // Set system properties for test configuration systemProperty 'spring.profiles.active', 's3-integration' diff --git a/server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java b/server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java new file mode 100644 index 000000000..a030feeda --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java @@ -0,0 +1,49 @@ +/****************************************************************************** + * 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; + +import org.junit.jupiter.api.Tag; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.postgresql.PostgreSQLContainer; + +/** + * Base class for tests that need a PostgreSQL database. + *

+ * The container is a JVM-wide singleton: it is started exactly once (in the static initializer) and + * shared by every test context, instead of being a context-scoped {@code @ServiceConnection} bean that + * Spring would start and stop for each distinct application context. With half a dozen distinct + * {@code @SpringBootTest} context configurations this turned six Postgres startups (and six full Flyway + * migration runs) into one. Testcontainers' Ryuk sidecar stops the container when the JVM exits, so no + * explicit shutdown is required. + *

+ * Because all contexts now share a single database, tests must keep cleaning up after themselves (via + * transactional rollback or an explicit tear-down) and use unique identifiers, exactly as they already + * had to when sharing a context. + */ +@Tag("integration") +public abstract class AbstractPostgresContainerTest { + + static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer("postgres:16.2"); + + static { + POSTGRES.start(); + } + + @DynamicPropertySource + static void datasourceProperties(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java index 45733edcd..e37e021ec 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java @@ -22,7 +22,6 @@ 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.ActiveProfiles; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; import org.springframework.transaction.PlatformTransactionManager; @@ -57,13 +56,8 @@ * {@link ExtensionService#deleteUserExtension(UserData, String, String, TargetPlatformVersion...)} * is protected by a {@code SELECT … FOR UPDATE NOWAIT} lock and therefore passes. */ -@SpringBootTest( - properties = { - "ovsx.elasticsearch.enabled=false" - } -) -@ActiveProfiles("test_db") -class ExtensionDeleteTest { +@SpringBootTest +class ExtensionDeleteTest extends AbstractPostgresContainerTest { private static final String NAMESPACE = "race-testns"; private static final String EXTENSION = "race-testext"; diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java index e4882057b..003df0c96 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java @@ -10,7 +10,6 @@ package org.eclipse.openvsx; import java.time.LocalDateTime; -import java.util.List; import jakarta.persistence.EntityManager; import org.jobrunr.scheduling.JobRequestScheduler; @@ -20,7 +19,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.data.domain.PageRequest; -import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.support.TransactionTemplate; @@ -46,13 +44,8 @@ * These tests exercise the end-to-end behaviour against a real database, as well as the query paths that * must exclude tombstones from public surfaces. */ -@SpringBootTest( - properties = { - "ovsx.elasticsearch.enabled=false" - } -) -@ActiveProfiles("test_db") -class ExtensionSoftDeleteTest { +@SpringBootTest +class ExtensionSoftDeleteTest extends AbstractPostgresContainerTest { private static final String NAMESPACE = "soft-delete-testns"; private static final String EXTENSION = "soft-delete-testext"; diff --git a/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java b/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java index 79222f0d4..fab7457a7 100644 --- a/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java +++ b/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java @@ -33,8 +33,8 @@ @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) @AutoConfigureTestRestTemplate -@ActiveProfiles({ "test", "test_db", "test_search" }) -class IntegrationTest { +@ActiveProfiles({ "test", "test_search" }) +class IntegrationTest extends AbstractPostgresContainerTest { protected final Logger logger = LoggerFactory.getLogger(IntegrationTest.class); diff --git a/server/src/test/java/org/eclipse/openvsx/TestDatabaseConfig.java b/server/src/test/java/org/eclipse/openvsx/TestDatabaseConfig.java deleted file mode 100644 index 12d20ab5a..000000000 --- a/server/src/test/java/org/eclipse/openvsx/TestDatabaseConfig.java +++ /dev/null @@ -1,30 +0,0 @@ -/****************************************************************************** - * 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; - -import org.springframework.boot.testcontainers.service.connection.ServiceConnection; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Profile; -import org.testcontainers.postgresql.PostgreSQLContainer; - -@Configuration -@Profile("test_db") -public class TestDatabaseConfig { - - @Bean - @ServiceConnection - PostgreSQLContainer postgreSQLContainer() { - return new PostgreSQLContainer("postgres:16.2"); - } -} diff --git a/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java b/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java index 2c0b9f9de..1c7c48d35 100644 --- a/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java @@ -31,6 +31,7 @@ import org.springframework.web.context.request.RequestContextHolder; import org.springframework.web.context.request.ServletRequestAttributes; +import org.eclipse.openvsx.AbstractPostgresContainerTest; import org.eclipse.openvsx.ExtensionService; import org.eclipse.openvsx.LocalRegistryService; import org.eclipse.openvsx.UserService; @@ -51,13 +52,10 @@ import static org.junit.jupiter.api.Assertions.*; @SpringBootTest( - webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, - properties = { - "ovsx.elasticsearch.enabled=false" - } + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT ) -@ActiveProfiles({ "test", "test_db" }) -class CacheServiceTest { +@ActiveProfiles("test") +class CacheServiceTest extends AbstractPostgresContainerTest { @Autowired CacheManager cache; diff --git a/server/src/test/java/org/eclipse/openvsx/ratelimit/RateLimitIntegrationTest.java b/server/src/test/java/org/eclipse/openvsx/ratelimit/RateLimitIntegrationTest.java index 416041da1..2635c3875 100644 --- a/server/src/test/java/org/eclipse/openvsx/ratelimit/RateLimitIntegrationTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ratelimit/RateLimitIntegrationTest.java @@ -27,10 +27,11 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.test.web.server.LocalServerPort; -import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.bean.override.mockito.MockitoBean; import redis.clients.jedis.RedisClusterClient; +import org.eclipse.openvsx.AbstractPostgresContainerTest; + import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -38,13 +39,11 @@ webEnvironment = WebEnvironment.RANDOM_PORT, properties = { "ovsx.rate-limit.enabled=true", - "ovsx.rate-limit.filters[0].url=/(api|vscode)/.*", - "ovsx.elasticsearch.enabled=false" + "ovsx.rate-limit.filters[0].url=/(api|vscode)/.*" } ) @AutoConfigureTestRestTemplate -@ActiveProfiles("test_db") -class RateLimitIntegrationTest { +class RateLimitIntegrationTest extends AbstractPostgresContainerTest { @LocalServerPort int port; diff --git a/server/src/test/java/org/eclipse/openvsx/repositories/NamespaceRepositoryTest.java b/server/src/test/java/org/eclipse/openvsx/repositories/NamespaceRepositoryTest.java index dd6894296..d7c4d3c00 100644 --- a/server/src/test/java/org/eclipse/openvsx/repositories/NamespaceRepositoryTest.java +++ b/server/src/test/java/org/eclipse/openvsx/repositories/NamespaceRepositoryTest.java @@ -18,20 +18,15 @@ 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.ActiveProfiles; +import org.eclipse.openvsx.AbstractPostgresContainerTest; import org.eclipse.openvsx.entities.Namespace; import static org.assertj.core.api.Assertions.assertThat; -@SpringBootTest( - properties = { - "ovsx.elasticsearch.enabled=false" - } -) -@ActiveProfiles("test_db") +@SpringBootTest @Transactional -class NamespaceRepositoryTest { +class NamespaceRepositoryTest extends AbstractPostgresContainerTest { @Autowired NamespaceRepository repo; 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 210bf071e..ac7b86f99 100644 --- a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java +++ b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java @@ -25,8 +25,8 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; -import org.springframework.test.context.ActiveProfiles; +import org.eclipse.openvsx.AbstractPostgresContainerTest; import org.eclipse.openvsx.entities.AdminScanDecision; import org.eclipse.openvsx.entities.Customer; import org.eclipse.openvsx.entities.DailyUsageStats; @@ -56,13 +56,8 @@ * Run the DB queries and assert no DB error, just to ensure that the queries * are consistent with the schema. */ -@SpringBootTest( - properties = { - "ovsx.elasticsearch.enabled=false" - } -) -@ActiveProfiles("test_db") -class RepositoryServiceSmokeTest { +@SpringBootTest +class RepositoryServiceSmokeTest extends AbstractPostgresContainerTest { private static final List STRING_LIST = List.of("id1", "id2"); diff --git a/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceIntegrationTest.java b/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceIntegrationTest.java index 41ab83f2f..29f358716 100644 --- a/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceIntegrationTest.java +++ b/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceIntegrationTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.beans.factory.annotation.Autowired; @@ -28,9 +29,9 @@ import org.springframework.test.context.DynamicPropertySource; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.util.ReflectionTestUtils; -import org.testcontainers.containers.localstack.LocalStackContainer; import org.testcontainers.junit.jupiter.Container; import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.localstack.LocalStackContainer; import org.testcontainers.utility.DockerImageName; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; @@ -50,6 +51,8 @@ import static org.junit.jupiter.api.Assertions.*; +@Tag("integration") +@Tag("s3") @Testcontainers @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = AwsStorageServiceIntegrationTest.TestConfig.class) @@ -60,7 +63,7 @@ class AwsStorageServiceIntegrationTest { @Container static LocalStackContainer localstack = new LocalStackContainer(DockerImageName.parse("localstack/localstack:4.7")) - .withServices(LocalStackContainer.Service.S3, LocalStackContainer.Service.IAM) + .withServices("s3", "iam") .withReuse(true); @Autowired @@ -78,7 +81,7 @@ class AwsStorageServiceIntegrationTest { static void configureProperties(DynamicPropertyRegistry registry) { registry.add( "ovsx.storage.aws.service-endpoint", - () -> localstack.getEndpointOverride(LocalStackContainer.Service.S3).toString()); + () -> localstack.getEndpoint().toString()); registry.add("ovsx.storage.aws.access-key-id", () -> "test"); registry.add("ovsx.storage.aws.secret-access-key", () -> "test"); registry.add("ovsx.storage.aws.region", () -> TEST_REGION); @@ -99,11 +102,11 @@ void setUp() { ReflectionTestUtils.setField( storageService, "serviceEndpoint", - localstack.getEndpointOverride(LocalStackContainer.Service.S3).toString()); + localstack.getEndpoint().toString()); ReflectionTestUtils.setField(storageService, "pathStyleAccess", true); testS3Client = S3Client.builder() - .endpointOverride(localstack.getEndpointOverride(LocalStackContainer.Service.S3)) + .endpointOverride(localstack.getEndpoint()) .credentialsProvider( StaticCredentialsProvider.create( AwsBasicCredentials.create("test", "test"))) @@ -516,7 +519,7 @@ void testValidAuthenticationButInsufficientBucketPermissions() throws IOExceptio ReflectionTestUtils.setField( restrictedBucketService, "serviceEndpoint", - localstack.getEndpointOverride(LocalStackContainer.Service.S3).toString()); + localstack.getEndpoint().toString()); ReflectionTestUtils.setField(restrictedBucketService, "pathStyleAccess", true); ReflectionTestUtils.setField(restrictedBucketService, "s3Client", null); @@ -553,7 +556,7 @@ private AwsStorageService createStorageServiceWithCredentials( ReflectionTestUtils.setField( awsStorageService, "serviceEndpoint", - localstack.getEndpointOverride(LocalStackContainer.Service.S3).toString()); + localstack.getEndpoint().toString()); ReflectionTestUtils.setField(awsStorageService, "pathStyleAccess", true); return awsStorageService; diff --git a/server/src/test/resources/application-test_search.yml b/server/src/test/resources/application-test_search.yml new file mode 100644 index 000000000..117680775 --- /dev/null +++ b/server/src/test/resources/application-test_search.yml @@ -0,0 +1,5 @@ +# Activated together with the "test_search" profile, which also provisions the Elasticsearch +# test container (see TestSearchConfig). Tests that need search enable it by activating that profile. +ovsx: + elasticsearch: + enabled: true diff --git a/server/src/test/resources/application.yml b/server/src/test/resources/application.yml index b51f7e2ca..ddef45cba 100644 --- a/server/src/test/resources/application.yml +++ b/server/src/test/resources/application.yml @@ -35,6 +35,11 @@ jobrunr: allow-anonymous-data-usage: false ovsx: + elasticsearch: + # Disabled by default for tests: the Elasticsearch container is expensive, so only tests that + # activate the "test_search" profile (which also provisions the container) enable it, via + # application-test_search.yml. + enabled: false redis: embedded: true storage: From 4ca44ac589b3b70e9048108654f6fce4cb956f3a Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 23 Jul 2026 15:03:01 +0200 Subject: [PATCH 09/17] remove unused parameter --- .../src/main/java/org/eclipse/openvsx/ExtensionService.java | 5 ++--- .../main/java/org/eclipse/openvsx/admin/AdminService.java | 2 +- .../java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index bf752d6b5..e4b2fbda0 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -327,20 +327,19 @@ public ResultJson deleteExtension( @Transactional(rollbackOn = ErrorResultException.class) public ResultJson purgeExtension( UserData user, - boolean restrictedToUser, String namespaceName, String extensionName, TargetPlatformVersion... targetVersions ) throws ErrorResultException { var extension = lockExtensionNoWait(namespaceName, extensionName); if (repositories - .isDeleteAllVersions(restrictedToUser ? user : null, namespaceName, extensionName, targetVersions)) { + .isDeleteAllVersions(null, namespaceName, extensionName, targetVersions)) { return purgeExtension(user, extension, true); } return purgeExtensionVersions( user, - resolveVersions(user, restrictedToUser, namespaceName, extensionName, targetVersions)); + resolveVersions(user, false, namespaceName, extensionName, targetVersions)); } private List resolveVersions( diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 7d24f92a3..146782403 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -210,7 +210,7 @@ public ResultJson purgeExtensionNoWait( String extensionName, TargetPlatformVersion... targetVersions ) throws ErrorResultException { - return extensions.purgeExtension(user, false, namespaceName, extensionName, targetVersions); + return extensions.purgeExtension(user, namespaceName, extensionName, targetVersions); } @Transactional(rollbackOn = ErrorResultException.class) diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java index 003df0c96..cf5abc702 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java @@ -171,7 +171,7 @@ void purgeExtensionVersion_physicallyRemovesRow() { persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); - extensionService.purgeExtension(owner(), false, NAMESPACE, EXTENSION, targets); + extensionService.purgeExtension(owner(), NAMESPACE, EXTENSION, targets); assertThat(versionExists("1.0.0")) .as("a purged version's row must be physically removed") @@ -193,7 +193,7 @@ void softDeleteThenPurge_removesTombstone() { extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); assertThat(versionRemoved("1.0.0")).isTrue(); - extensionService.purgeExtension(owner(), false, NAMESPACE, EXTENSION, targets); + extensionService.purgeExtension(owner(), NAMESPACE, EXTENSION, targets); assertThat(versionExists("1.0.0")) .as("purging a tombstone must physically remove its row") .isFalse(); From 0411cb23c4f5eb481f1f3a267d37780b43c98081 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 23 Jul 2026 16:05:24 +0200 Subject: [PATCH 10/17] move method from AdminService to ExtensionService --- .../org/eclipse/openvsx/ExtensionService.java | 7 ++++--- .../eclipse/openvsx/admin/AdminService.java | 19 ------------------- .../openvsx/mirror/DataMirrorJobRequest.java | 2 +- .../mirror/DataMirrorJobRequestHandler.java | 10 +++++----- .../openvsx/mirror/DataMirrorService.java | 2 +- .../MirrorExtensionHandlerInterceptor.java | 7 +++---- .../MirrorExtensionQueryRequestHandler.java | 2 +- .../openvsx/ExtensionSoftDeleteTest.java | 4 ++-- 8 files changed, 17 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index e4b2fbda0..4669a3385 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -321,11 +321,12 @@ public ResultJson deleteExtension( * Unlike {@link #deleteExtension(UserData, boolean, String, String, TargetPlatformVersion...)}, which * soft-deletes versions (keeping the row so the version identity stays reserved), this physically removes * the rows and frees the version identity for republishing. It is intended for administrative purge and - * automated cleanup (mirror, extension control) and performs no user-ownership check beyond the optional - * {@code restrictedToUser} version lookup. + * automated cleanup (mirror, extension control) and performs no user-ownership check. + *

+ * The method will try to lock the extension and fail with an {@code ErrorResultException} if it can't acquire it. */ @Transactional(rollbackOn = ErrorResultException.class) - public ResultJson purgeExtension( + public ResultJson purgeExtensionNoWait( UserData user, String namespaceName, String extensionName, diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 146782403..57c5e8a76 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -194,25 +194,6 @@ public ResultJson deleteExtensionNoWait( return extensions.deleteExtension(user, false, namespaceName, extensionName, targetVersions); } - /** - * Purge (permanently delete) the provided versions of an extension. If all versions shall be purged, the - * extension as a whole will be removed unless it is referenced by bundles or used as a dependency. - *

- * Unlike {@link #deleteExtensionNoWait}, this physically removes the rows from the database and storage, - * freeing the version identity for republishing. Intended for administrative purge and automated cleanup. - *

- * The method will try to lock the extension and fail with an {@code ErrorResultException} if it can't acquire it. - */ - @Transactional(rollbackOn = ErrorResultException.class) - public ResultJson purgeExtensionNoWait( - UserData user, - String namespaceName, - String extensionName, - TargetPlatformVersion... targetVersions - ) throws ErrorResultException { - return extensions.purgeExtension(user, namespaceName, extensionName, targetVersions); - } - @Transactional(rollbackOn = ErrorResultException.class) public ResultJson deleteNamespace(String namespaceName, UserData admin) throws ErrorResultException { var namespace = repositories.findNamespace(namespaceName); diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequest.java b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequest.java index ebe180b42..6e662ed69 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequest.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequest.java @@ -14,7 +14,7 @@ public class DataMirrorJobRequest implements JobRequest { @Override - public Class getJobRequestHandler() { + public Class> getJobRequestHandler() { return DataMirrorJobRequestHandler.class; } } diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java index 0e0972872..ebb5edae4 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java @@ -31,8 +31,8 @@ import org.w3c.dom.Element; import org.w3c.dom.NodeList; +import org.eclipse.openvsx.ExtensionService; import org.eclipse.openvsx.UrlConfigService; -import org.eclipse.openvsx.admin.AdminService; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.util.ErrorResultException; @@ -50,7 +50,7 @@ public class DataMirrorJobRequestHandler implements JobRequestHandler this.data = service); this.repositories = repositories; this.backgroundRestTemplate = backgroundRestTemplate; this.urlConfigService = urlConfigService; - this.admin = admin; + this.extensions = extensions; this.mirrorExtensionService = mirrorExtensionService; this.dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); } @@ -139,7 +139,7 @@ private void deleteOtherExtensions(List extensionIds, UserData mirrorUse ThreadLocalJobContext.getJobContext().logger().info("deleting " + extensionId); try { var namespace = extension.getNamespace(); - admin.purgeExtensionNoWait(mirrorUser, namespace.getName(), extension.getName()); + extensions.purgeExtensionNoWait(mirrorUser, namespace.getName(), extension.getName()); } catch (ErrorResultException e) { if (e.getStatus() != HttpStatus.NOT_FOUND) { logger.warn("mirror: failed to delete extension {}", extensionId, e); diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java index 804dfb0bf..2658d6ae8 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java @@ -247,7 +247,7 @@ public void deleteExtensionVersion(ExtensionVersion extVersion, UserData user) { var extension = extVersion.getExtension(); // The mirror must not keep tombstones: versions dropped upstream are purged so the mirror can // re-track upstream state (and re-publish a version that later reappears upstream). - admin.purgeExtensionNoWait( + extensions.purgeExtensionNoWait( user, extension.getNamespace().getName(), extension.getName(), diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionHandlerInterceptor.java b/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionHandlerInterceptor.java index bd83de565..ecc48ee3b 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionHandlerInterceptor.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionHandlerInterceptor.java @@ -33,13 +33,12 @@ public MirrorExtensionHandlerInterceptor(DataMirrorService dataMirror) { } @Override - public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) - throws Exception { + public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { var params = request.getRequestURI().equals("/vscode/item") ? extractQueryParams(request) : extractPathParams(request); - var namespaceName = (String) params.get("namespaceName"); - var extensionName = (String) params.get("extensionName"); + var namespaceName = params.get("namespaceName"); + var extensionName = params.get("extensionName"); if (!dataMirror.match(namespaceName, extensionName)) { response.reset(); response.setStatus(HttpServletResponse.SC_NOT_FOUND); diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionQueryRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionQueryRequestHandler.java index 770067e76..dc367cfff 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionQueryRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/MirrorExtensionQueryRequestHandler.java @@ -51,7 +51,7 @@ public ExtensionQueryResult getResult(ExtensionQueryParam param, int pageSize, i return result; } return local.toQueryResult( - result.results().get(0).extensions().stream() + result.results().getFirst().extensions().stream() .filter(e -> dataMirror.match(e.publisher().publisherName(), e.extensionName())) .collect(Collectors.toList())); } catch (NotFoundException | ResponseStatusException exc) { diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java index cf5abc702..1c17a7031 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java @@ -171,7 +171,7 @@ void purgeExtensionVersion_physicallyRemovesRow() { persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); - extensionService.purgeExtension(owner(), NAMESPACE, EXTENSION, targets); + extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION, targets); assertThat(versionExists("1.0.0")) .as("a purged version's row must be physically removed") @@ -193,7 +193,7 @@ void softDeleteThenPurge_removesTombstone() { extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); assertThat(versionRemoved("1.0.0")).isTrue(); - extensionService.purgeExtension(owner(), NAMESPACE, EXTENSION, targets); + extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION, targets); assertThat(versionExists("1.0.0")) .as("purging a tombstone must physically remove its row") .isFalse(); From 42b00bcbcc9a6a784727b3b68d796690d0ccbb47 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 23 Jul 2026 16:14:00 +0200 Subject: [PATCH 11/17] adapt AdminAPI --- .../src/main/java/org/eclipse/openvsx/admin/AdminAPI.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java index fa383c5a0..d143779b1 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminAPI.java @@ -41,6 +41,7 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ResponseStatusException; +import org.eclipse.openvsx.ExtensionService; import org.eclipse.openvsx.LocalRegistryService; import org.eclipse.openvsx.entities.AdminStatistics; import org.eclipse.openvsx.entities.NamespaceMembership; @@ -76,6 +77,7 @@ public class AdminAPI { private final RepositoryService repositories; private final AdminService admins; + private final ExtensionService extensions; private final SettingsService settings; private final LogService logs; private final LocalRegistryService local; @@ -84,6 +86,7 @@ public class AdminAPI { public AdminAPI( RepositoryService repositories, AdminService admins, + ExtensionService extensions, SettingsService settings, LogService logs, LocalRegistryService local, @@ -91,6 +94,7 @@ public AdminAPI( ) { this.repositories = repositories; this.admins = admins; + this.extensions = extensions; this.settings = settings; this.logs = logs; this.local = local; @@ -540,7 +544,7 @@ public ResponseEntity purgeExtension( targetVersions, TargetPlatformVersionJson::toTargetPlatformVersion, TargetPlatformVersion[]::new); - var result = admins.purgeExtensionNoWait(adminUser, namespaceName, extensionName, targets); + var result = extensions.purgeExtensionNoWait(adminUser, namespaceName, extensionName, targets); return ResponseEntity.ok(result); } catch (ErrorResultException exc) { return exc.toResponseEntity(); @@ -584,7 +588,7 @@ public ResponseEntity purgeExtension( targetVersions, TargetPlatformVersionJson::toTargetPlatformVersion, TargetPlatformVersion[]::new); - var result = admins.purgeExtensionNoWait(adminUser, namespaceName, extensionName, targets); + var result = extensions.purgeExtensionNoWait(adminUser, namespaceName, extensionName, targets); return ResponseEntity.ok(result); } catch (ErrorResultException exc) { return exc.toResponseEntity(); From 93068205250428eaf1e41dd9db025deda29b81fa Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 23 Jul 2026 16:28:16 +0200 Subject: [PATCH 12/17] rename methods used by extension control to match intent, malicious extension should be purged, they are prohibited from uploaded again separately --- .../org/eclipse/openvsx/admin/AdminService.java | 16 ++++++++-------- .../ExtensionControlJobRequestHandler.java | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 57c5e8a76..508e62b73 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -121,13 +121,13 @@ public void applicationStarted(ApplicationStartedEvent event) { * No further checks are made if the extension is referenced by bundles or as a dependency. */ @Transactional(rollbackOn = ErrorResultException.class) - public void deleteExtensionAndDependencies(UserData admin, String namespaceName, String extensionName) + public void purgeExtensionAndDependencies(UserData admin, String namespaceName, String extensionName) throws ErrorResultException { var extension = extensions.lockExtension(namespaceName, extensionName); - deleteExtensionAndDependencies(admin, extension, 0); + purgeExtensionAndDependencies(admin, extension, 0); } - private void deleteExtensionAndDependencies(UserData admin, Extension extension, int depth) + private void purgeExtensionAndDependencies(UserData admin, Extension extension, int depth) throws ErrorResultException { if (depth > 5) { throw new ErrorResultException( @@ -137,12 +137,12 @@ private void deleteExtensionAndDependencies(UserData admin, Extension extension, var bundledRefs = repositories.findBundledExtensionsReference(extension); for (var bundledRef : bundledRefs) { - deleteExtensionAndDependencies(admin, bundledRef, depth); + purgeExtensionAndDependencies(admin, bundledRef, depth); } var dependRefs = repositories.findDependenciesReference(extension); for (var dependRef : dependRefs) { - deleteExtensionAndDependencies(admin, dependRef, depth); + purgeExtensionAndDependencies(admin, dependRef, depth); } // We unconditionally purge the extension, @@ -150,10 +150,10 @@ private void deleteExtensionAndDependencies(UserData admin, Extension extension, extensions.purgeExtension(admin, extension, false); } - private void deleteExtensionAndDependencies(UserData admin, ExtensionVersion extVersion, int depth) { + private void purgeExtensionAndDependencies(UserData admin, ExtensionVersion extVersion, int depth) { var extension = extVersion.getExtension(); if (repositories.countVersions(extension.getNamespace().getName(), extension.getName()) == 1) { - deleteExtensionAndDependencies(admin, extension, depth + 1); + purgeExtensionAndDependencies(admin, extension, depth + 1); return; } @@ -170,7 +170,7 @@ private void deleteExtensionAndDependencies(UserData admin, ExtensionVersion ext * This method is intended for non-user interaction as it will wait till the lock can be acquired. */ @Transactional(rollbackOn = ErrorResultException.class) - public void deleteExtension(UserData admin, String namespaceName, String extensionName) + public void purgeExtension(UserData admin, String namespaceName, String extensionName) throws ErrorResultException { var extension = extensions.lockExtension(namespaceName, extensionName); extensions.purgeExtension(admin, extension, false); diff --git a/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java index ca9dbf5e4..99540ed1c 100644 --- a/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java @@ -77,12 +77,12 @@ private void processMaliciousExtensions(JsonNode json) { if (extensionId != null && repositories.hasExtension(extensionId.namespace(), extensionId.extension())) { logger.info("delete malicious extension"); if (service.deleteTransitively) { - admin.deleteExtensionAndDependencies( + admin.purgeExtensionAndDependencies( extensionControlUser, extensionId.namespace(), extensionId.extension()); } else { - admin.deleteExtension(extensionControlUser, extensionId.namespace(), extensionId.extension()); + admin.purgeExtension(extensionControlUser, extensionId.namespace(), extensionId.extension()); } } } From f6dc302ec25d76c4634921fdea255db54f606560 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 23 Jul 2026 18:02:52 +0200 Subject: [PATCH 13/17] do not check for bundles when deleting an extension --- .../main/java/org/eclipse/openvsx/ExtensionService.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index 4669a3385..1fd103149 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -581,15 +581,6 @@ public ResultJson purgeExtensionVersion(UserData user, ExtensionVersion extVersi } private void checkNoDependencies(Extension extension) throws ErrorResultException { - var bundledRefs = repositories.findBundledExtensionsReference(extension); - if (!bundledRefs.isEmpty()) { - throw new ErrorResultException( - "Extension " + NamingUtil.toExtensionId(extension) - + " is bundled by the following extension packs: " - + bundledRefs.stream() - .map(NamingUtil::toFileFormat) - .collect(Collectors.joining(", "))); - } var dependRefs = repositories.findDependenciesReference(extension); if (!dependRefs.isEmpty()) { throw new ErrorResultException( From 85717b38a2dabc91343a304f11674518613fdfb1 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 23 Jul 2026 18:15:07 +0200 Subject: [PATCH 14/17] fix test --- .../java/org/eclipse/openvsx/UserAPITest.java | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java index 3712a2b7d..236bbf657 100644 --- a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java @@ -593,24 +593,6 @@ void testDeleteLastExtensionVersion() throws Exception { .andExpect(content().json(successJson("Deleted foobar.baz"))); } - @Test - void testDeleteBundledExtension() throws Exception { - var userData = mockUserData(); - mockExtension(userData, 2, 1, 0); - mockMvc.perform( - post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") - .content( - "[{\"targetPlatform\":\"universal\",\"version\":\"1.0.0\"},{\"targetPlatform\":\"universal\",\"version\":\"2.0.0\"}]") - .contentType(MediaType.APPLICATION_JSON) - .with(user("test_user")) - .with(csrf().asHeader())) - .andExpect(status().isBadRequest()) - .andExpect( - content().json( - errorJson( - "Extension foobar.baz is bundled by the following extension packs: foobar.bundle-1.0.0"))); - } - @Test void testDeleteDependingExtension() throws Exception { var userData = mockUserData(); @@ -814,7 +796,8 @@ private List mockExtension( .thenReturn(Streamable.of(versions)); Mockito.when(repositories.findLatestVersions(user)).thenReturn(List.of(versions.getLast())); Mockito.when( - repositories.isDeleteAllVersions(any(), eq("foobar"), eq("baz"), any(TargetPlatformVersion[].class))) + repositories + .isDeleteAllActiveVersions(any(), eq("foobar"), eq("baz"), any(TargetPlatformVersion[].class))) .then(new Answer() { @Override public Boolean answer(InvocationOnMock invocation) { From cff21cf189e7da2668261a67b50767f3f1fac5f3 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 23 Jul 2026 20:19:02 +0200 Subject: [PATCH 15/17] purge extension without checking dependencies in DataMirrorService --- .../openvsx/mirror/DataMirrorJobRequestHandler.java | 3 +-- .../org/eclipse/openvsx/mirror/DataMirrorService.java | 8 +------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java index ebb5edae4..bad5e0451 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorJobRequestHandler.java @@ -138,8 +138,7 @@ private void deleteOtherExtensions(List extensionIds, UserData mirrorUse var extensionId = NamingUtil.toExtensionId(extension); ThreadLocalJobContext.getJobContext().logger().info("deleting " + extensionId); try { - var namespace = extension.getNamespace(); - extensions.purgeExtensionNoWait(mirrorUser, namespace.getName(), extension.getName()); + extensions.purgeExtension(mirrorUser, extension, false); } catch (ErrorResultException e) { if (e.getStatus() != HttpStatus.NOT_FOUND) { logger.warn("mirror: failed to delete extension {}", extensionId, e); diff --git a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java index 2658d6ae8..56b16e0e4 100644 --- a/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java +++ b/server/src/main/java/org/eclipse/openvsx/mirror/DataMirrorService.java @@ -37,7 +37,6 @@ import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.storage.StorageUtilService; import org.eclipse.openvsx.util.NamingUtil; -import org.eclipse.openvsx.util.TargetPlatformVersion; import org.eclipse.openvsx.util.TimeUtil; @Component @@ -244,14 +243,9 @@ private void addReview(ReviewJson json, Extension extension) { } public void deleteExtensionVersion(ExtensionVersion extVersion, UserData user) { - var extension = extVersion.getExtension(); // The mirror must not keep tombstones: versions dropped upstream are purged so the mirror can // re-track upstream state (and re-publish a version that later reappears upstream). - extensions.purgeExtensionNoWait( - user, - extension.getNamespace().getName(), - extension.getName(), - TargetPlatformVersion.of(extVersion.getTargetPlatform(), extVersion.getVersion())); + extensions.purgeExtensionVersion(user, extVersion); } public void mirrorNamespaceMetadata(String namespaceName) { From d3da7d9af3bd07410df2e5475e6ffc7976646875 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Fri, 24 Jul 2026 12:10:07 +0200 Subject: [PATCH 16/17] next iteration, include namespace checks --- .../org/eclipse/openvsx/ExtensionService.java | 87 ++------- .../java/org/eclipse/openvsx/UserAPI.java | 17 +- .../eclipse/openvsx/admin/AdminService.java | 5 +- .../ExtensionVersionJooqRepository.java | 21 +-- .../repositories/RepositoryService.java | 7 +- .../eclipse/openvsx/ExtensionDeleteTest.java | 5 +- .../openvsx/ExtensionSoftDeleteTest.java | 178 +++++++++++++++++- .../java/org/eclipse/openvsx/UserAPITest.java | 58 +++++- .../eclipse/openvsx/admin/AdminAPITest.java | 34 +--- .../openvsx/cache/CacheServiceTest.java | 6 +- .../RepositoryServiceSmokeTest.java | 2 +- 11 files changed, 295 insertions(+), 125 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index 1fd103149..fc619b9a2 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -269,32 +269,15 @@ private boolean canBeReactivated(ExtensionVersion extVersion) { } /** - * Delete an extension version published by the given user. - *

- * The extension will be locked for the operation. If the lock can not be acquired, i.e. the extension - * is updated at the same time, the operation will fail. - *

- * If the resolved extension version has not been published by the given user, - * a {@code ErrorResultException} will be thrown. - */ - @Transactional(rollbackOn = ErrorResultException.class) - public ResultJson deleteUserExtension( - UserData user, - String namespaceName, - String extensionName, - TargetPlatformVersion... targetVersions - ) throws ErrorResultException { - return deleteExtension(user, true, namespaceName, extensionName, targetVersions); - } - - /** - * Deletes the given extension. + * Soft-deletes the given extension versions. *

* The extension will be locked for the operation. If the lock can not be acquired, i.e. the extension * is updated at the same time, the operation will fail. *

* If {@code restrictedToUser} is {@code true}, the deletion operation is only successful if the user * has published the respective extension version. + *

+ * The versions to delete must be named explicitly; an empty {@code targetVersions} deletes nothing. */ @Transactional(rollbackOn = ErrorResultException.class) public ResultJson deleteExtension( @@ -305,14 +288,14 @@ public ResultJson deleteExtension( TargetPlatformVersion... targetVersions ) throws ErrorResultException { var extension = lockExtensionNoWait(namespaceName, extensionName); - if (repositories - .isDeleteAllVersions(restrictedToUser ? user : null, namespaceName, extensionName, targetVersions)) { - return deleteExtension(user, extension, true); + var versions = resolveVersions(user, restrictedToUser, namespaceName, extensionName, targetVersions); + // if all active versions of the extension would get deactivated, check for dependencies + if (extension.isActive() && + repositories.isDeleteAllActiveVersions(namespaceName, extensionName, targetVersions)) { + checkNoDependencies(extension); } - return deleteExtensionVersions( - user, - resolveVersions(user, restrictedToUser, namespaceName, extensionName, targetVersions)); + return deleteExtensionVersions(user, versions); } /** @@ -333,14 +316,18 @@ public ResultJson purgeExtensionNoWait( TargetPlatformVersion... targetVersions ) throws ErrorResultException { var extension = lockExtensionNoWait(namespaceName, extensionName); - if (repositories - .isDeleteAllVersions(null, namespaceName, extensionName, targetVersions)) { + if (targetVersions.length == 0) { return purgeExtension(user, extension, true); } - return purgeExtensionVersions( - user, - resolveVersions(user, false, namespaceName, extensionName, targetVersions)); + var versions = resolveVersions(user, false, namespaceName, extensionName, targetVersions); + // if all active versions of the extension would get deactivated, check for dependencies + if (extension.isActive() && + repositories.isDeleteAllActiveVersions(namespaceName, extensionName, targetVersions)) { + checkNoDependencies(extension); + } + + return purgeExtensionVersions(user, versions); } private List resolveVersions( @@ -457,40 +444,6 @@ private ResultJson combineResults(List results) { return result; } - /** - * Soft-delete the given extension: mark all its versions as removed and hide the extension. - *

- * The extension and version rows are kept so their identities stay reserved and can never be - * republished; only the version files are stripped from storage. Use - * {@link #purgeExtension(UserData, Extension, boolean)} to physically remove the extension. - *

- * If {@code checkDependencies} is {@code true} and this extension is referenced by a bundle or used - * as a dependency, the operation will fail. - * - * @param user the user that will be used for logging the operation - * @param extension the extension to soft-delete - * @param checkDependencies whether to check if this extension is still referenced by bundles or as a dependency - */ - @Transactional(rollbackOn = ErrorResultException.class) - public ResultJson deleteExtension(UserData user, Extension extension, boolean checkDependencies) - throws ErrorResultException { - if (checkDependencies) { - checkNoDependencies(extension); - } - - for (var extVersion : repositories.findVersions(extension)) { - if (!extVersion.isRemoved()) { - softDeleteExtensionVersion(user, extVersion); - } - } - - updateExtension(extension); - - var result = ResultJson.success("Deleted " + NamingUtil.toExtensionId(extension)); - logs.logAction(user, result); - return result; - } - /** * Soft-delete a single extension version: strip its files from storage and mark it as removed, * but keep the row so the version identity stays reserved. Does not touch the parent extension; @@ -563,7 +516,7 @@ public ResultJson purgeExtension(UserData user, Extension extension, boolean che search.removeSearchEntry(extension); - var result = ResultJson.success("Deleted " + NamingUtil.toExtensionId(extension)); + var result = ResultJson.success("Purged " + NamingUtil.toExtensionId(extension)); logs.logAction(user, result); return result; } @@ -575,7 +528,7 @@ public ResultJson purgeExtensionVersion(UserData user, ExtensionVersion extVersi extension.getVersions().remove(extVersion); updateExtension(extension); - var result = ResultJson.success("Deleted " + NamingUtil.toLogFormat(extVersion)); + var result = ResultJson.success("Purged " + NamingUtil.toLogFormat(extVersion)); logs.logAction(user, result); return result; } diff --git a/server/src/main/java/org/eclipse/openvsx/UserAPI.java b/server/src/main/java/org/eclipse/openvsx/UserAPI.java index 47f1cd538..246b1a494 100644 --- a/server/src/main/java/org/eclipse/openvsx/UserAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/UserAPI.java @@ -412,11 +412,26 @@ public ResponseEntity deleteExtension( throw new ResponseStatusException(HttpStatus.FORBIDDEN); } try { + var namespace = repositories.findNamespace(namespaceName); + if (namespace == null) { + var json = NamespaceDetailsJson + .error("Extension not found: " + NamingUtil.toExtensionId(namespaceName, extensionName)); + return new ResponseEntity<>(json, HttpStatus.NOT_FOUND); + } + + // Authorize before touching the extension: only namespace members may delete. + // Owners may delete any version; non-owner members are restricted to versions + // they published themselves (enforced via restrictedToUser). + var isOwner = repositories.isNamespaceOwner(user, namespace); + if (!isOwner && !repositories.hasMembership(user, namespace)) { + throw new ResponseStatusException(HttpStatus.FORBIDDEN); + } + var targets = CollectionUtil.toArray( targetVersions, TargetPlatformVersionJson::toTargetPlatformVersion, TargetPlatformVersion[]::new); - var result = extensions.deleteUserExtension(user, namespaceName, extensionName, targets); + var result = extensions.deleteExtension(user, !isOwner, namespaceName, extensionName, targets); return ResponseEntity.ok(result); } catch (NotFoundException exc) { var json = NamespaceDetailsJson diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 508e62b73..6dd65e2f9 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -177,8 +177,9 @@ public void purgeExtension(UserData admin, String namespaceName, String extensio } /** - * Delete the provided versions of an extension. If all versions of the extension shall be deleted, the - * extension as a whole will be removed unless it is referenced by bundles or used as a dependency. + * Soft-delete the provided versions of an extension. The versions must be named explicitly; an empty + * {@code targetVersions} deletes nothing. When the named versions cover all active versions and the + * extension is referenced by bundles or used as a dependency, the operation will fail. *

* The method will try to lock the extension and fail with an {@code ErrorResultException} if it can't acquire it. *

diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java index 5b3f1beed..56e5f569e 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java @@ -16,7 +16,6 @@ import org.jooq.*; import org.jooq.Record; import org.jooq.impl.DSL; -import org.jspecify.annotations.Nullable; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -1540,7 +1539,7 @@ public boolean hasSameVersion(ExtensionVersion extVersion) { .and(EXTENSION_VERSION.PRE_RELEASE.eq(!extVersion.isPreRelease()))); } - public Integer count(String namespaceName, String extensionName) { + public Integer countVersions(String namespaceName, String extensionName) { return dsl.select(DSL.count().as("count")) .from(EXTENSION_VERSION) .join(EXTENSION) @@ -1552,14 +1551,13 @@ public Integer count(String namespaceName, String extensionName) { .fetchOne("count", Integer.class); } - public boolean isDeleteAllVersions( - @Nullable UserData user, + public boolean isDeleteAllActiveVersions( String namespaceName, String extensionName, TargetPlatformVersion... targetVersions ) { if (targetVersions.length == 0) { - return true; + return false; } var all = dsl.select(DSL.count(EXTENSION_VERSION.ID).as("all")) @@ -1568,6 +1566,7 @@ public boolean isDeleteAllVersions( .join(NAMESPACE).on(NAMESPACE.ID.eq(EXTENSION.NAMESPACE_ID)) .and(NAMESPACE.NAME.equalIgnoreCase(namespaceName)) .and(EXTENSION.NAME.equalIgnoreCase(extensionName)) + .and(EXTENSION_VERSION.ACTIVE.eq(true)) .fetchOne("all", Integer.class); var rows = Arrays.stream(targetVersions).map((tv) -> DSL.row(tv.version(), tv.targetPlatform())) @@ -1582,18 +1581,10 @@ public boolean isDeleteAllVersions( .join(EXTENSION).on(EXTENSION.ID.eq(EXTENSION_VERSION.EXTENSION_ID)) .join(NAMESPACE).on(NAMESPACE.ID.eq(EXTENSION.NAMESPACE_ID)); - if (user != null) { - actualSelect = actualSelect.join(PERSONAL_ACCESS_TOKEN) - .on(PERSONAL_ACCESS_TOKEN.ID.eq(EXTENSION_VERSION.PUBLISHED_WITH_ID)); - } - var condition = actualSelect .where(NAMESPACE.NAME.equalIgnoreCase(namespaceName)) - .and(EXTENSION.NAME.equalIgnoreCase(extensionName)); - - if (user != null) { - condition = condition.and(PERSONAL_ACCESS_TOKEN.USER_DATA.eq(user.getId())); - } + .and(EXTENSION.NAME.equalIgnoreCase(extensionName)) + .and(EXTENSION_VERSION.ACTIVE.eq(true)); var actual = condition.fetchOne("actual", Integer.class); 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 c14b07ada..6d3b5d72f 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java @@ -624,7 +624,7 @@ public Streamable findTargetPlatformVersions( } public int countVersions(String namespaceName, String extensionName) { - return extensionVersionJooqRepo.count(namespaceName, extensionName); + return extensionVersionJooqRepo.countVersions(namespaceName, extensionName); } public Slice findNotMigratedItems(Pageable page) { @@ -897,13 +897,12 @@ public List findRemoveFileResourceTypeResourceMigrationItems(int return migrationItemJooqRepo.findRemoveFileResourceTypeResourceMigrationItems(offset, limit); } - public boolean isDeleteAllVersions( - @Nullable UserData user, + public boolean isDeleteAllActiveVersions( String namespaceName, String extensionName, TargetPlatformVersion... targetVersions ) { - return extensionVersionJooqRepo.isDeleteAllVersions(user, namespaceName, extensionName, targetVersions); + return extensionVersionJooqRepo.isDeleteAllActiveVersions(namespaceName, extensionName, targetVersions); } public List findSimilarExtensionsByLevenshtein( diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java index e37e021ec..01943d04b 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionDeleteTest.java @@ -52,9 +52,6 @@ * The race condition is deliberately triggered by intercepting the boundary call that * separates the check from the act (using a spy), pausing the delete operation just * long enough for the concurrent publish to commit. - *

- * {@link ExtensionService#deleteUserExtension(UserData, String, String, TargetPlatformVersion...)} - * is protected by a {@code SELECT … FOR UPDATE NOWAIT} lock and therefore passes. */ @SpringBootTest class ExtensionDeleteTest extends AbstractPostgresContainerTest { @@ -135,7 +132,7 @@ void userDeleteExtension_doesNotDeleteAnotherPublishersConcurrentlyAddedVersion( publisherFinished.await(RACE_WINDOW_TIMEOUT_SECONDS, TimeUnit.SECONDS); } return result; - }).when(repositories).isDeleteAllVersions(any(), any(), any(), any()); + }).when(repositories).isDeleteAllActiveVersions(any(), any(), any(), any()); var owner = new UserData(); owner.setId(ownerId); diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java index 1c17a7031..112aa3694 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java @@ -19,7 +19,9 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.data.domain.PageRequest; +import org.springframework.data.util.Streamable; import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.support.TransactionTemplate; @@ -30,10 +32,14 @@ import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.search.SearchUtilService; +import org.eclipse.openvsx.util.ErrorResultException; import org.eclipse.openvsx.util.TargetPlatform; import org.eclipse.openvsx.util.TargetPlatformVersion; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; /** * Integration tests for the soft-delete (immutable version) feature in {@link ExtensionService}. @@ -54,7 +60,7 @@ class ExtensionSoftDeleteTest extends AbstractPostgresContainerTest { @Autowired ExtensionService extensionService; - @Autowired + @MockitoSpyBean RepositoryService repositories; @Autowired @@ -234,6 +240,157 @@ void findVersionsForUrls_excludesRemovedVersions() { .containsExactly(TargetPlatform.NAME_UNIVERSAL); } + /** + * The dependency guard must fire when a delete removes the last active versions, even if + * older tombstones still occupy rows. Regression test for the flaw where the guard was keyed on the + * total row count (tombstones included) instead of the active versions, so deleting all active + * versions of a depended-on extension slipped through without the check. + */ + @Test + void deleteExtension_runsDependencyCheckWhenDeletingAllActiveVersionsDespiteTombstone() { + persistVersion("0.9.0", TargetPlatform.NAME_UNIVERSAL, false, true); // pre-existing tombstone + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); // the only active version + + // Simulate another extension depending on this one, so the dependency guard must reject. + doReturn(Streamable.of(dependantReference())).when(repositories).findDependenciesReference(any()); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + assertThatThrownBy(() -> extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets)) + .as("deleting all active versions of a depended-on extension must run the dependency check") + .isInstanceOf(ErrorResultException.class); + + assertThat(versionRemoved("1.0.0")) + .as("a rejected delete must leave the active version untouched") + .isFalse(); + assertThat(versionActive("1.0.0")) + .as("the active version must survive the rejected delete") + .isTrue(); + } + + /** + * Deleting only a subset of the active versions is not a delete-all, so the dependency guard must + * not fire: the selected version is soft-deleted and the remaining active version stays live. + */ + @Test + void deleteExtension_skipsDependencyCheckWhenDeletingSubsetOfActiveVersions() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + // Even if a dependency exists, deleting a subset must not trigger the guard. + doReturn(Streamable.of(dependantReference())).when(repositories).findDependenciesReference(any()); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + extensionService.deleteExtension(owner(), false, NAMESPACE, EXTENSION, targets); + + assertThat(versionRemoved("1.0.0")) + .as("deleting a subset must soft-delete the selected version without a dependency check") + .isTrue(); + assertThat(versionActive("2.0.0")) + .as("the remaining active version must stay live") + .isTrue(); + } + + /** + * Purging explicit versions must remove only those versions: pre-existing tombstones and the + * extension record itself must survive so reserved identities stay reserved. + */ + @Test + void purgeExtension_withExplicitTargets_keepsExtensionAndTombstones() { + persistVersion("0.9.0", TargetPlatform.NAME_UNIVERSAL, false, true); // pre-existing tombstone + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION, targets); + + assertThat(versionExists("1.0.0")) + .as("the purged version's row must be physically removed") + .isFalse(); + assertThat(versionExists("0.9.0")) + .as("a tombstone the caller did not select must survive a scoped purge") + .isTrue(); + assertThat(extensionExists()) + .as("a scoped purge must not remove the extension record itself") + .isTrue(); + } + + /** + * Purging all the active versions by naming them explicitly must still only remove those versions; + * the (now inactive) extension record must remain. Only an unscoped purge removes the extension. + */ + @Test + void purgeExtension_purgingAllActiveVersionsKeepsExtensionEntity() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + var targets = new TargetPlatformVersion[] { + TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"), + TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "2.0.0") + }; + extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION, targets); + + assertThat(versionExists("1.0.0")).isFalse(); + assertThat(versionExists("2.0.0")).isFalse(); + assertThat(extensionExists()) + .as("purging named versions must not remove the extension record, even when they are all active") + .isTrue(); + } + + /** + * The dependency guard must also fire on the purge path when purging all active versions. + */ + @Test + void purgeExtension_runsDependencyCheckWhenPurgingAllActiveVersions() { + persistVersion("0.9.0", TargetPlatform.NAME_UNIVERSAL, false, true); // pre-existing tombstone + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + // Simulate another extension depending on this one. + doReturn(Streamable.of(dependantReference())).when(repositories).findDependenciesReference(any()); + + var targets = TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"); + assertThatThrownBy(() -> extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION, targets)) + .as("purging all active versions of a depended-on extension must run the dependency check") + .isInstanceOf(ErrorResultException.class); + + assertThat(versionExists("1.0.0")) + .as("a rejected purge must leave the active version in place") + .isTrue(); + assertThat(versionExists("0.9.0")) + .as("a rejected purge must leave tombstones in place") + .isTrue(); + } + + /** + * An unscoped purge (no target versions) removes the whole extension, tombstones and all. + */ + @Test + void purgeExtension_withEmptyTargets_removesWholeExtensionIncludingTombstones() { + persistVersion("0.9.0", TargetPlatform.NAME_UNIVERSAL, false, true); // pre-existing tombstone + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION); + + assertThat(versionExists("1.0.0")).isFalse(); + assertThat(versionExists("0.9.0")) + .as("an unscoped purge must remove reserved tombstones too") + .isFalse(); + assertThat(extensionExists()) + .as("an unscoped purge must remove the extension record") + .isFalse(); + } + + private ExtensionVersion dependantReference() { + var namespace = new Namespace(); + namespace.setName("dependant-ns"); + var extension = new Extension(); + extension.setName("dependant-ext"); + extension.setNamespace(namespace); + var extVersion = new ExtensionVersion(); + extVersion.setExtension(extension); + extVersion.setVersion("1.0.0"); + extVersion.setTargetPlatform(TargetPlatform.NAME_UNIVERSAL); + return extVersion; + } + private void persistVersion(String version, String targetPlatform, boolean active, boolean removed) { new TransactionTemplate(txManager).executeWithoutResult(status -> { var extension = em.find(Extension.class, extensionId); @@ -268,6 +425,25 @@ private boolean versionRemoved(String version) { version) > 0; } + private boolean versionActive(String version) { + return count( + "select ev.id from ExtensionVersion ev where ev.version = :version " + + "and ev.extension.namespace.name = :namespace and ev.active = true and ev.removed = false", + version) > 0; + } + + private boolean extensionExists() { + return Boolean.TRUE.equals( + new TransactionTemplate(txManager).execute( + status -> !em.createQuery( + "select e.id from Extension e " + + "where e.name = :name and e.namespace.name = :namespace") + .setParameter("name", EXTENSION) + .setParameter("namespace", NAMESPACE) + .getResultList() + .isEmpty())); + } + private Long removedByOf(String version) { return new TransactionTemplate(txManager).execute( status -> em.createQuery( diff --git a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java index 236bbf657..c757055ab 100644 --- a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java @@ -540,6 +540,10 @@ void testDeleteExtensionNotPublisher() throws Exception { Mockito.doReturn(otherUser).when(users).findLoggedInUser(); mockExtension(userData, 2, 0, 0); + // A namespace member, but not the publisher of the targeted version: the version lookup is + // scoped to the caller, so it is not found and the delete fails with 404. + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); mockMvc.perform( post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") .content("[{\"targetPlatform\":\"universal\",\"version\":\"1.0.0\"}]") @@ -549,10 +553,29 @@ void testDeleteExtensionNotPublisher() throws Exception { .andExpect(status().isNotFound()); } + @Test + void testDeleteExtensionNotMember() throws Exception { + var userData = mockUserData(); + mockExtension(userData, 2, 0, 0); + // Neither owner nor member of the namespace: rejected before the extension is touched. + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(false); + mockMvc.perform( + post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") + .content("[{\"targetPlatform\":\"universal\",\"version\":\"1.0.0\"}]") + .contentType(MediaType.APPLICATION_JSON) + .with(user("test_user")) + .with(csrf().asHeader())) + .andExpect(status().isForbidden()); + } + @Test void testDeleteExtension() throws Exception { var userData = mockUserData(); mockExtension(userData, 2, 0, 0); + // A member deletes all versions they published by naming them explicitly; each is soft-deleted. + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); mockMvc.perform( post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") .content( @@ -561,13 +584,33 @@ void testDeleteExtension() throws Exception { .with(user("test_user")) .with(csrf().asHeader())) .andExpect(status().isOk()) - .andExpect(content().json(successJson("Deleted foobar.baz"))); + .andExpect(content().json(successJson("Deleted foobar.baz 1.0.0\nDeleted foobar.baz 2.0.0"))); + } + + @Test + void testDeleteExtensionEmptyTargetsIsNoOp() throws Exception { + var userData = mockUserData(); + mockExtension(userData, 2, 0, 0); + // With the whole-extension shortcut removed, an empty target list deletes nothing. + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); + mockMvc.perform( + post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") + .content("[]") + .contentType(MediaType.APPLICATION_JSON) + .with(user("test_user")) + .with(csrf().asHeader())) + .andExpect(status().isOk()) + .andExpect(content().json(successJson(""))); } @Test void testDeleteExtensionVersion() throws Exception { var userData = mockUserData(); mockExtension(userData, 3, 0, 0); + // Non-owner member may delete versions they published themselves. + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); mockMvc.perform( post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") .content( @@ -583,6 +626,10 @@ void testDeleteExtensionVersion() throws Exception { void testDeleteLastExtensionVersion() throws Exception { var userData = mockUserData(); mockExtension(userData, 1, 0, 0); + // Non-owner member deleting the last version they published: soft-deleted per version + // (the extension record itself survives, deactivated). + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); mockMvc.perform( post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") .content("[{\"targetPlatform\":\"universal\",\"version\":\"1.0.0\"}]") @@ -590,13 +637,16 @@ void testDeleteLastExtensionVersion() throws Exception { .with(user("test_user")) .with(csrf().asHeader())) .andExpect(status().isOk()) - .andExpect(content().json(successJson("Deleted foobar.baz"))); + .andExpect(content().json(successJson("Deleted foobar.baz 1.0.0"))); } @Test void testDeleteDependingExtension() throws Exception { var userData = mockUserData(); mockExtension(userData, 2, 0, 1); + // Deleting all active versions of a depended-on extension triggers the dependency check. + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); mockMvc.perform( post("/user/extension/{namespace}/{extension}/delete", "foobar", "baz") .content( @@ -797,11 +847,11 @@ private List mockExtension( Mockito.when(repositories.findLatestVersions(user)).thenReturn(List.of(versions.getLast())); Mockito.when( repositories - .isDeleteAllActiveVersions(any(), eq("foobar"), eq("baz"), any(TargetPlatformVersion[].class))) + .isDeleteAllActiveVersions(eq("foobar"), eq("baz"), any(TargetPlatformVersion[].class))) .then(new Answer() { @Override public Boolean answer(InvocationOnMock invocation) { - return ((TargetPlatformVersion[]) invocation.getRawArguments()[3]).length == numberOfVersions; + return ((TargetPlatformVersion[]) invocation.getRawArguments()[2]).length == numberOfVersions; } }); diff --git a/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java b/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java index 8e33347dc..2c08efb1f 100644 --- a/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java @@ -421,7 +421,7 @@ void testDeleteExtension() throws Exception { .with(user("admin_user").authorities(new SimpleGrantedAuthority(("ROLE_ADMIN")))) .with(csrf().asHeader())) .andExpect(status().isOk()) - .andExpect(content().json(successJson("Deleted foobar.baz"))); + .andExpect(content().json(successJson("Deleted foobar.baz 1.0.0\nDeleted foobar.baz 2.0.0"))); } @Test @@ -433,9 +433,12 @@ void testDeleteExtensionWithToken() throws Exception { "/admin/api/extension/{namespace}/{extension}/delete?token={token}", "foobar", "baz", - token.getValue())) + token.getValue()) + .content( + "[{\"targetPlatform\":\"universal\",\"version\":\"1.0.0\"},{\"targetPlatform\":\"universal\",\"version\":\"2.0.0\"}]") + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) - .andExpect(content().json(successJson("Deleted foobar.baz"))); + .andExpect(content().json(successJson("Deleted foobar.baz 1.0.0\nDeleted foobar.baz 2.0.0"))); } @Test @@ -507,25 +510,7 @@ void testDeleteLastExtensionVersion() throws Exception { .with(user("admin_user").authorities(new SimpleGrantedAuthority(("ROLE_ADMIN")))) .with(csrf().asHeader())) .andExpect(status().isOk()) - .andExpect(content().json(successJson("Deleted foobar.baz"))); - } - - @Test - void testDeleteBundledExtension() throws Exception { - mockAdminUser(); - mockExtension(2, 1, 0); - mockMvc.perform( - post("/admin/extension/{namespace}/{extension}/delete", "foobar", "baz") - .content( - "[{\"targetPlatform\":\"universal\",\"version\":\"1.0.0\"},{\"targetPlatform\":\"universal\",\"version\":\"2.0.0\"}]") - .contentType(MediaType.APPLICATION_JSON) - .with(user("admin_user").authorities(new SimpleGrantedAuthority(("ROLE_ADMIN")))) - .with(csrf().asHeader())) - .andExpect(status().isBadRequest()) - .andExpect( - content().json( - errorJson( - "Extension foobar.baz is bundled by the following extension packs: foobar.bundle-1.0.0"))); + .andExpect(content().json(successJson("Deleted foobar.baz 1.0.0"))); } @Test @@ -2047,15 +2032,14 @@ private List mockExtension(int numberOfVersions, int numberOfB extension.getVersions().addAll(versions); Mockito.when( - repositories.isDeleteAllVersions( - any(), + repositories.isDeleteAllActiveVersions( eq(namespace.getName()), eq(extension.getName()), any(TargetPlatformVersion[].class))) .then(new Answer() { @Override public Boolean answer(InvocationOnMock invocation) { - var len = ((TargetPlatformVersion[]) invocation.getRawArguments()[3]).length; + var len = ((TargetPlatformVersion[]) invocation.getRawArguments()[2]).length; return len == 0 || len == numberOfVersions; } }); diff --git a/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java b/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java index 1c7c48d35..a5d811c49 100644 --- a/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/cache/CacheServiceTest.java @@ -287,7 +287,11 @@ void testDeleteExtension() throws IOException { extVersion.getTargetPlatform(), extVersion.getVersion()); - admins.deleteExtensionNoWait(admin, namespace.getName(), extension.getName()); + admins.deleteExtensionNoWait( + admin, + namespace.getName(), + extension.getName(), + TargetPlatformVersion.of(extVersion.getTargetPlatform(), extVersion.getVersion())); assertNull(getCache(CACHE_EXTENSION_JSON).get(cacheKey, ExtensionJson.class)); } } 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 ac7b86f99..fd44d3867 100644 --- a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java +++ b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java @@ -334,7 +334,7 @@ void testExecuteQueries() { "extensionName", "namespace"), () -> repositories.findLatestVersion(userData, "namespaceName", "extensionName"), - () -> repositories.isDeleteAllVersions(userData, "namespaceName", "extensionName"), + () -> repositories.isDeleteAllActiveVersions("namespaceName", "extensionName"), () -> repositories.deactivateAccessTokens(userData), () -> repositories.expireAccessTokens(NOW), () -> repositories.findExpiringAccessTokensWithoutNotification(NOW, page), From 981668c36dc25a8127770e61f31f2725b4677b40 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Fri, 24 Jul 2026 15:16:06 +0200 Subject: [PATCH 17/17] overhaul delete / purge mechanism --- .../org/eclipse/openvsx/ExtensionService.java | 50 ++++- .../java/org/eclipse/openvsx/UserAPI.java | 77 +++++++- .../eclipse/openvsx/admin/AdminService.java | 86 +++++---- .../ExtensionControlJobRequestHandler.java | 2 +- .../ExtensionControlService.java | 25 ++- .../json/VersionTargetPlatformsJson.java | 26 ++- .../repositories/ExtensionJooqRepository.java | 3 +- .../repositories/RepositoryService.java | 4 +- .../search/SimilarityCheckService.java | 2 + .../openvsx/ExtensionSoftDeleteTest.java | 60 ++++-- .../java/org/eclipse/openvsx/UserAPITest.java | 150 ++++++++++++++- .../openvsx/admin/AdminServiceTest.java | 178 ++++++++++++++++++ .../ExtensionControlServiceTest.java | 161 ++++++++++++++++ .../RepositoryServiceSmokeTest.java | 2 +- .../extension/extension-detail-view.tsx | 8 +- .../extension/extension-version-table.tsx | 14 +- webui/src/extension-registry-types.ts | 4 + .../pages/user/user-settings-namespaces.tsx | 3 + 18 files changed, 782 insertions(+), 73 deletions(-) create mode 100644 server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java create mode 100644 server/src/test/java/org/eclipse/openvsx/extension_control/ExtensionControlServiceTest.java diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index fc619b9a2..fd8cff8a6 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -288,10 +288,12 @@ public ResultJson deleteExtension( TargetPlatformVersion... targetVersions ) throws ErrorResultException { var extension = lockExtensionNoWait(namespaceName, extensionName); - var versions = resolveVersions(user, restrictedToUser, namespaceName, extensionName, targetVersions); + var uniqueVersions = distinctVersions(targetVersions); + var versions = resolveVersions(user, restrictedToUser, namespaceName, extensionName, uniqueVersions); + // if all active versions of the extension would get deactivated, check for dependencies if (extension.isActive() && - repositories.isDeleteAllActiveVersions(namespaceName, extensionName, targetVersions)) { + repositories.isDeleteAllActiveVersions(namespaceName, extensionName, uniqueVersions)) { checkNoDependencies(extension); } @@ -299,13 +301,16 @@ public ResultJson deleteExtension( } /** - * Purges (permanently deletes) the given extension or extension versions from the database and storage. + * Purges (permanently deletes) the given extension versions from the database and storage. *

* Unlike {@link #deleteExtension(UserData, boolean, String, String, TargetPlatformVersion...)}, which * soft-deletes versions (keeping the row so the version identity stays reserved), this physically removes * the rows and frees the version identity for republishing. It is intended for administrative purge and * automated cleanup (mirror, extension control) and performs no user-ownership check. *

+ * The versions to purge must be named explicitly; an empty {@code targetVersions} purges nothing. Use + * {@link #purgeExtension(UserData, Extension, boolean)} to purge an extension as a whole. + *

* The method will try to lock the extension and fail with an {@code ErrorResultException} if it can't acquire it. */ @Transactional(rollbackOn = ErrorResultException.class) @@ -316,20 +321,32 @@ public ResultJson purgeExtensionNoWait( TargetPlatformVersion... targetVersions ) throws ErrorResultException { var extension = lockExtensionNoWait(namespaceName, extensionName); - if (targetVersions.length == 0) { - return purgeExtension(user, extension, true); + var uniqueVersions = distinctVersions(targetVersions); + var versions = resolveVersions(user, false, namespaceName, extensionName, uniqueVersions); + + // if every version of the extension is purged, purge the extension as a whole so that its + // record, reviews and search entry are removed too and nothing is left orphaned + if (!versions.isEmpty() && versions.size() == repositories.countVersions(namespaceName, extensionName)) { + return purgeExtension(user, extension, extension.isActive()); } - var versions = resolveVersions(user, false, namespaceName, extensionName, targetVersions); // if all active versions of the extension would get deactivated, check for dependencies if (extension.isActive() && - repositories.isDeleteAllActiveVersions(namespaceName, extensionName, targetVersions)) { + repositories.isDeleteAllActiveVersions(namespaceName, extensionName, uniqueVersions)) { checkNoDependencies(extension); } return purgeExtensionVersions(user, versions); } + /** + * Returns the given target versions with duplicates removed, so that callers can reason about the + * exact set of versions to delete/purge without a repeated entry inflating any count-based check. + */ + private static TargetPlatformVersion[] distinctVersions(TargetPlatformVersion... targetVersions) { + return Arrays.stream(targetVersions).distinct().toArray(TargetPlatformVersion[]::new); + } + private List resolveVersions( UserData user, boolean restrictedToUser, @@ -337,7 +354,7 @@ private List resolveVersions( String extensionName, TargetPlatformVersion... targetVersions ) { - return Arrays.stream(targetVersions) + var versions = Arrays.stream(targetVersions) .map(target -> { var extVersion = restrictedToUser ? repositories.findVersionPublishedWithUser( @@ -364,6 +381,23 @@ private List resolveVersions( return extVersion; }) .toList(); + + // Guard against a mismatch between the requested versions and what was actually resolved: the + // resolved versions must correspond exactly to the requested (target platform, version) pairs. + // Otherwise, the count-based "delete/purge all versions" checks could be driven to a wrong + // conclusion (e.g. by duplicate or unexpectedly-resolving entries). + var requested = Arrays.stream(targetVersions).collect(Collectors.toSet()); + var resolved = versions.stream() + .map(v -> new TargetPlatformVersion(v.getTargetPlatform(), v.getVersion())) + .collect(Collectors.toSet()); + if (!resolved.equals(requested)) { + throw new ErrorResultException( + "The requested versions of " + NamingUtil.toExtensionId(namespaceName, extensionName) + + " could not be resolved.", + HttpStatus.BAD_REQUEST); + } + + return versions; } /** diff --git a/server/src/main/java/org/eclipse/openvsx/UserAPI.java b/server/src/main/java/org/eclipse/openvsx/UserAPI.java index 246b1a494..beb3e98a8 100644 --- a/server/src/main/java/org/eclipse/openvsx/UserAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/UserAPI.java @@ -12,6 +12,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import jakarta.servlet.http.HttpServletRequest; import org.slf4j.Logger; @@ -53,6 +54,7 @@ import org.eclipse.openvsx.json.TargetPlatformVersionJson; import org.eclipse.openvsx.json.UsageStatsListJson; import org.eclipse.openvsx.json.UserJson; +import org.eclipse.openvsx.json.VersionTargetPlatformsJson; import org.eclipse.openvsx.repositories.ExtensionScanRepository; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.security.CodedAuthException; @@ -236,6 +238,17 @@ public ResponseEntity deleteAccessToken(@PathVariable long id) { } } + /** + * Lists the extensions shown in the authenticated user's settings view. + *

+ * Only extensions the user published and whose namespace the user is currently + * a member of are returned. Extensions the user published in a namespace they have since left + * (or been removed from) are excluded, since the user no longer has any access to them (see + * {@link #getOwnExtension}). The list includes inactive and removed (soft-deleted) versions of + * the extensions that do qualify. + * + * @return {@code 200 OK} with the list of extensions, or {@code 403 Forbidden} if not logged in + */ @GetMapping( path = "/user/extensions", produces = MediaType.APPLICATION_JSON_VALUE @@ -246,7 +259,14 @@ public List getOwnExtensions() { throw new ResponseStatusException(HttpStatus.FORBIDDEN); } - var extVersions = repositories.findLatestVersions(user); + // Restrict to namespaces the user is currently a member of: a user who left a namespace must + // no longer see extensions they published there, even though they remain the publisher. + var memberNamespaceIds = repositories.findMemberships(user).stream() + .map(membership -> membership.getNamespace().getId()) + .collect(Collectors.toSet()); + var extVersions = repositories.findLatestVersions(user).stream() + .filter(ev -> memberNamespaceIds.contains(ev.getExtension().getNamespace().getId())) + .toList(); var types = new String[] { DOWNLOAD, MANIFEST, ICON, README, LICENSE, CHANGELOG, VSIXMANIFEST }; var fileUrls = storageUtil.getFileUrls(extVersions, UrlUtil.getBaseUrl(), types); @@ -366,6 +386,26 @@ private void enrichWithReviewStatus(ExtensionJson json, ExtensionVersion extVers } } + /** + * Returns an extension for the authenticated user's settings view, including every version's + * target platforms and, per version, whether the caller may delete it. + *

+ * Access is restricted to current namespace members: a member (owner or not) sees + * all versions of the extension, including versions they did not publish themselves and + * removed (soft-deleted) ones. A user who is not a member of the namespace has no access and + * receives {@code 404 Not Found}, even for versions they published while they were still a member. + *

+ * Each returned version carries a {@code canDelete} flag mirroring the authorization enforced by + * {@link #deleteExtension}: owners may delete any version, other members only the versions they + * published themselves. This lets the settings UI disable delete controls the caller is not + * allowed to use. + * + * @param namespaceName the namespace of the extension + * @param extensionName the extension name + * @return {@code 200 OK} with the extension, {@code 403 Forbidden} if not logged in, or + * {@code 404 Not Found} if the caller is not a namespace member or the extension does + * not exist + */ @GetMapping( path = "/user/extension/{namespaceName}/{extensionName}", produces = MediaType.APPLICATION_JSON_VALUE @@ -380,13 +420,39 @@ public ResponseEntity getOwnExtension( } try { + var namespace = repositories.findNamespace(namespaceName); + // Only current namespace members may inspect an extension here. A user who left the + // namespace (or was never a member) has no access, even to versions they published + // themselves. Members see every version, including ones they did not publish. + var isOwner = namespace != null && repositories.isNamespaceOwner(user, namespace); + var isMember = isOwner || (namespace != null && repositories.hasMembership(user, namespace)); + if (!isMember) { + var error = "Extension not found: " + NamingUtil.toExtensionId(namespaceName, extensionName); + throw new ErrorResultException(error, HttpStatus.NOT_FOUND); + } + + var latest = repositories.findLatestVersion(namespaceName, extensionName, null, false, false); + ExtensionJson json; - var latest = repositories.findLatestVersion(user, namespaceName, extensionName); if (latest != null) { json = local.toExtensionVersionJson(latest, null, false); + var extension = latest.getExtension(); + var allVersions = repositories.findTargetPlatformsGroupedByVersion(extension); + // Annotate each version with whether the caller may delete it, mirroring deleteExtension: + // owners may delete any version, other members only the versions they published themselves. + var deletableVersions = isOwner + ? null + : repositories.findTargetPlatformsGroupedByVersion(extension, user).stream() + .map(VersionTargetPlatformsJson::version) + .collect(Collectors.toSet()); json.setAllTargetPlatformVersions( - repositories.findTargetPlatformsGroupedByVersion(latest.getExtension(), user)); - json.setActive(latest.getExtension().isActive()); + allVersions.stream() + .map( + v -> v.withCanDelete( + deletableVersions == null || deletableVersions.contains(v.version()))) + .toList()); + json.setActive(extension.isActive()); + json.setRemoved(latest.isRemoved()); } else { var error = "Extension not found: " + NamingUtil.toExtensionId(namespaceName, extensionName); throw new ErrorResultException(error, HttpStatus.NOT_FOUND); @@ -456,7 +522,8 @@ public List getOwnNamespaces() { var namespace = membership.getNamespace(); var extensions = new LinkedHashMap(); var serverUrl = UrlUtil.getBaseUrl(); - repositories.findActiveExtensionsForUrls(namespace).forEach(extension -> { + // return all extension of the namespace, include deleted ones + repositories.findExtensionsForUrls(namespace).forEach(extension -> { String url = createApiUrl(serverUrl, "api", namespace.getName(), extension.getName()); extensions.put(extension.getName(), url); }); diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 6dd65e2f9..899173819 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -11,9 +11,11 @@ import java.time.ZoneId; import java.util.Comparator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import jakarta.persistence.EntityManager; @@ -116,53 +118,71 @@ public void applicationStarted(ApplicationStartedEvent event) { } /** - * Deletes the given extension together with all bundled or dependent extensions. + * Purges the given extension together with every extension that references it — i.e. the + * extension packs that bundle it and the extensions that declare a dependency on it — applied + * recursively, so that nothing is left referencing a purged extension. Note that this walks the + * reverse direction: it does not purge the extensions that the given extension itself + * bundles or depends on. *

- * No further checks are made if the extension is referenced by bundles or as a dependency. + * No dependency check is performed: referencing extensions are purged rather than blocking the + * operation. */ @Transactional(rollbackOn = ErrorResultException.class) - public void purgeExtensionAndDependencies(UserData admin, String namespaceName, String extensionName) + public void purgeExtensionAndReferencingExtensions(UserData admin, String namespaceName, String extensionName) throws ErrorResultException { var extension = extensions.lockExtension(namespaceName, extensionName); - purgeExtensionAndDependencies(admin, extension, 0); + purgeExtensionAndReferencingExtensions(admin, extension, new LinkedHashSet<>()); } - private void purgeExtensionAndDependencies(UserData admin, Extension extension, int depth) - throws ErrorResultException { - if (depth > 5) { - throw new ErrorResultException( - "Failed to delete extension and its dependencies. Exceeded maximum recursion depth.", - HttpStatus.INTERNAL_SERVER_ERROR); - } - - var bundledRefs = repositories.findBundledExtensionsReference(extension); - for (var bundledRef : bundledRefs) { - purgeExtensionAndDependencies(admin, bundledRef, depth); + private void purgeExtensionAndReferencingExtensions( + UserData admin, + Extension extension, + Set purgedExtensionIds + ) throws ErrorResultException { + // Break reference cycles (e.g. two extensions bundling each other) and avoid purging the same + // extension twice: skip if we already started purging it in this recursion. Tracking visited + // extensions (rather than a fixed recursion depth) also lets arbitrarily long reference chains + // be purged without a spurious depth-limit failure. + if (!purgedExtensionIds.add(extension.getId())) { + return; } - var dependRefs = repositories.findDependenciesReference(extension); - for (var dependRef : dependRefs) { - purgeExtensionAndDependencies(admin, dependRef, depth); + // Versions that reference this extension would break once it is purged: extension packs that + // bundle it and extensions that depend on it. A single version can do both, so de-duplicate by + // version id to avoid double-counting (which could skew the per-extension check below). + var referencingVersions = new LinkedHashMap(); + repositories.findBundledExtensionsReference(extension) + .forEach(version -> referencingVersions.putIfAbsent(version.getId(), version)); + repositories.findDependenciesReference(extension) + .forEach(version -> referencingVersions.putIfAbsent(version.getId(), version)); + + // Group the referencing versions by their extension so we can decide, per extension, whether to + // purge it as a whole (all of its versions reference this one) or only the referencing versions. + var referencingByExtensionId = referencingVersions.values().stream() + .collect(Collectors.groupingBy(version -> version.getExtension().getId())); + + for (var versions : referencingByExtensionId.values()) { + var referencingExtension = versions.getFirst().getExtension(); + var totalVersions = repositories.countVersions( + referencingExtension.getNamespace().getName(), + referencingExtension.getName()); + if (versions.size() >= totalVersions) { + // every version of the referencing extension references this one: purge it as a whole + // so that no empty extension record (or its reviews/search entry) is left orphaned. + purgeExtensionAndReferencingExtensions(admin, referencingExtension, purgedExtensionIds); + } else { + // only some versions reference this one: purge just those, keeping the extension. + for (var version : versions) { + extensions.purgeExtensionVersion(admin, version); + } + } } - // We unconditionally purge the extension, - // not checking if there are dependencies on this extension. + // Finally purge this extension itself. We unconditionally purge it, not checking whether other + // extensions reference it, because those referencing extensions have just been purged above. extensions.purgeExtension(admin, extension, false); } - private void purgeExtensionAndDependencies(UserData admin, ExtensionVersion extVersion, int depth) { - var extension = extVersion.getExtension(); - if (repositories.countVersions(extension.getNamespace().getName(), extension.getName()) == 1) { - purgeExtensionAndDependencies(admin, extension, depth + 1); - return; - } - - extensions.removeExtensionVersion(extVersion); - extension.getVersions().remove(extVersion); - extensions.updateExtension(extension); - logs.logAction(admin, ResultJson.success("Deleted " + NamingUtil.toLogFormat(extVersion))); - } - /** * Deletes the given extension unconditionally. No further checks are made if the extension * is referenced by bundles or as a dependency. diff --git a/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java index 99540ed1c..3668c74d6 100644 --- a/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlJobRequestHandler.java @@ -77,7 +77,7 @@ private void processMaliciousExtensions(JsonNode json) { if (extensionId != null && repositories.hasExtension(extensionId.namespace(), extensionId.extension())) { logger.info("delete malicious extension"); if (service.deleteTransitively) { - admin.purgeExtensionAndDependencies( + admin.purgeExtensionAndReferencingExtensions( extensionControlUser, extensionId.namespace(), extensionId.extension()); diff --git a/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlService.java b/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlService.java index a9ad61787..8e8449203 100644 --- a/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlService.java +++ b/server/src/main/java/org/eclipse/openvsx/extension_control/ExtensionControlService.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; import jakarta.persistence.EntityManager; import jakarta.transaction.Transactional; @@ -36,6 +37,7 @@ import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.search.SearchUtilService; import org.eclipse.openvsx.util.ExtensionId; +import org.eclipse.openvsx.util.NamingUtil; import org.eclipse.openvsx.util.TimeUtil; import static org.eclipse.openvsx.cache.CacheService.CACHE_MALICIOUS_EXTENSIONS; @@ -127,13 +129,32 @@ public void updateExtension( } var wasDeprecated = extension.isDeprecated(); + var oldReplacement = extension.getReplacement(); extension.setDeprecated(deprecated); extension.setDownloadable(downloadable); if (replacementId != null) { var replacement = repositories.findExtension(replacementId.extension(), replacementId.namespace()); - extension.setReplacement(replacement); + if (replacement == null || !replacement.isActive()) { + // Never point at a replacement that does not exist or has no active version; such a + // pointer would surface a dead replacement link on the extension. + if (replacement != null) { + logger.info( + "Ignoring inactive replacement {} configured for {}", + NamingUtil.toExtensionId(replacement), + NamingUtil.toExtensionId(extension)); + } + extension.setReplacement(null); + } else { + extension.setReplacement(replacement); + } } - if (deprecated != wasDeprecated) { + + // The replacement is part of the (cached) extension JSON, so evict when it changes too, not + // only when the deprecated flag flips. Compare by id, as the entity equals() is identity-based. + var replacementChanged = !Objects.equals( + oldReplacement != null ? oldReplacement.getId() : null, + extension.getReplacement() != null ? extension.getReplacement().getId() : null); + if (deprecated != wasDeprecated || replacementChanged) { cache.evictNamespaceDetails(extension); cache.evictLatestExtensionVersion(extension); cache.evictExtensionJsons(extension); diff --git a/server/src/main/java/org/eclipse/openvsx/json/VersionTargetPlatformsJson.java b/server/src/main/java/org/eclipse/openvsx/json/VersionTargetPlatformsJson.java index f107de4a9..9481f8cd2 100644 --- a/server/src/main/java/org/eclipse/openvsx/json/VersionTargetPlatformsJson.java +++ b/server/src/main/java/org/eclipse/openvsx/json/VersionTargetPlatformsJson.java @@ -11,4 +11,28 @@ import java.util.List; -public record VersionTargetPlatformsJson(String version, List targetPlatforms) {} +import com.fasterxml.jackson.annotation.JsonInclude; + +/** + * Groups the target platforms available for a single extension version. + * + * @param canDelete whether the current caller is allowed to delete this version. {@code null} means + * "not applicable / unrestricted" (e.g. the public or admin context, where this + * field is omitted from the response); a non-null value is only populated for the + * authenticated user settings view, where namespace owners may delete any version + * while other members may only delete versions they published themselves. + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public record VersionTargetPlatformsJson( + String version, + List targetPlatforms, + Boolean canDelete +) { + public VersionTargetPlatformsJson(String version, List targetPlatforms) { + this(version, targetPlatforms, null); + } + + public VersionTargetPlatformsJson withCanDelete(boolean canDelete) { + return new VersionTargetPlatformsJson(version, targetPlatforms, canDelete); + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java index bcc8fa1f9..65b9f92b9 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java @@ -255,11 +255,10 @@ public String findFirstUnresolvedDependency(List dependencies) { .fetchOne(unresolvedDependency); } - public List findActiveExtensionsForUrls(Namespace namespace) { + public List findExtensionsForUrls(Namespace namespace) { return dsl.select(EXTENSION.ID, EXTENSION.NAME) .from(EXTENSION) .where(EXTENSION.NAMESPACE_ID.eq(namespace.getId())) - .and(EXTENSION.ACTIVE.eq(true)) .fetch(row -> { var extension = new Extension(); extension.setId(row.get(EXTENSION.ID)); 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 6d3b5d72f..3ff1d3888 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java @@ -716,8 +716,8 @@ public List findVersionsForUrls(Extension extension, String ta return extensionVersionJooqRepo.findVersionsForUrls(extension, targetPlatform, version); } - public List findActiveExtensionsForUrls(Namespace namespace) { - return extensionJooqRepo.findActiveExtensionsForUrls(namespace); + public List findExtensionsForUrls(Namespace namespace) { + return extensionJooqRepo.findExtensionsForUrls(namespace); } public ExtensionVersion findExtensionVersion( diff --git a/server/src/main/java/org/eclipse/openvsx/search/SimilarityCheckService.java b/server/src/main/java/org/eclipse/openvsx/search/SimilarityCheckService.java index 08b9150f8..082ae1681 100644 --- a/server/src/main/java/org/eclipse/openvsx/search/SimilarityCheckService.java +++ b/server/src/main/java/org/eclipse/openvsx/search/SimilarityCheckService.java @@ -83,6 +83,8 @@ public PublishCheck.Result check(PublishCheck.Context context) { var extensionName = scan.getExtensionName(); var displayName = scan.getExtensionDisplayName(); + // if at least a version exists for that extension (regardless of active state), + // do not consider it as a new extension anymore if (config.isOnlyCheckNewExtensions() && repositories.countVersions(namespaceName, extensionName) > 0) { return PublishCheck.Result.pass(); } diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java index 112aa3694..88a56c092 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionSoftDeleteTest.java @@ -314,11 +314,11 @@ void purgeExtension_withExplicitTargets_keepsExtensionAndTombstones() { } /** - * Purging all the active versions by naming them explicitly must still only remove those versions; - * the (now inactive) extension record must remain. Only an unscoped purge removes the extension. + * Purging every version of an extension by naming them all removes the extension as a whole, + * so its record is not left orphaned. */ @Test - void purgeExtension_purgingAllActiveVersionsKeepsExtensionEntity() { + void purgeExtension_purgingAllVersionsRemovesExtension() { persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); @@ -331,8 +331,8 @@ void purgeExtension_purgingAllActiveVersionsKeepsExtensionEntity() { assertThat(versionExists("1.0.0")).isFalse(); assertThat(versionExists("2.0.0")).isFalse(); assertThat(extensionExists()) - .as("purging named versions must not remove the extension record, even when they are all active") - .isTrue(); + .as("purging all versions of an extension must remove the extension record too") + .isFalse(); } /** @@ -360,22 +360,58 @@ void purgeExtension_runsDependencyCheckWhenPurgingAllActiveVersions() { } /** - * An unscoped purge (no target versions) removes the whole extension, tombstones and all. + * Duplicate target versions must not inflate the "all active versions" check: purging the same + * subset version twice must be treated as purging that single version, so the dependency guard + * (which only applies when all active versions are removed) does not fire and the other version + * survives. Regression test for the duplicate-driven miscount. + */ + @Test + void purgeExtension_deDuplicatesTargetsBeforeCounting() { + persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + persistVersion("2.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); + + // The extension is depended on: the guard would reject removing ALL active versions. + doReturn(Streamable.of(dependantReference())).when(repositories).findDependenciesReference(any()); + + var duplicate = new TargetPlatformVersion[] { + TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0"), + TargetPlatformVersion.of(TargetPlatform.NAME_UNIVERSAL, "1.0.0") + }; + // Only a single distinct version is targeted, so this is not an "all versions" purge and must + // succeed without tripping the dependency guard. + extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION, duplicate); + + assertThat(versionExists("1.0.0")) + .as("the named version must be purged") + .isFalse(); + assertThat(versionExists("2.0.0")) + .as("a version that was not named must survive") + .isTrue(); + assertThat(extensionExists()) + .as("purging a subset must not remove the extension record") + .isTrue(); + } + + /** + * A purge with no target versions purges nothing: versions must be named explicitly (the + * whole-extension shortcut was removed). */ @Test - void purgeExtension_withEmptyTargets_removesWholeExtensionIncludingTombstones() { + void purgeExtension_withEmptyTargetsIsNoOp() { persistVersion("0.9.0", TargetPlatform.NAME_UNIVERSAL, false, true); // pre-existing tombstone persistVersion("1.0.0", TargetPlatform.NAME_UNIVERSAL, true, false); extensionService.purgeExtensionNoWait(owner(), NAMESPACE, EXTENSION); - assertThat(versionExists("1.0.0")).isFalse(); + assertThat(versionExists("1.0.0")) + .as("an empty-target purge must not remove any version") + .isTrue(); assertThat(versionExists("0.9.0")) - .as("an unscoped purge must remove reserved tombstones too") - .isFalse(); + .as("an empty-target purge must not remove tombstones") + .isTrue(); assertThat(extensionExists()) - .as("an unscoped purge must remove the extension record") - .isFalse(); + .as("an empty-target purge must not remove the extension record") + .isTrue(); } private ExtensionVersion dependantReference() { diff --git a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java index c757055ab..923cf870d 100644 --- a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java @@ -72,6 +72,7 @@ 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.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @WebMvcTest(UserAPI.class) @@ -109,6 +110,9 @@ class UserAPITest { @MockitoBean RepositoryService repositories; + @Autowired + StorageUtilService storageUtil; + @MockitoBean org.eclipse.openvsx.repositories.ExtensionScanRepository scanRepository; @@ -278,7 +282,14 @@ void testOwnNamespacesNotLoggedIn() throws Exception { @Test void testOwnExtension() throws Exception { var userData = mockUserData(); - mockExtension(userData, 2, 0, 0); + var versions = mockExtension(userData, 2, 0, 0); + var namespace = versions.getLast().getExtension().getNamespace(); + // The user is still a member of the namespace, so their published extension is listed. + var membership = new NamespaceMembership(); + membership.setNamespace(namespace); + membership.setUser(userData); + membership.setRole(NamespaceMembership.ROLE_CONTRIBUTOR); + Mockito.when(repositories.findMemberships(userData)).thenReturn(Streamable.of(membership)); mockMvc.perform( get("/user/extensions") .with(user("test_user"))) @@ -291,6 +302,20 @@ void testOwnExtension() throws Exception { }))); } + @Test + void testOwnExtensionExcludesNonMemberNamespace() throws Exception { + var userData = mockUserData(); + mockExtension(userData, 2, 0, 0); + // The user published the extension but is no longer a member of its namespace: it must not + // be listed. findMemberships returns nothing, so the published extension is filtered out. + Mockito.when(repositories.findMemberships(userData)).thenReturn(Streamable.empty()); + mockMvc.perform( + get("/user/extensions") + .with(user("test_user"))) + .andExpect(status().isOk()) + .andExpect(content().json("[]")); + } + @Test void testOwnExtensionNotLoggedIn() throws Exception { var userData = mockUserData(); @@ -299,6 +324,129 @@ void testOwnExtensionNotLoggedIn() throws Exception { .andExpect(status().isForbidden()); } + @Test + void testGetOwnExtensionAsNamespaceOwner() throws Exception { + var userData = mockUserData(); + var versions = mockExtension(userData, 2, 0, 0); + var latest = versions.getLast(); + latest.setId(42L); + var extension = latest.getExtension(); + // The caller owns the namespace: the unscoped lookup is used and every version is deletable. + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.findLatestVersion(eq("foobar"), eq("baz"), any(), eq(false), eq(false))) + .thenReturn(latest); + Mockito.when(repositories.findTargetPlatformsGroupedByVersion(extension)) + .thenReturn( + List.of( + new VersionTargetPlatformsJson( + "2.0.0", + List.of( + new TargetPlatformActiveJson( + TargetPlatform.NAME_UNIVERSAL, + true, + false))), + new VersionTargetPlatformsJson( + "1.0.0", + List.of( + new TargetPlatformActiveJson( + TargetPlatform.NAME_UNIVERSAL, + true, + false))))); + Mockito.when( + storageUtil.getFileUrls( + Mockito.anyCollection(), + Mockito.anyString(), + Mockito.any(String[].class))) + .thenReturn(java.util.Map.of(42L, new java.util.HashMap<>())); + mockMvc.perform( + get("/user/extension/{namespace}/{extension}", "foobar", "baz") + .with(user("test_user"))) + .andExpect(status().isOk()) + .andExpect(content().json("{\"name\":\"baz\",\"namespace\":\"foobar\"}")) + .andExpect(jsonPath("$.allTargetPlatformVersions[0].canDelete").value(true)) + .andExpect(jsonPath("$.allTargetPlatformVersions[1].canDelete").value(true)); + } + + @Test + void testGetOwnExtensionAsNamespaceMember() throws Exception { + var userData = mockUserData(); + var versions = mockExtension(userData, 2, 0, 0); + var latest = versions.getLast(); + latest.setId(42L); + var extension = latest.getExtension(); + // The caller is a namespace member but not an owner: the unscoped lookup lets them see every + // version, but only the versions they published themselves are marked deletable. + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(true); + Mockito.when(repositories.findLatestVersion(eq("foobar"), eq("baz"), any(), eq(false), eq(false))) + .thenReturn(latest); + Mockito.when(repositories.findTargetPlatformsGroupedByVersion(extension)) + .thenReturn( + List.of( + new VersionTargetPlatformsJson( + "2.0.0", + List.of( + new TargetPlatformActiveJson( + TargetPlatform.NAME_UNIVERSAL, + true, + false))), + new VersionTargetPlatformsJson( + "1.0.0", + List.of( + new TargetPlatformActiveJson( + TargetPlatform.NAME_UNIVERSAL, + true, + false))))); + // Only version 1.0.0 was published by this user. + Mockito.when(repositories.findTargetPlatformsGroupedByVersion(extension, userData)) + .thenReturn( + List.of( + new VersionTargetPlatformsJson( + "1.0.0", + List.of( + new TargetPlatformActiveJson( + TargetPlatform.NAME_UNIVERSAL, + true, + false))))); + Mockito.when( + storageUtil.getFileUrls( + Mockito.anyCollection(), + Mockito.anyString(), + Mockito.any(String[].class))) + .thenReturn(java.util.Map.of(42L, new java.util.HashMap<>())); + mockMvc.perform( + get("/user/extension/{namespace}/{extension}", "foobar", "baz") + .with(user("test_user"))) + .andExpect(status().isOk()) + .andExpect(content().json("{\"name\":\"baz\",\"namespace\":\"foobar\"}")) + .andExpect(jsonPath("$.allTargetPlatformVersions[0].version").value("2.0.0")) + .andExpect(jsonPath("$.allTargetPlatformVersions[0].canDelete").value(false)) + .andExpect(jsonPath("$.allTargetPlatformVersions[1].version").value("1.0.0")) + .andExpect(jsonPath("$.allTargetPlatformVersions[1].canDelete").value(true)); + } + + @Test + void testGetOwnExtensionNotMember() throws Exception { + var userData = mockUserData(); + mockExtension(userData, 2, 0, 0); + // Not a namespace member: no access at all, even to versions the user may have published + // while they were still a member => 404 without ever looking up any version. + Mockito.when(repositories.isNamespaceOwner(any(UserData.class), any(Namespace.class))).thenReturn(false); + Mockito.when(repositories.hasMembership(any(UserData.class), any(Namespace.class))).thenReturn(false); + mockMvc.perform( + get("/user/extension/{namespace}/{extension}", "foobar", "baz") + .with(user("test_user"))) + .andExpect(status().isNotFound()); + Mockito.verify(repositories, Mockito.never()) + .findLatestVersion(eq("foobar"), eq("baz"), any(), eq(false), eq(false)); + } + + @Test + void testGetOwnExtensionNotLoggedIn() throws Exception { + mockMvc.perform(get("/user/extension/{namespace}/{extension}", "foobar", "baz")) + .andExpect(status().isForbidden()); + } + @Test void testNamespaceMembers() throws Exception { mockNamespaceMemberships(NamespaceMembership.ROLE_OWNER); diff --git a/server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java b/server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java new file mode 100644 index 000000000..71937f18b --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java @@ -0,0 +1,178 @@ +/******************************************************************************** + * Copyright (c) 2026 Eclipse Foundation and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.admin; + +import java.util.Set; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.util.Streamable; + +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.UserData; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.util.TargetPlatform; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link AdminService#purgeExtensionAndReferencingExtensions(UserData, String, String)}. + *

+ * The cascade that purges an extension together with every extension that references it (packs bundling it, + * extensions depending on it), walking the reverse-reference direction. + */ +@ExtendWith(MockitoExtension.class) +class AdminServiceTest { + + private static final String NAMESPACE = "n"; + + @Mock + RepositoryService repositories; + + @Mock + ExtensionService extensions; + + @InjectMocks + AdminService adminService; + + private final UserData admin = new UserData(); + private long idSequence = 0; + + private Extension extension(String name) { + var namespace = new Namespace(); + namespace.setName(NAMESPACE); + var extension = new Extension(); + extension.setId(++idSequence); + extension.setName(name); + extension.setNamespace(namespace); + return extension; + } + + private ExtensionVersion version(Extension extension) { + var extVersion = new ExtensionVersion(); + extVersion.setId(++idSequence); + extVersion.setExtension(extension); + extVersion.setVersion("1.0.0"); + extVersion.setTargetPlatform(TargetPlatform.NAME_UNIVERSAL); + return extVersion; + } + + private void mockNoReferences(Extension extension) { + when(repositories.findBundledExtensionsReference(extension)).thenReturn(Streamable.empty()); + when(repositories.findDependenciesReference(extension)).thenReturn(Streamable.empty()); + } + + @Test + void purgesReferencingExtensionAsWholeWhenAllVersionsReference() { + var target = extension("target"); + var referencing = extension("referencing"); + // The referencing extension has two versions, both bundling the target. + var refV1 = version(referencing); + var refV2 = version(referencing); + + when(extensions.lockExtension(NAMESPACE, "target")).thenReturn(target); + when(repositories.findBundledExtensionsReference(target)).thenReturn(Streamable.of(refV1, refV2)); + when(repositories.findDependenciesReference(target)).thenReturn(Streamable.empty()); + when(repositories.countVersions(NAMESPACE, "referencing")).thenReturn(2); + mockNoReferences(referencing); + + adminService.purgeExtensionAndReferencingExtensions(admin, NAMESPACE, "target"); + + // The referencing extension is purged as a whole (not version-by-version) so nothing is orphaned. + verify(extensions).purgeExtension(admin, referencing, false); + verify(extensions).purgeExtension(admin, target, false); + verify(extensions, never()).purgeExtensionVersion(any(), any()); + } + + @Test + void purgesOnlyReferencingVersionsWhenSomeVersionsReference() { + var target = extension("target"); + var referencing = extension("referencing"); + // Only one of the referencing extension's three versions bundles the target. + var refV1 = version(referencing); + + when(extensions.lockExtension(NAMESPACE, "target")).thenReturn(target); + when(repositories.findBundledExtensionsReference(target)).thenReturn(Streamable.of(refV1)); + when(repositories.findDependenciesReference(target)).thenReturn(Streamable.empty()); + when(repositories.countVersions(NAMESPACE, "referencing")).thenReturn(3); + + adminService.purgeExtensionAndReferencingExtensions(admin, NAMESPACE, "target"); + + // Only the referencing version is purged; the extension keeps its other versions. + verify(extensions).purgeExtensionVersion(admin, refV1); + verify(extensions, never()).purgeExtension(admin, referencing, false); + verify(extensions).purgeExtension(admin, target, false); + } + + @Test + void handlesReferenceCyclesWithoutInfiniteRecursion() { + // a and b bundle each other; both are single-version extensions. + var a = extension("a"); + var b = extension("b"); + var aV = version(a); + var bV = version(b); + + when(extensions.lockExtension(NAMESPACE, "a")).thenReturn(a); + // versions bundling a -> b's version; versions bundling b -> a's version + when(repositories.findBundledExtensionsReference(a)).thenReturn(Streamable.of(bV)); + when(repositories.findDependenciesReference(a)).thenReturn(Streamable.empty()); + when(repositories.findBundledExtensionsReference(b)).thenReturn(Streamable.of(aV)); + when(repositories.findDependenciesReference(b)).thenReturn(Streamable.empty()); + when(repositories.countVersions(NAMESPACE, "a")).thenReturn(1); + when(repositories.countVersions(NAMESPACE, "b")).thenReturn(1); + + assertThatCode(() -> adminService.purgeExtensionAndReferencingExtensions(admin, NAMESPACE, "a")) + .doesNotThrowAnyException(); + + verify(extensions).purgeExtension(admin, a, false); + verify(extensions).purgeExtension(admin, b, false); + verify(extensions, never()).purgeExtensionVersion(any(), any()); + } + + @Test + void purgesDeepReferenceChainWithoutDepthLimit() { + // Chain of 8 single-version extensions where each references the previous one; the previous + // depth limit (> 5) would have aborted this legitimate chain, the visited-set does not. + var chain = new Extension[8]; + var chainVersions = new ExtensionVersion[8]; + for (var i = 0; i < chain.length; i++) { + chain[i] = extension("e" + i); + chainVersions[i] = version(chain[i]); + } + + when(extensions.lockExtension(NAMESPACE, "e0")).thenReturn(chain[0]); + for (var i = 0; i < chain.length; i++) { + // chain[i+1] references chain[i] + when(repositories.findBundledExtensionsReference(chain[i])) + .thenReturn(i + 1 < chain.length ? Streamable.of(chainVersions[i + 1]) : Streamable.empty()); + when(repositories.findDependenciesReference(chain[i])).thenReturn(Streamable.empty()); + if (i > 0) { + when(repositories.countVersions(NAMESPACE, "e" + i)).thenReturn(1); + } + } + + assertThatCode(() -> adminService.purgeExtensionAndReferencingExtensions(admin, NAMESPACE, "e0")) + .doesNotThrowAnyException(); + + for (var extension : chain) { + verify(extensions).purgeExtension(admin, extension, false); + } + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/extension_control/ExtensionControlServiceTest.java b/server/src/test/java/org/eclipse/openvsx/extension_control/ExtensionControlServiceTest.java new file mode 100644 index 000000000..1ad14e4e5 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/extension_control/ExtensionControlServiceTest.java @@ -0,0 +1,161 @@ +/******************************************************************************** + * Copyright (c) 2026 Eclipse Foundation and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.extension_control; + +import jakarta.persistence.EntityManager; +import org.jobrunr.scheduling.JobRequestScheduler; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import org.eclipse.openvsx.cache.CacheService; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.search.SearchUtilService; +import org.eclipse.openvsx.util.ExtensionId; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link ExtensionControlService#updateExtension}, focusing on how the replacement is + * resolved (never pointing at a missing/inactive extension) and on cache/search invalidation when the + * replacement changes. + */ +@ExtendWith(MockitoExtension.class) +class ExtensionControlServiceTest { + + private static final String NAMESPACE = "n"; + + @Mock + JobRequestScheduler scheduler; + + @Mock + RepositoryService repositories; + + @Mock + EntityManager entityManager; + + @Mock + SearchUtilService search; + + @Mock + CacheService cache; + + @InjectMocks + ExtensionControlService service; + + private long idSequence = 0; + + private Extension extension(String name, boolean deprecated, boolean active) { + var namespace = new Namespace(); + namespace.setName(NAMESPACE); + var extension = new Extension(); + extension.setId(++idSequence); + extension.setName(name); + extension.setNamespace(namespace); + extension.setDeprecated(deprecated); + extension.setActive(active); + return extension; + } + + private void mockExtension(Extension extension) { + when(repositories.findExtension(extension.getName(), NAMESPACE)).thenReturn(extension); + } + + private void verifyCachesEvicted(Extension extension) { + verify(cache).evictNamespaceDetails(extension); + verify(cache).evictLatestExtensionVersion(extension); + verify(cache).evictExtensionJsons(extension); + verify(search).updateSearchEntry(extension); + } + + @Test + void doesNotPointAtInactiveReplacement() { + var extension = extension("ext", true, true); + var replacement = extension("replacement", false, false); // inactive + mockExtension(extension); + mockExtension(replacement); + + service.updateExtension( + new ExtensionId(NAMESPACE, "ext"), + true, + new ExtensionId(NAMESPACE, "replacement"), + true); + + assertThat(extension.getReplacement()) + .as("an inactive replacement must not be set") + .isNull(); + } + + @Test + void pointsAtActiveReplacement() { + var extension = extension("ext", true, true); + var replacement = extension("replacement", false, true); // active + mockExtension(extension); + mockExtension(replacement); + + service.updateExtension( + new ExtensionId(NAMESPACE, "ext"), + true, + new ExtensionId(NAMESPACE, "replacement"), + true); + + assertThat(extension.getReplacement()).isSameAs(replacement); + // The replacement changed (null -> replacement) so caches must be evicted even though the + // deprecated flag did not change. + verifyCachesEvicted(extension); + } + + @Test + void evictsCachesWhenReplacementClearedWhileDeprecationUnchanged() { + var previousReplacement = extension("old-replacement", false, true); + var extension = extension("ext", true, true); // already deprecated + extension.setReplacement(previousReplacement); + var replacement = extension("replacement", false, false); // now inactive -> must be cleared + mockExtension(extension); + mockExtension(replacement); + + service.updateExtension( + new ExtensionId(NAMESPACE, "ext"), + true, + new ExtensionId(NAMESPACE, "replacement"), + true); + + assertThat(extension.getReplacement()) + .as("clearing an inactive replacement must null it out") + .isNull(); + verifyCachesEvicted(extension); + } + + @Test + void doesNotEvictCachesWhenNothingChanged() { + var replacement = extension("replacement", false, true); + var extension = extension("ext", true, true); // already deprecated + extension.setReplacement(replacement); + mockExtension(extension); + mockExtension(replacement); + + service.updateExtension( + new ExtensionId(NAMESPACE, "ext"), + true, + new ExtensionId(NAMESPACE, "replacement"), + true); + + assertThat(extension.getReplacement()).isSameAs(replacement); + verify(cache, never()).evictExtensionJsons(extension); + verify(search, never()).updateSearchEntry(extension); + } +} 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 fd44d3867..2c2332e33 100644 --- a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java +++ b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java @@ -319,7 +319,7 @@ void testExecuteQueries() { () -> repositories .findSignatureKeyPairPublicId("namespaceName", "extensionName", "targetPlatform", "version"), () -> repositories.findFirstMembership("namespaceName"), - () -> repositories.findActiveExtensionsForUrls(namespace), + () -> repositories.findExtensionsForUrls(namespace), () -> repositories.deactivateKeyPairs(), () -> repositories.hasExtension("namespaceName", "extensionName"), () -> repositories.findDeprecatedExtensions(extension), diff --git a/webui/src/components/extension/extension-detail-view.tsx b/webui/src/components/extension/extension-detail-view.tsx index 5b3cc94c4..2e79d02d6 100644 --- a/webui/src/components/extension/extension-detail-view.tsx +++ b/webui/src/components/extension/extension-detail-view.tsx @@ -39,8 +39,12 @@ export const ExtensionDetailView: FunctionComponent = const publicRoute = createRoute([ExtensionDetailRoutes.ROOT, extension.namespace, extension.name]); const allVersions = (extension.allTargetPlatformVersions ?? []).filter(v => !VERSION_ALIASES.includes(v.version)); + // Versions the current user is allowed to delete. canDelete is only populated in the user settings + // view (undefined means unrestricted, e.g. admin/purge); a non-owner member may only delete the + // versions they published themselves. "Delete All Versions" therefore operates on this subset. + const deletableVersions = canPurge ? allVersions : allVersions.filter(v => v.canDelete !== false); // A version can still be (soft-)deleted while it has at least one target platform that is not removed. - const hasDeletableVersions = allVersions.some(v => v.targetPlatforms.some(tp => !tp.removed)); + const hasDeletableVersions = deletableVersions.some(v => v.targetPlatforms.some(tp => !tp.removed)); return ( @@ -112,7 +116,7 @@ export const ExtensionDetailView: FunctionComponent = open={true} onClose={() => setDeleteAllOpen(false)} extension={extension} - versions={allVersions} + versions={deletableVersions} onRemove={onRemoveVersion} onDeleted={onVersionDeleted} /> diff --git a/webui/src/components/extension/extension-version-table.tsx b/webui/src/components/extension/extension-version-table.tsx index 4b327d94a..ebee878ca 100644 --- a/webui/src/components/extension/extension-version-table.tsx +++ b/webui/src/components/extension/extension-version-table.tsx @@ -58,6 +58,14 @@ export const ExtensionVersionTable: FunctionComponent {pagedVersions.map(v => { const allRemoved = v.targetPlatforms.length > 0 && v.targetPlatforms.every(tp => tp.removed); + // canDelete is only populated in the user settings view; undefined means unrestricted. + const notPublisher = v.canDelete === false; + const deleteDisabled = allRemoved || notPublisher; + const deleteTitle = notPublisher + ? 'Only the publisher or a namespace owner can delete this version' + : allRemoved + ? 'Version already removed' + : 'Delete version'; return ( @@ -88,10 +96,10 @@ export const ExtensionVersionTable: FunctionComponent onDeleteVersion(v)}> - + {onPurgeVersion && ( diff --git a/webui/src/extension-registry-types.ts b/webui/src/extension-registry-types.ts index c9aaddf77..f0c8379f6 100644 --- a/webui/src/extension-registry-types.ts +++ b/webui/src/extension-registry-types.ts @@ -144,6 +144,10 @@ export interface TargetPlatformActive { export interface VersionTargetPlatforms { version: string; targetPlatforms: TargetPlatformActive[]; + // Whether the current user may delete this version. Omitted (undefined) in contexts where deletion + // is unrestricted (admin) or not applicable (public); explicitly false for namespace members who + // did not publish this version and therefore may only delete their own versions. + canDelete?: boolean; } export type StarRating = 1 | 2 | 3 | 4 | 5; diff --git a/webui/src/pages/user/user-settings-namespaces.tsx b/webui/src/pages/user/user-settings-namespaces.tsx index 88b8c1bb6..92c106943 100644 --- a/webui/src/pages/user/user-settings-namespaces.tsx +++ b/webui/src/pages/user/user-settings-namespaces.tsx @@ -118,6 +118,9 @@ export const UserSettingsNamespaces: FunctionComponent = () => { filterUsers={(foundUser: UserData) => foundUser.provider !== user?.provider || foundUser.loginName !== user?.loginName } + fetchExtension={(abortController, extension) => + service.getExtension(abortController, chosenNamespace.name, extension.name) + } fixSelf={true} namespaceAccessUrl={namespaceAccessUrl} theme={pageSettings.themeType}