Refactor validation logic and improve annotation-based test coverage in WatermarkController

This commit is contained in:
antonarhipov 2025-10-31 09:15:03 +02:00
parent a62166e311
commit ade007433a
3 changed files with 447 additions and 79 deletions

View File

@ -14,7 +14,8 @@ public final class RegexPatternUtils {
private static final String WHITESPACE_REGEX = "\\s++";
private static final String EXTENSION_REGEX = "\\.(?:[^.]*+)?$";
private static final Pattern COLOR_PATTERN = Pattern.compile("^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$");
private static final Pattern COLOR_PATTERN =
Pattern.compile("^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$");
private RegexPatternUtils() {
super();

View File

@ -1,5 +1,7 @@
package stirling.software.SPDF.controller.api.security;
import static stirling.software.common.util.RegexPatternUtils.getColorPattern;
import java.awt.*;
import java.awt.image.BufferedImage;
import java.beans.PropertyEditorSupport;
@ -44,8 +46,6 @@ import stirling.software.SPDF.model.api.security.AddWatermarkRequest;
import stirling.software.common.service.CustomPDFDocumentFactory;
import stirling.software.common.util.*;
import static stirling.software.common.util.RegexPatternUtils.getColorPattern;
@Slf4j
@RestController
@RequestMapping("/api/v1/security")
@ -77,31 +77,39 @@ public class WatermarkController {
if (opacity < 0.0f || opacity > 1.0f) {
log.error("Opacity must be between 0.0 and 1.0, but got: {}", opacity);
throw ExceptionUtils.createIllegalArgumentException(
"error.opacityOutOfRange" , //TODO
"Opacity must be between 0.0 and 1.0, but got: {0}",
opacity);
"error.opacityOutOfRange", // TODO
"Opacity must be between 0.0 and 1.0, but got: {0}",
opacity);
}
// Validate rotation range: rotationMin <= rotationMax
Float rotationMin = request.getRotationMin();
Float rotationMax = request.getRotationMax();
if (rotationMin != null && rotationMax != null && rotationMin > rotationMax) {
log.error("Rotation minimum ({}) must be less than or equal to rotation maximum ({})", rotationMin, rotationMax);
log.error(
"Rotation minimum ({}) must be less than or equal to rotation maximum ({})",
rotationMin,
rotationMax);
throw ExceptionUtils.createIllegalArgumentException(
"error.rotationRangeInvalid" , //TODO
"Rotation minimum ({0}) must be less than or equal to rotation maximum ({1})",
rotationMin, rotationMax);
"error.rotationRangeInvalid", // TODO
"Rotation minimum ({0}) must be less than or equal to rotation maximum ({1})",
rotationMin,
rotationMax);
}
// Validate font size range: fontSizeMin <= fontSizeMax
Float fontSizeMin = request.getFontSizeMin();
Float fontSizeMax = request.getFontSizeMax();
if (fontSizeMin != null && fontSizeMax != null && fontSizeMin > fontSizeMax) {
log.error("Font size minimum ({}) must be less than or equal to font size maximum ({})", fontSizeMin, fontSizeMax);
log.error(
"Font size minimum ({}) must be less than or equal to font size maximum ({})",
fontSizeMin,
fontSizeMax);
throw ExceptionUtils.createIllegalArgumentException(
"error.fontSizeRangeInvalid" , //TODO
"Font size minimum ({0}) must be less than or equal to font size maximum ({1})",
fontSizeMin, fontSizeMax);
"error.fontSizeRangeInvalid", // TODO
"Font size minimum ({0}) must be less than or equal to font size maximum ({1})",
fontSizeMin,
fontSizeMax);
}
// Validate color format when not using random color
@ -110,11 +118,13 @@ public class WatermarkController {
if (customColor != null && !Boolean.TRUE.equals(randomColor)) {
// Check if color is valid hex format (#RRGGBB or #RRGGBBAA)
if (!getColorPattern().matcher(customColor).matches()) {
log.error("Invalid color format: {}. Expected hex format like #RRGGBB or #RRGGBBAA", customColor);
log.error(
"Invalid color format: {}. Expected hex format like #RRGGBB or #RRGGBBAA",
customColor);
throw ExceptionUtils.createIllegalArgumentException(
"error.invalidColorFormat" , //TODO
"Invalid color format: {0}. Expected hex format like #RRGGBB or #RRGGBBAA",
customColor);
"error.invalidColorFormat", // TODO
"Invalid color format: {0}. Expected hex format like #RRGGBB or #RRGGBBAA",
customColor);
}
}
@ -122,11 +132,13 @@ public class WatermarkController {
Float mirroringProbability = request.getMirroringProbability();
if (mirroringProbability != null
&& (mirroringProbability < 0.0f || mirroringProbability > 1.0f)) {
log.error("Mirroring probability must be between 0.0 and 1.0, but got: {}", mirroringProbability);
log.error(
"Mirroring probability must be between 0.0 and 1.0, but got: {}",
mirroringProbability);
throw ExceptionUtils.createIllegalArgumentException(
"error.mirroringProbabilityOutOfRange" , //TODO
"Mirroring probability must be between 0.0 and 1.0, but got: {0}",
mirroringProbability);
"error.mirroringProbabilityOutOfRange", // TODO
"Mirroring probability must be between 0.0 and 1.0, but got: {0}",
mirroringProbability);
}
// Validate watermark type
@ -134,11 +146,12 @@ public class WatermarkController {
if (watermarkType == null
|| (!watermarkType.equalsIgnoreCase("text")
&& !watermarkType.equalsIgnoreCase("image"))) {
log.error( "Watermark type must be 'text' or 'image', but got: {}", watermarkType);
log.error("Watermark type must be 'text' or 'image', but got: {}", watermarkType);
throw ExceptionUtils.createIllegalArgumentException(
"error.unsupportedWatermarkType" , //TODO
"Watermark type must be 'text' or 'image', but got: {0}",
watermarkType);
"error.unsupportedWatermarkType", // TODO
"Watermark type must be ''text'' or ''image'', but got: {0}", // single quotes
// must be escaped
watermarkType);
}
// Validate text watermark has text
@ -147,8 +160,8 @@ public class WatermarkController {
if (watermarkText == null || watermarkText.trim().isEmpty()) {
log.error("Watermark text is required when watermark type is 'text'");
throw ExceptionUtils.createIllegalArgumentException(
"error.watermarkTextRequired", //TODO
"Watermark text is required when watermark type is 'text'");
"error.watermarkTextRequired", // TODO
"Watermark text is required when watermark type is 'text'");
}
}
@ -158,28 +171,32 @@ public class WatermarkController {
if (watermarkImage == null || watermarkImage.isEmpty()) {
log.error("Watermark image is required when watermark type is 'image'");
throw ExceptionUtils.createIllegalArgumentException(
"error.watermarkImageRequired", //TODO
"Watermark image is required when watermark type is 'image'");
"error.watermarkImageRequired", // TODO
"Watermark image is required when watermark type is 'image'");
}
// Validate image type - only allow common image formats
String contentType = watermarkImage.getContentType();
String originalFilename = watermarkImage.getOriginalFilename();
if (contentType != null && !isSupportedImageType(contentType)) {
log.error("Unsupported image type: {}. Supported types: PNG, JPG, JPEG, GIF, BMP", contentType);
log.error(
"Unsupported image type: {}. Supported types: PNG, JPG, JPEG, GIF, BMP",
contentType);
throw ExceptionUtils.createIllegalArgumentException(
"error.unsupportedContentType", //TODO
"Unsupported image type: {0}. Supported types: PNG, JPG, JPEG, GIF, BMP",
contentType);
"error.unsupportedContentType", // TODO
"Unsupported image type: {0}. Supported types: PNG, JPG, JPEG, GIF, BMP",
contentType);
}
// Additional check based on file extension
if (originalFilename != null && !hasSupportedImageExtension(originalFilename)) {
log.error("Unsupported image file extension in: {}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp", originalFilename);
log.error(
"Unsupported image file extension in: {}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp",
originalFilename);
throw ExceptionUtils.createIllegalArgumentException(
"error.unsupportedImageFileType", //TODO
"Unsupported image file extension in: {0}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp",
originalFilename);
"error.unsupportedImageFileType", // TODO
"Unsupported image file extension in: {0}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp",
originalFilename);
}
}

