fix: consolidating deletion logic in ExtensionService

server/src/main/java/org/eclipse/openvsx/ExtensionService.java

@@ -218,14 +218,47 @@ public void reactivateExtensions(UserData user) {
 
     @Transactional(rollbackOn = ErrorResultException.class)
     public ResultJson deleteExtension(
+            UserData user,
             String namespaceName,
             String extensionName,
-            List<TargetPlatformVersionJson> targetVersions,
-            UserData user
+            List<TargetPlatformVersionJson> targetVersions
     ) throws ErrorResultException {
-        // Lock the extension row (NOWAIT) so a delete-all and a concurrent publish can't interleave.
-        // Publishing takes the same lock (waiting); if the lock can not be acquired, this fails fast,
-        // and we ask the user to retry.
+        var extension = lockExtension(namespaceName, extensionName);
+        if (repositories.isDeleteAllVersions(namespaceName, extensionName, targetVersions, user)) {
+            return deleteExtension(user, extension);
+        }
+
+        // A user may only delete versions they published; foreign/unknown versions resolve to null
+        // and surface as 404 (without leaking that another publisher's version exists).
+        return deleteExtensionVersions(user, targetVersions.stream()
+                .map(target -> {
+                    var extVersion = repositories.findVersionForUser(user, 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());
+    }
+
+    /**
+     * Deletes the given pre-resolved extension versions without any ownership check.
+     * Callers are responsible for authorisation (e.g. scoping the lookup to the owning user
+     * before calling this, or using an admin-level unscoped lookup).
+     */
+    public ResultJson deleteExtensionVersions(UserData user, List<ExtensionVersion> versions) {
+        var results = new ArrayList<ResultJson>();
+        for (var version : versions) {
+            results.add(deleteExtension(user, version));
+        }
+        return combineResults(results);
+    }
+
+    /**
+     * Locks the extension row ({@code SELECT … FOR UPDATE NOWAIT}).
+     * this fails fast with {@code 409}
+     */
+    public Extension lockExtension(String namespaceName, String extensionName) throws ErrorResultException {
         Extension extension;
         try {
             extension = repositories.findExtensionForUpdateNoWait(extensionName, namespaceName);
@@ -235,34 +268,26 @@ public ResultJson deleteExtension(
                             + " can not be locked due to concurrent modification. Please try again.",
                     HttpStatus.CONFLICT);
         }
-
         if (extension == null) {
-            var message = "Extension not found: " + NamingUtil.toExtensionId(namespaceName, extensionName);
-            throw new ErrorResultException(message, HttpStatus.NOT_FOUND);
-        }
-
-        var results = new ArrayList<ResultJson>();
-        if (repositories.isDeleteAllVersions(namespaceName, extensionName, targetVersions, user)) {
-            results.add(deleteExtension(user, extension));
-        } else {
-            for (var targetVersion : targetVersions) {
-                var extVersion = repositories.findVersion(user, targetVersion.version(), targetVersion.targetPlatform(), extensionName, namespaceName);
-                if (extVersion == null) {
-                    var message = "Extension not found: " + NamingUtil.toLogFormat(namespaceName, extensionName, targetVersion.targetPlatform(), targetVersion.version());
-                    throw new ErrorResultException(message, HttpStatus.NOT_FOUND);
-                }
-
-                results.add(deleteExtension(user, extVersion));
-            }
+            throw new ErrorResultException(
+                    "Extension not found: " + NamingUtil.toExtensionId(namespaceName, extensionName),
+                    HttpStatus.NOT_FOUND);
         }
+        return extension;
+    }
 
+    /**
+     * Merges the per-version delete outcomes into a single result, concatenating any success and
+     * error messages.
+     */
+    public ResultJson combineResults(List<ResultJson> results) {
         var result = new ResultJson();
         result.setError(results.stream().map(ResultJson::getError).filter(Objects::nonNull).collect(Collectors.joining("\n")));
         result.setSuccess(results.stream().map(ResultJson::getSuccess).filter(Objects::nonNull).collect(Collectors.joining("\n")));
         return result;
     }
 
-    protected ResultJson deleteExtension(UserData user, Extension extension) throws ErrorResultException {
+    public ResultJson deleteExtension(UserData user, Extension extension) throws ErrorResultException {
         var bundledRefs = repositories.findBundledExtensionsReference(extension);
         if (!bundledRefs.isEmpty()) {
             throw new ErrorResultException("Extension " + NamingUtil.toExtensionId(extension)
@@ -280,13 +305,14 @@ protected ResultJson deleteExtension(UserData user, Extension extension) throws
         }
 
         cache.evictExtensionJsons(extension);
+        cache.evictNamespaceDetails(extension);
+        cache.evictLatestExtensionVersion(extension);
         for (var extVersion : repositories.findVersions(extension)) {
             removeExtensionVersion(extVersion);
         }
         for (var review : repositories.findAllReviews(extension)) {
             entityManager.remove(review);
         }
-
         var deprecatedExtensions = repositories.findDeprecatedExtensions(extension);
         for (var deprecatedExtension : deprecatedExtensions) {
             deprecatedExtension.setReplacement(null);
@@ -301,7 +327,7 @@ protected ResultJson deleteExtension(UserData user, Extension extension) throws
         return result;
     }
 
-    protected ResultJson deleteExtension(UserData user, ExtensionVersion extVersion) {
+    public ResultJson deleteExtension(UserData user, ExtensionVersion extVersion) {
         var extension = extVersion.getExtension();
         removeExtensionVersion(extVersion);
         extension.getVersions().remove(extVersion);

server/src/main/java/org/eclipse/openvsx/UserAPI.java

@@ -20,7 +20,6 @@
 
 import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.openvsx.accesstoken.AccessTokenService;
@@ -393,7 +392,7 @@ public ResponseEntity<ResultJson> deleteExtension(
             throw new ResponseStatusException(HttpStatus.FORBIDDEN);
         }
         try {
-            var result = extensions.deleteExtension(namespaceName, extensionName, targetVersions, user);
+            var result = extensions.deleteExtension(user, namespaceName, extensionName, targetVersions);
             return ResponseEntity.ok(result);
         } catch (ErrorResultException exc) {
             return exc.toResponseEntity();

server/src/main/java/org/eclipse/openvsx/admin/AdminService.java

@@ -18,7 +18,6 @@
 import static org.eclipse.openvsx.entities.FileResource.VSIXMANIFEST;
 
 import java.time.ZoneId;
-import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
@@ -53,7 +52,6 @@
 import org.eclipse.openvsx.mail.MailService;
 import org.eclipse.openvsx.migration.HandlerJobRequest;
 import org.eclipse.openvsx.repositories.RepositoryService;
-import org.eclipse.openvsx.scanning.ExtensionScanPersistenceService;
 import org.eclipse.openvsx.search.SearchUtilService;
 import org.eclipse.openvsx.storage.StorageUtilService;
 import org.eclipse.openvsx.util.ErrorResultException;
@@ -90,7 +88,6 @@ public class AdminService {
     private final JobRequestScheduler scheduler;
     private final MailService mail;
     private final LogService logs;
-    private final ExtensionScanPersistenceService scanPersistenceService;
 
     public AdminService(
             RepositoryService repositories,
@@ -105,8 +102,7 @@ public AdminService(
             CacheService cache,
             JobRequestScheduler scheduler,
             MailService mail,
-            LogService logs,
-            ExtensionScanPersistenceService scanPersistenceService
+            LogService logs
     ) {
         this.repositories = repositories;
         this.extensions = extensions;
@@ -121,7 +117,6 @@ public AdminService(
         this.scheduler = scheduler;
         this.mail = mail;
         this.logs = logs;
-        this.scanPersistenceService = scanPersistenceService;
     }
 
     @EventListener
@@ -156,28 +151,7 @@ public void deleteExtensionAndDependencies(Extension extension, UserData admin,
             deleteExtensionAndDependencies(dependRef, admin, depth);
         }
 
-        for (var extVersion : repositories.findVersions(extension)) {
-            removeExtensionVersion(extVersion);
-        }
-        for (var review : repositories.findAllReviews(extension)) {
-            entityManager.remove(review);
-        }
-
-        var deprecatedExtensions = repositories.findDeprecatedExtensions(extension);
-        for (var deprecatedExtension : deprecatedExtensions) {
-            deprecatedExtension.setReplacement(null);
-            cache.evictExtensionJsons(deprecatedExtension);
-        }
-
-        entityManager.remove(extension);
-
-        // evict the cache entries only after the changes have been commited
-        cache.evictExtensionJsons(extension);
-        cache.evictNamespaceDetails(extension);
-        cache.evictLatestExtensionVersion(extension);
-
-        search.removeSearchEntry(extension);
-        logs.logAction(admin, ResultJson.success("Deleted " + NamingUtil.toExtensionId(extension)));
+        extensions.deleteExtension(admin, extension);
     }
 
     protected void deleteExtensionAndDependencies(ExtensionVersion extVersion, UserData admin, int depth) {
@@ -187,10 +161,15 @@ protected void deleteExtensionAndDependencies(ExtensionVersion extVersion, UserD
             return;
         }
 
-        removeExtensionVersion(extVersion);
-        extension.getVersions().remove(extVersion);
-        extensions.updateExtension(extension);
-        logs.logAction(admin, ResultJson.success("Deleted " + NamingUtil.toLogFormat(extVersion)));
+        extensions.deleteExtension(admin, extVersion);
+    }
+
+    public ResultJson deleteExtension(
+            UserData adminUser,
+            String namespaceName,
+            String extensionName
+    ) {
+        return this.deleteExtension(adminUser, namespaceName, extensionName, null);
     }
 
     @Transactional(rollbackOn = ErrorResultException.class)
@@ -200,99 +179,21 @@ public ResultJson deleteExtension(
             String extensionName,
             List<TargetPlatformVersionJson> targetVersions
     ) {
+        var extension = extensions.lockExtension(namespaceName, extensionName);
         if (targetVersions == null || repositories.countVersions(namespaceName, extensionName) == targetVersions.size()) {
-            return deleteExtension(namespaceName, extensionName, adminUser);
-        }
-
-        var results = new ArrayList<ResultJson>();
-        for (var targetVersion : targetVersions) {
-            results.add(deleteExtension(namespaceName, extensionName, targetVersion.targetPlatform(), targetVersion.version(), adminUser));
-        }
-
-        var result = new ResultJson();
-        result.setError(results.stream().map(ResultJson::getError).filter(Objects::nonNull).collect(Collectors.joining("\n")));
-        result.setSuccess(results.stream().map(ResultJson::getSuccess).filter(Objects::nonNull).collect(Collectors.joining("\n")));
-        return result;
-    }
-
-    @Transactional(rollbackOn = ErrorResultException.class)
-    public ResultJson deleteExtension(String namespaceName, String extensionName, UserData admin)
-            throws ErrorResultException {
-        var extension = repositories.findExtension(extensionName, namespaceName);
-        if (extension == null) {
-            var extensionId = NamingUtil.toExtensionId(namespaceName, extensionName);
-            throw new ErrorResultException("Extension not found: " + extensionId, HttpStatus.NOT_FOUND);
-        }
-        return deleteExtension(extension, admin);
-    }
-
-    @Transactional(rollbackOn = ErrorResultException.class)
-    public ResultJson deleteExtension(String namespaceName, String extensionName, String targetPlatform, String version, UserData admin)
-            throws ErrorResultException {
-        var extVersion = repositories.findVersion(version, targetPlatform, extensionName, namespaceName);
-        if (extVersion == null) {
-            var message = "Extension not found: " + NamingUtil.toLogFormat(namespaceName, extensionName, targetPlatform, version);
-
-            throw new ErrorResultException(message, HttpStatus.NOT_FOUND);
-        }
-
-        return deleteExtension(extVersion, admin);
-    }
-
-    protected ResultJson deleteExtension(Extension extension, UserData admin) 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(", ")));
+            return extensions.deleteExtension(adminUser, extension);
         }
 
-        for (var extVersion : repositories.findVersions(extension)) {
-            removeExtensionVersion(extVersion);
-        }
-
-        for (var review : repositories.findAllReviews(extension)) {
-            entityManager.remove(review);
-        }
-
-        var deprecatedExtensions = repositories.findDeprecatedExtensions(extension);
-        for (var deprecatedExtension : deprecatedExtensions) {
-            deprecatedExtension.setReplacement(null);
-            cache.evictExtensionJsons(deprecatedExtension);
-        }
-
-        entityManager.remove(extension);
-
-        // evict the cache entries only after the changes have been commited
-        cache.evictExtensionJsons(extension);
-        cache.evictNamespaceDetails(extension);
-        cache.evictLatestExtensionVersion(extension);
-
-        search.removeSearchEntry(extension);
-
-        var result = ResultJson.success("Deleted " + NamingUtil.toExtensionId(extension));
-        logs.logAction(admin, result);
-        return result;
-    }
-
-    protected ResultJson deleteExtension(ExtensionVersion extVersion, UserData admin) {
-        var extension = extVersion.getExtension();
-        removeExtensionVersion(extVersion);
-        extension.getVersions().remove(extVersion);
-        extensions.updateExtension(extension);
-
-        var result = ResultJson.success("Deleted " + NamingUtil.toLogFormat(extVersion));
-        logs.logAction(admin, result);
-        return result;
+        // Admins may delete any publisher's version, so the unscoped lookup is used here.
+        return extensions.deleteExtensionVersions(adminUser, targetVersions.stream()
+                .map(target -> {
+                    var extVersion = 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());
     }
 
     @Transactional(rollbackOn = ErrorResultException.class)
@@ -333,16 +234,6 @@ private ResultJson deleteNamespace(Namespace namespace, UserData admin) {
         return result;
     }
 
-    private void removeExtensionVersion(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);
-        entityManager.remove(extVersion);
-    }
-
     private String userNotFoundMessage(String user) {
         return "User not found: " + user;
     }

server/src/main/java/org/eclipse/openvsx/migration/FixTargetPlatformsJobRequestHandler.java

@@ -13,6 +13,7 @@
 import org.eclipse.openvsx.ExtensionService;
 import org.eclipse.openvsx.admin.AdminService;
 import org.eclipse.openvsx.entities.ExtensionVersion;
+import org.eclipse.openvsx.json.TargetPlatformVersionJson;
 import org.eclipse.openvsx.util.NamingUtil;
 import org.jobrunr.jobs.annotations.Job;
 import org.jobrunr.jobs.context.JobRunrDashboardLogger;
@@ -23,6 +24,7 @@
 import org.springframework.stereotype.Component;
 
 import java.nio.file.Files;
+import java.util.List;
 
 @Component
 @ConditionalOnProperty(value = "ovsx.data.mirror.enabled", havingValue = "false", matchIfMissing = true)
@@ -79,11 +81,10 @@ public void run(MigrationJobRequest jobRequest) throws Exception {
     private void deleteExtension(ExtensionVersion extVersion) {
         var extension = extVersion.getExtension();
         admins.deleteExtension(
+                service.getUser(),
                 extension.getNamespace().getName(),
                 extension.getName(),
-                extVersion.getTargetPlatform(),
-                extVersion.getVersion(),
-                service.getUser()
+                List.of(new TargetPlatformVersionJson(extVersion.getTargetPlatform(), extVersion.getVersion()))
         );
     }
 }