From 51e61ae820fcc19a84f2ade1f2f9011ada9235e9 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Sat, 27 Dec 2025 20:28:40 +0100 Subject: [PATCH 01/15] implement max file size --- .../UpdateAttachmentsHandler.java | 89 +++++++++++++------ .../helper/FileSizeUtils.java | 50 +++++++++++ pom.xml | 2 +- samples/bookshop/.gitignore | 3 + samples/bookshop/pom.xml | 2 +- samples/bookshop/srv/attachments.cds | 3 + 6 files changed, 119 insertions(+), 30 deletions(-) create mode 100644 cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index 0a193b774..d5aca9565 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -7,8 +7,10 @@ import com.sap.cds.CdsData; import com.sap.cds.CdsDataProcessor; +import com.sap.cds.CdsDataProcessor.Filter; import com.sap.cds.CdsDataProcessor.Validator; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; +import com.sap.cds.feature.attachments.handler.applicationservice.helper.FileSizeUtils; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ModifyApplicationHandlerHelper; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ReadonlyDataContextEnhancer; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ThreadDataStorageReader; @@ -30,18 +32,28 @@ import com.sap.cds.services.utils.model.CqnUtils; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * The class {@link UpdateAttachmentsHandler} is an event handler that is called before an update - * event is executed. As updates in draft entities or non-draft entities can also be create-events, - * update-events or delete-events the handler needs to distinguish between the different cases. + * The class {@link UpdateAttachmentsHandler} is an event handler that is called + * before an update + * event is executed. As updates in draft entities or non-draft entities can + * also be create-events, + * update-events or delete-events the handler needs to distinguish between the + * different cases. */ @ServiceName(value = "*", type = ApplicationService.class) public class UpdateAttachmentsHandler implements EventHandler { private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); + public static final Filter VALMAX_FILTER = (path, element, type) -> element.findAnnotation("Validation.Maximum") + .isPresent(); private final ModifyAttachmentEventFactory eventFactory; private final AttachmentsReader attachmentsReader; @@ -54,17 +66,16 @@ public UpdateAttachmentsHandler( AttachmentService attachmentService, ThreadDataStorageReader storageReader) { this.eventFactory = requireNonNull(eventFactory, "eventFactory must not be null"); - this.attachmentsReader = - requireNonNull(attachmentsReader, "attachmentsReader must not be null"); - this.attachmentService = - requireNonNull(attachmentService, "attachmentService must not be null"); + this.attachmentsReader = requireNonNull(attachmentsReader, "attachmentsReader must not be null"); + this.attachmentService = requireNonNull(attachmentService, "attachmentService must not be null"); this.storageReader = requireNonNull(storageReader, "storageReader must not be null"); } @Before @HandlerOrder(OrderConstants.Before.CHECK_CAPABILITIES) void processBeforeForDraft(CdsUpdateEventContext context, List data) { - // before the attachment's readonly fields are removed by the runtime, preserve them in a custom + // before the attachment's readonly fields are removed by the runtime, preserve + // them in a custom // field in data ReadonlyDataContextEnhancer.preserveReadonlyFields( context.getTarget(), data, storageReader.get()); @@ -77,14 +88,19 @@ void processBefore(CdsUpdateEventContext context, List data) { boolean associationsAreUnchanged = associationsAreUnchanged(target, data); if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) { + + // Check here for size of new attachments + if (containsValMaxAnnotation(target, data)) { + long maxSizeValue = FileSizeUtils.convertValMaxToInt(getValMaxValue(target, data)); + logger.debug("Validation.Maximum annotation found with value: {}", maxSizeValue); + } + logger.debug("Processing before {} event for entity {}", context.getEvent(), target); CqnSelect select = CqnUtils.toSelect(context.getCqn(), context.getTarget()); - List attachments = - attachmentsReader.readAttachments(context.getModel(), target, select); + List attachments = attachmentsReader.readAttachments(context.getModel(), target, select); - List condensedAttachments = - ApplicationHandlerHelper.condenseAttachments(attachments, target); + List condensedAttachments = ApplicationHandlerHelper.condenseAttachments(attachments, target); ModifyApplicationHandlerHelper.handleAttachmentForEntities( target, data, condensedAttachments, eventFactory, context); @@ -94,8 +110,28 @@ void processBefore(CdsUpdateEventContext context, List data) { } } + private String getValMaxValue(CdsEntity entity, List data) { + AtomicReference annotationValue = new AtomicReference<>(); + CdsDataProcessor.create() + .addValidator(VALMAX_FILTER, (path, element, value) -> { + element.findAnnotation("Validation.Maximum") + .ifPresent(annotation -> annotationValue.set(annotation.getValue().toString())); + }) + .process(data, entity); + return annotationValue.get(); + } + + private boolean containsValMaxAnnotation(CdsEntity entity, List data) { + AtomicBoolean isIncluded = new AtomicBoolean(); + CdsDataProcessor.create() + .addValidator(VALMAX_FILTER, (path, element, value) -> isIncluded.set(true)) + .process(data, entity); + return isIncluded.get(); + } + private boolean associationsAreUnchanged(CdsEntity entity, List data) { - // TODO: check if this should be replaced with entity.assocations().noneMatch(...) + // TODO: check if this should be replaced with + // entity.assocations().noneMatch(...) return entity .compositions() .noneMatch( @@ -107,21 +143,18 @@ private void deleteRemovedAttachments( List data, CdsEntity entity, UserInfo userInfo) { - List condensedAttachments = - ApplicationHandlerHelper.condenseAttachments(data, entity); - - Validator validator = - (path, element, value) -> { - Map keys = ApplicationHandlerHelper.removeDraftKey(path.target().keys()); - boolean entryExists = - condensedAttachments.stream() - .anyMatch( - updatedData -> ApplicationHandlerHelper.areKeysInData(keys, updatedData)); - if (!entryExists) { - String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); - attachmentService.markAttachmentAsDeleted(new MarkAsDeletedInput(contentId, userInfo)); - } - }; + List condensedAttachments = ApplicationHandlerHelper.condenseAttachments(data, entity); + + Validator validator = (path, element, value) -> { + Map keys = ApplicationHandlerHelper.removeDraftKey(path.target().keys()); + boolean entryExists = condensedAttachments.stream() + .anyMatch( + updatedData -> ApplicationHandlerHelper.areKeysInData(keys, updatedData)); + if (!entryExists) { + String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); + attachmentService.markAttachmentAsDeleted(new MarkAsDeletedInput(contentId, userInfo)); + } + }; CdsDataProcessor.create() .addValidator(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, validator) .process(existingAttachments, entity); diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java new file mode 100644 index 000000000..daa2cc08b --- /dev/null +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java @@ -0,0 +1,50 @@ +package com.sap.cds.feature.attachments.handler.applicationservice.helper; + +import java.math.BigDecimal; +import java.util.Locale; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class FileSizeUtils { + private static final Pattern SIZE = Pattern.compile("^\\s*([0-9]+(?:\\.[0-9]+)?)\\s*([a-zA-Z]*)\\s*$"); + private static final Map MULTIPLIER = Map.ofEntries( + Map.entry("", 1L), + Map.entry("B", 1L), + + // Decimal + Map.entry("KB", 1000L), + Map.entry("MB", 1000L * 1000), + Map.entry("GB", 1000L * 1000 * 1000), + Map.entry("TB", 1000L * 1000 * 1000 * 1000), + + // Binary + Map.entry("KIB", 1024L), + Map.entry("MIB", 1024L * 1024), + Map.entry("GIB", 1024L * 1024 * 1024), + Map.entry("TIB", 1024L * 1024 * 1024 * 1024)); + + private FileSizeUtils() {} + + public static long convertValMaxToInt(String input) { + // First validate string + if (input == null) + throw new IllegalArgumentException("Value for Max File Size is null"); + + Matcher m = SIZE.matcher(input); + if (!m.matches()) { + throw new IllegalArgumentException("Invalid size: " + input); + } + BigDecimal value = new BigDecimal(m.group(1)); + String unitRaw = m.group(2) == null ? "" : m.group(2); + String unit = unitRaw.toUpperCase(); + + // if (unit.length() == 1) unit = unit + "B"; // for people using K instead of KB + Long mul = MULTIPLIER.get(unit); + if (mul == null) { + throw new IllegalArgumentException("Unkown Unit: " + unitRaw); + } + BigDecimal bytes = value.multiply(BigDecimal.valueOf(mul)); + return bytes.longValueExact(); + } +} diff --git a/pom.xml b/pom.xml index 57a91e70a..36d9c15bf 100644 --- a/pom.xml +++ b/pom.xml @@ -58,7 +58,7 @@ - 1.2.4 + 1.2.5-SNAPSHOT 17 ${java.version} UTF-8 diff --git a/samples/bookshop/.gitignore b/samples/bookshop/.gitignore index c161f228e..2ecc68df6 100644 --- a/samples/bookshop/.gitignore +++ b/samples/bookshop/.gitignore @@ -29,3 +29,6 @@ hs_err* .vscode .idea .reloadtrigger + +# added by cds +.cdsrc-private.json diff --git a/samples/bookshop/pom.xml b/samples/bookshop/pom.xml index f18606817..18d403302 100644 --- a/samples/bookshop/pom.xml +++ b/samples/bookshop/pom.xml @@ -51,7 +51,7 @@ com.sap.cds cds-feature-attachments - 1.2.4-SNAPSHOT + 1.2.5-SNAPSHOT diff --git a/samples/bookshop/srv/attachments.cds b/samples/bookshop/srv/attachments.cds index 30db69511..313df3261 100644 --- a/samples/bookshop/srv/attachments.cds +++ b/samples/bookshop/srv/attachments.cds @@ -7,6 +7,9 @@ extend my.Books with { attachments : Composition of many Attachments; } +annotate my.Books.attachments with @Validation.Maximum: '2MB'; + + // Add UI component for attachments table to the Browse Books App using {CatalogService as service} from '../app/services'; From d8dde3cd8f5679eff74d9f68d46b88182cf2ebaa Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Sun, 28 Dec 2025 20:28:17 +0100 Subject: [PATCH 02/15] improvements --- .../UpdateAttachmentsHandler.java | 18 ++++++++++++++---- .../helper/FileSizeUtils.java | 1 - samples/bookshop/srv/attachments.cds | 4 +++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index d5aca9565..b3c9af45d 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -5,6 +5,8 @@ import static java.util.Objects.requireNonNull; +import java.io.IOException; + import com.sap.cds.CdsData; import com.sap.cds.CdsDataProcessor; import com.sap.cds.CdsDataProcessor.Filter; @@ -34,8 +36,6 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,7 +52,7 @@ public class UpdateAttachmentsHandler implements EventHandler { private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); - public static final Filter VALMAX_FILTER = (path, element, type) -> element.findAnnotation("Validation.Maximum") + public static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") && element.findAnnotation("Validation.Maximum") .isPresent(); private final ModifyAttachmentEventFactory eventFactory; @@ -88,10 +88,20 @@ void processBefore(CdsUpdateEventContext context, List data) { boolean associationsAreUnchanged = associationsAreUnchanged(target, data); if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) { - // Check here for size of new attachments if (containsValMaxAnnotation(target, data)) { + List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); long maxSizeValue = FileSizeUtils.convertValMaxToInt(getValMaxValue(target, data)); + attachments.forEach(attachment -> { + try { + int size = attachment.getContent().available(); + if (size > maxSizeValue) { + throw new IllegalArgumentException("Attachment " + attachment.getFileName() + " exceeds the maximum allowed size of " + maxSizeValue + " bytes."); + } + } catch (IOException e) { + throw new RuntimeException("Failed to read attachment content size", e); + } + }); logger.debug("Validation.Maximum annotation found with value: {}", maxSizeValue); } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java index daa2cc08b..179217478 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java @@ -1,7 +1,6 @@ package com.sap.cds.feature.attachments.handler.applicationservice.helper; import java.math.BigDecimal; -import java.util.Locale; import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; diff --git a/samples/bookshop/srv/attachments.cds b/samples/bookshop/srv/attachments.cds index 313df3261..44178bf73 100644 --- a/samples/bookshop/srv/attachments.cds +++ b/samples/bookshop/srv/attachments.cds @@ -7,7 +7,9 @@ extend my.Books with { attachments : Composition of many Attachments; } -annotate my.Books.attachments with @Validation.Maximum: '2MB'; +annotate my.Books.attachments with { + content @Validation.Maximum: '20MB'; +} // Add UI component for attachments table to the Browse Books App From ca2d3a811a3850bfad434726ea3940abc26e7b7a Mon Sep 17 00:00:00 2001 From: Marvin Date: Mon, 12 Jan 2026 09:12:37 +0100 Subject: [PATCH 03/15] Apply suggestions from code review Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com> --- .../UpdateAttachmentsHandler.java | 18 +++++++++++++++++- .../helper/FileSizeUtils.java | 4 ++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index b3c9af45d..cf3f19244 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -52,7 +52,8 @@ public class UpdateAttachmentsHandler implements EventHandler { private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); - public static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") && element.findAnnotation("Validation.Maximum") + private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); + private static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") && element.findAnnotation("Validation.Maximum") .isPresent(); private final ModifyAttachmentEventFactory eventFactory; @@ -102,7 +103,22 @@ void processBefore(CdsUpdateEventContext context, List data) { throw new RuntimeException("Failed to read attachment content size", e); } }); + // Check here for size of new attachments + if (containsValMaxAnnotation(target, data)) { + List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); + long maxSizeValue = FileSizeUtils.convertValMaxToInt(getValMaxValue(target, data)); logger.debug("Validation.Maximum annotation found with value: {}", maxSizeValue); + attachments.forEach(attachment -> { + try { + int size = attachment.getContent().available(); + if (size > maxSizeValue) { + throw new IllegalArgumentException("Attachment " + attachment.getFileName() + " exceeds the maximum allowed size of " + maxSizeValue + " bytes."); + } + } catch (IOException e) { + throw new RuntimeException("Failed to read attachment content size", e); + } + }); + } } logger.debug("Processing before {} event for entity {}", context.getEvent(), target); diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java index 179217478..537396f68 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java @@ -25,7 +25,7 @@ public class FileSizeUtils { private FileSizeUtils() {} - public static long convertValMaxToInt(String input) { + public static long parseFileSizeToBytes(String input) { // First validate string if (input == null) throw new IllegalArgumentException("Value for Max File Size is null"); @@ -41,7 +41,7 @@ public static long convertValMaxToInt(String input) { // if (unit.length() == 1) unit = unit + "B"; // for people using K instead of KB Long mul = MULTIPLIER.get(unit); if (mul == null) { - throw new IllegalArgumentException("Unkown Unit: " + unitRaw); + throw new IllegalArgumentException("Unknown Unit: " + unitRaw); } BigDecimal bytes = value.multiply(BigDecimal.valueOf(mul)); return bytes.longValueExact(); From 9649f8d3989dea58e12336bd7b0d6f85c0d9ef78 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 12 Jan 2026 10:54:34 +0100 Subject: [PATCH 04/15] update exception handling --- .../UpdateAttachmentsHandler.java | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index cf3f19244..9f38773b1 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -52,9 +52,9 @@ public class UpdateAttachmentsHandler implements EventHandler { private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); - private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); - private static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") && element.findAnnotation("Validation.Maximum") - .isPresent(); + private static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") + && element.findAnnotation("Validation.Maximum") + .isPresent(); private final ModifyAttachmentEventFactory eventFactory; private final AttachmentsReader attachmentsReader; @@ -91,34 +91,24 @@ void processBefore(CdsUpdateEventContext context, List data) { if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) { // Check here for size of new attachments if (containsValMaxAnnotation(target, data)) { - List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); - long maxSizeValue = FileSizeUtils.convertValMaxToInt(getValMaxValue(target, data)); + try { + List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); + long maxSizeValue = FileSizeUtils.parseFileSizeToBytes(getValMaxValue(target, data)); + logger.debug("Validation.Maximum annotation found with value: {}", maxSizeValue); attachments.forEach(attachment -> { try { int size = attachment.getContent().available(); if (size > maxSizeValue) { - throw new IllegalArgumentException("Attachment " + attachment.getFileName() + " exceeds the maximum allowed size of " + maxSizeValue + " bytes."); + throw new IllegalArgumentException("Attachment " + attachment.getFileName() + + " exceeds the maximum allowed size of " + maxSizeValue + " bytes."); } } catch (IOException e) { throw new RuntimeException("Failed to read attachment content size", e); } }); - // Check here for size of new attachments - if (containsValMaxAnnotation(target, data)) { - List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); - long maxSizeValue = FileSizeUtils.convertValMaxToInt(getValMaxValue(target, data)); - logger.debug("Validation.Maximum annotation found with value: {}", maxSizeValue); - attachments.forEach(attachment -> { - try { - int size = attachment.getContent().available(); - if (size > maxSizeValue) { - throw new IllegalArgumentException("Attachment " + attachment.getFileName() + " exceeds the maximum allowed size of " + maxSizeValue + " bytes."); - } - } catch (IOException e) { - throw new RuntimeException("Failed to read attachment content size", e); - } - }); - } + } catch (ArithmeticException e) { + throw new IllegalArgumentException("Maximum file size value is too large", e); + } } logger.debug("Processing before {} event for entity {}", context.getEvent(), target); From 09997be66c7744fe1e0273fd5e4f30cec5d383ff Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 12 Jan 2026 13:27:20 +0100 Subject: [PATCH 05/15] wip --- .../applicationservice/UpdateAttachmentsHandler.java | 12 +++--------- .../readhelper/CountingInputStream.java | 5 +++++ .../readhelper/LazyProxyInputStream.java | 5 +++++ 3 files changed, 13 insertions(+), 9 deletions(-) create mode 100644 cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index 9f38773b1..e79bcbd46 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -21,7 +21,6 @@ import com.sap.cds.feature.attachments.handler.common.AttachmentsReader; import com.sap.cds.feature.attachments.service.AttachmentService; import com.sap.cds.feature.attachments.service.model.service.MarkAsDeletedInput; -import com.sap.cds.ql.cqn.CqnSelect; import com.sap.cds.reflect.CdsEntity; import com.sap.cds.services.cds.ApplicationService; import com.sap.cds.services.cds.CdsUpdateEventContext; @@ -31,7 +30,6 @@ import com.sap.cds.services.handler.annotations.ServiceName; import com.sap.cds.services.request.UserInfo; import com.sap.cds.services.utils.OrderConstants; -import com.sap.cds.services.utils.model.CqnUtils; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; @@ -87,12 +85,13 @@ void processBeforeForDraft(CdsUpdateEventContext context, List data) { void processBefore(CdsUpdateEventContext context, List data) { CdsEntity target = context.getTarget(); boolean associationsAreUnchanged = associationsAreUnchanged(target, data); + int contentLength = Integer.parseInt(context.getParameterInfo().getHeader("Content-Length")); if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) { // Check here for size of new attachments + List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); if (containsValMaxAnnotation(target, data)) { try { - List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); long maxSizeValue = FileSizeUtils.parseFileSizeToBytes(getValMaxValue(target, data)); logger.debug("Validation.Maximum annotation found with value: {}", maxSizeValue); attachments.forEach(attachment -> { @@ -112,13 +111,8 @@ void processBefore(CdsUpdateEventContext context, List data) { } logger.debug("Processing before {} event for entity {}", context.getEvent(), target); - - CqnSelect select = CqnUtils.toSelect(context.getCqn(), context.getTarget()); - List attachments = attachmentsReader.readAttachments(context.getModel(), target, select); - - List condensedAttachments = ApplicationHandlerHelper.condenseAttachments(attachments, target); ModifyApplicationHandlerHelper.handleAttachmentForEntities( - target, data, condensedAttachments, eventFactory, context); + target, data, attachments, eventFactory, context); if (!associationsAreUnchanged) { deleteRemovedAttachments(attachments, data, target, context.getUserInfo()); diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java new file mode 100644 index 000000000..6a03bf699 --- /dev/null +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java @@ -0,0 +1,5 @@ +package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; + +public class CountingInputStream { + +} diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java index 850b588f1..9d9f636e9 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java @@ -57,6 +57,11 @@ public void close() throws IOException { } } + @Override + public int available() throws IOException { + return delegate != null ? delegate.available() : 0; + } + private InputStream getDelegate() { attachmentStatusValidator.verifyStatus(status); From d71cae32ab781f095d10d4c99450ee663cc34926 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 13 Jan 2026 10:35:36 +0100 Subject: [PATCH 06/15] status quo --- .../helper/ExtendedErrorStatuses.java | 47 +++++++ .../readhelper/CountingInputStream.java | 129 +++++++++++++++++- .../_i18n/message.properties | 1 + 3 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java create mode 100644 cds-feature-attachments/src/main/resources/cds/com.sap.cds/cds-feature-attachments/_i18n/message.properties diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java new file mode 100644 index 000000000..0c035b669 --- /dev/null +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java @@ -0,0 +1,47 @@ +package com.sap.cds.feature.attachments.handler.applicationservice.helper; + +import com.sap.cds.services.ErrorStatus; +import com.sap.cds.services.ErrorStatuses; + +public enum ExtendedErrorStatuses implements ErrorStatus { + + CONTENT_TOO_LARGE(413, "The content size exceeds the maximum allowed limit.", 413); + + private final int code; + private final String description; + private final int httpStatus; + + private ExtendedErrorStatuses(int code, String description, int httpStatus) { + this.code = code; + this.description = description; + this.httpStatus = httpStatus; + } + + @Override + public String getDescription() { + return description; + } + + @Override + public int getHttpStatus() { + return httpStatus; + } + + /** + * @param code the code + * @return the ErrorStatus from this enum, associated with the given code or {@code null} + */ + public static ErrorStatus getByCode(int code) { + for (ErrorStatus errorStatus : ErrorStatuses.values()) { + if (errorStatus.getHttpStatus() == code) { + return errorStatus; + } + } + return null; + } + + @Override + public String getCodeString() { + return Integer.toString(code); + } +} diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java index 6a03bf699..72fc6565d 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java @@ -1,5 +1,128 @@ +/* + * © 2024-2025 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; -public class CountingInputStream { - -} +import java.io.IOException; +import java.io.InputStream; + +import com.sap.cds.feature.attachments.handler.applicationservice.helper.ExtendedErrorStatuses; +import com.sap.cds.services.ErrorStatuses; +import com.sap.cds.services.ServiceException; +import com.sap.cds.services.utils.CdsErrorStatuses; + +/** + * An InputStream wrapper that tracks bytes read and enforces a maximum size + * limit. + * This allows for memory-efficient streaming validation without buffering + * entire files. + */ +public class CountingInputStream extends InputStream { + + private final InputStream delegate; + private final long maxBytes; + private long bytesRead = 0; + + /** + * Creates a CountingInputStream that enforces a maximum size limit. + * + * @param delegate the underlying InputStream to wrap + * @param maxBytes the maximum number of bytes allowed to be read + * @throws IllegalArgumentException if maxBytes is negative + */ + public CountingInputStream(InputStream delegate, long maxBytes) { + if (maxBytes < 0) { + throw new IllegalArgumentException("maxBytes must be non-negative"); + } + this.delegate = delegate; + this.maxBytes = maxBytes; + } + + @Override + public int read() throws IOException { + int b = delegate.read(); + if (b != -1) { + checkLimit(1); + } + return b; + } + + @Override + public int read(byte[] b) throws IOException { + int result = delegate.read(b); + if (result > 0) { + checkLimit(result); + } + return result; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + int result = delegate.read(b, off, len); + if (result > 0) { + checkLimit(result); + } + return result; + } + + @Override + public long skip(long n) throws IOException { + long skipped = delegate.skip(n); + if (skipped > 0) { + checkLimit(skipped); + } + return skipped; + } + + @Override + public int available() throws IOException { + return delegate.available(); + } + + @Override + public void close() throws IOException { + if (delegate != null) { + delegate.close(); + } + } + + @Override + public void mark(int readlimit) { + delegate.mark(readlimit); + } + + @Override + public void reset() throws IOException { + delegate.reset(); + } + + @Override + public boolean markSupported() { + return delegate.markSupported(); + } + + /** + * Returns the number of bytes that have been read so far. + * + * @return the total bytes read + */ + public long getBytesRead() { + return bytesRead; + } + + /** + * Checks if adding the specified number of bytes would exceed the limit. + * Increments the bytesRead counter and throws an exception if limit is + * exceeded. + * + * @param bytes the number of bytes being read + * @throws ServiceException if reading these bytes would exceed maxBytes + */ + private void checkLimit(long bytes) { + bytesRead += bytes; + if (bytesRead > maxBytes) { + throw new ServiceException(ExtendedErrorStatuses.CONTENT_TOO_LARGE, + "File size exceeds the limit of {} bytes", maxBytes); + } + } +} \ No newline at end of file diff --git a/cds-feature-attachments/src/main/resources/cds/com.sap.cds/cds-feature-attachments/_i18n/message.properties b/cds-feature-attachments/src/main/resources/cds/com.sap.cds/cds-feature-attachments/_i18n/message.properties new file mode 100644 index 000000000..e9af11c93 --- /dev/null +++ b/cds-feature-attachments/src/main/resources/cds/com.sap.cds/cds-feature-attachments/_i18n/message.properties @@ -0,0 +1 @@ +AttachmentSizeExceeded = File size exceeds the limit of {0}. \ No newline at end of file From 8b400806cfc728656eeaafd7d4cd6554aa3bd239 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 13 Jan 2026 14:44:15 +0100 Subject: [PATCH 07/15] Refactor attachment validation and size handling; add message properties for size limits --- .../UpdateAttachmentsHandler.java | 52 +--------- .../ModifyApplicationHandlerHelper.java | 94 ++++++++++++++++--- .../readhelper/CountingInputStream.java | 9 +- ...message.properties => messages.properties} | 0 samples/bookshop/srv/attachments.cds | 1 - 5 files changed, 85 insertions(+), 71 deletions(-) rename cds-feature-attachments/src/main/resources/{cds/com.sap.cds/cds-feature-attachments/_i18n/message.properties => messages.properties} (100%) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index e79bcbd46..b340804d4 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -5,14 +5,10 @@ import static java.util.Objects.requireNonNull; -import java.io.IOException; - import com.sap.cds.CdsData; import com.sap.cds.CdsDataProcessor; -import com.sap.cds.CdsDataProcessor.Filter; import com.sap.cds.CdsDataProcessor.Validator; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; -import com.sap.cds.feature.attachments.handler.applicationservice.helper.FileSizeUtils; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ModifyApplicationHandlerHelper; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ReadonlyDataContextEnhancer; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ThreadDataStorageReader; @@ -28,13 +24,11 @@ import com.sap.cds.services.handler.annotations.Before; import com.sap.cds.services.handler.annotations.HandlerOrder; import com.sap.cds.services.handler.annotations.ServiceName; +import com.sap.cds.services.persistence.PersistenceService; import com.sap.cds.services.request.UserInfo; import com.sap.cds.services.utils.OrderConstants; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,9 +44,6 @@ public class UpdateAttachmentsHandler implements EventHandler { private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); - private static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") - && element.findAnnotation("Validation.Maximum") - .isPresent(); private final ModifyAttachmentEventFactory eventFactory; private final AttachmentsReader attachmentsReader; @@ -83,32 +74,12 @@ void processBeforeForDraft(CdsUpdateEventContext context, List data) { @Before @HandlerOrder(HandlerOrder.LATE) void processBefore(CdsUpdateEventContext context, List data) { + CdsEntity target = context.getTarget(); boolean associationsAreUnchanged = associationsAreUnchanged(target, data); - int contentLength = Integer.parseInt(context.getParameterInfo().getHeader("Content-Length")); if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) { - // Check here for size of new attachments List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); - if (containsValMaxAnnotation(target, data)) { - try { - long maxSizeValue = FileSizeUtils.parseFileSizeToBytes(getValMaxValue(target, data)); - logger.debug("Validation.Maximum annotation found with value: {}", maxSizeValue); - attachments.forEach(attachment -> { - try { - int size = attachment.getContent().available(); - if (size > maxSizeValue) { - throw new IllegalArgumentException("Attachment " + attachment.getFileName() - + " exceeds the maximum allowed size of " + maxSizeValue + " bytes."); - } - } catch (IOException e) { - throw new RuntimeException("Failed to read attachment content size", e); - } - }); - } catch (ArithmeticException e) { - throw new IllegalArgumentException("Maximum file size value is too large", e); - } - } logger.debug("Processing before {} event for entity {}", context.getEvent(), target); ModifyApplicationHandlerHelper.handleAttachmentForEntities( @@ -120,25 +91,6 @@ void processBefore(CdsUpdateEventContext context, List data) { } } - private String getValMaxValue(CdsEntity entity, List data) { - AtomicReference annotationValue = new AtomicReference<>(); - CdsDataProcessor.create() - .addValidator(VALMAX_FILTER, (path, element, value) -> { - element.findAnnotation("Validation.Maximum") - .ifPresent(annotation -> annotationValue.set(annotation.getValue().toString())); - }) - .process(data, entity); - return annotationValue.get(); - } - - private boolean containsValMaxAnnotation(CdsEntity entity, List data) { - AtomicBoolean isIncluded = new AtomicBoolean(); - CdsDataProcessor.create() - .addValidator(VALMAX_FILTER, (path, element, value) -> isIncluded.set(true)) - .process(data, entity); - return isIncluded.get(); - } - private boolean associationsAreUnchanged(CdsEntity entity, List data) { // TODO: check if this should be replaced with // entity.assocations().noneMatch(...) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java index 8bf509708..846d8afba 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java @@ -6,27 +6,43 @@ import com.sap.cds.CdsData; import com.sap.cds.CdsDataProcessor; import com.sap.cds.CdsDataProcessor.Converter; +import com.sap.cds.CdsDataProcessor.Filter; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; import com.sap.cds.feature.attachments.handler.applicationservice.modifyevents.ModifyAttachmentEvent; import com.sap.cds.feature.attachments.handler.applicationservice.modifyevents.ModifyAttachmentEventFactory; +import com.sap.cds.feature.attachments.handler.applicationservice.readhelper.CountingInputStream; import com.sap.cds.feature.attachments.handler.common.ApplicationHandlerHelper; import com.sap.cds.ql.cqn.Path; import com.sap.cds.reflect.CdsEntity; +import com.sap.cds.services.ErrorStatuses; import com.sap.cds.services.EventContext; +import com.sap.cds.services.ServiceException; + +import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; +import java.text.MessageFormat; import java.util.List; import java.util.Map; +import java.util.ResourceBundle; +import java.util.concurrent.atomic.AtomicReference; public final class ModifyApplicationHandlerHelper { + private static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") + && element.findAnnotation("Validation.Maximum") + .isPresent(); + /** * Handles attachments for entities. * - * @param entity the {@link CdsEntity entity} to handle attachments for - * @param data the given list of {@link CdsData data} + * @param entity the {@link CdsEntity entity} to handle attachments + * for + * @param data the given list of {@link CdsData data} * @param existingAttachments the given list of existing {@link CdsData data} - * @param eventFactory the {@link ModifyAttachmentEventFactory} to create the corresponding event - * @param eventContext the current {@link EventContext} + * @param eventFactory the {@link ModifyAttachmentEventFactory} to create + * the corresponding event + * @param eventContext the current {@link EventContext} */ public static void handleAttachmentForEntities( CdsEntity entity, @@ -34,10 +50,8 @@ public static void handleAttachmentForEntities( List existingAttachments, ModifyAttachmentEventFactory eventFactory, EventContext eventContext) { - Converter converter = - (path, element, value) -> - handleAttachmentForEntity( - existingAttachments, eventFactory, eventContext, path, (InputStream) value); + Converter converter = (path, element, value) -> handleAttachmentForEntity( + existingAttachments, eventFactory, eventContext, path, (InputStream) value); CdsDataProcessor.create() .addConverter(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, converter) @@ -47,11 +61,13 @@ public static void handleAttachmentForEntities( /** * Handles attachments for a single entity. * - * @param existingAttachments the list of existing {@link Attachments} to check against - * @param eventFactory the {@link ModifyAttachmentEventFactory} to create the corresponding event - * @param eventContext the current {@link EventContext} - * @param path the {@link Path} of the attachment - * @param content the content of the attachment + * @param existingAttachments the list of existing {@link Attachments} to check + * against + * @param eventFactory the {@link ModifyAttachmentEventFactory} to create + * the corresponding event + * @param eventContext the current {@link EventContext} + * @param path the {@link Path} of the attachment + * @param content the content of the attachment * @return the processed content as an {@link InputStream} */ public static InputStream handleAttachmentForEntity( @@ -64,12 +80,60 @@ public static InputStream handleAttachmentForEntity( ReadonlyDataContextEnhancer.restoreReadonlyFields((CdsData) path.target().values()); Attachments attachment = getExistingAttachment(keys, existingAttachments); String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); + String contentLength = eventContext.getParameterInfo().getHeader("Content-Length"); + // Wrap the stream with CountingInputStream if a max size is configured + InputStream wrappedContent = wrapWithCountingStream(content, path.target().entity(), existingAttachments, contentLength); // for the current request find the event to process - ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(content, contentId, attachment); + ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(wrappedContent, contentId, attachment); // process the event - return eventToProcess.processEvent(path, content, attachment, eventContext); + try { + return eventToProcess.processEvent(path, wrappedContent, attachment, eventContext); + } catch (UncheckedIOException e) { + // Extract the IOException with our custom message + IOException cause = e.getCause(); + throw new ServiceException(ErrorStatuses.BAD_REQUEST, cause.getMessage()); + } catch (RuntimeException e) { + // Check if nested UncheckedIOException + Throwable cause = e.getCause(); + if (cause instanceof UncheckedIOException) { + IOException ioCause = ((UncheckedIOException) cause).getCause(); + throw new ServiceException(ErrorStatuses.BAD_REQUEST, ioCause.getMessage()); + } + throw e; + } + } + + private static InputStream wrapWithCountingStream(InputStream content, CdsEntity entity, + List data, String contentLength) { + String maxSizeStr = getValMaxValue(entity, data); + + if (maxSizeStr != null && content != null) { + try { + long maxSize = FileSizeUtils.parseFileSizeToBytes(maxSizeStr); + if (contentLength != null && Long.parseLong(contentLength) > maxSize) { + throw new RuntimeException("File size exceeds the maximum allowed size of " + maxSizeStr); + } + return new CountingInputStream(content, maxSize); + } catch (ArithmeticException e) { + throw new ServiceException("Maximum file size value is too large", e); + } catch (RuntimeException e) { + throw new ServiceException(ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); + } + } + return content; + } + + private static String getValMaxValue(CdsEntity entity, List data) { + AtomicReference annotationValue = new AtomicReference<>(); + CdsDataProcessor.create() + .addValidator(VALMAX_FILTER, (path, element, value) -> { + element.findAnnotation("Validation.Maximum") + .ifPresent(annotation -> annotationValue.set(annotation.getValue().toString())); + }) + .process(data, entity); + return annotationValue.get(); } private static Attachments getExistingAttachment( diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java index 72fc6565d..285ea4874 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java @@ -5,11 +5,9 @@ import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; -import com.sap.cds.feature.attachments.handler.applicationservice.helper.ExtendedErrorStatuses; -import com.sap.cds.services.ErrorStatuses; import com.sap.cds.services.ServiceException; -import com.sap.cds.services.utils.CdsErrorStatuses; /** * An InputStream wrapper that tracks bytes read and enforces a maximum size @@ -121,8 +119,9 @@ public long getBytesRead() { private void checkLimit(long bytes) { bytesRead += bytes; if (bytesRead > maxBytes) { - throw new ServiceException(ExtendedErrorStatuses.CONTENT_TOO_LARGE, - "File size exceeds the limit of {} bytes", maxBytes); + throw new UncheckedIOException( + new IOException(String.format("File size exceeds the limit of %d bytes", maxBytes)) + ); } } } \ No newline at end of file diff --git a/cds-feature-attachments/src/main/resources/cds/com.sap.cds/cds-feature-attachments/_i18n/message.properties b/cds-feature-attachments/src/main/resources/messages.properties similarity index 100% rename from cds-feature-attachments/src/main/resources/cds/com.sap.cds/cds-feature-attachments/_i18n/message.properties rename to cds-feature-attachments/src/main/resources/messages.properties diff --git a/samples/bookshop/srv/attachments.cds b/samples/bookshop/srv/attachments.cds index 44178bf73..07381ba78 100644 --- a/samples/bookshop/srv/attachments.cds +++ b/samples/bookshop/srv/attachments.cds @@ -11,7 +11,6 @@ annotate my.Books.attachments with { content @Validation.Maximum: '20MB'; } - // Add UI component for attachments table to the Browse Books App using {CatalogService as service} from '../app/services'; From 149aa2b203253d82e223d74278fe9f13f166fdba Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 13 Jan 2026 16:10:14 +0100 Subject: [PATCH 08/15] formatting and stuff --- .../UpdateAttachmentsHandler.java | 40 +++++---- .../helper/ExtendedErrorStatuses.java | 10 ++- .../helper/FileSizeUtils.java | 82 ++++++++++--------- .../ModifyApplicationHandlerHelper.java | 62 +++++++------- .../readhelper/CountingInputStream.java | 21 ++--- .../CreateAttachmentsHandlerTest.java | 4 + .../UpdateAttachmentsHandlerTest.java | 4 + .../DraftPatchAttachmentsHandlerTest.java | 3 + 8 files changed, 119 insertions(+), 107 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index b340804d4..120e94148 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -24,7 +24,6 @@ import com.sap.cds.services.handler.annotations.Before; import com.sap.cds.services.handler.annotations.HandlerOrder; import com.sap.cds.services.handler.annotations.ServiceName; -import com.sap.cds.services.persistence.PersistenceService; import com.sap.cds.services.request.UserInfo; import com.sap.cds.services.utils.OrderConstants; import java.util.List; @@ -33,12 +32,9 @@ import org.slf4j.LoggerFactory; /** - * The class {@link UpdateAttachmentsHandler} is an event handler that is called - * before an update - * event is executed. As updates in draft entities or non-draft entities can - * also be create-events, - * update-events or delete-events the handler needs to distinguish between the - * different cases. + * The class {@link UpdateAttachmentsHandler} is an event handler that is called before an update + * event is executed. As updates in draft entities or non-draft entities can also be create-events, + * update-events or delete-events the handler needs to distinguish between the different cases. */ @ServiceName(value = "*", type = ApplicationService.class) public class UpdateAttachmentsHandler implements EventHandler { @@ -46,7 +42,6 @@ public class UpdateAttachmentsHandler implements EventHandler { private static final Logger logger = LoggerFactory.getLogger(UpdateAttachmentsHandler.class); private final ModifyAttachmentEventFactory eventFactory; - private final AttachmentsReader attachmentsReader; private final AttachmentService attachmentService; private final ThreadDataStorageReader storageReader; @@ -56,8 +51,8 @@ public UpdateAttachmentsHandler( AttachmentService attachmentService, ThreadDataStorageReader storageReader) { this.eventFactory = requireNonNull(eventFactory, "eventFactory must not be null"); - this.attachmentsReader = requireNonNull(attachmentsReader, "attachmentsReader must not be null"); - this.attachmentService = requireNonNull(attachmentService, "attachmentService must not be null"); + this.attachmentService = + requireNonNull(attachmentService, "attachmentService must not be null"); this.storageReader = requireNonNull(storageReader, "storageReader must not be null"); } @@ -105,18 +100,21 @@ private void deleteRemovedAttachments( List data, CdsEntity entity, UserInfo userInfo) { - List condensedAttachments = ApplicationHandlerHelper.condenseAttachments(data, entity); + List condensedAttachments = + ApplicationHandlerHelper.condenseAttachments(data, entity); - Validator validator = (path, element, value) -> { - Map keys = ApplicationHandlerHelper.removeDraftKey(path.target().keys()); - boolean entryExists = condensedAttachments.stream() - .anyMatch( - updatedData -> ApplicationHandlerHelper.areKeysInData(keys, updatedData)); - if (!entryExists) { - String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); - attachmentService.markAttachmentAsDeleted(new MarkAsDeletedInput(contentId, userInfo)); - } - }; + Validator validator = + (path, element, value) -> { + Map keys = ApplicationHandlerHelper.removeDraftKey(path.target().keys()); + boolean entryExists = + condensedAttachments.stream() + .anyMatch( + updatedData -> ApplicationHandlerHelper.areKeysInData(keys, updatedData)); + if (!entryExists) { + String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); + attachmentService.markAttachmentAsDeleted(new MarkAsDeletedInput(contentId, userInfo)); + } + }; CdsDataProcessor.create() .addValidator(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, validator) .process(existingAttachments, entity); diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java index 0c035b669..6a5bcf5ce 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java @@ -1,17 +1,19 @@ +/* + * © 2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ package com.sap.cds.feature.attachments.handler.applicationservice.helper; import com.sap.cds.services.ErrorStatus; import com.sap.cds.services.ErrorStatuses; public enum ExtendedErrorStatuses implements ErrorStatus { + CONTENT_TOO_LARGE(413, "The content size exceeds the maximum allowed limit.", 413); - CONTENT_TOO_LARGE(413, "The content size exceeds the maximum allowed limit.", 413); - - private final int code; + private final int code; private final String description; private final int httpStatus; - private ExtendedErrorStatuses(int code, String description, int httpStatus) { + ExtendedErrorStatuses(int code, String description, int httpStatus) { this.code = code; this.description = description; this.httpStatus = httpStatus; diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java index 537396f68..4884322a5 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java @@ -1,3 +1,6 @@ +/* + * © 2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ package com.sap.cds.feature.attachments.handler.applicationservice.helper; import java.math.BigDecimal; @@ -6,44 +9,45 @@ import java.util.regex.Pattern; public class FileSizeUtils { - private static final Pattern SIZE = Pattern.compile("^\\s*([0-9]+(?:\\.[0-9]+)?)\\s*([a-zA-Z]*)\\s*$"); - private static final Map MULTIPLIER = Map.ofEntries( - Map.entry("", 1L), - Map.entry("B", 1L), - - // Decimal - Map.entry("KB", 1000L), - Map.entry("MB", 1000L * 1000), - Map.entry("GB", 1000L * 1000 * 1000), - Map.entry("TB", 1000L * 1000 * 1000 * 1000), - - // Binary - Map.entry("KIB", 1024L), - Map.entry("MIB", 1024L * 1024), - Map.entry("GIB", 1024L * 1024 * 1024), - Map.entry("TIB", 1024L * 1024 * 1024 * 1024)); - - private FileSizeUtils() {} - - public static long parseFileSizeToBytes(String input) { - // First validate string - if (input == null) - throw new IllegalArgumentException("Value for Max File Size is null"); - - Matcher m = SIZE.matcher(input); - if (!m.matches()) { - throw new IllegalArgumentException("Invalid size: " + input); - } - BigDecimal value = new BigDecimal(m.group(1)); - String unitRaw = m.group(2) == null ? "" : m.group(2); - String unit = unitRaw.toUpperCase(); - - // if (unit.length() == 1) unit = unit + "B"; // for people using K instead of KB - Long mul = MULTIPLIER.get(unit); - if (mul == null) { - throw new IllegalArgumentException("Unknown Unit: " + unitRaw); - } - BigDecimal bytes = value.multiply(BigDecimal.valueOf(mul)); - return bytes.longValueExact(); + private static final Pattern SIZE = + Pattern.compile("^\\s*([0-9]+(?:\\.[0-9]+)?)\\s*([a-zA-Z]*)\\s*$"); + private static final Map MULTIPLIER = + Map.ofEntries( + Map.entry("", 1L), + Map.entry("B", 1L), + + // Decimal + Map.entry("KB", 1000L), + Map.entry("MB", 1000L * 1000), + Map.entry("GB", 1000L * 1000 * 1000), + Map.entry("TB", 1000L * 1000 * 1000 * 1000), + + // Binary + Map.entry("KIB", 1024L), + Map.entry("MIB", 1024L * 1024), + Map.entry("GIB", 1024L * 1024 * 1024), + Map.entry("TIB", 1024L * 1024 * 1024 * 1024)); + + private FileSizeUtils() {} + + public static long parseFileSizeToBytes(String input) { + // First validate string + if (input == null) throw new IllegalArgumentException("Value for Max File Size is null"); + + Matcher m = SIZE.matcher(input); + if (!m.matches()) { + throw new IllegalArgumentException("Invalid size: " + input); } + BigDecimal value = new BigDecimal(m.group(1)); + String unitRaw = m.group(2) == null ? "" : m.group(2); + String unit = unitRaw.toUpperCase(); + + // if (unit.length() == 1) unit = unit + "B"; // for people using K instead of KB + Long mul = MULTIPLIER.get(unit); + if (mul == null) { + throw new IllegalArgumentException("Unknown Unit: " + unitRaw); + } + BigDecimal bytes = value.multiply(BigDecimal.valueOf(mul)); + return bytes.longValueExact(); + } } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java index 846d8afba..cf662490b 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java @@ -17,32 +17,28 @@ import com.sap.cds.services.ErrorStatuses; import com.sap.cds.services.EventContext; import com.sap.cds.services.ServiceException; - import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; -import java.text.MessageFormat; import java.util.List; import java.util.Map; -import java.util.ResourceBundle; import java.util.concurrent.atomic.AtomicReference; public final class ModifyApplicationHandlerHelper { - private static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") - && element.findAnnotation("Validation.Maximum") - .isPresent(); + private static final Filter VALMAX_FILTER = + (path, element, type) -> + element.getName().contentEquals("content") + && element.findAnnotation("Validation.Maximum").isPresent(); /** * Handles attachments for entities. * - * @param entity the {@link CdsEntity entity} to handle attachments - * for - * @param data the given list of {@link CdsData data} + * @param entity the {@link CdsEntity entity} to handle attachments for + * @param data the given list of {@link CdsData data} * @param existingAttachments the given list of existing {@link CdsData data} - * @param eventFactory the {@link ModifyAttachmentEventFactory} to create - * the corresponding event - * @param eventContext the current {@link EventContext} + * @param eventFactory the {@link ModifyAttachmentEventFactory} to create the corresponding event + * @param eventContext the current {@link EventContext} */ public static void handleAttachmentForEntities( CdsEntity entity, @@ -50,8 +46,10 @@ public static void handleAttachmentForEntities( List existingAttachments, ModifyAttachmentEventFactory eventFactory, EventContext eventContext) { - Converter converter = (path, element, value) -> handleAttachmentForEntity( - existingAttachments, eventFactory, eventContext, path, (InputStream) value); + Converter converter = + (path, element, value) -> + handleAttachmentForEntity( + existingAttachments, eventFactory, eventContext, path, (InputStream) value); CdsDataProcessor.create() .addConverter(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, converter) @@ -61,13 +59,11 @@ public static void handleAttachmentForEntities( /** * Handles attachments for a single entity. * - * @param existingAttachments the list of existing {@link Attachments} to check - * against - * @param eventFactory the {@link ModifyAttachmentEventFactory} to create - * the corresponding event - * @param eventContext the current {@link EventContext} - * @param path the {@link Path} of the attachment - * @param content the content of the attachment + * @param existingAttachments the list of existing {@link Attachments} to check against + * @param eventFactory the {@link ModifyAttachmentEventFactory} to create the corresponding event + * @param eventContext the current {@link EventContext} + * @param path the {@link Path} of the attachment + * @param content the content of the attachment * @return the processed content as an {@link InputStream} */ public static InputStream handleAttachmentForEntity( @@ -82,10 +78,12 @@ public static InputStream handleAttachmentForEntity( String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); String contentLength = eventContext.getParameterInfo().getHeader("Content-Length"); // Wrap the stream with CountingInputStream if a max size is configured - InputStream wrappedContent = wrapWithCountingStream(content, path.target().entity(), existingAttachments, contentLength); + InputStream wrappedContent = + wrapWithCountingStream(content, path.target().entity(), existingAttachments, contentLength); // for the current request find the event to process - ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(wrappedContent, contentId, attachment); + ModifyAttachmentEvent eventToProcess = + eventFactory.getEvent(wrappedContent, contentId, attachment); // process the event try { @@ -105,8 +103,8 @@ public static InputStream handleAttachmentForEntity( } } - private static InputStream wrapWithCountingStream(InputStream content, CdsEntity entity, - List data, String contentLength) { + private static InputStream wrapWithCountingStream( + InputStream content, CdsEntity entity, List data, String contentLength) { String maxSizeStr = getValMaxValue(entity, data); if (maxSizeStr != null && content != null) { @@ -119,7 +117,8 @@ private static InputStream wrapWithCountingStream(InputStream content, CdsEntity } catch (ArithmeticException e) { throw new ServiceException("Maximum file size value is too large", e); } catch (RuntimeException e) { - throw new ServiceException(ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); + throw new ServiceException( + ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); } } return content; @@ -128,10 +127,13 @@ private static InputStream wrapWithCountingStream(InputStream content, CdsEntity private static String getValMaxValue(CdsEntity entity, List data) { AtomicReference annotationValue = new AtomicReference<>(); CdsDataProcessor.create() - .addValidator(VALMAX_FILTER, (path, element, value) -> { - element.findAnnotation("Validation.Maximum") - .ifPresent(annotation -> annotationValue.set(annotation.getValue().toString())); - }) + .addValidator( + VALMAX_FILTER, + (path, element, value) -> { + element + .findAnnotation("Validation.Maximum") + .ifPresent(annotation -> annotationValue.set(annotation.getValue().toString())); + }) .process(data, entity); return annotationValue.get(); } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java index 285ea4874..0159da5ad 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java @@ -3,19 +3,16 @@ */ package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; +import com.sap.cds.services.ServiceException; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; -import com.sap.cds.services.ServiceException; - /** - * An InputStream wrapper that tracks bytes read and enforces a maximum size - * limit. - * This allows for memory-efficient streaming validation without buffering - * entire files. + * An InputStream wrapper that tracks bytes read and enforces a maximum size limit. This allows for + * memory-efficient streaming validation without buffering entire files. */ -public class CountingInputStream extends InputStream { +public final class CountingInputStream extends InputStream { private final InputStream delegate; private final long maxBytes; @@ -109,9 +106,8 @@ public long getBytesRead() { } /** - * Checks if adding the specified number of bytes would exceed the limit. - * Increments the bytesRead counter and throws an exception if limit is - * exceeded. + * Checks if adding the specified number of bytes would exceed the limit. Increments the bytesRead + * counter and throws an exception if limit is exceeded. * * @param bytes the number of bytes being read * @throws ServiceException if reading these bytes would exceed maxBytes @@ -120,8 +116,7 @@ private void checkLimit(long bytes) { bytesRead += bytes; if (bytesRead > maxBytes) { throw new UncheckedIOException( - new IOException(String.format("File size exceeds the limit of %d bytes", maxBytes)) - ); + new IOException(String.format("File size exceeds the limit of %d bytes", maxBytes))); } } -} \ No newline at end of file +} diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/CreateAttachmentsHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/CreateAttachmentsHandlerTest.java index a02e0f583..534206126 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/CreateAttachmentsHandlerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/CreateAttachmentsHandlerTest.java @@ -32,6 +32,7 @@ import com.sap.cds.services.handler.annotations.Before; import com.sap.cds.services.handler.annotations.HandlerOrder; import com.sap.cds.services.handler.annotations.ServiceName; +import com.sap.cds.services.request.ParameterInfo; import com.sap.cds.services.runtime.CdsRuntime; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -67,6 +68,9 @@ void setup() { createContext = mock(CdsCreateEventContext.class); event = mock(ModifyAttachmentEvent.class); + + ParameterInfo parameterInfo = mock(ParameterInfo.class); + when(createContext.getParameterInfo()).thenReturn(parameterInfo); } @Test diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java index 7c40fedc2..514d968a8 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java @@ -38,6 +38,7 @@ import com.sap.cds.services.handler.annotations.Before; import com.sap.cds.services.handler.annotations.HandlerOrder; import com.sap.cds.services.handler.annotations.ServiceName; +import com.sap.cds.services.request.ParameterInfo; import com.sap.cds.services.request.UserInfo; import com.sap.cds.services.runtime.CdsRuntime; import java.io.InputStream; @@ -89,6 +90,9 @@ void setup() { selectCaptor = ArgumentCaptor.forClass(CqnSelect.class); when(eventFactory.getEvent(any(), any(), any())).thenReturn(event); userInfo = mock(UserInfo.class); + + ParameterInfo parameterInfo = mock(ParameterInfo.class); + when(updateContext.getParameterInfo()).thenReturn(parameterInfo); } @Test diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/draftservice/DraftPatchAttachmentsHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/draftservice/DraftPatchAttachmentsHandlerTest.java index fe203e9ac..b37f8879d 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/draftservice/DraftPatchAttachmentsHandlerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/draftservice/DraftPatchAttachmentsHandlerTest.java @@ -29,6 +29,7 @@ import com.sap.cds.services.handler.annotations.HandlerOrder; import com.sap.cds.services.handler.annotations.ServiceName; import com.sap.cds.services.persistence.PersistenceService; +import com.sap.cds.services.request.ParameterInfo; import com.sap.cds.services.runtime.CdsRuntime; import java.io.InputStream; import java.util.List; @@ -63,6 +64,8 @@ void setup() { event = mock(ModifyAttachmentEvent.class); when(eventFactory.getEvent(any(), any(), any())).thenReturn(event); selectCaptor = ArgumentCaptor.forClass(CqnSelect.class); + ParameterInfo parameterInfo = mock(ParameterInfo.class); + when(eventContext.getParameterInfo()).thenReturn(parameterInfo); } @Test From 3a2165fefe8415d37acaa2e3f5231c8c8267b37a Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 13 Jan 2026 17:18:55 +0100 Subject: [PATCH 09/15] fix tests --- .../UpdateAttachmentsHandler.java | 105 +++++++++++++----- .../common/ApplicationHandlerHelper.java | 53 ++++++--- .../UpdateAttachmentsHandlerTest.java | 63 ++++++----- 3 files changed, 152 insertions(+), 69 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index 120e94148..cbd43a57a 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -5,9 +5,9 @@ import static java.util.Objects.requireNonNull; +import java.util.HashMap; + import com.sap.cds.CdsData; -import com.sap.cds.CdsDataProcessor; -import com.sap.cds.CdsDataProcessor.Validator; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ModifyApplicationHandlerHelper; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ReadonlyDataContextEnhancer; @@ -17,7 +17,10 @@ import com.sap.cds.feature.attachments.handler.common.AttachmentsReader; import com.sap.cds.feature.attachments.service.AttachmentService; import com.sap.cds.feature.attachments.service.model.service.MarkAsDeletedInput; +import com.sap.cds.ql.cqn.CqnSelect; import com.sap.cds.reflect.CdsEntity; +import com.sap.cds.services.ErrorStatuses; +import com.sap.cds.services.ServiceException; import com.sap.cds.services.cds.ApplicationService; import com.sap.cds.services.cds.CdsUpdateEventContext; import com.sap.cds.services.handler.EventHandler; @@ -26,15 +29,20 @@ import com.sap.cds.services.handler.annotations.ServiceName; import com.sap.cds.services.request.UserInfo; import com.sap.cds.services.utils.OrderConstants; +import com.sap.cds.services.utils.model.CqnUtils; + import java.util.List; import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * The class {@link UpdateAttachmentsHandler} is an event handler that is called before an update - * event is executed. As updates in draft entities or non-draft entities can also be create-events, - * update-events or delete-events the handler needs to distinguish between the different cases. + * The class {@link UpdateAttachmentsHandler} is an event handler that is called + * before an update + * event is executed. As updates in draft entities or non-draft entities can + * also be create-events, + * update-events or delete-events the handler needs to distinguish between the + * different cases. */ @ServiceName(value = "*", type = ApplicationService.class) public class UpdateAttachmentsHandler implements EventHandler { @@ -44,6 +52,7 @@ public class UpdateAttachmentsHandler implements EventHandler { private final ModifyAttachmentEventFactory eventFactory; private final AttachmentService attachmentService; private final ThreadDataStorageReader storageReader; + private final AttachmentsReader attachmentsReader; public UpdateAttachmentsHandler( ModifyAttachmentEventFactory eventFactory, @@ -51,9 +60,9 @@ public UpdateAttachmentsHandler( AttachmentService attachmentService, ThreadDataStorageReader storageReader) { this.eventFactory = requireNonNull(eventFactory, "eventFactory must not be null"); - this.attachmentService = - requireNonNull(attachmentService, "attachmentService must not be null"); + this.attachmentService = requireNonNull(attachmentService, "attachmentService must not be null"); this.storageReader = requireNonNull(storageReader, "storageReader must not be null"); + this.attachmentsReader = requireNonNull(attachmentsReader, "attachmentsReader must not be null"); } @Before @@ -76,12 +85,19 @@ void processBefore(CdsUpdateEventContext context, List data) { if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) { List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); + // Query database only for validation (single query for all attachments) + CqnSelect select = CqnUtils.toSelect(context.getCqn(), context.getTarget()); + List dbAttachments = attachmentsReader.readAttachments(context.getModel(), target, select); + + // Validate: ensure request attachments exist in DB or are new (no ID yet) + validateAttachments(attachments, dbAttachments, target); + logger.debug("Processing before {} event for entity {}", context.getEvent(), target); ModifyApplicationHandlerHelper.handleAttachmentForEntities( target, data, attachments, eventFactory, context); if (!associationsAreUnchanged) { - deleteRemovedAttachments(attachments, data, target, context.getUserInfo()); + deleteRemovedAttachments(dbAttachments, data, target, context.getUserInfo()); } } } @@ -96,27 +112,60 @@ private boolean associationsAreUnchanged(CdsEntity entity, List data) { } private void deleteRemovedAttachments( - List existingAttachments, - List data, + List dbAttachments, + List requestData, CdsEntity entity, UserInfo userInfo) { - List condensedAttachments = - ApplicationHandlerHelper.condenseAttachments(data, entity); - - Validator validator = - (path, element, value) -> { - Map keys = ApplicationHandlerHelper.removeDraftKey(path.target().keys()); - boolean entryExists = - condensedAttachments.stream() - .anyMatch( - updatedData -> ApplicationHandlerHelper.areKeysInData(keys, updatedData)); - if (!entryExists) { - String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); - attachmentService.markAttachmentAsDeleted(new MarkAsDeletedInput(contentId, userInfo)); - } - }; - CdsDataProcessor.create() - .addValidator(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, validator) - .process(existingAttachments, entity); + List requestAttachments = ApplicationHandlerHelper.condenseAttachments(requestData, entity); + + for (Attachments dbAttachment : dbAttachments) { + Map dbKeys = new HashMap<>(); + entity.keyElements().forEach(keyElement -> { + String keyName = keyElement.getName(); + Object value = dbAttachment.get(keyName); + if (value != null) { + dbKeys.put(keyName, value); + } + }); + Map keys = ApplicationHandlerHelper.removeDraftKey(dbKeys); + + boolean existsInRequest = requestAttachments.stream() + .anyMatch(requestAttachment -> ApplicationHandlerHelper.areKeysInData(keys, requestAttachment)); + + if (!existsInRequest && dbAttachment.getContentId() != null) { + attachmentService.markAttachmentAsDeleted(new MarkAsDeletedInput(dbAttachment.getContentId(), userInfo)); + } + } + } + + private void validateAttachments(List requestAttachments, List dbAttachments, + CdsEntity target) { + for (Attachments requestAttachment : requestAttachments) { + // Build a map of keys from the attachment based on entity key elements + Map keysMap = new HashMap<>(); + target.keyElements().forEach(keyElement -> { + String keyName = keyElement.getName(); + Object value = requestAttachment.get(keyName); + if (value != null) { + keysMap.put(keyName, value); + } + }); + + Map keys = ApplicationHandlerHelper.removeDraftKey(keysMap); + + // Skip validation if attachment has no keys (new attachment being created) + if (keys.isEmpty()) { + continue; + } + + // If attachment has keys, verify it exists in the database + boolean existsInDb = dbAttachments.stream() + .anyMatch(db -> ApplicationHandlerHelper.areKeysInData(keys, db)); + + if (!existsInDb) { + throw new ServiceException(ErrorStatuses.NOT_FOUND, + "Attachment with keys " + keys + " not found"); + } + } } } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java index 97fbca93a..67082fed6 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java @@ -20,7 +20,8 @@ import java.util.concurrent.atomic.AtomicBoolean; /** - * The class {@link ApplicationHandlerHelper} provides helper methods for the attachment application + * The class {@link ApplicationHandlerHelper} provides helper methods for the + * attachment application * handlers. */ public final class ApplicationHandlerHelper { @@ -28,20 +29,21 @@ public final class ApplicationHandlerHelper { private static final String ANNOTATION_CORE_MEDIA_TYPE = "Core.MediaType"; /** - * A filter for media content fields. The filter checks if the entity is a media entity and if the + * A filter for media content fields. The filter checks if the entity is a media + * entity and if the * element has the annotation "Core.MediaType". */ - public static final Filter MEDIA_CONTENT_FILTER = - (path, element, type) -> - isMediaEntity(path.target().type()) - && element.findAnnotation(ANNOTATION_CORE_MEDIA_TYPE).isPresent(); + public static final Filter MEDIA_CONTENT_FILTER = (path, element, type) -> isMediaEntity(path.target().type()) + && element.findAnnotation(ANNOTATION_CORE_MEDIA_TYPE).isPresent(); /** * Checks if the data contains a content field. * - * @param entity The {@link CdsEntity entity} type of the given the data to check - * @param data The data to check - * @return true if the data contains a content field, false otherwise + * @param entity The {@link CdsEntity entity} type of the given the data to + * check + * @param data The data to check + * @return true if the data contains a content field, + * false otherwise */ public static boolean containsContentField(CdsEntity entity, List data) { AtomicBoolean isIncluded = new AtomicBoolean(); @@ -52,20 +54,42 @@ public static boolean containsContentField(CdsEntity entity, Listtrue if the entity is a media entity, false otherwise + * @return true if the entity is a media entity, false + * otherwise */ public static boolean isMediaEntity(CdsStructuredType baseEntity) { return baseEntity.getAnnotationValue(ANNOTATION_IS_MEDIA_DATA, false); } /** - * Condenses the attachments from the given data into a list of {@link Attachments attachments}. + * Extracts key fields from CdsData based on the entity definition. * - * @param data the list of {@link CdsData} to process + * @param data The CdsData to extract keys from + * @param entity The entity definition + * @return A map of key fields and their values + */ + public static Map extractKeys(CdsData data, CdsEntity entity) { + Map keys = new HashMap<>(); + entity.keyElements().forEach(keyElement -> { + String keyName = keyElement.getName(); + Object value = data.get(keyName); + if (value != null) { + keys.put(keyName, value); + } + }); + return keys; + } + + /** + * Condenses the attachments from the given data into a list of + * {@link Attachments attachments}. + * + * @param data the list of {@link CdsData} to process * @param entity the {@link CdsEntity entity} type of the given data * @return a list of {@link Attachments attachments} condensed from the data */ @@ -73,8 +97,7 @@ public static List condenseAttachments( List data, CdsEntity entity) { List resultList = new ArrayList<>(); - Validator validator = - (path, element, value) -> resultList.add(Attachments.of(path.target().values())); + Validator validator = (path, element, value) -> resultList.add(Attachments.of(path.target().values())); CdsDataProcessor.create().addValidator(MEDIA_CONTENT_FILTER, validator).process(data, entity); return resultList; diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java index 514d968a8..bd7048cec 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java @@ -80,9 +80,8 @@ void setup() { attachmentsReader = mock(AttachmentsReader.class); attachmentService = mock(AttachmentService.class); storageReader = mock(ThreadDataStorageReader.class); - cut = - new UpdateAttachmentsHandler( - eventFactory, attachmentsReader, attachmentService, storageReader); + cut = new UpdateAttachmentsHandler( + eventFactory, attachmentsReader, attachmentService, storageReader); event = mock(ModifyAttachmentEvent.class); updateContext = mock(CdsUpdateEventContext.class); @@ -252,8 +251,8 @@ void existingDataFoundAndUsed() { var model = runtime.getCdsModel(); var target = updateContext.getTarget(); when(attachmentsReader.readAttachments( - eq(model), eq(target), any(CqnFilterableStatement.class))) - .thenReturn(List.of(Attachments.of(root))); + eq(model), eq(target), any(CqnFilterableStatement.class))) + .thenReturn(root.getAttachments().stream().map(Attachments::of).toList()); cut.processBefore(updateContext, List.of(root)); @@ -268,10 +267,15 @@ void existingDataFoundAndUsed() { void noExistingDataFound() { var id = getEntityAndMockContext(RootTable_.CDS_NAME); when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) - .thenReturn(List.of(Attachments.create())); + .thenReturn(List.of()); var testStream = mock(InputStream.class); - var root = fillRootData(testStream, id); + var root = RootTable.create(); + root.setId(id); + var attachment = Attachments.create(); + // No ID set - this is a new attachment + attachment.setContent(testStream); + root.setAttachments(List.of(attachment)); cut.processBefore(updateContext, List.of(root)); @@ -303,6 +307,8 @@ void selectIsUsedWithFilterAndWhere() { CqnUpdate update = Update.entity(entityWithKeys).byId("test"); var serviceEntity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME).orElseThrow(); mockTargetInUpdateContext(serviceEntity, update); + when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) + .thenReturn(List.of(attachment)); cut.processBefore(updateContext, List.of(attachment)); @@ -324,6 +330,8 @@ void selectIsUsedWithFilter() { CqnUpdate update = Update.entity(entityWithKeys); var serviceEntity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME).orElseThrow(); mockTargetInUpdateContext(serviceEntity, update); + when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) + .thenReturn(List.of(attachment)); cut.processBefore(updateContext, List.of(attachment)); @@ -343,6 +351,8 @@ void selectIsUsedWithWhere() { CqnUpdate update = Update.entity(Attachment_.CDS_NAME).byId("test"); var serviceEntity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME).orElseThrow(); mockTargetInUpdateContext(serviceEntity, update); + when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) + .thenReturn(List.of(attachment)); cut.processBefore(updateContext, List.of(attachment)); @@ -361,9 +371,10 @@ void selectIsUsedWithAttachmentId() { attachment.put(UP_ID, "test_up_id"); attachment.setContent(mock(InputStream.class)); var serviceEntity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME).orElseThrow(); - CqnUpdate update = - Update.entity(Attachment_.class).where(entity -> entity.ID().eq(attachment.getId())); + CqnUpdate update = Update.entity(Attachment_.class).where(entity -> entity.ID().eq(attachment.getId())); mockTargetInUpdateContext(serviceEntity, update); + when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) + .thenReturn(List.of(attachment)); cut.processBefore(updateContext, List.of(attachment)); @@ -384,16 +395,16 @@ void selectIsCorrectForMultipleAttachments() { attachment2.setId(UUID.randomUUID().toString()); attachment2.put(UP_ID, "test_multiple 2"); attachment2.setContent(mock(InputStream.class)); - CqnUpdate update = - Update.entity(Attachment_.class) - .where( - attachment -> - attachment - .ID() - .eq(attachment1.getId()) - .or(attachment.ID().eq(attachment2.getId()))); + CqnUpdate update = Update.entity(Attachment_.class) + .where( + attachment -> attachment + .ID() + .eq(attachment1.getId()) + .or(attachment.ID().eq(attachment2.getId()))); var serviceEntity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME).orElseThrow(); mockTargetInUpdateContext(serviceEntity, update); + when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) + .thenReturn(List.of(attachment1, attachment2)); cut.processBefore(updateContext, List.of(attachment1, attachment2)); @@ -429,12 +440,14 @@ void noContentInDataButAssociationIsChangedAndDeleteCalled() { var attachment = Attachments.create(); attachment.setId(UUID.randomUUID().toString()); + attachment.put(UP_ID, id); // Set parent key so deletion logic can match it attachment.setContent(mock(InputStream.class)); attachment.setContentId("document id"); var existingRoot = RootTable.create(); + existingRoot.setId(id); existingRoot.setAttachments(List.of(attachment)); when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) - .thenReturn(List.of(Attachments.of(existingRoot))); + .thenReturn(existingRoot.getAttachments().stream().map(Attachments::of).toList()); when(updateContext.getUserInfo()).thenReturn(userInfo); cut.processBefore(updateContext, List.of(root)); @@ -457,8 +470,7 @@ void classHasCorrectAnnotation() { @Test void methodHasCorrectAnnotations() throws NoSuchMethodException { - var method = - cut.getClass().getDeclaredMethod("processBefore", CdsUpdateEventContext.class, List.class); + var method = cut.getClass().getDeclaredMethod("processBefore", CdsUpdateEventContext.class, List.class); var updateBeforeAnnotation = method.getAnnotation(Before.class); var updateHandlerOrderAnnotation = method.getAnnotation(HandlerOrder.class); @@ -485,8 +497,7 @@ private String getEntityAndMockContext(String cdsName) { private String mockTargetInUpdateContext(CdsEntity serviceEntity) { var id = UUID.randomUUID().toString(); - var update = - Update.entity(serviceEntity.getQualifiedName()).where(entity -> entity.get("ID").eq(id)); + var update = Update.entity(serviceEntity.getQualifiedName()).where(entity -> entity.get("ID").eq(id)); mockTargetInUpdateContext(serviceEntity, update); return id; } @@ -503,8 +514,8 @@ private Map getAttachmentKeyMap(Attachments attachment) { private String getRefString(String key, String value) { return """ - {"ref":["%s"]},"=",{"val":"%s"} - """ + {"ref":["%s"]},"=",{"val":"%s"} + """ .formatted(key, value) .replace(" ", "") .replace("\n", ""); @@ -512,8 +523,8 @@ private String getRefString(String key, String value) { private String getOrCondition(String key1, String key2) { return """ - [{"ref":["ID"]},"=",{"val":"%s"},"or",{"ref":["ID"]},"=",{"val":"%s"}] - """ + [{"ref":["ID"]},"=",{"val":"%s"},"or",{"ref":["ID"]},"=",{"val":"%s"}] + """ .formatted(key1, key2) .replace(" ", "") .replace("\n", ""); From ff5a4148a2756b17d5e3d5600d9dc3c946b29713 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 13 Jan 2026 17:47:45 +0100 Subject: [PATCH 10/15] tests --- .../helper/ExtendedErrorStatusesTest.java | 31 ++++ .../helper/FileSizeUtilsTest.java | 61 +++++++ .../readhelper/CountingInputStreamTest.java | 163 ++++++++++++++++++ 3 files changed, 255 insertions(+) create mode 100644 cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java create mode 100644 cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java create mode 100644 cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java new file mode 100644 index 000000000..fd1afa46c --- /dev/null +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java @@ -0,0 +1,31 @@ +/* + * © 2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ +package com.sap.cds.feature.attachments.handler.applicationservice.helper; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class ExtendedErrorStatusesTest { + + @Test + void contentTooLargeHasCorrectProperties() { + assertThat(ExtendedErrorStatuses.CONTENT_TOO_LARGE.getCodeString()).isEqualTo("413"); + assertThat(ExtendedErrorStatuses.CONTENT_TOO_LARGE.getDescription()) + .isEqualTo("The content size exceeds the maximum allowed limit."); + assertThat(ExtendedErrorStatuses.CONTENT_TOO_LARGE.getHttpStatus()).isEqualTo(413); + } + + @Test + void getByCode_existingCode_returnsErrorStatus() { + var result = ExtendedErrorStatuses.getByCode(400); + assertThat(result).isNotNull(); + } + + @Test + void getByCode_nonExistingCode_returnsNull() { + var result = ExtendedErrorStatuses.getByCode(999); + assertThat(result).isNull(); + } +} \ No newline at end of file diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java new file mode 100644 index 000000000..e07fe3b3d --- /dev/null +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java @@ -0,0 +1,61 @@ +/* + * © 2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ +package com.sap.cds.feature.attachments.handler.applicationservice.helper; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class FileSizeUtilsTest { + + @ParameterizedTest + @CsvSource({ + "100, 100", + "100B, 100", + "1KB, 1000", + "1MB, 1000000", + "1GB, 1000000000", + "1TB, 1000000000000", + "1KIB, 1024", + "1MIB, 1048576", + "1GIB, 1073741824", + "1TIB, 1099511627776", + "2.5KB, 2500", + "1.5MB, 1500000", + " 100 , 100", + " 100 KB , 100000" + }) + void parseFileSizeToBytes_validInput(String input, long expected) { + assertThat(FileSizeUtils.parseFileSizeToBytes(input)).isEqualTo(expected); + } + + @Test + void parseFileSizeToBytes_nullInput_throwsException() { + assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Value for Max File Size is null"); + } + + @ParameterizedTest + @CsvSource({ + "invalid", + "100XB", + "abc KB", + "-100" + }) + void parseFileSizeToBytes_invalidFormat_throwsException(String input) { + assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes(input)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void parseFileSizeToBytes_unknownUnit_throwsException() { + assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes("100XB")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Unknown Unit"); + } +} \ No newline at end of file diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java new file mode 100644 index 000000000..0697f6586 --- /dev/null +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java @@ -0,0 +1,163 @@ +/* + * © 2024-2025 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ +package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import org.junit.jupiter.api.Test; + +class CountingInputStreamTest { + + @Test + void constructor_negativeMaxBytes_throwsException() { + var delegate = new ByteArrayInputStream(new byte[0]); + assertThatThrownBy(() -> new CountingInputStream(delegate, -1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("maxBytes must be non-negative"); + } + + @Test + void read_singleByte_tracksCount() throws IOException { + var data = new byte[] { 1, 2, 3 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + + cut.read(); + assertThat(cut.getBytesRead()).isEqualTo(1); + + cut.read(); + assertThat(cut.getBytesRead()).isEqualTo(2); + + cut.close(); + } + + @Test + void read_byteArray_tracksCount() throws IOException { + var data = new byte[] { 1, 2, 3, 4, 5 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + + var buffer = new byte[3]; + int bytesRead = cut.read(buffer); + + assertThat(bytesRead).isEqualTo(3); + assertThat(cut.getBytesRead()).isEqualTo(3); + } + + @Test + void read_byteArrayWithOffset_tracksCount() throws IOException { + var data = new byte[] { 1, 2, 3, 4, 5 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + + var buffer = new byte[5]; + int bytesRead = cut.read(buffer, 0, 3); + + assertThat(bytesRead).isEqualTo(3); + assertThat(cut.getBytesRead()).isEqualTo(3); + } + + @Test + void read_exceedsLimit_throwsException() { + var data = new byte[] { 1, 2, 3, 4, 5 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 3); + + assertThatThrownBy(() -> { + var buffer = new byte[5]; + int bytesRead = cut.read(buffer); + }); + + } + + @Test + void skip_tracksCount() throws IOException { + var data = new byte[] { 1, 2, 3, 4, 5 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + + long skipped = cut.skip(3); + cut.close(); + + assertThat(skipped).isEqualTo(3); + assertThat(cut.getBytesRead()).isEqualTo(3); + + } + + @Test + void skip_exceedsLimit_throwsException() { + var data = new byte[] { 1, 2, 3, 4, 5 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 2); + + assertThatThrownBy(() -> { + long skipped = cut.skip(3); + }); + } + + @Test + void available_delegatesToUnderlying() throws IOException { + var data = new byte[] { 1, 2, 3 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + cut.close(); + + assertThat(cut.available()).isEqualTo(3); + } + + @Test + void close_closesDelegate() throws IOException { + var data = new byte[] { 1, 2, 3 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + cut.close(); + + // ByteArrayInputStream doesn't really close, but we verify no exception + assertThat(cut).isNotNull(); + } + + @Test + void markSupported_delegatesToUnderlying() { + var data = new byte[] { 1, 2, 3 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + + assertThat(cut.markSupported()).isTrue(); + } + + @Test + void markAndReset_delegatesToUnderlying() throws IOException { + var data = new byte[] { 1, 2, 3 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + + cut.mark(10); + cut.read(); + cut.read(); + assertThat(cut.getBytesRead()).isEqualTo(2); + + cut.reset(); + int value = cut.read(); + assertThat(value).isEqualTo(1); + cut.close(); + } + + @Test + void read_endOfStream_returnsMinusOne() throws IOException { + var data = new byte[] { 1 }; + var delegate = new ByteArrayInputStream(data); + var cut = new CountingInputStream(delegate, 10); + + cut.read(); // read the only byte + int result = cut.read(); // should return -1 + + assertThat(result).isEqualTo(-1); + assertThat(cut.getBytesRead()).isEqualTo(1); // count shouldn't increase + cut.close(); + } +} \ No newline at end of file From 96a83fb669a6f4fbc87700187edace1e47507ec5 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Wed, 14 Jan 2026 15:09:56 +0100 Subject: [PATCH 11/15] wip --- .../UpdateAttachmentsHandler.java | 93 +++----- .../helper/FileSizeUtils.java | 2 +- .../ModifyApplicationHandlerHelper.java | 44 +--- .../readhelper/CountingInputStream.java | 122 ---------- .../readhelper/LazyProxyInputStream.java | 5 - .../common/ApplicationHandlerHelper.java | 53 +++-- .../UpdateAttachmentsHandlerTest.java | 30 ++- .../helper/ExtendedErrorStatusesTest.java | 2 +- .../helper/FileSizeUtilsTest.java | 89 ++++++-- .../readhelper/CountingInputStreamTest.java | 163 -------------- integration-tests/db/data-model.cds | 3 +- integration-tests/pom.xml | 2 +- .../Attachments2SizeValidationDraftTest.java | 213 ++++++++++++++++++ .../DraftOdataRequestValidationBase.java | 2 +- ...ttachments2SizeValidationNonDraftTest.java | 211 +++++++++++++++++ .../OdataRequestValidationBase.java | 30 ++- ...aRequestValidationWithTestHandlerTest.java | 2 +- .../helper/RootEntityBuilder.java | 9 + integration-tests/srv/test-service.cds | 4 + pom.xml | 6 + 20 files changed, 632 insertions(+), 453 deletions(-) delete mode 100644 cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java delete mode 100644 cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java create mode 100644 integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/Attachments2SizeValidationDraftTest.java create mode 100644 integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/Attachments2SizeValidationNonDraftTest.java diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index cbd43a57a..ab612cf3d 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -5,8 +5,6 @@ import static java.util.Objects.requireNonNull; -import java.util.HashMap; - import com.sap.cds.CdsData; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; import com.sap.cds.feature.attachments.handler.applicationservice.helper.ModifyApplicationHandlerHelper; @@ -19,8 +17,6 @@ import com.sap.cds.feature.attachments.service.model.service.MarkAsDeletedInput; import com.sap.cds.ql.cqn.CqnSelect; import com.sap.cds.reflect.CdsEntity; -import com.sap.cds.services.ErrorStatuses; -import com.sap.cds.services.ServiceException; import com.sap.cds.services.cds.ApplicationService; import com.sap.cds.services.cds.CdsUpdateEventContext; import com.sap.cds.services.handler.EventHandler; @@ -30,19 +26,16 @@ import com.sap.cds.services.request.UserInfo; import com.sap.cds.services.utils.OrderConstants; import com.sap.cds.services.utils.model.CqnUtils; - +import java.util.HashMap; import java.util.List; import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * The class {@link UpdateAttachmentsHandler} is an event handler that is called - * before an update - * event is executed. As updates in draft entities or non-draft entities can - * also be create-events, - * update-events or delete-events the handler needs to distinguish between the - * different cases. + * The class {@link UpdateAttachmentsHandler} is an event handler that is called before an update + * event is executed. As updates in draft entities or non-draft entities can also be create-events, + * update-events or delete-events the handler needs to distinguish between the different cases. */ @ServiceName(value = "*", type = ApplicationService.class) public class UpdateAttachmentsHandler implements EventHandler { @@ -60,9 +53,11 @@ public UpdateAttachmentsHandler( AttachmentService attachmentService, ThreadDataStorageReader storageReader) { this.eventFactory = requireNonNull(eventFactory, "eventFactory must not be null"); - this.attachmentService = requireNonNull(attachmentService, "attachmentService must not be null"); + this.attachmentService = + requireNonNull(attachmentService, "attachmentService must not be null"); this.storageReader = requireNonNull(storageReader, "storageReader must not be null"); - this.attachmentsReader = requireNonNull(attachmentsReader, "attachmentsReader must not be null"); + this.attachmentsReader = + requireNonNull(attachmentsReader, "attachmentsReader must not be null"); } @Before @@ -83,21 +78,18 @@ void processBefore(CdsUpdateEventContext context, List data) { boolean associationsAreUnchanged = associationsAreUnchanged(target, data); if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) { - List attachments = ApplicationHandlerHelper.condenseAttachments(data, target); + logger.debug("Processing before {} event for entity {}", context.getEvent(), target); // Query database only for validation (single query for all attachments) CqnSelect select = CqnUtils.toSelect(context.getCqn(), context.getTarget()); - List dbAttachments = attachmentsReader.readAttachments(context.getModel(), target, select); - - // Validate: ensure request attachments exist in DB or are new (no ID yet) - validateAttachments(attachments, dbAttachments, target); + List attachments = + attachmentsReader.readAttachments(context.getModel(), target, select); - logger.debug("Processing before {} event for entity {}", context.getEvent(), target); ModifyApplicationHandlerHelper.handleAttachmentForEntities( target, data, attachments, eventFactory, context); if (!associationsAreUnchanged) { - deleteRemovedAttachments(dbAttachments, data, target, context.getUserInfo()); + deleteRemovedAttachments(attachments, data, target, context.getUserInfo()); } } } @@ -116,55 +108,32 @@ private void deleteRemovedAttachments( List requestData, CdsEntity entity, UserInfo userInfo) { - List requestAttachments = ApplicationHandlerHelper.condenseAttachments(requestData, entity); + List requestAttachments = + ApplicationHandlerHelper.condenseAttachments(requestData, entity); for (Attachments dbAttachment : dbAttachments) { Map dbKeys = new HashMap<>(); - entity.keyElements().forEach(keyElement -> { - String keyName = keyElement.getName(); - Object value = dbAttachment.get(keyName); - if (value != null) { - dbKeys.put(keyName, value); - } - }); + entity + .keyElements() + .forEach( + keyElement -> { + String keyName = keyElement.getName(); + Object value = dbAttachment.get(keyName); + if (value != null) { + dbKeys.put(keyName, value); + } + }); Map keys = ApplicationHandlerHelper.removeDraftKey(dbKeys); - boolean existsInRequest = requestAttachments.stream() - .anyMatch(requestAttachment -> ApplicationHandlerHelper.areKeysInData(keys, requestAttachment)); + boolean existsInRequest = + requestAttachments.stream() + .anyMatch( + requestAttachment -> + ApplicationHandlerHelper.areKeysInData(keys, requestAttachment)); if (!existsInRequest && dbAttachment.getContentId() != null) { - attachmentService.markAttachmentAsDeleted(new MarkAsDeletedInput(dbAttachment.getContentId(), userInfo)); - } - } - } - - private void validateAttachments(List requestAttachments, List dbAttachments, - CdsEntity target) { - for (Attachments requestAttachment : requestAttachments) { - // Build a map of keys from the attachment based on entity key elements - Map keysMap = new HashMap<>(); - target.keyElements().forEach(keyElement -> { - String keyName = keyElement.getName(); - Object value = requestAttachment.get(keyName); - if (value != null) { - keysMap.put(keyName, value); - } - }); - - Map keys = ApplicationHandlerHelper.removeDraftKey(keysMap); - - // Skip validation if attachment has no keys (new attachment being created) - if (keys.isEmpty()) { - continue; - } - - // If attachment has keys, verify it exists in the database - boolean existsInDb = dbAttachments.stream() - .anyMatch(db -> ApplicationHandlerHelper.areKeysInData(keys, db)); - - if (!existsInDb) { - throw new ServiceException(ErrorStatuses.NOT_FOUND, - "Attachment with keys " + keys + " not found"); + attachmentService.markAttachmentAsDeleted( + new MarkAsDeletedInput(dbAttachment.getContentId(), userInfo)); } } } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java index 4884322a5..7a6a69e9a 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtils.java @@ -39,7 +39,7 @@ public static long parseFileSizeToBytes(String input) { throw new IllegalArgumentException("Invalid size: " + input); } BigDecimal value = new BigDecimal(m.group(1)); - String unitRaw = m.group(2) == null ? "" : m.group(2); + String unitRaw = m.group(2); // Cannot be null due to * quantifier in regex String unit = unitRaw.toUpperCase(); // if (unit.length() == 1) unit = unit + "B"; // for people using K instead of KB diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java index cf662490b..b8b9b488f 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java @@ -10,16 +10,13 @@ import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; import com.sap.cds.feature.attachments.handler.applicationservice.modifyevents.ModifyAttachmentEvent; import com.sap.cds.feature.attachments.handler.applicationservice.modifyevents.ModifyAttachmentEventFactory; -import com.sap.cds.feature.attachments.handler.applicationservice.readhelper.CountingInputStream; import com.sap.cds.feature.attachments.handler.common.ApplicationHandlerHelper; import com.sap.cds.ql.cqn.Path; import com.sap.cds.reflect.CdsEntity; import com.sap.cds.services.ErrorStatuses; import com.sap.cds.services.EventContext; import com.sap.cds.services.ServiceException; -import java.io.IOException; import java.io.InputStream; -import java.io.UncheckedIOException; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; @@ -77,35 +74,7 @@ public static InputStream handleAttachmentForEntity( Attachments attachment = getExistingAttachment(keys, existingAttachments); String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); String contentLength = eventContext.getParameterInfo().getHeader("Content-Length"); - // Wrap the stream with CountingInputStream if a max size is configured - InputStream wrappedContent = - wrapWithCountingStream(content, path.target().entity(), existingAttachments, contentLength); - - // for the current request find the event to process - ModifyAttachmentEvent eventToProcess = - eventFactory.getEvent(wrappedContent, contentId, attachment); - - // process the event - try { - return eventToProcess.processEvent(path, wrappedContent, attachment, eventContext); - } catch (UncheckedIOException e) { - // Extract the IOException with our custom message - IOException cause = e.getCause(); - throw new ServiceException(ErrorStatuses.BAD_REQUEST, cause.getMessage()); - } catch (RuntimeException e) { - // Check if nested UncheckedIOException - Throwable cause = e.getCause(); - if (cause instanceof UncheckedIOException) { - IOException ioCause = ((UncheckedIOException) cause).getCause(); - throw new ServiceException(ErrorStatuses.BAD_REQUEST, ioCause.getMessage()); - } - throw e; - } - } - - private static InputStream wrapWithCountingStream( - InputStream content, CdsEntity entity, List data, String contentLength) { - String maxSizeStr = getValMaxValue(entity, data); + String maxSizeStr = getValMaxValue(path.target().entity(), existingAttachments); if (maxSizeStr != null && content != null) { try { @@ -113,15 +82,22 @@ private static InputStream wrapWithCountingStream( if (contentLength != null && Long.parseLong(contentLength) > maxSize) { throw new RuntimeException("File size exceeds the maximum allowed size of " + maxSizeStr); } - return new CountingInputStream(content, maxSize); } catch (ArithmeticException e) { throw new ServiceException("Maximum file size value is too large", e); } catch (RuntimeException e) { throw new ServiceException( ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); + } catch (Exception e) { + throw new ServiceException( + ErrorStatuses.BAD_REQUEST, "Failed to process attachment size", e); } } - return content; + + // for the current request find the event to process + ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(content, contentId, attachment); + + // process the event + return eventToProcess.processEvent(path, content, attachment, eventContext); } private static String getValMaxValue(CdsEntity entity, List data) { diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java deleted file mode 100644 index 0159da5ad..000000000 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java +++ /dev/null @@ -1,122 +0,0 @@ -/* - * © 2024-2025 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. - */ -package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; - -import com.sap.cds.services.ServiceException; -import java.io.IOException; -import java.io.InputStream; -import java.io.UncheckedIOException; - -/** - * An InputStream wrapper that tracks bytes read and enforces a maximum size limit. This allows for - * memory-efficient streaming validation without buffering entire files. - */ -public final class CountingInputStream extends InputStream { - - private final InputStream delegate; - private final long maxBytes; - private long bytesRead = 0; - - /** - * Creates a CountingInputStream that enforces a maximum size limit. - * - * @param delegate the underlying InputStream to wrap - * @param maxBytes the maximum number of bytes allowed to be read - * @throws IllegalArgumentException if maxBytes is negative - */ - public CountingInputStream(InputStream delegate, long maxBytes) { - if (maxBytes < 0) { - throw new IllegalArgumentException("maxBytes must be non-negative"); - } - this.delegate = delegate; - this.maxBytes = maxBytes; - } - - @Override - public int read() throws IOException { - int b = delegate.read(); - if (b != -1) { - checkLimit(1); - } - return b; - } - - @Override - public int read(byte[] b) throws IOException { - int result = delegate.read(b); - if (result > 0) { - checkLimit(result); - } - return result; - } - - @Override - public int read(byte[] b, int off, int len) throws IOException { - int result = delegate.read(b, off, len); - if (result > 0) { - checkLimit(result); - } - return result; - } - - @Override - public long skip(long n) throws IOException { - long skipped = delegate.skip(n); - if (skipped > 0) { - checkLimit(skipped); - } - return skipped; - } - - @Override - public int available() throws IOException { - return delegate.available(); - } - - @Override - public void close() throws IOException { - if (delegate != null) { - delegate.close(); - } - } - - @Override - public void mark(int readlimit) { - delegate.mark(readlimit); - } - - @Override - public void reset() throws IOException { - delegate.reset(); - } - - @Override - public boolean markSupported() { - return delegate.markSupported(); - } - - /** - * Returns the number of bytes that have been read so far. - * - * @return the total bytes read - */ - public long getBytesRead() { - return bytesRead; - } - - /** - * Checks if adding the specified number of bytes would exceed the limit. Increments the bytesRead - * counter and throws an exception if limit is exceeded. - * - * @param bytes the number of bytes being read - * @throws ServiceException if reading these bytes would exceed maxBytes - */ - private void checkLimit(long bytes) { - bytesRead += bytes; - if (bytesRead > maxBytes) { - throw new UncheckedIOException( - new IOException(String.format("File size exceeds the limit of %d bytes", maxBytes))); - } - } -} diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java index 9d9f636e9..850b588f1 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java @@ -57,11 +57,6 @@ public void close() throws IOException { } } - @Override - public int available() throws IOException { - return delegate != null ? delegate.available() : 0; - } - private InputStream getDelegate() { attachmentStatusValidator.verifyStatus(status); diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java index 67082fed6..18eea5567 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java @@ -20,8 +20,7 @@ import java.util.concurrent.atomic.AtomicBoolean; /** - * The class {@link ApplicationHandlerHelper} provides helper methods for the - * attachment application + * The class {@link ApplicationHandlerHelper} provides helper methods for the attachment application * handlers. */ public final class ApplicationHandlerHelper { @@ -29,21 +28,20 @@ public final class ApplicationHandlerHelper { private static final String ANNOTATION_CORE_MEDIA_TYPE = "Core.MediaType"; /** - * A filter for media content fields. The filter checks if the entity is a media - * entity and if the + * A filter for media content fields. The filter checks if the entity is a media entity and if the * element has the annotation "Core.MediaType". */ - public static final Filter MEDIA_CONTENT_FILTER = (path, element, type) -> isMediaEntity(path.target().type()) - && element.findAnnotation(ANNOTATION_CORE_MEDIA_TYPE).isPresent(); + public static final Filter MEDIA_CONTENT_FILTER = + (path, element, type) -> + isMediaEntity(path.target().type()) + && element.findAnnotation(ANNOTATION_CORE_MEDIA_TYPE).isPresent(); /** * Checks if the data contains a content field. * - * @param entity The {@link CdsEntity entity} type of the given the data to - * check - * @param data The data to check - * @return true if the data contains a content field, - * false otherwise + * @param entity The {@link CdsEntity entity} type of the given the data to check + * @param data The data to check + * @return true if the data contains a content field, false otherwise */ public static boolean containsContentField(CdsEntity entity, List data) { AtomicBoolean isIncluded = new AtomicBoolean(); @@ -54,13 +52,11 @@ public static boolean containsContentField(CdsEntity entity, Listtrue if the entity is a media entity, false - * otherwise + * @return true if the entity is a media entity, false otherwise */ public static boolean isMediaEntity(CdsStructuredType baseEntity) { return baseEntity.getAnnotationValue(ANNOTATION_IS_MEDIA_DATA, false); @@ -69,27 +65,29 @@ public static boolean isMediaEntity(CdsStructuredType baseEntity) { /** * Extracts key fields from CdsData based on the entity definition. * - * @param data The CdsData to extract keys from + * @param data The CdsData to extract keys from * @param entity The entity definition * @return A map of key fields and their values */ public static Map extractKeys(CdsData data, CdsEntity entity) { Map keys = new HashMap<>(); - entity.keyElements().forEach(keyElement -> { - String keyName = keyElement.getName(); - Object value = data.get(keyName); - if (value != null) { - keys.put(keyName, value); - } - }); + entity + .keyElements() + .forEach( + keyElement -> { + String keyName = keyElement.getName(); + Object value = data.get(keyName); + if (value != null) { + keys.put(keyName, value); + } + }); return keys; } /** - * Condenses the attachments from the given data into a list of - * {@link Attachments attachments}. + * Condenses the attachments from the given data into a list of {@link Attachments attachments}. * - * @param data the list of {@link CdsData} to process + * @param data the list of {@link CdsData} to process * @param entity the {@link CdsEntity entity} type of the given data * @return a list of {@link Attachments attachments} condensed from the data */ @@ -97,7 +95,8 @@ public static List condenseAttachments( List data, CdsEntity entity) { List resultList = new ArrayList<>(); - Validator validator = (path, element, value) -> resultList.add(Attachments.of(path.target().values())); + Validator validator = + (path, element, value) -> resultList.add(Attachments.of(path.target().values())); CdsDataProcessor.create().addValidator(MEDIA_CONTENT_FILTER, validator).process(data, entity); return resultList; diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java index bd7048cec..9a9103979 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java @@ -80,8 +80,9 @@ void setup() { attachmentsReader = mock(AttachmentsReader.class); attachmentService = mock(AttachmentService.class); storageReader = mock(ThreadDataStorageReader.class); - cut = new UpdateAttachmentsHandler( - eventFactory, attachmentsReader, attachmentService, storageReader); + cut = + new UpdateAttachmentsHandler( + eventFactory, attachmentsReader, attachmentService, storageReader); event = mock(ModifyAttachmentEvent.class); updateContext = mock(CdsUpdateEventContext.class); @@ -251,7 +252,7 @@ void existingDataFoundAndUsed() { var model = runtime.getCdsModel(); var target = updateContext.getTarget(); when(attachmentsReader.readAttachments( - eq(model), eq(target), any(CqnFilterableStatement.class))) + eq(model), eq(target), any(CqnFilterableStatement.class))) .thenReturn(root.getAttachments().stream().map(Attachments::of).toList()); cut.processBefore(updateContext, List.of(root)); @@ -371,7 +372,8 @@ void selectIsUsedWithAttachmentId() { attachment.put(UP_ID, "test_up_id"); attachment.setContent(mock(InputStream.class)); var serviceEntity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME).orElseThrow(); - CqnUpdate update = Update.entity(Attachment_.class).where(entity -> entity.ID().eq(attachment.getId())); + CqnUpdate update = + Update.entity(Attachment_.class).where(entity -> entity.ID().eq(attachment.getId())); mockTargetInUpdateContext(serviceEntity, update); when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) .thenReturn(List.of(attachment)); @@ -395,12 +397,14 @@ void selectIsCorrectForMultipleAttachments() { attachment2.setId(UUID.randomUUID().toString()); attachment2.put(UP_ID, "test_multiple 2"); attachment2.setContent(mock(InputStream.class)); - CqnUpdate update = Update.entity(Attachment_.class) - .where( - attachment -> attachment - .ID() - .eq(attachment1.getId()) - .or(attachment.ID().eq(attachment2.getId()))); + CqnUpdate update = + Update.entity(Attachment_.class) + .where( + attachment -> + attachment + .ID() + .eq(attachment1.getId()) + .or(attachment.ID().eq(attachment2.getId()))); var serviceEntity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME).orElseThrow(); mockTargetInUpdateContext(serviceEntity, update); when(attachmentsReader.readAttachments(any(), any(), any(CqnFilterableStatement.class))) @@ -470,7 +474,8 @@ void classHasCorrectAnnotation() { @Test void methodHasCorrectAnnotations() throws NoSuchMethodException { - var method = cut.getClass().getDeclaredMethod("processBefore", CdsUpdateEventContext.class, List.class); + var method = + cut.getClass().getDeclaredMethod("processBefore", CdsUpdateEventContext.class, List.class); var updateBeforeAnnotation = method.getAnnotation(Before.class); var updateHandlerOrderAnnotation = method.getAnnotation(HandlerOrder.class); @@ -497,7 +502,8 @@ private String getEntityAndMockContext(String cdsName) { private String mockTargetInUpdateContext(CdsEntity serviceEntity) { var id = UUID.randomUUID().toString(); - var update = Update.entity(serviceEntity.getQualifiedName()).where(entity -> entity.get("ID").eq(id)); + var update = + Update.entity(serviceEntity.getQualifiedName()).where(entity -> entity.get("ID").eq(id)); mockTargetInUpdateContext(serviceEntity, update); return id; } diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java index fd1afa46c..ebd1c747f 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java @@ -28,4 +28,4 @@ void getByCode_nonExistingCode_returnsNull() { var result = ExtendedErrorStatuses.getByCode(999); assertThat(result).isNull(); } -} \ No newline at end of file +} diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java index e07fe3b3d..ee3da49c2 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/FileSizeUtilsTest.java @@ -14,25 +14,58 @@ class FileSizeUtilsTest { @ParameterizedTest @CsvSource({ - "100, 100", - "100B, 100", - "1KB, 1000", - "1MB, 1000000", - "1GB, 1000000000", - "1TB, 1000000000000", - "1KIB, 1024", - "1MIB, 1048576", - "1GIB, 1073741824", - "1TIB, 1099511627776", - "2.5KB, 2500", - "1.5MB, 1500000", - " 100 , 100", - " 100 KB , 100000" + "100, 100", + "100B, 100", + "1KB, 1000", + "1MB, 1000000", + "1GB, 1000000000", + "1TB, 1000000000000", + "1KIB, 1024", + "1MIB, 1048576", + "1GIB, 1073741824", + "1TIB, 1099511627776", + "2.5KB, 2500", + "1.5MB, 1500000", + " 100 , 100", + " 100 KB , 100000", + "0, 0", + "0B, 0", + "001KB, 1000", + "0.5MB, 500000" }) void parseFileSizeToBytes_validInput(String input, long expected) { assertThat(FileSizeUtils.parseFileSizeToBytes(input)).isEqualTo(expected); } + @ParameterizedTest + @CsvSource({ + "1kb, 1000", + "1mb, 1000000", + "1gb, 1000000000", + "1tb, 1000000000000", + "1kib, 1024", + "1mib, 1048576", + "1gib, 1073741824", + "1tib, 1099511627776", + "100b, 100" + }) + void parseFileSizeToBytes_lowercaseUnits(String input, long expected) { + assertThat(FileSizeUtils.parseFileSizeToBytes(input)).isEqualTo(expected); + } + + @ParameterizedTest + @CsvSource({ + "1Kb, 1000", + "1Mb, 1000000", + "1Gb, 1000000000", + "1KiB, 1024", + "1MiB, 1048576", + "1GiB, 1073741824" + }) + void parseFileSizeToBytes_mixedCaseUnits(String input, long expected) { + assertThat(FileSizeUtils.parseFileSizeToBytes(input)).isEqualTo(expected); + } + @Test void parseFileSizeToBytes_nullInput_throwsException() { assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes(null)) @@ -41,21 +74,35 @@ void parseFileSizeToBytes_nullInput_throwsException() { } @ParameterizedTest - @CsvSource({ - "invalid", - "100XB", - "abc KB", - "-100" - }) + @CsvSource({"invalid", "abc KB", "-100", "''"}) void parseFileSizeToBytes_invalidFormat_throwsException(String input) { assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes(input)) .isInstanceOf(IllegalArgumentException.class); } + @Test + void parseFileSizeToBytes_whitespaceOnly_throwsException() { + assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes(" ")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid size"); + } + + @Test + void parseFileSizeToBytes_fractionalBytes_throwsException() { + assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes("0.5")) + .isInstanceOf(ArithmeticException.class); + } + + @Test + void parseFileSizeToBytes_fractionalBytesWithUnit_throwsException() { + assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes("0.5B")) + .isInstanceOf(ArithmeticException.class); + } + @Test void parseFileSizeToBytes_unknownUnit_throwsException() { assertThatThrownBy(() -> FileSizeUtils.parseFileSizeToBytes("100XB")) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Unknown Unit"); } -} \ No newline at end of file +} diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java deleted file mode 100644 index 0697f6586..000000000 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStreamTest.java +++ /dev/null @@ -1,163 +0,0 @@ -/* - * © 2024-2025 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. - */ -package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.UncheckedIOException; -import org.junit.jupiter.api.Test; - -class CountingInputStreamTest { - - @Test - void constructor_negativeMaxBytes_throwsException() { - var delegate = new ByteArrayInputStream(new byte[0]); - assertThatThrownBy(() -> new CountingInputStream(delegate, -1)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("maxBytes must be non-negative"); - } - - @Test - void read_singleByte_tracksCount() throws IOException { - var data = new byte[] { 1, 2, 3 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - - cut.read(); - assertThat(cut.getBytesRead()).isEqualTo(1); - - cut.read(); - assertThat(cut.getBytesRead()).isEqualTo(2); - - cut.close(); - } - - @Test - void read_byteArray_tracksCount() throws IOException { - var data = new byte[] { 1, 2, 3, 4, 5 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - - var buffer = new byte[3]; - int bytesRead = cut.read(buffer); - - assertThat(bytesRead).isEqualTo(3); - assertThat(cut.getBytesRead()).isEqualTo(3); - } - - @Test - void read_byteArrayWithOffset_tracksCount() throws IOException { - var data = new byte[] { 1, 2, 3, 4, 5 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - - var buffer = new byte[5]; - int bytesRead = cut.read(buffer, 0, 3); - - assertThat(bytesRead).isEqualTo(3); - assertThat(cut.getBytesRead()).isEqualTo(3); - } - - @Test - void read_exceedsLimit_throwsException() { - var data = new byte[] { 1, 2, 3, 4, 5 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 3); - - assertThatThrownBy(() -> { - var buffer = new byte[5]; - int bytesRead = cut.read(buffer); - }); - - } - - @Test - void skip_tracksCount() throws IOException { - var data = new byte[] { 1, 2, 3, 4, 5 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - - long skipped = cut.skip(3); - cut.close(); - - assertThat(skipped).isEqualTo(3); - assertThat(cut.getBytesRead()).isEqualTo(3); - - } - - @Test - void skip_exceedsLimit_throwsException() { - var data = new byte[] { 1, 2, 3, 4, 5 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 2); - - assertThatThrownBy(() -> { - long skipped = cut.skip(3); - }); - } - - @Test - void available_delegatesToUnderlying() throws IOException { - var data = new byte[] { 1, 2, 3 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - cut.close(); - - assertThat(cut.available()).isEqualTo(3); - } - - @Test - void close_closesDelegate() throws IOException { - var data = new byte[] { 1, 2, 3 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - cut.close(); - - // ByteArrayInputStream doesn't really close, but we verify no exception - assertThat(cut).isNotNull(); - } - - @Test - void markSupported_delegatesToUnderlying() { - var data = new byte[] { 1, 2, 3 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - - assertThat(cut.markSupported()).isTrue(); - } - - @Test - void markAndReset_delegatesToUnderlying() throws IOException { - var data = new byte[] { 1, 2, 3 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - - cut.mark(10); - cut.read(); - cut.read(); - assertThat(cut.getBytesRead()).isEqualTo(2); - - cut.reset(); - int value = cut.read(); - assertThat(value).isEqualTo(1); - cut.close(); - } - - @Test - void read_endOfStream_returnsMinusOne() throws IOException { - var data = new byte[] { 1 }; - var delegate = new ByteArrayInputStream(data); - var cut = new CountingInputStream(delegate, 10); - - cut.read(); // read the only byte - int result = cut.read(); // should return -1 - - assertThat(result).isEqualTo(-1); - assertThat(cut.getBytesRead()).isEqualTo(1); // count shouldn't increase - cut.close(); - } -} \ No newline at end of file diff --git a/integration-tests/db/data-model.cds b/integration-tests/db/data-model.cds index a06fb7cc1..fdca565f3 100644 --- a/integration-tests/db/data-model.cds +++ b/integration-tests/db/data-model.cds @@ -13,6 +13,7 @@ entity Roots : cuid { on attachments.parentKey = $self.ID; items : Composition of many Items on items.parentID = $self.ID; + attachments2 : Composition of many Attachments; } entity Items : cuid { @@ -29,4 +30,4 @@ entity Events : cuid { itemId : UUID; items : Composition of many Items on items.parentID = $self.ID; -} +} \ No newline at end of file diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index c3df8bc73..67686ed49 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -26,7 +26,7 @@ org.springframework.boot spring-boot-dependencies - 3.5.7 + 3.5.9 pom import diff --git a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/Attachments2SizeValidationDraftTest.java b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/Attachments2SizeValidationDraftTest.java new file mode 100644 index 000000000..da7d349cd --- /dev/null +++ b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/Attachments2SizeValidationDraftTest.java @@ -0,0 +1,213 @@ +/* + * © 2024-2024 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ +package com.sap.cds.feature.attachments.integrationtests.draftservice; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.sap.cds.Struct; +import com.sap.cds.feature.attachments.generated.integration.test.cds4j.sap.attachments.Attachments; +import com.sap.cds.feature.attachments.generated.integration.test.cds4j.testdraftservice.DraftRoots; +import com.sap.cds.feature.attachments.generated.integration.test.cds4j.testdraftservice.DraftRoots_; +import com.sap.cds.feature.attachments.integrationtests.common.MockHttpRequestHelper; +import com.sap.cds.feature.attachments.integrationtests.constants.Profiles; +import com.sap.cds.ql.Select; +import java.io.IOException; +import java.io.InputStream; +import java.util.Objects; +import org.junit.jupiter.api.Test; +import org.springframework.test.context.ActiveProfiles; + +@ActiveProfiles(Profiles.TEST_HANDLER_DISABLED) +class Attachments2SizeValidationDraftTest extends DraftOdataRequestValidationBase { + + private static final String BASE_URL = MockHttpRequestHelper.ODATA_BASE_URL + "TestDraftService/"; + private static final String BASE_ROOT_URL = BASE_URL + "DraftRoots"; + + @Test + void uploadContentWithin5MBLimitSucceeds() throws Exception { + // Arrange: Create draft with attachments2 + var draftRoot = createNewDraftWithAttachments2(); + var attachment = draftRoot.getAttachments2().get(0); + + // Act & Assert: Upload 3MB content (within limit) succeeds + byte[] content = new byte[3 * 1024 * 1024]; // 3MB + var url = buildDraftAttachment2ContentUrl(draftRoot.getId(), attachment.getId()); + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, content, status().isNoContent()); + } + + @Test + void uploadContentExceeding5MBLimitFails() throws Exception { + // Arrange: Create draft with attachments2 + var draftRoot = createNewDraftWithAttachments2(); + var attachment = draftRoot.getAttachments2().get(0); + + // Act: Try to upload 6MB content (exceeds limit) + byte[] content = new byte[6 * 1024 * 1024]; // 6MB + var url = buildDraftAttachment2ContentUrl(draftRoot.getId(), attachment.getId()); + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, content, status().is(413)); + + // Assert: Error response with HTTP 413 status code indicates size limit exceeded + } + + @Test + void updateContentStayingWithin5MBLimitSucceeds() throws Exception { + // Arrange: Create draft with attachments2 and upload initial content + var draftRoot = createNewDraftWithAttachments2(); + var attachment = draftRoot.getAttachments2().get(0); + + // Upload initial 2MB content + byte[] initialContent = new byte[2 * 1024 * 1024]; // 2MB + var url = buildDraftAttachment2ContentUrl(draftRoot.getId(), attachment.getId()); + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, initialContent, status().isNoContent()); + + // Act & Assert: Update with 3MB content (still within limit) succeeds + byte[] updatedContent = new byte[3 * 1024 * 1024]; // 3MB + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, updatedContent, status().isNoContent()); + } + + @Test + void updateContentExceeding5MBLimitFails() throws Exception { + // Arrange: Create draft with attachments2 and upload initial content + var draftRoot = createNewDraftWithAttachments2(); + var attachment = draftRoot.getAttachments2().get(0); + + // Upload initial 2MB content + byte[] initialContent = new byte[2 * 1024 * 1024]; // 2MB + var url = buildDraftAttachment2ContentUrl(draftRoot.getId(), attachment.getId()); + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, initialContent, status().isNoContent()); + + // Act & Assert: Try to update with 6MB content (exceeds limit) - should fail with HTTP 413 + byte[] updatedContent = new byte[6 * 1024 * 1024]; // 6MB + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, updatedContent, status().is(413)); + } + + // Helper methods + private DraftRoots createNewDraftWithAttachments2() throws Exception { + // Create new draft + var responseRootCdsData = + requestHelper.executePostWithODataResponseAndAssertStatusCreated(BASE_ROOT_URL, "{}"); + var draftRoot = Struct.access(responseRootCdsData).as(DraftRoots.class); + + // Update root with title + draftRoot.setTitle("Root with attachments2"); + var rootUrl = getRootUrl(draftRoot.getId(), false); + requestHelper.executePatchWithODataResponseAndAssertStatusOk(rootUrl, draftRoot.toJson()); + + // Create attachment2 + var attachment = Attachments.create(); + attachment.setFileName("testFile.txt"); + attachment.setMimeType("text/plain"); + var attachmentUrl = rootUrl + "/attachments2"; + var responseAttachmentCdsData = + requestHelper.executePostWithODataResponseAndAssertStatusCreated( + attachmentUrl, attachment.toJson()); + var createdAttachment = Struct.access(responseAttachmentCdsData).as(Attachments.class); + + // Build result with the attachment + draftRoot.setAttachments2(java.util.List.of(createdAttachment)); + return draftRoot; + } + + private DraftRoots selectStoredDraftWithAttachments2(String rootId) { + var select = + Select.from(DraftRoots_.class) + .where(r -> r.ID().eq(rootId).and(r.IsActiveEntity().eq(false))) + .columns(r -> r._all(), r -> r.attachments2().expand()); + + var result = persistenceService.run(select); + return result.single(DraftRoots.class); + } + + private String getRootUrl(String rootId, boolean isActiveEntity) { + return BASE_ROOT_URL + "(ID=" + rootId + ",IsActiveEntity=" + isActiveEntity + ")"; + } + + private String buildDraftAttachment2ContentUrl(String rootId, String attachmentId) { + return BASE_ROOT_URL + + "(ID=" + + rootId + + ",IsActiveEntity=false)" + + "/attachments2(ID=" + + attachmentId + + ",up__ID=" + + rootId + + ",IsActiveEntity=false)" + + "/content"; + } + + // Required abstract method implementations + @Override + protected void verifyContentId(String contentId, String attachmentId) { + assertThat(contentId).isEqualTo(attachmentId); + } + + @Override + protected void verifyContent(InputStream attachment, String testContent) throws IOException { + if (Objects.nonNull(testContent)) { + assertThat(attachment.readAllBytes()) + .isEqualTo(testContent.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + } else { + assertThat(attachment).isNull(); + } + } + + @Override + protected void verifyNoAttachmentEventsCalled() { + // no service handler - nothing to do + } + + @Override + protected void clearServiceHandlerContext() { + // no service handler - nothing to do + } + + @Override + protected void verifyEventContextEmptyForEvent(String... events) { + // no service handler - nothing to do + } + + @Override + protected void verifyOnlyTwoCreateEvents( + String newAttachmentContent, String newAttachmentEntityContent) { + // no service handler - nothing to do + } + + @Override + protected void verifyTwoCreateAndDeleteEvents( + String newAttachmentContent, String newAttachmentEntityContent) { + // no service handler - nothing to do + } + + @Override + protected void verifyTwoReadEvents() { + // no service handler - nothing to do + } + + @Override + protected void verifyOnlyTwoDeleteEvents( + String attachmentContentId, String attachmentEntityContentId) { + // no service handler - nothing to do + } + + @Override + protected void verifyTwoUpdateEvents( + String newAttachmentContent, + String attachmentContentId, + String newAttachmentEntityContent, + String attachmentEntityContentId) { + // no service handler - nothing to do + } + + @Override + protected void verifyTwoCreateAndRevertedDeleteEvents() { + // no service handler - nothing to do + } +} diff --git a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/DraftOdataRequestValidationBase.java b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/DraftOdataRequestValidationBase.java index 117726c4e..104b4bc38 100644 --- a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/DraftOdataRequestValidationBase.java +++ b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/draftservice/DraftOdataRequestValidationBase.java @@ -51,7 +51,7 @@ abstract class DraftOdataRequestValidationBase { protected TestPluginAttachmentsServiceHandler serviceHandler; @Autowired protected MockHttpRequestHelper requestHelper; - @Autowired private PersistenceService persistenceService; + @Autowired protected PersistenceService persistenceService; @Autowired private TableDataDeleter dataDeleter; @Autowired private TestPersistenceHandler testPersistenceHandler; diff --git a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/Attachments2SizeValidationNonDraftTest.java b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/Attachments2SizeValidationNonDraftTest.java new file mode 100644 index 000000000..f74fa24b2 --- /dev/null +++ b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/Attachments2SizeValidationNonDraftTest.java @@ -0,0 +1,211 @@ +/* + * © 2024-2024 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ +package com.sap.cds.feature.attachments.integrationtests.nondraftservice; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.sap.cds.feature.attachments.generated.integration.test.cds4j.sap.attachments.Attachments; +import com.sap.cds.feature.attachments.generated.integration.test.cds4j.testservice.AttachmentEntity; +import com.sap.cds.feature.attachments.generated.integration.test.cds4j.testservice.Roots; +import com.sap.cds.feature.attachments.integrationtests.constants.Profiles; +import com.sap.cds.feature.attachments.integrationtests.nondraftservice.helper.AttachmentsBuilder; +import com.sap.cds.feature.attachments.integrationtests.nondraftservice.helper.RootEntityBuilder; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.TimeUnit; +import org.awaitility.Awaitility; +import org.junit.jupiter.api.Test; +import org.springframework.test.context.ActiveProfiles; + +@ActiveProfiles(Profiles.TEST_HANDLER_DISABLED) +class Attachments2SizeValidationNonDraftTest extends OdataRequestValidationBase { + + @Test + void uploadContentWithin5MBLimitSucceeds() throws Exception { + // Arrange: Create root with attachments2 + var serviceRoot = buildServiceRootWithAttachments2(); + postServiceRoot(serviceRoot); + + var selectedRoot = selectStoredRootWithAttachments2(); + var attachment = getRandomRootAttachment2(selectedRoot); + + // Act & Assert: Upload 3MB content (within limit) succeeds + byte[] content = new byte[3 * 1024 * 1024]; // 3MB + var url = buildNavigationAttachment2Url(selectedRoot.getId(), attachment.getId()) + "/content"; + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, content, status().isNoContent()); + } + + @Test + void uploadContentExceeding5MBLimitFails() throws Exception { + // Arrange: Create root with attachments2 + var serviceRoot = buildServiceRootWithAttachments2(); + postServiceRoot(serviceRoot); + + var selectedRoot = selectStoredRootWithAttachments2(); + var attachment = getRandomRootAttachment2(selectedRoot); + + // Act: Try to upload 6MB content (exceeds limit) + byte[] content = new byte[6 * 1024 * 1024]; // 6MB + var url = buildNavigationAttachment2Url(selectedRoot.getId(), attachment.getId()) + "/content"; + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, content, status().is(413)); + + // Assert: Error response with HTTP 413 status code indicates size limit exceeded + } + + @Test + void updateContentStayingWithin5MBLimitSucceeds() throws Exception { + // Arrange: Create root with attachments2 and upload initial content + var serviceRoot = buildServiceRootWithAttachments2(); + postServiceRoot(serviceRoot); + + var selectedRoot = selectStoredRootWithAttachments2(); + var attachment = getRandomRootAttachment2(selectedRoot); + + // Upload initial 2MB content + byte[] initialContent = new byte[2 * 1024 * 1024]; // 2MB + var url = buildNavigationAttachment2Url(selectedRoot.getId(), attachment.getId()) + "/content"; + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, initialContent, status().isNoContent()); + + // Act & Assert: Update with 3MB content (still within limit) succeeds + byte[] updatedContent = new byte[3 * 1024 * 1024]; // 3MB + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, updatedContent, status().isNoContent()); + } + + @Test + void updateContentExceeding5MBLimitFails() throws Exception { + // Arrange: Create root with attachments2 and upload initial content + var serviceRoot = buildServiceRootWithAttachments2(); + postServiceRoot(serviceRoot); + + var selectedRoot = selectStoredRootWithAttachments2(); + var attachment = getRandomRootAttachment2(selectedRoot); + + // Upload initial 2MB content + byte[] initialContent = new byte[2 * 1024 * 1024]; // 2MB + var url = buildNavigationAttachment2Url(selectedRoot.getId(), attachment.getId()) + "/content"; + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, initialContent, status().isNoContent()); + + // Act & Assert: Try to update with 6MB content (exceeds limit) - should fail with HTTP 413 + byte[] updatedContent = new byte[6 * 1024 * 1024]; // 6MB + requestHelper.setContentType(org.springframework.http.MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, updatedContent, status().is(413)); + } + + // Helper methods + private Roots buildServiceRootWithAttachments2() { + return RootEntityBuilder.create() + .setTitle("Root with attachments2") + .addAttachments2( + AttachmentsBuilder.create().setFileName("testFile.txt").setMimeType("text/plain")) + .build(); + } + + private Roots selectStoredRootWithAttachments2() { + var select = + com.sap.cds.ql.Select.from( + com.sap.cds.feature.attachments.generated.integration.test.cds4j.testservice.Roots_ + .class) + .columns(r -> r._all(), r -> r.attachments2().expand()); + + var result = persistenceService.run(select); + return result.single(Roots.class); + } + + // Required abstract method implementations + @Override + protected void executeContentRequestAndValidateContent(String url, String content) + throws Exception { + Awaitility.await() + .atMost(10, TimeUnit.SECONDS) + .until( + () -> { + var response = requestHelper.executeGet(url); + return response.getResponse().getContentAsString().equals(content); + }); + + var response = requestHelper.executeGet(url); + assertThat(response.getResponse().getContentAsString()).isEqualTo(content); + } + + @Override + protected void verifyTwoDeleteEvents( + AttachmentEntity itemAttachmentEntityAfterChange, Attachments itemAttachmentAfterChange) { + // no service handler - nothing to do + } + + @Override + protected void verifyNumberOfEvents(String event, int number) { + // no service handler - nothing to do + } + + @Override + protected void verifyContentId( + Attachments attachmentWithExpectedContent, String attachmentId, String contentId) { + assertThat(attachmentWithExpectedContent.getContentId()).isEqualTo(attachmentId); + } + + @Override + protected void verifyContentAndContentId( + Attachments attachment, String testContent, Attachments itemAttachment) throws IOException { + assertThat(attachment.getContent().readAllBytes()) + .isEqualTo(testContent.getBytes(StandardCharsets.UTF_8)); + assertThat(attachment.getContentId()).isEqualTo(itemAttachment.getId()); + } + + @Override + protected void verifyContentAndContentIdForAttachmentEntity( + AttachmentEntity attachment, String testContent, AttachmentEntity itemAttachment) + throws IOException { + assertThat(attachment.getContent().readAllBytes()) + .isEqualTo(testContent.getBytes(StandardCharsets.UTF_8)); + assertThat(attachment.getContentId()).isEqualTo(itemAttachment.getId()); + } + + @Override + protected void clearServiceHandlerContext() { + // no service handler - nothing to do + } + + @Override + protected void clearServiceHandlerDocuments() { + // no service handler - nothing to do + } + + @Override + protected void verifySingleCreateEvent(String contentId, String content) { + // no service handler - nothing to do + } + + @Override + protected void verifySingleCreateAndUpdateEvent( + String resultContentId, String toBeDeletedContentId, String content) { + // no service handler - nothing to do + } + + @Override + protected void verifySingleDeletionEvent(String contentId) { + // no service handler - nothing to do + } + + @Override + protected void verifySingleReadEvent(String contentId) { + // no service handler - nothing to do + } + + @Override + protected void verifyNoAttachmentEventsCalled() { + // no service handler - nothing to do + } + + @Override + protected void verifyEventContextEmptyForEvent(String... events) { + // no service handler - nothing to do + } +} diff --git a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationBase.java b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationBase.java index 3757d976e..ea17a69ce 100644 --- a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationBase.java +++ b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationBase.java @@ -54,7 +54,7 @@ abstract class OdataRequestValidationBase { protected TestPluginAttachmentsServiceHandler serviceHandler; @Autowired protected MockHttpRequestHelper requestHelper; - @Autowired private PersistenceService persistenceService; + @Autowired protected PersistenceService persistenceService; @Autowired private TableDataDeleter dataDeleter; @Autowired private TestPersistenceHandler testPersistenceHandler; @@ -696,6 +696,10 @@ protected Attachments getRandomItemAttachment(Items selectedItem) { return selectedItem.getAttachments().get(0); } + protected Attachments getRandomRootAttachment2(Roots selectedRoot) { + return selectedRoot.getAttachments2().get(0); + } + private AttachmentEntity getRandomItemAttachmentEntity(Items selectedItem) { return selectedItem.getAttachmentEntities().get(0); } @@ -754,6 +758,30 @@ protected String buildNavigationAttachmentUrl(String rootId, String itemId, Stri + ")"; } + protected String buildNavigationAttachment2Url(String rootId, String attachmentId) { + return "/odata/v4/TestService/Roots(" + + rootId + + ")/attachments2(ID=" + + attachmentId + + ",up__ID=" + + rootId + + ")"; + } + + protected String putContentForAttachment2(Roots selectedRoot, Attachments attachment) + throws Exception { + return putContentForAttachment2(selectedRoot, attachment, status().isNoContent()); + } + + protected String putContentForAttachment2( + Roots selectedRoot, Attachments attachment, ResultMatcher matcher) throws Exception { + var url = buildNavigationAttachment2Url(selectedRoot.getId(), attachment.getId()) + "/content"; + var testContent = "testContent" + attachment.getNote(); + requestHelper.setContentType(MediaType.APPLICATION_OCTET_STREAM); + requestHelper.executePutWithMatcher(url, testContent.getBytes(StandardCharsets.UTF_8), matcher); + return testContent; + } + protected String buildExpandAttachmentUrl(String rootId, String itemId) { return "/odata/v4/TestService/Roots(" + rootId diff --git a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationWithTestHandlerTest.java b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationWithTestHandlerTest.java index 6f9ea09dc..61491794c 100644 --- a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationWithTestHandlerTest.java +++ b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/OdataRequestValidationWithTestHandlerTest.java @@ -225,7 +225,7 @@ private void waitTillExpectedHandlerMessageSize(int expectedSize) { .until( () -> { var eventCalls = serviceHandler.getEventContext().size(); - logger.info( + logger.debug( "Waiting for expected size '{}' in handler context, was '{}'", expectedSize, eventCalls); diff --git a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/helper/RootEntityBuilder.java b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/helper/RootEntityBuilder.java index d7799949b..d7ecc0734 100644 --- a/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/helper/RootEntityBuilder.java +++ b/integration-tests/srv/src/test/java/com/sap/cds/feature/attachments/integrationtests/nondraftservice/helper/RootEntityBuilder.java @@ -32,6 +32,15 @@ public RootEntityBuilder addAttachments(AttachmentsEntityBuilder... attachments) return this; } + public RootEntityBuilder addAttachments2(AttachmentsBuilder... attachments) { + if (rootEntity.getAttachments2() == null) { + rootEntity.setAttachments2(new ArrayList<>()); + } + Arrays.stream(attachments) + .forEach(attachment -> rootEntity.getAttachments2().add(attachment.build())); + return this; + } + public RootEntityBuilder addItems(ItemEntityBuilder... items) { Arrays.stream(items).forEach(item -> rootEntity.getItems().add(item.build())); return this; diff --git a/integration-tests/srv/test-service.cds b/integration-tests/srv/test-service.cds index 8ac47e53f..bd49a3b18 100644 --- a/integration-tests/srv/test-service.cds +++ b/integration-tests/srv/test-service.cds @@ -1,5 +1,9 @@ using test.data.model as db from '../db/data-model'; +annotate db.Roots.attachments2 with { + content @Validation.Maximum: '5MB'; +}; + service TestService { entity Roots as projection on db.Roots; entity AttachmentEntity as projection on db.AttachmentEntity; diff --git a/pom.xml b/pom.xml index 36d9c15bf..19c388c07 100644 --- a/pom.xml +++ b/pom.xml @@ -129,6 +129,12 @@ test + + ch.qos.logback + logback-core + 1.5.23 + + com.sap.cds cds-feature-attachments From 0e26d1aa4cbea350f4ae66f819c63c67e44bc039 Mon Sep 17 00:00:00 2001 From: Marvin Date: Thu, 15 Jan 2026 12:10:31 +0100 Subject: [PATCH 12/15] Apply suggestions from code review Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com> --- .../applicationservice/helper/ExtendedErrorStatuses.java | 5 +++-- integration-tests/db/data-model.cds | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java index 6a5bcf5ce..80c17b3e4 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java @@ -34,13 +34,14 @@ public int getHttpStatus() { * @return the ErrorStatus from this enum, associated with the given code or {@code null} */ public static ErrorStatus getByCode(int code) { - for (ErrorStatus errorStatus : ErrorStatuses.values()) { - if (errorStatus.getHttpStatus() == code) { + for (ExtendedErrorStatuses errorStatus : ExtendedErrorStatuses.values()) { + if (Integer.parseInt(errorStatus.getCodeString()) == code) { return errorStatus; } } return null; } + } @Override public String getCodeString() { diff --git a/integration-tests/db/data-model.cds b/integration-tests/db/data-model.cds index fdca565f3..aae7c285f 100644 --- a/integration-tests/db/data-model.cds +++ b/integration-tests/db/data-model.cds @@ -30,4 +30,4 @@ entity Events : cuid { itemId : UUID; items : Composition of many Items on items.parentID = $self.ID; -} \ No newline at end of file +} From 29dd214728b78d0580a53a265e87b0badcfef95c Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Thu, 15 Jan 2026 12:11:06 +0100 Subject: [PATCH 13/15] improvements --- .../applicationservice/UpdateAttachmentsHandler.java | 12 +----------- .../helper/ModifyApplicationHandlerHelper.java | 9 +++------ 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index ab612cf3d..ba380bea8 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -112,17 +112,7 @@ private void deleteRemovedAttachments( ApplicationHandlerHelper.condenseAttachments(requestData, entity); for (Attachments dbAttachment : dbAttachments) { - Map dbKeys = new HashMap<>(); - entity - .keyElements() - .forEach( - keyElement -> { - String keyName = keyElement.getName(); - Object value = dbAttachment.get(keyName); - if (value != null) { - dbKeys.put(keyName, value); - } - }); + Map dbKeys = ApplicationHandlerHelper.extractKeys(dbAttachment, entity); Map keys = ApplicationHandlerHelper.removeDraftKey(dbKeys); boolean existsInRequest = diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java index b8b9b488f..72736c9dd 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java @@ -13,6 +13,7 @@ import com.sap.cds.feature.attachments.handler.common.ApplicationHandlerHelper; import com.sap.cds.ql.cqn.Path; import com.sap.cds.reflect.CdsEntity; +import com.sap.cds.services.ErrorStatus; import com.sap.cds.services.ErrorStatuses; import com.sap.cds.services.EventContext; import com.sap.cds.services.ServiceException; @@ -84,12 +85,8 @@ public static InputStream handleAttachmentForEntity( } } catch (ArithmeticException e) { throw new ServiceException("Maximum file size value is too large", e); - } catch (RuntimeException e) { - throw new ServiceException( - ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); - } catch (Exception e) { - throw new ServiceException( - ErrorStatuses.BAD_REQUEST, "Failed to process attachment size", e); + } catch (IllegalArgumentException e) { + throw new ServiceException(ErrorStatuses.BAD_REQUEST, "Failed to process attachment size"); } } From eeb811b0b64b67ce5742c0328ad2e98083d32c36 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Thu, 15 Jan 2026 12:21:48 +0100 Subject: [PATCH 14/15] updates --- .../handler/applicationservice/UpdateAttachmentsHandler.java | 1 - .../applicationservice/helper/ExtendedErrorStatuses.java | 4 +--- .../helper/ModifyApplicationHandlerHelper.java | 3 ++- .../applicationservice/helper/ExtendedErrorStatusesTest.java | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java index ba380bea8..3054e1b09 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java @@ -26,7 +26,6 @@ import com.sap.cds.services.request.UserInfo; import com.sap.cds.services.utils.OrderConstants; import com.sap.cds.services.utils.model.CqnUtils; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.slf4j.Logger; diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java index 80c17b3e4..ee898761a 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatuses.java @@ -4,7 +4,6 @@ package com.sap.cds.feature.attachments.handler.applicationservice.helper; import com.sap.cds.services.ErrorStatus; -import com.sap.cds.services.ErrorStatuses; public enum ExtendedErrorStatuses implements ErrorStatus { CONTENT_TOO_LARGE(413, "The content size exceeds the maximum allowed limit.", 413); @@ -34,14 +33,13 @@ public int getHttpStatus() { * @return the ErrorStatus from this enum, associated with the given code or {@code null} */ public static ErrorStatus getByCode(int code) { - for (ExtendedErrorStatuses errorStatus : ExtendedErrorStatuses.values()) { + for (ExtendedErrorStatuses errorStatus : values()) { if (Integer.parseInt(errorStatus.getCodeString()) == code) { return errorStatus; } } return null; } - } @Override public String getCodeString() { diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java index 72736c9dd..42982d2aa 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java @@ -13,7 +13,6 @@ import com.sap.cds.feature.attachments.handler.common.ApplicationHandlerHelper; import com.sap.cds.ql.cqn.Path; import com.sap.cds.reflect.CdsEntity; -import com.sap.cds.services.ErrorStatus; import com.sap.cds.services.ErrorStatuses; import com.sap.cds.services.EventContext; import com.sap.cds.services.ServiceException; @@ -87,6 +86,8 @@ public static InputStream handleAttachmentForEntity( throw new ServiceException("Maximum file size value is too large", e); } catch (IllegalArgumentException e) { throw new ServiceException(ErrorStatuses.BAD_REQUEST, "Failed to process attachment size"); + } catch (RuntimeException e) { + throw new ServiceException(ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); } } diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java index ebd1c747f..74fdd89ce 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ExtendedErrorStatusesTest.java @@ -19,7 +19,7 @@ void contentTooLargeHasCorrectProperties() { @Test void getByCode_existingCode_returnsErrorStatus() { - var result = ExtendedErrorStatuses.getByCode(400); + var result = ExtendedErrorStatuses.getByCode(413); assertThat(result).isNotNull(); } From 1909f5c76bbdde99791557aa44e1419cbbe7fd4f Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Fri, 16 Jan 2026 13:18:08 +0100 Subject: [PATCH 15/15] add stream wrapper --- .../ModifyApplicationHandlerHelper.java | 79 +++++++++++-------- .../readhelper/CountingInputStream.java | 73 +++++++++++++++++ 2 files changed, 117 insertions(+), 35 deletions(-) create mode 100644 cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java index 42982d2aa..23d145db2 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java @@ -10,6 +10,7 @@ import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; import com.sap.cds.feature.attachments.handler.applicationservice.modifyevents.ModifyAttachmentEvent; import com.sap.cds.feature.attachments.handler.applicationservice.modifyevents.ModifyAttachmentEventFactory; +import com.sap.cds.feature.attachments.handler.applicationservice.readhelper.CountingInputStream; import com.sap.cds.feature.attachments.handler.common.ApplicationHandlerHelper; import com.sap.cds.ql.cqn.Path; import com.sap.cds.reflect.CdsEntity; @@ -23,19 +24,19 @@ public final class ModifyApplicationHandlerHelper { - private static final Filter VALMAX_FILTER = - (path, element, type) -> - element.getName().contentEquals("content") - && element.findAnnotation("Validation.Maximum").isPresent(); + private static final Filter VALMAX_FILTER = (path, element, type) -> element.getName().contentEquals("content") + && element.findAnnotation("Validation.Maximum").isPresent(); /** * Handles attachments for entities. * - * @param entity the {@link CdsEntity entity} to handle attachments for - * @param data the given list of {@link CdsData data} + * @param entity the {@link CdsEntity entity} to handle attachments + * for + * @param data the given list of {@link CdsData data} * @param existingAttachments the given list of existing {@link CdsData data} - * @param eventFactory the {@link ModifyAttachmentEventFactory} to create the corresponding event - * @param eventContext the current {@link EventContext} + * @param eventFactory the {@link ModifyAttachmentEventFactory} to create + * the corresponding event + * @param eventContext the current {@link EventContext} */ public static void handleAttachmentForEntities( CdsEntity entity, @@ -43,10 +44,8 @@ public static void handleAttachmentForEntities( List existingAttachments, ModifyAttachmentEventFactory eventFactory, EventContext eventContext) { - Converter converter = - (path, element, value) -> - handleAttachmentForEntity( - existingAttachments, eventFactory, eventContext, path, (InputStream) value); + Converter converter = (path, element, value) -> handleAttachmentForEntity( + existingAttachments, eventFactory, eventContext, path, (InputStream) value); CdsDataProcessor.create() .addConverter(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, converter) @@ -56,11 +55,13 @@ public static void handleAttachmentForEntities( /** * Handles attachments for a single entity. * - * @param existingAttachments the list of existing {@link Attachments} to check against - * @param eventFactory the {@link ModifyAttachmentEventFactory} to create the corresponding event - * @param eventContext the current {@link EventContext} - * @param path the {@link Path} of the attachment - * @param content the content of the attachment + * @param existingAttachments the list of existing {@link Attachments} to check + * against + * @param eventFactory the {@link ModifyAttachmentEventFactory} to create + * the corresponding event + * @param eventContext the current {@link EventContext} + * @param path the {@link Path} of the attachment + * @param content the content of the attachment * @return the processed content as an {@link InputStream} */ public static InputStream handleAttachmentForEntity( @@ -74,28 +75,15 @@ public static InputStream handleAttachmentForEntity( Attachments attachment = getExistingAttachment(keys, existingAttachments); String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); String contentLength = eventContext.getParameterInfo().getHeader("Content-Length"); - String maxSizeStr = getValMaxValue(path.target().entity(), existingAttachments); - - if (maxSizeStr != null && content != null) { - try { - long maxSize = FileSizeUtils.parseFileSizeToBytes(maxSizeStr); - if (contentLength != null && Long.parseLong(contentLength) > maxSize) { - throw new RuntimeException("File size exceeds the maximum allowed size of " + maxSizeStr); - } - } catch (ArithmeticException e) { - throw new ServiceException("Maximum file size value is too large", e); - } catch (IllegalArgumentException e) { - throw new ServiceException(ErrorStatuses.BAD_REQUEST, "Failed to process attachment size"); - } catch (RuntimeException e) { - throw new ServiceException(ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); - } - } + + InputStream wrappedContent = wrapWithCountingStream(content, path.target().entity(), existingAttachments, + contentLength); // for the current request find the event to process - ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(content, contentId, attachment); + ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(wrappedContent, contentId, attachment); // process the event - return eventToProcess.processEvent(path, content, attachment, eventContext); + return eventToProcess.processEvent(path, wrappedContent, attachment, eventContext); } private static String getValMaxValue(CdsEntity entity, List data) { @@ -120,6 +108,27 @@ private static Attachments getExistingAttachment( .orElse(Attachments.create()); } + private static InputStream wrapWithCountingStream( + InputStream content, CdsEntity entity, List data, String contentLength) { + String maxSizeStr = getValMaxValue(entity, data); + + if (maxSizeStr != null && content != null) { + try { + long maxSize = FileSizeUtils.parseFileSizeToBytes(maxSizeStr); + // if (contentLength != null && Long.parseLong(contentLength) > maxSize) { + // throw new RuntimeException(); + // } + return new CountingInputStream(content, maxSize); + } catch (ArithmeticException e) { + throw new ServiceException("Maximum file size value is too large", e); + } catch (RuntimeException e) { + throw new ServiceException( + ExtendedErrorStatuses.CONTENT_TOO_LARGE, "AttachmentSizeExceeded", maxSizeStr); + } + } + return content; + } + private ModifyApplicationHandlerHelper() { // avoid instantiation } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java new file mode 100644 index 000000000..5c503a386 --- /dev/null +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/CountingInputStream.java @@ -0,0 +1,73 @@ +/* + * © 2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ +package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; + +import com.sap.cds.feature.attachments.handler.applicationservice.helper.ExtendedErrorStatuses; +import com.sap.cds.services.ServiceException; +import java.io.IOException; +import java.io.InputStream; + +public class CountingInputStream extends InputStream { + + private final InputStream delegate; + private long byteCount = 0; + private long maxBytes; + + public CountingInputStream(InputStream delegate, long maxBytes) { + this.delegate = delegate; + this.maxBytes = maxBytes; + } + + @Override + public int read() throws IOException { + int b = delegate.read(); + if (b != -1) { + checkLimit(1); + } + return b; + } + + @Override + public int read(byte[] b) throws IOException { + int bytesRead = delegate.read(b); + if (bytesRead > 0) { + checkLimit(bytesRead); + } + return bytesRead; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + int bytesRead = delegate.read(b, off, len); + if (bytesRead > 0) { + checkLimit(bytesRead); + } + return bytesRead; + } + + @Override + public long skip(long n) throws IOException { + long skipped = delegate.skip(n); + if (skipped > 0) { + checkLimit(skipped); + } + return skipped; + } + + @Override + public void close() throws IOException { + if (delegate != null) + delegate.close(); + } + + private void checkLimit(long bytes) { + byteCount += bytes; + if (byteCount > maxBytes) { + throw new ServiceException( + ExtendedErrorStatuses.CONTENT_TOO_LARGE, + "AttachmentSizeExceeded", + maxBytes); + } + } +}