Skip to content

Commit

Permalink
feat(Image Api) Ignore transformation on vector images #30196 (#30267)
Browse files Browse the repository at this point in the history
### Proposed Changes
* Added a utility method to check for vector image extensions in
`UtilMethods`.
* Updated `ImageFilterExporter` to skip transformation for vector images
(SVG, EPS, etc.).

### Checklist
- [x] Tests
- [x] Translations
- [x] Security Implications Contemplated (add notes if applicable)

### Additional Info
This pull request addresses issue #30196, ensuring that the Image API
ignores vector images during processing. Now only raster images will be
transformed.

### Videos
#### Original

https://github.com/user-attachments/assets/8e6f04f7-aad9-4d20-a92b-b9e5c7bac485

#### Updated

https://github.com/user-attachments/assets/bb2408f0-3db5-47f4-9dd2-85ba21e98bd0
  • Loading branch information
valentinogiardino authored Oct 7, 2024
1 parent 5738ed5 commit 0028f3c
Show file tree
Hide file tree
Showing 5 changed files with 1,729 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import com.dotmarketing.exception.DotRuntimeException;
import com.dotmarketing.image.filter.ImageFilter;
import com.dotmarketing.image.filter.ImageFilterAPI;
import com.dotmarketing.image.filter.ImageFilterApiImpl;
import com.dotmarketing.image.filter.PDFImageFilter;
import com.dotmarketing.portlets.contentlet.business.BinaryContentExporter;
import com.dotmarketing.portlets.contentlet.business.BinaryContentExporterException;
Expand All @@ -18,8 +17,6 @@
import com.dotmarketing.util.UtilMethods;
import io.vavr.control.Try;



