From 3dab9e97da0d13c6317c9787141df8155f219533 Mon Sep 17 00:00:00 2001 From: Scott Wicken <1562170+swicken@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:40:10 -0400 Subject: [PATCH 1/3] feat(storage): store push-publishing bundles durably in S3 asset storage Fifth slice of the S3 asset storage work. With FEATURE_FLAG_S3_ASSET_STORAGE on, generated and received publishing bundles are staged privately and published to the publishing-bundles S3 group before replacing the local archive, reads restore archives on demand, and committed bundle deletion schedules a retrying bundleArchiveCleanup job. The cleanup processor does not register while the flag is off, and flag-off bundle handling matches main. --- docs/testing/BINARY_S3_STORAGE.md | 23 ++- .../publishing/bundlers/FileAssetBundler.java | 11 +- .../ajax/RemotePublishAjaxAction.java | 21 ++- .../dotcms/publisher/bundle/bean/Bundle.java | 3 + .../bundle/business/BundleAPIImpl.java | 3 + .../publisher/pusher/PushPublisher.java | 4 +- .../publisher/receiver/BundlePublisher.java | 4 +- .../com/dotcms/publishing/BundlerUtil.java | 9 + .../dotcms/publishing/PublisherAPIImpl.java | 8 +- .../manifest/ManifestReaderFactory.java | 5 + .../publishing/manifest/ManifestUtil.java | 15 +- .../output/BundleArchiveCleanupProcessor.java | 55 ++++++ .../output/BundleArchiveStorage.java | 147 +++++++++++++++ .../publishing/output/BundleOutput.java | 16 ++ .../output/TarGzipBundleOutput.java | 45 ++++- .../dotcms/rest/BundlePublisherResource.java | 6 +- .../java/com/dotcms/rest/BundleResource.java | 18 +- .../v1/publishing/PublishingRetryHelper.java | 2 +- .../dotcms/storage/AssetStorageFeature.java | 4 +- .../FileSystemStoragePersistenceAPIImpl.java | 3 +- .../storage/AssetStorageFeatureLatchTest.java | 4 +- .../storage/AssetStorageFeatureTest.java | 15 ++ .../dotcms/storage/BinaryS3StorageTest.java | 52 ++++++ .../test/java/com/dotcms/Junit5Suite1.java | 3 +- .../PublishingArchiveStorageTest.java | 176 ++++++++++++++++++ 25 files changed, 621 insertions(+), 31 deletions(-) create mode 100644 dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java create mode 100644 dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java create mode 100644 dotcms-integration/src/test/java/com/dotcms/publishing/PublishingArchiveStorageTest.java diff --git a/docs/testing/BINARY_S3_STORAGE.md b/docs/testing/BINARY_S3_STORAGE.md index d31e1a4e8327..47e4eca0d5f8 100644 --- a/docs/testing/BINARY_S3_STORAGE.md +++ b/docs/testing/BINARY_S3_STORAGE.md @@ -36,7 +36,7 @@ overrides through `Config.setProperty` do re-read it, which is how tests switch that mock `Config` statically must call `AssetStorageFeature.reset()` themselves. With the flag off, the S3 job queues (`binaryAssetCleanup`, `binaryFieldCleanup`, -`binaryAssetBackfill`) are not registered. +`binaryAssetBackfill`, `bundleArchiveCleanup`) are not registered. ### Enabling the flag is not rollback-safe @@ -79,8 +79,8 @@ The storage chain (`ChainableStoragePersistenceAPI`) and its providers change as `BinaryAssetStorageAPI` (`APILocator.getBinaryAssetStorageAPI()`) manages binary assets and completed renditions through the storage chain. Binary assets use the `binary-assets` group under the asset root, laid out as `{inode[0]}/{inode[1]}/{inode}/{field}/{fileName}`. Renditions use the -`generated-assets` group under the `dotGenerated` root. With the flag on, keys in both groups keep -their mixed-case names; other groups are still lowercased. +`generated-assets` group under the `dotGenerated` root. With the flag on, keys in these groups keep +their mixed-case names, as do publishing bundles (below); other groups are still lowercased. With the flag on, S3 keeps one immutable copy of each set of bytes for these two groups, at `asset-blobs/sha256/////` @@ -289,6 +289,21 @@ repair transaction. The copies are uploaded before the commit, so a rollback lis them, and their metadata, if the repair rolls back; a failed deletion is logged and leaves an unreferenced object behind. +## Publishing bundles + +With the flag on, completed push-publishing archives (`.tar.gz`, including their +manifest) are stored durably in the `publishing-bundles` group, with the local bundle directory as +a cache (`BundleArchiveStorage`). Bundle ids keep their case and are checked so an archive cannot +resolve outside the bundle directory. Generated and received bundles are written to a private +staging file and published to S3 before they replace the last complete local archive, so an +incomplete upload never becomes visible. Reading a bundle's manifest or payload restores the +archive from S3 only when its bytes are needed. Storage failures are reported as failures, not as a +missing bundle. + +Deleting a bundle records a `bundleArchiveCleanup` job after the deletion commits. The job deletes +the S3 object and keeps the local bytes if remote deletion fails, so the existing queue can retry. +With the flag off, bundles are written, read and deleted in the bundle directory as before. + ## Local cache eviction With the flag on, the local asset directory is a cache that `BinaryCacheEvictionJob` can trim. @@ -400,7 +415,7 @@ stack: ```sh ./mvnw install -pl :dotcms-core --am -DskipTests -Ddocker.skip ./mvnw verify -pl :dotcms-integration -Dmaven.build.cache.enabled=false -Dcoreit.test.skip=false \ - -Dit.test=BinaryAssetStorageIntegrationTest,ContentletBackupStorageTest,SharedAssetStorageIntegrationTest,BinaryAssetStarterRestoreTest + -Dit.test=BinaryAssetStorageIntegrationTest,ContentletBackupStorageTest,SharedAssetStorageIntegrationTest,BinaryAssetStarterRestoreTest,PublishingArchiveStorageTest ``` These default to flag-off mode, where the S3 cases are skipped. To run the S3 cases, create a diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java index 0ed1753ab53e..c072db894934 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java @@ -371,7 +371,16 @@ private void writeFileToDisk(Host host, String languageId, BundleOutput output, else { //only write if changed if(!output.exists(filePath) || output.lastModified(filePath) != cal.getTimeInMillis()){ - File oldAsset = new File(APILocator.getFileAssetAPI().getRealAssetPathIgnoreExtensionCase(fileAssetWrapper.getAsset().getInode(), fileAssetWrapper.getAsset().getUnderlyingFileName())); + final File oldAsset = com.dotcms.storage.AssetStorageFeature.isEnabled() + ? APILocator.getBinaryAssetStorageAPI().getBinaryFile(fileAssetWrapper.getAsset().getInode(), FileAssetAPI.BINARY_FIELD) + : new File(APILocator.getFileAssetAPI().getRealAssetPathIgnoreExtensionCase(fileAssetWrapper.getAsset().getInode(), fileAssetWrapper.getAsset().getUnderlyingFileName())); + if (oldAsset == null) { + Logger.warn(this, String.format( + "Binary file not found for inode '%s' (underlying name: '%s') — skipping bundle", + fileAssetWrapper.getAsset().getInode(), + fileAssetWrapper.getAsset().getUnderlyingFileName())); + return; + } if(output.exists(filePath)) { output.delete(filePath); } diff --git a/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java b/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java index 09425e7b3294..6815ab16a466 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java @@ -413,7 +413,7 @@ public void retry ( HttpServletRequest request, HttpServletResponse response ) t Verify if the bundle exist and was created correctly..., meaning, if there is not a .tar.gz file is because something happened on the creation of the bundle. */ - File bundleFile = new File( ConfigUtils.getBundlePath() + File.separator + basicConfig.getId() + ".tar.gz" ); + File bundleFile = com.dotcms.publishing.output.TarGzipBundleOutput.getBundleTarGzipFile(basicConfig.getId()); if ( !bundleFile.exists() ) { Logger.warn( this.getClass(), "No Push Publish Bundle with id: " + bundleId + " found." ); appendMessage( responseMessage, "publisher_retry.error.not.found", bundleId, true ); @@ -520,7 +520,9 @@ public void downloadBundle ( HttpServletRequest request, HttpServletResponse res ArrayList list = new ArrayList<>( 1 ); list.add( bundleRoot ); - File bundle = new File( bundleRoot + File.separator + ".." + File.separator + config.getId() + ".tar.gz" ); + File bundle = com.dotcms.storage.AssetStorageFeature.isEnabled() + ? com.dotcms.publishing.output.TarGzipBundleOutput.getBundleTarGzipFile(config.getId()) + : new File( bundleRoot + File.separator + ".." + File.separator + config.getId() + ".tar.gz" ); if ( !bundle.exists() ) { response.sendError( 500, "No Bundle Found" ); return; @@ -634,7 +636,14 @@ public void downloadUnpushedBundle ( HttpServletRequest request, HttpServletResp //Clean the just created bundle because on each download we will generate a new bundle file with a new id in order to avoid conflicts with ids final File bundleRoot = BundlerUtil.getBundleRoot( bundleId ); final File compressedBundle = new File( ConfigUtils.getBundlePath() + File.separator + bundleId + ".tar.gz" ); - if ( compressedBundle.exists() ) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try { + com.dotcms.publishing.output.BundleArchiveStorage.getInstance().delete(bundleId); + } catch (DotDataException e) { + Logger.error(this, "Unable to remove generated download bundle " + bundleId, e); + } + } + else if ( compressedBundle.exists() ) { compressedBundle.delete(); if ( bundleRoot.exists() ) { com.liferay.util.FileUtil.deltree( bundleRoot ); @@ -689,7 +698,11 @@ public void uploadBundle ( HttpServletRequest request, HttpServletResponse respo status = PublishAuditAPI.getInstance().updateAuditTable( endpointId, endpointId, bundleFolder ); // Write file on FS - FileUtil.writeToFile( bundle, bundlePath + bundleName ); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + com.dotcms.publishing.output.BundleArchiveStorage.getInstance().receive(bundleName, bundle); + } else { + FileUtil.writeToFile( bundle, bundlePath + bundleName ); + } if ( !status.getStatus().equals( Status.PUBLISHING_BUNDLE ) ) { PushPublisherJob.triggerPushPublisherJob(bundleName, status); diff --git a/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java b/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java index 8169d9dd560a..4db4ef745b11 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java @@ -92,6 +92,9 @@ public void setFilterKey(final String filterKey) { * @return boolean - true if the bundle exists. */ public boolean bundleTgzExists() { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + return com.dotcms.publishing.output.BundleArchiveStorage.getInstance().exists(id); + } return Try.of(()->new File( ConfigUtils.getBundlePath() + File.separator + id + ".tar.gz" ).exists()).getOrElse(false); diff --git a/dotCMS/src/main/java/com/dotcms/publisher/bundle/business/BundleAPIImpl.java b/dotCMS/src/main/java/com/dotcms/publisher/bundle/business/BundleAPIImpl.java index 8036a699f11e..588823f83e0d 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/bundle/business/BundleAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/bundle/business/BundleAPIImpl.java @@ -119,6 +119,9 @@ public Bundle getBundleById(String id) throws DotDataException { @WrapInTransaction @Override public void deleteBundle(String id) throws DotDataException { + if (UtilMethods.isSet(id)) { + com.dotcms.publishing.output.BundleArchiveCleanupProcessor.enqueue(id); + } bundleFactory.deleteBundle(id); } diff --git a/dotCMS/src/main/java/com/dotcms/publisher/pusher/PushPublisher.java b/dotCMS/src/main/java/com/dotcms/publisher/pusher/PushPublisher.java index c82f47b36fec..783627f527ae 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/pusher/PushPublisher.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/pusher/PushPublisher.java @@ -162,7 +162,9 @@ public PublisherConfig process ( final PublishStatus status ) throws DotPublishi File bundleRoot = BundlerUtil.getBundleRoot(this.config.getName(), false); final List list = new ArrayList<>(1); list.add(bundleRoot); - File bundleFile = new File(bundleRoot + ".tar.gz"); + File bundleFile = com.dotcms.storage.AssetStorageFeature.isEnabled() + ? com.dotcms.publishing.output.TarGzipBundleOutput.getBundleTarGzipFile(this.config.getId()) + : new File(bundleRoot + ".tar.gz"); List environments = APILocator.getEnvironmentAPI().findEnvironmentsByBundleId(this.config.getId()); diff --git a/dotCMS/src/main/java/com/dotcms/publisher/receiver/BundlePublisher.java b/dotCMS/src/main/java/com/dotcms/publisher/receiver/BundlePublisher.java index 7806cade4eb6..1de4a1e981c1 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/receiver/BundlePublisher.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/receiver/BundlePublisher.java @@ -199,7 +199,9 @@ public PublisherConfig process ( final PublishStatus status ) throws DotPublishi // Extract file to a directory InputStream bundleIS = null; try { - bundleIS = Files.newInputStream(Paths.get(bundlePath + bundleName)); + bundleIS = com.dotcms.storage.AssetStorageFeature.isEnabled() + ? Files.newInputStream(com.dotcms.publishing.output.TarGzipBundleOutput.getBundleTarGzipFile(bundleID).toPath()) + : Files.newInputStream(Paths.get(bundlePath + bundleName)); untar(bundleIS, folderOut.getAbsolutePath() + File.separator + bundleName, bundleName); } finally { CloseUtils.closeQuietly(bundleIS); diff --git a/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java b/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java index af9196bb9699..80c0d93b566d 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java @@ -142,6 +142,9 @@ private static boolean bundleExists(String bundleId) { final File bundleRoot = BundlerUtil.getBundleRoot( basicConfig.getName(), false ); final File bundleStaticFile = new File(bundleRoot.getAbsolutePath() + PublisherConfig.STATIC_SUFFIX); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + return bundleStaticFile.exists() || com.dotcms.publishing.output.BundleArchiveStorage.getInstance().exists(bundleId); + } if ( !bundleStaticFile.exists() ) { return true; } @@ -210,6 +213,9 @@ public static void writeBundleXML(final PublisherConfig config, final BundleOutp try (final OutputStream outputStream = output.addFile(bundleXmlFilePath)) { objectToXML(config, outputStream); } catch ( IOException e ) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + throw new DotRuntimeException("Unable to include bundle descriptor", e); + } Logger.error( BundlerUtil.class, e.getMessage(), e ); } } @@ -522,6 +528,9 @@ public static String sanitizeBundleName(String bundleName) throws DotPublisherEx } public static boolean tarGzipExists(final String bundleId) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + return com.dotcms.publishing.output.BundleArchiveStorage.getInstance().exists(bundleId); + } final File bundleTarGzip = TarGzipBundleOutput.getBundleTarGzipFile(bundleId); return bundleTarGzip.exists(); } diff --git a/dotCMS/src/main/java/com/dotcms/publishing/PublisherAPIImpl.java b/dotCMS/src/main/java/com/dotcms/publishing/PublisherAPIImpl.java index ebb80ae72537..ba980f221ee9 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/PublisherAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/PublisherAPIImpl.java @@ -154,6 +154,9 @@ public PublishStatus publish ( PublisherConfig config, PublishStatus status) thr } else { addBundleXMLIntoBundle(config, output); } + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + output.complete(); + } } else { Logger.info(this, "Retrying bundle: " + config.getId() + ", we don't need to run bundlers again"); @@ -202,6 +205,9 @@ private void addManifestIntoBundleOutput(final BundleOutput output, manifestBuilder.close(); output.copyFile(manifestFile, File.separator + ManifestBuilder.MANIFEST_NAME); } catch (final IOException e) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + throw new com.dotmarketing.exception.DotRuntimeException("Unable to include bundle manifest", e); + } Logger.error(PublisherAPIImpl.class, "Error trying to copy the manifest file: " + e.getMessage()); } @@ -441,4 +447,4 @@ private FilterDescriptor createFilterFromFile(final Path path) { } } -} \ No newline at end of file +} diff --git a/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestReaderFactory.java b/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestReaderFactory.java index e980608cb336..28b13c5535a7 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestReaderFactory.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestReaderFactory.java @@ -24,6 +24,11 @@ public CSVManifestReader createCSVManifestReader(final String bundleID){ throw new IllegalArgumentException("Manifest not found for: " + bundleID); } + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try (Reader reader = manifestInputStream.get()) { + return new CSVManifestReader(reader); + } + } return new CSVManifestReader(manifestInputStream.get()); } catch (IOException e) { throw new DotRuntimeException(e); diff --git a/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestUtil.java b/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestUtil.java index 0740aff214b4..ae85d7744740 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestUtil.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/manifest/ManifestUtil.java @@ -32,7 +32,16 @@ public class ManifestUtil { public static Optional getManifestInputStream(@NotNull File bundleTarGzipFile) throws IOException { - if(bundleTarGzipFile.getCanonicalPath().startsWith(ConfigUtils.getBundlePath())) { + final boolean s3 = com.dotcms.storage.AssetStorageFeature.isEnabled(); + final boolean allowed = s3 + ? bundleTarGzipFile.getCanonicalFile().toPath().startsWith(new File(ConfigUtils.getBundlePath()).getCanonicalFile().toPath()) + : bundleTarGzipFile.getCanonicalPath().startsWith(ConfigUtils.getBundlePath()); + if(allowed) { + if (s3 && !bundleTarGzipFile.exists() && bundleTarGzipFile.getName().endsWith(".tar.gz") + && bundleTarGzipFile.getCanonicalFile().getParentFile().equals(new File(ConfigUtils.getBundlePath()).getCanonicalFile())) { + final String name = bundleTarGzipFile.getName(); + bundleTarGzipFile = TarGzipBundleOutput.getBundleTarGzipFile(name.substring(0, name.length() - ".tar.gz".length())); + } try (final FileInputStream fileInputStream = new FileInputStream(bundleTarGzipFile); BufferedInputStream in = new BufferedInputStream(fileInputStream); GzipCompressorInputStream gzIn = new GzipCompressorInputStream(in); @@ -42,6 +51,10 @@ public static Optional getManifestInputStream(@NotNull File bundleTarGzi while ((entry = (TarArchiveEntry) tarInputStream.getNextEntry()) != null) { if (entry.isFile() && entry.getName().equals(ManifestBuilder.MANIFEST_NAME)) { + if (s3) { + return Optional.of(new java.io.StringReader(IOUtils.toString(tarInputStream, + java.nio.charset.Charset.defaultCharset()))); + } final File tempManifestFile = FileUtil.createTemporaryFile("Manifest_"); try (final FileOutputStream fileOutputStream = new FileOutputStream(tempManifestFile)) { IOUtils.copy(tarInputStream, fileOutputStream); diff --git a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java new file mode 100644 index 000000000000..f0ffd49c14c3 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java @@ -0,0 +1,55 @@ +package com.dotcms.publishing.output; + +import com.dotcms.business.CloseDBIfOpened; +import com.dotcms.jobs.business.error.JobProcessingException; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.jobs.business.processor.ExponentialBackoffRetryPolicy; +import com.dotcms.jobs.business.processor.JobProcessor; +import com.dotcms.jobs.business.processor.Queue; +import com.dotcms.storage.AssetStorageFeature; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.exception.DotDataException; +import java.util.Map; +import javax.enterprise.context.Dependent; + +/** The existing transactional job queue retains archive deletion intent across storage failures. */ +@Dependent +@Queue(BundleArchiveCleanupProcessor.QUEUE) +@ExponentialBackoffRetryPolicy +public class BundleArchiveCleanupProcessor implements JobProcessor { + public static final String QUEUE = "bundleArchiveCleanup"; + + public static void enqueue(final String id) throws DotDataException { + if (!AssetStorageFeature.isEnabled()) return; + BundleArchiveStorage.localArchive(id); + if (!DbConnectionFactory.inTransaction()) { + throw new DotDataException("Archive cleanup must be recorded in the bundle deletion transaction"); + } + APILocator.getJobQueueManagerAPI().createJob(QUEUE, Map.of("bundleId", id)); + } + + @Override + @CloseDBIfOpened + public void process(final Job job) throws JobProcessingException { + if (!AssetStorageFeature.isEnabled()) { + throw new JobProcessingException(job.id(), "S3 asset storage is disabled"); + } + final String id = (String) job.parameters().get("bundleId"); + try { + if (!new DotConnect().setSQL("select id from publishing_bundle where id = ?") + .addParam(id).loadObjectResults().isEmpty()) { + throw new JobProcessingException(job.id(), "Bundle still exists: " + id); + } + BundleArchiveStorage.getInstance().delete(id); + } catch (DotDataException | RuntimeException e) { + throw new JobProcessingException(job.id(), "Unable to remove bundle archive " + id, e); + } + } + + @Override + public Map getResultMetadata(final Job job) { + return Map.of("bundleId", job.parameters().get("bundleId")); + } +} diff --git a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java new file mode 100644 index 000000000000..2992e00f860d --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java @@ -0,0 +1,147 @@ +package com.dotcms.publishing.output; + +import com.dotcms.storage.AmazonS3StoragePersistenceAPIImpl; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.ChainableStoragePersistenceAPIBuilder; +import com.dotcms.storage.FileSystemStoragePersistenceAPIImpl; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotcms.storage.StoragePersistenceProvider; +import com.dotcms.storage.StorageType; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotRuntimeException; +import com.dotmarketing.util.ConfigUtils; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +/** Completed publishing archives, including their manifest, share one durable object and local cache. */ +public final class BundleArchiveStorage { + public static final String GROUP = "publishing-bundles"; + private StoragePersistenceAPI storage; + private boolean initialized; + + private static class Holder { + private static final BundleArchiveStorage INSTANCE = new BundleArchiveStorage(null); + } + + public static BundleArchiveStorage getInstance() { + return Holder.INSTANCE; + } + + public BundleArchiveStorage(final StoragePersistenceAPI storage) { + this.storage = storage; + } + + /** Resolves a pathname without downloading, for producers and cleanup. */ + public static File localArchive(final String id) { + final File file = new File(ConfigUtils.getBundlePath(), id + ".tar.gz"); + if (AssetStorageFeature.isEnabled()) { + if (id == null || id.isBlank() || id.equals(".") || id.equals("..") + || id.contains("/") || id.contains("\\") || id.indexOf('\0') >= 0) { + throw new IllegalArgumentException("Invalid bundle id"); + } + try { + if (!file.getCanonicalFile().toPath().equals(new File(ConfigUtils.getBundlePath()) + .getCanonicalFile().toPath().resolve(id + ".tar.gz"))) { + throw new IllegalArgumentException("Bundle archive is outside the bundle directory"); + } + } catch (IOException e) { + throw new DotRuntimeException(e); + } + } + return file; + } + + private synchronized StoragePersistenceAPI storage() throws DotDataException { + if (storage == null) { + storage = new ChainableStoragePersistenceAPIBuilder() + .add(StoragePersistenceProvider.INSTANCE.get().getStorage(StorageType.FILE_SYSTEM)) + .add(AmazonS3StoragePersistenceAPIImpl.withPlainPaths()).get(); + } + if (!initialized) { + final File root = new File(ConfigUtils.getBundlePath()); + if (!root.isDirectory() && !root.mkdirs()) { + throw new DotDataException("Unable to create bundle directory " + root); + } + storage.createGroup(GROUP, Map.of(FileSystemStoragePersistenceAPIImpl.GROUP_DIRECTORY, root)); + initialized = true; + } + return storage; + } + + /** Missing objects retain the expected pathname; storage failures are never reported as absence. */ + public boolean exists(final String id) { + final File file = localArchive(id); + if (file.isFile() || !AssetStorageFeature.isEnabled()) return file.isFile(); + try { + return storage().existsObject(GROUP, file.getName()); + } catch (DotDataException e) { + throw new DotRuntimeException("Unable to check publishing archive " + id, e); + } + } + + /** Restores only when the archive bytes are required. */ + public File get(final String id) { + final File file = localArchive(id); + if (!AssetStorageFeature.isEnabled()) return file; + try { + final File restored = storage().pullFile(GROUP, file.getName()); + return restored == null ? file : restored; + } catch (DotDataException e) { + throw new DotRuntimeException("Unable to retrieve publishing archive " + id, e); + } + } + + /** The caller supplies a closed, complete archive. The chain publishes remotely before replacing local bytes. */ + public void store(final String id, final File source) throws IOException { + if (!AssetStorageFeature.isEnabled()) return; + final File archive = localArchive(id); + try { + storage().pushFile(GROUP, archive.getName(), source, Map.of()); + } catch (DotDataException e) { + throw new IOException("Unable to store publishing archive " + id, e); + } + } + + /** Own staging file keeps incomplete uploads away from the last complete archive. Caller owns the input stream. */ + public void receive(final String fileName, final InputStream input) throws IOException { + if (!fileName.endsWith(".tar.gz")) throw new IllegalArgumentException("Expected a bundle archive"); + final String id = fileName.substring(0, fileName.length() - ".tar.gz".length()); + final Path target = localArchive(id).toPath(); + if (!AssetStorageFeature.isEnabled()) { + com.dotmarketing.util.FileUtil.writeToFile(input, target.toString()); + return; + } + Files.createDirectories(target.getParent()); + final Path stage = Files.createTempFile(target.getParent(), ".bundle-upload-", ".tmp"); + try { + Files.copy(input, stage, java.nio.file.StandardCopyOption.REPLACE_EXISTING); + store(id, stage.toFile()); + } finally { + Files.deleteIfExists(stage); + } + } + + /** Called after committed bundle deletion; retains local bytes if remote deletion fails. */ + public void delete(final String id) throws DotDataException { + if (!AssetStorageFeature.isEnabled()) return; + final File archive = localArchive(id); + storage().deleteObjectAndReferences(GROUP, archive.getName()); + final File extracted = new File(archive.getParentFile(), id); + try { + if (!extracted.getCanonicalFile().toPath().equals(archive.getCanonicalFile().getParentFile().toPath().resolve(id))) { + throw new DotDataException("Bundle extraction directory is outside the bundle directory"); + } + com.liferay.util.FileUtil.deltree(extracted); + if (archive.exists() || extracted.exists()) { + throw new DotDataException("Unable to remove bundle cache " + id); + } + } catch (IOException e) { + throw new DotDataException(e); + } + } + +} diff --git a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleOutput.java b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleOutput.java index 0a32b6de53ae..7566dafe5f7a 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleOutput.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleOutput.java @@ -23,6 +23,9 @@ public BundleOutput(final PublisherConfig publisherConfig){ public abstract void create() throws IOException; + /** Called only after every bundler and the manifest have completed successfully. */ + public void complete() throws IOException { } + /** * Add a new file into the output * @@ -51,6 +54,19 @@ public OutputStream addFile(File file) throws FileCreationException { * @throws IOException */ public void copyFile(File source, String destinationPath) throws IOException { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + final var binaries = com.dotmarketing.business.APILocator.getBinaryAssetStorageAPI(); + try (var lease = binaries.acquireCacheLease(); var input = binaries.openLocalFile(source)) { + copyFileInternal(source, destinationPath); + } catch (com.dotmarketing.exception.DotDataException e) { + throw new IOException("Unable to read bundle source " + source, e); + } + return; + } + copyFileInternal(source, destinationPath); + } + + private void copyFileInternal(File source, String destinationPath) throws IOException { final boolean userHardLink = Config.getBooleanProperty("CONTENT_VERSION_HARD_LINK", true) && this.useHardLinkByDefault(); diff --git a/dotCMS/src/main/java/com/dotcms/publishing/output/TarGzipBundleOutput.java b/dotCMS/src/main/java/com/dotcms/publishing/output/TarGzipBundleOutput.java index 349e2e40e4de..c8de3fdf71c0 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/output/TarGzipBundleOutput.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/output/TarGzipBundleOutput.java @@ -21,18 +21,23 @@ public class TarGzipBundleOutput extends BundleOutput { private File tarGzipFile; private TarArchiveOutputStream tarArchiveOutputStream; + private java.nio.file.Path stagedArchive; private int GZIP_OUTPUT_STREAM_BUFFER_SIZE = Config .getIntProperty("GZIP_OUTPUT_STREAM_BUFFER_SIZE", 65536); public TarGzipBundleOutput(final PublisherConfig publisherConfig) throws IOException { super(publisherConfig); - tarGzipFile = getBundleTarGzipFile(publisherConfig.getId()); + tarGzipFile = BundleArchiveStorage.localArchive(publisherConfig.getId()); } @Override public void create() throws IOException { - final OutputStream outputStream = Files.newOutputStream(tarGzipFile.toPath()); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + Files.createDirectories(tarGzipFile.toPath().getParent()); + stagedArchive = Files.createTempFile(tarGzipFile.toPath().getParent(), ".bundle-build-", ".tmp"); + } + final OutputStream outputStream = Files.newOutputStream(stagedArchive == null ? tarGzipFile.toPath() : stagedArchive); tarArchiveOutputStream = new TarArchiveOutputStream(new GZIPOutputStream(outputStream, GZIP_OUTPUT_STREAM_BUFFER_SIZE)); @@ -42,8 +47,22 @@ public void create() throws IOException { } public static File getBundleTarGzipFile(final String bundleId) { - final String fileName = String.format("%s%s%s.tar.gz",ConfigUtils.getBundlePath(),File.separator,bundleId); - return new File(fileName); + return BundleArchiveStorage.getInstance().get(bundleId); + } + + @Override + public boolean bundleFileExists() { + return com.dotcms.storage.AssetStorageFeature.isEnabled() + ? BundleArchiveStorage.getInstance().exists(publisherConfig.getId()) : super.bundleFileExists(); + } + + @Override + public void complete() throws IOException { + if (stagedArchive != null) { + tarArchiveOutputStream.close(); + tarArchiveOutputStream = null; + BundleArchiveStorage.getInstance().store(publisherConfig.getId(), stagedArchive.toFile()); + } } @Override @@ -68,8 +87,14 @@ public void delete(final String filePath) { @Override public void close() throws IOException { - if (UtilMethods.isSet(tarArchiveOutputStream)) { - tarArchiveOutputStream.close(); + try { + if (UtilMethods.isSet(tarArchiveOutputStream)) { + tarArchiveOutputStream.close(); + } + } finally { + if (stagedArchive != null) { + Files.deleteIfExists(stagedArchive); + } } } @@ -95,7 +120,13 @@ public void innerCopyFile(final File source, final String destinationPath) throw tarArchiveEntry.setSize(source.length()); tarArchiveOutputStream.putArchiveEntry(tarArchiveEntry); - IOUtils.copy(new FileInputStream(source), tarArchiveOutputStream); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try (var input = Files.newInputStream(source.toPath())) { + IOUtils.copy(input, tarArchiveOutputStream); + } + } else { + IOUtils.copy(new FileInputStream(source), tarArchiveOutputStream); + } } finally { tarArchiveOutputStream.closeArchiveEntry(); } diff --git a/dotCMS/src/main/java/com/dotcms/rest/BundlePublisherResource.java b/dotCMS/src/main/java/com/dotcms/rest/BundlePublisherResource.java index 4bd341636b71..dd6a6aab87f5 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/BundlePublisherResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/BundlePublisherResource.java @@ -161,7 +161,11 @@ private Bundle publishBundle(final boolean forcePush, } //Write file on FS - FileUtil.writeToFile(bundleStream, bundlePath + fileName); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + com.dotcms.publishing.output.BundleArchiveStorage.getInstance().receive(fileName, bundleStream); + } else { + FileUtil.writeToFile(bundleStream, bundlePath + fileName); + } //Start thread diff --git a/dotCMS/src/main/java/com/dotcms/rest/BundleResource.java b/dotCMS/src/main/java/com/dotcms/rest/BundleResource.java index 536bb211b4e4..ea21f9b1f84d 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/BundleResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/BundleResource.java @@ -779,8 +779,7 @@ public final Response downloadBundle(@Context final HttpServletRequest request, if (!UtilMethods.isSet(bundle)) { throw new DoesNotExistException("Bundle with ID: " + bundleId + " not found"); } - final File bundleFile = new File( - ConfigUtils.getBundlePath() + File.separator + bundle.getId() + ".tar.gz"); + final File bundleFile = TarGzipBundleOutput.getBundleTarGzipFile(bundle.getId()); if (!bundleFile.exists()) { throw new DoesNotExistException( "The bundle has not been generated for the provided bundle ID: " @@ -788,7 +787,8 @@ public final Response downloadBundle(@Context final HttpServletRequest request, } final String bundleName = bundle.getName().replaceAll("[^\\w.-]", "_"); response.setHeader( "Content-Disposition", "attachment; filename=" +bundleName +"-"+ bundle.getId() + ".tar.gz" ); - return Response.ok(bundleFile, "application/x-tgz").build(); + return Response.ok(com.dotcms.storage.AssetStorageFeature.isEnabled() + ? java.nio.file.Files.newInputStream(bundleFile.toPath()) : bundleFile, "application/x-tgz").build(); }catch (DoesNotExistException e){ Logger.error(this,e.getMessage()); return ExceptionMapperUtil.createResponse("",e.getMessage(),Response.Status.NOT_FOUND); @@ -928,7 +928,11 @@ public final Response uploadBundleSync(@Context final HttpServletRequest request final String bundleName = BundlerUtil.sanitizeBundleName(fileName); final String bundlePath = ConfigUtils.getBundlePath() + File.separator; - FileUtil.writeToFile(inputStream, bundlePath + bundleName); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + com.dotcms.publishing.output.BundleArchiveStorage.getInstance().receive(bundleName, inputStream); + } else { + FileUtil.writeToFile(inputStream, bundlePath + bundleName); + } final String bundleFolder = bundleName.substring(0, bundleName.indexOf(".tar.gz")); final String endpointId = initData.getUser().getUserId(); @@ -986,7 +990,11 @@ public final void uploadBundleAsync(@Context final HttpServletRequest request, @ final String bundleName = BundlerUtil.sanitizeBundleName(fileName); final String bundlePath = ConfigUtils.getBundlePath() + File.separator; - FileUtil.writeToFile(inputStream, bundlePath + bundleName); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + com.dotcms.publishing.output.BundleArchiveStorage.getInstance().receive(bundleName, inputStream); + } else { + FileUtil.writeToFile(inputStream, bundlePath + bundleName); + } final String bundleFolder = bundleName.substring(0, bundleName.indexOf(".tar.gz")); final String endpointId = initData.getUser().getUserId(); diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingRetryHelper.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingRetryHelper.java index c7364360b8b7..af41c83b6cc4 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingRetryHelper.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingRetryHelper.java @@ -283,7 +283,7 @@ private RetryResultDTO retryPushBundle( throws DotPublisherException, DotDataException { // Verify bundle tar.gz file exists - final File bundleFile = new File(ConfigUtils.getBundlePath() + File.separator + basicConfig.getId() + ".tar.gz"); + final File bundleFile = com.dotcms.publishing.output.TarGzipBundleOutput.getBundleTarGzipFile(basicConfig.getId()); if (!bundleFile.exists()) { Logger.warn(this, "No Push Publish Bundle with id: " + bundleId + " found."); throw new DotPublisherException("Bundle file not found: " + bundleId); diff --git a/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java b/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java index af53e866af69..22f7c37d3fec 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java +++ b/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java @@ -1,5 +1,6 @@ package com.dotcms.storage; +import com.dotcms.publishing.output.BundleArchiveCleanupProcessor; import com.dotcms.storage.binary.BinaryAssetBackfillProcessor; import com.dotcms.storage.binary.BinaryAssetCleanupProcessor; import com.dotcms.storage.binary.BinaryFieldCleanupProcessor; @@ -20,7 +21,8 @@ public final class AssetStorageFeature { /** Job processors that belong to the S3 lifecycle and must not register while it is disabled. */ private static final Set> JOB_PROCESSORS = Set.of(BinaryAssetBackfillProcessor.class, - BinaryAssetCleanupProcessor.class, BinaryFieldCleanupProcessor.class); + BinaryAssetCleanupProcessor.class, BinaryFieldCleanupProcessor.class, + BundleArchiveCleanupProcessor.class); private static volatile Boolean enabled; diff --git a/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java b/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java index 665cf679e259..8d3ee9241cf6 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java @@ -99,7 +99,8 @@ public void addGroupMapping(final String groupName, final File folder) { private String normalizePath(final String groupName, final String path) { return AssetStorageFeature.isEnabled() && (com.dotcms.storage.binary.BinaryAssetStorageAPI.BINARY_ASSETS_GROUP.equalsIgnoreCase(groupName) - || com.dotcms.storage.binary.BinaryAssetStorageAPI.GENERATED_ASSETS_GROUP.equalsIgnoreCase(groupName)) + || com.dotcms.storage.binary.BinaryAssetStorageAPI.GENERATED_ASSETS_GROUP.equalsIgnoreCase(groupName) + || com.dotcms.publishing.output.BundleArchiveStorage.GROUP.equalsIgnoreCase(groupName)) ? path : path.toLowerCase(); } diff --git a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java index 50e1fbad4be1..58d29ec1a60b 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mockStatic; +import com.dotcms.publishing.output.BundleArchiveCleanupProcessor; import com.dotcms.storage.binary.BinaryAssetBackfillProcessor; import com.dotcms.storage.binary.BinaryAssetCleanupProcessor; import com.dotcms.storage.binary.BinaryFieldCleanupProcessor; @@ -15,7 +16,8 @@ class AssetStorageFeatureLatchTest { private static final List> S3_PROCESSORS = List.of(BinaryAssetBackfillProcessor.class, - BinaryAssetCleanupProcessor.class, BinaryFieldCleanupProcessor.class); + BinaryAssetCleanupProcessor.class, BinaryFieldCleanupProcessor.class, + BundleArchiveCleanupProcessor.class); private String previous; diff --git a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java index c16f2540ce3f..8c24e9068cb7 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java @@ -23,6 +23,21 @@ import static org.mockito.Mockito.*; class AssetStorageFeatureTest { + @Test + void disabledPublishingArchivesDoNotInitializeStorage(@org.junit.jupiter.api.io.TempDir Path root) throws Exception { + Config.setProperty(AssetStorageFeature.FLAG, false); + final var provider = mock(StoragePersistenceAPI.class); + final var archives = new com.dotcms.publishing.output.BundleArchiveStorage(provider); + try (var paths = mockStatic(ConfigUtils.class)) { + paths.when(ConfigUtils::getBundlePath).thenReturn(root.toString()); + final File file = Files.writeString(root.resolve("Mixed-Bundle.tar.gz"), "filesystem bundle").toFile(); + assertEquals(file, archives.get("Mixed-Bundle")); + archives.store("Mixed-Bundle", file); + archives.delete("Mixed-Bundle"); + assertEquals("filesystem bundle", Files.readString(file.toPath())); + verifyNoInteractions(provider); + } + } @TempDir Path root; private String previousFlag; private String previousRoot; diff --git a/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java b/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java index 935ff720fc3d..fe41befa58a2 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java @@ -186,6 +186,58 @@ private String readRemote(AmazonS3StoragePersistenceAPIImpl remote, String group finally { remote.releaseRetrievedFile(file); } } + @Test + void publishingArchivesPreserveCaseAndLastCompletedUpload() throws Exception { + final var archives = new com.dotcms.publishing.output.BundleArchiveStorage( + new ChainableStoragePersistenceAPI(new JsonWriterDelegate(), List.of(fs, s3), mock(Chainable404StorageCache.class))); + final String id = "Mixed-Case-Bundle"; + final String key = com.dotcms.publishing.output.BundleArchiveStorage.GROUP + "/" + id + ".tar.gz"; + final File source = Files.writeString(root.resolve("archive.tmp"), "last complete archive").toFile(); + archives.store(id, source); + final File local = archives.get(id); + assertEquals(id + ".tar.gz", local.getName()); + assertEquals(Files.readString(source.toPath()), client.getObjectAsString(bucket, key)); + Files.delete(local.toPath()); // Only this verified test cache copy is cleared. + assertTrue(archives.exists(id)); + assertFalse(local.exists(), "An existence check must not download the archive"); + assertEquals("last complete archive", Files.readString(archives.get(id).toPath())); + + try (var incomplete = new java.io.InputStream() { + int remaining = 10; + @Override public int read() throws java.io.IOException { + if (--remaining < 0) throw new java.io.IOException("Interrupted upload"); + return 'x'; + } + }) { + assertThrows(java.io.IOException.class, () -> archives.receive(id + ".tar.gz", incomplete)); + } + assertEquals("last complete archive", Files.readString(local.toPath())); + assertEquals("last complete archive", client.getObjectAsString(bucket, key)); + try (var files = Files.list(local.toPath().getParent())) { + assertFalse(files.anyMatch(path -> path.getFileName().toString().startsWith(".bundle-upload-"))); + } + assertThrows(IllegalArgumentException.class, () -> archives.get("../outside")); + final Path alias = local.toPath().resolveSibling("Alias.tar.gz"); + Files.createSymbolicLink(alias, local.toPath()); + assertThrows(IllegalArgumentException.class, () -> archives.get("Alias")); + Files.delete(alias); + archives.store(id + "-neighbor", source); + archives.store(id + ".tar.gz-shadow", source); + try (var input = Files.newInputStream(local.toPath())) { + archives.delete(id); + assertEquals("last complete archive", new String(input.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8)); + } + assertFalse(archives.get(id).exists()); + assertFalse(archives.exists(id), "A longer neighboring object key is not the requested archive"); + assertFalse(client.doesObjectExist(bucket, key)); + assertEquals("last complete archive", Files.readString(archives.get(id + "-neighbor").toPath())); + + final var failing = mock(StoragePersistenceAPI.class); + when(failing.pullFile(anyString(), anyString())).thenThrow(new com.dotmarketing.exception.DotDataException("S3 unavailable")); + assertThrows(com.dotmarketing.exception.DotRuntimeException.class, + () -> new com.dotcms.publishing.output.BundleArchiveStorage(failing).get(id)); + } + @Test void immutableReplacementCommitsAndRollsBackWithRealPostgres() throws Exception { String jdbc = System.getProperty("s3.test.jdbc"); diff --git a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java index 476bb7d06687..a729ef81664a 100644 --- a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java +++ b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java @@ -80,7 +80,8 @@ com.dotcms.storage.binary.BinaryAssetStorageIntegrationTest.class, com.dotcms.storage.binary.ContentletBackupStorageTest.class, com.dotcms.storage.binary.SharedAssetStorageIntegrationTest.class, - com.dotcms.storage.binary.BinaryAssetStarterRestoreTest.class + com.dotcms.storage.binary.BinaryAssetStarterRestoreTest.class, + com.dotcms.publishing.PublishingArchiveStorageTest.class }) public class Junit5Suite1 { diff --git a/dotcms-integration/src/test/java/com/dotcms/publishing/PublishingArchiveStorageTest.java b/dotcms-integration/src/test/java/com/dotcms/publishing/PublishingArchiveStorageTest.java new file mode 100644 index 000000000000..fc520f5561ee --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/publishing/PublishingArchiveStorageTest.java @@ -0,0 +1,176 @@ +package com.dotcms.publishing; + +import com.dotcms.datagen.FileAssetDataGen; +import com.dotcms.datagen.FolderDataGen; +import com.dotcms.publisher.bundle.bean.Bundle; +import com.dotcms.publisher.pusher.PushPublisherConfig; +import com.dotcms.publishing.manifest.CSVManifestBuilder; +import com.dotcms.publishing.manifest.ManifestReaderFactory; +import com.dotcms.publishing.manifest.ManifestUtil; +import com.dotcms.publishing.output.BundleArchiveCleanupProcessor; +import com.dotcms.publishing.output.BundleArchiveStorage; +import com.dotcms.publishing.output.TarGzipBundleOutput; +import com.dotcms.storage.AmazonS3StoragePersistenceAPIImpl; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.HibernateUtil; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.UUID; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +public class PublishingArchiveStorageTest { + @BeforeAll + static void initialize() throws Exception { + IntegrationTestInitService.getInstance().init(); + assertEquals(Boolean.getBoolean("s3.cms.enabled"), AssetStorageFeature.isEnabled()); + } + + @Test + void publicBundleGenerationRestoresManifestAndPayloadAndCleansAfterCommit(@TempDir Path temporary) throws Exception { + final var folder = new FolderDataGen().nextPersisted(); + final var bundleAPI = APILocator.getBundleAPI(); + final var user = APILocator.systemUser(); + final var archives = BundleArchiveStorage.getInstance(); + final Bundle bundle = new Bundle("S3 archive acceptance", null, null, user.getUserId()); + bundle.setId("Mix-" + UUID.randomUUID().toString().replace("-", "")); + bundle.setOperation(PublisherConfig.Operation.PUBLISH.ordinal()); + final String filterKey = "archive-" + bundle.getId() + ".yml"; + APILocator.getPublisherAPI().addFilterDescriptor(new FilterDescriptor(filterKey, + "Archive acceptance", java.util.Map.of("dependencies", true, "relationships", true), false, "")); + bundle.setFilterKey(filterKey); + bundleAPI.saveBundle(bundle); + try { + final byte[] payload = "Mixed-case publishing payload from S3".getBytes(java.nio.charset.StandardCharsets.UTF_8); + final File upload = Files.write(temporary.resolve("ArchivePayload.TXT"), payload).toFile(); + final var content = new FileAssetDataGen(folder, upload).nextPersisted(); + APILocator.getContentletAPI().publish(content, user, false); + com.dotcms.publisher.business.PublisherAPI.getInstance().saveBundleAssets( + List.of(content.getIdentifier()), bundle.getId(), user); + if (AssetStorageFeature.isEnabled()) { + assertTrue(APILocator.getBinaryAssetStorageAPI().evictLocalFile(content.getBinary(FileAssetAPI.BINARY_FIELD))); + } + final File archive = bundleAPI.generateTarGzipBundleFile(bundle); + final byte[] complete = Files.readAllBytes(archive.toPath()); + assertTrue(complete.length > payload.length); + assertPayload(archive, payload); + + if (AssetStorageFeature.isEnabled()) { + assertRemoteCopyAndClearLocal(bundle.getId(), archive, complete); + } + assertTrue(bundle.bundleTgzExists()); + if (AssetStorageFeature.isEnabled()) { + assertFalse(archive.exists(), "Listing a bundle must not download its archive"); + } + assertTrue(ManifestUtil.manifestExists(bundle.getId())); + final var manifest = ManifestReaderFactory.INSTANCE.createCSVManifestReader(bundle.getId()); + assertEquals("PUBLISH", manifest.getMetadata(CSVManifestBuilder.OPERATION_METADATA_NAME)); + assertTrue(manifest.getIncludedAssets().stream().anyMatch(asset -> asset.id().equals(content.getIdentifier()))); + assertArrayEquals(complete, Files.readAllBytes(TarGzipBundleOutput.getBundleTarGzipFile(bundle.getId()).toPath())); + + if (AssetStorageFeature.isEnabled()) { + final var config = new PublisherConfig(); + config.setId(bundle.getId()); + assertThrows(IOException.class, () -> { + try (var output = new TarGzipBundleOutput(config)) { + output.create(); + try (var part = output.addFile("incomplete.txt")) { part.write(payload); } + throw new IOException("Bundler failed before completion"); + } + }); + assertArrayEquals(complete, Files.readAllBytes(archive.toPath())); + assertRemoteCopyAndClearLocal(bundle.getId(), archive, complete); + try (var manifestInput = ManifestUtil.getManifestInputStream(archive).orElseThrow()) { + assertTrue(org.apache.commons.io.IOUtils.toString(manifestInput).contains("PUBLISH")); + } + assertPayload(archive, payload); + + HibernateUtil.startTransaction(); + try { + bundleAPI.deleteBundleAndDependencies(bundle.getId(), user); + assertFalse(cleanupJobs(bundle.getId()).isEmpty()); + assertArrayEquals(complete, Files.readAllBytes(archives.get(bundle.getId()).toPath())); + } finally { + HibernateUtil.rollbackTransaction(); + } + assertNotNull(bundleAPI.getBundleById(bundle.getId())); + assertTrue(cleanupJobs(bundle.getId()).isEmpty()); + bundleAPI.deleteBundleAndDependencies(bundle.getId(), user); + final var jobs = cleanupJobs(bundle.getId()); + assertFalse(jobs.isEmpty()); + final var job = APILocator.getJobQueueManagerAPI().getJob(jobs.getFirst().get("id").toString()); + final var failedProvider = mock(StoragePersistenceAPI.class); + when(failedProvider.deleteObjectAndReferences(anyString(), anyString())).thenThrow(new DotDataException("S3 unavailable")); + final var failedStorage = new BundleArchiveStorage(failedProvider); + try (var storage = mockStatic(BundleArchiveStorage.class, CALLS_REAL_METHODS)) { + storage.when(BundleArchiveStorage::getInstance).thenReturn(failedStorage); + assertThrows(com.dotcms.jobs.business.error.JobProcessingException.class, + () -> new BundleArchiveCleanupProcessor().process(job)); + } + assertArrayEquals(complete, Files.readAllBytes(archive.toPath())); + new BundleArchiveCleanupProcessor().process(job); + new BundleArchiveCleanupProcessor().process(job); + assertFalse(archives.get(bundle.getId()).exists()); + assertFalse(ManifestUtil.manifestExists(bundle.getId())); + } else { + assertTrue(cleanupJobs(bundle.getId()).isEmpty()); + bundleAPI.deleteBundleAndDependencies(bundle.getId(), user); + assertTrue(cleanupJobs(bundle.getId()).isEmpty()); + assertArrayEquals(complete, Files.readAllBytes(archive.toPath()), "Disabled deletion retains main's archive behavior"); + } + } finally { + if (bundleAPI.getBundleById(bundle.getId()) != null) { + bundleAPI.deleteBundleAndDependencies(bundle.getId(), user); + } + if (AssetStorageFeature.isEnabled()) archives.delete(bundle.getId()); + else Files.deleteIfExists(BundleArchiveStorage.localArchive(bundle.getId()).toPath()); + FolderDataGen.remove(folder); + } + } + + private static List> cleanupJobs(String id) throws DotDataException { + return new DotConnect().setSQL("select id from job where queue_name = ? and parameters ->> 'bundleId' = ?") + .addParam(BundleArchiveCleanupProcessor.QUEUE).addParam(id).loadObjectResults(); + } + + private static void assertRemoteCopyAndClearLocal(String id, File local, byte[] expected) throws Exception { + final var remote = AmazonS3StoragePersistenceAPIImpl.withPlainPaths(); + final File copy = remote.pullFile(BundleArchiveStorage.GROUP, id + ".tar.gz"); + assertNotNull(copy); + try { + assertArrayEquals(expected, Files.readAllBytes(copy.toPath())); + } finally { + remote.releaseRetrievedFile(copy); + } + Files.delete(local.toPath()); + } + + private static void assertPayload(File archive, byte[] expected) throws IOException { + boolean found = false; + try (var input = new TarArchiveInputStream(new GzipCompressorInputStream(Files.newInputStream(archive.toPath())))) { + org.apache.commons.compress.archivers.tar.TarArchiveEntry entry; + while ((entry = input.getNextEntry()) != null) { + if (entry.isFile() && entry.getName().endsWith("/ArchivePayload.TXT")) { + assertArrayEquals(expected, input.readAllBytes()); + found = true; + } + } + } + assertTrue(found, "The real content bundler must include the cold binary payload"); + } +} From 1c9e448335451f18c33c95bb2d3f681d1460a410 Mon Sep 17 00:00:00 2001 From: Scott Wicken <1562170+swicken@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:08:16 -0400 Subject: [PATCH 2/3] refactor(storage): take the publishing bundle remote store from the storage provider --- .../com/dotcms/publishing/output/BundleArchiveStorage.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java index 2992e00f860d..9de91b430b8f 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java @@ -1,6 +1,5 @@ package com.dotcms.publishing.output; -import com.dotcms.storage.AmazonS3StoragePersistenceAPIImpl; import com.dotcms.storage.AssetStorageFeature; import com.dotcms.storage.ChainableStoragePersistenceAPIBuilder; import com.dotcms.storage.FileSystemStoragePersistenceAPIImpl; @@ -59,7 +58,7 @@ private synchronized StoragePersistenceAPI storage() throws DotDataException { if (storage == null) { storage = new ChainableStoragePersistenceAPIBuilder() .add(StoragePersistenceProvider.INSTANCE.get().getStorage(StorageType.FILE_SYSTEM)) - .add(AmazonS3StoragePersistenceAPIImpl.withPlainPaths()).get(); + .add(StoragePersistenceProvider.remoteObjectStorage()).get(); } if (!initialized) { final File root = new File(ConfigUtils.getBundlePath()); From 4f3af4b2bcacb8136c80d67a150409b55e12b393 Mon Sep 17 00:00:00 2001 From: Scott Wicken <1562170+swicken@users.noreply.github.com> Date: Fri, 2 Oct 2026 10:55:22 -0400 Subject: [PATCH 3/3] fix(storage): bound durable bundle retention and harden flag-on bundle paths These fixes apply only with FEATURE_FLAG_S3_ASSET_STORAGE on; flag-off bundle paths are unchanged. Durable bundle archives now age out. BinaryCleanupJob.cleanUpOldBundles calls a new expireDurableBundles method, which asks BundleArchiveStorage.expireOlderThan to delete S3 archives whose last-modified time is older than CLEANUP_BUNDLES_OLDER_THAN_DAYS, the same threshold the local bundle directory cleanup uses. Previously the S3 copies were only removed when a user deleted bundle history. The bundle pages no longer depend on S3 being reachable. Bundle.bundleTgzExists and the retry-button check in BundlerUtil use a new BundleArchiveStorage.existsForDisplay, which logs a storage failure and reports the archive as missing, while exists stays strict for publishing and retry decisions. view_unpushed_bundles.jsp and edit_publish_bundle.jsp check once per bundle instead of up to three times. Static publishing fails the bundle with an IOException when a File Asset's binary is missing, matching the flag-off failure, instead of skipping the file and reporting success. The cache lease is held across finding and copying the binary, and the log text with the wrong wording is gone. BundleArchiveStorage.receive derives the bundle id from the part of the file name before the first .tar.gz, the rule the receiving endpoints and BundlePublisher use, so names such as release.tar.gz-v2.tar.gz or x.tar.gzip are stored where they are read. The bundleArchiveCleanup job treats a bundle row that exists again as a successful no-op instead of a retryable failure, and holds a per-bundle lock, also held by BundleArchiveStorage.store, across its row check and delete. The download cleanup in RemotePublishAjaxAction now also catches unchecked storage exceptions. Adds BundleArchiveStorageTest and FileAssetBundlerTest, and documents retention, the page check, the id rule and the remaining cleanup race in BINARY_S3_STORAGE.md. --- docs/testing/BINARY_S3_STORAGE.md | 31 ++- .../publishing/bundlers/FileAssetBundler.java | 51 +++-- .../ajax/RemotePublishAjaxAction.java | 2 +- .../dotcms/publisher/bundle/bean/Bundle.java | 6 +- .../com/dotcms/publishing/BundlerUtil.java | 10 +- .../output/BundleArchiveCleanupProcessor.java | 23 ++- .../output/BundleArchiveStorage.java | 104 +++++++++- .../quartz/job/BinaryCleanupJob.java | 26 ++- .../publishing/edit_publish_bundle.jsp | 5 +- .../publishing/view_unpushed_bundles.jsp | 9 +- .../bundlers/FileAssetBundlerTest.java | 124 ++++++++++++ .../output/BundleArchiveStorageTest.java | 177 ++++++++++++++++++ 12 files changed, 531 insertions(+), 37 deletions(-) create mode 100644 dotCMS/src/test/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundlerTest.java create mode 100644 dotCMS/src/test/java/com/dotcms/publishing/output/BundleArchiveStorageTest.java diff --git a/docs/testing/BINARY_S3_STORAGE.md b/docs/testing/BINARY_S3_STORAGE.md index 47e4eca0d5f8..719edc04c44b 100644 --- a/docs/testing/BINARY_S3_STORAGE.md +++ b/docs/testing/BINARY_S3_STORAGE.md @@ -296,12 +296,35 @@ manifest) are stored durably in the `publishing-bundles` group, with the local b a cache (`BundleArchiveStorage`). Bundle ids keep their case and are checked so an archive cannot resolve outside the bundle directory. Generated and received bundles are written to a private staging file and published to S3 before they replace the last complete local archive, so an -incomplete upload never becomes visible. Reading a bundle's manifest or payload restores the -archive from S3 only when its bytes are needed. Storage failures are reported as failures, not as a -missing bundle. +incomplete upload never becomes visible. A received file's bundle id is the part of its name +before the first `.tar.gz`, which is the rule the receiving endpoints and `BundlePublisher` use to +find the archive again. Reading a bundle's manifest or payload restores the archive from S3 only +when its bytes are needed. Storage failures are reported as failures, not as a missing bundle, for +every publishing and retry decision. The bundle pages are the exception: whether to show a +download link or a retry button is checked once per bundle, and a storage failure there is logged +as a warning and shown as "not generated", so an S3 outage does not stop the page from rendering. + +With the flag on, static publishing fails the bundle when a File Asset's binary is missing, so it +can be retried. This matches the flag-off behavior, where the copy of the missing file fails. Deleting a bundle records a `bundleArchiveCleanup` job after the deletion commits. The job deletes the S3 object and keeps the local bytes if remote deletion fails, so the existing queue can retry. +If a bundle row with the same id exists again when the job runs, for example because a receiver got +the same bundle again, the job keeps the archive and finishes successfully. On one node, storing an +archive and the job's row check and delete hold the same lock, so the job cannot delete an archive +stored between its check and its delete. That lock does not cover other cluster nodes, or a +receiver whose new bundle row is not yet committed when its archive is stored. In those cases the +new archive can still be deleted, and the receive fails with an error and can be retried. + +Durable archives are kept for the same time as local ones. When `BinaryCleanupJob` deletes files +older than `CLEANUP_BUNDLES_OLDER_THAN_DAYS` (default 4) from the bundle directory, with the flag on +it also deletes S3 archives whose S3 last-modified time is older than that, together with their +local copy and extraction directory. A bundle older than that can no longer be retried or +downloaded, as with the flag off. A value below 1 turns off both cleanups. An archive that cannot be +deleted is logged and tried again on the next run. If several nodes run the cleanup job, each one +lists the group and deletes the same expired archives, which is harmless because deleting an archive +that is already gone succeeds. + With the flag off, bundles are written, read and deleted in the bundle directory as before. ## Local cache eviction @@ -402,7 +425,7 @@ docker run -d --rm --name binary-cleanup-postgres-test \ -e POSTGRES_DB=binary_storage_test postgres:16-alpine ./mvnw test -pl :dotcms-core -Dmaven.build.cache.enabled=false \ - -Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest,BinaryAssetReferenceTest,BinaryCacheEvictionJobTest,BinaryFileSystemStorageTest,BinaryAssetStorageAPIImplTest,MetadataLocalCacheTest,BinaryAssetCleanupTransactionTest,BinaryAssetCleanupProcessorTest,ContentletBackupStorageGateTest,BinaryFieldCleanupProcessorTest,AssetJobEventSerializationTest,BinaryAssetBackfillCheckpointTest,ImportStarterWorkflowCleanupTest,BinaryAssetBackfillProcessorTest,BinaryAssetBackfillTest,ExportStarterFailureTest \ + -Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest,BinaryAssetReferenceTest,BinaryCacheEvictionJobTest,BinaryFileSystemStorageTest,BinaryAssetStorageAPIImplTest,MetadataLocalCacheTest,BinaryAssetCleanupTransactionTest,BinaryAssetCleanupProcessorTest,ContentletBackupStorageGateTest,BinaryFieldCleanupProcessorTest,AssetJobEventSerializationTest,BinaryAssetBackfillCheckpointTest,ImportStarterWorkflowCleanupTest,BinaryAssetBackfillProcessorTest,BinaryAssetBackfillTest,ExportStarterFailureTest,BundleArchiveStorageTest,FileAssetBundlerTest \ -Ds3.test.endpoint=http://127.0.0.1:19002 \ -Ds3.test.jdbc=jdbc:postgresql://127.0.0.1:19003/binary_storage_test diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java index c072db894934..4c192dc66be7 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundler.java @@ -24,6 +24,7 @@ import com.dotcms.publishing.PublisherConfig; import com.dotcms.publishing.PublisherConfig.Operation; import com.dotcms.publishing.output.BundleOutput; +import com.dotcms.storage.binary.BinaryAssetStorageAPI; import com.dotmarketing.beans.Host; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.UserAPI; @@ -323,6 +324,21 @@ private void writeFileAsset(final BundleOutput output, final FileAsset fileAsset } } + /** + * Writes one File Asset into the bundle for one language: its XML descriptor (except for static + * publishing) and a copy of its binary file, or removes them when an incremental live-only bundle + * no longer includes the asset. Files already in the bundle with the same modification date are + * left alone. + * + * @param host the {@link Host} the File Asset lives in + * @param languageId the language folder to write the asset under + * @param output the bundle being written + * @param fileAssetWrapper the File Asset with its version info and identifier + * @throws IOException if the asset's binary file is missing or cannot be copied, which + * fails the bundle so it can be retried + * @throws DotDataException if the binary file cannot be looked up + * @throws DotSecurityException if the asset cannot be read + */ private void writeFileToDisk(Host host, String languageId, BundleOutput output, FileAssetWrapper fileAssetWrapper) throws IOException, DotDataException, DotSecurityException { @@ -371,22 +387,27 @@ private void writeFileToDisk(Host host, String languageId, BundleOutput output, else { //only write if changed if(!output.exists(filePath) || output.lastModified(filePath) != cal.getTimeInMillis()){ - final File oldAsset = com.dotcms.storage.AssetStorageFeature.isEnabled() - ? APILocator.getBinaryAssetStorageAPI().getBinaryFile(fileAssetWrapper.getAsset().getInode(), FileAssetAPI.BINARY_FIELD) - : new File(APILocator.getFileAssetAPI().getRealAssetPathIgnoreExtensionCase(fileAssetWrapper.getAsset().getInode(), fileAssetWrapper.getAsset().getUnderlyingFileName())); - if (oldAsset == null) { - Logger.warn(this, String.format( - "Binary file not found for inode '%s' (underlying name: '%s') — skipping bundle", - fileAssetWrapper.getAsset().getInode(), - fileAssetWrapper.getAsset().getUnderlyingFileName())); - return; - } - if(output.exists(filePath)) { - output.delete(filePath); - } + // With S3 asset storage on, the cache lease keeps the binary from being evicted + // between finding it and copying it. A null resource is allowed and never closed. + try (BinaryAssetStorageAPI.CacheLease lease = com.dotcms.storage.AssetStorageFeature.isEnabled() + ? APILocator.getBinaryAssetStorageAPI().acquireCacheLease() : null) { + final File oldAsset = com.dotcms.storage.AssetStorageFeature.isEnabled() + ? APILocator.getBinaryAssetStorageAPI().getBinaryFile(fileAssetWrapper.getAsset().getInode(), FileAssetAPI.BINARY_FIELD) + : new File(APILocator.getFileAssetAPI().getRealAssetPathIgnoreExtensionCase(fileAssetWrapper.getAsset().getInode(), fileAssetWrapper.getAsset().getUnderlyingFileName())); + if (oldAsset == null) { + // Fail the bundle, as a missing source file does with the flag off, so it can be retried. + throw new IOException(String.format( + "Binary file not found for File Asset inode '%s' (underlying name: '%s')", + fileAssetWrapper.getAsset().getInode(), + fileAssetWrapper.getAsset().getUnderlyingFileName())); + } + if(output.exists(filePath)) { + output.delete(filePath); + } - FileUtil.copyFile(oldAsset, output.getFile(filePath), true); - output.setLastModified(filePath, cal.getTimeInMillis()); + FileUtil.copyFile(oldAsset, output.getFile(filePath), true); + output.setLastModified(filePath, cal.getTimeInMillis()); + } } } } diff --git a/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java b/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java index 6815ab16a466..ab1e902776e3 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/ajax/RemotePublishAjaxAction.java @@ -639,7 +639,7 @@ public void downloadUnpushedBundle ( HttpServletRequest request, HttpServletResp if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { try { com.dotcms.publishing.output.BundleArchiveStorage.getInstance().delete(bundleId); - } catch (DotDataException e) { + } catch (DotDataException | RuntimeException e) { Logger.error(this, "Unable to remove generated download bundle " + bundleId, e); } } diff --git a/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java b/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java index 4db4ef745b11..fd628ead910e 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/bundle/bean/Bundle.java @@ -88,12 +88,14 @@ public void setFilterKey(final String filterKey) { } /** - * Checks if the bundle was already generated based on the id: BUNDLE_ID.tar.gz + * Checks if the bundle was already generated based on the id: BUNDLE_ID.tar.gz. The bundle + * pages use this to decide whether to show download links. With S3 asset storage on, a storage + * failure is logged and reported as false so the page still renders. * @return boolean - true if the bundle exists. */ public boolean bundleTgzExists() { if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { - return com.dotcms.publishing.output.BundleArchiveStorage.getInstance().exists(id); + return com.dotcms.publishing.output.BundleArchiveStorage.getInstance().existsForDisplay(id); } return Try.of(()->new File( ConfigUtils.getBundlePath() + File.separator + id + ".tar.gz" ).exists()).getOrElse(false); diff --git a/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java b/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java index 80c0d93b566d..dccbc2000709 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java @@ -136,6 +136,14 @@ public static boolean isRetryable(final String bundleId) { } } + /** + * Tells {@link #isRetryable(String)} whether the bundle's files are present. With S3 asset + * storage on, the archive check is the display check, which reports a storage failure as + * false instead of throwing, because the audit detail page calls this while rendering. + * + * @param bundleId the bundle id + * @return true if the bundle's files are present + */ private static boolean bundleExists(String bundleId) { final PublisherConfig basicConfig = new PublisherConfig(); basicConfig.setId(bundleId); @@ -143,7 +151,7 @@ private static boolean bundleExists(String bundleId) { final File bundleStaticFile = new File(bundleRoot.getAbsolutePath() + PublisherConfig.STATIC_SUFFIX); if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { - return bundleStaticFile.exists() || com.dotcms.publishing.output.BundleArchiveStorage.getInstance().exists(bundleId); + return bundleStaticFile.exists() || com.dotcms.publishing.output.BundleArchiveStorage.getInstance().existsForDisplay(bundleId); } if ( !bundleStaticFile.exists() ) { return true; diff --git a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java index f0ffd49c14c3..ee439b6522af 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java @@ -11,7 +11,9 @@ import com.dotmarketing.common.db.DotConnect; import com.dotmarketing.db.DbConnectionFactory; import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.util.Logger; import java.util.Map; +import java.util.concurrent.locks.Lock; import javax.enterprise.context.Dependent; /** The existing transactional job queue retains archive deletion intent across storage failures. */ @@ -30,6 +32,17 @@ public static void enqueue(final String id) throws DotDataException { APILocator.getJobQueueManagerAPI().createJob(QUEUE, Map.of("bundleId", id)); } + /** + * Deletes the archive of a deleted bundle. If a bundle row with the same id exists again, for + * example because a receiver got the same bundle again after its history was deleted, the + * archive belongs to that bundle and the job finishes without deleting anything. The row check + * and the delete run under {@link BundleArchiveStorage#archiveLock(String)}, so an archive + * stored on this node cannot be deleted between them. + * + * @param job the cleanup job, whose {@code bundleId} parameter names the bundle + * @throws JobProcessingException if the flag is off or the archive cannot be deleted, so the + * queue retries the job + */ @Override @CloseDBIfOpened public void process(final Job job) throws JobProcessingException { @@ -37,14 +50,20 @@ public void process(final Job job) throws JobProcessingException { throw new JobProcessingException(job.id(), "S3 asset storage is disabled"); } final String id = (String) job.parameters().get("bundleId"); + final BundleArchiveStorage archives = BundleArchiveStorage.getInstance(); + final Lock lock = archives.archiveLock(id); + lock.lock(); try { if (!new DotConnect().setSQL("select id from publishing_bundle where id = ?") .addParam(id).loadObjectResults().isEmpty()) { - throw new JobProcessingException(job.id(), "Bundle still exists: " + id); + Logger.info(this, "Bundle " + id + " exists again, keeping its archive"); + return; } - BundleArchiveStorage.getInstance().delete(id); + archives.delete(id); } catch (DotDataException | RuntimeException e) { throw new JobProcessingException(job.id(), "Unable to remove bundle archive " + id, e); + } finally { + lock.unlock(); } } diff --git a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java index 9de91b430b8f..6b23d79b1c19 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java @@ -3,24 +3,33 @@ import com.dotcms.storage.AssetStorageFeature; import com.dotcms.storage.ChainableStoragePersistenceAPIBuilder; import com.dotcms.storage.FileSystemStoragePersistenceAPIImpl; +import com.dotcms.storage.ObjectSnapshot; import com.dotcms.storage.StoragePersistenceAPI; import com.dotcms.storage.StoragePersistenceProvider; import com.dotcms.storage.StorageType; import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.util.ConfigUtils; +import com.dotmarketing.util.Logger; +import com.google.common.util.concurrent.Striped; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Instant; import java.util.Map; +import java.util.concurrent.locks.Lock; /** Completed publishing archives, including their manifest, share one durable object and local cache. */ public final class BundleArchiveStorage { public static final String GROUP = "publishing-bundles"; + private static final String SUFFIX = ".tar.gz"; private StoragePersistenceAPI storage; + /** The durable provider, when this class built the chain itself; used to list archives by age. */ + private StoragePersistenceAPI remote; private boolean initialized; + private final Striped archiveLocks = Striped.lock(64); private static class Holder { private static final BundleArchiveStorage INSTANCE = new BundleArchiveStorage(null); @@ -56,9 +65,10 @@ public static File localArchive(final String id) { private synchronized StoragePersistenceAPI storage() throws DotDataException { if (storage == null) { + remote = StoragePersistenceProvider.remoteObjectStorage(); storage = new ChainableStoragePersistenceAPIBuilder() .add(StoragePersistenceProvider.INSTANCE.get().getStorage(StorageType.FILE_SYSTEM)) - .add(StoragePersistenceProvider.remoteObjectStorage()).get(); + .add(remote).get(); } if (!initialized) { final File root = new File(ConfigUtils.getBundlePath()); @@ -82,6 +92,38 @@ public boolean exists(final String id) { } } + /** + * Tells a page whether to show a bundle's archive as available, for example to offer a download + * link or a retry button. Unlike {@link #exists(String)}, a storage failure or an invalid id is + * logged and reported as {@code false}, so an S3 outage cannot stop a bundle listing from + * rendering. Publishing and retry decisions must keep using {@link #exists(String)}. + * + * @param id the bundle id + * @return true if the archive is known to exist, false if it is missing or could not be checked + */ + public boolean existsForDisplay(final String id) { + try { + return exists(id); + } catch (RuntimeException e) { + Logger.warn(BundleArchiveStorage.class, + "Unable to check publishing archive " + id + ", showing it as missing: " + e.getMessage()); + return false; + } + } + + /** + * Returns the lock that orders storing an archive against deleting it. Storing a bundle's archive + * holds it, and the cleanup job holds it while it checks that the bundle row is gone and deletes + * the archive, so on one node a bundle received again cannot lose its new archive between that + * check and the delete. The lock is local to this server. + * + * @param id the bundle id + * @return the lock for that bundle id + */ + public Lock archiveLock(final String id) { + return archiveLocks.get(id); + } + /** Restores only when the archive bytes are required. */ public File get(final String id) { final File file = localArchive(id); @@ -94,21 +136,44 @@ public File get(final String id) { } } - /** The caller supplies a closed, complete archive. The chain publishes remotely before replacing local bytes. */ + /** + * Publishes a closed, complete archive as the bundle's archive. The chain stores it in S3 before + * it replaces the local copy. Holds {@link #archiveLock(String)} while storing, so the cleanup + * job cannot delete it between checking for the bundle row and deleting. + * + * @param id the bundle id + * @param source the complete archive to publish; the caller keeps ownership of this file + * @throws IOException if the archive cannot be stored + */ public void store(final String id, final File source) throws IOException { if (!AssetStorageFeature.isEnabled()) return; final File archive = localArchive(id); + final Lock lock = archiveLock(id); + lock.lock(); try { storage().pushFile(GROUP, archive.getName(), source, Map.of()); } catch (DotDataException e) { throw new IOException("Unable to store publishing archive " + id, e); + } finally { + lock.unlock(); } } - /** Own staging file keeps incomplete uploads away from the last complete archive. Caller owns the input stream. */ + /** + * Saves an uploaded bundle archive. The upload is copied to a private staging file first, so an + * incomplete upload never replaces the last complete archive. The bundle id is the part of the + * file name before the first {@code .tar.gz}, the same rule the receiving endpoints and + * {@code BundlePublisher} use to find the archive again, so any name they accept is stored + * where they will read it. + * + * @param fileName the uploaded file name, which must contain {@code .tar.gz} + * @param input the upload; the caller owns and closes it + * @throws IOException if the upload cannot be read or stored + */ public void receive(final String fileName, final InputStream input) throws IOException { - if (!fileName.endsWith(".tar.gz")) throw new IllegalArgumentException("Expected a bundle archive"); - final String id = fileName.substring(0, fileName.length() - ".tar.gz".length()); + final int suffix = fileName.indexOf(SUFFIX); + if (suffix < 0) throw new IllegalArgumentException("Expected a bundle archive"); + final String id = fileName.substring(0, suffix); final Path target = localArchive(id).toPath(); if (!AssetStorageFeature.isEnabled()) { com.dotmarketing.util.FileUtil.writeToFile(input, target.toString()); @@ -143,4 +208,33 @@ public void delete(final String id) throws DotDataException { } } + /** + * Deletes durable archives last written before the cutoff, together with their local copy and + * extraction directory. This is the S3 counterpart of the age-based bundle directory cleanup in + * {@code BinaryCleanupJob}, so archives in S3 are kept for the same number of days as local + * ones. An archive that cannot be deleted is logged and left for the next run. + * + * @param cutoff archives whose stored last-modified time is strictly before this are deleted + * @return the number of archives deleted + * @throws DotDataException if the stored archives cannot be listed + */ + public int expireOlderThan(final Instant cutoff) throws DotDataException { + if (!AssetStorageFeature.isEnabled()) return 0; + final StoragePersistenceAPI chain = storage(); + int deleted = 0; + for (final ObjectSnapshot object : (remote == null ? chain : remote).listObjectSnapshots(GROUP, "")) { + final String name = object.path(); + if (object.modified() >= cutoff.toEpochMilli() || !name.endsWith(SUFFIX)) continue; + final String id = name.substring(0, name.length() - SUFFIX.length()); + try { + delete(id); + deleted++; + } catch (DotDataException | RuntimeException e) { + Logger.warn(BundleArchiveStorage.class, "Unable to expire publishing archive " + name + + ", it will be retried on the next cleanup: " + e.getMessage()); + } + } + return deleted; + } + } diff --git a/dotCMS/src/main/java/com/dotmarketing/quartz/job/BinaryCleanupJob.java b/dotCMS/src/main/java/com/dotmarketing/quartz/job/BinaryCleanupJob.java index 107264bcfe6e..5fa5b6f16433 100644 --- a/dotCMS/src/main/java/com/dotmarketing/quartz/job/BinaryCleanupJob.java +++ b/dotCMS/src/main/java/com/dotmarketing/quartz/job/BinaryCleanupJob.java @@ -1,5 +1,8 @@ package com.dotmarketing.quartz.job; +import com.dotcms.publishing.output.BundleArchiveStorage; +import com.dotcms.storage.AssetStorageFeature; +import com.dotmarketing.exception.DotDataException; import com.dotmarketing.util.Config; import com.dotmarketing.util.ConfigUtils; import com.dotmarketing.util.Logger; @@ -87,7 +90,8 @@ void cleanUpTmpUploadedFiles() { } /** - * Deletes from /assets/bundles + * Deletes from /assets/bundles, and with S3 asset storage on also expires the durable copies of + * bundle archives older than the same number of days. */ void cleanUpOldBundles() { final int days = Config.getIntProperty(CLEANUP_BUNDLES_OLDER_THAN_DAYS, 4); @@ -98,6 +102,26 @@ void cleanUpOldBundles() { Logger.info(this.getClass(), String.format(FILE_DELETION_STATUS_MSG, days, DAYS, ConfigUtils.getBundlePath())); final File bundleFolder = new File(ConfigUtils.getBundlePath()); FileUtil.cleanTree(bundleFolder, olderThan); + expireDurableBundles(olderThan); + } + + /** + * With S3 asset storage on, deletes the S3 copies of bundle archives last written before the + * cutoff, so they are kept as long as the local ones. Does nothing with the flag off. A storage + * failure is logged and the next run tries again. + * + * @param olderThan archives stored before this time are deleted + */ + void expireDurableBundles(final Date olderThan) { + if (!AssetStorageFeature.isEnabled()) { + return; + } + try { + final int expired = BundleArchiveStorage.getInstance().expireOlderThan(olderThan.toInstant()); + Logger.info(this.getClass(), String.format("Expired %d stored bundle archives older than %s", expired, olderThan)); + } catch (final DotDataException | RuntimeException e) { + Logger.warn(this.getClass(), "Unable to expire stored bundle archives: " + e.getMessage(), e); + } } /** diff --git a/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/edit_publish_bundle.jsp b/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/edit_publish_bundle.jsp index 7cc3f0ce56f9..89f9200ae9c4 100644 --- a/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/edit_publish_bundle.jsp +++ b/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/edit_publish_bundle.jsp @@ -97,10 +97,11 @@ <%= LanguageUtil.get(pageContext, "id") %>: -
<%if(bundle.bundleTgzExists()){%><%} %> + <% final boolean bundleTgzExists = bundle.bundleTgzExists(); %> +
- <%if(bundle.bundleTgzExists()){%> + <%if(bundleTgzExists){%> (<%= LanguageUtil.get(pageContext, "Download") %>) diff --git a/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/view_unpushed_bundles.jsp b/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/view_unpushed_bundles.jsp index d3c5dd967ed5..60cdb9e08033 100644 --- a/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/view_unpushed_bundles.jsp +++ b/dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/view_unpushed_bundles.jsp @@ -75,7 +75,8 @@ User bundleOwner = APILocator.getUserAPI().loadUserById(bundle.getOwner()); PublisherAPI publisherAPI = PublisherAPI.getInstance(); - List assets = publisherAPI.getQueueElementsByBundleId(bundle.getId());%> + List assets = publisherAPI.getQueueElementsByBundleId(bundle.getId()); + final boolean bundleTgzExists = bundle.bundleTgzExists();%> @@ -83,7 +84,7 @@ <%=StringEscapeUtils.unescapeJava(bundle.getName())%> ( <%=bundle.getId() %> ) - <%if(bundle.bundleTgzExists()){%> + <%if(bundleTgzExists){%> - <%=LanguageUtil.get(pageContext, "Already Generated") %> / Filter: <%if(bundle.getOperation()==null || bundle.getOperation()==0){%> <%=(bundle.getFilterKey()!=null) ?bundle.getFilterKey().replace(".yml", "") :""%> @@ -114,14 +115,14 @@
<%= LanguageUtil.get(pageContext, "Delete") %>
- <%if(bundle.bundleTgzExists()){%> + <%if(bundleTgzExists){%>
<%= LanguageUtil.get(pageContext, "Download") %>
<%} %>
- <%if(bundle.bundleTgzExists()){%> + <%if(bundleTgzExists){%> <%=LanguageUtil.get(pageContext, "Regenerate") %> / <%= LanguageUtil.get(pageContext, "Download") %> <%}else{ %> <%=LanguageUtil.get(pageContext, "Generate") %> / <%= LanguageUtil.get(pageContext, "Download") %> diff --git a/dotCMS/src/test/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundlerTest.java b/dotCMS/src/test/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundlerTest.java new file mode 100644 index 000000000000..e5db6a53b218 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/enterprise/publishing/bundlers/FileAssetBundlerTest.java @@ -0,0 +1,124 @@ +package com.dotcms.enterprise.publishing.bundlers; + +import com.dotcms.publishing.PublisherConfig; +import com.dotcms.publishing.output.BundleOutput; +import com.dotcms.publishing.output.DirectoryBundleOutput; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.binary.BinaryAssetStorageAPI; +import com.dotmarketing.beans.Host; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo; +import com.dotmarketing.portlets.fileassets.business.FileAsset; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; +import com.dotmarketing.util.Config; +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Date; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +/** + * Unit tests for how {@link FileAssetBundler} copies a File Asset's binary into a static bundle + * when S3 asset storage is on. + */ +class FileAssetBundlerTest { + + private static final String INODE = "abc123"; + + @TempDir Path root; + private String previousFlag; + private BinaryAssetStorageAPI binaries; + private BinaryAssetStorageAPI.CacheLease lease; + private PublisherConfig config; + private BundleOutput output; + private File copied; + + @BeforeEach + void enable() { + previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + Config.setProperty(AssetStorageFeature.FLAG, true); + binaries = mock(BinaryAssetStorageAPI.class); + lease = mock(BinaryAssetStorageAPI.CacheLease.class); + when(binaries.acquireCacheLease()).thenReturn(lease); + config = mock(PublisherConfig.class); + when(config.isStatic()).thenReturn(true); + when(config.liveOnly()).thenReturn(false); + output = new DirectoryBundleOutput(config, root.resolve("bundle").toFile()); + copied = root.resolve("bundle/live/demo.dotcms.com/1/images/logo.png").toFile(); + } + + @AfterEach + void restore() { + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + } + + /** A missing binary fails the bundle, as the failed copy does with the flag off, instead of skipping the file. */ + @Test + void missingBinaryFailsTheBundle() throws Exception { + when(binaries.getBinaryFile(INODE, FileAssetAPI.BINARY_FIELD)).thenReturn(null); + try (var locator = mockStatic(APILocator.class)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(binaries); + final IOException failure = assertThrows(IOException.class, this::writeFileToDisk); + assertTrue(failure.getMessage().contains(INODE)); + } + assertFalse(copied.exists()); + verify(lease).close(); + } + + /** A present binary is copied into the bundle before the cache lease is released. */ + @Test + void presentBinaryIsCopiedWhileTheLeaseIsHeld() throws Exception { + final File source = Files.writeString(root.resolve("logo.png"), "pixels").toFile(); + when(binaries.getBinaryFile(INODE, FileAssetAPI.BINARY_FIELD)).thenReturn(source); + final AtomicBoolean copiedBeforeRelease = new AtomicBoolean(); + doAnswer(call -> { + copiedBeforeRelease.set(copied.exists()); + return null; + }).when(lease).close(); + try (var locator = mockStatic(APILocator.class)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(binaries); + writeFileToDisk(); + } + assertEquals("pixels", Files.readString(copied.toPath())); + assertTrue(copiedBeforeRelease.get(), "The lease must be held until the copy is done"); + } + + private void writeFileToDisk() throws Exception { + final FileAssetBundler bundler = new FileAssetBundler(); + final Field configField = FileAssetBundler.class.getDeclaredField("config"); + configField.setAccessible(true); + configField.set(bundler, config); + + final Host host = mock(Host.class); + when(host.getHostname()).thenReturn("demo.dotcms.com"); + final FileAsset asset = mock(FileAsset.class); + when(asset.getInode()).thenReturn(INODE); + when(asset.getURI()).thenReturn("/images/logo.png"); + when(asset.getUnderlyingFileName()).thenReturn("logo.png"); + final ContentletVersionInfo info = mock(ContentletVersionInfo.class); + when(info.getLiveInode()).thenReturn(INODE); + when(info.getVersionTs()).thenReturn(new Date()); + final FileAssetWrapper wrapper = new FileAssetWrapper(); + wrapper.setAsset(asset); + wrapper.setInfo(info); + + final Method write = FileAssetBundler.class.getDeclaredMethod("writeFileToDisk", + Host.class, String.class, BundleOutput.class, FileAssetWrapper.class); + write.setAccessible(true); + try { + write.invoke(bundler, host, "1", output, wrapper); + } catch (InvocationTargetException e) { + throw (Exception) e.getCause(); + } + } +} diff --git a/dotCMS/src/test/java/com/dotcms/publishing/output/BundleArchiveStorageTest.java b/dotCMS/src/test/java/com/dotcms/publishing/output/BundleArchiveStorageTest.java new file mode 100644 index 000000000000..e0109c2e0510 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/publishing/output/BundleArchiveStorageTest.java @@ -0,0 +1,177 @@ +package com.dotcms.publishing.output; + +import com.dotcms.jobs.business.job.Job; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.ObjectSnapshot; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.util.Config; +import com.dotmarketing.util.ConfigUtils; +import com.google.common.collect.ImmutableMap; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.MockedStatic; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.*; + +/** + * Unit tests for the flag-on behavior of {@link BundleArchiveStorage} and + * {@link BundleArchiveCleanupProcessor}, using a mocked storage chain instead of S3. + */ +class BundleArchiveStorageTest { + + @TempDir Path root; + private String previousFlag; + private MockedStatic paths; + private StoragePersistenceAPI provider; + private BundleArchiveStorage archives; + + @BeforeEach + void enable() { + previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + Config.setProperty(AssetStorageFeature.FLAG, true); + paths = mockStatic(ConfigUtils.class); + paths.when(ConfigUtils::getBundlePath).thenReturn(root.toString()); + provider = mock(StoragePersistenceAPI.class); + archives = new BundleArchiveStorage(provider); + } + + @AfterEach + void restore() { + paths.close(); + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + } + + /** A received file is stored under the id the receivers derive: the name before the first ".tar.gz". */ + @Test + void receiveStoresUnderTheIdTheReadersUse() throws Exception { + archives.receive("release.tar.gz-v2.tar.gz", new ByteArrayInputStream("a".getBytes(StandardCharsets.UTF_8))); + verify(provider).pushFile(eq(BundleArchiveStorage.GROUP), eq("release.tar.gz"), any(File.class), anyMap()); + + archives.receive("x.tar.gzip", new ByteArrayInputStream("b".getBytes(StandardCharsets.UTF_8))); + verify(provider).pushFile(eq(BundleArchiveStorage.GROUP), eq("x.tar.gz"), any(File.class), anyMap()); + + assertThrows(IllegalArgumentException.class, + () -> archives.receive("bundle.zip", new ByteArrayInputStream(new byte[0]))); + } + + /** The page check reports an S3 failure as a missing archive; the publishing check still fails. */ + @Test + void displayCheckToleratesStorageFailure() throws Exception { + when(provider.existsObject(anyString(), anyString())).thenThrow(new DotDataException("S3 unavailable")); + assertThrows(RuntimeException.class, () -> archives.exists("Some-Bundle")); + assertFalse(archives.existsForDisplay("Some-Bundle")); + assertFalse(archives.existsForDisplay("../outside"), "An invalid id is shown as missing, not thrown"); + } + + /** Only archives stored strictly before the cutoff are expired, and one failure does not stop the rest. */ + @Test + void expiresOnlyArchivesOlderThanTheCutoff() throws Exception { + final long cutoff = Instant.parse("2026-09-01T00:00:00Z").toEpochMilli(); + when(provider.listObjectSnapshots(BundleArchiveStorage.GROUP, "")).thenReturn(List.of( + new ObjectSnapshot("Failing-Bundle.tar.gz", null, "e1", cutoff - 10), + new ObjectSnapshot("Old-Bundle.tar.gz", null, "e2", cutoff - 1), + new ObjectSnapshot("Exact-Bundle.tar.gz", null, "e3", cutoff), + new ObjectSnapshot("New-Bundle.tar.gz", null, "e4", cutoff + 1000), + new ObjectSnapshot("notes.txt", null, "e5", cutoff - 1), + new ObjectSnapshot("nested/Inner.tar.gz", null, "e6", cutoff - 1))); + when(provider.deleteObjectAndReferences(BundleArchiveStorage.GROUP, "Failing-Bundle.tar.gz")) + .thenThrow(new DotDataException("S3 unavailable")); + + assertEquals(1, archives.expireOlderThan(Instant.ofEpochMilli(cutoff))); + + verify(provider).deleteObjectAndReferences(BundleArchiveStorage.GROUP, "Failing-Bundle.tar.gz"); + verify(provider).deleteObjectAndReferences(BundleArchiveStorage.GROUP, "Old-Bundle.tar.gz"); + verify(provider, never()).deleteObjectAndReferences(BundleArchiveStorage.GROUP, "Exact-Bundle.tar.gz"); + verify(provider, never()).deleteObjectAndReferences(BundleArchiveStorage.GROUP, "New-Bundle.tar.gz"); + verify(provider, never()).deleteObjectAndReferences(BundleArchiveStorage.GROUP, "notes.txt"); + verify(provider, never()).deleteObjectAndReferences(BundleArchiveStorage.GROUP, "nested/Inner.tar.gz"); + } + + /** With the flag off, expiry touches no storage at all. */ + @Test + void disabledFlagExpiresNothing() throws Exception { + Config.setProperty(AssetStorageFeature.FLAG, false); + assertEquals(0, archives.expireOlderThan(Instant.now())); + verifyNoInteractions(provider); + } + + /** Storing an archive waits while the cleanup job holds the bundle's archive lock. */ + @Test + void storeWaitsForTheArchiveLock() throws Exception { + final File source = root.resolve("source.tmp").toFile(); + assertTrue(source.createNewFile()); + final CountDownLatch locked = new CountDownLatch(1); + final AtomicBoolean pushedWhileLocked = new AtomicBoolean(true); + // The holder runs on its own thread, because the static ConfigUtils mock only applies to this one. + final CompletableFuture holder = CompletableFuture.runAsync(() -> { + final Lock lock = archives.archiveLock("Locked-Bundle"); + lock.lock(); + try { + locked.countDown(); + Thread.sleep(300); + pushedWhileLocked.set(mockingDetails(provider).getInvocations().stream() + .anyMatch(call -> call.getMethod().getName().equals("pushFile"))); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + lock.unlock(); + } + }); + assertTrue(locked.await(10, TimeUnit.SECONDS)); + archives.store("Locked-Bundle", source); + holder.get(10, TimeUnit.SECONDS); + assertFalse(pushedWhileLocked.get(), "Store must not run while the lock is held"); + verify(provider).pushFile(eq(BundleArchiveStorage.GROUP), eq("Locked-Bundle.tar.gz"), eq(source), anyMap()); + } + + /** A bundle that exists again keeps its archive, and the job completes instead of failing. */ + @Test + void cleanupKeepsTheArchiveOfABundleThatExistsAgain() throws Exception { + try (var queries = mockConstruction(DotConnect.class, withSettings().defaultAnswer(RETURNS_SELF), + (query, context) -> when(query.loadObjectResults()).thenReturn(List.of(Map.of("id", "Again")))); + var instance = mockStatic(BundleArchiveStorage.class, CALLS_REAL_METHODS)) { + instance.when(BundleArchiveStorage::getInstance).thenReturn(archives); + assertDoesNotThrow(() -> new BundleArchiveCleanupProcessor().process(job("Again"))); + } + verify(provider, never()).deleteObjectAndReferences(anyString(), anyString()); + } + + /** A bundle that is gone has its archive deleted. */ + @Test + void cleanupDeletesTheArchiveOfADeletedBundle() throws Exception { + try (var queries = mockConstruction(DotConnect.class, withSettings().defaultAnswer(RETURNS_SELF), + (query, context) -> when(query.loadObjectResults()).thenReturn(List.of())); + var instance = mockStatic(BundleArchiveStorage.class, CALLS_REAL_METHODS)) { + instance.when(BundleArchiveStorage::getInstance).thenReturn(archives); + new BundleArchiveCleanupProcessor().process(job("Gone")); + } + verify(provider).deleteObjectAndReferences(BundleArchiveStorage.GROUP, "Gone.tar.gz"); + } + + private Job job(final String bundleId) { + final Job job = mock(Job.class); + when(job.id()).thenReturn("bundle-cleanup-test"); + when(job.parameters()).thenReturn(ImmutableMap.of("bundleId", bundleId)); + return job; + } +}