From 1ffe209958907452d907ce33bd272586ca081835 Mon Sep 17 00:00:00 2001 From: yma Date: Mon, 2 Dec 2024 11:45:48 +0800 Subject: [PATCH 1/8] Fix checksum validation for archive generation download --- .../archive/controller/ArchiveController.java | 90 ++++++++++++++++--- .../util/HistoricalContentListReader.java | 63 ++++++------- .../util/HistoricalContentListReaderTest.java | 18 ++-- 3 files changed, 122 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index b62e345..09a6838 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -33,6 +33,7 @@ import org.commonjava.indy.service.archive.config.PreSeedConfig; import org.commonjava.indy.service.archive.model.ArchiveStatus; import org.commonjava.indy.service.archive.model.dto.HistoricalContentDTO; +import org.commonjava.indy.service.archive.model.dto.HistoricalEntryDTO; import org.commonjava.indy.service.archive.util.HistoricalContentListReader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,10 +48,15 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorCompletionService; @@ -79,6 +85,15 @@ public class ArchiveController private final String PART_ARCHIVE_SUFFIX = PART_SUFFIX + ARCHIVE_SUFFIX; + private static final Set CHECKSUMS = Collections.unmodifiableSet( new HashSet() + { + { + add( ".md5" ); + add( ".sha1" ); + add( ".sha256" ); + } + } ); + @Inject HistoricalContentListReader reader; @@ -146,11 +161,14 @@ protected Boolean doGenerate( HistoricalContentDTO content ) content.getBuildConfigId() ); recordInProgress( content.getBuildConfigId() ); - Map downloadPaths = reader.readPaths( content ); + Map entryDTOs = reader.readEntries( content ); + Map downloadPaths = new HashMap<>(); + entryDTOs.forEach( ( key, value ) -> downloadPaths.put( key, value.getPath() ) ); + Optional archive; try { - downloadArtifacts( downloadPaths, content ); + downloadArtifacts( entryDTOs, downloadPaths, content ); archive = generateArchive( new ArrayList<>( downloadPaths.values() ), content ); } catch ( final InterruptedException e ) @@ -226,7 +244,8 @@ public String getStatus( String buildConfigId ) return treated.get( buildConfigId ); } - private void downloadArtifacts( final Map downloadPaths, final HistoricalContentDTO content ) + private void downloadArtifacts( final Map entryDTOs, + final Map downloadPaths, final HistoricalContentDTO content ) throws InterruptedException, ExecutionException, IOException { BasicCookieStore cookieStore = new BasicCookieStore(); @@ -236,13 +255,23 @@ private void downloadArtifacts( final Map downloadPaths, final H File dir = new File( contentBuildDir ); dir.delete(); - fileTrackedContent( contentBuildDir, content ); - unpackHistoricalArchive( contentBuildDir, content.getBuildConfigId() ); + HistoricalContentDTO originalTracked = unpackHistoricalArchive( contentBuildDir, content.getBuildConfigId() ); + Map> originalChecksumsMap = new HashMap<>(); + if ( originalTracked != null ) + { + Map originalEntries = reader.readEntries( originalTracked ); + originalEntries.forEach( ( key, entry ) -> originalChecksumsMap.put( key, new ArrayList<>( + Arrays.asList( entry.getSha1(), entry.getSha256(), entry.getMd5() ) ) ) ); + } for ( String path : downloadPaths.keySet() ) { String filePath = downloadPaths.get( path ); - executor.submit( download( contentBuildDir, path, filePath, cookieStore ) ); + HistoricalEntryDTO entry = entryDTOs.get( path ); + List checksums = + new ArrayList<>( Arrays.asList( entry.getSha1(), entry.getSha256(), entry.getMd5() ) ); + List originalChecksums = originalChecksumsMap.get( path ); + executor.submit( download( contentBuildDir, path, filePath, checksums, originalChecksums, cookieStore ) ); } int success = 0; int failed = 0; @@ -257,6 +286,8 @@ private void downloadArtifacts( final Map downloadPaths, final H failed++; } } + // file the latest tracked json at the end + fileTrackedContent( contentBuildDir, content ); logger.info( "Artifacts download completed, success:{}, failed:{}", success, failed ); } @@ -369,14 +400,14 @@ private void fileTrackedContent( String contentBuildDir, final HistoricalContent } } - private void unpackHistoricalArchive( String contentBuildDir, String buildConfigId ) + private HistoricalContentDTO unpackHistoricalArchive( String contentBuildDir, String buildConfigId ) throws IOException { final File archive = new File( archiveDir, buildConfigId + ARCHIVE_SUFFIX ); if ( !archive.exists() ) { logger.debug( "Don't find historical archive for buildConfigId: {}.", buildConfigId ); - return; + return null; } logger.info( "Start unpacking historical archive for buildConfigId: {}.", buildConfigId ); @@ -393,16 +424,53 @@ private void unpackHistoricalArchive( String contentBuildDir, String buildConfig } inputStream.close(); + + File originalTracked = new File( contentBuildDir, buildConfigId ); + if ( originalTracked.exists() ) + { + return objectMapper.readValue( originalTracked, HistoricalContentDTO.class ); + } + else + { + logger.debug( "No tracked json file found after zip unpack for buildConfigId {}", buildConfigId ); + return null; + } } - private Callable download( String contentBuildDir, final String path, final String filePath, + private boolean validateChecksum( final String filePath, final List current, final List original ) + { + if ( CHECKSUMS.stream().anyMatch( suffix -> filePath.toLowerCase().endsWith( "." + suffix ) ) ) + { + // skip to validate checksum files + return false; + } + if ( original == null || original.isEmpty() || original.stream().allMatch( Objects::isNull ) ) + { + return false; + } + if ( original.get( 0 ) != null && original.get( 0 ).equals( current.get( 0 ) ) ) + { + return true; + } + if ( original.get( 1 ) != null && original.get( 1 ).equals( current.get( 1 ) ) ) + { + return true; + } + return original.get( 2 ) != null && original.get( 2 ).equals( current.get( 2 ) ); + } + + private Callable download( final String contentBuildDir, final String path, final String filePath, + final List checksums, final List originalChecksums, final CookieStore cookieStore ) { return () -> { final File target = new File( contentBuildDir, filePath ); - if ( target.exists() ) + + if ( target.exists() && validateChecksum( filePath, checksums, originalChecksums ) ) { - logger.trace( "<< readPaths( HistoricalContentDTO content ) + public Map readEntries( HistoricalContentDTO content ) { - Map pathMap = new HashMap<>(); + Map pathMap = new HashMap<>(); HistoricalEntryDTO[] downloads = content.getDownloads(); - if ( downloads != null ) + if ( downloads == null ) { - for ( HistoricalEntryDTO download : downloads ) - { - String path = download.getPath(); - String packageType = download.getStoreKey().getPackageType(); + return pathMap; + } + for ( HistoricalEntryDTO download : downloads ) + { + String path = download.getPath(); + String packageType = download.getStoreKey().getPackageType(); - if ( packageType.equals( NPM_PKG_KEY ) && !path.endsWith( ".tgz" ) ) - { - // Ignore the npm package metadata in archive - continue; - } - if ( path.contains( "maven-metadata.xml" ) ) - { - // Ignore maven-metadata.xml in archive - continue; - } - // ensure every entry has an available localUrl - buildDownloadUrl( download ); + if ( packageType.equals( NPM_PKG_KEY ) && !path.endsWith( ".tgz" ) ) + { + // Ignore the npm package metadata in archive + continue; + } + if ( path.contains( "maven-metadata.xml" ) ) + { + // Ignore maven-metadata.xml in archive + continue; + } + // ensure every entry has an available localUrl + buildDownloadUrl( download ); - // local url would be preferred to download artifact - String url = download.getLocalUrl(); - if ( url == null ) - { - url = download.getOriginUrl(); - } - if ( url != null ) - { - pathMap.put( url, download.getPath() ); - } + // local url would be preferred to download artifact + String url = download.getLocalUrl(); + if ( url == null ) + { + url = download.getOriginUrl(); + } + if ( url != null ) + { + pathMap.put( url, download ); } } return pathMap; diff --git a/src/test/java/org/commonjava/indy/service/archive/util/HistoricalContentListReaderTest.java b/src/test/java/org/commonjava/indy/service/archive/util/HistoricalContentListReaderTest.java index b7743da..2e76fe3 100644 --- a/src/test/java/org/commonjava/indy/service/archive/util/HistoricalContentListReaderTest.java +++ b/src/test/java/org/commonjava/indy/service/archive/util/HistoricalContentListReaderTest.java @@ -16,7 +16,6 @@ package org.commonjava.indy.service.archive.util; import io.quarkus.test.junit.QuarkusTest; -import org.commonjava.indy.service.archive.config.PreSeedConfig; import org.commonjava.indy.service.archive.model.StoreKey; import org.commonjava.indy.service.archive.model.StoreType; import org.commonjava.indy.service.archive.model.dto.HistoricalContentDTO; @@ -24,6 +23,7 @@ import org.junit.jupiter.api.Test; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -58,13 +58,15 @@ public void testMavenReadPaths() entryDTOs.add( entry ); entryDTOs.add( metaEntry ); - HistoricalContentDTO contentDTO = new HistoricalContentDTO( "8888", entryDTOs.toArray( - new HistoricalEntryDTO[entryDTOs.size()] ) ); + HistoricalContentDTO contentDTO = + new HistoricalContentDTO( "8888", entryDTOs.toArray( new HistoricalEntryDTO[entryDTOs.size()] ) ); TestPreSeedConfig preSeedConfig = new TestPreSeedConfig( Optional.of( MAIN_INDY ) ); HistoricalContentListReader reader = new HistoricalContentListReader( preSeedConfig ); - Map paths = reader.readPaths( contentDTO ); + Map entryMaps = reader.readEntries( contentDTO ); + Map paths = new HashMap<>(); + entryMaps.forEach( ( key, value ) -> paths.put( key, value.getPath() ) ); assertThat( paths.size(), equalTo( 1 ) ); String storePath = MAIN_INDY + "/api/content" + entry.getStorePath(); @@ -86,13 +88,15 @@ public void testNPMReadPaths() entryDTOs.add( npmEntry ); entryDTOs.add( npmMetaEntry ); - HistoricalContentDTO contentDTO = new HistoricalContentDTO( "8888", entryDTOs.toArray( - new HistoricalEntryDTO[entryDTOs.size()] ) ); + HistoricalContentDTO contentDTO = + new HistoricalContentDTO( "8888", entryDTOs.toArray( new HistoricalEntryDTO[entryDTOs.size()] ) ); TestPreSeedConfig preSeedConfig = new TestPreSeedConfig( Optional.of( MAIN_INDY ) ); HistoricalContentListReader reader = new HistoricalContentListReader( preSeedConfig ); - Map paths = reader.readPaths( contentDTO ); + Map entryMaps = reader.readEntries( contentDTO ); + Map paths = new HashMap<>(); + entryMaps.forEach( ( key, value ) -> paths.put( key, value.getPath() ) ); assertThat( paths.size(), equalTo( 1 ) ); String storePath = MAIN_INDY + "/api/content" + npmEntry.getStorePath(); From fb9068e3c8d05775e46ebf19baacfb879ea5b56c Mon Sep 17 00:00:00 2001 From: yma Date: Mon, 2 Dec 2024 14:04:17 +0800 Subject: [PATCH 2/8] Fix archive generation process with synchronized lock --- .../archive/controller/ArchiveController.java | 85 ++++++++++++++----- .../archive/jaxrs/ArchiveManageResources.java | 39 ++++----- 2 files changed, 82 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index 09a6838..2560f2e 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -29,7 +29,6 @@ import org.apache.http.impl.client.BasicCookieStore; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.commonjava.indy.service.archive.config.PreSeedConfig; import org.commonjava.indy.service.archive.model.ArchiveStatus; import org.commonjava.indy.service.archive.model.dto.HistoricalContentDTO; @@ -47,6 +46,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.security.KeyManagementException; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -58,6 +60,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorCompletionService; import java.util.concurrent.ExecutorService; @@ -94,6 +97,18 @@ public class ArchiveController } } ); + private static final Map buildConfigLocks = new ConcurrentHashMap<>(); + + private static final int threads = 4 * Runtime.getRuntime().availableProcessors(); + + private static final ExecutorService generateExecutor = + Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { + final Thread t = new Thread( r ); + t.setName( "Generate-" + t.getName() ); + t.setDaemon( true ); + return t; + } ); + @Inject HistoricalContentListReader reader; @@ -115,7 +130,7 @@ public class ArchiveController @PostConstruct public void init() - throws IOException + throws IOException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException { int threads = 4 * Runtime.getRuntime().availableProcessors(); executorService = Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { @@ -125,11 +140,8 @@ public void init() return t; } ); - final PoolingHttpClientConnectionManager ccm = new PoolingHttpClientConnectionManager(); - ccm.setMaxTotal( 500 ); - RequestConfig rc = RequestConfig.custom().build(); - client = HttpClients.custom().setConnectionManager( ccm ).setDefaultRequestConfig( rc ).build(); + client = HttpClients.custom().setDefaultRequestConfig( rc ).build(); String storeDir = preSeedConfig.storageDir().orElse( "data" ); contentDir = String.format( "%s%s", storeDir, CONTENT_DIR ); @@ -145,21 +157,48 @@ public void destroy() public void generate( HistoricalContentDTO content ) { - int threads = 4 * Runtime.getRuntime().availableProcessors(); - ExecutorService generateExecutor = Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { - final Thread t = new Thread( r ); - t.setName( "Generate-" + t.getName() ); - t.setDaemon( true ); - return t; - } ); - generateExecutor.execute( () -> doGenerate( content ) ); + String buildConfigId = content.getBuildConfigId(); + Object lock = buildConfigLocks.computeIfAbsent( buildConfigId, k -> new Object() ); + synchronized ( lock ) + { + while ( isInProgress( buildConfigId ) ) + { + logger.info( "There is already generation process in progress for buildConfigId {}, try lock wait.", + buildConfigId ); + try + { + lock.wait(); + } + catch ( InterruptedException e ) + { + Thread.currentThread().interrupt(); + return; + } + } + recordInProgress( content.getBuildConfigId() ); + generateExecutor.execute( () -> { + try + { + doGenerate( content ); + } + finally + { + synchronized ( lock ) + { + recordCompleted( content.getBuildConfigId() ); + buildConfigLocks.remove( buildConfigId ); + lock.notifyAll(); + logger.info( "lock released, buildConfigId {}", buildConfigId ); + } + } + } ); + } } protected Boolean doGenerate( HistoricalContentDTO content ) { logger.info( "Handle generate event: {}, build config id: {}", EVENT_GENERATE_ARCHIVE, content.getBuildConfigId() ); - recordInProgress( content.getBuildConfigId() ); Map entryDTOs = reader.readEntries( content ); Map downloadPaths = new HashMap<>(); @@ -195,7 +234,6 @@ protected Boolean doGenerate( HistoricalContentDTO content ) created = renderArchive( archive.get(), content.getBuildConfigId() ); } - recordCompleted( content.getBuildConfigId() ); return created; } @@ -239,11 +277,17 @@ public boolean statusExists( final String buildConfigId ) return treated.containsKey( buildConfigId ); } - public String getStatus( String buildConfigId ) + public String getStatus( final String buildConfigId ) { return treated.get( buildConfigId ); } + public boolean isInProgress( final String buildConfigId ) + { + return statusExists( buildConfigId ) && getStatus( buildConfigId ).equals( + ArchiveStatus.inProgress.getArchiveStatus() ); + } + private void downloadArtifacts( final Map entryDTOs, final Map downloadPaths, final HistoricalContentDTO content ) throws InterruptedException, ExecutionException, IOException @@ -533,12 +577,7 @@ private void restoreGenerateStatusFromDisk() throws IOException List contents = walkAllFiles( archiveDir ); for ( File content : contents ) { - if ( content.getName().endsWith( PART_ARCHIVE_SUFFIX ) ) - { - treated.put( content.getName().split( PART_ARCHIVE_SUFFIX )[0], - ArchiveStatus.inProgress.getArchiveStatus() ); - } - else if ( content.getName().endsWith( ARCHIVE_SUFFIX ) ) + if ( content.getName().endsWith( ARCHIVE_SUFFIX ) ) { treated.put( content.getName().split( ARCHIVE_SUFFIX )[0], ArchiveStatus.completed.getArchiveStatus() ); } diff --git a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java index 6b0a383..b01d36d 100644 --- a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java +++ b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java @@ -18,6 +18,19 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.smallrye.mutiny.Uni; import io.vertx.core.eventbus.EventBus; +import jakarta.inject.Inject; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.Response.ResponseBuilder; +import jakarta.ws.rs.core.UriInfo; import org.apache.commons.io.FileUtils; import org.commonjava.indy.service.archive.controller.ArchiveController; import org.commonjava.indy.service.archive.model.dto.HistoricalContentDTO; @@ -31,19 +44,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import jakarta.ws.rs.PathParam; -import jakarta.inject.Inject; -import jakarta.ws.rs.Consumes; -import jakarta.ws.rs.DELETE; -import jakarta.ws.rs.GET; -import jakarta.ws.rs.POST; -import jakarta.ws.rs.Path; -import jakarta.ws.rs.Produces; -import jakarta.ws.rs.core.Context; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.Response.ResponseBuilder; -import jakarta.ws.rs.core.UriInfo; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -72,14 +72,15 @@ public class ArchiveManageResources @Inject EventBus bus; - @Operation( description = "Generate archive based on tracked content" ) @APIResponse( responseCode = "202", description = "The archive created request is accepted" ) - @RequestBody( description = "The tracked content definition JSON", name = "body", required = true, content = @Content( mediaType = APPLICATION_JSON, example = - "{" + "\"buildConfigId\": \"XXX\"," + "\"downloads\":" + "[{" + " \"storeKey\": \"\"," - + " \"path\": \"\"," + " \"md5\": \"\"," + " \"sha256\": \"\"," - + " \"sha1\": \"\"," + " \"size\": 001" + " }," + "..." - + "]}", schema = @Schema( implementation = HistoricalContentDTO.class ) ) ) + @RequestBody( description = "The tracked content definition JSON", name = "body", required = true, + content = @Content( mediaType = APPLICATION_JSON, + example = "{" + "\"buildConfigId\": \"XXX\"," + "\"downloads\":" + "[{" + + " \"storeKey\": \"\"," + " \"path\": \"\"," + " \"md5\": \"\"," + + " \"sha256\": \"\"," + " \"sha1\": \"\"," + " \"size\": 001" + + " }," + "..." + "]}", + schema = @Schema( implementation = HistoricalContentDTO.class ) ) ) @POST @Path( "generate" ) @Consumes( APPLICATION_JSON ) From 6c61b709b6b9cc635d74cbd0aa5545fc31653823 Mon Sep 17 00:00:00 2001 From: yma Date: Tue, 3 Dec 2024 00:48:38 +0800 Subject: [PATCH 3/8] Add API to delete archive with checksum validation --- .../archive/controller/ArchiveController.java | 95 ++++++++++++++++--- .../archive/jaxrs/ArchiveManageResources.java | 21 ++++ 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index 2560f2e..85ffc69 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -29,6 +29,7 @@ import org.apache.http.impl.client.BasicCookieStore; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.commonjava.indy.service.archive.config.PreSeedConfig; import org.commonjava.indy.service.archive.model.ArchiveStatus; import org.commonjava.indy.service.archive.model.dto.HistoricalContentDTO; @@ -43,11 +44,12 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.MappedByteBuffer; +import java.nio.channels.FileChannel; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.security.KeyManagementException; -import java.security.KeyStoreException; +import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; @@ -88,6 +90,16 @@ public class ArchiveController private final String PART_ARCHIVE_SUFFIX = PART_SUFFIX + ARCHIVE_SUFFIX; + private static final int threads = 4 * Runtime.getRuntime().availableProcessors(); + + private final ExecutorService generateExecutor = + Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { + final Thread t = new Thread( r ); + t.setName( "Generate-" + t.getName() ); + t.setDaemon( true ); + return t; + } ); + private static final Set CHECKSUMS = Collections.unmodifiableSet( new HashSet() { { @@ -99,15 +111,13 @@ public class ArchiveController private static final Map buildConfigLocks = new ConcurrentHashMap<>(); - private static final int threads = 4 * Runtime.getRuntime().availableProcessors(); + private static final String SHA_256 = "SHA-256"; - private static final ExecutorService generateExecutor = - Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { - final Thread t = new Thread( r ); - t.setName( "Generate-" + t.getName() ); - t.setDaemon( true ); - return t; - } ); + private static final Long BOLCK_SIZE = 100 * 1024 * 1024L; + + private static final String HEX_DIGITS = "0123456789abcdef"; + + private static final char[] HEX_ARRAY = HEX_DIGITS.toCharArray(); @Inject HistoricalContentListReader reader; @@ -130,7 +140,7 @@ public class ArchiveController @PostConstruct public void init() - throws IOException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException + throws IOException { int threads = 4 * Runtime.getRuntime().availableProcessors(); executorService = Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { @@ -140,8 +150,11 @@ public void init() return t; } ); + final PoolingHttpClientConnectionManager ccm = new PoolingHttpClientConnectionManager(); + ccm.setMaxTotal( 500 ); + RequestConfig rc = RequestConfig.custom().build(); - client = HttpClients.custom().setDefaultRequestConfig( rc ).build(); + client = HttpClients.custom().setConnectionManager( ccm ).setDefaultRequestConfig( rc ).build(); String storeDir = preSeedConfig.storageDir().orElse( "data" ); contentDir = String.format( "%s%s", storeDir, CONTENT_DIR ); @@ -259,6 +272,64 @@ public void deleteArchive( final String buildConfigId ) logger.info( "Historical archive for build config id: {} is deleted.", buildConfigId ); } + public void deleteArchiveWithChecksum( final String buildConfigId, final String checksum ) + throws IOException + { + logger.info( "Start to delete archive with checksum validation, buildConfigId {}, checksum {}", buildConfigId, + checksum ); + File zip = new File( archiveDir, buildConfigId + ARCHIVE_SUFFIX ); + if ( !zip.exists() ) + { + return; + } + + try (FileChannel channel = new FileInputStream( zip ).getChannel()) + { + MessageDigest digest = MessageDigest.getInstance( SHA_256 ); + long position = 0; + long size = channel.size(); + + while ( position < size ) + { + long remaining = size - position; + long currentBlock = Math.min( remaining, BOLCK_SIZE ); + MappedByteBuffer buffer = channel.map( FileChannel.MapMode.READ_ONLY, position, currentBlock ); + digest.update( buffer ); + position += currentBlock; + } + + String stored = bytesToHex( digest.digest() ); + // only delete the zip once checksum is equaled + if ( stored.equals( checksum ) ) + { + zip.delete(); + logger.info( "Historical archive for build config id: {} is deleted, checksum {}.", buildConfigId, + stored ); + } + else + { + logger.info( "Don't delete the {} zip, transferred checksum {}, but stored checksum {}.", buildConfigId, + checksum, stored ); + } + } + catch ( NoSuchAlgorithmException e ) + { + logger.error( "No such algorithm SHA-256 Exception", e ); + } + } + + private String bytesToHex( byte[] hash ) + { + char[] hexChars = new char[hash.length * 2]; + for ( int i = 0; i < hash.length; i++ ) + { + int v = hash[i] & 0xFF; + hexChars[i * 2] = HEX_ARRAY[v >>> 4]; + hexChars[i * 2 + 1] = HEX_ARRAY[v & 0x0F]; + } + return new String( hexChars ); + } + public void cleanup() throws IOException { diff --git a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java index b01d36d..96a8c60 100644 --- a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java +++ b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java @@ -188,6 +188,27 @@ public Uni delete( final @PathParam( "buildConfigId" ) String buildCon return Uni.createFrom().item( noContent().build() ); } + @Operation( description = "Delete the archive by buildConfigId and checksum" ) + @APIResponse( responseCode = "204", description = "The history archive is deleted or doesn't exist" ) + @Path( "{buildConfigId}/{checksum}" ) + @DELETE + public Uni deleteWithChecksum( final @PathParam( "buildConfigId" ) String buildConfigId, + final @PathParam( "checksum" ) String checksum, + final @Context UriInfo uriInfo ) + { + try + { + controller.deleteArchiveWithChecksum( buildConfigId, checksum ); + } + catch ( final IOException e ) + { + final String message = "Failed to delete historical archive for build config id: " + buildConfigId; + logger.error( message, e ); + return fromResponse( message ); + } + return Uni.createFrom().item( noContent().build() ); + } + @Operation( description = "Clean up all the temp workplace" ) @APIResponse( responseCode = "204", description = "The workplace cleanup is finished" ) @Path( "cleanup" ) From 452b14d8d3f0e35b4a744d6dc357e45d7ee5928b Mon Sep 17 00:00:00 2001 From: yma Date: Tue, 3 Dec 2024 11:53:16 +0800 Subject: [PATCH 4/8] Delete target to prevent the obsolete file still existed caused by http error --- .../archive/controller/ArchiveController.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index 85ffc69..54ed5c1 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -103,9 +103,9 @@ public class ArchiveController private static final Set CHECKSUMS = Collections.unmodifiableSet( new HashSet() { { - add( ".md5" ); add( ".sha1" ); add( ".sha256" ); + add( ".md5" ); } } ); @@ -299,7 +299,7 @@ public void deleteArchiveWithChecksum( final String buildConfigId, final String } String stored = bytesToHex( digest.digest() ); - // only delete the zip once checksum is equaled + // only delete the zip once checksum is matched if ( stored.equals( checksum ) ) { zip.delete(); @@ -374,6 +374,7 @@ private void downloadArtifacts( final Map entryDTOs, Map> originalChecksumsMap = new HashMap<>(); if ( originalTracked != null ) { + logger.trace( "originalChecksumsMap generated for {}", content.getBuildConfigId() ); Map originalEntries = reader.readEntries( originalTracked ); originalEntries.forEach( ( key, entry ) -> originalChecksumsMap.put( key, new ArrayList<>( Arrays.asList( entry.getSha1(), entry.getSha256(), entry.getMd5() ) ) ) ); @@ -530,6 +531,7 @@ private HistoricalContentDTO unpackHistoricalArchive( String contentBuildDir, St ZipEntry entry; while ( ( entry = inputStream.getNextEntry() ) != null ) { + logger.trace( "entry path:" + entry.getName() ); File outputFile = new File( contentBuildDir, entry.getName() ); outputFile.getParentFile().mkdirs(); try ( FileOutputStream outputStream = new FileOutputStream( outputFile ) ) @@ -554,7 +556,7 @@ private HistoricalContentDTO unpackHistoricalArchive( String contentBuildDir, St private boolean validateChecksum( final String filePath, final List current, final List original ) { - if ( CHECKSUMS.stream().anyMatch( suffix -> filePath.toLowerCase().endsWith( "." + suffix ) ) ) + if ( CHECKSUMS.stream().anyMatch( suffix -> filePath.toLowerCase().endsWith( suffix ) ) ) { // skip to validate checksum files return false; @@ -563,14 +565,17 @@ private boolean validateChecksum( final String filePath, final List curr { return false; } + // once sha1 is matched, skip downloading if ( original.get( 0 ) != null && original.get( 0 ).equals( current.get( 0 ) ) ) { return true; } + // once sha256 is matched, skip downloading if ( original.get( 1 ) != null && original.get( 1 ).equals( current.get( 1 ) ) ) { return true; } + // once md5 is matched, skip downloading return original.get( 2 ) != null && original.get( 2 ).equals( current.get( 2 ) ); } @@ -584,7 +589,7 @@ private Callable download( final String contentBuildDir, final String p if ( target.exists() && validateChecksum( filePath, checksums, originalChecksums ) ) { logger.debug( - "<< download( final String contentBuildDir, final String p context.setCookieStore( cookieStore ); final HttpGet request = new HttpGet( path ); InputStream input = null; + if ( target.exists() ) + { + // prevent the obsolete file still existed caused by http error + target.delete(); + } try { CloseableHttpResponse response = client.execute( request, context ); From 47f98686ea98191aecd0dcce15de9e7b1a719e5f Mon Sep 17 00:00:00 2001 From: yma Date: Tue, 3 Dec 2024 12:09:05 +0800 Subject: [PATCH 5/8] Use DigestUtils to compute checksum --- .../archive/controller/ArchiveController.java | 53 +++---------------- 1 file changed, 7 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index 54ed5c1..0e7f533 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -20,6 +20,7 @@ import jakarta.annotation.PreDestroy; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; +import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.io.IOUtils; import org.apache.http.client.CookieStore; import org.apache.http.client.config.RequestConfig; @@ -44,13 +45,9 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import java.nio.MappedByteBuffer; -import java.nio.channels.FileChannel; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -111,14 +108,6 @@ public class ArchiveController private static final Map buildConfigLocks = new ConcurrentHashMap<>(); - private static final String SHA_256 = "SHA-256"; - - private static final Long BOLCK_SIZE = 100 * 1024 * 1024L; - - private static final String HEX_DIGITS = "0123456789abcdef"; - - private static final char[] HEX_ARRAY = HEX_DIGITS.toCharArray(); - @Inject HistoricalContentListReader reader; @@ -283,51 +272,23 @@ public void deleteArchiveWithChecksum( final String buildConfigId, final String return; } - try (FileChannel channel = new FileInputStream( zip ).getChannel()) + try (FileInputStream fis = new FileInputStream( zip )) { - MessageDigest digest = MessageDigest.getInstance( SHA_256 ); - long position = 0; - long size = channel.size(); - - while ( position < size ) - { - long remaining = size - position; - long currentBlock = Math.min( remaining, BOLCK_SIZE ); - MappedByteBuffer buffer = channel.map( FileChannel.MapMode.READ_ONLY, position, currentBlock ); - digest.update( buffer ); - position += currentBlock; - } + String storedChecksum = DigestUtils.sha256Hex( fis ); - String stored = bytesToHex( digest.digest() ); // only delete the zip once checksum is matched - if ( stored.equals( checksum ) ) + if ( storedChecksum.equals( checksum ) ) { zip.delete(); logger.info( "Historical archive for build config id: {} is deleted, checksum {}.", buildConfigId, - stored ); + storedChecksum ); } else { - logger.info( "Don't delete the {} zip, transferred checksum {}, but stored checksum {}.", buildConfigId, - checksum, stored ); + logger.info( "Don't delete the {} zip, transferred checksum {}, but stored checksum {}.", + buildConfigId, checksum, storedChecksum ); } } - catch ( NoSuchAlgorithmException e ) - { - logger.error( "No such algorithm SHA-256 Exception", e ); - } - } - - private String bytesToHex( byte[] hash ) - { - char[] hexChars = new char[hash.length * 2]; - for ( int i = 0; i < hash.length; i++ ) - { - int v = hash[i] & 0xFF; - hexChars[i * 2] = HEX_ARRAY[v >>> 4]; - hexChars[i * 2 + 1] = HEX_ARRAY[v & 0x0F]; - } - return new String( hexChars ); } public void cleanup() From dac7f702a450ac3c888298ffd9c44b8be945ceff Mon Sep 17 00:00:00 2001 From: yma Date: Tue, 3 Dec 2024 12:35:22 +0800 Subject: [PATCH 6/8] Add configurable thread-multiplier for executors --- .../service/archive/config/PreSeedConfig.java | 3 ++ .../archive/controller/ArchiveController.java | 30 ++++++++----------- src/main/resources/application.yaml | 3 +- .../archive/util/TestPreSeedConfig.java | 6 ++++ 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/config/PreSeedConfig.java b/src/main/java/org/commonjava/indy/service/archive/config/PreSeedConfig.java index f7baaaa..d56b8a1 100644 --- a/src/main/java/org/commonjava/indy/service/archive/config/PreSeedConfig.java +++ b/src/main/java/org/commonjava/indy/service/archive/config/PreSeedConfig.java @@ -31,4 +31,7 @@ public interface PreSeedConfig @WithName( "not-used-days-cleanup" ) Optional notUsedDaysCleanup(); + + @WithName( "thread-multiplier" ) + Optional threadMultiplier(); } diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index 0e7f533..b6fab58 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -72,6 +72,7 @@ @ApplicationScoped public class ArchiveController { + private final Logger logger = LoggerFactory.getLogger( getClass() ); public final static String EVENT_GENERATE_ARCHIVE = "generate-archive"; @@ -79,24 +80,12 @@ public class ArchiveController public final static String ARCHIVE_DIR = "/archive"; - private final Logger logger = LoggerFactory.getLogger( getClass() ); - private final String ARCHIVE_SUFFIX = ".zip"; private final String PART_SUFFIX = ".part"; private final String PART_ARCHIVE_SUFFIX = PART_SUFFIX + ARCHIVE_SUFFIX; - private static final int threads = 4 * Runtime.getRuntime().availableProcessors(); - - private final ExecutorService generateExecutor = - Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { - final Thread t = new Thread( r ); - t.setName( "Generate-" + t.getName() ); - t.setDaemon( true ); - return t; - } ); - private static final Set CHECKSUMS = Collections.unmodifiableSet( new HashSet() { { @@ -117,7 +106,9 @@ public class ArchiveController @Inject ObjectMapper objectMapper; - private ExecutorService executorService; + private ExecutorService downloadExecutor; + + private ExecutorService generateExecutor; private CloseableHttpClient client; @@ -131,17 +122,22 @@ public class ArchiveController public void init() throws IOException { - int threads = 4 * Runtime.getRuntime().availableProcessors(); - executorService = Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { + int threads = preSeedConfig.threadMultiplier().orElse( 4 ) * Runtime.getRuntime().availableProcessors(); + downloadExecutor = Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { final Thread t = new Thread( r ); t.setName( "Download-" + t.getName() ); t.setDaemon( true ); return t; } ); + generateExecutor = Executors.newFixedThreadPool( threads, ( final Runnable r ) -> { + final Thread t = new Thread( r ); + t.setName( "Generate-" + t.getName() ); + t.setDaemon( true ); + return t; + } ); final PoolingHttpClientConnectionManager ccm = new PoolingHttpClientConnectionManager(); ccm.setMaxTotal( 500 ); - RequestConfig rc = RequestConfig.custom().build(); client = HttpClients.custom().setConnectionManager( ccm ).setDefaultRequestConfig( rc ).build(); @@ -325,7 +321,7 @@ private void downloadArtifacts( final Map entryDTOs, throws InterruptedException, ExecutionException, IOException { BasicCookieStore cookieStore = new BasicCookieStore(); - ExecutorCompletionService executor = new ExecutorCompletionService<>( executorService ); + ExecutorCompletionService executor = new ExecutorCompletionService<>( downloadExecutor ); String contentBuildDir = String.format( "%s/%s", contentDir, content.getBuildConfigId() ); File dir = new File( contentBuildDir ); diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index b82efe5..ac6f007 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -46,4 +46,5 @@ quarkus: pre-seed: main-indy: https://indy-gateway-master-devel.psi.redhat.com storage-dir: data - not-used-days-cleanup: 10 \ No newline at end of file + not-used-days-cleanup: 10 + thread-multiplier: 4 \ No newline at end of file diff --git a/src/test/java/org/commonjava/indy/service/archive/util/TestPreSeedConfig.java b/src/test/java/org/commonjava/indy/service/archive/util/TestPreSeedConfig.java index a9d8a5b..8bab7d4 100644 --- a/src/test/java/org/commonjava/indy/service/archive/util/TestPreSeedConfig.java +++ b/src/test/java/org/commonjava/indy/service/archive/util/TestPreSeedConfig.java @@ -46,4 +46,10 @@ public Optional notUsedDaysCleanup() return Optional.of( 365l ); } + @Override + public Optional threadMultiplier() + { + return Optional.of( 4 ); + } + } From c6a9cd93eb571b31e5aee6520ff4db87a08cb77f Mon Sep 17 00:00:00 2001 From: yma Date: Wed, 4 Dec 2024 19:11:52 +0800 Subject: [PATCH 7/8] Change response for synchronized generation and add timeout error handling --- .../archive/controller/ArchiveController.java | 67 +++++++++++++------ .../archive/jaxrs/ArchiveManageResources.java | 16 +++-- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index b6fab58..b8a75f4 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -64,6 +64,9 @@ import java.util.concurrent.ExecutorCompletionService; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -153,44 +156,59 @@ public void destroy() IOUtils.closeQuietly( client, null ); } - public void generate( HistoricalContentDTO content ) + public boolean generate( HistoricalContentDTO content ) { String buildConfigId = content.getBuildConfigId(); Object lock = buildConfigLocks.computeIfAbsent( buildConfigId, k -> new Object() ); synchronized ( lock ) { - while ( isInProgress( buildConfigId ) ) + if ( isInProgress( buildConfigId ) ) { logger.info( "There is already generation process in progress for buildConfigId {}, try lock wait.", buildConfigId ); - try - { - lock.wait(); - } - catch ( InterruptedException e ) - { - Thread.currentThread().interrupt(); - return; - } + // Conflicted generation, just return with expected http response immediately + return false; } - recordInProgress( content.getBuildConfigId() ); - generateExecutor.execute( () -> { + + recordInProgress( buildConfigId ); + Future future = generateExecutor.submit( () -> { try { doGenerate( content ); } + catch ( Exception e ) + { + // If error happens on generation, remove the status to make sure coming generation + removeStatus( buildConfigId ); + logger.error( "Generation failed for buildConfigId {}", buildConfigId, e ); + } finally { - synchronized ( lock ) - { - recordCompleted( content.getBuildConfigId() ); - buildConfigLocks.remove( buildConfigId ); - lock.notifyAll(); - logger.info( "lock released, buildConfigId {}", buildConfigId ); - } + recordCompleted( buildConfigId ); + buildConfigLocks.remove( buildConfigId ); + logger.info( "lock released, buildConfigId {}", buildConfigId ); } } ); + try + { + future.get( 60, TimeUnit.MINUTES ); + } + catch ( TimeoutException e ) + { + // If timeout happens on generation, cancel and remove the status to make sure coming generation + future.cancel( true ); + removeStatus( buildConfigId ); + logger.error( "Generation timeout for buildConfigId {}", buildConfigId, e ); + } + catch ( InterruptedException | ExecutionException e ) + { + // If future task level error happens on generation, cancel and remove the status to make sure coming generation + future.cancel( true ); + removeStatus( buildConfigId ); + logger.error( "Generation future task level failed for buildConfigId {}", buildConfigId, e ); + } } + return true; } protected Boolean doGenerate( HistoricalContentDTO content ) @@ -622,15 +640,20 @@ private void restoreGenerateStatusFromDisk() throws IOException } } - private void recordInProgress( String buildConfigId ) + private void recordInProgress( final String buildConfigId ) { treated.remove( buildConfigId ); treated.put( buildConfigId, ArchiveStatus.inProgress.getArchiveStatus() ); } - private void recordCompleted( String buildConfigId ) + private void recordCompleted( final String buildConfigId ) { treated.remove( buildConfigId ); treated.put( buildConfigId, ArchiveStatus.completed.getArchiveStatus() ); } + + private void removeStatus( final String buildConfigId ) + { + treated.remove( buildConfigId ); + } } diff --git a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java index 96a8c60..be4dbe8 100644 --- a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java +++ b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java @@ -51,6 +51,7 @@ import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; import static jakarta.ws.rs.core.MediaType.APPLICATION_OCTET_STREAM; +import static jakarta.ws.rs.core.Response.Status.CONFLICT; import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static jakarta.ws.rs.core.Response.accepted; import static jakarta.ws.rs.core.Response.noContent; @@ -74,6 +75,7 @@ public class ArchiveManageResources @Operation( description = "Generate archive based on tracked content" ) @APIResponse( responseCode = "202", description = "The archive created request is accepted" ) + @APIResponse( responseCode = "409", description = "The archive created request is conflicted" ) @RequestBody( description = "The tracked content definition JSON", name = "body", required = true, content = @Content( mediaType = APPLICATION_JSON, example = "{" + "\"buildConfigId\": \"XXX\"," + "\"downloads\":" + "[{" @@ -104,11 +106,17 @@ public Uni create( final @Context UriInfo uriInfo, final InputStream b return fromResponse( message ); } - controller.generate( content ); + boolean accepted = controller.generate( content ); return Uni.createFrom() - .item( accepted().type( MediaType.TEXT_PLAIN ) - .entity( "Archive created request is accepted." ) - .build() ); + .item( accepted ? + accepted().type( MediaType.TEXT_PLAIN ) + .entity( "Archive created request is accepted." ) + .build() : + Response.status( CONFLICT ) + .type( MediaType.TEXT_PLAIN ) + .entity( + "Another generation with same build config ID was already in progress, request is conflicted." ) + .build() ); } @Operation( description = "Get the status of generating archive based on build config Id" ) From 5ce8feba6d7ff0fb0c1e3bc249410f36f0d5e843 Mon Sep 17 00:00:00 2001 From: yma Date: Fri, 6 Dec 2024 09:27:57 +0800 Subject: [PATCH 8/8] Fix response to accepted for concurrent generation --- .../archive/controller/ArchiveController.java | 7 +++---- .../archive/jaxrs/ArchiveManageResources.java | 16 ++++------------ 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java index b8a75f4..1b76c48 100644 --- a/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java +++ b/src/main/java/org/commonjava/indy/service/archive/controller/ArchiveController.java @@ -156,7 +156,7 @@ public void destroy() IOUtils.closeQuietly( client, null ); } - public boolean generate( HistoricalContentDTO content ) + public void generate( HistoricalContentDTO content ) { String buildConfigId = content.getBuildConfigId(); Object lock = buildConfigLocks.computeIfAbsent( buildConfigId, k -> new Object() ); @@ -166,8 +166,8 @@ public boolean generate( HistoricalContentDTO content ) { logger.info( "There is already generation process in progress for buildConfigId {}, try lock wait.", buildConfigId ); - // Conflicted generation, just return with expected http response immediately - return false; + // Conflicted generation, just return immediately + return; } recordInProgress( buildConfigId ); @@ -208,7 +208,6 @@ public boolean generate( HistoricalContentDTO content ) logger.error( "Generation future task level failed for buildConfigId {}", buildConfigId, e ); } } - return true; } protected Boolean doGenerate( HistoricalContentDTO content ) diff --git a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java index be4dbe8..96a8c60 100644 --- a/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java +++ b/src/main/java/org/commonjava/indy/service/archive/jaxrs/ArchiveManageResources.java @@ -51,7 +51,6 @@ import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; import static jakarta.ws.rs.core.MediaType.APPLICATION_OCTET_STREAM; -import static jakarta.ws.rs.core.Response.Status.CONFLICT; import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static jakarta.ws.rs.core.Response.accepted; import static jakarta.ws.rs.core.Response.noContent; @@ -75,7 +74,6 @@ public class ArchiveManageResources @Operation( description = "Generate archive based on tracked content" ) @APIResponse( responseCode = "202", description = "The archive created request is accepted" ) - @APIResponse( responseCode = "409", description = "The archive created request is conflicted" ) @RequestBody( description = "The tracked content definition JSON", name = "body", required = true, content = @Content( mediaType = APPLICATION_JSON, example = "{" + "\"buildConfigId\": \"XXX\"," + "\"downloads\":" + "[{" @@ -106,17 +104,11 @@ public Uni create( final @Context UriInfo uriInfo, final InputStream b return fromResponse( message ); } - boolean accepted = controller.generate( content ); + controller.generate( content ); return Uni.createFrom() - .item( accepted ? - accepted().type( MediaType.TEXT_PLAIN ) - .entity( "Archive created request is accepted." ) - .build() : - Response.status( CONFLICT ) - .type( MediaType.TEXT_PLAIN ) - .entity( - "Another generation with same build config ID was already in progress, request is conflicted." ) - .build() ); + .item( accepted().type( MediaType.TEXT_PLAIN ) + .entity( "Archive created request is accepted." ) + .build() ); } @Operation( description = "Get the status of generating archive based on build config Id" )