diff --git a/docs/testing/BINARY_S3_STORAGE.md b/docs/testing/BINARY_S3_STORAGE.md index 127241a53077..314699014adc 100644 --- a/docs/testing/BINARY_S3_STORAGE.md +++ b/docs/testing/BINARY_S3_STORAGE.md @@ -387,6 +387,66 @@ per direct child record on every listing, and that includes the tombstone of eve child, so it grows with the number of temporary names ever written directly in that folder until tombstones can be reclaimed. +## Renditions and rendering + +With the flag on, completed image-filter outputs are uploaded to the `generated-assets` group as +they are produced, at `generated-assets/{first}/{second}/{inode}/dotGenerated_...`, with the same +relative layout under the local `dotGenerated` root. A cold read restores a completed rendition +from S3 without running the filter again. A warm hit is served from local disk without contacting +S3, so an S3 outage does not break a cached response and a warm read cannot republish an +invalidated object. A cold read, which has to look in S3, fails while S3 is unavailable. +Incomplete filter outputs are never uploaded or evicted. Thumbnail invalidation and source +deletion target that inode's directory. With the flag off, renditions keep the existing +two-character layout. Existing rendition objects are not moved to the new layout. + +The exporter finds an existing rendition by predicting the path each filter writes, so each +filter's prediction must match its output; the JPEG filter, for example, predicts `.jpg`. Some +requests leave the image unchanged, for instance a maximum width larger than the image. Such a +filter writes nothing, so the first request for it looks for its predicted output in S3 and finds +nothing. The node then remembers that this output is never produced, and later requests on that +node skip the lookup. Only an output written at its predicted path is uploaded. + +Upload failures do not fail the request. If S3 rejects a rendition upload, the locally produced +rendition is served and a warning is logged. The rendition then stays local only: later requests +on that node are warm hits and do not retry the upload, eviction keeps the file because it has no +durable copy, and other nodes generate their own. Uploads run after the image-generation permit is +released. + +Rendition keys include the original's revision key, so replacing bytes under the same inode and +filename never selects the previous revision's rendition. Crops read the focal point from the +content's metadata snapshot, including focal points saved on a temporary upload, and the Java and +native crop engines use the same pinned coordinates. When a focal point is read through the +content API instead, as `$content.image.fpx` does, content the current user cannot read, or that +does not exist, has no focal point, as with the flag off; only a failed lookup is an error. + +Compiled Sass output also uses the `generated-assets` group. Its key covers the source and +dependency bytes, site, live or working mode, compiler options and application build, so a cold +node restores a matching CSS output without running Sass. Source maps keep their existing private, +uncached behavior. As with renditions, a compiled output whose upload fails is still served, +with a warning, and is kept locally only. Each change to the key, such as an edit to an imported +partial, a working-mode compile of an edit in progress, or an upgrade, stores a new CSS object +without removing the old one. Old objects are removed only when the main Sass file's generated +outputs are invalidated or its inode's binaries are deleted, so for a main file that rarely +changes they accumulate. Superseded compiled CSS objects are not reclaimed yet. + +Markdown files are read through the protected FileAsset stream, and VTL and included files are +opened through the binary asset API, so a file whose local copy was evicted is restored from S3 +before it is rendered. A missing VTL or included file outside the asset root cannot be restored, +so it is reported as not found, as with the flag off. When restoring a VTL, macro or included file +fails, the render fails with a storage error that is not cached as a missing template, so the next +render tries again. Templates, containers, pages and other database-built sources keep their +existing failure handling. + +### Known limitation: long cache leases + +Image-filter execution holds a cache lease for the whole export: the S3 lookups, the pixel work +and the uploads of every filter in the chain. Sass compilation holds a lease while it gathers and +copies the sources, which includes database lookups, and while it reads a stored output, but not +while Dart Sass runs. Eviction skips every file while any lease is held, so on a node with steady +image traffic a sweep can be deferred again and again, and the local cache can stay above its +budget. Narrowing the rendition lease to the moments a filter reads a local file needs the filters +to restore their inputs themselves, which is a larger change and is left as a follow-up. + ## Local cache eviction With the flag on, the local asset directory is a cache that `BinaryCacheEvictionJob` can trim. @@ -485,7 +545,7 @@ docker run -d --rm --name binary-cleanup-postgres-test \ -e POSTGRES_DB=binary_storage_test postgres:16-alpine ./mvnw test -pl :dotcms-core -Dmaven.build.cache.enabled=false \ - -Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest,BinaryAssetReferenceTest,BinaryCacheEvictionJobTest,BinaryFileSystemStorageTest,BinaryAssetStorageAPIImplTest,MetadataLocalCacheTest,BinaryAssetCleanupTransactionTest,BinaryAssetCleanupProcessorTest,ContentletBackupStorageGateTest,BinaryFieldCleanupProcessorTest,AssetJobEventSerializationTest,BinaryAssetBackfillCheckpointTest,ImportStarterWorkflowCleanupTest,BinaryAssetBackfillProcessorTest,BinaryAssetBackfillTest,ExportStarterFailureTest,BundleArchiveStorageTest,FileAssetBundlerTest,TemporaryAssetStorageTest,WebdavAssetStorageTest,TemporaryMetadataStorageTest,WebdavTemporaryStorageTest \ + -Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest,BinaryAssetReferenceTest,BinaryCacheEvictionJobTest,BinaryFileSystemStorageTest,BinaryAssetStorageAPIImplTest,MetadataLocalCacheTest,BinaryAssetCleanupTransactionTest,BinaryAssetCleanupProcessorTest,ContentletBackupStorageGateTest,BinaryFieldCleanupProcessorTest,AssetJobEventSerializationTest,BinaryAssetBackfillCheckpointTest,ImportStarterWorkflowCleanupTest,BinaryAssetBackfillProcessorTest,BinaryAssetBackfillTest,ExportStarterFailureTest,BundleArchiveStorageTest,FileAssetBundlerTest,TemporaryAssetStorageTest,WebdavAssetStorageTest,TemporaryMetadataStorageTest,WebdavTemporaryStorageTest,ImageFilterExporterFocalPointTest,ImageFilterExporterTest,ImageFilterExporterSharedStoreTest,ImageFilterExporterEngineSelectionTest,ImageFilterExporterStorageTest,FocalPointAPIImplTest,StoredVelocityFileLoaderTest \ -Ds3.test.endpoint=http://127.0.0.1:19002 \ -Ds3.test.jdbc=jdbc:postgresql://127.0.0.1:19003/binary_storage_test @@ -498,7 +558,7 @@ stack: ```sh ./mvnw install -pl :dotcms-core --am -DskipTests -Ddocker.skip ./mvnw verify -pl :dotcms-integration -Dmaven.build.cache.enabled=false -Dcoreit.test.skip=false \ - -Dit.test=BinaryAssetStorageIntegrationTest,ContentletBackupStorageTest,SharedAssetStorageIntegrationTest,BinaryAssetStarterRestoreTest,PublishingArchiveStorageTest,DotWebdavHelperTest + -Dit.test=BinaryAssetStorageIntegrationTest,ContentletBackupStorageTest,SharedAssetStorageIntegrationTest,BinaryAssetStarterRestoreTest,PublishingArchiveStorageTest,DotWebdavHelperTest,AssetTemplateStorageTest,CSSAssetStorageTest ``` These default to flag-off mode, where the S3 cases are skipped. To run the S3 cases, create a diff --git a/dotCMS/src/main/java/com/dotcms/csspreproc/DotCSSCompiler.java b/dotCMS/src/main/java/com/dotcms/csspreproc/DotCSSCompiler.java index 3783e606e14c..1afa218e3374 100644 --- a/dotCMS/src/main/java/com/dotcms/csspreproc/DotCSSCompiler.java +++ b/dotCMS/src/main/java/com/dotcms/csspreproc/DotCSSCompiler.java @@ -54,6 +54,7 @@ abstract class DotCSSCompiler { protected final String inputURI; protected final Host inputHost; protected final boolean inputLive; + protected final Map sourceDigests = new java.util.TreeMap<>(); public Set getAllImportedURI() { return allImportedURI; @@ -165,6 +166,18 @@ protected List filterImportedUris(String baseUri, List uris) thr protected File createCompileDir(Host currentHost, String uri, boolean live) throws IOException, DotStateException, DotDataException, DotSecurityException { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try (var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease()) { + sourceDigests.clear(); + return createCompileDirInternal(currentHost, uri, live); + } + } + return createCompileDirInternal(currentHost, uri, live); + } + + private File createCompileDirInternal(Host currentHost, String uri, boolean live) + throws IOException, DotStateException, DotDataException, DotSecurityException { + File compDir = new File(APILocator.getFileAssetAPI().getRealAssetPathTmpBinary() + File.separator + "css_compile_space" + File.separator + UUIDGenerator.generateUuid()); compDir.mkdirs(); @@ -195,6 +208,7 @@ protected File createCompileDir(Host currentHost, String uri, boolean live) if (file != null && InodeUtils.isSet(file.getInode())) { File ff = file.getBinary(FileAssetAPI.BINARY_FIELD); if (ff != null && ff.isFile()) { + recordSource(host.getHostname() + fileuri, ff); // destfile: /../../../tmpdir/host/path/to/asset.extension @@ -288,6 +302,7 @@ protected File createCompileDir(Host currentHost, String uri, boolean live) getAllImportedURI().add(assetUri); f.getParentFile().mkdirs(); FileUtil.copyFile(asset.getFileAsset(), f); + recordSource(inputHost.getHostname() + assetUri, f); } } @@ -295,6 +310,14 @@ protected File createCompileDir(Host currentHost, String uri, boolean live) return compDir; } + private void recordSource(final String path, final File file) throws IOException { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try (var input = java.nio.file.Files.newInputStream(file.toPath())) { + sourceDigests.put(path, org.apache.commons.codec.digest.DigestUtils.sha256Hex(input)); + } + } + } + private String addImportUnderscore(String url) { return url.substring(0, url.lastIndexOf('/')) + "/_" + url.substring(url.lastIndexOf('/') + 1, url.length()); } diff --git a/dotCMS/src/main/java/com/dotcms/csspreproc/DotLibSassCompiler.java b/dotCMS/src/main/java/com/dotcms/csspreproc/DotLibSassCompiler.java index f6c5b10f3903..bc6613273204 100644 --- a/dotCMS/src/main/java/com/dotcms/csspreproc/DotLibSassCompiler.java +++ b/dotCMS/src/main/java/com/dotcms/csspreproc/DotLibSassCompiler.java @@ -62,6 +62,20 @@ public DotLibSassCompiler(final Host host, final String uri, final boolean live, this.sourceMap = sourceMap; } + /** + * Compiles the SCSS or Sass file into {@link #getOutput()}. + * + *

