diff --git a/docs/testing/BINARY_S3_STORAGE.md b/docs/testing/BINARY_S3_STORAGE.md index 209c452d6bac..4718697bb8d8 100644 --- a/docs/testing/BINARY_S3_STORAGE.md +++ b/docs/testing/BINARY_S3_STORAGE.md @@ -35,6 +35,20 @@ table may not be seen at first read, and a runtime change is ignored until resta overrides through `Config.setProperty` do re-read it, which is how tests switch modes. Tests that mock `Config` statically must call `AssetStorageFeature.reset()` themselves. +With the flag off, the S3 cleanup job queues (`binaryAssetCleanup`, `binaryFieldCleanup`) are not +registered. + +### Enabling the flag is not rollback-safe + +Enabling the flag is a one-way step for any content written while it is on. Check-in stores the +active binary only under a `.revisions//` key recorded in `contentlet_as_json` +(`storageKey`/`metadataStorageKey`); no legacy flat file is written, and the local copy may later +be evicted to S3. Neither a release without this code nor this release with the flag turned back +off reads those revision keys (the reader falls back to the legacy field folder), so affected +binaries resolve as stale or missing. Leaving the flag off, the default, changes nothing. Keeping +a legacy-path copy for rollback is not implemented; treat enabling the flag as requiring a +forward-only recovery plan. + ## Storage layer behavior with the flag on The storage chain (`ChainableStoragePersistenceAPI`) and its providers change as follows: @@ -59,9 +73,6 @@ The storage chain (`ChainableStoragePersistenceAPI`) and its providers change as per lookup. Only listings that need every key (`listObjectPaths`, `listObjectSnapshots`, `deleteGroup` and static push) follow every page. -`SharedExtractedMetadata` caches byte-derived Tika extraction at -`extracted-metadata//.json`. Nothing calls it yet; metadata -generation starts using it in a later slice. ## Binary asset API @@ -92,6 +103,105 @@ leave its `{inode}/{field}` directory. This applies with the flag on or off. With the flag off, the API uses the existing filesystem/NFS paths and makes no remote calls. +## Content binaries and immutable revisions + +With the flag on, new CMS binary writes use +`binary-assets/{a}/{b}/{inode}/{field}/.revisions/{revision-id}/{filename}`. The Binary field JSON +keeps its original `value` filename and adds `storageKey` (and `metadataStorageKey` for its +metadata). The field reference changes in the content transaction, so an uncommitted replacement +does not overwrite the prior object, and a rollback keeps the previous revision. Legacy binary +JSON and storage keys remain readable. Reconstructed files keep the stored revision path without +I/O. Old revisions are kept until whole-inode cleanup. A check-in that rolls back deletes the +revision and revision metadata it uploaded, through a rollback listener; the key carries a fresh +UUID, so no other version can reference it. If that delete fails it is logged and the object is +left behind. Metadata written under a different key during the rolled-back check-in, and +revisions abandoned by a rollback to a savepoint, are not reclaimed. + +Custom metadata is copied to replacements, and metadata files and cache entries identify the exact +binary revision. Binary HTTP responses (`BinaryExporterServlet`) hold a cache lease while they +resolve, export and open the file, and release it before streaming, so a slow client does not +defer eviction. `FileAsset.getInputStream` and `Contentlet.getBinaryStream` also hold it only until +the stream is open, because a lease must be released on the thread that took it and a caller may +read or close the stream elsewhere. On a local disk an open file survives eviction, so this is +safe. Eviction on an NFS asset directory, where deleting an open file can break the read, +has not been validated. + +## Metadata from evicted originals + +With the flag on, `FileStorageAPI` restores the exact binary only when metadata generation needs +its bytes, and holds a cache lease through basic inspection, hashing and Tika parsing. Restore +failures propagate, and so do shared-extraction storage failures, instead of producing a +successful partial metadata result. + +Byte-derived Tika extraction is shared through `SharedExtractedMetadata`, cached at +`extracted-metadata//.json`. The configuration hash covers the +parser bundle version, the binary metadata schema version and the extracted-text limit. Filenames, +local paths, fallback titles and modification times are added per use afterwards, and custom +attributes and focal points stay in their content-owned snapshots. Unknown parser versions and +flag-off mode extract directly. Empty or failed extractions are not published. Shared extraction +records are not reclaimed yet. + +JSON metadata hydration reads the linked image's owner key, not the parent content's binary key. +Complete stored metadata avoids normalizing or downloading the original. Missing metadata can be +regenerated from a cold legacy or revision path. + +## Durable deletion + +Whole-inode deletion, deletion of one language of multilingual content, and old-version +maintenance record one `binaryAssetCleanup` job per deleted inode in the same database transaction +as the content deletion. Main leaves the files of a deleted language on disk (#9146); with the +flag on, that path now records cleanup jobs and, when `BACKUP_DELETED_CONTENTLETS_TO_DISK` is on, +recovery archives like the other deletion paths. + +The job carries the exact binary and metadata paths stored for the inode when it was recorded, so +recording it lists the inode's objects inside the deletion transaction, and a storage listing +failure aborts the deletion. The worker refuses while a content version with that inode exists, +then deletes the recorded metadata before the recorded source objects, so a failure leaves the +sources available for a retry. It never deletes by prefix: an inode can be re-created after the +deletion (push publishing keeps the sender's inodes), and the revisions it uploads survive. Objects +uploaded under the deleted inode by a transaction that overlapped the deletion are therefore not +reclaimed. The inode's completed renditions and legacy image cache are still removed whole, since +they regenerate on demand. S3 failures use the job queue's retry policy; after retries are +exhausted the job stays failed and can be retried through the job management API. Direct +submissions through the public job endpoint are rejected. With the flag off, workers do not touch +storage and pending jobs are not silently completed. + +## Binary field trash + +With the flag on, deleting a binary field records a `binaryFieldCleanup` request in the same +transaction as the field deletion. Requests cover historical and working versions up to the +deletion time; values from a later field with the same name are kept. + +Cleanup runs as a queued job, never inside the caller's request. It handles one content row per +transaction: it locks that row, rechecks that it was not edited after the field was removed, +uploads and verifies a recovery ZIP, clears the old field reference, records the exact cleanup +inventory, and advances the job's saved cursor, all in the row's own commit. Only that row is locked +while its archive uploads, and a retry resumes after the last committed row. A separate step checks that +the archived files are no longer referenced and that the ZIP is still available before deleting +metadata, originals and renditions. It never deletes a whole field prefix, so uploads made after +the inventory was captured survive a retry. Direct submissions through the public job endpoint are +rejected; only field deletion and `ContentletAPI.cleanField` create this work. With the flag off, scheduling and local +trash behave as before. + +## Deleted-content recovery archives + +When both the flag and the existing `BACKUP_DELETED_CONTENTLETS_TO_DISK` option are on, deletion +writes a verified S3 recovery ZIP before removing content. Archives live in the +`deleted-content-backups` group at `//.zip`. Full destruction and +all-version deletion archive each version, deleting one language archives each version in that +language, and single-version deletion archives only that version. A failed backup aborts the +deletion. Binary cleanup never deletes recovery archives. Field trash ZIPs use the same group and +layout. + +Each ZIP contains `contentlet.json` (the row's complete typed field data), `contentlet.xml`, and +`assets/` entries under the original binary and metadata paths, including binary fields whose +definitions were removed. Operators can download the ZIPs from S3 for manual recovery; internal +callers can use `ContentletBackupStorage.list(identifier)` and `open(key)`. There is no automatic +database restore, and archives are kept until removed or expired by the bucket's lifecycle policy. + +With the flag on, the `deleteAllVersionsandBackup` interceptor, previously a no-op, calls its +implementation and the all-version deletion hooks. With the flag off it stays a no-op. + ## Local cache eviction With the flag on, the local asset directory is a cache that `BinaryCacheEvictionJob` can trim. @@ -171,8 +281,10 @@ an S3-compatible custom endpoint needs a key and secret. ## Running the checks The S3 checks use the real filesystem provider, chain and AWS adapter against a disposable -MinIO bucket. `BinaryS3StorageTest` is skipped unless `s3.test.endpoint` is set; the other -tests always run. The credentials below are disposable local test values. +MinIO bucket, and the transaction checks use a disposable PostgreSQL database; each test creates +and removes its own schema. `BinaryS3StorageTest` is skipped unless `s3.test.endpoint` is set, and +the PostgreSQL cases are skipped unless `s3.test.jdbc` is set. The credentials below are disposable +local test values. ```sh docker run -d --rm --name binary-s3-test \ @@ -181,11 +293,27 @@ docker run -d --rm --name binary-s3-test \ -e MINIO_ROOT_PASSWORD=binary-storage-test \ minio/minio:latest server /data +docker run -d --rm --name binary-cleanup-postgres-test \ + -p 127.0.0.1:19003:5432 \ + -e POSTGRES_USER=binary-storage-test \ + -e POSTGRES_PASSWORD=binary-storage-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 \ - -Ds3.test.endpoint=http://127.0.0.1:19002 + -Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest,BinaryAssetReferenceTest,BinaryCacheEvictionJobTest,BinaryFileSystemStorageTest,BinaryAssetStorageAPIImplTest,MetadataLocalCacheTest,BinaryAssetCleanupTransactionTest,BinaryAssetCleanupProcessorTest,ContentletBackupStorageGateTest,BinaryFieldCleanupProcessorTest,AssetJobEventSerializationTest \ + -Ds3.test.endpoint=http://127.0.0.1:19002 \ + -Ds3.test.jdbc=jdbc:postgresql://127.0.0.1:19003/binary_storage_test -docker stop binary-s3-test +docker stop binary-s3-test binary-cleanup-postgres-test +``` + +The CMS integration checks are registered in `Junit5Suite1` and run against the full integration +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 ``` CI does not yet provide the MinIO service, so `BinaryS3StorageTest` does not run there. diff --git a/dotCMS/src/main/java/com/dotcms/content/business/json/ContentletJsonAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/business/json/ContentletJsonAPIImpl.java index 8b451fd270c1..913c85d6e66f 100644 --- a/dotCMS/src/main/java/com/dotcms/content/business/json/ContentletJsonAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/business/json/ContentletJsonAPIImpl.java @@ -33,7 +33,6 @@ import com.dotmarketing.exception.DotSecurityException; import com.dotmarketing.portlets.categories.business.CategoryAPI; import com.dotmarketing.portlets.categories.model.Category; -import com.dotmarketing.portlets.contentlet.business.BinaryFileFilter; import com.dotmarketing.portlets.contentlet.business.ContentletAPI; import com.dotmarketing.portlets.contentlet.business.HostAPI; import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; @@ -83,8 +82,6 @@ */ public class ContentletJsonAPIImpl implements ContentletJsonAPI { - private static final BinaryFileFilter binaryFileFilter = new BinaryFileFilter(); - final IdentifierAPI identifierAPI; final ContentTypeAPI contentTypeAPI; final FileAssetAPI fileAssetAPI; @@ -401,6 +398,15 @@ private boolean isNotMappable(final Field field) { */ private Optional getBinary(final Field field, final String inode, final FieldValue storedValue) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled() + && storedValue instanceof com.dotcms.content.model.type.system.AbstractBinaryFieldType) { + final String key = ((com.dotcms.content.model.type.system.AbstractBinaryFieldType) storedValue).storageKey(); + if (key != null) { + return Optional.of(com.dotcms.storage.binary.BinaryAssetReference.withMetadata( + com.dotcms.storage.binary.BinaryAssetReference.localFile(inode, field.variable(), key), + inode, field.variable(), ((com.dotcms.content.model.type.system.AbstractBinaryFieldType) storedValue).metadataStorageKey())); + } + } // This validation is here to prevent an exception. // Cause the json gets saved twice by internalCheckin and the first time it does it no inode is set yet @@ -422,17 +428,31 @@ private Optional getBinary(final Field field, final String inode, final Object storedName = null != storedValue ? storedValue.value() : null; if (storedName instanceof String && isSet((String) storedName) && !((String) storedName).contains("/") && !((String) storedName).contains("\\")) { - return Optional.of(new java.io.File(binaryFileFolder, (String) storedName)); + final File file = new java.io.File(binaryFileFolder, (String) storedName); + if (com.dotcms.storage.AssetStorageFeature.isEnabled() + && storedValue instanceof com.dotcms.content.model.type.system.AbstractBinaryFieldType) { + return Optional.of(com.dotcms.storage.binary.BinaryAssetReference.withMetadata(file, inode, + field.variable(), ((com.dotcms.content.model.type.system.AbstractBinaryFieldType) storedValue).metadataStorageKey())); + } + return Optional.of(file); } - // Legacy json without a stored file name: fall back to listing the folder. No exists() - // pre-check — listFiles() returns null for a missing folder. - final java.io.File[] files = binaryFileFolder.listFiles(binaryFileFilter); - if (files != null && files.length > 0) { - return Optional.of(files[0]); + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { + final File[] files = binaryFileFolder.listFiles(new com.dotmarketing.portlets.contentlet.business.BinaryFileFilter()); + return files != null && files.length > 0 ? Optional.of(files[0]) : Optional.empty(); } - return Optional.empty(); + // Legacy json without a stored filename resolves through binary storage. + try { + final File file = APILocator.getBinaryAssetStorageAPI() + .getBinaryFile(inode, field.variable()); + return Optional.ofNullable(file); + } catch (final DotDataException e) { + Logger.debug(this, () -> String.format( + "Binary not found for inode '%s', field '%s': %s", + inode, field.variable(), e.getMessage())); + return Optional.empty(); + } } /** @@ -474,6 +494,14 @@ private Optional> hydrateThenGetFieldValue(final Object value, fin final Optional fieldValueBuilder = field.fieldValue(value); if (fieldValueBuilder.isPresent()) { FieldValueBuilder builder = fieldValueBuilder.get(); + if (com.dotcms.storage.AssetStorageFeature.isEnabled() && value instanceof File + && builder instanceof com.dotcms.content.model.type.system.BinaryFieldType.Builder) { + ((com.dotcms.content.model.type.system.BinaryFieldType.Builder) builder).storageKey( + com.dotcms.storage.binary.BinaryAssetReference.keyOf((File) value, + contentlet.getInode(), field.variable())) + .metadataStorageKey(com.dotcms.storage.binary.BinaryAssetReference.metadataKeyOf( + (File) value, contentlet.getInode(), field.variable())); + } final List> delegateAndFields = getHydrationDelegatesFromAnnotations(builder.getClass()); for (Tuple2 delegateAndField : delegateAndFields) { final HydrationDelegate delegate = delegateAndField._1(); diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/DropOldContentletRunner.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/DropOldContentletRunner.java index 0aa79459c643..d35297084d5d 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/DropOldContentletRunner.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/DropOldContentletRunner.java @@ -173,10 +173,17 @@ public int deleteOldContent() { dc.setSQL(String.format(DELETE_TAG_INODES, inodes)); dc.loadResult(conn); + if (com.dotcms.storage.AssetStorageFeature.isEnabled() && CLEAN_DEAD_INODE_FROM_FS) { + for (final String inode : inodeList) { + com.dotcms.storage.binary.BinaryAssetCleanupProcessor.enqueue(inode); + } + } conn.commit(); conn.setAutoCommit(true); - deleteFromAssetsDir(inodeList); + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { + deleteFromAssetsDir(inodeList); + } inodeList.clear(); if (isInterrupted()) { diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index 7728a217d6ca..3972e4d4fd1d 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -1,6 +1,7 @@ package com.dotcms.content.elasticsearch.business; import com.dotcms.api.system.event.ContentletSystemEventUtil; +import com.dotcms.storage.binary.BinaryAssetStorageAPI; import com.dotcms.api.web.HttpServletRequestThreadLocal; import com.dotcms.business.CloseDBIfOpened; import com.dotcms.business.WrapInTransaction; @@ -1820,6 +1821,12 @@ public void cleanField(final Structure structure, final Date deletionDate, final // Binary fields have nothing to do with database. if (field instanceof BinaryField) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + // Archiving uploads to S3; queue it so this caller neither waits nor holds row locks. + com.dotcms.storage.binary.BinaryFieldCleanupProcessor.enqueue(structure.getInode(), field.variable(), + deletionDate == null ? new Date() : deletionDate); + return; + } int batchSize = 500; int offset = 0; final List contentlets = new ArrayList<>(); @@ -2710,6 +2717,12 @@ private Optional checkAndRunDeleteAsWorkflow(final Contentlet contentle return Optional.empty(); } + /** + * {@inheritDoc} + *