View File

@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;
import java.util.Collections;
import java.util.Set;
import org.apache.pdfbox.pdmodel.PDDocument;
import org.junit.jupiter.api.BeforeEach;
@ -17,6 +18,11 @@ import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.mock.web.MockMultipartFile;
import org.springframework.web.multipart.MultipartFile;
import jakarta.validation.ConstraintViolation;
import jakarta.validation.Validation;
import jakarta.validation.Validator;
import jakarta.validation.ValidatorFactory;
import stirling.software.SPDF.model.api.security.AddWatermarkRequest;
import stirling.software.common.service.CustomPDFDocumentFactory;
@ -29,12 +35,11 @@ class WatermarkValidationTest {
@InjectMocks private WatermarkController watermarkController;
private AddWatermarkRequest request;
private MockMultipartFile mockPdfFile;
@BeforeEach
void setUp() throws Exception {
request = new AddWatermarkRequest();
mockPdfFile =
MockMultipartFile mockPdfFile =
new MockMultipartFile(
"fileInput", "test.pdf", "application/pdf", "test content".getBytes());
request.setFileInput(mockPdfFile);
@ -626,86 +631,431 @@ class WatermarkValidationTest {
@DisplayName("Annotation-based Validation Tests")
class AnnotationBasedValidationTests {
private Validator validator;
@BeforeEach
void setUpValidator() {
ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
validator = factory.getValidator();
}
@Test
@DisplayName("Should enforce count minimum of 1")
@DisplayName("Should reject count below minimum of 1")
void testCountMinimum() {
// Note: This tests the @Min annotation on count field
// The actual validation happens at the framework level
// Arrange
request.setCount(0);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("count")),
"Should have violation on 'count' field");
}
@Test
@DisplayName("Should enforce count maximum of 1000")
@DisplayName("Should accept valid count value")
void testCountValid() {
// Arrange
request.setCount(5);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(v -> v.getPropertyPath().toString().equals("count")),
"Should have no violations on 'count' field");
}
@Test
@DisplayName("Should reject count above maximum of 1000")
void testCountMaximum() {
// Note: This tests the @Max annotation on count field
// Arrange
request.setCount(1001);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("count")),
"Should have violation on 'count' field");
}
@Test
@DisplayName("Should enforce rotation min/max bounds of -360 to 360")
void testRotationBounds() {
// Note: This tests the @DecimalMin/@DecimalMax annotations
@DisplayName("Should reject rotationMin below -360")
void testRotationMinBelowBound() {
// Arrange
request.setRotationMin(-361f);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("rotationMin")),
"Should have violation on 'rotationMin' field");
}
@Test
@DisplayName("Should reject rotationMax above 360")
void testRotationMaxAboveBound() {
// Arrange
request.setRotationMax(361f);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("rotationMax")),
"Should have violation on 'rotationMax' field");
}
@Test
@DisplayName("Should enforce font size bounds of 1.0 to 500.0")
void testFontSizeBounds() {
// Note: This tests the @DecimalMin/@DecimalMax annotations
@DisplayName("Should accept valid rotation values")
void testRotationValid() {
// Arrange
request.setRotationMin(-180f);
request.setRotationMax(180f);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(
v ->
v.getPropertyPath().toString().equals("rotationMin")
|| v.getPropertyPath()
.toString()
.equals("rotationMax")),
"Should have no violations on rotation fields");
}
@Test
@DisplayName("Should reject fontSizeMin below 1.0")
void testFontSizeMinBelowBound() {
// Arrange
request.setFontSizeMin(0.5f);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("fontSizeMin")),
"Should have violation on 'fontSizeMin' field");
}
@Test
@DisplayName("Should reject fontSizeMax above 500.0")
void testFontSizeMaxAboveBound() {
// Arrange
request.setFontSizeMax(501f);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("fontSizeMax")),
"Should have violation on 'fontSizeMax' field");
}
@Test
@DisplayName("Should enforce per-letter font count bounds of 1 to 20")
void testPerLetterFontCountBounds() {
// Note: This tests the @Min/@Max annotations
@DisplayName("Should accept valid font size values")
void testFontSizeValid() {
// Arrange
request.setFontSizeMin(10f);
request.setFontSizeMax(100f);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(
v ->
v.getPropertyPath().toString().equals("fontSizeMin")
|| v.getPropertyPath()
.toString()
.equals("fontSizeMax")),
"Should have no violations on font size fields");
}
@Test
@DisplayName("Should reject perLetterFontCount below 1")
void testPerLetterFontCountMinimum() {
// Arrange
request.setPerLetterFontCount(0);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(
v ->
v.getPropertyPath()
.toString()
.equals("perLetterFontCount")),
"Should have violation on 'perLetterFontCount' field");
}
@Test
@DisplayName("Should reject perLetterFontCount above 20")
void testPerLetterFontCountMaximum() {
// Arrange
request.setPerLetterFontCount(21);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(
v ->
v.getPropertyPath()
.toString()
.equals("perLetterFontCount")),
"Should have violation on 'perLetterFontCount' field");
}
@Test
@DisplayName("Should enforce per-letter color count bounds of 1 to 20")
void testPerLetterColorCountBounds() {
// Note: This tests the @Min/@Max annotations
@DisplayName("Should accept valid perLetterFontCount value")
void testPerLetterFontCountValid() {
// Arrange
request.setPerLetterFontCount(5);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(
v ->
v.getPropertyPath()
.toString()
.equals("perLetterFontCount")),
"Should have no violations on 'perLetterFontCount' field");
}
@Test
@DisplayName("Should reject perLetterColorCount below 1")
void testPerLetterColorCountMinimum() {
// Arrange
request.setPerLetterColorCount(0);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(
v ->
v.getPropertyPath()
.toString()
.equals("perLetterColorCount")),
"Should have violation on 'perLetterColorCount' field");
}
@Test
@DisplayName("Should reject perLetterColorCount above 20")
void testPerLetterColorCountMaximum() {
// Arrange
request.setPerLetterColorCount(21);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(
v ->
v.getPropertyPath()
.toString()
.equals("perLetterColorCount")),
"Should have violation on 'perLetterColorCount' field");
}
@Test
@DisplayName("Should enforce margin bounds of 0.0 to 500.0")
void testMarginBounds() {
// Note: This tests the @DecimalMin/@DecimalMax annotations
@DisplayName("Should accept valid perLetterColorCount value")
void testPerLetterColorCountValid() {
// Arrange
request.setPerLetterColorCount(4);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(
v ->
v.getPropertyPath()
.toString()
.equals("perLetterColorCount")),
"Should have no violations on 'perLetterColorCount' field");
}
@Test
@DisplayName("Should reject margin below 0.0")
void testMarginBelowBound() {
// Arrange
request.setMargin(-1f);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("margin")),
"Should have violation on 'margin' field");
}
@Test
@DisplayName("Should reject margin above 500.0")
void testMarginAboveBound() {
// Arrange
request.setMargin(501f);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("margin")),
"Should have violation on 'margin' field");
}
@Test
@DisplayName("Should enforce image scale bounds of 0.1 to 10.0")
void testImageScaleBounds() {
// Note: This tests the @DecimalMin/@DecimalMax annotations
@DisplayName("Should accept valid margin value")
void testMarginValid() {
// Arrange
request.setMargin(10f);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(v -> v.getPropertyPath().toString().equals("margin")),
"Should have no violations on 'margin' field");
}
@Test
@DisplayName("Should reject imageScale below 0.1")
void testImageScaleBelowBound() {
// Arrange
request.setImageScale(0.05f);
request.setImageScale(11f);
// Framework validation would reject this before reaching controller
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("imageScale")),
"Should have violation on 'imageScale' field");
}
@Test
@DisplayName("Should validate bounds format pattern")
void testBoundsFormatPattern() {
// Note: This tests the @Pattern annotation on bounds field
request.setBounds("invalid");
// Framework validation would reject this before reaching controller
@DisplayName("Should reject imageScale above 10.0")
void testImageScaleAboveBound() {
// Arrange
request.setImageScale(11f);
request.setBounds("100,100,200,200"); // Valid format
// Framework validation would accept this
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("imageScale")),
"Should have violation on 'imageScale' field");
}
@Test
@DisplayName("Should accept valid imageScale value")
void testImageScaleValid() {
// Arrange
request.setImageScale(1.5f);
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(v -> v.getPropertyPath().toString().equals("imageScale")),
"Should have no violations on 'imageScale' field");
}
@Test
@DisplayName("Should reject invalid bounds format")
void testBoundsInvalidFormat() {
// Arrange
request.setBounds("invalid");
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertFalse(violations.isEmpty(), "Should have validation errors");
assertTrue(
violations.stream()
.anyMatch(v -> v.getPropertyPath().toString().equals("bounds")),
"Should have violation on 'bounds' field");
}
@Test
@DisplayName("Should accept valid bounds format")
void testBoundsValidFormat() {
// Arrange
request.setBounds("100,100,200,200");
// Act
Set<ConstraintViolation<AddWatermarkRequest>> violations = validator.validate(request);
// Assert
assertTrue(
violations.stream()
.noneMatch(v -> v.getPropertyPath().toString().equals("bounds")),
"Should have no violations on 'bounds' field");
}
}
}