With S3 asset storage on, a compiled CSS output stored under a key that covers the source + * bytes, site, mode, options and build is reused, locally or restored from S3, without running + * Dart Sass. A new output is written locally and uploaded. If storing it fails, the compiled + * CSS is still returned and a warning is logged. A cache lease is held only while the sources + * are copied and while a stored output is read, not while Dart Sass runs. + * + * @throws DotSecurityException if a source cannot be read by the system user + * @throws DotStateException if the dotCMS state is inconsistent + * @throws DotDataException if a source or its version information cannot be loaded + * @throws IOException if the compile directory cannot be prepared + */ @Override public void compile() throws DotSecurityException, DotStateException, DotDataException, IOException { @@ -88,7 +102,9 @@ public void compile() throws DotSecurityException, DotStateException, DotDataExc } final FileAsset mainFile = APILocator.getFileAssetAPI() - .fromContentlet(APILocator.getContentletAPI().find(info.get().getWorkingInode(), + .fromContentlet(APILocator.getContentletAPI().find( + com.dotcms.storage.AssetStorageFeature.isEnabled() && live + ? info.get().getLiveInode() : info.get().getWorkingInode(), APILocator.systemUser(), true)); // build directories to build scss @@ -102,10 +118,31 @@ public void compile() throws DotSecurityException, DotStateException, DotDataExc } final File compileDestinationFile = new File(compileTargetFile.getAbsoluteFile() + ".css"); final CompilerOptions options = new CompilerOptions.Builder().sourceMap(this.sourceMap).build(); + // Source maps remain request-private and uncached, as in the servlet's existing contract. + final File storedCSS = com.dotcms.storage.AssetStorageFeature.isEnabled() && !sourceMap + ? compiledCacheFile(mainFile.getInode(), options) : null; + if (storedCSS != null) { + try (var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease()) { + final File cached = APILocator.getBinaryAssetStorageAPI().getGeneratedFile(storedCSS); + if (cached != null && (req == null || req.getParameter("recompile") == null)) { + this.output = java.nio.file.Files.readAllBytes(cached.toPath()); + return; + } + } + } final DartSassCompiler compiler = new DartSassCompiler(options, compileTargetFile, compileDestinationFile); final Optional out = compiler.compile(); handleOutput(compiler.terminalOutput()); this.output = out.isPresent() ? out.get().getBytes() : null; + if (storedCSS != null && this.output != null && compiler.terminalOutput().exitValue() == 0) { + try { + storeCompiledOutput(storedCSS); + } catch (final Exception e) { + // Availability first: the CSS is compiled, so a storage failure must not fail the request. + Logger.warnAndDebug(DotLibSassCompiler.class, "Unable to store compiled CSS for " + inputHost + + ":" + inputURI + "; serving the compiled output: " + e.getMessage(), e); + } + } } catch (final Exception ex) { final String errorMsg = String.format("Unable to compile SASS code in %s:%s [ live:%s ]: %s", inputHost, inputURI, inputLive, ex.getMessage()); @@ -119,6 +156,46 @@ public void compile() throws DotSecurityException, DotStateException, DotDataExc } } + private File compiledCacheFile(final String inode, final CompilerOptions options) { + final StringBuilder identity = new StringBuilder("css-v1:") + .append(com.liferay.portal.util.ReleaseInfo.getVersion()).append(':') + .append(com.liferay.portal.util.ReleaseInfo.getBuildNumber()).append(':') + .append(inputHost.getIdentifier()).append(':').append(inputURI).append(':') + .append(live).append(':').append(options.generate()); + sourceDigests.forEach((path, digest) -> identity.append(':').append(path.length()) + .append(':').append(path).append(':').append(digest)); + final String hash = org.apache.commons.codec.digest.DigestUtils.md5Hex(identity.toString()); + return new File(com.dotmarketing.util.ConfigUtils.getDotGeneratedPath(), + inode.charAt(0) + "/" + inode.charAt(1) + "/" + inode + "/dotGenerated_css_" + hash + ".css"); + } + + /** + * Writes the compiled CSS atomically to its local {@code dotGenerated} path and uploads it. + * If the upload fails, the local file is kept: later requests on this node serve it, and + * eviction never removes it because it has no durable copy. + * + * @param target the local path of the compiled CSS + * @throws IOException if the local file cannot be written + * @throws DotDataException if the upload fails + */ + private void storeCompiledOutput(final File target) throws IOException, DotDataException { + final java.nio.file.Path path = target.toPath(); + java.nio.file.Files.createDirectories(path.getParent()); + final java.nio.file.Path temporary = java.nio.file.Files.createTempFile(path.getParent(), "css-", ".tmp"); + try { + java.nio.file.Files.write(temporary, this.output); + try { + java.nio.file.Files.move(temporary, path, java.nio.file.StandardCopyOption.ATOMIC_MOVE, + java.nio.file.StandardCopyOption.REPLACE_EXISTING); + } catch (java.nio.file.AtomicMoveNotSupportedException e) { + java.nio.file.Files.move(temporary, path, java.nio.file.StandardCopyOption.REPLACE_EXISTING); + } + APILocator.getBinaryAssetStorageAPI().storeGeneratedFile(target); + } finally { + java.nio.file.Files.deleteIfExists(temporary); + } + } + /** * Handles special situations related to how Dart SASS notifies Users when errors or warnings were generated during * the compilation. The {@link RuntimeUtils.TerminalOutput} contains important information generated by the CLI diff --git a/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/DotResourceLoader.java b/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/DotResourceLoader.java index 136157492bf0..e2c4103b61bd 100644 --- a/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/DotResourceLoader.java +++ b/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/DotResourceLoader.java @@ -2,12 +2,14 @@ import com.dotmarketing.business.CacheLocator; +import com.dotmarketing.exception.DotDataException; import com.dotmarketing.util.Config; import com.dotmarketing.util.Logger; import com.dotmarketing.util.UtilMethods; import java.io.InputStream; import org.apache.commons.collections.ExtendedProperties; import org.apache.velocity.exception.ResourceNotFoundException; +import org.apache.velocity.exception.VelocityException; import org.apache.velocity.runtime.resource.Resource; import org.apache.velocity.runtime.resource.loader.ResourceLoader; @@ -22,6 +24,19 @@ public DotResourceLoader() { } + /** + * Opens the Velocity source for a resource path, choosing the loader by the path's type. + * + *

A failure normally records a cache miss and is reported as a + * {@link ResourceNotFoundException}. With S3 asset storage on, a {@link DotDataException} + * from a loader that reads a stored file (VTL, macro, legacy VL and include files) means the + * file could not be restored from storage. It is reported as a plain {@link VelocityException} + * and no miss is recorded, so the next render tries the storage again. + * + * @param filePath the Velocity resource path + * @return the resource source + * @throws ResourceNotFoundException if the path is empty or the resource cannot be loaded + */ @Override public InputStream getResourceStream(final String filePath) throws ResourceNotFoundException { if (!UtilMethods.isSet(filePath)) { @@ -75,6 +90,10 @@ public InputStream getResourceStream(final String filePath) throws ResourceNotFo } catch (Exception e) { Logger.warn(this, "filePath: " + filePath + ", msg:" + e.getMessage(), e); + if (com.dotcms.storage.AssetStorageFeature.isEnabled() + && e instanceof DotDataException && readsStoredFile(key.type)) { + throw new VelocityException("Unable to load stored resource: " + key.path, e); + } CacheLocator.getVeloctyResourceCache().addMiss(key.path); throw new ResourceNotFoundException("Cannot parse velocity file : " + key.path, e); } @@ -82,6 +101,29 @@ public InputStream getResourceStream(final String filePath) throws ResourceNotFo } + /** + * Tells whether a resource type is loaded from a file that S3 asset storage may have to + * restore. These are the types {@link #getResourceStream} hands to {@link VTLLoader} and + * {@link IncludeLoader}; the other loaders build their source from the database. + * + * @param type the resource type + * @return {@code true} for VTL, macro, legacy VL and include files + */ + private static boolean readsStoredFile(final VelocityType type) { + switch (type) { + case CONTAINER: + case TEMPLATE: + case CONTENT: + case FIELD: + case CONTENT_TYPE: + case SITE: + case HTMLPAGE: + return false; + default: + return true; + } + } + /* * (non-Javadoc) * diff --git a/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/IncludeLoader.java b/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/IncludeLoader.java index 2ac5c1db0efa..ff3bf6394eec 100644 --- a/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/IncludeLoader.java +++ b/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/IncludeLoader.java @@ -58,16 +58,35 @@ private Set buildAllowedExtensions() { } + /** + * Checks that an included file may be served: it must be under the Velocity root or the asset + * root and have an allowed extension. + * + *

With S3 asset storage on, a missing file under the asset root passes this check so that + * it can be restored from storage. A missing file anywhere else is reported as not found, as + * with the flag off, rather than logged as a possible attack. + * + * @param filePath the file path from the Velocity resource key + * @return the file to serve + * @throws IOException if a canonical path cannot be resolved + * @throws ResourceNotFoundException if the file is missing or not allowed + */ private File allowedToServe(final String filePath) throws IOException { final File fileToServe = new File(filePath); - if (!fileToServe.exists()) { + final boolean s3 = com.dotcms.storage.AssetStorageFeature.isEnabled(); + if (!fileToServe.exists() + && (!s3 || !fileToServe.getCanonicalFile().toPath().startsWith(allowedPaths[1]))) { throw new ResourceNotFoundException(CANNOT_FIND_RESOURCE + filePath); } String canonicalPath = fileToServe.getCanonicalPath(); String fileExtension = UtilMethods.getFileExtension(canonicalPath).toLowerCase(); - if ((canonicalPath.contains(allowedPaths[0]) || canonicalPath.contains(allowedPaths[1])) + final boolean allowedPath = s3 + ? fileToServe.getCanonicalFile().toPath().startsWith(allowedPaths[0]) + || fileToServe.getCanonicalFile().toPath().startsWith(allowedPaths[1]) + : canonicalPath.contains(allowedPaths[0]) || canonicalPath.contains(allowedPaths[1]); + if (allowedPath && allowedExtensions.contains(fileExtension)) { return fileToServe; @@ -78,17 +97,21 @@ private File allowedToServe(final String filePath) throws IOException { } - InputStream streamFile(String filePath) throws IOException { + InputStream streamFile(String filePath) throws IOException, com.dotmarketing.exception.DotDataException { Logger.debug(this, "Not a CMS Velocity File : " + filePath); final File fileToServe = allowedToServe(filePath); - return new BufferedInputStream(Files.newInputStream(fileToServe.toPath())); + return new BufferedInputStream(com.dotcms.storage.AssetStorageFeature.isEnabled() + ? com.dotmarketing.business.APILocator.getBinaryAssetStorageAPI().openLocalFile(fileToServe) + : Files.newInputStream(fileToServe.toPath())); } @Override - public InputStream writeObject(final VelocityResourceKey key) { + public InputStream writeObject(final VelocityResourceKey key) throws com.dotmarketing.exception.DotDataException { try { return streamFile(key.path); + } catch (com.dotmarketing.exception.DotDataException e) { + throw e; } catch (Exception e) { throw new ResourceNotFoundException(CANNOT_FIND_RESOURCE + key.path, e); } diff --git a/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/VTLLoader.java b/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/VTLLoader.java index 872ce24d534e..744a395992b1 100644 --- a/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/VTLLoader.java +++ b/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/VTLLoader.java @@ -80,7 +80,36 @@ private void init() { } - InputStream streamFile(String filePath) throws IOException { + /** + * Tells whether a file is under the asset root, the only place S3 asset storage can restore + * a missing file from. + * + * @param file the file to check + * @return {@code true} if its canonical path is under the canonical asset root + * @throws IOException if a canonical path cannot be resolved + */ + private static boolean underAssetRoot(final File file) throws IOException { + final String assetRoot = ConfigUtils.getAssetPath(); + return UtilMethods.isSet(assetRoot) && file.getCanonicalFile().toPath() + .startsWith(new File(assetRoot).getCanonicalFile().toPath()); + } + + /** + * Opens a VTL, macro or legacy VL file from the Velocity root, the dynamic content path or + * the asset directories, refusing paths outside them. + * + *

With S3 asset storage on, the file is opened through the binary asset API, so a file + * whose local copy was evicted is restored first. A missing file outside the asset root + * cannot be restored and is reported as not found, as with the flag off. + * + * @param filePath the file path from the Velocity resource key + * @return the file contents + * @throws IOException if the file cannot be read + * @throws DotDataException if restoring the file from storage fails + * @throws ResourceNotFoundException if the file is missing or outside the allowed roots + */ + InputStream streamFile(String filePath) throws IOException, DotDataException { + final boolean s3 = com.dotcms.storage.AssetStorageFeature.isEnabled(); boolean serveFile = false; Logger.debug(this, "Not a CMS Velocity File : " + filePath); @@ -96,13 +125,20 @@ InputStream streamFile(String filePath) throws IOException { if (!f.exists()) { f = new java.io.File(filePath); } - if (!f.exists()) { + // A missing file can only be restored from storage when it is under the asset root. + if (!f.exists() && (!s3 || !underAssetRoot(f))) { throw new ResourceNotFoundException("cannot find resource"); } String canon = f.getCanonicalPath(); File dynamicContent = new File(ConfigUtils.getDynamicContentPath()); - if (assetRealCanoncalPath != null && canon.startsWith(assetRealCanoncalPath)) { + if (s3) { + final java.nio.file.Path path = f.getCanonicalFile().toPath(); + serveFile = (assetRealCanoncalPath != null && path.startsWith(assetRealCanoncalPath)) + || (velocityCanoncalPath != null && path.startsWith(velocityCanoncalPath)) + || (assetCanoncalPath != null && path.startsWith(assetCanoncalPath)) + || path.startsWith(dynamicContent.getCanonicalFile().toPath()); + } else if (assetRealCanoncalPath != null && canon.startsWith(assetRealCanoncalPath)) { serveFile = true; } else if (velocityCanoncalPath != null && canon.startsWith(velocityCanoncalPath)) { serveFile = true; @@ -115,7 +151,9 @@ InputStream streamFile(String filePath) throws IOException { Logger.warn(this, "POSSIBLE HACK ATTACK DotResourceLoader: " + lookingFor); throw new ResourceNotFoundException("cannot find resource"); } - return new BufferedInputStream(Files.newInputStream(f.toPath())); + return new BufferedInputStream(s3 + ? com.dotmarketing.business.APILocator.getBinaryAssetStorageAPI().openLocalFile(f) + : Files.newInputStream(f.toPath())); } diff --git a/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/MarkdownTool.java b/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/MarkdownTool.java index 0f333fa3fd39..980d7ff08e5f 100755 --- a/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/MarkdownTool.java +++ b/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/MarkdownTool.java @@ -122,6 +122,11 @@ private String getFileContents(String path, boolean parseFirst) throws Throwable template.merge(ctx, writer); return writer.toString(); } else { + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + try (var input = asset.getInputStream()) { + return new String(input.readAllBytes()); + } + } return new String(FileUtil.getBytes(asset.getFileAsset().getAbsoluteFile())); } diff --git a/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/WebAPI.java b/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/WebAPI.java index 299a3e4cbf95..7f843d41caa8 100644 --- a/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/WebAPI.java +++ b/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/WebAPI.java @@ -1,5 +1,6 @@ package com.dotcms.rendering.velocity.viewtools; import com.dotcms.rendering.velocity.services.VelocityType; +import java.io.File; import java.io.IOException; import java.io.StringWriter; import java.text.SimpleDateFormat; @@ -41,6 +42,7 @@ import com.dotmarketing.portlets.contentlet.model.Contentlet; import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo; import com.dotmarketing.portlets.fileassets.business.FileAsset; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; import com.dotmarketing.portlets.languagesmanager.business.LanguageAPI; import com.dotmarketing.portlets.languagesmanager.model.Language; import com.dotmarketing.util.Config; @@ -399,8 +401,13 @@ public String getAssetPath(final String path, final Host host){ String conInode = PREVIEW_MODE && EDIT_MODE ? cvi.get().getWorkingInode() : cvi.get().getLiveInode(); FileAsset file = APILocator.getFileAssetAPI().fromContentlet(APILocator.getContentletAPI().find(conInode, user, true)); - - return APILocator.getFileAssetAPI().getRealAssetPath(conInode, file.getUnderlyingFileName()); + + if (!com.dotcms.storage.AssetStorageFeature.isEnabled()) { + return APILocator.getFileAssetAPI().getRealAssetPath(conInode, file.getUnderlyingFileName()); + } + final File binaryFile = APILocator.getBinaryAssetStorageAPI() + .getBinaryFile(conInode, FileAssetAPI.BINARY_FIELD); + return binaryFile != null ? binaryFile.getAbsolutePath() : null; }catch (Exception e) { Logger.warn(this, e.getMessage()); diff --git a/dotCMS/src/main/java/com/dotmarketing/image/filter/CropImageFilter.java b/dotCMS/src/main/java/com/dotmarketing/image/filter/CropImageFilter.java index 244af377f7ea..5942f06a2a1a 100644 --- a/dotCMS/src/main/java/com/dotmarketing/image/filter/CropImageFilter.java +++ b/dotCMS/src/main/java/com/dotmarketing/image/filter/CropImageFilter.java @@ -116,9 +116,13 @@ else if(widthInput >0 && heightInput >0){ protected Optional calcFocalPoint(final BufferedImage src, final Map parameters) { final Dimension current = new Dimension(src.getWidth(), src.getHeight()); - Optional optPoint = new FocalPointAPIImpl().parseFocalPointFromParams(parameters); + final boolean resolved = com.dotcms.storage.AssetStorageFeature.isEnabled() + && parameters.containsKey(RESOLVED_CROP_FOCAL_POINT); + Optional optPoint = resolved + ? new FocalPointAPIImpl().parseFocalPoint(parameters.get(RESOLVED_CROP_FOCAL_POINT)[0]) + : new FocalPointAPIImpl().parseFocalPointFromParams(parameters); - if (optPoint.isEmpty()) { + if (optPoint.isEmpty() && !resolved) { final String inode = parameters.get("assetInodeOrIdentifier")[0]; final String fieldVar = parameters.get("fieldVarName")[0]; optPoint = new FocalPointAPIImpl().readFocalPoint(inode, fieldVar); diff --git a/dotCMS/src/main/java/com/dotmarketing/image/filter/ImageFilter.java b/dotCMS/src/main/java/com/dotmarketing/image/filter/ImageFilter.java index bbc1c3a7685d..8324aa8395a4 100644 --- a/dotCMS/src/main/java/com/dotmarketing/image/filter/ImageFilter.java +++ b/dotCMS/src/main/java/com/dotmarketing/image/filter/ImageFilter.java @@ -25,6 +25,7 @@ import java.util.Optional; public abstract class ImageFilter implements ImageFilterIf { + public static final String RESOLVED_CROP_FOCAL_POINT = "_resolvedCropFocalPoint"; protected static final String FILE_EXT = "png"; public static final String CROP = "crop"; @@ -82,8 +83,12 @@ private String getUniqueFileName(File file, Map parameters, St } if (CROP.equals(thisFilter)) { - Optional optPoint = new FocalPointAPIImpl().parseFocalPointFromParams(parameters); - if(optPoint.isEmpty()) { + final boolean resolved = com.dotcms.storage.AssetStorageFeature.isEnabled() + && parameters.containsKey(RESOLVED_CROP_FOCAL_POINT); + Optional optPoint = resolved + ? new FocalPointAPIImpl().parseFocalPoint(parameters.get(RESOLVED_CROP_FOCAL_POINT)[0]) + : new FocalPointAPIImpl().parseFocalPointFromParams(parameters); + if(optPoint.isEmpty() && !resolved) { String fieldVar = parameters.get("fieldVarName")[0]; optPoint =new FocalPointAPIImpl().readFocalPoint(inode, fieldVar); } @@ -96,6 +101,12 @@ private String getUniqueFileName(File file, Map parameters, St MessageDigest digest = java.security.MessageDigest.getInstance("MD5"); + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + final String revision = com.dotcms.storage.binary.BinaryAssetReference.keyOf(file); + if (revision != null) { + digest.update(revision.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + } + } digest.update((inode + sb.toString() + this.getClass() + file.getName()).getBytes()); StringBuilder ret = new StringBuilder(); @@ -193,20 +204,33 @@ protected final File getResultsFile(File file, Map parameters, String inode =null; - if (file.getName().startsWith(WebKeys.GENERATED_FILE)) { + final boolean s3Assets = com.dotcms.storage.AssetStorageFeature.isEnabled(); + final boolean generated = file.getName().startsWith(WebKeys.GENERATED_FILE) + && (!s3Assets || Try.of(() -> file.getCanonicalFile().toPath().startsWith( + new File(ConfigUtils.getDotGeneratedPath()).getCanonicalFile().toPath())).getOrElse(false)); + if (generated) { inode = file.getName(); String fileNameNoExt = this.getUniqueFileName(file, parameters, inode); String resultFilePath = fileFolderPath + File.separator + fileNameNoExt + "." + fileExt; return new File(resultFilePath); } else { try { - inode = RegEX.find(file.getCanonicalPath(), "[\\w]{8}(-[\\w]{4}){3}-[\\w]{12}").get(0).getMatch(); + final String revision = com.dotcms.storage.AssetStorageFeature.isEnabled() + ? com.dotcms.storage.binary.BinaryAssetReference.keyOf(file) : null; + inode = revision != null ? revision.split("/")[2] + : RegEX.find(file.getCanonicalPath(), "[\\w]{8}(-[\\w]{4}){3}-[\\w]{12}").get(0).getMatch(); } catch (Exception e) { inode = parameters.get("assetInodeOrIdentifier")[0]; } + if (s3Assets && (inode == null || !inode.matches("[A-Za-z0-9_-]{2,}"))) { + throw new IllegalArgumentException("Invalid rendition inode"); + } File dirs = new File(ConfigUtils.getDotGeneratedPath() + File.separator + inode.charAt(0) + File.separator + inode.charAt(1)); + if (s3Assets) { + dirs = new File(dirs, inode); + } if (!dirs.exists()) { dirs.mkdirs(); } diff --git a/dotCMS/src/main/java/com/dotmarketing/image/filter/JpegImageFilter.java b/dotCMS/src/main/java/com/dotmarketing/image/filter/JpegImageFilter.java index 513cdb90946a..3f2dd5e82d08 100644 --- a/dotCMS/src/main/java/com/dotmarketing/image/filter/JpegImageFilter.java +++ b/dotCMS/src/main/java/com/dotmarketing/image/filter/JpegImageFilter.java @@ -24,6 +24,24 @@ public String[] getAcceptedParameters(){ }; } + + /** + * Predicts the file this filter writes. With S3 asset storage on, this is the {@code .jpg} + * path that {@link #runFilter} writes, so an existing rendition is found locally or in S3 + * without running the filter. With the flag off, it keeps the inherited {@code .png} + * prediction, so the existing cache lookups behave exactly as before. + * + * @param file the input image + * @param parameters the request's filter parameters + * @return the predicted output file + */ + @Override + public File getResultsFile(final File file, final Map parameters) { + return com.dotcms.storage.AssetStorageFeature.isEnabled() + ? getResultsFile(file, parameters, "jpg") + : super.getResultsFile(file, parameters); + } + public File runFilter(File file, Map parameters) { int quality = parameters.get(getPrefix() +"q") != null?Integer.parseInt(parameters.get(getPrefix() +"q")[0]):85; boolean progressive = (parameters.get(getPrefix() +"p") != null); diff --git a/dotCMS/src/main/java/com/dotmarketing/image/filter/ResizeImageFilter.java b/dotCMS/src/main/java/com/dotmarketing/image/filter/ResizeImageFilter.java index 098f33deddcb..953fc907006c 100644 --- a/dotCMS/src/main/java/com/dotmarketing/image/filter/ResizeImageFilter.java +++ b/dotCMS/src/main/java/com/dotmarketing/image/filter/ResizeImageFilter.java @@ -20,6 +20,14 @@ public String[] getAcceptedParameters(){ "minh (int) specifies minHeight" }; } + @Override + public File getResultsFile(final File file, final Map parameters) { + // Match the GIF delegate's cache key so a remote rendition can be found without regenerating it. + return com.dotcms.storage.AssetStorageFeature.isEnabled() && file.getName().endsWith(".gif") + ? new ResizeGifImageFilter().getResultsFile(file, parameters) + : super.getResultsFile(file, parameters); + } + public File runFilter(final File file, Map parameters) { final int w = Try.of(()-> Integer.parseInt(parameters.getOrDefault(getPrefix() + "w", new String[]{"0"})[0])).getOrElse(0); diff --git a/dotCMS/src/main/java/com/dotmarketing/image/focalpoint/FocalPointAPIImpl.java b/dotCMS/src/main/java/com/dotmarketing/image/focalpoint/FocalPointAPIImpl.java index 17a00283c7bf..3f47b89b8eab 100644 --- a/dotCMS/src/main/java/com/dotmarketing/image/focalpoint/FocalPointAPIImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/image/focalpoint/FocalPointAPIImpl.java @@ -120,9 +120,15 @@ private Optional readFocalPointMeta(final Contentlet contentlet, fin try { final Metadata metadata = fileMetadataAPI.getMetadata(contentlet, fieldVar); + if (metadata == null && com.dotcms.storage.AssetStorageFeature.isEnabled()) { + return Optional.empty(); + } return parseFocalPoint( (String) metadata.getCustomMeta().get(FOCAL_POINT)); }catch (Exception e){ + if (com.dotcms.storage.AssetStorageFeature.isEnabled()) { + throw new DotRuntimeException("Unable to read focal-point metadata", e); + } Logger.debug (FocalPointAPIImpl.class, "Metadata Error retrieving focal point from custom metadata", e); } return Optional.empty(); @@ -183,12 +189,22 @@ public Optional parseFocalPointFromParams(final Map findContentletByInode(final String inode){ - return Optional.ofNullable(Try.of(()->contentletAPI.find(inode, currentUserSupplier.get(), false)).getOrNull()); + final var lookup = Try.of(() -> contentletAPI.find(inode, currentUserSupplier.get(), false)); + if (com.dotcms.storage.AssetStorageFeature.isEnabled() && lookup.isFailure() + && lookup.getCause() instanceof DotDataException) { + throw new DotRuntimeException(lookup.getCause()); + } + return Optional.ofNullable(lookup.getOrNull()); } /** diff --git a/dotCMS/src/main/java/com/dotmarketing/image/vips/VipsCropImageFilter.java b/dotCMS/src/main/java/com/dotmarketing/image/vips/VipsCropImageFilter.java index 79654f1abfee..1f1be16717fa 100644 --- a/dotCMS/src/main/java/com/dotmarketing/image/vips/VipsCropImageFilter.java +++ b/dotCMS/src/main/java/com/dotmarketing/image/vips/VipsCropImageFilter.java @@ -95,8 +95,12 @@ protected void transform(final File in, final File out, final Map calcFocalPoint(final Dimension current, final Map parameters) { final FocalPointAPIImpl api = new FocalPointAPIImpl(); - Optional optPoint = api.parseFocalPointFromParams(parameters); - if (optPoint.isEmpty() && parameters.get("assetInodeOrIdentifier") != null + final boolean resolved = com.dotcms.storage.AssetStorageFeature.isEnabled() + && parameters.containsKey(RESOLVED_CROP_FOCAL_POINT); + Optional optPoint = resolved + ? api.parseFocalPoint(parameters.get(RESOLVED_CROP_FOCAL_POINT)[0]) + : api.parseFocalPointFromParams(parameters); + if (optPoint.isEmpty() && !resolved && parameters.get("assetInodeOrIdentifier") != null && parameters.get("fieldVarName") != null) { final String inode = parameters.get("assetInodeOrIdentifier")[0]; final String fieldVar = parameters.get("fieldVarName")[0]; diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporter.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporter.java index 40eb0eb566a9..3d0a935d9e81 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporter.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporter.java @@ -7,14 +7,24 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Semaphore; import com.dotcms.api.web.HttpServletResponseThreadLocal; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.image.ImageEngine; +import com.dotmarketing.image.focalpoint.FocalPoint; +import com.dotmarketing.image.focalpoint.FocalPointAPI; +import com.dotmarketing.image.focalpoint.FocalPointAPIImpl; +import com.dotcms.storage.binary.BinaryAssetReference; +import com.dotmarketing.portlets.contentlet.model.Contentlet; import com.dotmarketing.image.filter.ImageFilter; import com.dotmarketing.image.filter.ImageFilterAPI; import com.dotmarketing.image.filter.PDFImageFilter; @@ -59,9 +69,21 @@ ImageFilterAPI imageFilterAPI() { * com.dotmarketing.portlets.contentlet.business.BinaryContentExporter#exportContent(java.io.File, * java.util.Map) */ - public BinaryContentExporterData exportContent(File file, final Map parameters) + public BinaryContentExporterData exportContent(File file, final Map suppliedParameters) throws BinaryContentExporterException { + if (s3Renditions()) { + try (var lease = APILocator.getBinaryAssetStorageAPI().acquireCacheLease()) { + return exportContentInternal(file, suppliedParameters); + } + } + return exportContentInternal(file, suppliedParameters); + } + private BinaryContentExporterData exportContentInternal(File file, final Map suppliedParameters) + throws BinaryContentExporterException { + + final Map parameters = s3Renditions() + ? new LinkedHashMap<>(suppliedParameters) : suppliedParameters; final String fileExtension = UtilMethods.getFileExtension(file.getName()); if (UtilMethods.isVectorImage(fileExtension)) { Logger.info(this.getClass(), "Skipping vector image transformation for " + fileExtension); @@ -72,6 +94,12 @@ public BinaryContentExporterData exportContent(File file, final Map> filters = imageFilterAPI().resolveFilters(parameters); + if (s3Renditions() && filters.isEmpty()) { + return new BinaryContentExporterData(file); + } + if (s3Renditions() && filters.containsKey(ImageFilter.CROP)) { + resolveCropFocalPoint(file, parameters); + } parameters.put("filter", filters.keySet().toArray(new String[0])); parameters.put("filters", filters.keySet().toArray(new String[0])); @@ -89,7 +117,7 @@ public BinaryContentExporterData exportContent(File file, final Map= MIN_VALID_FILE_LENGTH) { return new BinaryContentExporterData(shared); @@ -106,7 +134,7 @@ public BinaryContentExporterData exportContent(File file, final Map parameters) { + /** + * Runs one filter of the chain and returns the file to pass to the next filter. + * + *

With S3 asset storage on, the filter's predicted output is looked up first (local disk, + * then S3) so a completed rendition is reused without running the filter. A predicted output + * that this node has seen the filter never produce, because the filter returned its input + * unchanged, is not looked up again, so a no-op filter does not contact S3 on later requests. + * A newly produced output is uploaded after the image permit is released. If the upload + * fails, the local output is still served and a warning is logged; the file stays local only, + * and eviction keeps it because it has no durable copy. + * + *

With the flag off, this only runs the filter under the image permit, as before. + * + * @param imageFilter the filter to run + * @param fileIn the input image, either the original or the previous filter's output + * @param parameters the request's filter parameters + * @return the filter's output, or {@code fileIn} when the filter is a no-op or no image + * permit is available + * @throws DotDataException if the S3 lookup of the predicted output fails + */ + private File runFilter(ImageFilter imageFilter, final File fileIn,final Map parameters) + throws DotDataException { + final File predicted = s3Renditions() ? imageFilter.getResultsFile(fileIn, parameters) : null; + if (predicted != null && UNPRODUCED_RESULTS.getIfPresent(predicted.getPath()) == null) { + final File cached = cachedRendition(predicted); + if (cached != null) { + return cached; + } + } boolean canRun=false; + final File result; try { canRun = semaphore.tryAcquire(); @@ -137,20 +194,83 @@ private File runFilter(ImageFilter imageFilter, final File fileIn,final Map UNPRODUCED_RESULTS = Caffeine.newBuilder() + .maximumSize(10_000).build(); + /** Renditions smaller than this are treated as missing/corrupt (matches the legacy guard). */ private static final long MIN_VALID_FILE_LENGTH = 50L; + /** + * Returns the chain's final rendition when it already exists, so the request can skip the + * filters. + * + *

With S3 asset storage on, the final rendition is looked up locally and then in S3. When a + * filter in the chain is known on this node to return its input unchanged, the predicted final + * path will never be written, so no lookup is made and the chain runs instead. + * + * @param clazzes the filter chain, in order + * @param fileIn the input image + * @param parameters the request's filter parameters + * @return the existing final rendition, or empty when the chain must run + * @throws DotDataException if the S3 lookup fails + */ private Optional alreadyGenerated(final Collection> clazzes, final File fileIn, - final Map parameters) { + final Map parameters) throws DotDataException { + + if (s3Renditions()) { + File predicted = fileIn; + for (final Class filter : clazzes) { + predicted = Try.of(() -> filter.getDeclaredConstructor().newInstance()) + .getOrElseThrow(DotRuntimeException::new).getResultsFile(predicted, parameters); + if (UNPRODUCED_RESULTS.getIfPresent(predicted.getPath()) != null) { + return Optional.empty(); + } + } + return Optional.ofNullable(cachedRendition(predicted)); + } final File fileToReturn = finalResultFile(clazzes, fileIn, parameters); @@ -160,6 +280,51 @@ private Optional alreadyGenerated(final Collection parameters) + throws DotDataException { + final FocalPointAPIImpl focalPoints = new FocalPointAPIImpl(); + Optional point = focalPoints.parseFocalPointFromParams(parameters); + if (point.isEmpty()) { + final String revision = BinaryAssetReference.keyOf(source); + final Path root = Path.of(ConfigUtils.getAssetPath()).toAbsolutePath().normalize(); + final Path sourcePath = source.toPath().toAbsolutePath().normalize(); + final Path relative = root.relativize(sourcePath); + final boolean legacy = sourcePath.startsWith(root) && relative.getNameCount() == 5 + && relative.getName(0).toString().length() == 1 + && relative.getName(1).toString().length() == 1 + && relative.getName(2).toString().startsWith( + relative.getName(0).toString() + relative.getName(1)); + if (revision != null || legacy) { + final String[] path = relative.toString().replace(File.separatorChar, '/').split("/"); + final Contentlet snapshot = new Contentlet(); + snapshot.setInode(path[2]); + snapshot.getMap().put(path[3], source); + final var metadata = APILocator.getFileMetadataAPI().getMetadata(snapshot, path[3]); + if (metadata != null) { + point = focalPoints.parseFocalPoint((String) metadata.getCustomMeta().get(FocalPointAPI.FOCAL_POINT)); + } + } else if (parameters.get("assetInodeOrIdentifier") != null && parameters.get("fieldVarName") != null) { + point = focalPoints.readFocalPoint(parameters.get("assetInodeOrIdentifier")[0], + parameters.get("fieldVarName")[0]); + } + } + // An empty value pins absence too, so later filters cannot read a different metadata value. + parameters.put(ImageFilter.RESOLVED_CROP_FOCAL_POINT, new String[]{point.map(FocalPoint::toString).orElse("")}); + } + + private boolean s3Renditions() { + return com.dotcms.storage.AssetStorageFeature.isEnabled(); + } + + private File cachedRendition(final File localFile) throws DotDataException { + final File cached = APILocator.getBinaryAssetStorageAPI().getGeneratedFile(localFile); + if (cached == null || !cached.isFile() || cached.length() < MIN_VALID_FILE_LENGTH) { + return null; + } + return cached; + } + /** * Resolves the final cache file for a filter chain WITHOUT running any pixel work or checking * existence — it walks {@link ImageFilter#getResultsFile} through every filter exactly as the diff --git a/dotCMS/src/test/java/com/dotcms/rendering/velocity/services/StoredVelocityFileLoaderTest.java b/dotCMS/src/test/java/com/dotcms/rendering/velocity/services/StoredVelocityFileLoaderTest.java new file mode 100644 index 000000000000..bb50dd46d3c5 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/rendering/velocity/services/StoredVelocityFileLoaderTest.java @@ -0,0 +1,122 @@ +package com.dotcms.rendering.velocity.services; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.startsWith; +import static org.mockito.Mockito.*; + +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.binary.BinaryAssetStorageAPI; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.CacheLocator; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.portlets.containers.business.ContainerAPI; +import com.dotmarketing.portlets.languagesmanager.business.LanguageAPI; +import com.dotmarketing.portlets.languagesmanager.model.Language; +import com.dotmarketing.util.Config; +import com.dotmarketing.util.ConfigUtils; +import com.dotmarketing.util.Logger; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.velocity.exception.ResourceNotFoundException; +import org.apache.velocity.exception.VelocityException; +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; + +/** + * Checks how Velocity file loading behaves with S3 asset storage on: which failures are storage + * outages, and that a missing file outside the asset root is an ordinary miss. + */ +class StoredVelocityFileLoaderTest { + + @TempDir Path root; + private String previousFlag; + + @BeforeEach + void enableFlag() { + previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + Config.setProperty(AssetStorageFeature.FLAG, true); + } + + @AfterEach + void restoreFlag() { + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + } + + @Test + void databaseLoaderFailureRemainsACachedMiss() throws Exception { + final ContainerAPI containers = mock(ContainerAPI.class); + when(containers.findContainer(anyString(), any(), anyBoolean(), anyBoolean())) + .thenThrow(new DotDataException("database unavailable")); + final DotResourceCache cache = mock(DotResourceCache.class); + try (var locator = mockStatic(APILocator.class); var caches = mockStatic(CacheLocator.class)) { + locator.when(APILocator::getContainerAPI).thenReturn(containers); + caches.when(CacheLocator::getVeloctyResourceCache).thenReturn(cache); + final String path = "/LIVE/abc123_1/uuid.container"; + + assertThrows(ResourceNotFoundException.class, () -> new DotResourceLoader().getResourceStream(path)); + verify(cache).addMiss(path); + } + } + + @Test + void storageFailureOfAStoredTemplateIsNotCachedAsMissing() throws Exception { + final Path dynamic = Files.createDirectories(root.resolve("dynamic")); + final File template = Files.writeString(dynamic.resolve("Page.vtl"), "#set($x = 1)").toFile(); + final BinaryAssetStorageAPI binaries = mock(BinaryAssetStorageAPI.class); + when(binaries.openLocalFile(any())).thenThrow(new DotDataException("S3 unavailable")); + final DotResourceCache cache = mock(DotResourceCache.class); + // Built before the static stubbing below: Mockito cannot stub a mock inside another stubbing. + final LanguageAPI languages = languages(); + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class); + var caches = mockStatic(CacheLocator.class)) { + paths.when(ConfigUtils::getDynamicContentPath).thenReturn(dynamic.toString()); + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(binaries); + locator.when(APILocator::getLanguageAPI).thenReturn(languages); + caches.when(CacheLocator::getVeloctyResourceCache).thenReturn(cache); + + final var thrown = assertThrows(VelocityException.class, + () -> new DotResourceLoader().getResourceStream(template.getAbsolutePath())); + assertFalse(thrown instanceof ResourceNotFoundException, "An outage is not a missing template"); + verify(cache, never()).addMiss(any()); + } + } + + @Test + void missingTemplateOutsideTheAssetRootIsAPlainMiss(@TempDir Path elsewhere) throws Exception { + // The asset root sits under the dynamic content path so the loader's allowed-path check + // accepts it without the servlet configuration a unit test does not have. + final Path assets = Files.createDirectories(root.resolve("assets")); + final BinaryAssetStorageAPI binaries = mock(BinaryAssetStorageAPI.class); + when(binaries.openLocalFile(any())).thenAnswer( + call -> new ByteArrayInputStream("restored".getBytes(StandardCharsets.UTF_8))); + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class); + var logger = mockStatic(Logger.class)) { + paths.when(ConfigUtils::getDynamicContentPath).thenReturn(root.toString()); + paths.when(ConfigUtils::getAssetPath).thenReturn(assets.toString()); + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(binaries); + final VTLLoader loader = VTLLoader.instance(); + + final String missing = elsewhere.resolve("Missing.vtl").toString(); + assertThrows(ResourceNotFoundException.class, () -> loader.streamFile(missing)); + logger.verify(() -> Logger.warn(any(Object.class), startsWith("POSSIBLE HACK ATTACK")), never()); + + // A missing file under the asset root still reaches the restore path. + final String evicted = assets.resolve("a/b/abc123/fileAsset/Evicted.vtl").toString(); + try (var input = loader.streamFile(evicted)) { + assertEquals("restored", new String(input.readAllBytes(), StandardCharsets.UTF_8)); + } + verify(binaries).openLocalFile(new File(evicted)); + } + } + + private static LanguageAPI languages() { + final LanguageAPI languages = mock(LanguageAPI.class); + when(languages.getDefaultLanguage()).thenReturn(new Language(1)); + return languages; + } +} diff --git a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java index aa404f883e0d..d013fa537503 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/AssetStorageFeatureTest.java @@ -101,6 +101,78 @@ void disabledBackfillDoesNotCallAnyProvider() throws Exception { assertFalse(filesystem().backfillFile(GROUP, "unused", root.resolve("missing-file").toFile())); } + @Test + void bothCropEnginesUsePinnedPointOnlyWhenFeatureIsEnabled() { + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class)) { + final var javaCrop = new com.dotmarketing.image.filter.CropImageFilter() { + java.util.Optional focal(Map parameters) { + return calcFocalPoint(new java.awt.image.BufferedImage(100, 100, + java.awt.image.BufferedImage.TYPE_INT_RGB), parameters); + } + }; + final var nativeCrop = new com.dotmarketing.image.vips.VipsCropImageFilter() { + java.util.Optional focal(Map parameters) { + return calcFocalPoint(new java.awt.Dimension(100, 100), parameters); + } + }; + final Map parameters = new java.util.HashMap<>(); + parameters.put("fp", new String[]{"0.25,0.5"}); + parameters.put(com.dotmarketing.image.filter.ImageFilter.RESOLVED_CROP_FOCAL_POINT, + new String[]{"0.75,0.5"}); + assertEquals(new java.awt.Point(75, 50), javaCrop.focal(parameters).orElseThrow()); + assertEquals(javaCrop.focal(parameters), nativeCrop.focal(parameters)); + parameters.put(com.dotmarketing.image.filter.ImageFilter.RESOLVED_CROP_FOCAL_POINT, new String[]{""}); + assertTrue(javaCrop.focal(parameters).isEmpty()); + assertTrue(nativeCrop.focal(parameters).isEmpty()); + Config.setProperty(AssetStorageFeature.FLAG, false); + assertEquals(new java.awt.Point(25, 50), javaCrop.focal(parameters).orElseThrow()); + assertEquals(javaCrop.focal(parameters), nativeCrop.focal(parameters)); + } + } + + @Test + void metadataReadErrorsPropagateOnlyWhenEnabledIncludingLegacyFocalLookup() throws Exception { + final var storage = mock(StoragePersistenceAPI.class); + doThrow(new DotDataException("metadata unavailable")).when(storage).pullObject(anyString(), anyString(), any()); + final var provider = mock(StoragePersistenceProvider.class); + when(provider.getStorage(any())).thenReturn(storage); + final var metadataCache = mock(com.dotmarketing.portlets.contentlet.business.MetadataCache.class); + when(metadataCache.getMetadataMap(anyString())).thenReturn(null); + final var metadata = new FileStorageAPIImpl(new JsonReaderDelegate<>(Map.class), new JsonWriterDelegate(), + mock(MetadataGenerator.class), provider, metadataCache); + final var request = new FetchMetadataParams.Builder().cache(false).storageKey(new StorageKey.Builder() + .group("metadata").path("/asset-metadata.json").storage(StorageType.DEFAULT_CHAIN).build()).build(); + assertThrows(DotDataException.class, () -> metadata.retrieveMetaData(request)); + Config.setProperty(AssetStorageFeature.FLAG, false); + assertNull(metadata.retrieveMetaData(request), "Disabled reads retain legacy failure handling"); + + final var content = new com.dotmarketing.portlets.contentlet.model.Contentlet(); + content.setInode("abc123"); + content.getMap().put("HeroImage", root.resolve("Photo.PNG").toFile()); + final var contentAPI = mock(com.dotmarketing.portlets.contentlet.business.ContentletAPI.class); + when(contentAPI.find(eq("abc123"), any(), eq(false))).thenReturn(content); + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class); + var caches = mockStatic(com.dotmarketing.business.CacheLocator.class); + var paths = mockStatic(ConfigUtils.class)) { + paths.when(ConfigUtils::getAssetPath).thenReturn(root.toString()); + locator.when(com.dotmarketing.business.APILocator::getFileStorageAPI).thenReturn(metadata); + locator.when(com.dotmarketing.business.APILocator::getContentletAPI).thenReturn(contentAPI); + final var tempFiles = mock(com.dotcms.rest.api.v1.temp.TempFileAPI.class); + locator.when(com.dotmarketing.business.APILocator::getTempFileAPI).thenReturn(tempFiles); + caches.when(com.dotmarketing.business.CacheLocator::getMetadataCache).thenReturn(metadataCache); + final var contentMetadata = new FileMetadataAPIImpl(); + locator.when(com.dotmarketing.business.APILocator::getFileMetadataAPI).thenReturn(contentMetadata); + final var focal = new com.dotmarketing.image.focalpoint.FocalPointAPIImpl(); + assertTrue(focal.readFocalPoint("abc123", "HeroImage").isEmpty()); + Config.setProperty(AssetStorageFeature.FLAG, true); + assertThrows(com.dotmarketing.exception.DotRuntimeException.class, + () -> focal.readFocalPoint("abc123", "HeroImage")); + verify(storage, times(4)).pullObject(anyString(), anyString(), any()); + doReturn(null).when(storage).pullObject(anyString(), anyString(), any()); + assertTrue(focal.readFocalPoint("abc123", "HeroImage").isEmpty(), "Missing metadata is not an outage"); + } + } + @Test void databaseQueryFailuresAreNotReportedAsMissingObjectsWhenEnabled() throws Exception { final var connection = mock(java.sql.Connection.class); @@ -250,6 +322,25 @@ void disabledContentletReadUsesLegacyFilesystemAndPreservesStringTypeGuard() thr } } + @Test + void disabledExporterKeepsLegacyFilterParametersWithoutStorageCalls() throws Exception { + Config.setProperty(AssetStorageFeature.FLAG, false); + File original = Files.writeString(root.resolve("Original.png"), "x".repeat(60)).toFile(); + Map parameters = new java.util.HashMap<>(); + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class); + var engine = mockStatic(com.dotmarketing.image.ImageEngine.class)) { + var filters = mock(com.dotmarketing.image.filter.ImageFilterAPI.class); + when(filters.resolveFilters(any())).thenReturn(Map.of()); + engine.when(com.dotmarketing.image.ImageEngine::resolve).thenReturn(filters); + var result = new com.dotmarketing.portlets.contentlet.business.exporter.ImageFilterExporter() + .exportContent(original, parameters); + assertEquals(original, result.getDataFile()); + assertArrayEquals(new String[0], parameters.get("filter")); + assertArrayEquals(new String[0], parameters.get("filters")); + locator.verify(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI, never()); + } + } + @Test void enabledFileWrapperResolvesAgainAfterEviction() throws Exception { Path path = root.resolve("Theme.CSS"); @@ -553,4 +644,34 @@ void metadataKeysAreStableAcrossConcurrentCalls() throws Exception { for (var future : futures) future.get(20, TimeUnit.SECONDS); } } + + @Test + void warmRenditionSurvivesRemoteOutageWithoutRemoteVerification() throws Exception { + try (var paths = mockStatic(ConfigUtils.class); + var locator = mockStatic(com.dotmarketing.business.APILocator.class); + var engine = mockStatic(com.dotmarketing.image.ImageEngine.class)) { + paths.when(ConfigUtils::getDotGeneratedPath).thenReturn(root.resolve("generated").toString()); + File original = root.resolve("Original.png").toFile(); + javax.imageio.ImageIO.write(new java.awt.image.BufferedImage(80, 60, 1), "PNG", original); + Map params = new java.util.HashMap<>(); + params.put("resize_w", new String[]{"32"}); + params.put("fieldVarName", new String[]{"HeroImage"}); + params.put("assetInodeOrIdentifier", new String[]{"abc123"}); + File rendition = new com.dotmarketing.image.filter.ResizeImageFilter().getResultsFile(original, params); + Files.createDirectories(rendition.toPath().getParent()); + javax.imageio.ImageIO.write(new java.awt.image.BufferedImage(32, 24, 1), "PNG", rendition); + var storage = mock(StoragePersistenceAPI.class); + when(storage.pullFile(anyString(), anyString())).thenReturn(rendition); + when(storage.hasDurableCopy(anyString(), anyString(), any())).thenThrow(new DotDataException("S3 unavailable")); + locator.when(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI) + .thenReturn(new BinaryAssetStorageAPIImpl(storage)); + var filters = mock(com.dotmarketing.image.filter.ImageFilterAPI.class); + when(filters.resolveFilters(any())).thenReturn(Map.of("resize", com.dotmarketing.image.filter.ResizeImageFilter.class)); + engine.when(com.dotmarketing.image.ImageEngine::resolve).thenReturn(filters); + var result = new com.dotmarketing.portlets.contentlet.business.exporter.ImageFilterExporter().exportContent(original, params); + assertEquals(rendition, result.getDataFile()); + verify(storage, never()).hasDurableCopy(anyString(), anyString(), any()); + verify(storage, never()).pushFile(anyString(), anyString(), any(), any()); + } + } } diff --git a/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java b/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java index 9db9ba7209ac..9aa3d922cd2a 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/BinaryS3StorageTest.java @@ -897,6 +897,129 @@ public File runFilter(final File file, final Map parameters) { } } + @org.junit.jupiter.params.ParameterizedTest + @org.junit.jupiter.params.provider.ValueSource(strings = {"PNG", "gif"}) + void generatedImageRoundTripsWithoutRegenerationAndInvalidatesRemoteOnlyCopies(final String extension) throws Exception { + final String previousMode = Config.getStringProperty("BINARY_ASSET_STORAGE_TYPE", null); + Config.setProperty("BINARY_ASSET_STORAGE_TYPE", "BINARY_CHAIN"); + final File source = root.resolve("Upload." + extension).toFile(); + final var image = new java.awt.image.BufferedImage(80, 60, java.awt.image.BufferedImage.TYPE_INT_RGB); + javax.imageio.ImageIO.write(image, extension, source); + api.storeBinary("abc123", "HeroImage", "Photo." + extension, source); + final var imageAPI = mock(com.dotmarketing.image.filter.ImageFilterAPI.class); + when(imageAPI.resolveFilters(any())).thenReturn(Map.of("resize", CountingResizeImageFilter.class)); + CountingResizeImageFilter.generations = 0; + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class); + var engine = mockStatic(com.dotmarketing.image.ImageEngine.class)) { + locator.when(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI).thenReturn(api); + engine.when(com.dotmarketing.image.ImageEngine::resolve).thenReturn(imageAPI); + final var exporter = new com.dotmarketing.portlets.contentlet.business.exporter.ImageFilterExporter(); + final Map params = new java.util.HashMap<>(); + params.put("resize_w", new String[]{"32"}); + params.put("fieldVarName", new String[]{"HeroImage"}); + params.put("assetInodeOrIdentifier", new String[]{"abc123"}); + File original = api.getBinaryFile("abc123", "HeroImage", "Photo." + extension); + final File rendition = exporter.exportContent(original, params).getDataFile(); + assertEquals(32, javax.imageio.ImageIO.read(rendition).getWidth()); + assertEquals(1, CountingResizeImageFilter.generations); + final String key = BinaryAssetStorageAPI.GENERATED_ASSETS_GROUP + "/" + + root.resolve("dotGenerated").toRealPath().relativize(rendition.toPath()); + assertTrue(client.doesObjectExist(bucket, key)); + final byte[] expected = Files.readAllBytes(rendition.toPath()); + assertTrue(api.evictLocalFile(original)); + assertTrue(api.evictLocalFile(rendition)); + original = api.getBinaryFile("abc123", "HeroImage", "Photo." + extension); + final File restored = exporter.exportContent(original, params).getDataFile(); + assertArrayEquals(expected, Files.readAllBytes(restored.toPath())); + assertEquals(1, CountingResizeImageFilter.generations, "Cold rendition comes from S3, not pixel work"); + + // Warm reads must not resurrect an object invalidated by another node. + client.deleteObject(bucket, key); + exporter.exportContent(original, params); + assertFalse(client.doesObjectExist(bucket, key)); + assertEquals(1, CountingResizeImageFilter.generations); + assertFalse(api.evictLocalFile(restored), "No durable copy: retain the local rendition"); + api.deleteGeneratedFiles("abc123"); + assertFalse(client.doesObjectExist(bucket, key)); + assertNull(api.getGeneratedFile(restored), "Invalidated renditions must not return from S3"); + exporter.exportContent(original, params); + assertEquals(2, CountingResizeImageFilter.generations); + api.deleteBinary("abc123", "HeroImage"); + assertFalse(client.doesObjectExist(bucket, key), "Deleting the source also invalidates S3 renditions"); + } finally { + Config.setProperty("BINARY_ASSET_STORAGE_TYPE", previousMode); + } + } + + @Test + void chainedCropPinsSnapshotFocalPointAndRestoresChangedCropFromS3() throws Exception { + final File source = root.resolve("MixedCase.PNG").toFile(); + final var pixels = new java.awt.image.BufferedImage(80, 60, java.awt.image.BufferedImage.TYPE_INT_RGB); + for (int x = 0; x < 80; x++) { + for (int y = 0; y < 60; y++) { + pixels.setRGB(x, y, (x < 40 ? java.awt.Color.RED : java.awt.Color.BLUE).getRGB()); + } + } + javax.imageio.ImageIO.write(pixels, "PNG", source); + final File original = api.storeRevision("abc123", "HeroImage", source.getName(), source); + final var metadataAPI = mock(FileMetadataAPI.class); + final var point = new java.util.concurrent.atomic.AtomicReference("0.25,0.5"); + when(metadataAPI.getMetadata(any(), eq("HeroImage"))).thenAnswer(call -> { + final com.dotmarketing.portlets.contentlet.model.Contentlet snapshot = call.getArgument(0); + assertEquals("abc123", snapshot.getInode()); + assertEquals(original, snapshot.get("HeroImage"), "Use the requested revision, not the current DB version"); + final var metadata = mock(com.dotcms.storage.model.Metadata.class); + when(metadata.getCustomMeta()).thenReturn(point.get() == null ? Map.of() + : Map.of("focalPoint", point.get())); + return metadata; + }); + final var imageAPI = mock(com.dotmarketing.image.filter.ImageFilterAPI.class); + final Map> filters = new java.util.LinkedHashMap<>(); + filters.put("resize", CountingResizeImageFilter.class); + filters.put("crop", com.dotmarketing.image.filter.CropImageFilter.class); + when(imageAPI.resolveFilters(any())).thenReturn(filters); + try (var locator = mockStatic(com.dotmarketing.business.APILocator.class); + var engine = mockStatic(com.dotmarketing.image.ImageEngine.class)) { + locator.when(com.dotmarketing.business.APILocator::getBinaryAssetStorageAPI).thenReturn(api); + locator.when(com.dotmarketing.business.APILocator::getFileMetadataAPI).thenReturn(metadataAPI); + engine.when(com.dotmarketing.image.ImageEngine::resolve).thenReturn(imageAPI); + final var exporter = new com.dotmarketing.portlets.contentlet.business.exporter.ImageFilterExporter(); + final Map params = new java.util.LinkedHashMap<>(); + params.put("resize_w", new String[]{"40"}); + params.put("crop_w", new String[]{"10"}); + params.put("crop_h", new String[]{"10"}); + params.put("fieldVarName", new String[]{"HeroImage"}); + params.put("assetInodeOrIdentifier", new String[]{"abc123"}); + final File red = exporter.exportContent(original, params).getDataFile(); + assertEquals(java.awt.Color.RED.getRGB(), javax.imageio.ImageIO.read(red).getRGB(5, 5)); + assertFalse(params.containsKey("fp")); + assertFalse(params.containsKey(com.dotmarketing.image.filter.ImageFilter.RESOLVED_CROP_FOCAL_POINT)); + verify(metadataAPI, times(1)).getMetadata(any(), eq("HeroImage")); + + point.set("0.75,0.5"); + final File blue = exporter.exportContent(original, params).getDataFile(); + assertNotEquals(red, blue, "A changed focal point must select a different crop after resize"); + assertEquals(java.awt.Color.BLUE.getRGB(), javax.imageio.ImageIO.read(blue).getRGB(5, 5)); + final byte[] expected = Files.readAllBytes(blue.toPath()); + assertTrue(api.evictLocalFile(blue)); + assertArrayEquals(expected, Files.readAllBytes(exporter.exportContent(original, params).getDataFile().toPath())); + verify(metadataAPI, times(3)).getMetadata(any(), eq("HeroImage")); + + params.put("fp", new String[]{"0.25,0.5"}); + assertEquals(red, exporter.exportContent(original, params).getDataFile(), "Explicit fp overrides stored metadata"); + verify(metadataAPI, times(3)).getMetadata(any(), eq("HeroImage")); + params.remove("fp"); + point.set(null); + final File noPoint = exporter.exportContent(original, params).getDataFile(); + assertEquals(java.awt.Color.RED.getRGB(), javax.imageio.ImageIO.read(noPoint).getRGB(5, 5)); + verify(metadataAPI, times(4)).getMetadata(any(), eq("HeroImage")); + when(metadataAPI.getMetadata(any(), eq("HeroImage"))) + .thenThrow(new com.dotmarketing.exception.DotDataException("metadata unavailable")); + assertThrows(com.dotmarketing.portlets.contentlet.business.BinaryContentExporterException.class, + () -> exporter.exportContent(original, params), "Do not cache a crop with an unknown focal point"); + } + } + @Test void generatedEvictionRetainsUnbackedFilesAndRejectsOutsidePaths() throws Exception { final Path rendition = root.resolve("dotGenerated/a/b/abc123/dotGenerated_resize_0123456789abcdef.PNG"); 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 1841591db04e..e3a568c148dc 100644 --- a/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetReferenceTest.java +++ b/dotCMS/src/test/java/com/dotcms/storage/binary/BinaryAssetReferenceTest.java @@ -116,6 +116,37 @@ class BinaryAssetReferenceTest { } } + @Test void replacementRenditionsHaveDistinctKeysAndStayInTheAssetShard() throws Exception { + 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()); + paths.when(ConfigUtils::getDotGeneratedPath).thenReturn(root.resolve("generated").toString()); + File first = BinaryAssetReference.localFile("abc123", "HeroImage", "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"); + File second = BinaryAssetReference.localFile("abc123", "HeroImage", "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/Friday.PNG"); + Map params = new HashMap<>(); + params.put("resize_w", new String[]{"32"}); + params.put("fieldVarName", new String[]{"HeroImage"}); + params.put("assetInodeOrIdentifier", new String[]{"abc123"}); + var filter = new com.dotmarketing.image.filter.ResizeImageFilter(); + File oldRendition = filter.getResultsFile(first, params); + File newRendition = filter.getResultsFile(second, params); + assertNotEquals(oldRendition, newRendition); + assertEquals(root.resolve("generated/a/b/abc123").toFile().getCanonicalFile(), newRendition.getParentFile()); + assertEquals(newRendition, filter.getResultsFile(second, params)); + assertEquals(newRendition.getParentFile(), filter.getResultsFile(newRendition, params).getParentFile()); + File uploadedName = BinaryAssetReference.localFile("abc123", "HeroImage", + "a/b/abc123/HeroImage/.revisions/" + UUID.randomUUID() + "/dotGenerated_upload.PNG"); + assertEquals(newRendition.getParentFile(), filter.getResultsFile(uploadedName, params).getParentFile(), + "An uploaded filename alone must not be mistaken for a generated cache file"); + Config.setProperty(AssetStorageFeature.FLAG, false); + assertEquals(root.resolve("generated/a/b").toFile().getCanonicalFile(), + filter.getResultsFile(root.resolve("Original.PNG").toFile(), params).getParentFile()); + } 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)) { diff --git a/dotCMS/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPIImplTest.java b/dotCMS/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPIImplTest.java new file mode 100644 index 000000000000..770267fd9ae5 --- /dev/null +++ b/dotCMS/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPIImplTest.java @@ -0,0 +1,68 @@ +package com.dotmarketing.image.focalpoint; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import com.dotcms.rest.api.v1.temp.TempFileAPI; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.FileMetadataAPI; +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.util.Config; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Checks how a focal-point read treats a failed contentlet lookup. A visitor who cannot read the + * content, or content that does not exist, has no focal point; only a failed lookup with S3 asset + * storage on is an error. + */ +class FocalPointAPIImplTest { + + private static final String INODE = "abc123"; + private String previousFlag; + private final ContentletAPI contentlets = mock(ContentletAPI.class); + private final FocalPointAPIImpl focalPoints = new FocalPointAPIImpl(mock(FileMetadataAPI.class), + mock(TempFileAPI.class), contentlets, () -> null); + + @BeforeEach + void rememberFlag() { + previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + } + + @AfterEach + void restoreFlag() { + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + } + + @Test + void anonymousVisitorWithoutBackendReadAccessGetsNoFocalPoint() throws Exception { + Config.setProperty(AssetStorageFeature.FLAG, true); + when(contentlets.find(eq(INODE), any(), eq(false))).thenThrow(new DotSecurityException("not readable")); + assertTrue(focalPoints.readFocalPoint(INODE, "HeroImage").isEmpty()); + } + + @Test + void missingContentGetsNoFocalPoint() throws Exception { + Config.setProperty(AssetStorageFeature.FLAG, true); + when(contentlets.find(eq(INODE), any(), eq(false))).thenReturn(null); + assertTrue(focalPoints.readFocalPoint(INODE, "HeroImage").isEmpty()); + } + + @Test + void failedLookupIsAnErrorOnlyWhenEnabled() throws Exception { + final DotDataException failure = new DotDataException("database unavailable"); + when(contentlets.find(eq(INODE), any(), eq(false))).thenThrow(failure); + + Config.setProperty(AssetStorageFeature.FLAG, true); + final var thrown = assertThrows(DotRuntimeException.class, + () -> focalPoints.readFocalPoint(INODE, "HeroImage")); + assertSame(failure, thrown.getCause()); + + Config.setProperty(AssetStorageFeature.FLAG, false); + assertTrue(focalPoints.readFocalPoint(INODE, "HeroImage").isEmpty(), "Flag-off reads keep the legacy result"); + } +} diff --git a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporterFocalPointTest.java b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporterFocalPointTest.java new file mode 100644 index 000000000000..279731a5cf0b --- /dev/null +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporterFocalPointTest.java @@ -0,0 +1,94 @@ +package com.dotmarketing.portlets.contentlet.business.exporter; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import com.dotcms.storage.FileMetadataAPI; +import com.dotcms.storage.binary.BinaryAssetReference; +import com.dotcms.storage.model.Metadata; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.image.filter.ImageFilter; +import com.dotmarketing.image.focalpoint.FocalPointAPI; +import com.dotmarketing.portlets.contentlet.business.ContentletAPI; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.util.ConfigUtils; +import java.io.File; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class ImageFilterExporterFocalPointTest { + @TempDir Path root; + + @Test + void legacyAndRevisionCropsKeepTheirSourceMetadataSnapshot() throws Exception { + final var metadata = mock(FileMetadataAPI.class); + final var currentContent = mock(ContentletAPI.class); + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class)) { + paths.when(ConfigUtils::getAssetPath).thenReturn(root.toString()); + locator.when(APILocator::getFileMetadataAPI).thenReturn(metadata); + locator.when(APILocator::getContentletAPI).thenReturn(currentContent); + final var exporter = new ImageFilterExporter(); + for (String tail : new String[]{"Photo.PNG", + ".revisions/11111111-1111-1111-1111-111111111111/Photo.PNG"}) { + final File original = root.resolve("a/b/abc123/HeroImage/" + tail).toFile(); + final String oldKey = BinaryAssetReference.newMetadataKey(original, "abc123", "HeroImage"); + final String newKey = BinaryAssetReference.newMetadataKey(original, "abc123", "HeroImage"); + final File earlier = BinaryAssetReference.withMetadata(original, "abc123", "HeroImage", oldKey); + final File later = BinaryAssetReference.withMetadata(original, "abc123", "HeroImage", newKey); + when(metadata.getMetadata(any(Contentlet.class), eq("HeroImage"))).thenAnswer(call -> { + Contentlet snapshot = call.getArgument(0); + assertEquals("abc123", snapshot.getInode()); + File source = (File) snapshot.getMap().get("HeroImage"); + String key = BinaryAssetReference.metadataKeyOf(source); + if (key == null) { + return null; // An earlier legacy snapshot also pins the absence of an edit. + } + assertTrue(key.equals(oldKey) || key.equals(newKey)); + return new Metadata("HeroImage", Map.of(Metadata.CUSTOM_PROP_PREFIX + FocalPointAPI.FOCAL_POINT, + key.equals(oldKey) ? "0.25,0.5" : "0.75,0.5")); + }); + // Files are deliberately absent locally: resolving metadata cannot depend on a warm cache. + for (File source : new File[]{later, earlier, original, earlier}) { + final var parameters = parameters(); + exporter.resolveCropFocalPoint(source, parameters); + final String expected = source == later ? "0.75,0.5" : source == earlier ? "0.25,0.5" : ""; + assertArrayEquals(new String[]{expected}, parameters.get(ImageFilter.RESOLVED_CROP_FOCAL_POINT)); + } + } + verifyNoInteractions(currentContent); + } + } + + @Test + void explicitPointOverridesMetadataAndStorageFailuresAreNotCachedAsAbsence() throws Exception { + final var metadata = mock(FileMetadataAPI.class); + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class)) { + paths.when(ConfigUtils::getAssetPath).thenReturn(root.toString()); + locator.when(APILocator::getFileMetadataAPI).thenReturn(metadata); + final var exporter = new ImageFilterExporter(); + final File source = root.resolve("a/b/abc123/HeroImage/Photo.PNG").toFile(); + final var parameters = parameters(); + parameters.put("fp", new String[]{"0.1,0.2"}); + exporter.resolveCropFocalPoint(source, parameters); + assertArrayEquals(new String[]{"0.1,0.2"}, parameters.get(ImageFilter.RESOLVED_CROP_FOCAL_POINT)); + verifyNoInteractions(metadata); + + when(metadata.getMetadata(any(Contentlet.class), eq("HeroImage"))) + .thenThrow(new DotDataException("metadata unavailable")); + final var failed = parameters(); + assertThrows(DotDataException.class, () -> exporter.resolveCropFocalPoint(source, failed)); + assertFalse(failed.containsKey(ImageFilter.RESOLVED_CROP_FOCAL_POINT)); + } + } + + private static Map parameters() { + final Map parameters = new HashMap<>(); + parameters.put("assetInodeOrIdentifier", new String[]{"abc123"}); + parameters.put("fieldVarName", new String[]{"HeroImage"}); + return parameters; + } +} diff --git a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporterStorageTest.java b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporterStorageTest.java new file mode 100644 index 000000000000..79ad67a8ede5 --- /dev/null +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/business/exporter/ImageFilterExporterStorageTest.java @@ -0,0 +1,203 @@ +package com.dotmarketing.portlets.contentlet.business.exporter; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.StoragePersistenceAPI; +import com.dotcms.storage.binary.BinaryAssetStorageAPIImpl; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotRuntimeException; +import com.dotmarketing.image.ImageEngine; +import com.dotmarketing.image.filter.ImageFilter; +import com.dotmarketing.image.filter.ImageFilterAPI; +import com.dotmarketing.image.filter.JpegImageFilter; +import com.dotmarketing.image.filter.ResizeImageFilter; +import com.dotmarketing.util.Config; +import com.dotmarketing.util.ConfigUtils; +import java.awt.image.BufferedImage; +import java.io.File; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import javax.imageio.ImageIO; +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; + +/** + * Checks how renditions use S3 asset storage: which paths are looked up, when the remote store is + * contacted, and what is served when it is unavailable. The storage mock models the real chain: + * a file already on local disk is returned without contacting S3, and anything else is a remote + * lookup that fails while {@link #outage} is set. + */ +class ImageFilterExporterStorageTest { + + @TempDir Path root; + private String previousFlag; + private final AtomicBoolean outage = new AtomicBoolean(); + + @BeforeEach + void enableFlag() { + previousFlag = Config.getStringProperty(AssetStorageFeature.FLAG, null); + Config.setProperty(AssetStorageFeature.FLAG, true); + } + + @AfterEach + void restoreFlag() { + Config.setProperty(AssetStorageFeature.FLAG, previousFlag); + } + + @Test + void jpegPredictionIsTheWrittenFileOnlyWhenEnabled() throws Exception { + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class)) { + paths.when(ConfigUtils::getDotGeneratedPath).thenReturn(generatedRoot().toString()); + final File original = image("Original.png", 80, 60); + final Map parameters = parameters("jpeg"); + + final File predicted = new JpegImageFilter().getResultsFile(original, parameters); + final File written = new JpegImageFilter().runFilter(original, parameters); + assertEquals(written, predicted, "The prediction must be the file the filter writes"); + assertTrue(written.getName().endsWith(".jpg")); + assertTrue(written.isFile()); + + // With the flag off the inherited prediction, and so every cache path, is unchanged. + Config.setProperty(AssetStorageFeature.FLAG, false); + final File legacy = new JpegImageFilter().getResultsFile(original, parameters); + assertEquals(generatedRoot().resolve("a/b").toFile().getCanonicalFile(), legacy.getParentFile()); + assertTrue(legacy.getName().startsWith("dotGenerated_jpeg_")); + assertTrue(legacy.getName().endsWith(".png")); + } + } + + @Test + void warmJpegRenditionIsServedDuringAnOutageWithoutRemoteCalls() throws Exception { + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class); + var engine = mockStatic(ImageEngine.class)) { + final StoragePersistenceAPI storage = storage(paths, locator); + chain(engine, "jpeg", JpegImageFilter.class); + final File original = image("Original.png", 80, 60); + final var exporter = new ImageFilterExporter(); + + final File cold = exporter.exportContent(original, parameters("jpeg")).getDataFile(); + assertTrue(cold.getName().endsWith(".jpg")); + outage.set(true); + final File warm = exporter.exportContent(original, parameters("jpeg")).getDataFile(); + + // The same file, whichever spelling of the temporary directory each path uses. + assertEquals(cold.getCanonicalFile(), warm.getCanonicalFile()); + verify(storage, times(1)).pushFile(anyString(), anyString(), any(), any()); + verify(storage, times(1)).hasDurableCopy(anyString(), anyString(), any()); + } + } + + @Test + void noOpFiltersStopContactingStorageAfterTheFirstRequest() throws Exception { + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class); + var engine = mockStatic(ImageEngine.class)) { + final StoragePersistenceAPI storage = storage(paths, locator); + // A maximum width larger than the image leaves the image unchanged. + final File small = image("Small.png", 80, 60); + chain(engine, "resize", ResizeImageFilter.class); + final var exporter = new ImageFilterExporter(); + assertEquals(small, exporter.exportContent(small, resize("resize")).getDataFile()); + outage.set(true); + assertEquals(small, exporter.exportContent(small, resize("resize")).getDataFile()); + + // The same no-op ahead of a real conversion: only the conversion's output is stored. + outage.set(false); + final File other = image("Other.png", 80, 60); + final var filters = new LinkedHashMap>(); + filters.put("resize", ResizeImageFilter.class); + filters.put("jpeg", JpegImageFilter.class); + chain(engine, filters); + final File converted = exporter.exportContent(other, resize("resize", "jpeg")).getDataFile(); + assertTrue(converted.getName().endsWith(".jpg")); + outage.set(true); + assertEquals(converted.getCanonicalFile(), + exporter.exportContent(other, resize("resize", "jpeg")).getDataFile().getCanonicalFile()); + + verify(storage, times(1)).pushFile(anyString(), anyString(), any(), any()); + } + } + + @Test + void failedRenditionUploadStillServesTheLocalOutput() throws Exception { + try (var paths = mockStatic(ConfigUtils.class); var locator = mockStatic(APILocator.class); + var engine = mockStatic(ImageEngine.class)) { + final StoragePersistenceAPI storage = storage(paths, locator); + when(storage.pushFile(anyString(), anyString(), any(), any())) + .thenThrow(new DotRuntimeException("S3 write unavailable")); + chain(engine, "resize", ResizeImageFilter.class); + final File original = image("Original.png", 80, 60); + final Map parameters = parameters("resize"); + parameters.put("resize_w", new String[]{"32"}); + + final File result = new ImageFilterExporter().exportContent(original, parameters).getDataFile(); + + assertNotEquals(original, result); + assertTrue(result.isFile(), "The produced rendition is served from local disk"); + verify(storage).pushFile(anyString(), anyString(), any(), any()); + } + } + + private Path generatedRoot() { + return root.resolve("generated"); + } + + private File image(final String name, final int width, final int height) throws Exception { + final File file = root.resolve(name).toFile(); + ImageIO.write(new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB), "PNG", file); + return file; + } + + private static Map parameters(final String... filters) { + final Map parameters = new HashMap<>(); + parameters.put("filter", filters); + parameters.put("fieldVarName", new String[]{"HeroImage"}); + parameters.put("assetInodeOrIdentifier", new String[]{"abc123"}); + return parameters; + } + + private static Map resize(final String... filters) { + final Map parameters = parameters(filters); + parameters.put("resize_maxw", new String[]{"200"}); + return parameters; + } + + private StoragePersistenceAPI storage(final org.mockito.MockedStatic paths, + final org.mockito.MockedStatic locator) throws DotDataException { + paths.when(ConfigUtils::getDotGeneratedPath).thenReturn(generatedRoot().toString()); + final StoragePersistenceAPI storage = mock(StoragePersistenceAPI.class); + when(storage.pullFile(anyString(), anyString())).thenAnswer(call -> { + final File local = generatedRoot().resolve((String) call.getArgument(1)).toFile(); + if (local.isFile()) { + return local; + } + if (outage.get()) { + throw new DotDataException("S3 unavailable"); + } + return null; + }); + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(new BinaryAssetStorageAPIImpl(storage)); + return storage; + } + + private static void chain(final org.mockito.MockedStatic engine, final String name, + final Class filter) { + final var filters = new LinkedHashMap>(); + filters.put(name, filter); + chain(engine, filters); + } + + private static void chain(final org.mockito.MockedStatic engine, + final Map> filters) { + final ImageFilterAPI api = mock(ImageFilterAPI.class); + when(api.resolveFilters(any())).thenReturn(filters); + engine.when(ImageEngine::resolve).thenReturn(api); + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java index a729ef81664a..f3218af065cc 100644 --- a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java +++ b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java @@ -81,6 +81,8 @@ com.dotcms.storage.binary.ContentletBackupStorageTest.class, com.dotcms.storage.binary.SharedAssetStorageIntegrationTest.class, com.dotcms.storage.binary.BinaryAssetStarterRestoreTest.class, + com.dotcms.rendering.velocity.services.AssetTemplateStorageTest.class, + com.dotcms.csspreproc.CSSAssetStorageTest.class, com.dotcms.publishing.PublishingArchiveStorageTest.class }) public class Junit5Suite1 { diff --git a/dotcms-integration/src/test/java/com/dotcms/csspreproc/CSSAssetStorageTest.java b/dotcms-integration/src/test/java/com/dotcms/csspreproc/CSSAssetStorageTest.java new file mode 100644 index 000000000000..92d8fc36e05a --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/csspreproc/CSSAssetStorageTest.java @@ -0,0 +1,160 @@ +package com.dotcms.csspreproc; + +import com.dotcms.csspreproc.dartsass.DartSassCompiler; +import com.dotcms.datagen.FileAssetDataGen; +import com.dotcms.datagen.FolderDataGen; +import com.dotcms.ema.proxy.MockHttpCaptureResponse; +import com.dotcms.ema.proxy.MockPrintWriter; +import com.dotcms.mock.request.DotCMSMockRequestWithSession; +import com.dotcms.mock.response.MockHttpResponse; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.storage.binary.BinaryAssetStorageAPI; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.beans.Host; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.CacheLocator; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; +import com.dotmarketing.util.ConfigUtils; +import com.dotmarketing.util.WebKeys; +import java.io.File; +import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import javax.servlet.http.HttpSession; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +public class CSSAssetStorageTest { + @BeforeAll + static void initialize() throws Exception { + IntegrationTestInitService.getInstance().init(); + assertEquals(Boolean.getBoolean("s3.cms.enabled"), AssetStorageFeature.isEnabled()); + } + + @Test + void compiledImportsSurviveEvictionAndTrackWorkingAndLiveChanges(@TempDir Path temporary) throws Exception { + final Host site = APILocator.getHostAPI().findDefaultHost(APILocator.systemUser(), false); + final var folder = new FolderDataGen().site(site).nextPersisted(); + final var binaries = APILocator.getBinaryAssetStorageAPI(); + try { + final Path colorsUpload = Files.writeString(temporary.resolve("_Colors.scss"), "$tone: red;\n"); + final var colors = new FileAssetDataGen(folder, colorsUpload.toFile()).nextPersisted(); + final var source = new FileAssetDataGen(folder, Files.writeString(temporary.resolve("Theme.scss"), + "@import \"Colors\";\nbody { color: $tone; background: url(\"Photo.SVG\"); }\n").toFile()).nextPersisted(); + final var photo = new FileAssetDataGen(folder, Files.writeString(temporary.resolve("Photo.SVG"), + "").toFile()).nextPersisted(); + for (var content : List.of(colors, source, photo)) { + APILocator.getContentletAPI().publish(content, APILocator.systemUser(), false); + } + final var asset = APILocator.getFileAssetAPI().fromContentlet(source); + final var request = request(site, asset.getURI()); + final Path generated = Path.of(ConfigUtils.getDotGeneratedPath(), source.getInode().substring(0, 1), + source.getInode().substring(1, 2), source.getInode()); + + if (AssetStorageFeature.isEnabled()) { + final var failing = mock(BinaryAssetStorageAPI.class, org.mockito.AdditionalAnswers.delegatesTo(binaries)); + doThrow(new DotDataException("Injected CSS publication outage")).when(failing).storeGeneratedFile(any()); + try (var locator = mockStatic(APILocator.class, CALLS_REAL_METHODS)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(failing); + final var outage = new DotLibSassCompiler(site, asset.getURI(), false, request); + outage.compile(); + assertCSS(outage.getOutput(), "red"); + verify(failing).storeGeneratedFile(any()); + } + // A failed upload keeps the compiled CSS locally, and eviction must not remove it. + assertEquals(1, cssFiles(generated).size(), "Compiled CSS is kept locally after a failed upload"); + final File localOnly = cssFiles(generated).get(0).toFile(); + assertFalse(binaries.evictLocalFile(localOnly), "CSS without a durable copy must not be evicted"); + // Start the round trip below from a cold state, so its first compile uploads normally. + Files.delete(localOnly.toPath()); + } + + final var first = new DotLibSassCompiler(site, asset.getURI(), false, request); + first.compile(); + final byte[] red = first.getOutput(); + assertCSS(red, "red"); + assertTrue(new String(red, StandardCharsets.UTF_8).contains("Photo.SVG")); + + if (AssetStorageFeature.isEnabled()) { + assertEquals(1, cssFiles(generated).size()); + final File css = cssFiles(generated).get(0).toFile(); + assertTrue(binaries.evictLocalFile(css)); + for (var content : List.of(colors, source, photo)) { + assertTrue(binaries.evictLocalFile(content.getBinary(FileAssetAPI.BINARY_FIELD))); + } + CacheLocator.getCSSCache().clearCache(); + // Native compilation is forbidden here: the real storage chain must restore the completed CSS. + try (var nativeCompiler = mockConstruction(DartSassCompiler.class)) { + final var cold = new DotLibSassCompiler(site, asset.getURI(), false, request); + cold.compile(); + assertArrayEquals(red, cold.getOutput()); + final var response = new MockHttpCaptureResponse(new MockHttpResponse()) { + @Override public PrintWriter getWriter() { return new MockPrintWriter(getOutputStream()); } + }; + new CSSPreProcessServlet().doGet(request, response); + assertCSS(response.getBytes(), "red"); + assertTrue(nativeCompiler.constructed().isEmpty(), "Cold CSS should be served without native recompilation"); + } + assertArrayEquals(red, Files.readAllBytes(css.toPath())); + } else { + assertTrue(cssFiles(generated).isEmpty(), "Disabled mode must not create S3 CSS artifacts"); + } + + final var edit = APILocator.getContentletAPI().checkout(colors.getInode(), APILocator.systemUser(), false); + final Path replacement = Files.createDirectories(temporary.resolve("edit")).resolve("_Colors.scss"); + edit.setBinary(FileAssetAPI.BINARY_FIELD, Files.writeString(replacement, "$tone: blue;\n").toFile()); + APILocator.getContentletAPI().checkin(edit, APILocator.systemUser(), false); + assertEquals("$tone: red;\n", Files.readString(colors.getBinary(FileAssetAPI.BINARY_FIELD).toPath())); + final var working = new DotLibSassCompiler(site, asset.getURI(), false, request); + working.compile(); + assertCSS(working.getOutput(), "blue"); + final var live = new DotLibSassCompiler(site, asset.getURI(), true, request); + live.compile(); + assertCSS(live.getOutput(), "red"); + if (AssetStorageFeature.isEnabled()) { + final var outputs = cssFiles(generated); + assertEquals(3, outputs.size()); + for (Path output : outputs) { + assertTrue(binaries.evictLocalFile(output.toFile())); + } + binaries.deleteAllBinaries(source.getInode()); + for (Path output : outputs) { + assertNull(binaries.getGeneratedFile(output.toFile()), "Inode deletion must remove cold CSS variants"); + } + } + } finally { + CacheLocator.getCSSCache().clearCache(); + FolderDataGen.remove(folder); + } + } + + private static void assertCSS(byte[] data, String color) { + final String actual = new String(data, StandardCharsets.UTF_8); + assertTrue(CssComparator.areSemanticallyEqual(actual, + "body { color: " + color + "; background: url(\"Photo.SVG\"); }"), actual); + } + + private static List cssFiles(Path folder) throws Exception { + if (!Files.isDirectory(folder)) return List.of(); + try (var files = Files.list(folder)) { + return files.filter(path -> path.getFileName().toString().startsWith("dotGenerated_css_")).toList(); + } + } + + private static DotCMSMockRequestWithSession request(Host site, String uri) throws Exception { + final HttpSession session = mock(HttpSession.class); + when(session.getAttribute(WebKeys.CMS_SELECTED_HOST_ID)).thenReturn(site.getIdentifier()); + final var request = new DotCMSMockRequestWithSession(session, false); + request.setRemoteHost(site.getIdentifier()); + request.setAttribute(com.liferay.portal.util.WebKeys.USER, APILocator.getUserAPI().loadUserById("dotcms.org.1")); + request.setRequestURI(uri); + return request; + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/rendering/velocity/services/AssetTemplateStorageTest.java b/dotcms-integration/src/test/java/com/dotcms/rendering/velocity/services/AssetTemplateStorageTest.java new file mode 100644 index 000000000000..87e446acb4f2 --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rendering/velocity/services/AssetTemplateStorageTest.java @@ -0,0 +1,116 @@ +package com.dotcms.rendering.velocity.services; + +import com.dotcms.datagen.FileAssetDataGen; +import com.dotcms.datagen.FolderDataGen; +import com.dotcms.storage.AssetStorageFeature; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.velocity.exception.ResourceNotFoundException; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.*; + +public class AssetTemplateStorageTest { + @BeforeAll + static void initialize() throws Exception { + IntegrationTestInitService.getInstance().init(); + assertEquals(Boolean.getBoolean("s3.cms.enabled"), AssetStorageFeature.isEnabled()); + } + + @Test + void markdownReadsColdFileThroughTheStreamAPI(@TempDir Path temporary) throws Throwable { + final var site = APILocator.getHostAPI().findDefaultHost(APILocator.systemUser(), false); + final var folder = new FolderDataGen().site(site).nextPersisted(); + try { + final var content = new FileAssetDataGen(folder, + Files.writeString(temporary.resolve("MixedCase.md"), "# Cold Markdown\n\nStored in S3.\n").toFile()).nextPersisted(); + APILocator.getContentletAPI().publish(content, APILocator.systemUser(), false); + final var file = content.getBinary(FileAssetAPI.BINARY_FIELD); + if (AssetStorageFeature.isEnabled()) { + assertTrue(APILocator.getBinaryAssetStorageAPI().evictLocalFile(file)); + } + final var session = org.mockito.Mockito.mock(javax.servlet.http.HttpSession.class); + final var request = new com.dotcms.mock.request.DotCMSMockRequestWithSession(session, false); + request.setAttribute(com.liferay.portal.util.WebKeys.USER, APILocator.getUserAPI().loadUserById("dotcms.org.1")); + final var context = org.mockito.Mockito.mock(org.apache.velocity.tools.view.context.ViewContext.class); + org.mockito.Mockito.when(context.getRequest()).thenReturn(request); + final var markdown = new com.dotcms.rendering.velocity.viewtools.MarkdownTool(); + markdown.init(context); + final String rendered = markdown.parseFile("//" + site.getHostname() + + APILocator.getFileAssetAPI().fromContentlet(content).getURI()); + assertTrue(rendered.contains("

Cold Markdown

")); + assertTrue(rendered.contains("Stored in S3.")); + } finally { + FolderDataGen.remove(folder); + } + } + + @Test + void physicalTemplatePathsRemainReadableAfterCacheEviction(@TempDir Path temporary) throws Exception { + final var folder = new FolderDataGen().nextPersisted(); + try { + final var upload = Files.writeString(temporary.resolve("Theme-MixedCase.CSS"), "body { color: purple; }"); + final var content = new FileAssetDataGen(folder, upload.toFile()).nextPersisted(); + final var file = content.getBinary(FileAssetAPI.BINARY_FIELD); + final var binaries = APILocator.getBinaryAssetStorageAPI(); + if (AssetStorageFeature.isEnabled()) { + assertTrue(binaries.evictLocalFile(file)); + IncludeLoader.instance(); + final var unavailable = org.mockito.Mockito.mock(com.dotcms.storage.binary.BinaryAssetStorageAPI.class); + final var failure = new com.dotmarketing.exception.DotDataException("S3 unavailable"); + org.mockito.Mockito.when(unavailable.openLocalFile(org.mockito.ArgumentMatchers.any())).thenThrow(failure); + try (var locator = org.mockito.Mockito.mockStatic(APILocator.class, org.mockito.Mockito.CALLS_REAL_METHODS)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(unavailable); + final var thrown = assertThrows(org.apache.velocity.exception.VelocityException.class, + () -> new DotResourceLoader().getResourceStream(file.getAbsolutePath())); + assertFalse(thrown instanceof ResourceNotFoundException, "An outage is not a missing template"); + assertSame(failure, thrown.getCause()); + } + } + try (var input = new DotResourceLoader().getResourceStream(file.getAbsolutePath())) { + assertEquals(Files.readString(upload), new String(input.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8)); + } + if (AssetStorageFeature.isEnabled()) { + assertTrue(binaries.evictLocalFile(file)); + } + try (var input = VTLLoader.instance().streamFile(file.getAbsolutePath())) { + assertEquals(Files.readString(upload), new String(input.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8)); + } + if (AssetStorageFeature.isEnabled()) { + binaries.deleteAllBinaries(content.getInode()); + assertThrows(java.io.IOException.class, () -> IncludeLoader.instance().streamFile(file.getAbsolutePath())); + assertThrows(java.io.IOException.class, () -> VTLLoader.instance().streamFile(file.getAbsolutePath())); + } + } finally { + FolderDataGen.remove(folder); + } + } + + @Test + void enabledLoadersRejectSiblingDirectoriesBeforeStorageAccess() throws Exception { + org.junit.jupiter.api.Assumptions.assumeTrue(AssetStorageFeature.isEnabled()); + // An allowed directory's name as a substring must not make another directory readable. + final Path assets = Path.of(com.dotmarketing.util.ConfigUtils.getAssetPath()).toAbsolutePath(); + final Path sibling = assets.resolveSibling(assets.getFileName() + "-outside-" + java.util.UUID.randomUUID()); + final Path source = sibling.resolve("Forbidden.CSS"); + Files.createDirectories(sibling); + Files.writeString(source, "must not be read"); + final var include = IncludeLoader.instance(); + final var vtl = VTLLoader.instance(); + final var storage = org.mockito.Mockito.mock(com.dotcms.storage.binary.BinaryAssetStorageAPI.class); + try (var locator = org.mockito.Mockito.mockStatic(APILocator.class)) { + locator.when(APILocator::getBinaryAssetStorageAPI).thenReturn(storage); + assertThrows(ResourceNotFoundException.class, () -> include.streamFile(source.toString())); + assertThrows(ResourceNotFoundException.class, () -> vtl.streamFile(source.toString())); + org.mockito.Mockito.verifyNoInteractions(storage); + } finally { + Files.delete(source); + Files.delete(sibling); + } + } +} 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 index 8957a8c0f404..11a24c7c184e 100644 --- a/dotcms-integration/src/test/java/com/dotcms/storage/binary/BinaryAssetStorageIntegrationTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/storage/binary/BinaryAssetStorageIntegrationTest.java @@ -773,6 +773,95 @@ private com.dotcms.storage.FetchMetadataParams metadataRequest(final String path .path(path).storage(com.dotcms.storage.StorageType.DEFAULT_CHAIN).build()).build(); } + @org.junit.jupiter.params.ParameterizedTest + @org.junit.jupiter.params.provider.ValueSource(booleans = {false, true}) + void s3CmsImageUsesItsOwnRenditionDirectoryAndRestoresAfterEviction(final boolean legacy, + @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 upload = uploads.resolve("dotGenerated_UserUpload.PNG").toFile(); + final java.awt.image.BufferedImage pixels = new java.awt.image.BufferedImage(64, 48, + java.awt.image.BufferedImage.TYPE_INT_RGB); + for (int x = 0; x < 64; x++) { + for (int y = 0; y < 48; y++) { + pixels.setRGB(x, y, (x < 32 ? java.awt.Color.RED : java.awt.Color.BLUE).getRGB()); + } + } + javax.imageio.ImageIO.write(pixels, "PNG", upload); + Contentlet asset = new FileAssetDataGen(folder, upload).nextPersisted(); + final String inode = asset.getInode(); + if (legacy) { + // Model a pre-feature binary without changing the runtime feature flag. + final String field = FileAssetAPI.BINARY_FIELD; + final File local = new BinaryAssetReference.StoredBinary(null, null, upload.getName()) + .localFile(inode, field); + Files.createDirectories(local.toPath().getParent()); + Files.copy(upload.toPath(), local.toPath()); + final var mapper = new com.fasterxml.jackson.databind.ObjectMapper(); + final var json = (com.fasterxml.jackson.databind.node.ObjectNode) mapper.readTree( + new com.dotmarketing.common.db.DotConnect() + .setSQL("select contentlet_as_json from contentlet where inode = ?") + .addParam(inode).getString("contentlet_as_json")); + final var binary = (com.fasterxml.jackson.databind.node.ObjectNode) json.path("fields").path(field); + binary.remove("storageKey"); + binary.remove("metadataStorageKey"); + new com.dotmarketing.common.db.DotConnect() + .setSQL("update contentlet set contentlet_as_json = ?::jsonb where inode = ?") + .addParam(mapper.writeValueAsString(json)).addParam(inode).loadResult(); + com.dotmarketing.business.CacheLocator.getContentletCache().remove(inode); + asset = contentletAPI.find(inode, user, false); + assertNull(BinaryAssetReference.find(inode, field)); + assertTrue(binaryAssetStorageAPI.backfillBinary(inode, field, local)); + } + final java.util.Map parameters = new java.util.HashMap<>(); + parameters.put("filter", new String[]{"resize"}); + parameters.put("resize_w", new String[]{"24"}); + parameters.put("fieldVarName", new String[]{FileAssetAPI.BINARY_FIELD}); + parameters.put("assetInodeOrIdentifier", new String[]{inode}); + final var exporter = new com.dotmarketing.portlets.contentlet.business.exporter.ImageFilterExporter(); + final File original = asset.getBinary(FileAssetAPI.BINARY_FIELD); + final File rendition = exporter.exportContent(original, parameters).getDataFile(); + assertEquals(24, javax.imageio.ImageIO.read(rendition).getWidth()); + assertEquals(java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getDotGeneratedPath(), + inode.substring(0, 1), inode.substring(1, 2), inode).toFile().getCanonicalFile(), + rendition.getParentFile()); + final byte[] expected = Files.readAllBytes(rendition.toPath()); + assertTrue(binaryAssetStorageAPI.evictLocalFile(original)); + assertTrue(binaryAssetStorageAPI.evictLocalFile(rendition)); + final File restored = exporter.exportContent(contentletAPI.find(inode, user, false) + .getBinary(FileAssetAPI.BINARY_FIELD), parameters).getDataFile(); + assertArrayEquals(expected, Files.readAllBytes(restored.toPath())); + parameters.put("filter", new String[]{"resize", "crop"}); + parameters.put("crop_w", new String[]{"6"}); + parameters.put("crop_h", new String[]{"6"}); + APILocator.getFileMetadataAPI().putCustomMetadataAttributes(asset, + java.util.Map.of(FileAssetAPI.BINARY_FIELD, java.util.Map.of("focalPoint", "0.25,0.5"))); + final File redSource = asset.getBinary(FileAssetAPI.BINARY_FIELD); + final File red = exporter.exportContent(redSource, parameters).getDataFile(); + assertEquals(java.awt.Color.RED.getRGB(), javax.imageio.ImageIO.read(red).getRGB(3, 3)); + APILocator.getFileMetadataAPI().putCustomMetadataAttributes(asset, + java.util.Map.of(FileAssetAPI.BINARY_FIELD, java.util.Map.of("focalPoint", "0.75,0.5"))); + final File blueSource = contentletAPI.find(inode, user, false).getBinary(FileAssetAPI.BINARY_FIELD); + final File blue = exporter.exportContent(blueSource, parameters).getDataFile(); + assertNotEquals(red, blue, "A focal edit must change the chained crop cache key"); + assertEquals(java.awt.Color.BLUE.getRGB(), javax.imageio.ImageIO.read(blue).getRGB(3, 3)); + final byte[] cropBytes = Files.readAllBytes(blue.toPath()); + assertTrue(binaryAssetStorageAPI.evictLocalFile(blue)); + assertArrayEquals(cropBytes, Files.readAllBytes(exporter.exportContent(blueSource, parameters).getDataFile().toPath())); + assertTrue(binaryAssetStorageAPI.evictLocalFile(red)); + assertEquals(red, exporter.exportContent(redSource, parameters).getDataFile(), + "An older metadata snapshot must retain its focal crop after a later edit"); + assertEquals(java.awt.Color.RED.getRGB(), javax.imageio.ImageIO.read(red).getRGB(3, 3)); + assertFalse(parameters.containsKey("fp"), "An export must not pin metadata in the caller's reusable parameters"); + APILocator.getFileAssetAPI().cleanThumbnailsFromFileAsset(APILocator.getFileAssetAPI().fromContentlet(asset)); + assertFalse(restored.exists()); + assertNull(binaryAssetStorageAPI.getGeneratedFile(restored), "CMS invalidation must delete the S3 rendition"); + } finally { + FolderDataGen.remove(folder); + } + } + private void assertBinaryMetadataLength(final Contentlet contentlet, final long expected) throws Exception { final com.dotcms.storage.model.Metadata metadata = APILocator.getFileMetadataAPI() .getOrGenerateMetadata(contentlet, "heroImage");