With S3 asset storage on, the deletion runs in a transaction (joining the caller's, if + * any), so the binary cleanup jobs it records commit or roll back with the deleted rows. With + * the flag off the transaction handling is unchanged.

+ */ @RequestCost(Price.CONTENT_DELETE) @Override public boolean delete(final Contentlet contentlet, final User user, @@ -2729,7 +2742,16 @@ public boolean delete(final Contentlet contentlet, final User user, try { boolean isSite = contentlet.isHost(); - deleted = this.deleteContentlets(contentlets, user, respectFrontendRoles, isSite); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + // S3 cleanup jobs must commit with the deleted rows, and callers such as the Site + // Browser and WebDAV reach this method without a transaction. + final boolean[] result = new boolean[1]; + LocalTransaction.wrap(() -> result[0] = this.deleteContentlets(contentlets, user, + respectFrontendRoles, isSite)); + deleted = result[0]; + } else { + deleted = this.deleteContentlets(contentlets, user, respectFrontendRoles, isSite); + } HibernateUtil.addCommitListener (() -> this.localSystemEventsAPI.notify( new ContentletDeletedEvent<>(contentlet, user))); @@ -3187,7 +3209,8 @@ private boolean destroyContentlets(final List contentlets, final Use this.logContentletActivity(contentlet, "Content Destroyed", user); } - this.backupDestroyedContentlets(contentlets, user); + this.backupDestroyedContentlets(com.dotcms.storage.AssetStorageFeature.isEnabled() + ? contentletsVersion : contentlets, user); // Collected before the delete so the journal never depends on post-deletion object // state — see journalContentDeletes. @@ -3283,10 +3306,19 @@ private void deleteElementFromPublishQueueTable(final List contentle } } - private void backupDestroyedContentlets(final List contentlets, final User user) { + private void backupDestroyedContentlets(final List contentlets, final User user) throws DotDataException { if(!Config.getBooleanProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", false)){ return; } + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + final Set backedUp = new HashSet<>(); + for (Contentlet contentlet : contentlets) { + if (backedUp.add(contentlet.getInode())) { + com.dotcms.storage.binary.ContentletBackupStorage.getInstance().store(contentlet); + } + } + return; + } if (contentlets.size() > 0) { final XStream xstream = XStreamHandler.newXStreamInstance(); @@ -3488,6 +3520,13 @@ private void deleteContentlet(final List contentlets, final User use contentletsLanguageList.forEach( contentletLanguage -> contentletLanguage.setIndexPolicy( contentletToDelete.getIndexPolicy())); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + // Main keeps these files on disk (#9146); in S3 they would never be reclaimed. + // Cleanup is recorded before the delete, so a caller without a transaction + // fails before any row is removed. + this.backupDestroyedContentlets(contentletsLanguageList, user); + this.deleteBinaryFiles(contentletsLanguageList, null); + } this.contentFactory.delete(contentletsLanguageList, false); for (final Contentlet contentlet : contentlets) { @@ -3635,6 +3674,9 @@ public void deleteAllVersionsandBackup(List contentlets, User user, // Collected before the delete — see journalContentDeletes. final Set destroyedIdentifiers = deferredRemovalIdentifiers(contentletsVersion); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + backupDestroyedContentlets(contentletsVersion, APILocator.systemUser()); + } contentFactory.delete(contentletsVersion); // Same durability requirement as destroyContentlets: the index removal below is deferred @@ -3649,7 +3691,9 @@ public void deleteAllVersionsandBackup(List contentlets, User user, CacheLocator.getIdentifierCache().removeFromCacheByVersionable(contentlet); } - backupDestroyedContentlets(contentlets, APILocator.systemUser()); + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { + backupDestroyedContentlets(contentlets, APILocator.systemUser()); + } deleteBinaryFiles(contentletsVersion, null); } @@ -3728,6 +3772,9 @@ public void deleteVersion(Contentlet contentlet, User user, boolean respectFront ArrayList contentlets = new ArrayList<>(); contentlets.add(contentlet); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + backupDestroyedContentlets(contentlets, user); + } contentFactory.deleteVersion(contentlet); Optional cinfo = APILocator.getVersionableAPI() @@ -3745,7 +3792,9 @@ public void deleteVersion(Contentlet contentlet, User user, boolean respectFront deleteBinaryFiles(contentlets, null); - fileMetadataAPI.removeVersionMetadata(contentlet); + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { + fileMetadataAPI.removeVersionMetadata(contentlet); + } } @@ -5969,6 +6018,10 @@ private Contentlet internalCheckin(Contentlet contentlet, workingContentlet = contentlet; if (createNewVersion) { workingContentlet = findWorkingContentlet(contentlet); + } else if (com.dotcms.storage.AssetStorageFeature.isEnabled() && !contentType.fields(BinaryField.class).isEmpty()) { + // The incoming object already contains edits. Preserve the database snapshot + // before save so binary replacement cannot mistake an upload for the old file. + workingContentlet = contentFactory.findInDb(contentlet.getInode()).orElse(contentlet); } String workingContentletInode = (workingContentlet == null) ? "" : workingContentlet.getInode(); @@ -6124,6 +6177,15 @@ private Contentlet internalCheckin(Contentlet contentlet, handleBinaries(contentlet, createNewVersion, contentType, workingContentlet, contentletRaw); + if (com.dotcms.storage.AssetStorageFeature.isEnabled() && !contentType.fields(BinaryField.class).isEmpty()) { + // Persist immutable binary references alongside the filename in this transaction. + final String json = APILocator.getContentletJsonAPI().toJson(contentlet); + final String jsonValue = DbConnectionFactory.isPostgres() ? "?::jsonb" : "?"; + new DotConnect().setSQL("update contentlet set contentlet_as_json = " + jsonValue + " where inode = ?") + .addParam(json).addParam(contentlet.getInode()).loadResult(); + contentlet.getMap().put(Contentlet.CONTENTLET_AS_JSON, json); + } + updatePublishAndExpireDates(contentlet, systemUser, contentletRaw); final Structure hostStructure = CacheLocator.getContentTypeCache() @@ -6782,6 +6844,10 @@ private Contentlet relateTags(final Contentlet contentlet, final Map> uploadedMetadata = new HashMap<>(); + + // loop over the new field values + // if we have a new temp file or a deleted file + // do it to the new inode directory + for (com.dotcms.contenttype.model.field.Field field : contentType.fields( + BinaryField.class)) { + try { + + final String velocityVarNm = field.variable(); + File incomingFile = contentletRaw.getBinary(velocityVarNm); + if (validateEmptyFile && incomingFile != null && incomingFile.length() == 0 + && !Config.getBooleanProperty("CONTENT_ALLOW_ZERO_LENGTH_FILES", false)) { + throw new DotContentletStateException( + "Cannot checkin 0 length file: " + incomingFile); + } + + // if the user has removed this file via ui + if (incomingFile == null || incomingFile.getAbsolutePath().contains("-removed-")) { + contentlet.setBinary(velocityVarNm, null); + + // Retain prior revision metadata until durable cleanup; this save can roll back. + + } else { // if we have an incoming file + if (incomingFile.exists()) { + + //If the incoming file is temp resource we need to find out if there is any metadata associated + final Optional tempResourceId = tempApi.getTempResourceId( + incomingFile); + + //The physical file name is preserved across versions. + //No need to update the name. We will only reference the file through the logical asset-name + final String oldFileName = incomingFile.getName(); + + File oldFile = null; + if (UtilMethods.isSet(oldInode)) { + oldFile = workingContentlet.getBinary(velocityVarNm); + + // do we have an inline edited file, if so use that + // (tmpDir stays as filesystem code — not binary storage) + File editedFile = new File( + tmpDir.getAbsolutePath() + File.separator + velocityVarNm + + File.separator + WebKeys.TEMP_FILE_PREFIX + + oldFileName); + if (editedFile.exists()) { + incomingFile = editedFile; + } + } + + // Never overwrite bytes referenced by committed content. The JSON reference + // switches in the database transaction; rollback leaves the old object intact. + final File newFile; + if (oldFile != null && oldFile.equals(incomingFile) && newInode.equals(oldInode) + && com.dotcms.storage.binary.BinaryAssetReference.keyOf(oldFile, newInode, velocityVarNm) != null) { + newFile = oldFile; + } else { + newFile = binaryStorageAPI.storeRevision(newInode, velocityVarNm, + oldFileName, incomingFile); + // A rolled-back check-in never commits a reference to this new key. + com.dotcms.storage.binary.BinaryAssetCleanupProcessor.deleteRevisionOnRollback(newInode, + velocityVarNm, com.dotcms.storage.binary.BinaryAssetReference.keyOf( + newFile, newInode, velocityVarNm)); + } + + contentlet.setBinary(velocityVarNm, newFile); + binaryHandled = true; + + //This copies the metadata associated with the temp resource passed if any. + if (tempResourceId.isPresent()) { + final Optional optionalMetadata = fileMetadataAPI.getMetadata( + tempResourceId.get()); + if (optionalMetadata.isPresent()) { + final Metadata tempMeta = optionalMetadata.get(); + uploadedMetadata.put(velocityVarNm, tempMeta.getCustomMeta()); + Logger.debug(ESContentletAPIImpl.class, + String.format("Metadata copied from temp resource: `%s` ", + tempResourceId.get())); + } + } + } + } + } catch (IOException | DotDataException e) { + throw new DotContentletValidationException( + "Error occurred while processing the file:" + e.getMessage(), e); + } + } + if (binaryHandled && workingContentlet != contentlet) { + fileMetadataAPI.copyCustomMetadataForCheckin(workingContentlet, contentlet); + } + if (!uploadedMetadata.isEmpty()) { + fileMetadataAPI.putCustomMetadataAttributesForCheckin(contentlet, uploadedMetadata); + } + return binaryHandled; } private void checkPermission( @@ -9401,10 +9587,25 @@ public List findFieldValues(String structureInode, Field field, User use } /** - * @param contentlets - * @param field + * Deletes the binary files of the given content versions. With S3 asset storage on and no field + * given, it records one durable cleanup job per distinct inode in the current transaction instead, + * because callers can pass the same version more than once. Otherwise it removes the metadata and + * the local binary and resized-image folders, as before. + * + * @param contentlets the deleted content versions + * @param field the field whose files to delete, or {@code null} for every field + * @throws DotDataException if a cleanup job cannot be recorded */ - private void deleteBinaryFiles(List contentlets, Field field) { + private void deleteBinaryFiles(List contentlets, Field field) throws DotDataException { + if (com.dotcms.storage.AssetStorageFeature.isEnabled() && field == null) { + final Set enqueued = new HashSet<>(); + for (final Contentlet contentlet : contentlets) { + if (enqueued.add(contentlet.getInode())) { + com.dotcms.storage.binary.BinaryAssetCleanupProcessor.enqueue(contentlet.getInode()); + } + } + return; + } this.destroyMetadata(contentlets); contentlets.forEach(con -> { @@ -9497,6 +9698,8 @@ private String getContentletCacheAssetPath(Contentlet con, Field field) { @Override public File getBinaryFile(final String contentletInode, final String velocityVariableName, final User user) throws DotDataException, DotSecurityException { + // Preserve main's filesystem/NFS behavior while S3 assets are disabled. + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { Logger.debug(this, "Retrieving binary file name : getBinaryFileName()."); @@ -9545,6 +9748,37 @@ public File getBinaryFile(final String contentletInode, final String velocityVar throw new DotDataException("File System error.", e); } return binaryFile; + + } + + + Logger.debug(this, "Retrieving binary file name : getBinaryFileName()."); + + Contentlet con = contentFactory.find(contentletInode); + + if (!permissionAPI.doesUserHavePermission(con, PermissionAPI.PERMISSION_READ, user)) { + if (null != user) { + throw new DotSecurityException(String.format( + "Unauthorized Access user [%s , %s] trying to access contentlet identified by `%s`.", + user.getUserId(), user.getEmailAddress(), con.getIdentifier())); + } else { + throw new DotSecurityException( + "Unauthorized Access null user trying to access contentlet. "); + } + } + + File binaryFile = null; + try { + binaryFile = APILocator.getBinaryAssetStorageAPI() + .getBinaryFile(contentletInode, velocityVariableName); + } catch (Exception e) { + Logger.error(this, + "Error occurred while retrieving binary file name : getBinaryFileName(). ContentletInode : " + + contentletInode + + " velocityVaribleName : " + velocityVariableName); + throw new DotDataException("File System error.", e); + } + return binaryFile; } @CloseDBIfOpened diff --git a/dotCMS/src/main/java/com/dotcms/content/model/hydration/MetadataDelegate.java b/dotCMS/src/main/java/com/dotcms/content/model/hydration/MetadataDelegate.java index ac4e49cb174f..c2d4ca25c25d 100644 --- a/dotCMS/src/main/java/com/dotcms/content/model/hydration/MetadataDelegate.java +++ b/dotCMS/src/main/java/com/dotcms/content/model/hydration/MetadataDelegate.java @@ -6,6 +6,8 @@ import com.dotcms.contenttype.model.field.ImageField; import com.dotcms.exception.ExceptionUtil; import com.dotcms.storage.FileMetadataAPI; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.binary.BinaryAssetReference; import com.dotcms.storage.FileStorageAPI; import com.dotcms.storage.GenerateMetadataConfig; import com.dotcms.storage.StorageKey; @@ -84,7 +86,7 @@ private Map getMetadataMap(final Field field, final Contentlet c final Optional fileAsContentOptional = findLinkedBinary(contentlet, (ImageField) field); if (fileAsContentOptional.isPresent()) { final Contentlet fileAsset = fileAsContentOptional.get(); - final String path = Try.of(()-> fileMetadataAPI.getFileName(contentlet, FileAssetAPI.BINARY_FIELD)).getOrElse("unk"); + final String path = Try.of(()-> fileMetadataAPI.getFileName(AssetStorageFeature.isEnabled() ? fileAsset : contentlet, FileAssetAPI.BINARY_FIELD)).getOrElse("unk"); final File file = (File) fileAsset.get(FileAssetAPI.BINARY_FIELD); metadataMap = getMetadataMap(file, path); } @@ -121,7 +123,9 @@ File normalize(final File in) { final String[] parts = path.split(Pattern.quote(File.separator)); final int fileIndex = parts.length - 1; - final int folderIndex = fileIndex - 1; + final int revisionDepth = AssetStorageFeature.isEnabled() && parts.length >= 7 + && ".revisions".equals(parts[fileIndex - 2]) ? 2 : 0; + final int folderIndex = fileIndex - 1 - revisionDepth; final int inodeIndex = folderIndex - 1; final int char2Index = inodeIndex - 1; final int char1Index = char2Index - 1; @@ -132,6 +136,15 @@ File normalize(final File in) { final String char2 = parts[char2Index]; final String char1 = parts[char1Index]; + if (AssetStorageFeature.isEnabled() && inode.startsWith(char1 + char2) + && char1.length() == 1 && char2.length() == 1) { + // Missing local files are valid references; the storage reader restores them lazily. + return revisionDepth == 0 + ? new BinaryAssetReference.StoredBinary(null, null, file).localFile(inode, folder) + : BinaryAssetReference.localFile(inode, folder, String.join("/", + java.util.Arrays.copyOfRange(parts, char1Index, parts.length))); + } + final String assetsRootPath = ConfigUtils.getAbsoluteAssetsRootPath(); //Rebuild the path and prepend the new local assets real-path final String rebuiltPath = String.join(File.separator, assetsRootPath, char1, char2, @@ -164,8 +177,10 @@ private Map getMetadataMap(final File file, final String p final StorageType storageType = StoragePersistenceProvider.getStorageType(); final FileStorageAPI fileStorageAPI = APILocator.getFileStorageAPI(); final Predicate filterBasicMetadataKey = metadataKey -> true; - final File normalized = normalize(file); - return fileStorageAPI.generateMetaData(normalized, + final boolean s3 = AssetStorageFeature.isEnabled(); + // Preserve eager filesystem normalization when S3 mode is disabled. + final File normalized = s3 ? null : normalize(file); + return fileStorageAPI.generateMetaData(() -> s3 ? normalize(file) : normalized, new GenerateMetadataConfig.Builder() //if there's metadata already generated this should find it for us (all that if we have an inode) //otherwise it'll give us the basic md. Nothing gets stored/saved here. @@ -175,6 +190,8 @@ private Map getMetadataMap(final File file, final String p .store(false) .cache(false) .metaDataKeyFilter(filterBasicMetadataKey) + .getIfOnlyHasCustomMetadata(metadata -> s3 && metadata.keySet().containsAll(fieldNames) + ? Map.of() : metadata) .build()); } diff --git a/dotCMS/src/main/java/com/dotcms/content/model/type/system/AbstractBinaryFieldType.java b/dotCMS/src/main/java/com/dotcms/content/model/type/system/AbstractBinaryFieldType.java index 44a80dda5cde..6863d85e20dc 100644 --- a/dotCMS/src/main/java/com/dotcms/content/model/type/system/AbstractBinaryFieldType.java +++ b/dotCMS/src/main/java/com/dotcms/content/model/type/system/AbstractBinaryFieldType.java @@ -42,6 +42,16 @@ default String type() { @JsonDeserialize(using = MetadataMapDeserializer.class) Map metadata(); + @Nullable + @JsonProperty("storageKey") + @com.fasterxml.jackson.annotation.JsonInclude(com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL) + String storageKey(); + + @Nullable + @JsonProperty("metadataStorageKey") + @com.fasterxml.jackson.annotation.JsonInclude(com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL) + String metadataStorageKey(); + @Hydration(properties = { @HydrateWith(delegate = MetadataDelegate.class, propertyName = "metadata") }) diff --git a/dotCMS/src/main/java/com/dotcms/contenttype/model/field/BinaryField.java b/dotCMS/src/main/java/com/dotcms/contenttype/model/field/BinaryField.java index ead62e99b112..73f5521bc540 100644 --- a/dotCMS/src/main/java/com/dotcms/contenttype/model/field/BinaryField.java +++ b/dotCMS/src/main/java/com/dotcms/contenttype/model/field/BinaryField.java @@ -87,6 +87,11 @@ public Optional fieldValue(final Object value){ if (value instanceof File) { final File file = (File) value; + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + return Optional.of(BinaryFieldType.builder().value(file.getName()) + .storageKey(com.dotcms.storage.binary.BinaryAssetReference.keyOf(file)) + .metadataStorageKey(com.dotcms.storage.binary.BinaryAssetReference.metadataKeyOf(file))); + } return Optional.of(BinaryFieldType.builder().value(file.getName())); } diff --git a/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java b/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java index d6a79f530a64..dc14f369a4a7 100644 --- a/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java @@ -169,7 +169,9 @@ public JobQueueManagerAPIImpl(@Named("queueProducer") JobQueue jobQueue, this.realTimeJobMonitor = realTimeJobMonitor; // Register discovered processors by CDI - discovery.discoverJobProcessors().forEach(this::registerProcessor); + discovery.discoverJobProcessors().stream() + .filter(com.dotcms.storage.AssetStorageFeature::allowsJobProcessor) + .forEach(this::registerProcessor); APILocator.getLocalSystemEventsAPI().subscribe( JobCancelRequestEvent.class, diff --git a/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java b/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java index 3b746872819f..7417a755a469 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java +++ b/dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java @@ -1,8 +1,12 @@ package com.dotcms.storage; +import com.dotcms.storage.binary.BinaryAssetCleanupProcessor; +import com.dotcms.storage.binary.BinaryFieldCleanupProcessor; import com.dotmarketing.util.Config; import com.dotmarketing.util.Logger; +import java.util.Set; + /** * Startup configuration for the opt-in S3 asset lifecycle. Disabled preserves filesystem/NFS behavior. * @@ -13,6 +17,10 @@ public final class AssetStorageFeature { public static final String FLAG = "FEATURE_FLAG_S3_ASSET_STORAGE"; + /** Job processors that belong to the S3 lifecycle and must not register while it is disabled. */ + private static final Set> JOB_PROCESSORS = Set.of(BinaryAssetCleanupProcessor.class, + BinaryFieldCleanupProcessor.class); + private static volatile Boolean enabled; private AssetStorageFeature() { } @@ -43,6 +51,17 @@ public static boolean isEnabled() { return value; } + /** + * Tells the job queue whether a discovered processor may register. S3 lifecycle processors are + * skipped while the feature is disabled, so their queues do not exist, as on a build without them. + * + * @param processor the discovered job processor class + * @return {@code false} only for an S3 lifecycle processor while the feature is disabled + */ + public static boolean allowsJobProcessor(final Class processor) { + return isEnabled() || !JOB_PROCESSORS.contains(processor); + } + /** * Forgets the value read at first use, so the next {@link #isEnabled()} reads the configuration * again. Called by {@link Config#setProperty(String, Object)} for in-memory overrides of the flag, diff --git a/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPI.java b/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPI.java index d37129bfc710..c64de9b55c65 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPI.java +++ b/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPI.java @@ -34,6 +34,34 @@ public interface FileMetadataAPI { String DEFAULT_METADATA_GROUP_NAME = DOT_METADATA; String METADATA_JSON = "-metadata.json"; + /** + * Lists the stored metadata paths that belong to a deleted inode: legacy per-field metadata, any + * metadata left under the inode folder, and the metadata of each revision in {@code binaryPaths}. + * Whole-inode deletion records this list with its cleanup job, so the job later deletes exactly + * these paths and never metadata written after the deletion. Only the S3 asset lifecycle calls + * this; the default returns nothing, as filesystem/NFS cleanup keeps metadata. + * + * @param inode the deleted contentlet inode + * @param binaryPaths the inode's stored binary paths at deletion time + * @return the metadata paths to delete, each starting with {@code /} + * @throws DotDataException if metadata cannot be listed or a path escapes the inode + */ + default java.util.List listMetadataForInode(String inode, java.util.List binaryPaths) + throws DotDataException { + return java.util.List.of(); + } + + /** + * Deletes the metadata paths recorded by {@link #listMetadataForInode} for a deleted inode and + * evicts them from the metadata cache. Deleting a path that is already gone succeeds, so a retried + * cleanup job can call this again. The default does nothing. + * + * @param inode the deleted contentlet inode + * @param metadataPaths the recorded metadata paths, each under the inode's folder + * @throws DotDataException if a path escapes the inode or remains after deletion + */ + default void removeMetadataPaths(String inode, java.util.List metadataPaths) throws DotDataException { } + /** * Metadata file generator. * @param contentlet @@ -43,12 +71,30 @@ public interface FileMetadataAPI { default String getFileName (final Contentlet contentlet, final String fieldVariableName) { final String inode = contentlet.getInode(); + if (AssetStorageFeature.isEnabled() && contentlet.get(fieldVariableName) instanceof File) { + final String metadataKey = com.dotcms.storage.binary.BinaryAssetReference.metadataKeyOf( + (File) contentlet.get(fieldVariableName), inode, fieldVariableName); + if (metadataKey != null) { + return metadataKey; + } + final String revision = com.dotcms.storage.binary.BinaryAssetReference.keyOf( + (File) contentlet.get(fieldVariableName), inode, fieldVariableName); + if (revision != null) { + return File.separator + revision + METADATA_JSON; + } + } final String fileName = fieldVariableName + METADATA_JSON; return StringUtils.builder(File.separator, inode.charAt(0), File.separator, inode.charAt(1), File.separator, inode, File.separator, fileName).toString(); } + /** Cache and persistent metadata must identify the same binary snapshot, without reading its bytes. */ + default String getMetadataCacheKey(final Contentlet contentlet, final String fieldVariableName) { + return AssetStorageFeature.isEnabled() ? getFileName(contentlet, fieldVariableName) + : contentlet.getInode() + ":" + fieldVariableName; + } + /** * Reads INDEX_METADATA_FIELDS for pre-configured metadata fields * @return @@ -176,6 +222,19 @@ void putCustomMetadataAttributes(Contentlet contentlet, final Map> customAttributesByField) throws DotDataException; + /** + * Check-in publishes its binary and metadata references together after handling all fields. + * The default writes the attributes directly, as {@link #putCustomMetadataAttributes} does. + * + * @param contentlet the contentlet being checked in + * @param customAttributesByField custom attributes keyed by binary field variable + * @throws DotDataException if the attributes cannot be stored + */ + default void putCustomMetadataAttributesForCheckin(Contentlet contentlet, + Map> customAttributesByField) throws DotDataException { + putCustomMetadataAttributes(contentlet, customAttributesByField); + } + /** * Write custom metadata to linked to a temporary file * @param tempResourceId @@ -203,6 +262,18 @@ Optional getMetadata(final String tempResourceId) */ void copyCustomMetadata(Contentlet source, Contentlet destination) throws DotDataException; + /** + * Check-in copies attributes to its new binary revision before publishing the content JSON. + * The default copies them directly, as {@link #copyCustomMetadata} does. + * + * @param source the contentlet whose custom attributes are copied + * @param destination the contentlet receiving them + * @throws DotDataException if the attributes cannot be copied + */ + default void copyCustomMetadataForCheckin(Contentlet source, Contentlet destination) throws DotDataException { + copyCustomMetadata(source, destination); + } + /** * This forces the metadata into a contentlet. No validation type is performed * This means that a pdf file could end-up with something like "isImage:true" diff --git a/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPIImpl.java b/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPIImpl.java index 7cde4326a956..5b824420a986 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPIImpl.java @@ -11,7 +11,13 @@ import com.dotcms.contenttype.model.field.FieldVariable; import com.dotcms.cost.RequestCost; import com.dotcms.cost.RequestPrices.Price; +import com.dotcms.storage.binary.BinaryAssetReference; import com.dotcms.storage.model.BasicMetadataFields; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.db.HibernateUtil; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.dotcms.storage.model.ContentletMetadata; import com.dotcms.storage.model.Metadata; import com.dotmarketing.business.APILocator; @@ -40,6 +46,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -106,6 +113,10 @@ private ContentletMetadata internalGenerateContentletMetadata(final Contentlet c final SortedSet fullBinaryFieldNameSet, final boolean overrideMetadata) throws IOException, DotDataException { + if (AssetStorageFeature.isEnabled()) { + return generateImmutableMetadata(contentlet, basicBinaryFieldNameSet, + fullBinaryFieldNameSet, overrideMetadata); + } final Map fieldMap = contentlet.getContentType().fieldMap(); Logger.debug(this, ()-> "Generating the metadata for contentlet, id = " + contentlet.getIdentifier()); @@ -120,6 +131,63 @@ private ContentletMetadata internalGenerateContentletMetadata(final Contentlet c return new ContentletMetadata(fullMetadata, basicMetadata); } + private ContentletMetadata generateImmutableMetadata(final Contentlet contentlet, + final Set basicFields, final Set fullFields, + final boolean override) throws DotDataException { + final Contentlet requested = new Contentlet(contentlet); + final Map full = new HashMap<>(); + final Map basic = new HashMap<>(); + final Set fields = new TreeSet<>(basicFields); + fields.addAll(fullFields); + for (final String field : fields) { + if (requested.get(field) == null) { + continue; + } + final StorageKey storageKey = new StorageKey.Builder() + .group(Config.getStringProperty(METADATA_GROUP_NAME, DOT_METADATA)) + .path(getFileName(requested, field)) + .storage(StoragePersistenceProvider.getStorageType()).build(); + final Map previous = fileStorageAPI.retrieveRawMetaData(storageKey); + Map metadata = previous == null ? Map.of() : previous; + final boolean generate = override || metadata.isEmpty() + || metadata.keySet().stream().allMatch(key -> key.startsWith(Metadata.CUSTOM_PROP_PREFIX) + || key.equals(BasicMetadataFields.EDITABLE_AS_TEXT.key())); + if (generate) { + final Set indexedKeys = getMetadataFields(contentlet.getContentType().fieldMap().get(field).id()); + try { + metadata = new HashMap<>(fileStorageAPI.generateMetaData( + () -> Try.of(() -> requested.getBinary(field)).get(), + new GenerateMetadataConfig.Builder().full(fullFields.contains(field)) + .override(true).store(false).cache(false) + .metaDataKeyFilter(key -> indexedKeys.isEmpty() || indexedKeys.contains(key)) + .storageKey(storageKey) + .build())); + } catch (final IllegalArgumentException missingBinary) { + Logger.debug(this, () -> "Cannot generate metadata for missing binary: " + field); + continue; + } + if (previous != null) { + metadata.putAll(filterNonCustomMetadataFields(previous)); + } + final Map generated = metadata; + // Publish only against the snapshot we read. Reindexing a historical snapshot + // must not replace newer metadata, even when the binary bytes are unchanged. + publishMetadata(contentlet, Set.of(field), (snapshot, name) -> generated, requested); + } else { + metadataCache.addMetadataMap(getMetadataCacheKey(requested, field), + filterNonBasicMetadataFields(metadata)); + } + if (fullFields.contains(field)) { + full.put(field, new Metadata(field, metadata)); + } + if (basicFields.contains(field)) { + basic.put(field, new Metadata(field, fullFields.contains(field) + ? filterNonBasicMetadataFields(metadata) : metadata)); + } + } + return new ContentletMetadata(full, basic); + } + /** * Basic metadata generation entry point. * @param contentlet @@ -164,13 +232,13 @@ private Map generateBasicMetadata(final Contentlet contentlet, // if it is included on the full keys, we only have to store the meta in the cache. metadataMap = filterNonBasicMetadataFields(metadata.getMap()); - metadataCache.addMetadataMap(contentlet.getInode() + StringPool.COLON + binaryFieldName, metadataMap); + metadataCache.addMetadataMap(getMetadataCacheKey(contentlet, binaryFieldName), metadataMap); } else { //get Old metadata from cache so we don't loose any custom attributes final Metadata mergeWithMetadata = internalGetGenerateMetadata(contentlet, binaryFieldName,false, false); - final String cacheKey = contentlet.getInode() + StringPool.COLON + binaryFieldName; + final String cacheKey = getMetadataCacheKey(contentlet, binaryFieldName); try { metadataMap = this.fileStorageAPI.generateMetaData( @@ -365,8 +433,7 @@ private Metadata internalGetGenerateMetadata(final Contentlet contentlet, final final Map metadataMap = fileStorageAPI.retrieveMetaData( new FetchMetadataParams.Builder() .projectionMapForCache(this::filterNonBasicMetadataFields) - .cache(() -> contentlet.getInode() + StringPool.COLON - + fieldVariableName) + .cache(() -> getMetadataCacheKey(contentlet, fieldVariableName)) .storageKey(new StorageKey.Builder().group(metadataBucketName) .path(metadataPath).storage(storageType).build()) .build() @@ -598,10 +665,113 @@ private Optional getDefaultMetadata(final Contentlet contentlet, final } /** - * Given a contentlet this will iterate over all the binary fields it has and remove the associated metadata - * @param contentlet - * @return + * Lists a deleted inode's legacy and revision metadata from storage. With the flag off it returns + * nothing. See {@link FileMetadataAPI#listMetadataForInode}. + * + * @param inode the deleted contentlet inode + * @param binaryPaths the inode's stored binary paths at deletion time + * @return the metadata paths to delete, each starting with {@code /} + * @throws DotDataException if metadata cannot be listed or a path escapes the inode + */ + @Override + public List listMetadataForInode(final String inode, final List binaryPaths) throws DotDataException { + if (!AssetStorageFeature.isEnabled()) { + return List.of(); + } + final String prefix = metadataInodePrefix(inode); + final String group = Config.getStringProperty(METADATA_GROUP_NAME, DOT_METADATA); + final StoragePersistenceAPI storage = StoragePersistenceProvider.INSTANCE.get() + .getStorage(StoragePersistenceProvider.getStorageType()); + final Set paths = new LinkedHashSet<>(); + // Legacy metadata can exist even when its field/source no longer exists. + for (final String path : storage.listObjectPaths(group, "/" + prefix)) { + if (path.endsWith(METADATA_JSON)) { + paths.add(path.startsWith("/") ? path : "/" + path); + } + } + for (final String path : binaryPaths) { + if (!path.startsWith(prefix) || !Path.of(path).normalize().toString().equals(path)) { + throw new DotDataException("Binary path escapes metadata cleanup inode: " + path); + } + final String relative = path.substring(prefix.length()); + final int slash = relative.indexOf('/'); + if (slash >= 0) { + paths.add("/" + prefix + relative.substring(0, slash) + METADATA_JSON); + if (relative.contains("/.revisions/")) { + paths.add("/" + path + METADATA_JSON); + final String metadataParent = "/" + path.substring(0, path.lastIndexOf('/') + 1); + for (final String metadataPath : storage.listObjectPaths(group, metadataParent)) { + if (metadataPath.endsWith(METADATA_JSON)) { + paths.add(metadataPath.startsWith("/") ? metadataPath : "/" + metadataPath); + } + } + } + } + } + for (final String path : paths) { + requireMetadataPathOf(prefix, path); + } + return List.copyOf(paths); + } + + /** + * Deletes exactly the metadata paths recorded for a deleted inode and evicts them from the + * metadata cache. With the flag off it does nothing. See {@link FileMetadataAPI#removeMetadataPaths}. + * + * @param inode the deleted contentlet inode + * @param metadataPaths the recorded metadata paths, each under the inode's folder + * @throws DotDataException if a path escapes the inode or remains after deletion + */ + @Override + public void removeMetadataPaths(final String inode, final List metadataPaths) throws DotDataException { + if (!AssetStorageFeature.isEnabled()) { + return; + } + final String prefix = metadataInodePrefix(inode); + for (final String path : metadataPaths) { + requireMetadataPathOf(prefix, path); + } + final String group = Config.getStringProperty(METADATA_GROUP_NAME, DOT_METADATA); + final StoragePersistenceAPI storage = StoragePersistenceProvider.INSTANCE.get() + .getStorage(StoragePersistenceProvider.getStorageType()); + for (final String path : metadataPaths) { + storage.deleteObjectAndReferences(group, path); + if (storage.existsObject(group, path)) { + throw new DotDataException("Metadata remains after deletion: " + path); + } + metadataCache.removeMetadata(path); + } + } + + /** + * Returns the storage prefix {@code {a}/{b}/{inode}/} that owns an inode's metadata. + * + * @param inode the contentlet inode + * @return the prefix, without a leading slash + * @throws IllegalArgumentException if the inode could escape the asset layout + */ + private static String metadataInodePrefix(final String inode) { + if (inode == null || !inode.matches("[A-Za-z0-9_-]{2,}")) { + throw new IllegalArgumentException("Invalid metadata inode"); + } + return inode.charAt(0) + "/" + inode.charAt(1) + "/" + inode + "/"; + } + + /** + * Rejects a metadata path that is not a normalized path under the inode's folder, so a recorded + * or listed path can never delete another inode's metadata. + * + * @param prefix the inode prefix from {@link #metadataInodePrefix} + * @param path the metadata path, starting with {@code /} + * @throws DotDataException if the path escapes the inode */ + private static void requireMetadataPathOf(final String prefix, final String path) throws DotDataException { + if (path == null || !path.startsWith("/" + prefix) || !Path.of(path).normalize().toString().equals(path)) { + throw new DotDataException("Metadata path escapes cleanup inode: " + path); + } + } + + /** Removes metadata associated with a contentlet's binary fields and local metadata paths. */ public Map> removeMetadata(final Contentlet contentlet) { final Map> removedMetaPaths = new HashMap<>(); final StorageType storageType = StoragePersistenceProvider.getStorageType(); @@ -739,15 +909,39 @@ public Metadata getFullMetadataNoCache(final File binary, final Supplier> customAttributesByField) throws DotDataException { + if (AssetStorageFeature.isEnabled()) { + publishMetadata(contentlet, customAttributesByField.keySet(), (snapshot, field) -> { + final Metadata previous = getFullMetadataNoCache(snapshot, field); + final Map metadata = previous == null ? new HashMap<>() + : new HashMap<>(previous.getMap()); + final Map attributes = customAttributesByField.get(field); + if (attributes.isEmpty()) { + metadata.keySet().removeIf(key -> key.startsWith(Metadata.CUSTOM_PROP_PREFIX)); + } else { + attributes.forEach((key, value) -> metadata.put(Metadata.CUSTOM_PROP_PREFIX + key, value)); + } + return metadata; + }); + return; + } + putCustomMetadataAttributesForCheckin(contentlet, customAttributesByField); + } + + @Override + public void putCustomMetadataAttributesForCheckin(final Contentlet contentlet, + final Map> customAttributesByField) throws DotDataException { + final StorageType storageType = StoragePersistenceProvider.getStorageType(); final String metadataBucketName = Config .getStringProperty(METADATA_GROUP_NAME, DOT_METADATA); - customAttributesByField.forEach((fieldName, customAttributes) -> { + for (final var entry : customAttributesByField.entrySet()) { + final String fieldName = entry.getKey(); + final Map customAttributes = entry.getValue(); final String metadataPath = getFileName(contentlet, fieldName); try { fileStorageAPI.putCustomMetadataAttributes((new FetchMetadataParams.Builder() - .cache(() -> contentlet.getInode() + StringPool.COLON + fieldName) + .cache(() -> getMetadataCacheKey(contentlet, fieldName)) .projectionMapForCache(this::filterNonBasicMetadataFields) .forceInsert(true) .storageKey( @@ -757,12 +951,113 @@ public void putCustomMetadataAttributes(final Contentlet contentlet, .build()), customAttributes); }catch (Exception e){ + if (AssetStorageFeature.isEnabled()) { + throw new DotDataException("Unable to save binary metadata for " + fieldName, e); + } Logger.error(FileMetadataAPIImpl.class, "Error saving custom attributes", e); } - }); + } } + /** + * Only the database reference is mutable. Upload failures and rollbacks leave committed + * metadata intact; the row lock makes concurrent custom-attribute merges read the latest edit. + */ + @CloseDBIfOpened + private void publishMetadata(final Contentlet contentlet, + final Set updatedFields, + final io.vavr.CheckedFunction2> update) throws DotDataException { + publishMetadata(contentlet, updatedFields, update, null); + } + + private void publishMetadata(final Contentlet contentlet, + final Set updatedFields, + final io.vavr.CheckedFunction2> update, + final Contentlet expectedSnapshot) throws DotDataException { + final boolean skipChangedSnapshot = expectedSnapshot != null; + if (updatedFields.isEmpty()) { + return; + } + final boolean localTransaction = HibernateUtil.startLocalTransactionIfNeeded(); + try { + final String inode = contentlet.getInode(); + final String previousJson = new DotConnect() + .setSQL("select contentlet_as_json from contentlet where inode = ? for update") + .addParam(inode).getString("contentlet_as_json"); + final boolean missingJson = previousJson == null || previousJson.isBlank(); + if (missingJson && !skipChangedSnapshot) { + throw new DotDataException("Cannot edit metadata for missing content: " + inode); + } + final ObjectMapper mapper = new ObjectMapper(); + final ObjectNode json = (ObjectNode) mapper.readTree(missingJson ? "{\"fields\":{}}" : previousJson); + final ObjectNode fields = (ObjectNode) json.path("fields"); + final Contentlet snapshot = new Contentlet(contentlet); + final Map references = new HashMap<>(); + final Set binaryFields = contentlet.getContentType().fields(BinaryField.class).stream() + .map(Field::variable).collect(Collectors.toSet()); + for (final String field : updatedFields) { + if (!binaryFields.contains(field) + || !((skipChangedSnapshot ? expectedSnapshot : contentlet).get(field) instanceof File)) { + throw new DotDataException("Cannot edit metadata for an absent binary field: " + field); + } + final BinaryAssetReference.StoredBinary stored = BinaryAssetReference.fromJson(fields.path(field), inode, field); + final File current = stored == null ? null : stored.localFile(inode, field); + final File requested = (File) (skipChangedSnapshot ? expectedSnapshot : contentlet).get(field); + if (current == null || !current.toPath().toAbsolutePath().normalize() + .equals(requested.toPath().toAbsolutePath().normalize())) { + if (skipChangedSnapshot) { + continue; + } + throw new DotDataException("Binary changed before metadata edit: " + field); + } + snapshot.getMap().put(field, current); + if (skipChangedSnapshot && !getFileName(snapshot, field).equals(getFileName(expectedSnapshot, field))) { + continue; + } + final Map metadata = Try.of(() -> update.apply(snapshot, field)) + .getOrElseThrow(DotDataException::new); + if (metadata == null) { + continue; + } + final String key = BinaryAssetReference.newMetadataKey(current, inode, field); + final boolean storedMetadata = fileStorageAPI.setMetadata(new FetchMetadataParams.Builder() + .cache(() -> key) + .projectionMapForCache(this::filterNonBasicMetadataFields) + .storageKey(new StorageKey.Builder() + .group(Config.getStringProperty(METADATA_GROUP_NAME, DOT_METADATA)) + .path(key).storage(StoragePersistenceProvider.getStorageType()).build()) + .build(), metadata); + if (!storedMetadata) { + throw new DotDataException("Metadata was not stored: " + field); + } + ((ObjectNode) fields.path(field)).put("metadataStorageKey", key); + references.put(field, BinaryAssetReference.withMetadata(current, inode, field, key)); + } + if (!references.isEmpty()) { + final String updatedJson = mapper.writeValueAsString(json); + new DotConnect().setSQL("update contentlet set contentlet_as_json = " + + (DbConnectionFactory.isPostgres() ? "?::jsonb" : "?") + " where inode = ?") + .addParam(updatedJson).addParam(inode).loadResult(); + // ContentletCache may share the supplied object with other requests. Do not mutate + // it before commit. A writer can find the content again to read its pending reference. + HibernateUtil.addSyncCommitListener(() -> { + CacheLocator.getContentletCache().remove(inode); + references.forEach((field, file) -> contentlet.getMap().put(field, file)); + contentlet.getMap().put(Contentlet.CONTENTLET_AS_JSON, updatedJson); + }); + } + if (localTransaction) { + HibernateUtil.commitTransaction(); + } + } catch (Exception e) { + if (localTransaction) { + HibernateUtil.rollbackTransaction(); + } + throw new DotDataException("Unable to publish binary metadata", e); + } + } + /** * Build a tmp resource path so that temp files will get created under a more suitable location */ @@ -841,6 +1136,39 @@ public Optional getMetadata(final String tempResourceId) */ public void copyCustomMetadata(final Contentlet source, final Contentlet destination) throws DotDataException { + if (!AssetStorageFeature.isEnabled()) { + copyCustomMetadataForCheckin(source, destination); + return; + } + if (!source.getContentType().baseType().equals(destination.getContentType().baseType())) { + throw new DotDataException("Source and destination contentlet are not the same type."); + } + final Map> copiedAttributes = new HashMap<>(); + for (final Field field : source.getContentType().fields(BinaryField.class)) { + final String name = field.variable(); + if (source.get(name) == null || destination.get(name) == null + || getFileName(source, name).equals(getFileName(destination, name))) { + continue; + } + final Metadata metadata = getFullMetadataNoCache(source, name); + if (metadata != null) { + copiedAttributes.put(name, metadata.getCustomMetaWithPrefix()); + } + } + publishMetadata(destination, copiedAttributes.keySet(), (snapshot, field) -> { + final Metadata previous = getFullMetadataNoCache(snapshot, field); + final Map metadata = previous == null ? new HashMap<>() + : new HashMap<>(previous.getMap()); + metadata.keySet().removeIf(key -> key.startsWith(Metadata.CUSTOM_PROP_PREFIX)); + metadata.putAll(copiedAttributes.get(field)); + return previous == null && metadata.isEmpty() + || previous != null && metadata.equals(previous.getMap()) ? null : metadata; + }); + } + + @Override + public void copyCustomMetadataForCheckin(final Contentlet source, final Contentlet destination) + throws DotDataException { if (!source.getContentType().baseType().equals(destination.getContentType().baseType())) { throw new DotDataException("Source and destination contentlet are not the same type."); } @@ -858,6 +1186,10 @@ public void copyCustomMetadata(final Contentlet source, final Contentlet destina for (final String binaryFieldName : binaryFieldNames) { final String sourceMetadataPath = getFileName(source, binaryFieldName); + if (AssetStorageFeature.isEnabled() && (destination.get(binaryFieldName) == null + || sourceMetadataPath.equals(getFileName(destination, binaryFieldName)))) { + continue; + } final Map metadataMap = fileStorageAPI.retrieveMetaData( new FetchMetadataParams.Builder() .cache(false) @@ -880,8 +1212,7 @@ public void copyCustomMetadata(final Contentlet source, final Contentlet destina ); } else { fileStorageAPI.setMetadata(new FetchMetadataParams.Builder() - .cache(() -> destination.getInode() + StringPool.COLON - + binaryFieldName) + .cache(() -> getMetadataCacheKey(destination, binaryFieldName)) .projectionMapForCache(this::filterNonBasicMetadataFields) .storageKey( new StorageKey.Builder().group(metadataBucketName) @@ -902,6 +1233,17 @@ public void copyCustomMetadata(final Contentlet source, final Contentlet destina */ @Override public void setMetadata(final Contentlet contentlet, final Map binariesMetadata) throws DotDataException { + if (AssetStorageFeature.isEnabled()) { + final Map> updates = new HashMap<>(); + for (final Field field : contentlet.getContentType().fields(BinaryField.class)) { + final Metadata metadata = binariesMetadata.get(field.variable()); + if (contentlet.get(field.variable()) != null && metadata != null) { + updates.put(field.variable(), metadata.getMap()); + } + } + publishMetadata(contentlet, updates.keySet(), (snapshot, field) -> updates.get(field)); + return; + } removeMetadata(contentlet); final Set validFields = contentlet.getContentType().fields(BinaryField.class).stream() .filter(field -> contentlet.get(field.variable()) != null) @@ -914,7 +1256,7 @@ public void setMetadata(final Contentlet contentlet, final Map if(null != metadata){ final String destMetadataPath = getFileName(contentlet, validField.variable()); fileStorageAPI.setMetadata(new FetchMetadataParams.Builder() - .cache(() -> contentlet.getInode() + StringPool.COLON + validField.variable()) + .cache(() -> getMetadataCacheKey(contentlet, validField.variable())) .projectionMapForCache(this::filterNonBasicMetadataFields) .storageKey( new StorageKey.Builder().group(metadataBucketName) diff --git a/dotCMS/src/main/java/com/dotcms/storage/MetadataGeneratorImpl.java b/dotCMS/src/main/java/com/dotcms/storage/MetadataGeneratorImpl.java index 57291f9cec57..2a1598cfd0ff 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/MetadataGeneratorImpl.java +++ b/dotCMS/src/main/java/com/dotcms/storage/MetadataGeneratorImpl.java @@ -57,6 +57,12 @@ public Map tikaBasedMetadata(final File binary, final long try { final TikaUtils tikaUtils = new TikaUtils(); + if (AssetStorageFeature.isEnabled() && tikaUtils.extractorVersion() != null) { + return SharedExtractedMetadata.getInstance().get(binary, tikaUtils.extractorVersion(), + com.dotmarketing.business.APILocator.getFileMetadataAPI().getBinaryMetadataVersion(), + (int) maxLength, snapshot -> serializable(tikaUtils.getForcedMetaDataMap(snapshot, (int) maxLength))); + } + final Map metaDataMap = tikaUtils .getForcedMetaDataMap(binary, (int) maxLength); return metaDataMap.entrySet().stream() @@ -64,11 +70,20 @@ public Map tikaBasedMetadata(final File binary, final long collect(Collectors.toMap(Entry::getKey, e -> (Serializable) e.getValue())); } catch (Exception e) { + if (AssetStorageFeature.isEnabled() && (e instanceof com.dotmarketing.exception.DotDataException + || e instanceof java.io.IOException)) { + throw new com.dotmarketing.exception.DotRuntimeException("Unable to read or publish shared metadata", e); + } Logger.warnAndDebug(MetadataGeneratorImpl.class, e.getMessage(), e); } return ImmutableMap.of(); } + private static Map serializable(Map values) { + return values.entrySet().stream().filter(entry -> entry.getValue() instanceof Serializable) + .collect(Collectors.toMap(Entry::getKey, entry -> (Serializable) entry.getValue())); + } + @Override public TreeMap standAloneMetadata(final File binary){ final TreeMap metadataMap = new TreeMap<>(Comparator.naturalOrder()); diff --git a/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryAssetCleanupProcessor.java b/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryAssetCleanupProcessor.java new file mode 100644 index 000000000000..5a9a0f0ed53f --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryAssetCleanupProcessor.java @@ -0,0 +1,195 @@ +package com.dotcms.storage.binary; + +import com.dotcms.business.CloseDBIfOpened; +import com.dotcms.jobs.business.error.JobProcessingException; +import com.dotcms.jobs.business.error.JobValidationException; +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.jobs.business.processor.Validator; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.FileMetadataAPI; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.db.HibernateUtil; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.util.Logger; +import com.liferay.util.FileUtil; +import java.io.File; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import javax.enterprise.context.Dependent; + +/** Cleanup intent is committed with the content deletion and survives failed S3 requests/restarts. */ +@Dependent +@Queue(BinaryAssetCleanupProcessor.QUEUE) +@ExponentialBackoffRetryPolicy +public class BinaryAssetCleanupProcessor implements JobProcessor, Validator { + public static final String QUEUE = "binaryAssetCleanup"; + + /** + * Records the cleanup of a deleted inode's binaries and metadata in the caller's deletion + * transaction. The job carries the exact binary and metadata paths stored for the inode at this + * moment, so the worker later deletes only those. An inode can be re-created afterwards (push + * publishing keeps the sender's inodes), and revisions it uploads must survive this cleanup. + * With the flag off this does nothing. + * + * @param inode the deleted contentlet inode + * @throws DotDataException if there is no transaction, storage cannot be listed, or the job + * cannot be recorded; the caller's deletion then rolls back + */ + public static void enqueue(final String inode) throws DotDataException { + if (!AssetStorageFeature.isEnabled()) { + return; + } + validateInode(inode); + if (!DbConnectionFactory.inTransaction()) { + throw new DotDataException("Binary cleanup must be recorded in the content deletion transaction"); + } + final List binaries = List.copyOf(APILocator.getBinaryAssetStorageAPI().listBinaryPaths(inode)); + final List metadata = List.copyOf(APILocator.getFileMetadataAPI().listMetadataForInode(inode, binaries)); + APILocator.getJobQueueManagerAPI().createJob(QUEUE, + Map.of("inode", inode, "binaries", binaries, "metadata", metadata)); + } + + /** + * Registers a rollback listener that deletes a revision a check-in has just uploaded, together + * with its revision metadata. The key carries a fresh UUID, so no other content version can + * reference it, and an uncommitted inode is never reached by whole-inode cleanup. A failed delete + * is logged and the object is left behind, because a rollback listener must not throw. + * Outside a transaction no listener is registered. + * + * @param inode the inode the revision was uploaded under + * @param field the binary field variable + * @param key the uploaded revision key, or {@code null} when the file is not a revision + */ + public static void deleteRevisionOnRollback(final String inode, final String field, final String key) { + if (key == null) { + return; + } + HibernateUtil.addRollbackListener(() -> { + try { + APILocator.getFileMetadataAPI().removeMetadataPaths(inode, + List.of("/" + key + FileMetadataAPI.METADATA_JSON)); + APILocator.getBinaryAssetStorageAPI().deleteBinaryPaths(inode, field, List.of(key)); + } catch (Exception e) { + Logger.warn(BinaryAssetCleanupProcessor.class, + "Unable to delete binary revision " + key + " after rollback: " + e.getMessage()); + } + }); + } + + private static void validateInode(final String inode) { + if (inode == null || !inode.matches("[A-Za-z0-9_-]{2,}")) { + throw new IllegalArgumentException("Invalid binary cleanup inode"); + } + } + + /** + * Rejects submissions through the public job endpoint, which always injects {@code userId}. + * Only content deletion records this work, through {@link #enqueue(String)}. + * + * @param parameters the job parameters + * @throws JobValidationException if the flag is off or the job came from the public endpoint + */ + @Override + public void validate(final Map parameters) throws JobValidationException { + if (!AssetStorageFeature.isEnabled() || parameters.containsKey("userId")) { + throw new JobValidationException("Binary cleanup is internal to enabled content deletion"); + } + } + + /** + * Deletes the binaries and metadata recorded for a deleted inode, refusing while any content + * version with that inode exists. Metadata goes first, then each recorded binary path, then the + * inode's renditions and legacy image cache. Nothing outside the recorded inventory is deleted, so + * revisions uploaded after the deletion survive. Each step tolerates paths that are already gone, + * so a retry after a partial failure finishes the work. + * + * @param job the cleanup job, carrying {@code inode}, {@code binaries} and {@code metadata} + * @throws JobProcessingException if the flag is off, the version exists, or a deletion fails; + * the queue retries it + */ + @Override + @CloseDBIfOpened + public void process(final Job job) throws JobProcessingException { + if (!AssetStorageFeature.isEnabled()) { + // Leave a failed/retryable job rather than losing pending S3 cleanup when disabled. + throw new JobProcessingException(job.id(), "S3 asset storage is disabled"); + } + final String inode = (String) job.parameters().get("inode"); + validateInode(inode); + try { + final List binaryPaths = strings(job.parameters().get("binaries")); + final List metadataPaths = strings(job.parameters().get("metadata")); + if (!new DotConnect().setSQL("select inode from contentlet where inode = ?") + .addParam(inode).loadObjectResults().isEmpty()) { + throw new JobProcessingException(job.id(), "Content version still exists: " + inode); + } + final BinaryAssetStorageAPI binaries = APILocator.getBinaryAssetStorageAPI(); + APILocator.getFileMetadataAPI().removeMetadataPaths(inode, metadataPaths); + for (final Map.Entry> field : byField(inode, binaryPaths).entrySet()) { + binaries.deleteBinaryPaths(inode, field.getKey(), field.getValue()); + } + binaries.deleteGeneratedFiles(inode); + final File legacyCache = new File(APILocator.getFileAssetAPI().getRealAssetsRootPath(), + "cache/" + inode.charAt(0) + "/" + inode.charAt(1) + "/" + inode); + FileUtil.deltree(legacyCache); + if (legacyCache.exists()) { + throw new DotDataException("Unable to remove legacy cache for " + inode); + } + } catch (DotDataException | RuntimeException e) { + throw new JobProcessingException(job.id(), "Unable to clean binary assets for " + inode, e); + } + } + + /** + * Groups recorded binary paths by their field folder, for the per-field delete API. Paths directly + * under the inode folder have no field; on a local layer they are the legacy metadata files that + * the metadata step has already deleted, so they are skipped. A path outside the inode is rejected. + * + * @param inode the deleted inode + * @param paths the recorded binary paths + * @return the paths keyed by field variable, in recorded order + * @throws DotDataException if a path is not under the inode's folder + */ + private static Map> byField(final String inode, final List paths) + throws DotDataException { + final String prefix = inode.charAt(0) + "/" + inode.charAt(1) + "/" + inode + "/"; + final Map> fields = new LinkedHashMap<>(); + for (final String path : paths) { + if (!path.startsWith(prefix)) { + throw new DotDataException("Binary cleanup path escapes its inode: " + path); + } + final int slash = path.indexOf('/', prefix.length()); + if (slash > prefix.length()) { + fields.computeIfAbsent(path.substring(prefix.length(), slash), field -> new ArrayList<>()) + .add(path); + } + } + return fields; + } + + /** + * Reads a recorded path list from the job parameters. + * + * @param value the parameter value + * @return the paths + * @throws DotDataException if the job carries no inventory, as jobs recorded before it did + */ + private static List strings(final Object value) throws DotDataException { + if (!(value instanceof List)) { + throw new DotDataException("Binary cleanup job has no recorded inventory"); + } + return ((List) value).stream().map(String.class::cast).toList(); + } + + @Override + public Map getResultMetadata(final Job job) { + return Map.of("inode", job.parameters().get("inode")); + } +} diff --git a/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryAssetReference.java b/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryAssetReference.java index 026c343f3790..e8d4fdda5942 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryAssetReference.java +++ b/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryAssetReference.java @@ -251,6 +251,21 @@ public static File localFile(final String inode, final String field, final Strin return new File(ConfigUtils.getAssetPath(), key); } + /** The contentlet inode and field variable name that own a revision key. */ + public record Owner(String inode, String field) { } + + /** + * Reads the owner out of a revision key with the {@code i/n///.revisions//} + * layout. This only parses the key; {@link #localFile(String, String, String)} still validates it. + * + * @param key the revision key, relative to the asset root + * @return the owner named by the key, or null when the key does not have the revision layout + */ + public static Owner ownerOf(final String key) { + final String[] parts = key.split("/", -1); + return parts.length == 7 && ".revisions".equals(parts[4]) ? new Owner(parts[2], parts[3]) : null; + } + /** Pure path conversion: JSON serialization must not stat NFS or download an object. */ public static String keyOf(final File file, final String inode, final String field) { final String key = keyOf(file); diff --git a/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryFieldCleanupProcessor.java b/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryFieldCleanupProcessor.java new file mode 100644 index 000000000000..9b5c2802b4e5 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/storage/binary/BinaryFieldCleanupProcessor.java @@ -0,0 +1,289 @@ +package com.dotcms.storage.binary; + +import com.dotcms.business.CloseDBIfOpened; +import com.dotcms.jobs.business.error.JobProcessingException; +import com.dotcms.jobs.business.error.JobValidationException; +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.jobs.business.processor.Validator; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.FileMetadataAPI; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotcms.storage.StoragePersistenceProvider; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.CacheLocator; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.db.HibernateUtil; +import com.dotmarketing.db.LocalTransaction; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.util.Config; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import java.nio.file.Path; +import java.util.Date; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import javax.enterprise.context.Dependent; + +/** Field removal and its archived, exact cleanup inventory use the existing transactional queue. */ +@Dependent +@Queue(BinaryFieldCleanupProcessor.QUEUE) +@ExponentialBackoffRetryPolicy +public class BinaryFieldCleanupProcessor implements JobProcessor, Validator { + public static final String QUEUE = "binaryFieldCleanup"; + private static final ObjectMapper JSON = new ObjectMapper(); + + @Override public void validate(Map parameters) throws JobValidationException { + // The public job endpoint always injects userId; only the authorized field API records this work. + if (!AssetStorageFeature.isEnabled() || parameters.containsKey("userId")) { + throw new JobValidationException("Field cleanup is internal to enabled content field deletion"); + } + } + + public static void enqueue(String type, String field, Date deletedBefore) throws DotDataException { + requireTransaction(); + prefix(type, field); + APILocator.getJobQueueManagerAPI().createJob(QUEUE, + Map.of("type", type, "field", field, "deletedBefore", deletedBefore.getTime())); + } + + private static void requireTransaction() throws DotDataException { + if (!AssetStorageFeature.isEnabled() || !DbConnectionFactory.inTransaction()) { + throw new DotDataException("S3 field cleanup requires a transaction and the feature flag"); + } + } + + private static String prefix(String inode, String field) { + new BinaryAssetReference.StoredBinary(null, null, "inventory").localFile(inode, field); + return inode.charAt(0) + "/" + inode.charAt(1) + "/" + inode + "/" + field; + } + + private static StoragePersistenceAPI metadataStorage() { + return StoragePersistenceProvider.INSTANCE.get().getStorage(StoragePersistenceProvider.getStorageType()); + } + + private static String metadataGroup() { + return Config.getStringProperty(StoragePersistenceProvider.METADATA_GROUP_NAME, FileMetadataAPI.DOT_METADATA); + } + + /** + * Archives the removed field across every version of a content type's contentlets, one row per + * transaction. Each transaction locks only its row, rechecks that the row was not edited after the + * field was removed, archives it, and advances the job's committed {@code afterInode} cursor, so a + * retry resumes after the last archived row and no lock is held while other rows upload. + * + * @param job the per-type cleanup job, carrying {@code type}, {@code field} and {@code deletedBefore} + * @throws DotDataException if a row cannot be archived or the cursor was changed by another worker + */ + private static void cleanType(final Job job) throws Exception { + if (DbConnectionFactory.inTransaction()) { + throw new DotDataException("Field cleanup commits each row on its own and cannot join a transaction"); + } + // The manager may retry an older Job instance. Always reload the committed cursor. + CacheLocator.getJobCache().remove(job); + final Job current = APILocator.getJobQueueManagerAPI().getJob(job.id()); + if (current == null || !QUEUE.equals(current.queueName())) { + throw new DotDataException("Field cleanup job does not exist"); + } + final var parameters = current.parameters(); + final String type = (String) parameters.get("type"); + final String field = (String) parameters.get("field"); + final Date deletedBefore = new Date(((Number) parameters.get("deletedBefore")).longValue()); + prefix(type, field); + String after = (String) parameters.getOrDefault("afterInode", ""); + while (true) { + final var candidates = new DotConnect().setSQL("select inode from contentlet where structure_inode = ? " + + "and inode > ? and mod_date <= ? order by inode limit 100") + .addParam(type).addParam(after).addParam(deletedBefore).loadObjectResults(); + if (candidates.isEmpty()) return; + for (var candidate : candidates) { + final String inode = candidate.get("inode").toString(); + final String previous = after; + try { + LocalTransaction.wrapReturn(() -> { + // The lock and timestamp recheck protect an edit made since the candidate scan. + final var rows = new DotConnect().setSQL("select inode, identifier, contentlet_as_json " + + "from contentlet where inode = ? and mod_date <= ? for update") + .addParam(inode).addParam(deletedBefore).loadObjectResults(); + if (!rows.isEmpty()) archiveRow(rows.get(0), field); + // Compare the cursor so an overlapping worker cannot overwrite newer progress. + final var saved = new DotConnect().setSQL("update job set parameters = parameters || ?::jsonb, " + + "updated_at = current_timestamp where id = ? and queue_name = ? " + + "and coalesce(parameters->>'afterInode', '') = ? returning id") + .addParam(JSON.writeValueAsString(Map.of("afterInode", inode))) + .addParam(job.id()).addParam(QUEUE).addParam(previous).loadObjectResults(); + if (saved.size() != 1) { + throw new DotDataException("Field cleanup cursor changed or job was removed"); + } + return null; + }); + } catch (Exception failure) { + throw new DotDataException("Unable to preserve removed binary field " + inode + "/" + field, failure); + } + CacheLocator.getJobCache().remove(job); + after = inode; + } + } + } + + /** + * Archives and clears the removed field on one contentlet row that the caller has locked in the + * current transaction: it uploads a verified recovery ZIP of the row's field bytes and metadata, + * removes the field from the row's JSON, and records the per-inode cleanup job. + * + * @param row the locked row, with {@code inode}, {@code identifier} and {@code contentlet_as_json} + * @param field the removed field's variable name + * @throws Exception if the archive cannot be verified or the row cannot be updated + */ + static void archiveRow(final Map row, final String field) throws Exception { + final String inode = row.get("inode").toString(); + final Object value = row.get("contentlet_as_json"); + final String json = value == null || value.toString().isBlank() + ? APILocator.getContentletJsonAPI().toJson(APILocator.getContentletAPI() + .find(inode, APILocator.systemUser(), false)) : value.toString(); + final ObjectNode document = (ObjectNode) JSON.readTree(json); + final var storedField = document.path("fields").path(field); + if (!storedField.isMissingNode() && !"Binary".equals(storedField.path("type").asText())) return; + final String owner = prefix(inode, field); + final var binaryAPI = APILocator.getBinaryAssetStorageAPI(); + final List binaries = binaryAPI.listBinaryPaths(inode).stream() + .filter(path -> path.startsWith(owner + "/")).toList(); + final var reference = BinaryAssetReference.fromJson(document.path("fields").path(field), inode, field); + if (reference != null) { + final String expected = reference.storageKey() == null + ? owner + "/" + reference.fileName() : reference.storageKey(); + if (!binaries.contains(expected)) throw new DotDataException("Missing binary before field backup: " + expected); + } + final Set metadata = new LinkedHashSet<>(); + final Set parents = new LinkedHashSet<>(); + final String legacyParent = "/" + owner.substring(0, owner.lastIndexOf('/') + 1); + parents.add(legacyParent); + for (String path : binaries) parents.add("/" + path.substring(0, path.lastIndexOf('/') + 1)); + for (String parent : parents) { + // Local revision directories contain originals too. Their complete contents + // are already in the physical inventory; only S3 can distinguish the groups. + final var storage = parent.equals(legacyParent) ? metadataStorage() + : StoragePersistenceProvider.INSTANCE.get().getStorage(StoragePersistenceProvider.remoteStorageType()); + for (String path : storage.listObjectPaths(metadataGroup(), parent)) { + final String absolute = path.startsWith("/") ? path : "/" + path; + if (parent.equals(legacyParent) && absolute.substring(parent.length()).contains("/")) continue; + if (ownsMetadata(owner, absolute)) metadata.add(absolute); + } + } + if (reference != null && reference.metadataStorageKey() != null) metadata.add(reference.metadataStorageKey()); + if (binaries.isEmpty() && metadata.isEmpty() && reference == null) return; + final String backup = ContentletBackupStorage.getInstance().storeField( + row.get("identifier").toString(), inode, json, binaries, List.copyOf(metadata)); + ((ObjectNode) document.path("fields")).remove(field); + new DotConnect().setSQL("update contentlet set contentlet_as_json = ?::jsonb where inode = ?") + .addParam(JSON.writeValueAsString(document)).addParam(inode).loadResult(); + APILocator.getJobQueueManagerAPI().createJob(QUEUE, Map.of("inode", inode, "field", field, + "backup", backup, "binaries", binaries, "metadata", List.copyOf(metadata))); + CacheLocator.getContentletCache().remove(inode); + HibernateUtil.addCommitListener(() -> CacheLocator.getContentletCache().remove(inode)); + final var snapshot = APILocator.getContentletAPI().find(inode, APILocator.systemUser(), false); + new com.dotcms.rendering.velocity.services.ContentletLoader().invalidate(snapshot); + } + + private static boolean ownsMetadata(String owner, String path) { + // Filesystem metadata historically folds case; S3 metadata can retain its logical spelling. + final String prefix = ("/" + owner).toLowerCase(java.util.Locale.ROOT); + final String normalized = path.toLowerCase(java.util.Locale.ROOT); + return path.endsWith(FileMetadataAPI.METADATA_JSON) + && (normalized.startsWith(prefix + "/") || normalized.startsWith(prefix + ".") + || normalized.equals(prefix + FileMetadataAPI.METADATA_JSON)) + && Path.of(path).normalize().toString().equals(path); + } + + /** + * Runs a cleanup job. A per-type job archives the removed field one row per transaction (see + * {@link #cleanType(Job)}); a per-inode job verifies its archived inventory and deletes it in a + * single transaction. + * + * @param job the cleanup job + * @throws JobProcessingException if archiving or deletion fails; the queue retries it + */ + @Override + @CloseDBIfOpened + public void process(Job job) throws JobProcessingException { + try { + validate(job.parameters()); + if (job.parameters().containsKey("type")) { + cleanType(job); + return; + } + LocalTransaction.wrapReturn(() -> { + cleanInode(job); + return null; + }); + } catch (Exception failure) { + throw new JobProcessingException(job.id(), "Unable to clean removed binary field", failure); + } + } + + /** + * Deletes one inode's archived field bytes and metadata once they are verified unreferenced and + * their recovery archive is still present. Runs inside the caller's transaction. + * + * @param job the per-inode cleanup job, carrying the exact inventory recorded at archive time + * @throws Exception if the inventory is invalid, still referenced, or cannot be deleted + */ + private static void cleanInode(final Job job) throws Exception { + requireTransaction(); + final var parameters = job.parameters(); + final String field = (String) parameters.get("field"); + final String inode = (String) parameters.get("inode"); + final String owner = prefix(inode, field); + final List binaries = strings(parameters.get("binaries")); + final List metadata = strings(parameters.get("metadata")); + for (String path : binaries) { + if (!path.startsWith(owner + "/") || path.contains("\\") + || !Path.of(path).normalize().toString().equals(path)) { + throw new DotDataException("Invalid field cleanup inventory"); + } + } + for (String path : metadata) if (!ownsMetadata(owner, path)) throw new DotDataException("Invalid metadata inventory"); + final var rows = new DotConnect().setSQL("select contentlet_as_json from contentlet where inode = ? for update") + .addParam(inode).loadObjectResults(); + for (var row : rows) { + final var fields = JSON.readTree(row.get("contentlet_as_json").toString()).path("fields").fields(); + while (fields.hasNext()) { + final var entry = fields.next(); + if (!"Binary".equals(entry.getValue().path("type").asText())) continue; + final var reference = BinaryAssetReference.fromJson(entry.getValue(), inode, entry.getKey()); + if (reference == null) continue; + final String key = reference.storageKey() == null + ? prefix(inode, entry.getKey()) + "/" + reference.fileName() : reference.storageKey(); + if (binaries.contains(key) || metadata.contains(reference.metadataStorageKey())) { + throw new DotDataException("Archived field bytes are referenced again; retain them"); + } + } + } + final String backup = (String) parameters.get("backup"); + if (!backup.contains("/" + inode + "/")) throw new DotDataException("Archive owner does not match cleanup"); + // An operator may have removed the archive since enqueue; retain sources in that case. + try (var ignored = ContentletBackupStorage.getInstance().open(backup)) { } + for (String path : metadata) { + metadataStorage().deleteObjectAndReferences(metadataGroup(), path); + if (metadataStorage().existsObject(metadataGroup(), path)) throw new DotDataException("Metadata deletion incomplete"); + CacheLocator.getMetadataCache().removeMetadata(path); + } + final var binariesAPI = APILocator.getBinaryAssetStorageAPI(); + binariesAPI.deleteBinaryPaths(inode, field, binaries); + binariesAPI.deleteGeneratedFiles(inode); + final var legacyCache = new java.io.File(APILocator.getFileAssetAPI().getRealAssetsRootPath(), "cache/" + owner); + com.liferay.util.FileUtil.deltree(legacyCache); + if (legacyCache.exists()) throw new DotDataException("Legacy field cache deletion incomplete"); + } + + private static List strings(Object value) { + return ((List) value).stream().map(String.class::cast).toList(); + } + + @Override public Map getResultMetadata(Job job) { return Map.of("field", job.parameters().get("field")); } +} diff --git a/dotCMS/src/main/java/com/dotcms/storage/binary/ContentletBackupStorage.java b/dotCMS/src/main/java/com/dotcms/storage/binary/ContentletBackupStorage.java new file mode 100644 index 000000000000..2dea04a861e8 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/storage/binary/ContentletBackupStorage.java @@ -0,0 +1,202 @@ +package com.dotcms.storage.binary; + +import com.dotcms.content.business.json.ContentletJsonHelper; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.FileMetadataAPI; +import com.dotcms.storage.StorageKey; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotcms.storage.StoragePersistenceProvider; +import com.dotcms.util.xstream.XStreamHandler; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.util.Config; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.File; +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.UUID; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +/** Self-contained, immutable recovery archives, independent of normal binary cleanup. */ +public final class ContentletBackupStorage { + public static final String GROUP = "deleted-content-backups"; + private StoragePersistenceAPI storage; + private boolean initialized; + + private static class Holder { + private static final ContentletBackupStorage INSTANCE = new ContentletBackupStorage(null); + } + + public static ContentletBackupStorage getInstance() { return Holder.INSTANCE; } + + public ContentletBackupStorage(StoragePersistenceAPI storage) { this.storage = storage; } + + private synchronized StoragePersistenceAPI storage() throws DotDataException { + if (!AssetStorageFeature.isEnabled()) throw new DotDataException("S3 asset storage is disabled"); + if (storage == null) storage = StoragePersistenceProvider.remoteObjectStorage(); + if (!initialized) { + storage.createGroup(GROUP); + initialized = true; + } + return storage; + } + + private static void validateId(String id) { + if (id == null || !id.matches("[A-Za-z0-9_-]{2,}")) throw new IllegalArgumentException("Invalid backup owner"); + } + + /** Lock the persisted snapshot until deletion commits; never move or remove its source files. */ + public String store(Contentlet requested) throws DotDataException { + if (!AssetStorageFeature.isEnabled() || !DbConnectionFactory.inTransaction()) { + throw new DotDataException("S3 content backup requires the content deletion transaction"); + } + validateId(requested.getInode()); + Path archive = null; + try { + final var rows = new DotConnect().setSQL("select contentlet_as_json from contentlet where inode = ? for update") + .addParam(requested.getInode()).loadObjectResults(); + if (rows.isEmpty()) throw new DotDataException("Content version disappeared before backup"); + final Object persisted = rows.getFirst().get("contentlet_as_json"); + final Contentlet content = persisted == null || persisted.toString().isBlank() + ? new Contentlet(APILocator.getContentletAPI().find(requested.getInode(), APILocator.systemUser(), false)) + : APILocator.getContentletJsonAPI().toMutableContentlet( + ContentletJsonHelper.INSTANCE.get().immutableFromJson(persisted.toString())); + validateId(content.getIdentifier()); + final String json = persisted == null || persisted.toString().isBlank() + ? APILocator.getContentletJsonAPI().toJson(content) : persisted.toString(); + final String key = content.getIdentifier() + "/" + content.getInode() + "/" + UUID.randomUUID() + ".zip"; + archive = Files.createTempFile("contentlet-backup-", ".zip"); + try (var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease(); + var zip = new ZipOutputStream(Files.newOutputStream(archive))) { + text(zip, "contentlet.json", json); + zip.putNextEntry(new ZipEntry("contentlet.xml")); + XStreamHandler.newXStreamInstance().toXML(content, zip); + zip.closeEntry(); + final var metadataAPI = APILocator.getFileMetadataAPI(); + final var mapper = new ObjectMapper(); + for (final var field : BinaryAssetReference.fromContentJson(json, content.getInode()).entrySet()) { + final var reference = field.getValue(); + final File binary = reference.localFile(content.getInode(), field.getKey()); + final String revision = reference.storageKey(); + final String path = revision == null + ? content.getInode().charAt(0) + "/" + content.getInode().charAt(1) + "/" + + content.getInode() + "/" + field.getKey() + "/" + binary.getName() + : revision; + zip.putNextEntry(new ZipEntry("assets/" + path)); + try (var input = APILocator.getBinaryAssetStorageAPI().openLocalFile(binary)) { input.transferTo(zip); } + zip.closeEntry(); + content.getMap().put(field.getKey(), binary); + final String metadataPath = metadataAPI.getFileName(content, field.getKey()); + final var metadata = APILocator.getFileStorageAPI().retrieveRawMetaData(new StorageKey.Builder() + .group(Config.getStringProperty(StoragePersistenceProvider.METADATA_GROUP_NAME, FileMetadataAPI.DOT_METADATA)) + .path(metadataPath).storage(StoragePersistenceProvider.getStorageType()).build()); + if (metadata != null) { + text(zip, "assets/" + metadataPath.substring(1).toLowerCase(java.util.Locale.ROOT), + mapper.writeValueAsString(metadata)); + } else if (BinaryAssetReference.metadataKeyOf(binary) != null) { + throw new IOException("Missing referenced backup metadata " + field.getKey()); + } + } + } + if (!storage().backfillFile(GROUP, key, archive.toFile())) { + throw new DotDataException("Recovery archive was not verified in S3"); + } + return key; + } catch (Exception failure) { + throw new DotDataException("Unable to back up content version " + requested.getInode(), failure); + } finally { + if (archive != null) { + try { Files.deleteIfExists(archive); } + catch (IOException failure) { com.dotmarketing.util.Logger.warn(ContentletBackupStorage.class, "Unable to remove backup staging file", failure); } + } + } + } + + private static void text(ZipOutputStream zip, String name, String value) throws IOException { + zip.putNextEntry(new ZipEntry(name)); + zip.write(value.getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + + /** Archive the exact field inventory while its owner row is locked by the caller. */ + String storeField(String identifier, String inode, String json, List binaries, + List metadata) throws DotDataException { + if (!AssetStorageFeature.isEnabled() || !DbConnectionFactory.inTransaction()) { + throw new DotDataException("Field backup requires the cleanup transaction"); + } + validateId(identifier); + validateId(inode); + Path archive = null; + try { + archive = Files.createTempFile("binary-field-backup-", ".zip"); + try (var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease(); + var zip = new ZipOutputStream(Files.newOutputStream(archive))) { + text(zip, "contentlet.json", json); + for (String path : binaries) { + zip.putNextEntry(new ZipEntry("binary-assets/" + path)); + try (var input = APILocator.getBinaryAssetStorageAPI().openLocalFile( + new File(com.dotmarketing.util.ConfigUtils.getAssetPath(), path))) { + input.transferTo(zip); + } + zip.closeEntry(); + } + for (String path : metadata) { + final var raw = APILocator.getFileStorageAPI().retrieveRawMetaData(new StorageKey.Builder() + .group(Config.getStringProperty(StoragePersistenceProvider.METADATA_GROUP_NAME, FileMetadataAPI.DOT_METADATA)) + .path(path).storage(StoragePersistenceProvider.getStorageType()).build()); + if (raw == null) throw new IOException("Missing field metadata " + path); + text(zip, "dotmetadata/" + path.substring(1), new ObjectMapper().writeValueAsString(raw)); + } + } + final String key = identifier + "/" + inode + "/" + UUID.randomUUID() + ".zip"; + if (!storage().backfillFile(GROUP, key, archive.toFile())) { + throw new DotDataException("Field recovery archive was not verified in S3"); + } + return key; + } catch (Exception failure) { + throw new DotDataException("Unable to archive binary field for " + inode, failure); + } finally { + if (archive != null) { + try { Files.deleteIfExists(archive); } + catch (IOException failure) { com.dotmarketing.util.Logger.warn(ContentletBackupStorage.class, + "Unable to remove field backup staging file", failure); } + } + } + } + + public List list(String identifier) throws DotDataException { + validateId(identifier); + return storage().listObjectPaths(GROUP, identifier + "/"); + } + + /** Closing the recovery stream releases its private download; no permanent local copy is required. */ + public InputStream open(String key) throws DotDataException, IOException { + if (key == null || !key.matches("[A-Za-z0-9_-]{2,}/[A-Za-z0-9_-]{2,}/[a-f0-9-]{36}\\.zip")) { + throw new IllegalArgumentException("Invalid recovery archive key"); + } + final StoragePersistenceAPI remote = storage(); + final File download = remote.pullFile(GROUP, key); + if (download == null) throw new IOException("Missing recovery archive"); + final InputStream input; + try { input = Files.newInputStream(download.toPath()); } + catch (IOException failure) { remote.releaseRetrievedFile(download); throw failure; } + return new FilterInputStream(input) { + @Override public void close() throws IOException { + try { super.close(); } + finally { + try { remote.releaseRetrievedFile(download); } + catch (DotDataException failure) { throw new IOException("Unable to release recovery download", failure); } + } + } + }; + } +} diff --git a/dotCMS/src/main/java/com/dotcms/util/marshal/JacksonMarshalUtilsImpl.java b/dotCMS/src/main/java/com/dotcms/util/marshal/JacksonMarshalUtilsImpl.java index 2812ace4ea37..c0d1132664ba 100644 --- a/dotCMS/src/main/java/com/dotcms/util/marshal/JacksonMarshalUtilsImpl.java +++ b/dotCMS/src/main/java/com/dotcms/util/marshal/JacksonMarshalUtilsImpl.java @@ -24,6 +24,10 @@ public class JacksonMarshalUtilsImpl implements MarshalUtils{ private final Lazy defaultMapper = Lazy.of(() -> { final ObjectMapper objectMapper = new ObjectMapper(); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + objectMapper.registerModule(new com.fasterxml.jackson.module.paramnames.ParameterNamesModule( + com.fasterxml.jackson.annotation.JsonCreator.Mode.PROPERTIES)); + } objectMapper.registerModule(new Jdk8Module()); objectMapper.registerModule(new GuavaModule()); objectMapper.registerModule(new JavaTimeModule().addSerializer(java.sql.Time.class, new SqlTimeStampSerializer())); diff --git a/dotCMS/src/main/java/com/dotcms/util/xstream/XStreamHandler.java b/dotCMS/src/main/java/com/dotcms/util/xstream/XStreamHandler.java index 398ef6bcdb4f..77105e38420b 100644 --- a/dotCMS/src/main/java/com/dotcms/util/xstream/XStreamHandler.java +++ b/dotCMS/src/main/java/com/dotcms/util/xstream/XStreamHandler.java @@ -19,6 +19,14 @@ public static XStream newXStreamInstance(final String encoding) { @Override protected MapperWrapper wrapMapper(final MapperWrapper next) { return new MapperWrapper(next) { + @Override + public String serializedClass(final Class type) { + // Metadata identity travels in the bundle metadata, not in a Java File subtype. + return super.serializedClass(com.dotcms.storage.AssetStorageFeature.isEnabled() + && com.dotcms.storage.binary.BinaryAssetReference.isMetadataFile(type) + ? java.io.File.class : type); + } + @Override public boolean shouldSerializeMember(final Class definedIn, final String fieldName) { @@ -34,6 +42,14 @@ public boolean shouldSerializeMember(final Class definedIn, } }; + xstream.registerConverter(new com.thoughtworks.xstream.converters.extended.FileConverter() { + @Override + public boolean canConvert(final Class type) { + return com.dotcms.storage.AssetStorageFeature.isEnabled() + && com.dotcms.storage.binary.BinaryAssetReference.isMetadataFile(type); + } + }); + //Allow only the classes that are in the trusted list xstream.allowTypesByWildcard(TrustedListMatcher.patterns); @@ -93,4 +109,4 @@ public static boolean matches(Class clazz) { } } -} \ No newline at end of file +} diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletAPIInterceptor.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletAPIInterceptor.java index 702b1276027f..c23971fa5a6a 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletAPIInterceptor.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletAPIInterceptor.java @@ -2571,7 +2571,16 @@ public long contentletIdentifierCount() throws DotDataException { public void deleteAllVersionsandBackup(List contentlets, User user, boolean respectFrontendRoles) throws DotDataException, DotSecurityException, DotContentletStateException { - // Not implemented + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) return; + for (ContentletAPIPreHook pre : preHooks) { + if (!pre.delete(contentlets, user, respectFrontendRoles, true)) { + throw new DotRuntimeException(String.format(PREHOOK_FAILED_MESSAGE, pre.getClass().getName())); + } + } + conAPI.deleteAllVersionsandBackup(contentlets, user, respectFrontendRoles); + for (ContentletAPIPostHook post : postHooks) { + post.delete(contentlets, user, respectFrontendRoles, true); + } } @Override diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/model/Contentlet.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/model/Contentlet.java index eabc9a67adc4..36ac4de31305 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/model/Contentlet.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/model/Contentlet.java @@ -310,7 +310,8 @@ public Contentlet(final Map mapIn) { * @param contentlet */ public Contentlet(final Contentlet contentlet) { - this(contentlet.getMap()); + this(com.dotcms.storage.AssetStorageFeature.isEnabled() + ? new HashMap<>(contentlet.map) : contentlet.getMap()); this.setIndexPolicy(contentlet.getIndexPolicy()); } @@ -1214,39 +1215,78 @@ public void setBinary(com.dotcms.contenttype.model.field.Field field, File newFi * @throws IOException */ public java.io.File getBinary(String velocityVarName)throws IOException { - final Object rawValue = map.get(velocityVarName); - File f = (rawValue instanceof File) ? (File) rawValue : null; - if((f==null || !f.exists()) ){ - f=null; - map.remove(velocityVarName); + // Preserve main's filesystem/NFS behavior while S3 assets are disabled. + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { + final Object rawValue = map.get(velocityVarName); + File f = (rawValue instanceof File) ? (File) rawValue : null; + if((f==null || !f.exists()) ){ + f=null; + map.remove(velocityVarName); if ( map.get( INODE_KEY ) != null && InodeUtils.isSet( (String) map.get( INODE_KEY ) ) ) { String inode = (String) map.get(INODE_KEY); - try{ - java.io.File binaryFileFolder = new java.io.File(APILocator.getFileAssetAPI().getRealAssetsRootPath() - + java.io.File.separator - + inode.charAt(0) - + java.io.File.separator - + inode.charAt(1) - + java.io.File.separator - + inode - + java.io.File.separator - + velocityVarName); - if(binaryFileFolder.exists()){ - java.io.File[] files = binaryFileFolder.listFiles(new BinaryFileFilter()); - if(null != files && files.length > 0){ - f = files[0]; - map.put(velocityVarName, f); - } - } - }catch(Exception e){ - Logger.error(this,"Error occured while retrieving binary file name : getBinaryFileName(). ContentletInode : "+inode+" velocityVaribleName : "+velocityVarName ); - throw new IOException("File System error."); - } - } + try{ + java.io.File binaryFileFolder = new java.io.File(APILocator.getFileAssetAPI().getRealAssetsRootPath() + + java.io.File.separator + + inode.charAt(0) + + java.io.File.separator + + inode.charAt(1) + + java.io.File.separator + + inode + + java.io.File.separator + + velocityVarName); + if(binaryFileFolder.exists()){ + java.io.File[] files = binaryFileFolder.listFiles(new BinaryFileFilter()); + if(null != files && files.length > 0){ + f = files[0]; + map.put(velocityVarName, f); + } + } + }catch(Exception e){ + Logger.error(this,"Error occured while retrieving binary file name : getBinaryFileName(). ContentletInode : "+inode+" velocityVaribleName : "+velocityVarName ); + throw new IOException("File System error."); + } + } + } + + return f; + + + } + + final Object rawValue = map.get(velocityVarName); + File f = (rawValue instanceof File) ? (File) rawValue : null; + if (f != null && f.exists()) { + return f; } + f = null; + if (map.get(INODE_KEY) != null && InodeUtils.isSet((String) map.get(INODE_KEY))) { + String inode = (String) map.get(INODE_KEY); + try { + final String revisionKey = rawValue instanceof File + ? com.dotcms.storage.binary.BinaryAssetReference.keyOf((File) rawValue) : null; + // Check-in assigns the new inode before it reads the binary, so a revision carried + // forward from the previous version is restored from the owner its key names. + final com.dotcms.storage.binary.BinaryAssetReference.Owner owner = revisionKey == null + ? null : com.dotcms.storage.binary.BinaryAssetReference.ownerOf(revisionKey); + f = revisionKey == null + ? APILocator.getBinaryAssetStorageAPI().getBinaryFile(inode, velocityVarName) + : owner == null + ? APILocator.getBinaryAssetStorageAPI().getRevisionFile(inode, velocityVarName, revisionKey) + : APILocator.getBinaryAssetStorageAPI().getRevisionFile(owner.inode(), owner.field(), revisionKey); + if (rawValue instanceof File) { + f = com.dotcms.storage.binary.BinaryAssetReference.preserveMetadata(f, (File) rawValue); + } + if (f != null) { + map.put(velocityVarName, f); + } + } catch (Exception e) { + Logger.error(this, "Error retrieving binary file: inode=" + inode + + " field=" + velocityVarName); + throw new IOException("File System error."); + } + } return f; - } /** @@ -1256,6 +1296,13 @@ public java.io.File getBinary(String velocityVarName)throws IOException { * @throws IOException */ public InputStream getBinaryStream(String velocityVarName) throws IOException{ + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try (var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease()) { + final File binary = getBinary(velocityVarName); + if (binary == null) throw new java.io.FileNotFoundException("Missing binary field " + velocityVarName); + return Files.newInputStream(binary.toPath()); + } + } InputStream fis = Files.newInputStream(getBinary(velocityVarName).toPath()); return fis; } diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/ContentletTransformer.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/ContentletTransformer.java index f0ae4ded7508..35205bce0b21 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/ContentletTransformer.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/ContentletTransformer.java @@ -16,7 +16,6 @@ import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.exception.DotSecurityException; -import com.dotmarketing.portlets.contentlet.business.BinaryFileFilter; import com.dotmarketing.portlets.contentlet.model.Contentlet; import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; import com.dotmarketing.portlets.folders.model.Folder; @@ -334,6 +333,10 @@ private static void populateFields(final Contentlet contentlet, final Map 0) { binaryFile = files[0]; } } value = binaryFile; + } } else { value = getObjectValue(originalMap, field); } diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAsset.java b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAsset.java index 19375838fe2d..d7c6eda0ded8 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAsset.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAsset.java @@ -195,6 +195,11 @@ public void setMimeType(String mimeType) { @JsonIgnore public InputStream getInputStream() throws IOException { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try (var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease()) { + return new BufferedInputStream(Files.newInputStream(getFileAsset().toPath())); + } + } return new BufferedInputStream(Files.newInputStream(getFileAsset().toPath())); } @@ -214,7 +219,7 @@ public void setBinary(final com.dotcms.contenttype.model.field.Field field, fina public File getFileAsset() { // Calling the getBinary method can be relatively expensive since it constantly verifies // the existence of the file on disk. Therefore, we'll keep a file reference at hand - if (null == file) { + if (null == file || (com.dotcms.storage.AssetStorageFeature.isEnabled() && !file.isFile())) { try { file = getBinary(FileAssetAPI.BINARY_FIELD); } catch (final IOException e) { diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPI.java b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPI.java index 542fb846138f..b911db77b6e8 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPI.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPI.java @@ -258,37 +258,42 @@ public List findFileAssetsByParentable(final Parentable parent, public boolean moveFile ( Contentlet fileAssetCont, Host host, User user, boolean respectFrontendRoles ) throws DotStateException, DotDataException, DotSecurityException; /** - * - * @param inode - * @param fileName - * @param ext - * @return + * @param inode The File Asset's Inode. + * @param fileName The File Asset's name. + * @param ext The File Asset's extension. + * @return The absolute path of the File Asset. + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. */ + @Deprecated public String getRealAssetPath(String inode, String fileName, String ext); /** - * - * @param inode - * @return + * @param inode The File Asset's Inode. + * @return The absolute path of the File Asset's field directory. + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. */ + @Deprecated public String getRealAssetPath(String inode); /** - * Returns the file on the filesystem that backup the fileAsset - * @param inode - * @param fileName generally speaking this method is expected to be called using the Underlying File Name property - * e.g. getRealAssetPath(inode, fileAsset.getUnderlyingFileName()) - * @return + * Returns the file on the filesystem that backs the fileAsset. + * @param inode The File Asset's Inode. + * @param fileName The Underlying File Name property, e.g. fileAsset.getUnderlyingFileName() + * @return The absolute path of the File Asset. + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. */ + @Deprecated String getRealAssetPath(String inode, String fileName); /** - * This method returns the file on the filesystem that backup the fileAsset ignoring the case of the extension + * Returns the file on the filesystem that backs the fileAsset, ignoring the case of the extension. * - * @param inode - * @param fileName - * @return the real path of the asset + * @param inode The File Asset's Inode. + * @param fileName The Underlying File Name property. + * @return The absolute path of the File Asset. + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. */ + @Deprecated String getRealAssetPathIgnoreExtensionCase(String inode, String fileName); /** diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPIImpl.java b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPIImpl.java index c9abcdeb8990..2d218948a821 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPIImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetAPIImpl.java @@ -600,8 +600,21 @@ public FileAsset find(final String inode, final User user, final boolean respect * @param ext The File Asset's extension. * * @return The absolute path of the File Asset. + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. */ + @Deprecated public String getRealAssetPath(final String inode, final String fileName, final String ext) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try { + final File binary = APILocator.getBinaryAssetStorageAPI().getBinaryFile(inode, FileAssetAPI.BINARY_FIELD); + if (binary != null && fileName.equals(UtilMethods.getFileName(binary.getName())) + && java.util.Objects.toString(ext, "").equalsIgnoreCase(UtilMethods.getFileExtension(binary.getName()))) { + return binary.getAbsolutePath(); + } + } catch (DotDataException e) { + throw new com.dotmarketing.exception.DotRuntimeException("Unable to resolve file asset " + inode, e); + } + } String realPath = Config.getStringProperty("ASSET_REAL_PATH"); if (UtilMethods.isSet(realPath) && !realPath.endsWith(java.io.File.separator)) { realPath += java.io.File.separator; @@ -623,12 +636,13 @@ public String getRealAssetPath(final String inode, final String fileName, final } /** - * Returns the file on the filesystem that backup the fileAsset - * @param inode - * @param fileName generally speaking this method is expected to be called using the Underlying File Name property - * e.g. getRealAssetPath(inode, fileAsset.getUnderlyingFileName()) - * @return + * Returns the file on the filesystem that backs the fileAsset. + * @param inode The File Asset's Inode. + * @param fileName The Underlying File Name property, e.g. fileAsset.getUnderlyingFileName() + * @return The absolute path of the File Asset. + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. */ + @Deprecated @Override public String getRealAssetPath(String inode, String fileName) { @@ -639,12 +653,13 @@ public String getRealAssetPath(String inode, String fileName) { } /** - * Returns the file on the filesystem that backup the fileAsset ignoring the case of the extension - * @param inode - * @param fileName generally speaking this method is expected to be called using the Underlying File Name property - * e.g. getRealAssetPathIgnoreExtensionCase(inode, fileAsset.getUnderlyingFileName()) - * @return + * Returns the file on the filesystem that backs the fileAsset, ignoring the case of the extension. + * @param inode The File Asset's Inode. + * @param fileName The Underlying File Name property, e.g. fileAsset.getUnderlyingFileName() + * @return The absolute path of the File Asset. + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. */ + @Deprecated @Override public String getRealAssetPathIgnoreExtensionCase(String inode, String fileName) { String extension = UtilMethods.getFileExtensionIgnoreCase(fileName); @@ -673,6 +688,10 @@ public String getRealAssetsRootPath() { return ConfigUtils.getAbsoluteAssetsRootPath(); } + /** + * @deprecated Use {@link com.dotcms.storage.binary.BinaryAssetStorageAPI#getBinaryFile} instead. + */ + @Deprecated public String getRealAssetPath(String inode) { String _inode = inode; String path = ""; @@ -811,6 +830,14 @@ public void cleanThumbnailsFromContentlet(Contentlet contentlet) { * @param fileAsset */ public void cleanThumbnailsFromFileAsset(IFileAsset fileAsset) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try { + APILocator.getBinaryAssetStorageAPI().deleteGeneratedFiles(fileAsset.getInode()); + return; + } catch (DotDataException e) { + throw new com.dotmarketing.exception.DotRuntimeException("Unable to invalidate S3 renditions", e); + } + } // Wiping out the thumbnails and resized versions // http://jira.dotmarketing.net/browse/DOTCMS-5911 final String inode = fileAsset.getInode(); diff --git a/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java b/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java index 64119b301c90..2e364735af5f 100644 --- a/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java +++ b/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java @@ -22,6 +22,7 @@ import com.dotmarketing.business.UserAPI; import com.dotmarketing.db.HibernateUtil; import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.exception.DotSecurityException; import com.dotmarketing.portlets.contentlet.business.ContentletAPI; import com.dotmarketing.portlets.structure.model.Structure; @@ -111,6 +112,16 @@ public void run(final JobExecutionContext jobContext) throws JobExecutionExcepti public static void triggerCleanUpJob(final Field field, final User user) { + if (com.dotcms.storage.AssetStorageFeature.isEnabled() + && field instanceof com.dotcms.contenttype.model.field.BinaryField) { + try { + com.dotcms.storage.binary.BinaryFieldCleanupProcessor.enqueue(field.contentTypeId(), field.variable(), new Date()); + } catch (final DotDataException e) { + throw new DotRuntimeException("Unable to schedule binary field cleanup for " + field.variable(), e); + } + return; + } + final Map nextExecutionData = Map .of("field", field, "deletionDate", Calendar.getInstance().getTime(), diff --git a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java index b8c33539b048..9bac5fd656b1 100644 --- a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java +++ b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Optional; import java.util.TimeZone; +import java.util.concurrent.atomic.AtomicBoolean; import javax.imageio.ImageIO; import javax.imageio.spi.IIORegistry; @@ -171,6 +172,9 @@ public void init() throws ServletException { * Processes incoming requests for binary files. Requests issued to this servlet might come directly * to it or through another servlet, such as the {@code SpeedyAssetServlet} class which is accessed using * the legacy {@code /dotAsset/} path to display files. + *

With S3 asset storage on, the request holds a local cache lease while it resolves, exports + * and opens the file, and releases it before streaming, so a slow client cannot defer eviction. + * An open file on a local disk survives eviction.

* * @param req The {@link HttpServletRequest} object. * @param resp The {@link HttpServletResponse} object. @@ -181,6 +185,36 @@ public void init() throws ServletException { @SuppressWarnings("unchecked") @Override public void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + final var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease(); + // The lease is a read lock: release it once, on this thread, whichever happens first. + final AtomicBoolean released = new AtomicBoolean(); + final Runnable releaseLease = () -> { + if (released.compareAndSet(false, true)) { + lease.close(); + } + }; + try { + serveBinary(req, resp, releaseLease); + } finally { + releaseLease.run(); + } + } else { + serveBinary(req, resp, () -> { }); + } + } + + /** + * Resolves, exports and streams the requested binary. + * + * @param req the request + * @param resp the response + * @param releaseLease releases the cache lease; called once the file to stream is open + * @throws ServletException if the request cannot be served + * @throws IOException if the response cannot be written + */ + private void serveBinary(HttpServletRequest req, HttpServletResponse resp, final Runnable releaseLease) + throws ServletException, IOException { String servletPath = req.getServletPath(); String uri = req.getRequestURI().substring(servletPath.length()); String[] uriPieces = uri.split("/"); @@ -583,6 +617,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl if (ranges.isEmpty() || ranges.get(0).equals(full)) { // Return full file. input = new RandomAccessFile(data.getDataFile(), "r"); + releaseLease.run(); SpeedyAssetServletUtil.ByteRange r = full; resp.setContentType(fileAssetAPI.getMimeType(data.getDataFile().getName())); resp.setHeader("Content-Range", "bytes " + r.start + "-" + r.end + "/" + r.total); @@ -592,6 +627,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl } else if (ranges.size() == 1){ SpeedyAssetServletUtil.ByteRange range = ranges.get(0); input = new RandomAccessFile(data.getDataFile(), "r"); + releaseLease.run(); // Check if Range is syntactically valid. If not, then return 416. if (range.start > range.end) { resp.setHeader("Content-Range", "bytes */" + fileLen); // Required in 416. @@ -607,6 +643,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl resp.setContentType("multipart/byteranges; boundary=" + SpeedyAssetServletUtil.MULTIPART_BOUNDARY); resp.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT); input = new RandomAccessFile(data.getDataFile(), "r"); + releaseLease.run(); for (SpeedyAssetServletUtil.ByteRange r : ranges) { if (r.start > r.end) { resp.setHeader("Content-Range", "bytes */" + fileLen); // Required in 416. @@ -635,6 +672,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl } }else{ is = java.nio.file.Files.newInputStream(data.getDataFile().toPath()); + releaseLease.run(); int count = 0; byte[] buffer = new byte[4096]; out = resp.getOutputStream(); diff --git a/dotCMS/src/test/java/com/dotcms/storage/AssetJobEventSerializationTest.java b/dotCMS/src/test/java/com/dotcms/storage/AssetJobEventSerializationTest.java new file mode 100644 index 000000000000..1372584a46b2 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/storage/AssetJobEventSerializationTest.java @@ -0,0 +1,42 @@ +package com.dotcms.storage; + +import static org.junit.jupiter.api.Assertions.*; + +import com.dotcms.jobs.business.api.events.JobCompletedEvent; +import com.dotcms.jobs.business.api.events.JobCreatedEvent; +import com.dotcms.jobs.business.api.events.JobFailedEvent; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.jobs.business.job.JobState; +import com.dotcms.util.marshal.JacksonMarshalUtilsImpl; +import com.dotmarketing.util.Config; +import java.io.StringWriter; +import java.time.LocalDateTime; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class AssetJobEventSerializationTest { + @Test + void cleanupEventsCanBeReadBackFromTheNotificationStore() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try { + Config.setProperty(AssetStorageFeature.FLAG, true); + final var codec = new JacksonMarshalUtilsImpl(); + final var job = Job.builder().id("cleanup-test").queueName("bundleArchiveCleanup") + .state(JobState.PENDING).parameters(Map.of("bundleId", "Mixed-Case")).build(); + final var now = LocalDateTime.now(); + final var json = new com.fasterxml.jackson.databind.ObjectMapper(); + for (var event : List.of(new JobCreatedEvent(job.id(), job.queueName(), now, job.parameters()), + new JobCompletedEvent(job, now), new JobFailedEvent(job, now))) { + final var encoded = new StringWriter(); + codec.marshal(encoded, event); + final Object restored = codec.unmarshal(encoded.toString(), event.getClass()); + final var encodedAgain = new StringWriter(); + codec.marshal(encodedAgain, restored); + assertEquals(json.readTree(encoded.toString()), json.readTree(encodedAgain.toString())); + } + } finally { + Config.setProperty(AssetStorageFeature.FLAG, previous); + } + } +} diff --git a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java index 722bd390ad81..7f1e5867ee8d 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureLatchTest.java @@ -3,13 +3,19 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mockStatic; +import com.dotcms.storage.binary.BinaryAssetCleanupProcessor; +import com.dotcms.storage.binary.BinaryFieldCleanupProcessor; import com.dotmarketing.util.Config; +import java.util.List; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; class AssetStorageFeatureLatchTest { + private static final List> S3_PROCESSORS = List.of(BinaryAssetCleanupProcessor.class, + BinaryFieldCleanupProcessor.class); + private String previous; @BeforeEach @@ -43,4 +49,14 @@ void inMemoryOverridesSwitchTheMode() { Config.setProperty(AssetStorageFeature.FLAG, false); assertFalse(AssetStorageFeature.isEnabled()); } + + @Test + void s3JobProcessorsRegisterOnlyWhenEnabled() { + Config.setProperty(AssetStorageFeature.FLAG, false); + S3_PROCESSORS.forEach(p -> assertFalse(AssetStorageFeature.allowsJobProcessor(p), p.getName())); + assertTrue(AssetStorageFeature.allowsJobProcessor(String.class), "Other processors are unaffected"); + + Config.setProperty(AssetStorageFeature.FLAG, true); + S3_PROCESSORS.forEach(p -> assertTrue(AssetStorageFeature.allowsJobProcessor(p), p.getName())); + } } diff --git a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java index 89172f0ef6cd..cab7f9a311b1 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java @@ -4,6 +4,8 @@ import com.dotcms.storage.binary.BinaryAssetStorageAPIImpl; import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotRuntimeException; +import com.dotmarketing.portlets.fileassets.business.FileAsset; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; import com.dotmarketing.util.Config; import com.dotmarketing.util.ConfigUtils; import org.junit.jupiter.api.AfterEach; @@ -16,6 +18,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicInteger; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -136,6 +139,29 @@ void filesystemMetadataReplacementKeepsPriorValueDuringAndAfterFailedSerializati } } + @Test + void receivedMetadataOnlyUsesLegacyRecursiveCleanupWhenFeatureIsDisabled() throws Exception { + final var storage = mock(FileStorageAPI.class); + final var content = mock(com.dotmarketing.portlets.contentlet.model.Contentlet.class); + final var type = mock(com.dotcms.contenttype.model.type.ContentType.class); + when(content.getContentType()).thenReturn(type); + when(type.fields(com.dotcms.contenttype.model.field.BinaryField.class)).thenReturn(List.of()); + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class); + var caches = mockStatic(com.dotmarketing.business.CacheLocator.class)) { + locator.when(com.dotmarketing.business.APILocator::getFileStorageAPI).thenReturn(storage); + caches.when(com.dotmarketing.business.CacheLocator::getMetadataCache) + .thenReturn(mock(com.dotmarketing.portlets.contentlet.business.MetadataCache.class)); + final var api = spy(new FileMetadataAPIImpl()); + doReturn(Map.of()).when(api).removeMetadata(content); + api.setMetadata(content, Map.of()); + verify(api, never()).removeMetadata(content); + verifyNoInteractions(storage); + Config.setProperty(AssetStorageFeature.FLAG, false); + api.setMetadata(content, Map.of()); + verify(api).removeMetadata(content); + } + } + @Test void disabledFilesystemProviderKeepsLegacyPathAndMissingFileBehavior() throws Exception { Config.setProperty(AssetStorageFeature.FLAG, false); @@ -151,6 +177,73 @@ void disabledFilesystemProviderKeepsLegacyPathAndMissingFileBehavior() throws Ex assertThrows(IllegalArgumentException.class, () -> storage.pullFile(GROUP, KEY)); } + @Test + void disabledContentletReadUsesLegacyFilesystemAndPreservesStringTypeGuard() throws Exception { + Config.setProperty(AssetStorageFeature.FLAG, false); + Path path = root.resolve(KEY); + Files.createDirectories(path.getParent()); + Files.writeString(path, "legacy binary"); + var fileAPI = mock(FileAssetAPI.class); + when(fileAPI.getRealAssetsRootPath()).thenReturn(root.toString()); + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class)) { + locator.when(com.dotmarketing.business.APILocator::getFileAssetAPI).thenReturn(fileAPI); + var content = new com.dotmarketing.portlets.contentlet.model.Contentlet(); + content.setInode("abc123"); + content.getMap().put("HeroImage", "MyFile.PNG"); + assertEquals(path.toFile(), content.getBinary("HeroImage")); + locator.verify(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI, never()); + } + } + + @Test + void enabledFileWrapperResolvesAgainAfterEviction() throws Exception { + Path path = root.resolve("Theme.CSS"); + AtomicInteger calls = new AtomicInteger(); + FileAsset wrapper = mock(FileAsset.class, CALLS_REAL_METHODS); + doAnswer(invocation -> { + calls.incrementAndGet(); + if (!Files.exists(path)) Files.writeString(path, "body {}"); + return path.toFile(); + }).when(wrapper).getBinary(FileAssetAPI.BINARY_FIELD); + assertTrue(wrapper.getFileAsset().exists()); + Files.delete(path); + final var api = mock(BinaryAssetStorageAPI.class); + final var lease = mock(BinaryAssetStorageAPI.CacheLease.class); + when(api.acquireCacheLease()).thenReturn(lease); + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class)) { + locator.when(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI).thenReturn(api); + try (var input = wrapper.getInputStream()) { + assertEquals("body {}", new String(input.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8)); + } + } + verify(lease).close(); + assertEquals(2, calls.get()); + } + + @Test + void enabledThumbnailCleanupDoesNotFallThroughToLegacyShardDeletion() throws Exception { + final Path generated = root.resolve("generated"); + final Path legacy = generated.resolve("a/b/dotGenerated_resize_1234.png"); + Files.createDirectories(legacy.getParent()); + Files.writeString(legacy, "legacy neighboring rendition"); + final var asset = mock(FileAsset.class); + when(asset.getInode()).thenReturn("abc123"); + final var api = mock(com.dotmarketing.portlets.fileassets.business.FileAssetAPIImpl.class, CALLS_REAL_METHODS); + doReturn(root.toString()).when(api).getRealAssetsRootPath(); + final var storage = mock(BinaryAssetStorageAPI.class); + try (var paths = mockStatic(ConfigUtils.class); + var locator = mockStatic(com.dotmarketing.business.APILocator.class)) { + paths.when(ConfigUtils::getDotGeneratedPath).thenReturn(generated.toString()); + locator.when(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI).thenReturn(storage); + api.cleanThumbnailsFromFileAsset(asset); + assertTrue(Files.exists(legacy), "S3 invalidation must not clear the legacy shared shard"); + Config.setProperty(AssetStorageFeature.FLAG, false); + api.cleanThumbnailsFromFileAsset(asset); + assertFalse(Files.exists(legacy), "Disabled behavior must retain main's original cleanup"); + verify(storage, times(1)).deleteGeneratedFiles("abc123"); + } + } + @Test void stalledRenditionInvalidationDoesNotBlockAnotherAssetRead() throws Exception { final var storage = mock(StoragePersistenceAPI.class); diff --git a/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java b/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java index 4c97c3db12c2..935ff720fc3d 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java @@ -186,6 +186,83 @@ private String readRemote(AmazonS3StoragePersistenceAPIImpl remote, String group finally { remote.releaseRetrievedFile(file); } } + @Test + void immutableReplacementCommitsAndRollsBackWithRealPostgres() throws Exception { + String jdbc = System.getProperty("s3.test.jdbc"); + org.junit.jupiter.api.Assumptions.assumeTrue(jdbc != null, "Supply s3.test.jdbc for transaction coverage"); + String schema = "binary_revisions_" + UUID.randomUUID().toString().replace("-", ""); + var contentApi = new BinaryAssetStorageAPIImpl(new ChainableStoragePersistenceAPI( + new JsonWriterDelegate(), List.of(fs, s3), mock(Chainable404StorageCache.class)), true); + try (var writer = java.sql.DriverManager.getConnection(jdbc, CREDENTIAL, CREDENTIAL); + var observer = java.sql.DriverManager.getConnection(jdbc, CREDENTIAL, CREDENTIAL)) { + writer.createStatement().execute("create schema " + schema); + try { + writer.createStatement().execute("set search_path to " + schema); + observer.createStatement().execute("set search_path to " + schema); + writer.createStatement().execute("create table contentlet (inode varchar(255) primary key, contentlet_as_json jsonb)"); + writer.createStatement().execute("insert into contentlet values ('abc123', '{}')"); + File source = root.resolve("upload.tmp").toFile(); + Files.writeString(source.toPath(), "last committed pixels"); + File original = contentApi.storeRevision("abc123", "HeroImage", "Friday.GIF", source); + String originalKey = com.dotcms.storage.binary.BinaryAssetReference.keyOf(original); + writeReference(writer, originalKey); + + writer.setAutoCommit(false); + Files.writeString(source.toPath(), "uncommitted replacement"); + File replacement = contentApi.storeRevision("abc123", "HeroImage", "Friday.GIF", source); + String replacementKey = com.dotcms.storage.binary.BinaryAssetReference.keyOf(replacement); + assertNotEquals(originalKey, replacementKey); + writeReference(writer, replacementKey); + com.dotmarketing.db.DbConnectionFactory.setConnection(observer); + assertEquals("last committed pixels", Files.readString(contentApi.getBinaryFile("abc123", "HeroImage").toPath())); + writer.rollback(); + assertTrue(contentApi.evictLocalFile(original)); + assertTrue(contentApi.evictLocalFile(replacement)); + assertEquals("last committed pixels", Files.readString(contentApi.getBinaryFile("abc123", "HeroImage", "Friday.GIF").toPath())); + + // The same immutable replacement can be referenced by a later successful save. + writeReference(writer, replacementKey); + writer.commit(); + File current = contentApi.getBinaryFile("abc123", "HeroImage"); + assertEquals("Friday.GIF", current.getName()); + assertEquals("uncommitted replacement", Files.readString(current.toPath())); + assertTrue(contentApi.evictLocalFile(original)); + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class)) { + locator.when(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI).thenReturn(contentApi); + var snapshot = new com.dotmarketing.portlets.contentlet.model.Contentlet(); + snapshot.setInode("abc123"); + snapshot.getMap().put("HeroImage", original); + assertEquals("last committed pixels", Files.readString(snapshot.getBinary("HeroImage").toPath())); + } + assertEquals("last committed pixels", readStoredBinary(originalKey)); + + // An authoritative cleared field must not discover an older revision by listing. + writer.createStatement().execute("update contentlet set contentlet_as_json = '{}'"); + writer.commit(); + assertNull(contentApi.getBinaryFile("abc123", "HeroImage")); + contentApi.deleteAllBinaries("abc123"); + assertTrue(client.listObjectsV2(bucket, GROUP + "/a/b/abc123/").getObjectSummaries().isEmpty()); + } finally { + writer.rollback(); + writer.setAutoCommit(true); + writer.createStatement().execute("drop schema " + schema + " cascade"); + } + } finally { + com.dotmarketing.db.DbConnectionFactory.closeConnection(); + } + } + + private void writeReference(java.sql.Connection connection, String key) throws Exception { + var binary = com.dotcms.content.model.type.system.BinaryFieldType.builder() + .value("Friday.GIF").storageKey(key).build(); + String json = new com.fasterxml.jackson.databind.ObjectMapper() + .writeValueAsString(Map.of("fields", Map.of("HeroImage", binary))); + try (var statement = connection.prepareStatement("update contentlet set contentlet_as_json = ?::jsonb where inode = 'abc123'")) { + statement.setString(1, json); + assertEquals(1, statement.executeUpdate()); + } + } + @Test @EnabledIfSystemProperty(named = "s3.test.sts.accessKey", matches = ".+") void temporaryRoleCredentialsCanRefreshDuringBinaryLifecycle() throws Exception { @@ -532,6 +609,107 @@ void renditionInvalidationPreservesNeighboringAssetAndItsColdRead() throws Excep assertEquals("neighbor pixels", Files.readString(api.getGeneratedFile(neighbor.toFile()).toPath())); } + @Test + void metadataOutageDoesNotRegenerateAndRecoveryRecreatesTheMappedCache() throws Exception { + final String group = "metadata"; + final String path = "/a/b/abc123/asset-metadata.json"; + final Path cacheRoot = Files.createDirectories(root.resolve("custom-cache")); + fs.addGroupMapping(group, cacheRoot.toFile()); + final var chain = new ChainableStoragePersistenceAPI(new JsonWriterDelegate(), List.of(fs, s3), + mock(Chainable404StorageCache.class)); + final var provider = mock(StoragePersistenceProvider.class); + when(provider.getStorage(any())).thenReturn(chain); + final var generator = mock(MetadataGenerator.class); + final var metadata = new FileStorageAPIImpl(new JsonReaderDelegate<>(Map.class), new JsonWriterDelegate(), + generator, provider, mock(com.dotmarketing.portlets.contentlet.business.MetadataCache.class)); + final var key = new StorageKey.Builder().group(group).path(path).storage(StorageType.DEFAULT_CHAIN).build(); + final var request = new FetchMetadataParams.Builder().cache(false).storageKey(key).build(); + metadata.setMetadata(request, Map.of("dot:focalPoint", "0.75,0.5")); + org.apache.commons.io.FileUtils.deleteDirectory(cacheRoot.toFile()); + + final AWSS3Storage unavailable = spy(storage); + final var outage = new com.amazonaws.services.s3.model.AmazonS3Exception("injected read outage"); + outage.setStatusCode(503); + doThrow(outage).when(unavailable).downloadFile(eq(bucket), anyString(), any(File.class)); + final var failingS3 = new AmazonS3StoragePersistenceAPIImpl(unavailable, bucket, + AmazonS3StoragePersistenceAPIImpl.PathEncryptionMode.NONE); + when(provider.getStorage(any())).thenReturn(new ChainableStoragePersistenceAPI(new JsonWriterDelegate(), + List.of(fs, failingS3), mock(Chainable404StorageCache.class))); + assertThrows(com.dotmarketing.exception.DotDataException.class, () -> metadata.retrieveMetaData(request)); + final var sourceReads = new java.util.concurrent.atomic.AtomicInteger(); + final java.util.function.Supplier source = () -> { + sourceReads.incrementAndGet(); + return root.resolve("must-not-be-read.PNG").toFile(); + }; + final var config = new GenerateMetadataConfig.Builder().storageKey(key).build(); + assertThrows(com.dotmarketing.exception.DotDataException.class, () -> metadata.generateMetaData(source, config)); + assertEquals(0, sourceReads.get(), "An outage must not start metadata regeneration"); + verifyNoInteractions(generator); + assertFalse(Files.exists(cacheRoot)); + when(provider.getStorage(any())).thenReturn(chain); + assertEquals("0.75,0.5", metadata.retrieveMetaData(request).get("dot:focalPoint")); + final Path local = cacheRoot.resolve(path.substring(1)); + assertTrue(Files.isRegularFile(local), "Restore must preserve the custom cache mapping"); + assertFalse(Files.exists(root.resolve(group)), "Restore must not silently relocate the cache"); + final var absent = new FetchMetadataParams.Builder().cache(false).storageKey(new StorageKey.Builder() + .group(group).path("/missing.json").storage(StorageType.DEFAULT_CHAIN).build()).build(); + assertNull(metadata.retrieveMetaData(absent), "A real S3 404 is still normal absence"); + + Files.writeString(local, "broken JSON"); + when(provider.getStorage(any())).thenReturn(new ChainableStoragePersistenceAPI(new JsonWriterDelegate(), + List.of(fs), mock(Chainable404StorageCache.class))); + assertThrows(com.dotmarketing.exception.DotDataException.class, () -> metadata.retrieveMetaData(request)); + assertEquals("broken JSON", Files.readString(local), + "Without a durable copy, read errors must not delete evidence or regenerate metadata"); + verifyNoInteractions(generator); + when(provider.getStorage(any())).thenReturn(chain); + assertEquals("0.75,0.5", metadata.retrieveMetaData(request).get("dot:focalPoint"), + "An unreadable local copy is replaced from S3"); + assertNotEquals("broken JSON", Files.readString(local)); + Files.delete(local); + assertEquals("0.75,0.5", metadata.retrieveMetaData(request).get("dot:focalPoint")); + org.apache.commons.io.FileUtils.deleteDirectory(cacheRoot.toFile()); + assertEquals(local.toFile().getCanonicalFile(), chain.pullFile(group, path), + "File restoration must also preserve a removed cache directory's mapping"); + } + + @Test + void metadataReplacementSurvivesUploadFailureAndColdRead() throws Exception { + final String group = "metadata"; + final String path = "/a/b/abc123/HeroImage-metadata.json"; + final var chain = new ChainableStoragePersistenceAPI(new JsonWriterDelegate(), List.of(fs, s3), + mock(Chainable404StorageCache.class)); + final var provider = mock(StoragePersistenceProvider.class); + when(provider.getStorage(any())).thenReturn(chain); + final var metadata = new FileStorageAPIImpl(new JsonReaderDelegate<>(Map.class), new JsonWriterDelegate(), + mock(MetadataGenerator.class), provider, + mock(com.dotmarketing.portlets.contentlet.business.MetadataCache.class)); + final var request = new FetchMetadataParams.Builder().cache(false) + .storageKey(new StorageKey.Builder().group(group).path(path).storage(StorageType.DEFAULT_CHAIN).build()).build(); + metadata.setMetadata(request, Map.of("dot:credit", "Original")); + metadata.setMetadata(request, Map.of("dot:credit", "Replacement")); + assertEquals("Replacement", metadata.retrieveMetaData(request).get("dot:credit"), + "The local cache must receive the replacement without a pre-delete"); + + final AWSS3Storage failingUpload = spy(storage); + doThrow(new com.amazonaws.AmazonClientException("injected upload failure")) + .when(failingUpload).uploadFile(any(com.amazonaws.services.s3.model.PutObjectRequest.class)); + final var failingS3 = new AmazonS3StoragePersistenceAPIImpl(failingUpload, bucket, + AmazonS3StoragePersistenceAPIImpl.PathEncryptionMode.NONE); + when(provider.getStorage(any())).thenReturn(new ChainableStoragePersistenceAPI(new JsonWriterDelegate(), + List.of(fs, failingS3), mock(Chainable404StorageCache.class))); + assertThrows(com.dotmarketing.exception.DotDataException.class, + () -> metadata.setMetadata(request, Map.of("dot:credit", "Lost update"))); + when(provider.getStorage(any())).thenReturn(chain); + assertEquals("Replacement", metadata.retrieveMetaData(request).get("dot:credit")); + Files.delete(fs.pullFile(group, path).toPath()); + assertEquals("Replacement", metadata.retrieveMetaData(request).get("dot:credit"), + "The prior remote value must survive the rejected upload"); + metadata.putCustomMetadataAttributes(request, Map.of("credit", "Recovered")); + Files.delete(fs.pullFile(group, path).toPath()); + assertEquals("Recovered", metadata.retrieveMetaData(request).get("dot:credit")); + } + @Test void s3ObjectSerializationUsesIndependentStagingAndCleansUpFailures() throws Exception { final var staged = new java.util.concurrent.CopyOnWriteArrayList(); diff --git a/dotCMS/src/test/java/com/dotcms/storage/MetadataLocalCacheTest.java b/dotCMS/src/test/java/com/dotcms/storage/MetadataLocalCacheTest.java index f978c6885633..2b744ee2f51b 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/MetadataLocalCacheTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/MetadataLocalCacheTest.java @@ -102,6 +102,32 @@ class MetadataLocalCacheTest { Config.setProperty(AssetStorageFeature.FLAG, oldFlag); } } + @Test void sharedExtractionStorageFailureIsNotAnEmptySuccessfulParse() throws Exception { + final String oldFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + final var file = Files.writeString(root.resolve("Extract.Txt"), "text").toFile(); + final var shared = mock(SharedExtractedMetadata.class); + final var metadata = mock(FileMetadataAPI.class); + try (var locator = mockStatic(APILocator.class); + var sharedFactory = mockStatic(SharedExtractedMetadata.class); + var tika = mockConstruction(com.dotcms.tika.TikaUtils.class, (mock, context) -> { + when(mock.extractorVersion()).thenReturn("test-parser"); + when(mock.getForcedMetaDataMap(any(), anyInt())).thenReturn(Map.of("content", "text")); + })) { + locator.when(APILocator::getFileMetadataAPI).thenReturn(metadata); + sharedFactory.when(SharedExtractedMetadata::getInstance).thenReturn(shared); + Config.setProperty(AssetStorageFeature.FLAG, true); + when(shared.get(eq(file), anyString(), anyInt(), anyInt(), any())) + .thenThrow(new DotDataException("S3 unavailable")); + assertThrows(com.dotmarketing.exception.DotRuntimeException.class, + () -> new MetadataGeneratorImpl().tikaBasedMetadata(file, 100)); + Config.setProperty(AssetStorageFeature.FLAG, false); + clearInvocations(shared); + assertEquals("text", new MetadataGeneratorImpl().tikaBasedMetadata(file, 100).get("content")); + verifyNoInteractions(shared); + } finally { + Config.setProperty(AssetStorageFeature.FLAG, oldFlag); + } + } /** * A binary that is neither cached nor stored durably is absent, not a failed restore, so metadata diff --git a/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetCleanupProcessorTest.java b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetCleanupProcessorTest.java new file mode 100644 index 000000000000..9b8a03fe90e4 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetCleanupProcessorTest.java @@ -0,0 +1,228 @@ +package com.dotcms.storage.binary; + +import com.dotcms.jobs.business.api.JobQueueManagerAPI; +import com.dotcms.jobs.business.error.JobProcessingException; +import com.dotcms.jobs.business.error.JobValidationException; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.FileMetadataAPI; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.db.HibernateUtil; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; +import com.dotmarketing.util.Config; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +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.ArgumentCaptor; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class BinaryAssetCleanupProcessorTest { + @TempDir Path root; + private String previousFlag; + private static final String INODE = "abc123"; + private static final String OLD_REVISION = + "a/b/abc123/heroImage/.revisions/11111111-1111-1111-1111-111111111111/old.png"; + private static final String NEW_REVISION = + "a/b/abc123/heroImage/.revisions/22222222-2222-2222-2222-222222222222/new.png"; + private static final String OLD_METADATA = "/" + OLD_REVISION + FileMetadataAPI.METADATA_JSON; + + @BeforeEach void enable() { + previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + Config.setProperty(AssetStorageFeature.FLAG, true); + } + + @AfterEach void restore() { + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + } + + private Job job(final Map parameters) { + Job job = mock(Job.class); + when(job.id()).thenReturn("cleanup-test"); + when(job.parameters()).thenReturn(com.google.common.collect.ImmutableMap.copyOf(parameters)); + return job; + } + + private Job job() { + return job(Map.of("inode", INODE, "binaries", List.of(OLD_REVISION), "metadata", List.of(OLD_METADATA))); + } + + @Test void enqueueRequiresTransactionAndPropagatesJournalFailure() throws Exception { + var queue = mock(JobQueueManagerAPI.class); + var storage = mock(BinaryAssetStorageAPI.class); + try (var locator = mockStatic(APILocator.class); + var connection = mockStatic(DbConnectionFactory.class)) { + locator.when(APILocator::getJobQueueManagerAPI).thenReturn(queue); + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(storage); + locator.when(APILocator::getFileMetadataAPI).thenReturn(mock(FileMetadataAPI.class)); + assertThrows(DotDataException.class, () -> BinaryAssetCleanupProcessor.enqueue(INODE)); + verifyNoInteractions(queue, storage); + connection.when(DbConnectionFactory::inTransaction).thenReturn(true); + when(queue.createJob(eq(BinaryAssetCleanupProcessor.QUEUE), anyMap())) + .thenThrow(new DotDataException("database unavailable")); + assertThrows(DotDataException.class, () -> BinaryAssetCleanupProcessor.enqueue(INODE)); + verify(storage, never()).deleteBinaryPaths(anyString(), anyString(), anyList()); + verify(storage, never()).deleteAllBinaries(anyString()); + } + } + + @Test void disabledFlagDoesNotCreateOrExecuteCleanup() throws Exception { + Config.setProperty(AssetStorageFeature.FLAG, false); + try (var locator = mockStatic(APILocator.class)) { + BinaryAssetCleanupProcessor.enqueue(INODE); + assertThrows(JobProcessingException.class, () -> new BinaryAssetCleanupProcessor().process(job())); + locator.verifyNoInteractions(); + } + } + + @Test void publicJobEndpointSubmissionsAreRejected() { + var processor = new BinaryAssetCleanupProcessor(); + // The public endpoint always adds userId; content deletion never does. + assertThrows(JobValidationException.class, + () -> processor.validate(Map.of("inode", INODE, "userId", "backend-user"))); + assertDoesNotThrow(() -> processor.validate( + Map.of("inode", INODE, "binaries", List.of(), "metadata", List.of()))); + Config.setProperty(AssetStorageFeature.FLAG, false); + assertThrows(JobValidationException.class, () -> processor.validate(Map.of("inode", INODE))); + } + + @Test void revisionUploadedAfterEnqueueSurvivesCleanup() throws Exception { + var queue = mock(JobQueueManagerAPI.class); + var storage = mock(BinaryAssetStorageAPI.class); + var metadata = mock(FileMetadataAPI.class); + var files = mock(FileAssetAPI.class); + when(files.getRealAssetsRootPath()).thenReturn(root.toString()); + // At deletion time only the old revision exists. + when(storage.listBinaryPaths(INODE)).thenReturn(List.of(OLD_REVISION)); + when(metadata.listMetadataForInode(INODE, List.of(OLD_REVISION))).thenReturn(List.of(OLD_METADATA)); + @SuppressWarnings("unchecked") + ArgumentCaptor> recorded = ArgumentCaptor.forClass(Map.class); + try (var queries = mockConstruction(DotConnect.class, withSettings().defaultAnswer(RETURNS_SELF), + (query, context) -> when(query.loadObjectResults()).thenReturn(List.of())); + var locator = mockStatic(APILocator.class); + var connection = mockStatic(DbConnectionFactory.class)) { + connection.when(DbConnectionFactory::inTransaction).thenReturn(true); + locator.when(APILocator::getJobQueueManagerAPI).thenReturn(queue); + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(storage); + locator.when(APILocator::getFileMetadataAPI).thenReturn(metadata); + locator.when(APILocator::getFileAssetAPI).thenReturn(files); + BinaryAssetCleanupProcessor.enqueue(INODE); + verify(queue).createJob(eq(BinaryAssetCleanupProcessor.QUEUE), recorded.capture()); + + // A push-publish receiver re-creates the inode and uploads a new revision before the job runs. + when(storage.listBinaryPaths(INODE)).thenReturn(List.of(OLD_REVISION, NEW_REVISION)); + new BinaryAssetCleanupProcessor().process(job(recorded.getValue())); + + verify(metadata).removeMetadataPaths(INODE, List.of(OLD_METADATA)); + verify(storage).deleteBinaryPaths(INODE, "heroImage", List.of(OLD_REVISION)); + verify(storage, never()).deleteBinaryPaths(anyString(), anyString(), + argThat(paths -> paths.contains(NEW_REVISION))); + verify(storage, never()).deleteAllBinaries(anyString()); + } + } + + @Test void referencedVersionIsNeverDeleted() throws Exception { + try (var queries = mockConstruction(DotConnect.class, withSettings().defaultAnswer(RETURNS_SELF), + (query, context) -> when(query.loadObjectResults()).thenReturn(List.of(Map.of("inode", INODE)))); + var locator = mockStatic(APILocator.class)) { + assertThrows(JobProcessingException.class, () -> new BinaryAssetCleanupProcessor().process(job())); + locator.verify(APILocator::getBinaryAssetStorageAPI, never()); + } + } + + @Test void jobWithoutInventoryDeletesNothing() throws Exception { + try (var locator = mockStatic(APILocator.class)) { + assertThrows(JobProcessingException.class, + () -> new BinaryAssetCleanupProcessor().process(job(Map.of("inode", INODE)))); + locator.verify(APILocator::getBinaryAssetStorageAPI, never()); + } + } + + @Test void remoteFailureIsRetryableAndPreservesLegacyCache() throws Exception { + var storage = mock(BinaryAssetStorageAPI.class); + var files = mock(FileAssetAPI.class); + when(files.getRealAssetsRootPath()).thenReturn(root.toString()); + Path cache = root.resolve("cache/a/b/abc123/old-image.png"); + Files.createDirectories(cache.getParent()); + Files.writeString(cache, "cached pixels"); + doThrow(new DotDataException("S3 unavailable")).doNothing() + .when(storage).deleteBinaryPaths(INODE, "heroImage", List.of(OLD_REVISION)); + try (var queries = mockConstruction(DotConnect.class, withSettings().defaultAnswer(RETURNS_SELF), + (query, context) -> when(query.loadObjectResults()).thenReturn(List.of())); + var locator = mockStatic(APILocator.class)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(storage); + locator.when(APILocator::getFileAssetAPI).thenReturn(files); + locator.when(APILocator::getFileMetadataAPI).thenReturn(mock(FileMetadataAPI.class)); + var processor = new BinaryAssetCleanupProcessor(); + assertThrows(JobProcessingException.class, () -> processor.process(job())); + assertTrue(Files.exists(cache)); + processor.process(job()); + assertFalse(Files.exists(cache)); + processor.process(job()); // Retry after completion is harmless. + } + } + + @Test void invalidInodeCannotEscapeAssetRoot() { + Job job = job(Map.of("inode", "../../outside", "binaries", List.of(), "metadata", List.of())); + assertThrows(IllegalArgumentException.class, () -> new BinaryAssetCleanupProcessor().process(job)); + } + + @Test void metadataFailureRetainsSourcesForRetry() throws Exception { + var storage = mock(BinaryAssetStorageAPI.class); + var metadata = mock(FileMetadataAPI.class); + var files = mock(FileAssetAPI.class); + when(files.getRealAssetsRootPath()).thenReturn(root.toString()); + doThrow(new DotDataException("metadata bucket unavailable")).doNothing() + .when(metadata).removeMetadataPaths(INODE, List.of(OLD_METADATA)); + try (var queries = mockConstruction(DotConnect.class, withSettings().defaultAnswer(RETURNS_SELF), + (query, context) -> when(query.loadObjectResults()).thenReturn(List.of())); + var locator = mockStatic(APILocator.class)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(storage); + locator.when(APILocator::getFileMetadataAPI).thenReturn(metadata); + locator.when(APILocator::getFileAssetAPI).thenReturn(files); + var processor = new BinaryAssetCleanupProcessor(); + assertThrows(JobProcessingException.class, () -> processor.process(job())); + verify(storage, never()).deleteBinaryPaths(anyString(), anyString(), anyList()); + processor.process(job()); + verify(storage).deleteBinaryPaths(INODE, "heroImage", List.of(OLD_REVISION)); + } + } + + @Test void rollbackDeletesOnlyTheJustUploadedRevisionAndNeverThrows() throws Exception { + var storage = mock(BinaryAssetStorageAPI.class); + var metadata = mock(FileMetadataAPI.class); + AtomicReference listener = new AtomicReference<>(); + try (var transactions = mockStatic(HibernateUtil.class); + var locator = mockStatic(APILocator.class)) { + transactions.when(() -> HibernateUtil.addRollbackListener(any(Runnable.class))) + .thenAnswer(call -> { + listener.set(call.getArgument(0)); + return null; + }); + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(storage); + locator.when(APILocator::getFileMetadataAPI).thenReturn(metadata); + + BinaryAssetCleanupProcessor.deleteRevisionOnRollback(INODE, "heroImage", null); + assertNull(listener.get(), "A file that is not a new revision is never deleted"); + + BinaryAssetCleanupProcessor.deleteRevisionOnRollback(INODE, "heroImage", NEW_REVISION); + verifyNoInteractions(storage, metadata); + listener.get().run(); + verify(metadata).removeMetadataPaths(INODE, List.of("/" + NEW_REVISION + FileMetadataAPI.METADATA_JSON)); + verify(storage).deleteBinaryPaths(INODE, "heroImage", List.of(NEW_REVISION)); + + doThrow(new DotDataException("S3 unavailable")).when(storage) + .deleteBinaryPaths(INODE, "heroImage", List.of(NEW_REVISION)); + assertDoesNotThrow(() -> listener.get().run()); + } + } +} diff --git a/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetCleanupTransactionTest.java b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetCleanupTransactionTest.java new file mode 100644 index 000000000000..ba28894dc606 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetCleanupTransactionTest.java @@ -0,0 +1,107 @@ +package com.dotcms.storage.binary; + +import com.dotcms.cluster.business.ServerAPI; +import com.dotcms.jobs.business.api.JobQueueManagerAPI; +import com.dotcms.jobs.business.queue.PostgresJobQueue; +import com.dotcms.storage.AssetStorageFeature; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.startup.runonce.Task250113CreatePostgresJobQueueTables; +import com.dotmarketing.util.Config; +import java.sql.Connection; +import java.sql.DriverManager; +import java.util.UUID; +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.mockito.Mockito.*; + +/** Runs against a disposable PostgreSQL database; no demo data is needed. */ +class BinaryAssetCleanupTransactionTest { + @Test void cleanupIntentCommitsAndRollsBackWithContentDeletion() throws Exception { + String jdbc = System.getProperty("s3.test.jdbc"); + assumeTrue(jdbc != null, "Supply s3.test.jdbc for an isolated PostgreSQL database"); + String previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + String schema = "binary_cleanup_" + UUID.randomUUID().toString().replace("-", ""); + try (Connection writer = DriverManager.getConnection(jdbc, "binary-storage-test", "binary-storage-test"); + Connection observer = DriverManager.getConnection(jdbc, "binary-storage-test", "binary-storage-test"); + var locator = mockStatic(APILocator.class)) { + Config.setProperty(AssetStorageFeature.FLAG, true); + writer.createStatement().execute("create schema " + schema); + try { + writer.createStatement().execute("set search_path to " + schema); + observer.createStatement().execute("set search_path to " + schema); + DbConnectionFactory.setConnection(writer); + new Task250113CreatePostgresJobQueueTables().executeUpgrade(); + writer.createStatement().execute("create table contentlet (inode varchar(255) primary key)"); + writer.createStatement().execute("insert into contentlet values ('abc123')"); + var server = mock(ServerAPI.class); + when(server.readServerId()).thenReturn("binary-cleanup-test"); + locator.when(APILocator::getServerAPI).thenReturn(server); + var queue = new PostgresJobQueue(); + var manager = mock(JobQueueManagerAPI.class); + when(manager.createJob(eq(BinaryAssetCleanupProcessor.QUEUE), anyMap())) + .thenAnswer(call -> { + try { + return queue.createJob(call.getArgument(0), call.getArgument(1)); + } catch (com.dotcms.jobs.business.queue.error.JobQueueException e) { + throw new com.dotmarketing.exception.DotDataException("Unable to record cleanup", e); + } + }); + locator.when(APILocator::getJobQueueManagerAPI).thenReturn(manager); + // Enqueue records the inode's stored inventory; listing is its only storage call. + var binaries = mock(BinaryAssetStorageAPI.class); + when(binaries.listBinaryPaths("abc123")).thenReturn(java.util.List.of()); + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(binaries); + locator.when(APILocator::getFileMetadataAPI).thenReturn(mock(com.dotcms.storage.FileMetadataAPI.class)); + + writer.setAutoCommit(false); + writer.createStatement().execute("delete from contentlet where inode = 'abc123'"); + BinaryAssetCleanupProcessor.enqueue("abc123"); + assertEquals(1, count(writer, "job_queue")); + assertEquals(0, count(observer, "job_queue"), "Workers cannot see uncommitted cleanup"); + assertEquals(1, count(observer, "contentlet")); + writer.rollback(); + assertEquals(0, count(observer, "job_queue")); + assertEquals(0, count(observer, "job")); + assertEquals(0, count(observer, "job_history")); + assertEquals(1, count(observer, "contentlet")); + + // Fail the last queue insert: neither the content deletion nor partial intent may commit. + writer.createStatement().execute("drop table job_history"); + writer.createStatement().execute("delete from contentlet where inode = 'abc123'"); + assertThrows(com.dotmarketing.exception.DotDataException.class, + () -> BinaryAssetCleanupProcessor.enqueue("abc123")); + writer.rollback(); + assertEquals(1, count(observer, "contentlet")); + assertEquals(0, count(observer, "job")); + assertEquals(0, count(observer, "job_queue")); + + writer.createStatement().execute("delete from contentlet where inode = 'abc123'"); + BinaryAssetCleanupProcessor.enqueue("abc123"); + writer.commit(); + assertEquals(0, count(observer, "contentlet")); + assertEquals(1, count(observer, "job_queue")); + assertEquals(1, count(observer, "job")); + assertEquals(1, count(observer, "job_history")); + verify(binaries, never()).deleteBinaryPaths(anyString(), anyString(), anyList()); + verify(binaries, never()).deleteAllBinaries(anyString()); + } finally { + writer.rollback(); + writer.setAutoCommit(true); + writer.createStatement().execute("drop schema " + schema + " cascade"); + } + } finally { + DbConnectionFactory.closeConnection(); + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + } + } + + private int count(Connection connection, String table) throws Exception { + try (var statement = connection.createStatement(); + var results = statement.executeQuery("select count(*) from " + table)) { + assertTrue(results.next()); + return results.getInt(1); + } + } +} diff --git a/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetReferenceTest.java b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetReferenceTest.java index aefecc861f8b..1841591db04e 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetReferenceTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetReferenceTest.java @@ -1,10 +1,16 @@ package com.dotcms.storage.binary; +import com.dotcms.content.model.type.system.AbstractBinaryFieldType; +import com.dotcms.contenttype.model.field.BinaryField; import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.StoragePersistenceAPI; import com.dotmarketing.util.Config; import com.dotmarketing.util.ConfigUtils; +import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; import java.util.UUID; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -50,10 +56,144 @@ class BinaryAssetReferenceTest { } finally { Config.setProperty(AssetStorageFeature.FLAG, previous); } } + @Test void metadataFileUsesLegacyFileXmlWithoutLosingTheSourceReference() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var paths = mockStatic(ConfigUtils.class)) { + Config.setProperty(AssetStorageFeature.FLAG, true); + paths.when(ConfigUtils::getAssetPath).thenReturn(root.toString()); + final File original = new File(root.toFile(), "a/b/abc123/HeroImage/Mixed.GIF"); + final String metadataKey = BinaryAssetReference.newMetadataKey(original, "abc123", "HeroImage"); + final File snapshot = BinaryAssetReference.withMetadata(original, "abc123", "HeroImage", metadataKey); + final var serializer = com.dotcms.util.xstream.XStreamHandler.newXStreamInstance(); + final var input = new HashMap(); + input.put("asset", snapshot); + final String xml = serializer.toXML(input); + assertFalse(xml.contains("MetadataFile")); + final Map restored = (Map) serializer.fromXML(xml); + assertEquals(File.class, restored.get("asset").getClass()); + assertEquals(original, restored.get("asset")); + assertEquals(metadataKey, BinaryAssetReference.metadataKeyOf(snapshot)); + Config.setProperty(AssetStorageFeature.FLAG, false); + assertEquals(serializer.toXML(new HashMap<>(Map.of("asset", original))), xml, + "Enabled wrappers must keep the original File wire format"); + } finally { + Config.setProperty(AssetStorageFeature.FLAG, previous); + } + } + + @Test void immutableFieldReferenceSerializesWithoutReadingFileAndFlagOffRetainsLegacyJson() throws Exception { + String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var paths = mockStatic(ConfigUtils.class)) { + paths.when(ConfigUtils::getAssetPath).thenReturn(root.toString()); + Config.setProperty(AssetStorageFeature.FLAG, true); + String key = "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"; + File file = BinaryAssetReference.localFile("abc123", "HeroImage", key); + assertFalse(file.exists()); + assertEquals(key, BinaryAssetReference.keyOf(file, "abc123", "HeroImage")); + assertNull(BinaryAssetReference.keyOf(file, "def456", "HeroImage"), "A new content version must copy the bytes before publishing its own reference"); + assertNull(BinaryAssetReference.keyOf(file, "abc123", "OtherImage")); + BinaryField field = mock(BinaryField.class, CALLS_REAL_METHODS); + var value = (AbstractBinaryFieldType) field.fieldValue(file).orElseThrow().build(); + assertEquals("Friday.PNG", value.value()); + assertEquals(key, value.storageKey()); + String json = new ObjectMapper().writeValueAsString(value); + assertTrue(json.contains(key)); + assertEquals(key, ((AbstractBinaryFieldType) new ObjectMapper().readValue(json, + com.dotcms.content.model.FieldValue.class)).storageKey()); + assertFalse(file.exists()); + + Config.setProperty(AssetStorageFeature.FLAG, false); + var legacy = (AbstractBinaryFieldType) field.fieldValue(file).orElseThrow().build(); + assertNull(legacy.storageKey()); + assertFalse(new ObjectMapper().writeValueAsString(legacy).contains("storageKey")); + var provider = mock(StoragePersistenceAPI.class); + var api = new BinaryAssetStorageAPIImpl(provider); + assertThrows(IllegalStateException.class, () -> api.storeRevision("abc123", "HeroImage", "Friday.PNG", file)); + assertThrows(IllegalStateException.class, () -> api.getRevisionFile("abc123", "HeroImage", key)); + verifyNoInteractions(provider); + } finally { + Config.setProperty(AssetStorageFeature.FLAG, previous); + } + } + + @Test void metadataEditsHaveIndependentReferencesAndSurviveJsonAndColdRestoration() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var paths = mockStatic(ConfigUtils.class)) { + Config.setProperty(AssetStorageFeature.FLAG, true); + paths.when(ConfigUtils::getAssetPath).thenReturn(root.toString()); + final String key = "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"; + final File binary = BinaryAssetReference.localFile("abc123", "HeroImage", key); + final String firstKey = BinaryAssetReference.newMetadataKey(binary, "abc123", "HeroImage"); + final File first = BinaryAssetReference.withMetadata(binary, "abc123", "HeroImage", firstKey); + final String secondKey = BinaryAssetReference.newMetadataKey(first, "abc123", "HeroImage"); + assertNotEquals(firstKey, secondKey); + assertEquals(key, BinaryAssetReference.keyOf(first)); + assertEquals(firstKey, BinaryAssetReference.metadataKeyOf( + BinaryAssetReference.preserveMetadata(binary, first))); + assertNull(BinaryAssetReference.metadataKeyOf(first, "def456", "HeroImage")); + assertThrows(IllegalArgumentException.class, () -> BinaryAssetReference.withMetadata( + binary, "abc123", "OtherField", firstKey)); + assertThrows(IllegalArgumentException.class, () -> BinaryAssetReference.withMetadata( + binary, "abc123", "HeroImage", firstKey + "/../outside")); + final BinaryField field = mock(BinaryField.class, CALLS_REAL_METHODS); + final var value = (AbstractBinaryFieldType) field.fieldValue(first).orElseThrow().build(); + final ObjectMapper mapper = new ObjectMapper(); + final String json = mapper.writeValueAsString(value); + assertEquals(firstKey, ((AbstractBinaryFieldType) mapper.readValue(json, + com.dotcms.content.model.FieldValue.class)).metadataStorageKey()); + final File hydrated = BinaryAssetReference.fromJson(mapper.readTree(json), "abc123", "HeroImage") + .localFile("abc123", "HeroImage"); + assertEquals(firstKey, BinaryAssetReference.metadataKeyOf(hydrated)); + assertFalse(hydrated.exists(), "JSON reconstruction must not require stored bytes"); + Config.setProperty(AssetStorageFeature.FLAG, false); + assertFalse(mapper.writeValueAsString(field.fieldValue(first).orElseThrow().build()) + .contains("metadataStorageKey")); + } finally { + Config.setProperty(AssetStorageFeature.FLAG, previous); + } + } + @Test void revisionReferencesCannotEscapeTheirOwner() { String key = "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"; assertThrows(IllegalArgumentException.class, () -> BinaryAssetReference.localFile("abc123", "OtherField", key)); assertThrows(IllegalArgumentException.class, () -> BinaryAssetReference.localFile("abc123", "HeroImage", key + "/../../outside")); assertThrows(IllegalArgumentException.class, () -> BinaryAssetReference.localFile("../outside", "HeroImage", key)); } + + @Test void ownerIsReadFromTheRevisionKeyLayout() { + String key = "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"; + assertEquals(new BinaryAssetReference.Owner("abc123", "HeroImage"), BinaryAssetReference.ownerOf(key)); + assertNull(BinaryAssetReference.ownerOf("a/b/abc123/HeroImage/Friday.PNG")); + assertNull(BinaryAssetReference.ownerOf(key + "/extra")); + } + + @Test void metadataIdentityFollowsTheSnapshotWithoutFilesystemAccess() { + String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var paths = mockStatic(ConfigUtils.class)) { + paths.when(ConfigUtils::getAssetPath).thenReturn(root.toString()); + var metadata = mock(com.dotcms.storage.FileMetadataAPI.class, CALLS_REAL_METHODS); + var content = new com.dotmarketing.portlets.contentlet.model.Contentlet(); + content.setInode("abc123"); + String firstKey = "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"; + File first = BinaryAssetReference.localFile("abc123", "HeroImage", firstKey); + content.getMap().put("HeroImage", first); + Config.setProperty(AssetStorageFeature.FLAG, true); + String firstMetadata = metadata.getFileName(content, "HeroImage"); + assertEquals("/" + firstKey + "-metadata.json", firstMetadata); + assertEquals(firstMetadata, metadata.getMetadataCacheKey(content, "HeroImage")); + File second = BinaryAssetReference.localFile("abc123", "HeroImage", + "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"); + content.getMap().put("HeroImage", second); + assertNotEquals(firstMetadata, metadata.getFileName(content, "HeroImage")); + assertNotEquals(firstMetadata, metadata.getMetadataCacheKey(content, "HeroImage")); + assertFalse(first.exists()); + assertFalse(second.exists()); + + Config.setProperty(AssetStorageFeature.FLAG, false); + assertEquals("/a/b/abc123/HeroImage-metadata.json", metadata.getFileName(content, "HeroImage")); + assertEquals("abc123:HeroImage", metadata.getMetadataCacheKey(content, "HeroImage")); + } finally { + Config.setProperty(AssetStorageFeature.FLAG, previous); + } + } } diff --git a/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryFieldCleanupCheckpointTest.java b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryFieldCleanupCheckpointTest.java new file mode 100644 index 000000000000..80f9c8809c38 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryFieldCleanupCheckpointTest.java @@ -0,0 +1,205 @@ +package com.dotcms.storage.binary; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.mockito.Mockito.*; + +import com.dotcms.business.interceptor.CoreDatabaseConnectionOps; +import com.dotcms.business.interceptor.CoreInterceptorLogger; +import com.dotcms.business.interceptor.CoreLicenseOps; +import com.dotcms.business.interceptor.InterceptorServiceProvider; +import com.dotcms.business.interceptor.TransactionOps; +import com.dotcms.cluster.business.ServerAPI; +import com.dotcms.jobs.business.api.JobQueueManagerAPI; +import com.dotcms.jobs.business.error.JobProcessingException; +import com.dotcms.jobs.business.queue.PostgresJobQueue; +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 com.dotmarketing.startup.runonce.Task250113CreatePostgresJobQueueTables; +import com.dotmarketing.util.Config; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.junit.jupiter.api.Test; + +/** + * Real PostgreSQL checks for how a removed binary field is archived across a content type: each row + * commits on its own with a saved cursor, and only the row being archived is locked. The per-row + * archive step itself (ZIP upload, JSON rewrite) is replaced here; its behavior has separate CMS tests. + */ +class BinaryFieldCleanupCheckpointTest { + private static final String TYPE = "type-a"; + private static final String FIELD = "image"; + private static final String STORED = "{\"fields\":{\"image\":{\"type\":\"Binary\",\"value\":\"Hero.PNG\"}}}"; + private static final String CLEARED = "{\"fields\":{}}"; + + @Test + void eachRowCommitsWithItsCursorAndARetryResumesAfterTheLastCommittedRow() throws Exception { + final String jdbc = System.getProperty("s3.test.jdbc"); + assumeTrue(jdbc != null, "Supply s3.test.jdbc for an isolated PostgreSQL database"); + final String previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + final String schema = "binary_field_cleanup_" + UUID.randomUUID().toString().replace("-", ""); + // Unit tests do not run application startup, which installs the transaction handling behind + // LocalTransaction. Install connection-level transactions so each row's commit and lock are real. + final var previousDb = InterceptorServiceProvider.getDatabaseOps(); + final var previousTx = InterceptorServiceProvider.getTransactionOps(); + final var previousLicense = InterceptorServiceProvider.getLicenseOps(); + final var previousLogger = InterceptorServiceProvider.getLogger(); + InterceptorServiceProvider.init(CoreDatabaseConnectionOps.INSTANCE, new ConnectionTransactions(), + CoreLicenseOps.INSTANCE, CoreInterceptorLogger.INSTANCE); + try (Connection writer = DriverManager.getConnection(jdbc, "binary-storage-test", "binary-storage-test"); + Connection observer = DriverManager.getConnection(jdbc, "binary-storage-test", "binary-storage-test"); + var locator = mockStatic(APILocator.class); + var rows = mockStatic(BinaryFieldCleanupProcessor.class, CALLS_REAL_METHODS)) { + Config.setProperty(AssetStorageFeature.FLAG, true); + writer.createStatement().execute("create schema " + schema); + try { + writer.createStatement().execute("set search_path to " + schema); + observer.createStatement().execute("set search_path to " + schema); + DbConnectionFactory.setConnection(writer); + new Task250113CreatePostgresJobQueueTables().executeUpgrade(); + writer.createStatement().execute("create table contentlet (inode varchar(255) primary key, " + + "identifier varchar(255), structure_inode varchar(255), mod_date timestamp, " + + "contentlet_as_json jsonb, note text)"); + for (String inode : List.of("c1", "c2", "c3", "c4", "c5")) insert(writer, inode, TYPE, "1 day"); + insert(writer, "c6", TYPE, "-1 day"); // edited after the field was removed + insert(writer, "o1", "type-b", "1 day"); // another content type + + final var server = mock(ServerAPI.class); + when(server.readServerId()).thenReturn("field-cleanup-checkpoint-test"); + locator.when(APILocator::getServerAPI).thenReturn(server); + final var queue = new PostgresJobQueue(); + final var manager = mock(JobQueueManagerAPI.class); + when(manager.getJob(anyString())).thenAnswer(call -> queue.getJob(call.getArgument(0))); + when(manager.createJob(anyString(), anyMap())).thenAnswer(call -> queue.createJob( + call.getArgument(0), call.getArgument(1))); + locator.when(APILocator::getJobQueueManagerAPI).thenReturn(manager); + final String id = queue.createJob(BinaryFieldCleanupProcessor.QUEUE, Map.of("type", TYPE, + "field", FIELD, "deletedBefore", System.currentTimeMillis())); + + final List archived = new ArrayList<>(); + final boolean[] failOnC3 = {true}; + rows.when(() -> BinaryFieldCleanupProcessor.archiveRow(anyMap(), eq(FIELD))).thenAnswer(call -> { + final String inode = ((Map) call.getArgument(0)).get("inode").toString(); + archived.add(inode); + // Only the row being archived may be locked: finished and waiting rows stay editable. + assertFalse(canEdit(observer, inode), inode + " must be locked while it is archived"); + for (String other : List.of("c1", "c2", "c3", "c4", "c5")) { + if (!other.equals(inode)) assertTrue(canEdit(observer, other), other + " must not be locked while " + inode + " is archived"); + } + new DotConnect().setSQL("update contentlet set contentlet_as_json = ?::jsonb where inode = ?") + .addParam(CLEARED).addParam(inode).loadResult(); + if (inode.equals("c3") && failOnC3[0]) throw new DotDataException("S3 unavailable"); + return null; + }); + + final var interrupted = assertThrows(JobProcessingException.class, + () -> new BinaryFieldCleanupProcessor().process(queue.getJob(id))); + assertEquals("S3 unavailable", rootCause(interrupted).getMessage(), interrupted.toString()); + assertEquals(List.of("c1", "c2", "c3"), archived); + assertEquals(CLEARED, json(observer, "c1"), "Rows archived before the failure stay committed"); + assertEquals(CLEARED, json(observer, "c2")); + assertEquals(STORED, json(observer, "c3"), "The failed row rolls back on its own"); + assertEquals(STORED, json(observer, "c4")); + assertEquals("c2", cursor(observer, id), "The cursor commits with the last archived row"); + + failOnC3[0] = false; + archived.clear(); + new BinaryFieldCleanupProcessor().process(queue.getJob(id)); + assertEquals(List.of("c3", "c4", "c5"), archived, "A retry resumes after the committed cursor"); + for (String inode : List.of("c1", "c2", "c3", "c4", "c5")) assertEquals(CLEARED, json(observer, inode)); + assertEquals(STORED, json(observer, "c6"), "Edits made after the field was removed are kept"); + assertEquals(STORED, json(observer, "o1")); + } finally { + writer.setAutoCommit(true); + writer.createStatement().execute("drop schema " + schema + " cascade"); + } + } finally { + DbConnectionFactory.closeConnection(); + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + InterceptorServiceProvider.init(previousDb, previousTx, previousLicense, previousLogger); + } + } + + /** + * The transaction calls LocalTransaction makes, applied directly to the test's JDBC connection. + * The application's implementation goes through Hibernate, which needs full startup. + */ + private static final class ConnectionTransactions implements TransactionOps { + @Override public boolean startLocalTransactionIfNeeded() throws Exception { + final Connection connection = DbConnectionFactory.getConnection(); + if (!connection.getAutoCommit()) return false; + connection.setAutoCommit(false); + return true; + } + @Override public void commitTransaction() throws Exception { DbConnectionFactory.getConnection().commit(); } + @Override public void rollbackTransaction() { + try { DbConnectionFactory.getConnection().rollback(); } catch (SQLException e) { throw new IllegalStateException(e); } + } + @Override public void handleTransactionInterruption(Connection connection, StackTraceElement[] stack) { } + @Override public void throwException(Throwable t) throws Exception { throw t instanceof Exception e ? e : new Exception(t); } + @Override public String getConfigProperty(String key, String defaultValue) { return defaultValue; } + @Override public void closeSessionSilently() { throw new UnsupportedOperationException(); } + @Override public void startTransaction() { throw new UnsupportedOperationException(); } + @Override public Object getSession() { throw new UnsupportedOperationException(); } + @Override public void setSession(Object session) { throw new UnsupportedOperationException(); } + @Override public Object createNewSession(Connection connection) { throw new UnsupportedOperationException(); } + } + + private static void insert(Connection connection, String inode, String type, String age) throws SQLException { + try (var insert = connection.prepareStatement("insert into contentlet values (?, ?, ?, " + + "current_timestamp - ?::interval, ?::jsonb, null)")) { + insert.setString(1, inode); + insert.setString(2, "id-" + inode); + insert.setString(3, type); + insert.setString(4, age); + insert.setString(5, STORED); + insert.executeUpdate(); + } + } + + /** Tries a short edit from another connection; a held row lock makes it time out. */ + private static boolean canEdit(Connection observer, String inode) throws SQLException { + observer.createStatement().execute("set lock_timeout = '300ms'"); + try (var update = observer.prepareStatement("update contentlet set note = 'probe' where inode = ?")) { + update.setString(1, inode); + update.executeUpdate(); + return true; + } catch (SQLException locked) { + return false; + } + } + + private static String json(Connection connection, String inode) throws SQLException { + try (var query = connection.prepareStatement("select contentlet_as_json::text from contentlet where inode = ?")) { + query.setString(1, inode); + try (var rows = query.executeQuery()) { + assertTrue(rows.next()); + return rows.getString(1).replace(" ", ""); + } + } + } + + private static String cursor(Connection connection, String id) throws SQLException { + try (var query = connection.prepareStatement("select parameters->>'afterInode' from job where id = ?")) { + query.setString(1, id); + try (var rows = query.executeQuery()) { + assertTrue(rows.next()); + return rows.getString(1); + } + } + } + + private static Throwable rootCause(Throwable failure) { + Throwable cause = failure; + while (cause.getCause() != null) cause = cause.getCause(); + return cause; + } +} diff --git a/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryFieldCleanupProcessorTest.java b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryFieldCleanupProcessorTest.java new file mode 100644 index 000000000000..b1fe4d330601 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryFieldCleanupProcessorTest.java @@ -0,0 +1,67 @@ +package com.dotcms.storage.binary; + +import com.dotcms.contenttype.model.field.BinaryField; +import com.dotcms.jobs.business.error.JobProcessingException; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.db.HibernateUtil; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.quartz.job.CleanUpFieldReferencesJob; +import com.dotmarketing.util.Config; +import java.util.Date; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class BinaryFieldCleanupProcessorTest { + @Test void disabledFieldDeletionRetainsLegacySchedulingAndRejectsS3Work() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var locator = mockStatic(APILocator.class); var transactions = mockStatic(HibernateUtil.class)) { + Config.setProperty(AssetStorageFeature.FLAG, false); + CleanUpFieldReferencesJob.triggerCleanUpJob(mock(BinaryField.class), mock(com.liferay.portal.model.User.class)); + transactions.verify(() -> HibernateUtil.addCommitListenerNoThrow(any())); + assertThrows(DotDataException.class, () -> BinaryFieldCleanupProcessor.enqueue("type", "image", new Date())); + final Job job = mock(Job.class); + assertThrows(JobProcessingException.class, () -> new BinaryFieldCleanupProcessor().process(job)); + locator.verifyNoInteractions(); + } finally { Config.setProperty(AssetStorageFeature.FLAG, previous); } + } + + @Test void fieldRemovalRequiresDurableQueueWriteInItsTransaction() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var locator = mockStatic(APILocator.class); var connection = mockStatic(DbConnectionFactory.class)) { + Config.setProperty(AssetStorageFeature.FLAG, true); + assertThrows(DotDataException.class, () -> BinaryFieldCleanupProcessor.enqueue("type", "image", new Date())); + connection.when(DbConnectionFactory::inTransaction).thenReturn(true); + assertThrows(com.dotcms.jobs.business.error.JobValidationException.class, + () -> new BinaryFieldCleanupProcessor().validate(Map.of("userId", "backend-user"))); + final var queue = mock(com.dotcms.jobs.business.api.JobQueueManagerAPI.class); + locator.when(APILocator::getJobQueueManagerAPI).thenReturn(queue); + when(queue.createJob(eq(BinaryFieldCleanupProcessor.QUEUE), anyMap())).thenThrow(new DotDataException("queue unavailable")); + assertThrows(DotDataException.class, () -> BinaryFieldCleanupProcessor.enqueue("type", "image", new Date())); + locator.verify(APILocator::getBinaryAssetStorageAPI, never()); + } finally { Config.setProperty(AssetStorageFeature.FLAG, previous); } + } + + @Test void exactInventoryRejectsSiblingAndTraversalBeforeAnyDeletion() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try { + Config.setProperty(AssetStorageFeature.FLAG, true); + final var storage = mock(StoragePersistenceAPI.class); + final var api = new BinaryAssetStorageAPIImpl(storage); + for (String path : List.of("a/b/abc/imageOther/x", "a/b/abc/image/../other/x", "a/b/abc/image")) { + assertThrows(IllegalArgumentException.class, () -> api.deleteBinaryPaths("abc", "image", + List.of("a/b/abc/image/good", path))); + } + verifyNoInteractions(storage); + Config.setProperty(AssetStorageFeature.FLAG, false); + assertThrows(DotDataException.class, () -> api.deleteBinaryPaths("abc", "image", List.of())); + verifyNoInteractions(storage); + } finally { Config.setProperty(AssetStorageFeature.FLAG, previous); } + } +} diff --git a/dotCMS/src/test/java/com/dotcms/storage/binary/ContentletBackupStorageGateTest.java b/dotCMS/src/test/java/com/dotcms/storage/binary/ContentletBackupStorageGateTest.java new file mode 100644 index 000000000000..a395572a3cc7 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/storage/binary/ContentletBackupStorageGateTest.java @@ -0,0 +1,55 @@ +package com.dotcms.storage.binary; + +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotRuntimeException; +import com.dotmarketing.portlets.contentlet.business.ContentletAPI; +import com.dotmarketing.portlets.contentlet.business.ContentletAPIInterceptor; +import com.dotmarketing.portlets.contentlet.business.ContentletAPIPreHook; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.util.Config; +import java.util.List; +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class ContentletBackupStorageGateTest { + @Test + void disabledBackupDoesNotInitializeStorageAndAllVersionDeletionRetainsLegacyNoOp() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var locator = mockStatic(APILocator.class)) { + Config.setProperty(AssetStorageFeature.FLAG, false); + final var remote = mock(StoragePersistenceAPI.class); + final var backups = new ContentletBackupStorage(remote); + assertThrows(DotDataException.class, () -> backups.store(new Contentlet())); + assertThrows(DotDataException.class, () -> backups.list("identifier")); + final var delegate = mock(ContentletAPI.class); + locator.when(APILocator::getContentletAPIImpl).thenReturn(delegate); + new ContentletAPIInterceptor().deleteAllVersionsandBackup(List.of(), null, false); + verifyNoInteractions(remote, delegate); + } finally { Config.setProperty(AssetStorageFeature.FLAG, previous); } + } + + @Test + void enabledAllVersionDeletionHonorsVetoAndPropagatesBackupFailure() throws Exception { + final String previous = Config.getStringProperty(AssetStorageFeature.FLAG, null); + try (var locator = mockStatic(APILocator.class)) { + Config.setProperty(AssetStorageFeature.FLAG, true); + final var delegate = mock(ContentletAPI.class); + locator.when(APILocator::getContentletAPIImpl).thenReturn(delegate); + final var interceptor = new ContentletAPIInterceptor(); + final var hook = mock(ContentletAPIPreHook.class); + interceptor.addPreHook(hook); + final List contents = List.of(new Contentlet()); + assertThrows(DotRuntimeException.class, () -> interceptor.deleteAllVersionsandBackup(contents, null, false)); + verifyNoInteractions(delegate); + when(hook.delete(contents, null, false, true)).thenReturn(true); + final var failure = new DotDataException("backup unavailable"); + doThrow(failure).when(delegate).deleteAllVersionsandBackup(contents, null, false); + assertSame(failure, assertThrows(DotDataException.class, + () -> interceptor.deleteAllVersionsandBackup(contents, null, false))); + } finally { Config.setProperty(AssetStorageFeature.FLAG, previous); } + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java index b772c96abb9d..c93697fcbfdc 100644 --- a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java +++ b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java @@ -76,7 +76,10 @@ FolderBulkDuplicateProcessorIT.class, FolderBulkDuplicateCancellationIT.class, FolderBulkDuplicateNotificationIT.class, - FolderBulkDuplicateHeartbeatIT.class + FolderBulkDuplicateHeartbeatIT.class, + com.dotcms.storage.binary.BinaryAssetStorageIntegrationTest.class, + com.dotcms.storage.binary.ContentletBackupStorageTest.class, + com.dotcms.storage.binary.SharedAssetStorageIntegrationTest.class }) public class Junit5Suite1 { diff --git a/dotcms-integration/src/test/java/com/dotcms/content/model/hydration/MetadataDelegateTest.java b/dotcms-integration/src/test/java/com/dotcms/content/model/hydration/MetadataDelegateTest.java index 8f66eb2b98ad..1d9b19f340f1 100644 --- a/dotcms-integration/src/test/java/com/dotcms/content/model/hydration/MetadataDelegateTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/content/model/hydration/MetadataDelegateTest.java @@ -79,4 +79,20 @@ public void Test_File_Asset_Path_Normalized() throws IOException, DotDataExcepti } + @Test + public void s3NormalizationRetainsColdLegacyAndRevisionPaths() throws Exception { + org.junit.Assume.assumeTrue(com.dotcms.storage.AssetStorageFeature.isEnabled()); + final String inode = java.util.UUID.randomUUID().toString(); + final String prefix = inode.charAt(0) + "/" + inode.charAt(1) + "/" + inode + "/HeroImage/"; + final MetadataDelegate delegate = new MetadataDelegate(); + for (String suffix : java.util.List.of("Mixed-Case.PNG", + ".revisions/" + java.util.UUID.randomUUID() + "/Mixed-Case.PNG")) { + final String key = prefix + suffix; + final File expected = new File(ConfigUtils.getAssetPath(), key); + Assert.assertFalse(expected.exists()); + Assert.assertEquals(expected, delegate.normalize(new File("/old/installation/assets", key))); + Assert.assertFalse("Normalization must not materialize the source", expected.exists()); + } + } + } diff --git a/dotcms-integration/src/test/java/com/dotcms/storage/FileMetadataAPITest.java b/dotcms-integration/src/test/java/com/dotcms/storage/FileMetadataAPITest.java index e75b337ad49e..c9bed0d5722d 100644 --- a/dotcms-integration/src/test/java/com/dotcms/storage/FileMetadataAPITest.java +++ b/dotcms-integration/src/test/java/com/dotcms/storage/FileMetadataAPITest.java @@ -1103,7 +1103,7 @@ public void calculateEditableAsTextOnlyWhenMetadataIsPresent() throws Exception // ║ Generating Test data ║ // ╚════════════════════════╝ final Contentlet fileAssetContent = getFileAssetContent(true, 1, TestFile.PDF); // fileAsset - CacheLocator.getMetadataCache().addMetadataMap(fileAssetContent.getInode() + ":" + FileAssetAPI.BINARY_FIELD, new HashMap<>()); + CacheLocator.getMetadataCache().addMetadataMap(fileMetadataAPI.getMetadataCacheKey(fileAssetContent, FileAssetAPI.BINARY_FIELD), new HashMap<>()); final Object metadataMap = fileAssetContent.get(FileAssetAPI.META_DATA_FIELD); // ╔════════════════════════╗ @@ -1145,7 +1145,7 @@ public void Test_Generate_Metadata_Reads_Existing_Without_Touching_Binary() thro assertTrue(binary.exists()); assertTrue(binary.delete()); CacheLocator.getMetadataCache() - .removeMetadata(fileAssetContent.getInode() + ":" + FILE_ASSET); + .removeMetadata(fileMetadataAPI.getMetadataCacheKey(fileAssetContent, FILE_ASSET)); // ╔════════════════════════╗ // ║ Executing Assertions ║ @@ -1160,6 +1160,7 @@ public void Test_Generate_Metadata_Reads_Existing_Without_Touching_Binary() thro reRead.getMap().get(BasicMetadataFields.SHA256_META_KEY.key())); assertNotNull("full metadata must also be served from storage", secondPass.getFullMetadataMap().get(FILE_ASSET)); + assertFalse("Reading existing metadata must not restore or reopen the binary", binary.exists()); } } diff --git a/dotcms-integration/src/test/java/com/dotcms/storage/binary/BinaryAssetStorageIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/storage/binary/BinaryAssetStorageIntegrationTest.java new file mode 100644 index 000000000000..fbd838370595 --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/storage/binary/BinaryAssetStorageIntegrationTest.java @@ -0,0 +1,834 @@ +package com.dotcms.storage.binary; + +import com.dotcms.contenttype.model.field.BinaryField; +import com.dotcms.contenttype.model.field.Field; +import com.dotcms.contenttype.model.field.TextField; +import com.dotcms.contenttype.model.type.ContentType; +import com.dotcms.datagen.ContentTypeDataGen; +import com.dotcms.datagen.ContentletDataGen; +import com.dotcms.datagen.FieldDataGen; +import com.dotcms.datagen.FileAssetDataGen; +import com.dotcms.datagen.FolderDataGen; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.portlets.contentlet.business.ContentletAPI; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; +import com.dotmarketing.portlets.folders.model.Folder; +import com.liferay.portal.model.User; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.nio.file.Files; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Integration tests for the Phase 2 BinaryAssetStorageAPI migration. + * Exercises migrated code paths (Contentlet.getBinary, handleBinaries, + * content versioning, FileAsset flow) end-to-end with real storage. + */ +@Tag("BinaryStorage") +public class BinaryAssetStorageIntegrationTest { + + private static User user; + private static ContentletAPI contentletAPI; + private static BinaryAssetStorageAPI binaryAssetStorageAPI; + + @BeforeAll + static void setUp() throws Exception { + IntegrationTestInitService.getInstance().init(); + assertEquals(Boolean.getBoolean("s3.cms.enabled"), com.dotcms.storage.AssetStorageFeature.isEnabled(), + "Integration configuration must exercise the requested storage mode"); + user = APILocator.systemUser(); + contentletAPI = APILocator.getContentletAPI(); + binaryAssetStorageAPI = APILocator.getBinaryAssetStorageAPI(); + } + + @Test + void s3ReplacementRollbackPreservesCommittedBinaryThroughCmsCheckin( + @org.junit.jupiter.api.io.TempDir java.nio.file.Path uploads) throws Exception { + org.junit.jupiter.api.Assumptions.assumeTrue(Boolean.getBoolean("s3.cms.enabled")); + assertTrue(com.dotcms.storage.AssetStorageFeature.isEnabled(), "CMS must actually opt in to S3"); + final ContentType type = new ContentTypeDataGen().nextPersisted(); + try { + final Field title = new FieldDataGen().velocityVarName("title").contentTypeId(type.id()) + .type(TextField.class).nextPersisted(); + ContentTypeDataGen.addField(title); + final Field binary = new FieldDataGen().velocityVarName("heroImage").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted(); + ContentTypeDataGen.addField(binary); + final Field attachment = new FieldDataGen().velocityVarName("attachment").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted(); + ContentTypeDataGen.addField(attachment); + final java.nio.file.Path before = uploads.resolve("before/Friday.Txt"); + final java.nio.file.Path after = uploads.resolve("after/Friday.Txt"); + Files.createDirectories(before.getParent()); + Files.createDirectories(after.getParent()); + Files.writeString(before, "last committed bytes"); + Files.writeString(after, "replacement bytes"); + final ContentletDataGen generator = new ContentletDataGen(type); + generator.setProperty("title", "S3 transaction test"); + generator.setProperty("heroImage", before.toFile()); + generator.setProperty("attachment", before.toFile()); + final Contentlet original = generator.nextPersisted(); + final File originalFile = original.getBinary("heroImage"); + final String originalKey = BinaryAssetReference.find(original.getInode(), "heroImage"); + assertNotNull(originalKey); + assertTrue(originalKey.contains("/.revisions/")); + assertEquals("Friday.Txt", originalFile.getName()); + assertBinaryMetadataLength(original, Files.size(before)); + APILocator.getFileMetadataAPI().putCustomMetadataAttributes(original, + java.util.Map.of("heroImage", java.util.Map.of("credit", "Original author"), + "attachment", java.util.Map.of("credit", "Original attachment author"))); + + File rolledBackFile; + com.dotmarketing.db.HibernateUtil.startTransaction(); + try { + final Contentlet edit = new Contentlet(original); + edit.setBinary("heroImage", after.toFile()); + final Contentlet changed = contentletAPI.checkinWithoutVersioning(edit, + (com.dotmarketing.portlets.structure.model.ContentletRelationships) null, + null, null, user, false); + assertEquals(original.getInode(), changed.getInode()); + rolledBackFile = changed.getBinary("heroImage"); + final String replacementKey = BinaryAssetReference.find(original.getInode(), "heroImage"); + assertNotNull(replacementKey, "Check-in must publish a stored reference, not just the upload filename"); + assertNotEquals(originalKey, replacementKey); + assertEquals(replacementKey, + BinaryAssetReference.keyOf(rolledBackFile), "Returned binary must reference stored bytes: " + rolledBackFile); + assertEquals("replacement bytes", Files.readString(rolledBackFile.toPath())); + assertBinaryMetadataLength(changed, Files.size(after)); + assertEquals("Original author", APILocator.getFileMetadataAPI() + .getOrGenerateMetadata(changed, "heroImage").getCustomMeta().get("credit")); + final java.util.Map receivedMetadata = new java.util.HashMap<>( + APILocator.getFileMetadataAPI().getFullMetadataNoCache(changed, "heroImage").getMap()); + receivedMetadata.put("dot:credit", "Received author"); + APILocator.getFileMetadataAPI().setMetadata(changed, java.util.Map.of("heroImage", + new com.dotcms.storage.model.Metadata("heroImage", receivedMetadata))); + assertEquals("Received author", APILocator.getFileMetadataAPI().getMetadata( + contentletAPI.find(changed.getInode(), user, false), "heroImage") + .getCustomMeta().get("credit")); + assertMetadataRestoredFromS3(original, Files.size(before)); + assertEquals("Original attachment author", APILocator.getFileMetadataAPI() + .getFullMetadataNoCache(original, "attachment").getCustomMeta().get("credit"), + "A publishing metadata update must preserve omitted fields"); + APILocator.getFileMetadataAPI().setMetadata(changed, java.util.Map.of()); + assertEquals("Received author", APILocator.getFileMetadataAPI().getMetadata( + contentletAPI.find(changed.getInode(), user, false), "heroImage") + .getCustomMeta().get("credit"), "An empty metadata bundle must not delete existing snapshots"); + } finally { + com.dotmarketing.db.HibernateUtil.rollbackTransaction(); + } + assertEquals(originalKey, BinaryAssetReference.find(original.getInode(), "heroImage")); + assertMetadataRestoredFromS3(contentletAPI.find(original.getInode(), user, false), Files.size(before)); + assertTrue(binaryAssetStorageAPI.evictLocalFile(originalFile)); + assertFalse(rolledBackFile.exists(), "Rollback must delete the local copy of the revision it uploaded"); + assertFalse(binaryAssetStorageAPI.listBinaryPaths(original.getInode()) + .contains(BinaryAssetReference.keyOf(rolledBackFile)), "Rollback must delete the uploaded revision from S3"); + assertEquals("last committed bytes", Files.readString(contentletAPI.find(original.getInode(), user, false) + .getBinary("heroImage").toPath())); + + final Contentlet edit = new Contentlet(original); + edit.setBinary("heroImage", after.toFile()); + final Contentlet committed = contentletAPI.checkinWithoutVersioning(edit, + (com.dotmarketing.portlets.structure.model.ContentletRelationships) null, + null, null, user, false); + final File committedFile = committed.getBinary("heroImage"); + assertNotEquals(originalKey, BinaryAssetReference.find(committed.getInode(), "heroImage")); + assertTrue(binaryAssetStorageAPI.evictLocalFile(committedFile)); + assertMetadataRestoredFromS3(committed, Files.size(after)); + assertEquals("replacement bytes", Files.readString(contentletAPI.find(committed.getInode(), user, false) + .getBinary("heroImage").toPath())); + assertEquals("Original author", APILocator.getFileMetadataAPI() + .getOrGenerateMetadata(original, "heroImage").getCustomMeta().get("credit")); + assertBinaryMetadataLength(original, Files.size(before)); + + final Contentlet multiEdit = new Contentlet(committed); + for (final String field : java.util.List.of("heroImage", "attachment")) { + final com.dotcms.rest.api.v1.temp.DotTempFile upload = APILocator.getTempFileAPI() + .createTempFile(field + ".Txt", + com.dotcms.rest.api.v1.temp.TempFileAPITest.mockHttpServletRequest(), + new java.io.ByteArrayInputStream(("updated " + field).getBytes(java.nio.charset.StandardCharsets.UTF_8))); + APILocator.getFileMetadataAPI().putCustomMetadataAttributes(upload.id, + java.util.Map.of(field, java.util.Map.of("credit", "Uploaded " + field))); + multiEdit.setBinary(field, upload.file); + } + final Contentlet multiSaved = contentletAPI.checkinWithoutVersioning(multiEdit, + (com.dotmarketing.portlets.structure.model.ContentletRelationships) null, + null, null, user, false); + for (final String field : java.util.List.of("heroImage", "attachment")) { + assertEquals("Uploaded " + field, APILocator.getFileMetadataAPI() + .getOrGenerateMetadata(multiSaved, field).getCustomMeta().get("credit"), + "Copying another field must not overwrite this upload's custom metadata"); + } + + com.dotmarketing.db.HibernateUtil.startTransaction(); + try { + contentletAPI.destroy(multiSaved, user, false); + assertMetadataRestoredFromS3(multiSaved, "updated heroImage".length(), "Uploaded heroImage"); + } finally { + com.dotmarketing.db.HibernateUtil.rollbackTransaction(); + } + assertNotNull(BinaryAssetReference.find(multiSaved.getInode(), "heroImage")); + assertEquals("updated heroImage", Files.readString(contentletAPI.find(multiSaved.getInode(), user, false) + .getBinary("heroImage").toPath())); + + final String inodePrefix = "/" + multiSaved.getInode().charAt(0) + "/" + + multiSaved.getInode().charAt(1) + "/" + multiSaved.getInode(); + final com.dotcms.storage.FetchMetadataParams orphanMetadata = metadataRequest( + inodePrefix + "/removedField-metadata.json"); + final com.dotcms.storage.FetchMetadataParams neighborMetadata = metadataRequest( + inodePrefix + "-neighbor/otherField-metadata.json"); + APILocator.getFileStorageAPI().setMetadata(orphanMetadata, java.util.Map.of("dot:credit", "Removed field")); + APILocator.getFileStorageAPI().setMetadata(neighborMetadata, java.util.Map.of("dot:credit", "Neighbor")); + Files.delete(java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getAssetPath()) + .resolve(orphanMetadata.getStorageKey().getPath().substring(1).toLowerCase(java.util.Locale.ROOT))); + + contentletAPI.destroy(multiSaved, user, false); + final java.util.List> jobs = new com.dotmarketing.common.db.DotConnect() + .setSQL("select id from job where queue_name = ? and parameters ->> 'inode' = ?") + .addParam(BinaryAssetCleanupProcessor.QUEUE).addParam(multiSaved.getInode()).loadObjectResults(); + assertFalse(jobs.isEmpty(), "CMS deletion must commit a durable cleanup job"); + final com.dotcms.jobs.business.job.Job job = APILocator.getJobQueueManagerAPI() + .getJob(jobs.getFirst().get("id").toString()); + new BinaryAssetCleanupProcessor().process(job); + assertTrue(binaryAssetStorageAPI.listBinaryPaths(multiSaved.getInode()).isEmpty()); + for (final Contentlet snapshot : java.util.List.of(original, committed, multiSaved)) { + for (final String field : java.util.List.of("heroImage", "attachment")) { + assertNull(APILocator.getFileMetadataAPI().getMetadata(snapshot, field), + "Cleanup must remove metadata for current and historical revisions"); + } + } + new BinaryAssetCleanupProcessor().process(job); // Durable retries after success are harmless. + assertNull(APILocator.getFileStorageAPI().retrieveMetaData(orphanMetadata), + "Remote-only metadata must be deleted even when its field and source are gone"); + assertNotNull(APILocator.getFileStorageAPI().retrieveMetaData(neighborMetadata), + "Inode cleanup must not delete a neighboring inode's metadata"); + APILocator.getFileStorageAPI().removeMetaData(neighborMetadata); + } finally { + ContentTypeDataGen.remove(type); + } + } + + @Test + void s3HttpResponseProtectsTheResolvedFileUntilItIsOpened( + @org.junit.jupiter.api.io.TempDir java.nio.file.Path uploads) throws Exception { + org.junit.jupiter.api.Assumptions.assumeTrue(Boolean.getBoolean("s3.cms.enabled")); + final Folder folder = new FolderDataGen().nextPersisted(); + try { + final File source = Files.writeString(uploads.resolve("Leased-Response.Txt"), "response bytes from S3").toFile(); + final Contentlet content = new FileAssetDataGen(folder, source).nextPersisted(); + ContentletDataGen.publish(content); + final File cached = content.getBinary(FileAssetAPI.BINARY_FIELD); + assertTrue(binaryAssetStorageAPI.evictLocalFile(cached)); + final String uri = "/contentAsset/raw-data/" + content.getInode() + "/" + FileAssetAPI.BINARY_FIELD + "/byInode/true"; + for (final String range : java.util.List.of("", "bytes=1-4")) { + final var request = new com.dotcms.mock.request.MockHeaderRequest(new com.dotcms.mock.request.MockSessionRequest(new com.dotcms.mock.request.MockServletPathRequest( + new com.dotcms.mock.request.MockHttpRequestIntegrationTest("localhost", uri).request(), "/contentAsset"))); + request.setHeader("range", range); + request.setAttribute(com.liferay.portal.util.WebKeys.USER, user); + final var output = new java.io.ByteArrayOutputStream(); + final var opened = new java.util.concurrent.atomic.AtomicBoolean(); + // The full response opens the file before it asks for the output stream; the range branch asks first. + final boolean sourceOpen = range.isEmpty(); + final var capture = new com.dotcms.mock.response.MockHttpStatusResponse(new com.dotcms.mock.response.MockHttpCaptureResponse( + org.mockito.Mockito.mock(javax.servlet.http.HttpServletResponse.class), output)); + final var response = new javax.servlet.http.HttpServletResponseWrapper(capture) { + @Override + public javax.servlet.ServletOutputStream getOutputStream() throws java.io.IOException { + opened.set(true); + try { + final boolean evicted = binaryAssetStorageAPI.evictLocalFile(cached); + if (sourceOpen) { + // The open handle still streams every byte, asserted below. + assertTrue(evicted, "Once the file is open, a slow client must not defer eviction"); + } else { + assertFalse(evicted, "Eviction must defer until the range branch has opened the file"); + assertTrue(cached.isFile()); + } + } catch (com.dotmarketing.exception.DotDataException e) { + throw new java.io.IOException(e); + } + return super.getOutputStream(); + } + }; + final var servlet = new com.dotmarketing.servlets.BinaryExporterServlet(); + servlet.init(); + servlet.doGet(request, response); + assertEquals(range.isEmpty() ? 200 : 206, response.getStatus()); + assertTrue(opened.get(), "The test must reach actual response streaming"); + final byte[] expected = Files.readAllBytes(source.toPath()); + assertArrayEquals(range.isEmpty() ? expected : java.util.Arrays.copyOfRange(expected, 1, 5), output.toByteArray()); + if (!sourceOpen) { + assertTrue(binaryAssetStorageAPI.evictLocalFile(cached), "Response completion must release the lease"); + } + } + } finally { + FolderDataGen.remove(folder); + } + } + + @Test + void s3StandaloneMetadataEditsAreIsolatedAndRollbackSafe( + @org.junit.jupiter.api.io.TempDir java.nio.file.Path uploads) throws Exception { + org.junit.jupiter.api.Assumptions.assumeTrue(Boolean.getBoolean("s3.cms.enabled")); + final Folder folder = new FolderDataGen().nextPersisted(); + final var metadataAPI = APILocator.getFileMetadataAPI(); + final String field = FileAssetAPI.BINARY_FIELD; + try { + final File upload = Files.writeString(uploads.resolve("MetadataHistory.Txt"), "unchanged binary bytes").toFile(); + final Contentlet asset = new FileAssetDataGen(folder, upload).nextPersisted(); + metadataAPI.putCustomMetadataAttributes(asset, java.util.Map.of(field, java.util.Map.of("credit", "Original"))); + final Contentlet before = new Contentlet(asset); + final String originalMetadataPath = metadataAPI.getFileName(before, field); + com.dotmarketing.business.CacheLocator.getContentletCache().remove(asset.getInode()); + com.dotmarketing.db.HibernateUtil.startTransaction(); + try { + metadataAPI.putCustomMetadataAttributes(asset, java.util.Map.of(field, java.util.Map.of("credit", "Pending"))); + assertEquals("Pending", metadataAPI.getMetadata(contentletAPI.find(asset.getInode(), user, false), field) + .getCustomMeta().get("credit"), "The writer must read its metadata update inside the transaction"); + final String observed = java.util.concurrent.CompletableFuture.supplyAsync(() -> { + try { + return (String) metadataAPI.getMetadata(contentletAPI.find(asset.getInode(), user, false), field) + .getCustomMeta().get("credit"); + } catch (Exception e) { + throw new java.util.concurrent.CompletionException(e); + } + }).get(30, java.util.concurrent.TimeUnit.SECONDS); + assertEquals("Original", observed, "Another request must not observe an uncommitted metadata edit"); + } finally { + com.dotmarketing.db.HibernateUtil.rollbackTransaction(); + } + final Contentlet rolledBack = contentletAPI.find(asset.getInode(), user, false); + assertEquals("Original", metadataAPI.getMetadata(rolledBack, field).getCustomMeta().get("credit")); + assertEquals(originalMetadataPath, metadataAPI.getFileName(rolledBack, field)); + + metadataAPI.putCustomMetadataAttributes(rolledBack, + java.util.Map.of(field, java.util.Map.of("credit", "Committed"))); + final Contentlet committed = contentletAPI.find(asset.getInode(), user, false); + assertNotEquals(originalMetadataPath, metadataAPI.getFileName(committed, field)); + assertEquals("Committed", metadataAPI.getMetadata(committed, field).getCustomMeta().get("credit")); + assertEquals("Original", metadataAPI.getMetadata(before, field).getCustomMeta().get("credit"), + "A prior content snapshot must retain its prior metadata"); + for (final Contentlet snapshot : java.util.List.of(before, committed)) { + final String path = metadataAPI.getFileName(snapshot, field); + Files.delete(java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getAssetPath()) + .resolve(path.substring(1).toLowerCase(java.util.Locale.ROOT))); + com.dotmarketing.business.CacheLocator.getMetadataCache() + .removeMetadata(metadataAPI.getMetadataCacheKey(snapshot, field)); + } + assertEquals("Original", metadataAPI.getMetadata(before, field).getCustomMeta().get("credit")); + assertEquals("Committed", metadataAPI.getMetadata(committed, field).getCustomMeta().get("credit")); + final String committedMetadataKey = metadataAPI.getFileName(committed, field); + assertTrue(binaryAssetStorageAPI.evictLocalFile(committed.getBinary(field))); + assertEquals(committedMetadataKey, BinaryAssetReference.metadataKeyOf(committed.getBinary(field)), + "Cold Contentlet binary restoration must preserve its metadata snapshot"); + assertEquals(committedMetadataKey, BinaryAssetReference.metadataKeyOf( + binaryAssetStorageAPI.getBinaryFile(asset.getInode(), field))); + assertEquals(committedMetadataKey, BinaryAssetReference.metadataKeyOf( + binaryAssetStorageAPI.getBinaryFile(asset.getInode(), field, "MetadataHistory.Txt"))); + + final Contentlet concurrentSnapshot = new Contentlet(committed); + final java.util.concurrent.CountDownLatch attemptingEdit = new java.util.concurrent.CountDownLatch(1); + java.util.concurrent.CompletableFuture concurrentEdit = null; + com.dotmarketing.db.HibernateUtil.startTransaction(); + try { + metadataAPI.putCustomMetadataAttributes(committed, + java.util.Map.of(field, java.util.Map.of("credit", "First concurrent edit"))); + concurrentEdit = java.util.concurrent.CompletableFuture.runAsync(() -> { + try { + attemptingEdit.countDown(); + metadataAPI.putCustomMetadataAttributes(concurrentSnapshot, + java.util.Map.of(field, java.util.Map.of("license", "Second concurrent edit"))); + } catch (Exception e) { + throw new java.util.concurrent.CompletionException(e); + } finally { + com.dotmarketing.db.DbConnectionFactory.closeConnection(); + } + }); + assertTrue(attemptingEdit.await(30, java.util.concurrent.TimeUnit.SECONDS)); + final var pending = concurrentEdit; + assertThrows(java.util.concurrent.TimeoutException.class, + () -> pending.get(250, java.util.concurrent.TimeUnit.MILLISECONDS), + "Concurrent metadata edits must wait for the content transaction"); + com.dotmarketing.db.HibernateUtil.commitTransaction(); + } finally { + if (com.dotmarketing.db.DbConnectionFactory.inTransaction()) { + com.dotmarketing.db.HibernateUtil.rollbackTransaction(); + } + if (concurrentEdit != null) { + concurrentEdit.get(30, java.util.concurrent.TimeUnit.SECONDS); + } + } + final var merged = metadataAPI.getMetadata(contentletAPI.find(asset.getInode(), user, false), field).getCustomMeta(); + assertEquals("First concurrent edit", merged.get("credit")); + assertEquals("Second concurrent edit", merged.get("license"), + "The second edit must merge against the newly committed metadata, not its stale snapshot"); + assertEquals("Original", metadataAPI.getMetadata(before, field).getCustomMeta().get("credit")); + } finally { + FolderDataGen.remove(folder); + } + } + + @Test + void s3MetadataCopyPreservesDestinationBytesAndHistoricalSnapshots( + @org.junit.jupiter.api.io.TempDir java.nio.file.Path uploads) throws Exception { + org.junit.jupiter.api.Assumptions.assumeTrue(Boolean.getBoolean("s3.cms.enabled")); + final Folder folder = new FolderDataGen().nextPersisted(); + final var metadataAPI = APILocator.getFileMetadataAPI(); + final String field = FileAssetAPI.BINARY_FIELD; + try { + final File sourceFile = Files.writeString(uploads.resolve("CopySource.Txt"), "source").toFile(); + final File destinationFile = Files.writeString(uploads.resolve("CopyDestination.Txt"), + "Destination binary must retain its own metadata.").toFile(); + final Contentlet source = new FileAssetDataGen(folder, sourceFile).nextPersisted(); + final Contentlet destination = new FileAssetDataGen(folder, destinationFile).nextPersisted(); + metadataAPI.putCustomMetadataAttributes(source, + java.util.Map.of(field, java.util.Map.of("credit", "Source author", "sourceOnly", "copied"))); + metadataAPI.putCustomMetadataAttributes(destination, + java.util.Map.of(field, java.util.Map.of("credit", "Destination author", "destinationOnly", "removed"))); + final Contentlet before = new Contentlet(destination); + final String beforeKey = metadataAPI.getFileName(before, field); + com.dotmarketing.db.HibernateUtil.startTransaction(); + try { + metadataAPI.copyCustomMetadata(source, destination); + final var pending = metadataAPI.getMetadata(contentletAPI.find(destination.getInode(), user, false), field); + assertEquals("Source author", pending.getCustomMeta().get("credit")); + assertEquals("copied", pending.getCustomMeta().get("sourceOnly")); + assertFalse(pending.getCustomMeta().containsKey("destinationOnly")); + assertEquals(Long.toString(destinationFile.length()), String.valueOf(pending.getFieldsMeta().get("length")), + "Copying attributes must retain the destination binary's generated metadata"); + assertEquals("Destination author", metadataAPI.getMetadata(before, field).getCustomMeta().get("credit")); + } finally { + com.dotmarketing.db.HibernateUtil.rollbackTransaction(); + } + final Contentlet rolledBack = contentletAPI.find(destination.getInode(), user, false); + assertEquals(beforeKey, metadataAPI.getFileName(rolledBack, field)); + assertEquals("Destination author", metadataAPI.getMetadata(rolledBack, field).getCustomMeta().get("credit")); + metadataAPI.copyCustomMetadata(source, rolledBack); + final Contentlet committed = contentletAPI.find(destination.getInode(), user, false); + assertNotEquals(beforeKey, metadataAPI.getFileName(committed, field)); + for (final Contentlet snapshot : java.util.List.of(before, committed)) { + final String path = metadataAPI.getFileName(snapshot, field); + Files.delete(java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getAssetPath()) + .resolve(path.substring(1).toLowerCase(java.util.Locale.ROOT))); + com.dotmarketing.business.CacheLocator.getMetadataCache().removeMetadata( + metadataAPI.getMetadataCacheKey(snapshot, field)); + } + assertEquals("Destination author", metadataAPI.getMetadata(before, field).getCustomMeta().get("credit")); + assertEquals("Source author", metadataAPI.getMetadata(committed, field).getCustomMeta().get("credit")); + assertArrayEquals(Files.readAllBytes(destinationFile.toPath()), Files.readAllBytes(committed.getBinary(field).toPath())); + + metadataAPI.putCustomMetadataAttributes(source, java.util.Map.of(field, java.util.Map.of())); + metadataAPI.copyCustomMetadata(source, committed); + final var cleared = metadataAPI.getMetadata(contentletAPI.find(destination.getInode(), user, false), field); + assertTrue(cleared.getCustomMeta().isEmpty(), "An empty source removes only destination custom attributes"); + assertEquals(Long.toString(destinationFile.length()), String.valueOf(cleared.getFieldsMeta().get("length"))); + assertEquals("Destination author", metadataAPI.getMetadata(before, field).getCustomMeta().get("credit")); + } finally { + FolderDataGen.remove(folder); + } + } + + @Test + void s3RegenerationPreservesSnapshotsAndRollback( + @org.junit.jupiter.api.io.TempDir java.nio.file.Path uploads) throws Exception { + org.junit.jupiter.api.Assumptions.assumeTrue(Boolean.getBoolean("s3.cms.enabled")); + final Folder folder = new FolderDataGen().nextPersisted(); + final var metadataAPI = APILocator.getFileMetadataAPI(); + final String field = FileAssetAPI.BINARY_FIELD; + final String overrideProperty = com.dotcms.storage.FileMetadataAPI.ALWAYS_REGENERATE_METADATA_ON_REINDEX; + final boolean previousOverride = com.dotmarketing.util.Config.getBooleanProperty(overrideProperty, false); + try { + final File upload = Files.writeString(uploads.resolve("RegenerateMetadata.Txt"), "metadata source bytes").toFile(); + final Contentlet asset = new FileAssetDataGen(folder, upload).nextPersisted(); + metadataAPI.setMetadata(asset, java.util.Map.of(field, + new com.dotcms.storage.model.Metadata(field, + java.util.Map.of("length", -1L, "dot:credit", "Original author")))); + final Contentlet before = new Contentlet(asset); + final String beforeKey = metadataAPI.getFileName(before, field); + com.dotmarketing.util.Config.setProperty(overrideProperty, true); + com.dotmarketing.db.HibernateUtil.startTransaction(); + try { + metadataAPI.generateContentletMetadata(asset); + final Contentlet pending = contentletAPI.find(asset.getInode(), user, false); + assertNotEquals(beforeKey, metadataAPI.getFileName(pending, field)); + assertEquals(Long.toString(upload.length()), + String.valueOf(metadataAPI.getMetadata(pending, field).getFieldsMeta().get("length"))); + assertEquals("-1", String.valueOf(metadataAPI.getMetadata(before, field).getFieldsMeta().get("length")), + "Regeneration must not overwrite the metadata object held by an older snapshot"); + } finally { + com.dotmarketing.db.HibernateUtil.rollbackTransaction(); + } + final Contentlet rolledBack = contentletAPI.find(asset.getInode(), user, false); + assertEquals(beforeKey, metadataAPI.getFileName(rolledBack, field)); + metadataAPI.generateContentletMetadata(rolledBack); + final Contentlet regenerated = contentletAPI.find(asset.getInode(), user, false); + assertNotEquals(beforeKey, metadataAPI.getFileName(regenerated, field)); + assertEquals("Original author", metadataAPI.getMetadata(regenerated, field).getCustomMeta().get("credit")); + + metadataAPI.putCustomMetadataAttributes(regenerated, + java.util.Map.of(field, java.util.Map.of("credit", "Current author"))); + final Contentlet current = contentletAPI.find(asset.getInode(), user, false); + final String currentKey = metadataAPI.getFileName(current, field); + metadataAPI.generateContentletMetadata(before); + assertEquals(currentKey, metadataAPI.getFileName(contentletAPI.find(asset.getInode(), user, false), field), + "Reindexing an old metadata snapshot must not publish over the current reference"); + for (final Contentlet snapshot : java.util.List.of(before, current)) { + final String path = metadataAPI.getFileName(snapshot, field); + Files.delete(java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getAssetPath()) + .resolve(path.substring(1).toLowerCase(java.util.Locale.ROOT))); + com.dotmarketing.business.CacheLocator.getMetadataCache() + .removeMetadata(metadataAPI.getMetadataCacheKey(snapshot, field)); + } + assertEquals("-1", String.valueOf(metadataAPI.getMetadata(before, field).getFieldsMeta().get("length"))); + assertEquals("Current author", metadataAPI.getMetadata(current, field).getCustomMeta().get("credit")); + assertEquals(Long.toString(upload.length()), + String.valueOf(metadataAPI.getMetadata(current, field).getFieldsMeta().get("length"))); + + com.dotmarketing.util.Config.setProperty(overrideProperty, false); + metadataAPI.setMetadata(current, java.util.Map.of(field, + new com.dotcms.storage.model.Metadata(field, java.util.Map.of("dot:credit", "Custom only")))); + // A custom edit reads the metadata through the UI projection before publishing it. + // Its derived editableAsText value must not make this a complete generated record. + metadataAPI.putCustomMetadataAttributes(current, + java.util.Map.of(field, java.util.Map.of("license", "Retained during generation"))); + final Contentlet customOnly = new Contentlet(current); + metadataAPI.generateContentletMetadata(current); + final Contentlet lazyGenerated = contentletAPI.find(asset.getInode(), user, false); + assertNotEquals(metadataAPI.getFileName(customOnly, field), metadataAPI.getFileName(lazyGenerated, field)); + assertEquals("Custom only", metadataAPI.getMetadata(lazyGenerated, field).getCustomMeta().get("credit")); + assertEquals("Retained during generation", metadataAPI.getMetadata(lazyGenerated, field).getCustomMeta().get("license")); + assertEquals(Long.toString(upload.length()), + String.valueOf(metadataAPI.getMetadata(lazyGenerated, field).getFieldsMeta().get("length"))); + assertFalse(metadataAPI.getMetadata(customOnly, field).getFieldsMeta().containsKey("length")); + final String warmKey = metadataAPI.getFileName(lazyGenerated, field); + metadataAPI.generateContentletMetadata(lazyGenerated); + assertEquals(warmKey, metadataAPI.getFileName(lazyGenerated, field), + "Generated metadata with custom attributes must be reused on a normal reindex"); + } finally { + com.dotmarketing.util.Config.setProperty(overrideProperty, previousOverride); + FolderDataGen.remove(folder); + } + } + + private String binaryObjectKey(String path) { + final String namespace = System.getProperty("DOT_STORAGE_FILE_METADATA_S3_NAMESPACE", ""); + return (namespace.isEmpty() ? "" : "asset-namespaces/" + namespace + "/") + + BinaryAssetStorageAPI.BINARY_ASSETS_GROUP + "/" + path; + } + + private java.util.Map readZipEntries(final byte[] bytes) throws Exception { + final java.util.Map entries = new java.util.HashMap<>(); + try (final var zip = new java.util.zip.ZipInputStream(new java.io.ByteArrayInputStream(bytes))) { + java.util.zip.ZipEntry entry; + while ((entry = zip.getNextEntry()) != null) { + assertNull(entries.put(entry.getName(), zip.readAllBytes()), "ZIP entries must not be duplicated"); + } + } + return entries; + } + + private com.dotcms.storage.FetchMetadataParams metadataRequest(final String path) { + return new com.dotcms.storage.FetchMetadataParams.Builder().cache(false) + .storageKey(new com.dotcms.storage.StorageKey.Builder().group(com.dotcms.storage.FileMetadataAPI.DOT_METADATA) + .path(path).storage(com.dotcms.storage.StorageType.DEFAULT_CHAIN).build()).build(); + } + + private void assertBinaryMetadataLength(final Contentlet contentlet, final long expected) throws Exception { + final com.dotcms.storage.model.Metadata metadata = APILocator.getFileMetadataAPI() + .getOrGenerateMetadata(contentlet, "heroImage"); + assertNotNull(metadata); + assertEquals(Long.toString(expected), String.valueOf(metadata.getFieldsMeta().get("length")), + "Metadata must describe the binary referenced by this content snapshot"); + } + + private void assertMetadataRestoredFromS3(final Contentlet contentlet, final long expected) throws Exception { + assertMetadataRestoredFromS3(contentlet, expected, "Original author"); + } + + private void assertMetadataRestoredFromS3(final Contentlet contentlet, final long expected, + final String credit) throws Exception { + final com.dotcms.storage.FileMetadataAPI metadataAPI = APILocator.getFileMetadataAPI(); + final String key = metadataAPI.getFileName(contentlet, "heroImage"); + // The existing metadata filesystem provider normalizes its paths to lowercase. + final java.nio.file.Path local = java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getAssetPath()) + .resolve(key.substring(1).toLowerCase(java.util.Locale.ROOT)); + Files.delete(local); + com.dotmarketing.business.CacheLocator.getMetadataCache() + .removeMetadata(metadataAPI.getMetadataCacheKey(contentlet, "heroImage")); + // getMetadata cannot regenerate from the binary: only the stored S3 object can satisfy this read. + final com.dotcms.storage.model.Metadata restored = metadataAPI.getMetadata(contentlet, "heroImage"); + assertNotNull(restored, "Metadata must survive loss of both local caches"); + assertEquals(Long.toString(expected), String.valueOf(restored.getFieldsMeta().get("length"))); + assertEquals(credit, restored.getCustomMeta().get("credit")); + assertTrue(Files.isRegularFile(local), "S3 retrieval must repopulate the local metadata cache"); + } + + /** + * AC-1: Contentlet binary checkin + retrieval works through abstraction. + * Creates a content type with a binary field, checks in a contentlet with a test file, + * and verifies retrieval through both Contentlet.getBinary() and BinaryAssetStorageAPI. + */ + @Test + void test_checkin_contentlet_with_binary_stores_and_retrieves_file() throws Exception { + + final String testContent = "integration-test-content-" + System.currentTimeMillis(); + ContentType contentType = null; + + try { + // Create content type with title + binary field + contentType = new ContentTypeDataGen().nextPersisted(); + + final Field titleField = new FieldDataGen() + .velocityVarName("title") + .contentTypeId(contentType.id()) + .type(TextField.class) + .nextPersisted(); + ContentTypeDataGen.addField(titleField); + + final Field binaryField = new FieldDataGen() + .velocityVarName("testBinary") + .contentTypeId(contentType.id()) + .type(BinaryField.class) + .nextPersisted(); + ContentTypeDataGen.addField(binaryField); + + // Create temp file with known content + final File tempFile = File.createTempFile("binary-test-", ".txt"); + tempFile.deleteOnExit(); + Files.writeString(tempFile.toPath(), testContent); + + // Create and checkin contentlet with binary + final ContentletDataGen dataGen = new ContentletDataGen(contentType); + dataGen.setProperty("title", "Binary Test"); + dataGen.setProperty("testBinary", tempFile); + final Contentlet checkedIn = dataGen.nextPersisted(); + + // Verify via Contentlet.getBinary (migrated in 02-01) + final File retrievedFile = checkedIn.getBinary("testBinary"); + assertNotNull(retrievedFile, "getBinary should return a file"); + assertTrue(retrievedFile.exists(), "Retrieved file should exist on disk"); + assertEquals(testContent, Files.readString(retrievedFile.toPath()), + "File content should match what was uploaded"); + + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { + final String inode = checkedIn.getInode(); + final java.nio.file.Path legacyPath = java.nio.file.Path.of( + com.dotmarketing.util.ConfigUtils.getAssetPath(), + inode.substring(0, 1), inode.substring(1, 2), inode, "testBinary", tempFile.getName()); + assertTrue(Files.isSameFile(legacyPath, retrievedFile.toPath()), + "Disabled mode must retain the legacy filesystem/NFS layout"); + assertNull(BinaryAssetReference.find(inode, "testBinary"), + "Disabled check-in must not persist an S3 revision reference"); + assertFalse(binaryAssetStorageAPI.evictLocalFile(retrievedFile)); + assertTrue(retrievedFile.exists(), "Disabled eviction must leave the original in place"); + } + + // Verify via BinaryAssetStorageAPI directly (2-arg, filename-less) + final File apiFile = binaryAssetStorageAPI.getBinaryFile( + checkedIn.getInode(), "testBinary"); + assertNotNull(apiFile, "BinaryAssetStorageAPI should find the file"); + assertEquals(retrievedFile.getName(), apiFile.getName(), + "API file name should match getBinary file name"); + + } finally { + if (contentType != null) { + ContentTypeDataGen.remove(contentType); + } + } + } + + /** + * AC-2: Content versioning preserves binaries through abstraction. + * Creates a contentlet with binary, checks out, modifies non-binary field, + * checks back in, and verifies binary survives the version. + */ + @Test + void test_content_version_preserves_binary() throws Exception { + + final String testContent = "version-test-content-" + System.currentTimeMillis(); + ContentType contentType = null; + + try { + contentType = new ContentTypeDataGen().nextPersisted(); + + final Field titleField = new FieldDataGen() + .velocityVarName("title") + .contentTypeId(contentType.id()) + .type(TextField.class) + .nextPersisted(); + ContentTypeDataGen.addField(titleField); + + final Field binaryField = new FieldDataGen() + .velocityVarName("testBinary") + .contentTypeId(contentType.id()) + .type(BinaryField.class) + .nextPersisted(); + ContentTypeDataGen.addField(binaryField); + + // Create temp file and checkin + final File tempFile = File.createTempFile("version-test-", ".txt"); + tempFile.deleteOnExit(); + Files.writeString(tempFile.toPath(), testContent); + + final ContentletDataGen dataGen = new ContentletDataGen(contentType); + dataGen.setProperty("title", "Version Test v1"); + dataGen.setProperty("testBinary", tempFile); + final Contentlet v1 = dataGen.nextPersisted(); + + // Checkout, modify title only, checkin again (new version) + final Contentlet checkout = contentletAPI.checkout(v1.getInode(), user, false); + checkout.setStringProperty("title", "Version Test v2"); + final Contentlet v2 = contentletAPI.checkin(checkout, user, false); + + // Verify binary survives on the new version + assertNotEquals(v1.getInode(), v2.getInode(), + "New version should have a different inode"); + + final File v2File = v2.getBinary("testBinary"); + assertNotNull(v2File, "Binary should exist on new version"); + assertTrue(v2File.exists(), "Binary file should exist on disk"); + assertEquals(testContent, Files.readString(v2File.toPath()), + "Binary content should match original after versioning"); + + } finally { + if (contentType != null) { + ContentTypeDataGen.remove(contentType); + } + } + } + + /** + * A contentlet with a new inode that still carries the previous version's binary, as check-in has + * after it assigns the new inode, must restore that evicted revision from the owner its key names. + * Check-in alone does not show this today, because saving the content JSON restores the file + * while it hydrates metadata, before check-in reads the binary. + */ + @Test + void s3BinaryCarriedToANewInodeIsRestoredFromItsOwner() throws Exception { + org.junit.jupiter.api.Assumptions.assumeTrue(Boolean.getBoolean("s3.cms.enabled")); + assertTrue(com.dotcms.storage.AssetStorageFeature.isEnabled(), "CMS must actually opt in to S3"); + final ContentType type = new ContentTypeDataGen().nextPersisted(); + try { + final Field title = new FieldDataGen().velocityVarName("title").contentTypeId(type.id()) + .type(TextField.class).nextPersisted(); + ContentTypeDataGen.addField(title); + final Field binary = new FieldDataGen().velocityVarName("testBinary").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted(); + ContentTypeDataGen.addField(binary); + final File source = File.createTempFile("evicted-version-", ".txt"); + source.deleteOnExit(); + Files.writeString(source.toPath(), "evicted bytes"); + + final Contentlet v1 = new ContentletDataGen(type).setProperty("title", "v1") + .setProperty("testBinary", source).nextPersisted(); + final File v1File = v1.getBinary("testBinary"); + assertNotNull(BinaryAssetReference.keyOf(v1File), "The binary must be stored as an immutable revision"); + assertTrue(binaryAssetStorageAPI.evictLocalFile(v1File)); + assertFalse(v1File.exists()); + + final Contentlet next = new Contentlet(v1); + next.setInode(com.dotmarketing.util.UUIDGenerator.generateUuid()); + next.getMap().put("testBinary", v1File); + + assertEquals("evicted bytes", Files.readString(next.getBinary("testBinary").toPath())); + } finally { + ContentTypeDataGen.remove(type); + } + } + + /** + * AC-3: FileAsset flow works end-to-end. + * Creates a FileAsset via FileAssetDataGen and verifies binary retrieval + * through both Contentlet.getBinary and BinaryAssetStorageAPI. + */ + @Test + void test_fileAsset_flow_end_to_end() throws Exception { + + final String testContent = "fileasset-test-content-" + System.currentTimeMillis(); + Folder folder = null; + + try { + folder = new FolderDataGen().nextPersisted(); + + // Create temp file with known content + final File tempFile = File.createTempFile("fileasset-test-", ".txt"); + tempFile.deleteOnExit(); + Files.writeString(tempFile.toPath(), testContent); + + // Create FileAsset via data generator + final Contentlet fileAsset = new FileAssetDataGen(folder, tempFile).nextPersisted(); + + // Verify via Contentlet.getBinary (exercises migrated read path) + final File retrievedFile = fileAsset.getBinary(FileAssetAPI.BINARY_FIELD); + assertNotNull(retrievedFile, "FileAsset binary should be retrievable"); + assertTrue(retrievedFile.exists(), "FileAsset binary should exist on disk"); + assertEquals(testContent, Files.readString(retrievedFile.toPath()), + "FileAsset content should match what was uploaded"); + + // Verify via BinaryAssetStorageAPI directly (2-arg, filename-less) + final File apiFile = binaryAssetStorageAPI.getBinaryFile( + fileAsset.getInode(), FileAssetAPI.BINARY_FIELD); + assertNotNull(apiFile, "BinaryAssetStorageAPI should find the FileAsset binary"); + assertTrue(apiFile.exists(), "API-retrieved file should exist"); + + } finally { + if (folder != null) { + FolderDataGen.remove(folder); + } + } + } + + /** + * AC-1 (backward compat): Deprecated getRealAssetPath still returns valid paths. + * Ensures the deprecated methods continue to work for callers that haven't migrated. + */ + @SuppressWarnings("deprecation") + @Test + void test_deprecated_getRealAssetPath_still_works() throws Exception { + + final String testContent = "deprecated-test-content-" + System.currentTimeMillis(); + Folder folder = null; + + try { + folder = new FolderDataGen().nextPersisted(); + + final File tempFile = File.createTempFile("deprecated-test-", ".txt"); + tempFile.deleteOnExit(); + Files.writeString(tempFile.toPath(), testContent); + + final Contentlet fileAsset = new FileAssetDataGen(folder, tempFile).nextPersisted(); + + // Call deprecated method — should still return a valid path + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + assertTrue(binaryAssetStorageAPI.evictLocalFile(fileAsset.getBinary(FileAssetAPI.BINARY_FIELD))); + } + final String realPath = APILocator.getFileAssetAPI() + .getRealAssetPath(fileAsset.getInode(), + APILocator.getFileAssetAPI().fromContentlet(fileAsset).getUnderlyingFileName()); + + assertNotNull(realPath, "Deprecated getRealAssetPath should return a path"); + + final File fileFromPath = new File(realPath); + assertTrue(fileFromPath.exists(), + "Path from deprecated method should point to an existing file"); + assertEquals(testContent, Files.readString(fileFromPath.toPath()), + "Content from deprecated path should match uploaded content"); + assertEquals(realPath, APILocator.getFileAssetAPI().getRealAssetPathIgnoreExtensionCase( + fileAsset.getInode(), fileFromPath.getName())); + final String differentName = APILocator.getFileAssetAPI().getRealAssetPath(fileAsset.getInode(), "DifferentName.txt"); + assertNotEquals(realPath, differentName); + assertTrue(differentName.endsWith("DifferentName.txt")); + + } finally { + if (folder != null) { + FolderDataGen.remove(folder); + } + } + } + +} diff --git a/dotcms-integration/src/test/java/com/dotcms/storage/binary/ContentletBackupStorageTest.java b/dotcms-integration/src/test/java/com/dotcms/storage/binary/ContentletBackupStorageTest.java new file mode 100644 index 000000000000..c5b52af10a5b --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/storage/binary/ContentletBackupStorageTest.java @@ -0,0 +1,380 @@ +package com.dotcms.storage.binary; + +import com.dotcms.contenttype.model.field.BinaryField; +import com.dotcms.datagen.ContentTypeDataGen; +import com.dotcms.datagen.ContentletDataGen; +import com.dotcms.datagen.FieldDataGen; +import com.dotcms.datagen.LanguageDataGen; +import com.dotcms.storage.AmazonS3StoragePersistenceAPIImpl; +import com.dotcms.util.IntegrationTestInitService; +import com.dotcms.variant.VariantAPI; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.util.Config; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.zip.ZipInputStream; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfSystemProperty; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +@EnabledIfSystemProperty(named = "s3.cms.enabled", matches = "true") +public class ContentletBackupStorageTest { + @BeforeAll static void initialize() throws Exception { IntegrationTestInitService.getInstance().init(); } + @TempDir Path uploads; + + @ParameterizedTest + @ValueSource(strings = {"destroy", "allVersions", "version", "language"}) + void coldBackupsSurviveCommittedBinaryAndMetadataCleanup(String deletion) throws Exception { + final String previous = Config.getStringProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", null); + final var type = new ContentTypeDataGen().nextPersisted(); + final var backups = ContentletBackupStorage.getInstance(); + String identifier = null; + try { + for (String name : List.of("heroImage", "attachment", "optionalBinary")) { + ContentTypeDataGen.addField(new FieldDataGen().velocityVarName(name).contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted()); + } + final var generator = new ContentletDataGen(type); + generator.setProperty("heroImage", source("first", "original image")); + generator.setProperty("attachment", source("attachment", "separate attachment")); + final var api = APILocator.getContentletAPI(); + final var user = APILocator.systemUser(); + Contentlet first = generator.nextPersisted(); + identifier = first.getIdentifier(); + APILocator.getFileMetadataAPI().putCustomMetadataAttributes(first, + Map.of("heroImage", Map.of("credit", "Original author"))); + first = api.find(first.getInode(), user, false); + final Contentlet edit = api.checkout(first.getInode(), user, false); + edit.setBinary("heroImage", source("second", "new image version")); + final Contentlet second = api.checkin(edit, user, false); + assertNotEquals(first.getInode(), second.getInode()); + // Deleting one language of multilingual content takes the per-language path (#9146). + final Contentlet otherLanguage = deletion.equals("language") ? ContentletDataGen.createNewVersion( + second, VariantAPI.DEFAULT_VARIANT, new LanguageDataGen().nextPersisted(), null) : null; + final List deleted = deletion.equals("version") ? List.of(first) : List.of(first, second); + final var expected = new HashMap>(); + for (Contentlet content : deleted) { + final var entries = new HashMap(); + for (String field : List.of("heroImage", "attachment")) { + final var binary = content.getBinary(field); + entries.put("assets/" + BinaryAssetReference.keyOf(binary), Files.readAllBytes(binary.toPath())); + org.awaitility.Awaitility.await().atMost(java.time.Duration.ofSeconds(5)) + .until(() -> APILocator.getBinaryAssetStorageAPI().evictLocalFile(binary)); + final String metadataPath = APILocator.getFileMetadataAPI().getFileName(content, field); + final var metadataKey = new com.dotcms.storage.StorageKey.Builder() + .group(com.dotcms.storage.FileMetadataAPI.DOT_METADATA).path(metadataPath) + .storage(com.dotcms.storage.StoragePersistenceProvider.getStorageType()).build(); + final var raw = APILocator.getFileStorageAPI().retrieveRawMetaData(metadataKey); + if (raw != null) { + assertTrue(APILocator.getFileStorageAPI().backfillMetadata(metadataKey)); + entries.put("assets/" + metadataPath.substring(1).toLowerCase(java.util.Locale.ROOT), + new com.fasterxml.jackson.databind.ObjectMapper().writeValueAsBytes(raw)); + Files.deleteIfExists(Path.of(com.dotmarketing.util.ConfigUtils.getAssetPath(), + metadataPath.substring(1).toLowerCase(java.util.Locale.ROOT))); + } + } + expected.put(content.getInode(), entries); + } + final var retired = APILocator.getBinaryAssetStorageAPI().storeRevision(first.getInode(), + "retiredBinary", "Shared-Mixed.Txt", source("retired", "retired field bytes")); + final var persistedMapper = new com.fasterxml.jackson.databind.ObjectMapper(); + final var raw = (com.fasterxml.jackson.databind.node.ObjectNode) persistedMapper.readTree(new DotConnect() + .setSQL("select contentlet_as_json from contentlet where inode = ?").addParam(first.getInode()) + .getString("contentlet_as_json")); + ((com.fasterxml.jackson.databind.node.ObjectNode) raw.get("fields")).set("retiredBinary", persistedMapper.valueToTree( + com.dotcms.content.model.type.system.BinaryFieldType.builder().value(retired.getName()) + .storageKey(BinaryAssetReference.keyOf(retired)).build())); + new DotConnect().setSQL("update contentlet set contentlet_as_json = ?::jsonb where inode = ?") + .addParam(persistedMapper.writeValueAsString(raw)).addParam(first.getInode()).loadResult(); + expected.get(first.getInode()).put("assets/" + BinaryAssetReference.keyOf(retired), Files.readAllBytes(retired.toPath())); + assertTrue(APILocator.getBinaryAssetStorageAPI().evictLocalFile(retired)); + Config.setProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", true); + switch (deletion) { + case "destroy" -> api.destroy(second, user, false); + case "allVersions" -> api.deleteAllVersionsandBackup(List.of(second), user, false); + case "version" -> api.deleteVersion(first, user, false); + case "language" -> { + api.archive(api.find(second.getInode(), user, false), user, false); + api.delete(api.find(second.getInode(), user, false), user, false); + } + default -> throw new AssertionError(deletion); + } + for (Contentlet content : deleted) { + assertTrue(new DotConnect().setSQL("select inode from contentlet where inode = ?") + .addParam(content.getInode()).loadObjectResults().isEmpty()); + final var jobs = new DotConnect().setSQL("select id from job where queue_name = ? and parameters ->> 'inode' = ?") + .addParam(BinaryAssetCleanupProcessor.QUEUE).addParam(content.getInode()).loadObjectResults(); + assertEquals(1, jobs.size(), "Each deleted inode needs exactly one cleanup job, even when listed twice"); + for (var job : jobs) new BinaryAssetCleanupProcessor().process( + APILocator.getJobQueueManagerAPI().getJob(job.get("id").toString())); + assertTrue(APILocator.getBinaryAssetStorageAPI().listBinaryPaths(content.getInode()).isEmpty()); + assertNull(APILocator.getFileMetadataAPI().getMetadata(content, "heroImage")); + } + final var keys = backups.list(identifier); + assertEquals(deleted.size(), keys.size(), "Each deleted inode needs its own complete backup"); + for (String key : keys) { + final var entries = new HashMap(); + try (var zip = new ZipInputStream(backups.open(key))) { + for (var entry = zip.getNextEntry(); entry != null; entry = zip.getNextEntry()) { + assertNull(entries.put(entry.getName(), zip.readAllBytes())); + } + } + final String inode = key.split("/")[1]; + assertTrue(new String(entries.get("contentlet.xml"), java.nio.charset.StandardCharsets.UTF_8).contains(inode)); + assertNotNull(new com.fasterxml.jackson.databind.ObjectMapper().readTree(entries.get("contentlet.json")).get("fields")); + for (var entry : expected.get(inode).entrySet()) { + if (entry.getKey().endsWith("-metadata.json")) { + final var mapper = new com.fasterxml.jackson.databind.ObjectMapper(); + assertEquals(mapper.readTree(entry.getValue()), mapper.readTree(entries.get(entry.getKey()))); + } else assertArrayEquals(entry.getValue(), entries.get(entry.getKey())); + } + } + if (otherLanguage != null) { + assertEquals("new image version", Files.readString(api.find(otherLanguage.getInode(), user, false) + .getBinary("heroImage").toPath()), "Deleting one language must keep the other's binaries"); + } + if (deletion.equals("version")) { + assertEquals("new image version", Files.readString(api.find(second.getInode(), user, false).getBinary("heroImage").toPath())); + } + } finally { + Config.setProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", false); + ContentTypeDataGen.remove(type); + if (identifier != null) { + final var remote = AmazonS3StoragePersistenceAPIImpl.withPlainPaths(); + for (String key : backups.list(identifier)) remote.deleteObjectAndReferences(ContentletBackupStorage.GROUP, key); + } + Config.setProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", previous); + } + } + + @Test + void failedBackupAbortsDeletionAndPreservesSources() throws Exception { + final String previous = Config.getStringProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", null); + final var type = new ContentTypeDataGen().nextPersisted(); + try { + ContentTypeDataGen.addField(new FieldDataGen().velocityVarName("heroImage").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted()); + final var generator = new ContentletDataGen(type); + generator.setProperty("heroImage", source("failure", "recoverable bytes")); + final Contentlet content = generator.nextPersisted(); + final var binary = content.getBinary("heroImage"); + final var remote = spy(AmazonS3StoragePersistenceAPIImpl.withPlainPaths()); + doThrow(new DotDataException("injected backup outage")).when(remote) + .backfillFile(eq(ContentletBackupStorage.GROUP), anyString(), any()); + final var backups = new ContentletBackupStorage(remote); + Config.setProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", true); + try (var factory = mockStatic(ContentletBackupStorage.class)) { + factory.when(ContentletBackupStorage::getInstance).thenReturn(backups); + assertThrows(DotDataException.class, () -> APILocator.getContentletAPI().destroy(content, APILocator.systemUser(), false)); + } + assertFalse(new DotConnect().setSQL("select inode from contentlet where inode = ?") + .addParam(content.getInode()).loadObjectResults().isEmpty()); + assertFalse(APILocator.getVersionableAPI().isDeleted(APILocator.getContentletAPI() + .find(content.getInode(), APILocator.systemUser(), false)), "Failed backup must roll back the archive state too"); + assertTrue(new DotConnect().setSQL("select id from job where queue_name = ? and parameters ->> 'inode' = ?") + .addParam(BinaryAssetCleanupProcessor.QUEUE).addParam(content.getInode()).loadObjectResults().isEmpty()); + assertTrue(binary.isFile(), "Backup must not move the caller's local source"); + assertTrue(APILocator.getBinaryAssetStorageAPI().evictLocalFile(binary)); + assertEquals("recoverable bytes", Files.readString(APILocator.getContentletAPI() + .find(content.getInode(), APILocator.systemUser(), false).getBinary("heroImage").toPath())); + assertTrue(backups.list(content.getIdentifier()).isEmpty()); + } finally { + Config.setProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", false); + ContentTypeDataGen.remove(type); + Config.setProperty("BACKUP_DELETED_CONTENTLETS_TO_DISK", previous); + } + } + + @Test + void removedFieldArchivesHistoricalAndUnreferencedRevisionsWithoutDeletingNewUploads() throws Exception { + final var type = new ContentTypeDataGen().nextPersisted(); + String identifier = null; + try { + final var field = new FieldDataGen().velocityVarName("heroImage").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted(); + ContentTypeDataGen.addField(field); + ContentTypeDataGen.addField(new FieldDataGen().velocityVarName("attachment").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted()); + final var generator = new ContentletDataGen(type); + generator.setProperty("heroImage", source("field-first", "first field bytes")); + generator.setProperty("attachment", source("field-sibling", "keep sibling")); + final var api = APILocator.getContentletAPI(); + final var user = APILocator.systemUser(); + final Contentlet first = generator.nextPersisted(); + identifier = first.getIdentifier(); + APILocator.getFileMetadataAPI().putCustomMetadataAttributes(first, Map.of("heroImage", Map.of("credit", "Keep this credit"))); + final var edit = api.checkout(first.getInode(), user, false); + edit.setBinary("heroImage", source("field-second", "second field bytes")); + final Contentlet second = api.checkin(edit, user, false); + final var binaries = APILocator.getBinaryAssetStorageAPI(); + final var orphan = binaries.storeRevision(first.getInode(), "heroImage", "Extra-Mixed-metadata.json", source("orphan", "unreferenced revision")); + final Map expected = new HashMap<>(); + for (Contentlet content : List.of(first, second)) { + for (String path : binaries.listBinaryPaths(content.getInode())) { + if (!path.contains("/heroImage/") || !(path.endsWith("/Shared-Mixed.Txt") || path.endsWith("/Extra-Mixed-metadata.json"))) continue; + final var file = new java.io.File(com.dotmarketing.util.ConfigUtils.getAssetPath(), path); + try (var input = binaries.openLocalFile(file)) { expected.put("binary-assets/" + path, input.readAllBytes()); } + org.awaitility.Awaitility.await().atMost(java.time.Duration.ofSeconds(5)).until(() -> binaries.evictLocalFile(file)); + } + } + APILocator.getContentTypeFieldAPI().delete(field, user); + final var requests = fieldJobs("type", type.id()); + assertEquals(1, requests.size(), "Field deletion must persist its cleanup request"); + new BinaryFieldCleanupProcessor().process(requests.getFirst()); + final Map archived = new HashMap<>(); + final var backups = ContentletBackupStorage.getInstance(); + assertEquals(2, backups.list(identifier).size(), "Historical and working versions both need archives"); + for (String key : backups.list(identifier)) { + try (var zip = new ZipInputStream(backups.open(key))) { + for (var entry = zip.getNextEntry(); entry != null; entry = zip.getNextEntry()) { + if (!entry.getName().equals("contentlet.json")) archived.put(entry.getName(), zip.readAllBytes()); + } + } + } + for (var entry : expected.entrySet()) assertArrayEquals(entry.getValue(), archived.get(entry.getKey())); + assertTrue(archived.values().stream().anyMatch(bytes -> new String(bytes, java.nio.charset.StandardCharsets.UTF_8).contains("Keep this credit"))); + + // A later incarnation of the same field must survive an old cleanup job and its retries. + ContentTypeDataGen.addField(new FieldDataGen().velocityVarName("heroImage").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted()); + final var replacement = binaries.storeRevision(second.getInode(), "heroImage", "New-Mixed.Txt", source("recreated", "new field incarnation")); + final var mapper = new com.fasterxml.jackson.databind.ObjectMapper(); + final var json = (com.fasterxml.jackson.databind.node.ObjectNode) mapper.readTree(new DotConnect() + .setSQL("select contentlet_as_json from contentlet where inode = ?").addParam(second.getInode()).getString("contentlet_as_json")); + ((com.fasterxml.jackson.databind.node.ObjectNode) json.path("fields")).putObject("heroImage") + .put("type", "Binary").put("value", replacement.getName()).put("storageKey", BinaryAssetReference.keyOf(replacement)); + new DotConnect().setSQL("update contentlet set contentlet_as_json = ?::jsonb where inode = ?") + .addParam(mapper.writeValueAsString(json)).addParam(second.getInode()).loadResult(); + for (Contentlet content : List.of(first, second)) { + final var jobs = fieldJobs("inode", content.getInode()); + assertEquals(1, jobs.size()); + new BinaryFieldCleanupProcessor().process(jobs.getFirst()); + new BinaryFieldCleanupProcessor().process(jobs.getFirst()); + com.dotmarketing.business.CacheLocator.getContentletCache().remove(content.getInode()); + assertEquals("keep sibling", Files.readString(api.find(content.getInode(), user, false).getBinary("attachment").toPath())); + assertTrue(binaries.listBinaryPaths(content.getInode()).stream() + .noneMatch(path -> expected.containsKey("binary-assets/" + path))); + } + assertEquals("new field incarnation", Files.readString(api.find(second.getInode(), user, false).getBinary("heroImage").toPath())); + assertFalse(orphan.exists()); + } finally { + ContentTypeDataGen.remove(type); + if (identifier != null) removeBackups(identifier); + } + } + + @Test + void fieldBackupFailureRollsBackReferenceRemovalAndCleanupFailureCanRetry() throws Exception { + final var type = new ContentTypeDataGen().nextPersisted(); + String identifier = null; + try { + final var field = new FieldDataGen().velocityVarName("heroImage").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted(); + ContentTypeDataGen.addField(field); + final var generator = new ContentletDataGen(type); + generator.setProperty("heroImage", source("field-failure", "preserved field bytes")); + final Contentlet content = generator.nextPersisted(); + identifier = content.getIdentifier(); + final String originalKey = BinaryAssetReference.find(content.getInode(), "heroImage"); + com.dotmarketing.db.HibernateUtil.startTransaction(); + try { + APILocator.getContentTypeFieldAPI().delete(field, APILocator.systemUser()); + assertEquals(1, fieldJobs("type", type.id()).size()); + } finally { com.dotmarketing.db.HibernateUtil.rollbackTransaction(); } + assertTrue(fieldJobs("type", type.id()).isEmpty(), "Rolled-back field deletion must not leave a cleanup request"); + assertEquals(originalKey, BinaryAssetReference.find(content.getInode(), "heroImage")); + APILocator.getContentTypeFieldAPI().delete(field, APILocator.systemUser()); + final var request = fieldJobs("type", type.id()).getFirst(); + final var remote = spy(AmazonS3StoragePersistenceAPIImpl.withPlainPaths()); + doThrow(new DotDataException("injected field backup outage")).when(remote) + .backfillFile(eq(ContentletBackupStorage.GROUP), anyString(), any()); + try (var factory = mockStatic(ContentletBackupStorage.class)) { + factory.when(ContentletBackupStorage::getInstance).thenReturn(new ContentletBackupStorage(remote)); + assertThrows(com.dotcms.jobs.business.error.JobProcessingException.class, + () -> new BinaryFieldCleanupProcessor().process(request)); + } + assertEquals(originalKey, BinaryAssetReference.find(content.getInode(), "heroImage")); + assertTrue(fieldJobs("inode", content.getInode()).isEmpty()); + new BinaryFieldCleanupProcessor().process(request); + final var cleanup = fieldJobs("inode", content.getInode()).getFirst(); + final var binaries = spy(APILocator.getBinaryAssetStorageAPI()); + doThrow(new DotDataException("injected binary delete outage")).when(binaries).deleteBinaryPaths(anyString(), anyString(), anyList()); + try (var locator = mockStatic(APILocator.class, CALLS_REAL_METHODS)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(binaries); + assertThrows(com.dotcms.jobs.business.error.JobProcessingException.class, + () -> new BinaryFieldCleanupProcessor().process(cleanup)); + } + assertTrue(APILocator.getBinaryAssetStorageAPI().listBinaryPaths(content.getInode()).contains(originalKey)); + new BinaryFieldCleanupProcessor().process(cleanup); + assertTrue(APILocator.getBinaryAssetStorageAPI().listBinaryPaths(content.getInode()).isEmpty()); + assertEquals(1, ContentletBackupStorage.getInstance().list(identifier).size()); + } finally { + ContentTypeDataGen.remove(type); + if (identifier != null) removeBackups(identifier); + } + } + + @Test + void directCleanFieldQueuesArchivingInsteadOfUploadingInline() throws Exception { + final var type = new ContentTypeDataGen().nextPersisted(); + String identifier = null; + try { + final var field = new FieldDataGen().velocityVarName("heroImage").contentTypeId(type.id()) + .type(BinaryField.class).nextPersisted(); + ContentTypeDataGen.addField(field); + final var generator = new ContentletDataGen(type); + generator.setProperty("heroImage", source("field-direct", "direct clean bytes")); + final Contentlet content = generator.nextPersisted(); + identifier = content.getIdentifier(); + final String originalKey = BinaryAssetReference.find(content.getInode(), "heroImage"); + final var backups = spy(ContentletBackupStorage.getInstance()); + try (var factory = mockStatic(ContentletBackupStorage.class, CALLS_REAL_METHODS)) { + factory.when(ContentletBackupStorage::getInstance).thenReturn(backups); + APILocator.getContentletAPI().cleanField( + new com.dotcms.contenttype.transform.contenttype.StructureTransformer(type).asStructure(), + new java.util.Date(), + new com.dotcms.contenttype.transform.field.LegacyFieldTransformer(field).asOldField(), + APILocator.systemUser(), false); + } + // The call must not upload recovery archives while its caller waits and holds row locks. + verify(backups, never()).storeField(anyString(), anyString(), anyString(), anyList(), anyList()); + assertEquals(originalKey, BinaryAssetReference.find(content.getInode(), "heroImage"), + "Archiving and clearing happen in the queued job, not inline"); + assertEquals(1, fieldJobs("type", type.id()).size(), + "A direct call queues the same cleanup request as field deletion"); + } finally { + ContentTypeDataGen.remove(type); + if (identifier != null) removeBackups(identifier); + } + } + + private List fieldJobs(String owner, String id) throws Exception { + final var rows = new DotConnect().setSQL("select id from job where queue_name = ? and parameters ->> ? = ?") + .addParam(BinaryFieldCleanupProcessor.QUEUE).addParam(owner).addParam(id).loadObjectResults(); + final var jobs = new java.util.ArrayList(); + for (var row : rows) jobs.add(APILocator.getJobQueueManagerAPI().getJob(row.get("id").toString())); + return jobs; + } + + private void removeBackups(String identifier) throws Exception { + final var remote = AmazonS3StoragePersistenceAPIImpl.withPlainPaths(); + for (String key : ContentletBackupStorage.getInstance().list(identifier)) remote.deleteObjectAndReferences(ContentletBackupStorage.GROUP, key); + } + + private java.io.File source(String directory, String bytes) throws Exception { + final Path path = uploads.resolve(directory).resolve("Shared-Mixed.Txt"); + Files.createDirectories(path.getParent()); + return Files.writeString(path, bytes).toFile(); + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/storage/binary/SharedAssetStorageIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/storage/binary/SharedAssetStorageIntegrationTest.java new file mode 100644 index 000000000000..769ba1cfb59f --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/storage/binary/SharedAssetStorageIntegrationTest.java @@ -0,0 +1,122 @@ +package com.dotcms.storage.binary; + +import com.dotcms.storage.AmazonS3StoragePersistenceAPIImpl; +import com.dotcms.storage.JsonReaderDelegate; +import com.dotcms.tika.TikaUtils; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.business.APILocator; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.UUID; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfSystemProperty; +import org.junit.jupiter.api.io.TempDir; +import static org.junit.jupiter.api.Assertions.*; + +@EnabledIfSystemProperty(named = "s3.cms.enabled", matches = "true") +public class SharedAssetStorageIntegrationTest { + @BeforeAll static void initialize() throws Exception { IntegrationTestInitService.getInstance().init(); } + @TempDir Path root; + + @Test void realTikaExtractionIsSharedWhileFilenameAndPathRemainPerUse() throws Exception { + final String text = "Shared extracted document " + UUID.randomUUID(); + final var first = Files.writeString(root.resolve("First-Name.Txt"), text).toFile(); + final var second = Files.writeString(root.resolve("Another-Name.Txt"), text).toFile(); + final String hash = org.apache.commons.codec.digest.DigestUtils.sha256Hex(text); + final var remote = AmazonS3StoragePersistenceAPIImpl.withPlainPaths(); + final String group = "extracted-metadata"; + try { + assertNotNull(new TikaUtils().extractorVersion(), "The actual loaded parser must identify its version"); + final var api = APILocator.getFileStorageAPI(); + final var firstMetadata = api.generateRawFullMetaData(first, 8192); + final var secondMetadata = api.generateRawFullMetaData(second, 8192); + assertEquals(first.getName(), firstMetadata.get("name")); + assertEquals(second.getName(), secondMetadata.get("name")); + assertNotEquals(firstMetadata.get("path"), secondMetadata.get("path")); + assertEquals(firstMetadata.get("content"), secondMetadata.get("content")); + assertTrue(secondMetadata.get("content").toString().contains(text)); + final var paths = remote.listObjectPaths(group, hash + "/"); + assertEquals(1, paths.size(), "Equal bytes with different filenames share one extraction"); + final var shared = (Map) remote.pullObject(group, paths.getFirst(), new JsonReaderDelegate<>(Map.class)); + assertFalse(shared.containsKey("name")); + assertFalse(shared.containsKey("path")); + assertEquals(firstMetadata.get("content"), shared.get("content")); + api.generateRawFullMetaData(second, 4096); + assertEquals(2, remote.listObjectPaths(group, hash + "/").size()); + } finally { + for (String key : remote.listObjectPaths(group, hash + "/")) remote.deleteObjectAndReferences(group, key); + } + } + @Test void coldRevisionMetadataAndLinkedImageUseTheExactOwner() throws Exception { + final var assets = APILocator.getBinaryAssetStorageAPI(); + final var files = APILocator.getFileStorageAPI(); + final var metadata = APILocator.getFileMetadataAPI(); + final String inode = UUID.randomUUID().toString(); + final String field = "HeroImage"; + final String text = "Original metadata bytes " + UUID.randomUUID(); + final var source = Files.writeString(root.resolve("Mixed-Name.Txt"), text).toFile(); + final String hash = org.apache.commons.codec.digest.DigestUtils.sha256Hex(text); + final var first = assets.storeRevision(inode, field, source.getName(), source); + Files.writeString(source.toPath(), "Later revision bytes"); + final var replacement = assets.storeRevision(inode, field, source.getName(), source); + final var config = new com.dotcms.storage.GenerateMetadataConfig.Builder() + .storageKey(new com.dotcms.storage.StorageKey.Builder().group("dotmetadata") + .path("/metadata-cold-" + inode).storage(com.dotcms.storage.StorageType.FILE_SYSTEM).build()) + .override(true).store(false).cache(false).full(false).build(); + try { + for (int mode = 0; mode < 3; mode++) { + assertTrue(assets.evictLocalFile(first)); + assertFalse(first.exists()); + final var result = switch (mode) { + case 0 -> files.generateRawBasicMetaData(first); + case 1 -> files.generateRawFullMetaData(first, 8192); + default -> files.generateMetaData(first, config); + }; + assertEquals(hash, result.get("sha256")); + assertEquals("Mixed-Name.Txt", result.get("name")); + if (mode == 1) assertTrue(result.get("content").toString().contains(text)); + assertEquals(text, Files.readString(first.toPath()), "Historical revision must not resolve latest bytes"); + } + } finally { + assets.deleteBinary(inode, field); + } + + final var type = new com.dotcms.datagen.ContentTypeDataGen().nextPersisted(); + try { + final var binaryField = new com.dotcms.datagen.FieldDataGen().contentTypeId(type.id()) + .velocityVarName("fileAsset").type(com.dotcms.contenttype.model.field.BinaryField.class).nextPersisted(); + com.dotcms.datagen.ContentTypeDataGen.addField(binaryField); + final var imageField = new com.dotcms.datagen.FieldDataGen().contentTypeId(type.id()) + .velocityVarName("linkedImage").type(com.dotcms.contenttype.model.field.ImageField.class).nextPersisted(); + com.dotcms.datagen.ContentTypeDataGen.addField(imageField); + final var linked = new com.dotcms.datagen.ContentletDataGen(type) + .setProperty("fileAsset", Files.writeString(root.resolve("Linked-Name.Txt"), text).toFile()) + .nextPersisted(); + final var parent = new com.dotcms.datagen.ContentletDataGen(type) + .setProperty("fileAsset", source).setProperty("linkedImage", linked.getIdentifier()).nextPersisted(); + final var linkedFile = linked.getBinary("fileAsset"); + final var delegate = new com.dotcms.content.model.hydration.MetadataDelegate(); + assertTrue(assets.evictLocalFile(linkedFile)); + final var imageBuilder = com.dotcms.content.model.type.ImageFieldType.builder().value(linked.getIdentifier()); + delegate.hydrate(imageBuilder, imageField, parent, "metadata"); + assertEquals(hash, imageBuilder.build().metadata().get("sha256"), "Image metadata belongs to its linked binary"); + assertEquals("Linked-Name.Txt", imageBuilder.build().metadata().get("name")); + assertFalse(linkedFile.exists(), "Complete stored metadata must not download the original"); + + // No metadata exists at this snapshot key: hydration must restore the evicted revision. + final var cold = new com.dotmarketing.portlets.contentlet.model.Contentlet(linked); + final String missingMetadata = BinaryAssetReference.newMetadataKey(linkedFile, linked.getInode(), "fileAsset"); + cold.setProperty("fileAsset", BinaryAssetReference.withMetadata(linkedFile, linked.getInode(), "fileAsset", missingMetadata)); + final var binaryBuilder = com.dotcms.content.model.type.system.BinaryFieldType.builder().value(linkedFile.getName()); + delegate.hydrate(binaryBuilder, binaryField, cold, "metadata"); + assertEquals(hash, binaryBuilder.build().metadata().get("sha256")); + assertTrue(linkedFile.exists()); + assertEquals(missingMetadata, metadata.getFileName(cold, "fileAsset")); + } finally { + com.dotcms.datagen.ContentTypeDataGen.remove(type); + } + } + +} diff --git a/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletAPITest.java b/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletAPITest.java index 1f7fb5993f9a..9e2246477bf1 100644 --- a/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletAPITest.java +++ b/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletAPITest.java @@ -3673,7 +3673,7 @@ public void deleteAllVersionsAndBackup() throws DotSecurityException, DotDataExc //Validations assertNotNull(versions); - assertEquals(versions.size(), 1); + assertEquals(com.dotcms.storage.AssetStorageFeature.isEnabled() ? 0 : 1, versions.size()); APILocator.getStructureAPI().delete(testStructure, user); }