/**
*
* An exporter that can take 1 or more filters in a chain
Expand All @@ -34,8 +31,7 @@ public class ImageFilterExporter implements BinaryContentExporter {

private final int allowedRequests = Config.getIntProperty("IMAGE_GENERATION_SIMULTANEOUS_REQUESTS", 10);

private final Semaphore semaphore = new Semaphore(allowedRequests);

private final Semaphore semaphore = new Semaphore(allowedRequests);

/*
* (non-Javadoc)
Expand All @@ -47,6 +43,12 @@ public class ImageFilterExporter implements BinaryContentExporter {
public BinaryContentExporterData exportContent(File file, final Map<String, String[]> parameters)
throws BinaryContentExporterException {

final String fileExtension = UtilMethods.getFileExtension(file.getName());
if (UtilMethods.isVectorImage(fileExtension)) {
Logger.info(this.getClass(), "Skipping vector image transformation for " + fileExtension);
return new BinaryContentExporterData(file);
}

Class<? extends ImageFilter> errorClass = ImageFilter.class;
try {

Expand All @@ -55,7 +57,7 @@ public BinaryContentExporterData exportContent(File file, final Map<String, Stri
parameters.put("filters", filters.keySet().toArray(new String[0]));

// run pdf filter first (if a pdf)
if(!filters.isEmpty() && "pdf".equals(UtilMethods.getFileExtension(file.getName())) && !filters.containsKey("pdf")) {
if(!filters.isEmpty() && "pdf".equals(fileExtension) && !filters.containsKey("pdf")) {
file = runFilter(new PDFImageFilter(), file, parameters);
}

Expand Down Expand Up @@ -127,9 +129,6 @@ private Optional<File> alreadyGenerated(final Collection<Class<? extends ImageFi
}
return Optional.of(fileToReturn);
}




public String getName() {
return "Image Filter Exporter";
Expand Down
13 changes: 13 additions & 0 deletions dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import java.util.Set;

/**
* Provides several widely used routines that handle, verify or format many data structures, such as
Expand Down Expand Up @@ -143,6 +144,8 @@ public class UtilMethods {

private static final String UTILMETHODS_DEFAULT_ENCODING = "UTF-8";

private static final Set<String> VECTOR_EXTENSIONS = Set.of("svg", "eps", "ai", "dxf");

public static final java.util.Date pidmsToDate(String d) {
java.text.ParsePosition pos = new java.text.ParsePosition(0);

Expand Down Expand Up @@ -3689,4 +3692,14 @@ public static String extractUserIdOrNull(final User user) {
return Optional.ofNullable(user).map(User::getUserId).orElse(null);
}

/**
* Determines whether the given file extension corresponds to a vector image format.
*
* @param fileExtension The extension of the file to be checked.
* @return true if the file extension indicates a vector image format; false otherwise.
*/
public static boolean isVectorImage(String fileExtension) {
return VECTOR_EXTENSIONS.contains(fileExtension);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
package com.dotmarketing.portlets.contentlet.business.exporter;

import static org.junit.jupiter.api.Assertions.*;

import java.io.File;
import java.net.URL;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

import com.dotmarketing.portlets.contentlet.business.BinaryContentExporter.BinaryContentExporterData;
import org.apache.commons.io.FileUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

/**
* Unit tests for the ImageFilterExporter class.
*/
class ImageFilterExporterTest {

private ImageFilterExporter exporter;

/**
* Set up the ImageFilterExporter instance before each test.
*/
@BeforeEach
public void setUp() {
exporter = new ImageFilterExporter();
}

/**
* Given a valid SVG image file and a set of parameters,
* When exporting the content of the image,
* Then the method should return a result without modifying the image.
*
* @throws Exception if an error occurs during export.
*/
@Test
void testExportContentWithSvgImage() throws Exception {
// Given
final URL url = getClass().getResource("/images/test.svg");
File inputFile = new File(Objects.requireNonNull(url).getFile());
Map<String, String[]> parameters = getSampleParameters();

// When
BinaryContentExporterData result = exporter.exportContent(inputFile, parameters);

// Then
assertNotNull(result);
assertTrue(FileUtils.contentEquals(inputFile, result.getDataFile())); // The file should not be transformed
}

/**
* Given a valid EPS image file and a set of parameters,
* When exporting the content of the image,
* Then the method should return a result without modifying the image.
*
* @throws Exception if an error occurs during export.
*/
@Test
void testExportContentWithEpsImage() throws Exception {
// Given
final URL url = getClass().getResource("/images/test.eps");
final File inputFile = new File(Objects.requireNonNull(url).getFile());
Map<String, String[]> parameters = getSampleParameters();

// When
final BinaryContentExporterData result = exporter.exportContent(inputFile, parameters);

// Then
assertNotNull(result);
assertTrue(FileUtils.contentEquals(inputFile, result.getDataFile())); // The file should not be transformed
}

/**
* Given a valid non-vector image file (JPG) and a set of parameters,
* When exporting the content of the image,
* Then the method should return a result with the image transformed.
*
* @throws Exception if an error occurs during export.
*/
@Test
void testExportContentWithNonVectorImage() throws Exception {
// Given
final URL url = getClass().getResource("/images/test.jpg");
final File inputFile = new File(Objects.requireNonNull(url).getFile());
Map<String, String[]> parameters = getSampleParameters();

// When
final BinaryContentExporterData result = exporter.exportContent(inputFile, parameters);

// Then
assertNotNull(result);
assertFalse(FileUtils.contentEquals(inputFile, result.getDataFile())); // The file should be transformed
}

/**
* Helper method to provide a sample parameters map for use in the exportContent tests.
*
* @return a Map containing sample parameters.
*/
final Map<String, String[]> getSampleParameters() {
Map<String, String[]> parameters = new HashMap<>();
parameters.put("contentAsset", new String[]{"image"});
parameters.put("r", new String[]{"1728068872149"});
parameters.put("e5150266-7e28-445b-b5dd-193dc38922c7", new String[]{"asset"});
parameters.put("byInode", new String[]{"true"});
parameters.put("resize_w", new String[]{"500"});
parameters.put("quality_q", new String[]{"50"});
parameters.put("webp_q", new String[]{"50"});
parameters.put("fieldVarName", new String[]{"asset"});
parameters.put("assetInodeOrIdentifier", new String[]{"e5150266-7e28-445b-b5dd-193dc38922c7"});
return parameters;
}
}
34 changes: 34 additions & 0 deletions dotCMS/src/test/java/com/dotmarketing/util/UtilMethodsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.dotmarketing.portlets.contentlet.model.Contentlet;
import com.liferay.portal.model.User;
import org.junit.Test;
import org.junit.jupiter.api.Assertions;

/**
* Unit test for {@link UtilMethods}
Expand Down Expand Up @@ -236,4 +237,37 @@ public void test_extractUserIdOrNull(){
assertEquals("userId", UtilMethods.extractUserIdOrNull(user));
}

final static String[] rasterImagesExtensions = new String[]{"webp", "png", "gif", "jpg"};
final static String[] vectorImagesExtensions = new String[]{"svg", "eps", "ai", "dxf"};

/**
* Given vector image extensions (SVG or EPS),
* When checking if are vector images,
* Then the method should return true for all vector extensions.
*/
@Test
public void testIsVectorImageWithVectorExtensions() {
// Given
// When & Then
for (String vectorExtension : vectorImagesExtensions) {
Assertions.assertTrue(UtilMethods.isVectorImage(vectorExtension),
"Expected transformation to be skipped for vector extension: " + vectorExtension);
}
}

/**
* Given raster image extensions (JPG, PNG, etc.),
* When checking if are vector images,
* Then the method should return false for all raster extensions.
*/
@Test
public void testIsVectorImageWithRasterExtensions() {
// Given
// When & Then
for (String rasterExtension : rasterImagesExtensions) {
Assertions.assertFalse(UtilMethods.isVectorImage(rasterExtension),
"Expected transformation not to be skipped for raster extension: " + rasterExtension);
}
}

}
Loading

0 comments on commit 0028f3c

Please sign in to comment.