From e3f00dd15ecd5171b118b7f024943522080f483f Mon Sep 17 00:00:00 2001 From: amvanbaren Date: Mon, 17 Jul 2023 19:09:57 +0300 Subject: [PATCH] Use job instead of @Async --- .../eclipse/openvsx/ExtensionProcessor.java | 7 +- .../org/eclipse/openvsx/ExtensionService.java | 17 +++- .../PublishExtensionVersionHandler.java | 59 ++----------- .../publish/PublishExtensionVersionJob.java | 86 ++++++++++++++++++ .../PublishExtensionVersionJobRequest.java | 37 ++++++++ .../PublishExtensionVersionJobService.java | 87 +++++++++++++++++++ .../PublishExtensionVersionService.java | 60 +------------ .../org/eclipse/openvsx/RegistryAPITest.java | 3 +- .../openvsx/eclipse/EclipseServiceTest.java | 6 +- 9 files changed, 244 insertions(+), 118 deletions(-) create mode 100644 server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJob.java create mode 100644 server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobRequest.java create mode 100644 server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobService.java diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java index 38793ce9d..d32ed4558 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java @@ -339,7 +339,12 @@ public FileResource getBinary(ExtensionVersion extVersion, String binaryName) { binary.setExtension(extVersion); binary.setName(binaryName); binary.setType(FileResource.DOWNLOAD); - binary.setContent(null); + try { + binary.setContent(Files.readAllBytes(extensionFile.getPath())); + } catch (IOException e) { + throw new RuntimeException(e); + } + return binary; } diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index 1caa5d052..8e362e4f1 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -23,11 +23,13 @@ import org.eclipse.openvsx.cache.CacheService; import org.eclipse.openvsx.entities.*; import org.eclipse.openvsx.publish.PublishExtensionVersionHandler; +import org.eclipse.openvsx.publish.PublishExtensionVersionJobRequest; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.search.SearchUtilService; import org.eclipse.openvsx.util.ErrorResultException; import org.eclipse.openvsx.util.TempFile; import org.eclipse.openvsx.util.TimeUtil; +import org.jobrunr.scheduling.JobRequestScheduler; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpStatus; @@ -50,6 +52,9 @@ public class ExtensionService { @Autowired PublishExtensionVersionHandler publishHandler; + @Autowired + JobRequestScheduler scheduler; + @Value("${ovsx.publishing.require-license:false}") boolean requireLicense; @@ -61,10 +66,14 @@ public ExtensionVersion mirrorVersion(TempFile extensionFile, String signatureNa } public ExtensionVersion publishVersion(InputStream content, PersonalAccessToken token) { - var extensionFile = createExtensionFile(content); - var download = doPublish(extensionFile, null, token, TimeUtil.getCurrentUTC(), true); - publishHandler.publishAsync(download, extensionFile, this); - return download.getExtension(); + try(var extensionFile = createExtensionFile(content)) { + var download = doPublish(extensionFile, null, token, TimeUtil.getCurrentUTC(), true); + publishHandler.persistDownload(download); + scheduler.enqueue(new PublishExtensionVersionJobRequest(download.getId())); + return download.getExtension(); + } catch (IOException e) { + throw new RuntimeException(e); + } } private FileResource doPublish(TempFile extensionFile, String binaryName, PersonalAccessToken token, LocalDateTime timestamp, boolean checkDependencies) { 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 41c2a8dcc..09284efd3 100644 --- a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java @@ -10,8 +10,9 @@ package org.eclipse.openvsx.publish; import com.google.common.base.Joiner; +import jakarta.persistence.EntityManager; +import jakarta.transaction.Transactional; import org.eclipse.openvsx.ExtensionProcessor; -import org.eclipse.openvsx.ExtensionService; import org.eclipse.openvsx.ExtensionValidator; import org.eclipse.openvsx.UserService; import org.eclipse.openvsx.adapter.VSCodeIdService; @@ -23,18 +24,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.retry.annotation.Retryable; -import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Component; -import jakarta.persistence.EntityManager; -import jakarta.transaction.Transactional; -import java.io.IOException; import java.time.LocalDateTime; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.function.Consumer; import java.util.stream.Collectors; @Component @@ -201,50 +196,6 @@ private List updateExistingPublicIds(Extension extension) { return updatedExtensions; } - @Async - @Retryable - public void publishAsync(FileResource download, TempFile extensionFile, ExtensionService extensionService) { - var extVersion = download.getExtension(); - - // Delete file resources in case publishAsync is retried - service.deleteFileResources(extVersion); - download.setId(0L); - - service.storeDownload(download, extensionFile); - service.persistResource(download); - try(var processor = new ExtensionProcessor(extensionFile)) { - Consumer consumer = resource -> { - service.storeResource(resource); - service.persistResource(resource); - }; - - if(integrityService.isEnabled()) { - var keyPair = extVersion.getSignatureKeyPair(); - if(keyPair != null) { - var signature = integrityService.generateSignature(download, extensionFile, keyPair); - consumer.accept(signature); - } else { - // Can happen when GenerateKeyPairJobRequestHandler hasn't run yet and there is no active SignatureKeyPair. - // This extension version should be assigned a SignatureKeyPair and a signature FileResource should be created - // by the ExtensionVersionSignatureJobRequestHandler migration. - logger.warn("Integrity service is enabled, but {} did not have an active key pair", NamingUtil.toLogFormat(extVersion)); - } - } - - processor.processEachResource(extVersion, consumer); - processor.getFileResources(extVersion).forEach(consumer); - consumer.accept(processor.generateSha256Checksum(extVersion)); - } - - // Update whether extension is active, the search index and evict cache - service.activateExtension(extVersion, extensionService); - try { - extensionFile.close(); - } catch (IOException e) { - logger.error("failed to delete temp file", e); - } - } - public void mirror(FileResource download, TempFile extensionFile, String signatureName) { var extVersion = download.getExtension(); service.mirrorResource(download); @@ -265,4 +216,10 @@ private FileResource getSignatureResource(String signatureName, ExtensionVersion resource.setType(FileResource.DOWNLOAD_SIG); return resource; } + + @Transactional + public void persistDownload(FileResource download) { + download.setStorageType(FileResource.STORAGE_DB); + entityManager.persist(download); + } } diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJob.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJob.java new file mode 100644 index 000000000..053530e44 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJob.java @@ -0,0 +1,86 @@ +/** ****************************************************************************** + * Copyright (c) 2023 Precies. Software Ltd 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.publish; + +import org.eclipse.openvsx.ExtensionProcessor; +import org.eclipse.openvsx.entities.FileResource; +import org.eclipse.openvsx.migration.MigrationService; +import org.eclipse.openvsx.util.NamingUtil; +import org.jobrunr.jobs.lambdas.JobRequestHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +import java.util.AbstractMap; +import java.util.function.Consumer; + +@Component +public class PublishExtensionVersionJob implements JobRequestHandler { + + protected final Logger logger = LoggerFactory.getLogger(PublishExtensionVersionJob.class); + + @Autowired + ExtensionVersionIntegrityService integrityService; + + @Autowired + PublishExtensionVersionJobService service; + + @Autowired + MigrationService migrations; + + @Override + public void run(PublishExtensionVersionJobRequest jobRequest) throws Exception { + var download = service.getFileResource(jobRequest.getDownloadId()); + var extVersion = download.getExtension(); + logger.info("Processing files for {}", NamingUtil.toLogFormat(extVersion)); + + // Delete file resources in case job is retried + service.deleteFileResources(extVersion); + + var content = migrations.getContent(download); + var extensionFile = migrations.getExtensionFile(new AbstractMap.SimpleEntry<>(download, content)); + try(var processor = new ExtensionProcessor(extensionFile)) { + Consumer consumer = resource -> { + service.storeResource(resource); + service.persistResource(resource); + }; + + if(integrityService.isEnabled()) { + var keyPair = extVersion.getSignatureKeyPair(); + if(keyPair != null) { + var signature = integrityService.generateSignature(download, extensionFile, keyPair); + consumer.accept(signature); + } else { + // Can happen when GenerateKeyPairJobRequestHandler hasn't run yet and there is no active SignatureKeyPair. + // This extension version should be assigned a SignatureKeyPair and a signature FileResource should be created + // by the ExtensionVersionSignatureJobRequestHandler migration. + logger.warn("Integrity service is enabled, but {} did not have an active key pair", NamingUtil.toLogFormat(extVersion)); + } + } + + processor.processEachResource(extVersion, consumer); + processor.getFileResources(extVersion).forEach(consumer); + consumer.accept(processor.generateSha256Checksum(extVersion)); + } + + service.storeDownload(download); + + // Update whether extension is active, the search index and evict cache + service.activateExtension(extVersion); + if(!download.getStorageType().equals(FileResource.STORAGE_DB)) { + // Don't store the binary content in the DB - it's now stored externally + download.setContent(null); + } + + service.updateResource(download); + logger.info("Published {}", NamingUtil.toLogFormat(extVersion)); + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobRequest.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobRequest.java new file mode 100644 index 000000000..bcf1443cf --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobRequest.java @@ -0,0 +1,37 @@ +/** ****************************************************************************** + * Copyright (c) 2023 Precies. Software Ltd 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.publish; + +import org.jobrunr.jobs.lambdas.JobRequest; +import org.jobrunr.jobs.lambdas.JobRequestHandler; + +public class PublishExtensionVersionJobRequest implements JobRequest { + + private long downloadId; + + public PublishExtensionVersionJobRequest() {} + + public PublishExtensionVersionJobRequest(long downloadId) { + this.downloadId = downloadId; + } + + public long getDownloadId() { + return downloadId; + } + + public void setDownloadId(long downloadId) { + this.downloadId = downloadId; + } + + @Override + public Class getJobRequestHandler() { + return PublishExtensionVersionJob.class; + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobService.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobService.java new file mode 100644 index 000000000..98c23642f --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionJobService.java @@ -0,0 +1,87 @@ +/** ****************************************************************************** + * Copyright (c) 2023 Precies. Software Ltd 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.publish; + +import jakarta.persistence.EntityManager; +import jakarta.transaction.Transactional; +import org.eclipse.openvsx.ExtensionService; +import org.eclipse.openvsx.entities.ExtensionVersion; +import org.eclipse.openvsx.entities.FileResource; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.storage.StorageUtilService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.retry.annotation.Retryable; +import org.springframework.stereotype.Component; + +@Component +public class PublishExtensionVersionJobService { + + @Autowired + EntityManager entityManager; + + @Autowired + RepositoryService repositories; + + @Autowired + StorageUtilService storageUtil; + + @Autowired + ExtensionService extensions; + + public FileResource getFileResource(long id) { + return entityManager.find(FileResource.class, id); + } + + @Transactional + public void deleteFileResources(ExtensionVersion extVersion) { + repositories.findFiles(extVersion) + .filter(f -> !f.getType().equals(FileResource.DOWNLOAD)) + .forEach(entityManager::remove); + } + + @Retryable + public void storeDownload(FileResource resource) { + // Store file resource in the DB or external storage + if (storageUtil.shouldStoreExternally(resource)) { + storageUtil.uploadFile(resource); + } else { + resource.setStorageType(FileResource.STORAGE_DB); + } + } + + @Retryable + public void storeResource(FileResource resource) { + // Store file resource in the DB or external storage + if (storageUtil.shouldStoreExternally(resource)) { + storageUtil.uploadFile(resource); + // Don't store the binary content in the DB - it's now stored externally + resource.setContent(null); + } else { + resource.setStorageType(FileResource.STORAGE_DB); + } + } + + @Transactional + public void persistResource(FileResource resource) { + entityManager.persist(resource); + } + + @Transactional + public void activateExtension(ExtensionVersion extVersion) { + extVersion.setActive(true); + extVersion = entityManager.merge(extVersion); + extensions.updateExtension(extVersion.getExtension()); + } + + @Transactional + public void updateResource(FileResource resource) { + entityManager.merge(resource); + } +} 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 dc719de02..eded00bd1 100644 --- a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java @@ -9,67 +9,23 @@ * ****************************************************************************** */ package org.eclipse.openvsx.publish; -import org.eclipse.openvsx.ExtensionService; +import jakarta.persistence.EntityManager; +import jakarta.transaction.Transactional; import org.eclipse.openvsx.entities.Extension; -import org.eclipse.openvsx.entities.ExtensionVersion; import org.eclipse.openvsx.entities.FileResource; -import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.storage.StorageUtilService; -import org.eclipse.openvsx.util.ErrorResultException; -import org.eclipse.openvsx.util.TempFile; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.retry.annotation.Retryable; import org.springframework.stereotype.Component; -import jakarta.persistence.EntityManager; -import jakarta.transaction.Transactional; -import java.io.IOException; -import java.nio.file.Files; - @Component public class PublishExtensionVersionService { - @Autowired - RepositoryService repositories; - @Autowired EntityManager entityManager; @Autowired StorageUtilService storageUtil; - @Transactional - public void deleteFileResources(ExtensionVersion extVersion) { - repositories.findFiles(extVersion).forEach(entityManager::remove); - } - - @Retryable - public void storeDownload(FileResource download, TempFile extensionFile) { - if (storageUtil.shouldStoreExternally(download)) { - storageUtil.uploadFile(download, extensionFile); - } else { - try { - download.setContent(Files.readAllBytes(extensionFile.getPath())); - } catch (IOException e) { - throw new ErrorResultException("Failed to read extension file", e); - } - - download.setStorageType(FileResource.STORAGE_DB); - } - } - - @Retryable - public void storeResource(FileResource resource) { - // Store file resource in the DB or external storage - if (storageUtil.shouldStoreExternally(resource)) { - storageUtil.uploadFile(resource); - // Don't store the binary content in the DB - it's now stored externally - resource.setContent(null); - } else { - resource.setStorageType(FileResource.STORAGE_DB); - } - } - @Transactional public void mirrorResource(FileResource resource) { resource.setStorageType(storageUtil.getActiveStorageType()); @@ -78,18 +34,6 @@ public void mirrorResource(FileResource resource) { entityManager.persist(resource); } - @Transactional - public synchronized void persistResource(FileResource resource) { - entityManager.persist(resource); - } - - @Transactional - public void activateExtension(ExtensionVersion extVersion, ExtensionService extensions) { - extVersion.setActive(true); - extVersion = entityManager.merge(extVersion); - extensions.updateExtension(extVersion.getExtension()); - } - @Transactional(Transactional.TxType.REQUIRES_NEW) public void updateExtensionPublicId(Extension extension) { entityManager.merge(extension); diff --git a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java index dcab83182..19a4a7631 100644 --- a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java @@ -36,6 +36,7 @@ import org.eclipse.openvsx.storage.StorageUtilService; import org.eclipse.openvsx.util.TargetPlatform; import org.eclipse.openvsx.util.VersionService; +import org.jobrunr.scheduling.JobRequestScheduler; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -84,7 +85,7 @@ @MockBean({ ClientRegistrationRepository.class, UpstreamRegistryService.class, GoogleCloudStorageService.class, AzureBlobStorageService.class, VSCodeIdService.class, AzureDownloadCountService.class, CacheService.class, - EclipseService.class, PublishExtensionVersionService.class, SimpleMeterRegistry.class + EclipseService.class, PublishExtensionVersionService.class, SimpleMeterRegistry.class, JobRequestScheduler.class }) public class RegistryAPITest { diff --git a/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java b/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java index a8769c7b1..8c7e2effd 100644 --- a/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java @@ -39,6 +39,7 @@ import org.eclipse.openvsx.util.ErrorResultException; import org.eclipse.openvsx.util.TargetPlatform; import org.eclipse.openvsx.util.VersionService; +import org.jobrunr.scheduling.JobRequestScheduler; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -65,9 +66,8 @@ @ExtendWith(SpringExtension.class) @MockBean({ EntityManager.class, SearchUtilService.class, GoogleCloudStorageService.class, AzureBlobStorageService.class, - VSCodeIdService.class, AzureDownloadCountService.class, CacheService.class, - UserService.class, PublishExtensionVersionHandler.class, - SimpleMeterRegistry.class + VSCodeIdService.class, AzureDownloadCountService.class, CacheService.class, UserService.class, + PublishExtensionVersionHandler.class, SimpleMeterRegistry.class, JobRequestScheduler.class }) public class EclipseServiceTest {