Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.sap.cds.feature.attachments.handler.applicationservice.CreateAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.DeleteAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.ItemsCountValidationHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.ReadAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.UpdateAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.helper.ModifyApplicationHandlerHelper;
Expand All @@ -22,6 +23,7 @@
import com.sap.cds.feature.attachments.handler.draftservice.DraftActiveAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.draftservice.DraftCancelAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.draftservice.DraftPatchAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.draftservice.DraftSaveItemsCountValidationHandler;
import com.sap.cds.feature.attachments.service.AttachmentService;
import com.sap.cds.feature.attachments.service.AttachmentsServiceImpl;
import com.sap.cds.feature.attachments.service.handler.DefaultAttachmentsServiceHandler;
Expand Down Expand Up @@ -133,6 +135,7 @@ public void eventHandlers(CdsRuntimeConfigurer configurer) {
configurer.eventHandler(
new ReadAttachmentsHandler(
attachmentService, new AttachmentStatusValidator(), scanRunner));
configurer.eventHandler(new ItemsCountValidationHandler());
} else {
logger.debug(
"No application service is available. Application service event handlers will not be registered.");
Expand All @@ -146,6 +149,7 @@ public void eventHandlers(CdsRuntimeConfigurer configurer) {
new DraftPatchAttachmentsHandler(persistenceService, eventFactory, defaultMaxSize));
configurer.eventHandler(new DraftCancelAttachmentsHandler(attachmentsReader, deleteEvent));
configurer.eventHandler(new DraftActiveAttachmentsHandler(storage));
configurer.eventHandler(new DraftSaveItemsCountValidationHandler());
} else {
logger.debug("No draft service is available. Draft event handlers will not be registered.");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* © 2024-2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors.
*/
package com.sap.cds.feature.attachments.handler.applicationservice;

import com.sap.cds.CdsData;
import com.sap.cds.feature.attachments.handler.common.ItemCountValidationHelper;
import com.sap.cds.services.cds.ApplicationService;
import com.sap.cds.services.cds.CdsCreateEventContext;
import com.sap.cds.services.cds.CdsUpdateEventContext;
import com.sap.cds.services.handler.EventHandler;
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 java.util.List;

/**
* The class {@link ItemsCountValidationHandler} validates {@code @Validation.MinItems} and
* {@code @Validation.MaxItems} annotations on attachment compositions during CREATE and UPDATE
* events. In draft mode (entity not yet activated) a warning is issued; on active entities an error
* is raised.
*/
@ServiceName(value = "*", type = ApplicationService.class)
public class ItemsCountValidationHandler implements EventHandler {

@Before
@HandlerOrder(HandlerOrder.LATE)
void processCreateBefore(CdsCreateEventContext context, List<CdsData> data) {
boolean isDraft = isDraftData(data);
ItemCountValidationHelper.validateItemCounts(
context.getTarget(), data, isDraft, context.getMessages());
}

@Before
@HandlerOrder(HandlerOrder.LATE)
void processUpdateBefore(CdsUpdateEventContext context, List<CdsData> data) {
boolean isDraft = isDraftData(data);
ItemCountValidationHelper.validateItemCounts(
context.getTarget(), data, isDraft, context.getMessages());
}

private boolean isDraftData(List<CdsData> data) {
return data.stream().anyMatch(d -> Boolean.FALSE.equals(d.get("IsActiveEntity")));
Comment on lines +42 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic Error: isDraft detection uses anyMatch, so a mixed batch of draft and active records is treated as all-draft

If a data list contains both draft records (IsActiveEntity=false) and active records (IsActiveEntity=true or absent), anyMatch returns true and all records in the batch are validated with isDraft=true (warnings only). Active records in the same batch would silently miss the required error.

The draft/active distinction should be evaluated per record, not once for the whole list. This is closely related to the per-record aggregation fix in ItemCountValidationHelper; passing isDraft per row into the helper (or moving the isDraft check inside the helper loop) would resolve both issues.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* © 2024-2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors.
*/
package com.sap.cds.feature.attachments.handler.common;

import com.sap.cds.CdsData;
import com.sap.cds.reflect.CdsEntity;
import com.sap.cds.services.messages.Messages;
import java.util.List;

public final class ItemCountValidationHelper {

private static final String MIN_ITEMS_KEY = "attachment_minItems";
private static final String MAX_ITEMS_KEY = "attachment_maxItems";

public static void validateItemCounts(
CdsEntity entity, List<? extends CdsData> data, boolean isDraft, Messages messages) {
entity
.compositions()
.forEach(
composition -> {
String compositionName = composition.getName();

// only validate if the composition was included in the payload
boolean presentInPayload =
data.stream().anyMatch(d -> d.containsKey(compositionName));
if (!presentInPayload) {
return;
}

long count =
data.stream()
.filter(d -> d.containsKey(compositionName))
.mapToLong(
d -> {
Object val = d.get(compositionName);
return val instanceof List<?> list ? list.size() : 0L;
})
.sum();

composition
.<Object>findAnnotation("Validation.MinItems")
.ifPresent(
annotation -> {
Object value = annotation.getValue();
if (value instanceof Boolean)
return; // bare annotation without value → skip
int limit = ((Number) value).intValue();
if (count < limit) {
String msgKey =
MIN_ITEMS_KEY + "_" + entity.getName() + "_" + compositionName;
issueMessage(
isDraft,
messages,
msgKey,
limit,
entity.getQualifiedName(),
compositionName);
}
});

composition
.<Object>findAnnotation("Validation.MaxItems")
.ifPresent(
annotation -> {
Object value = annotation.getValue();
if (value instanceof Boolean)
return; // bare annotation without value → skip
int limit = ((Number) value).intValue();
if (count > limit) {
String msgKey =
MAX_ITEMS_KEY + "_" + entity.getName() + "_" + compositionName;
issueMessage(
isDraft,
messages,
msgKey,
limit,
entity.getQualifiedName(),
compositionName);
}
});
});
}

private static void issueMessage(
boolean isDraft,
Messages messages,
String msgKey,
int limit,
String entityQualifiedName,
String compositionName) {
if (isDraft) {
messages.warn(msgKey, limit).target(entityQualifiedName, e -> e.to(compositionName));
} else {
messages.error(msgKey, limit).target(entityQualifiedName, e -> e.to(compositionName));
}
}

private ItemCountValidationHelper() {
// avoid instantiation
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* © 2024-2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors.
*/
package com.sap.cds.feature.attachments.handler.draftservice;

import com.sap.cds.CdsData;
import com.sap.cds.feature.attachments.handler.common.ItemCountValidationHelper;
import com.sap.cds.services.draft.DraftSaveEventContext;
import com.sap.cds.services.draft.DraftService;
import com.sap.cds.services.handler.EventHandler;
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 java.util.List;

/**
* The class {@link DraftSaveItemsCountValidationHandler} validates {@code @Validation.MinItems} and
* {@code @Validation.MaxItems} annotations on attachment compositions when a draft is saved
* (activated). As draft activation transitions to an active entity, violations always raise errors.
*/
@ServiceName(value = "*", type = DraftService.class)
public class DraftSaveItemsCountValidationHandler implements EventHandler {

@Before
@HandlerOrder(HandlerOrder.LATE)
void processBeforeDraftSave(DraftSaveEventContext context, List<CdsData> data) {
// isDraft=false: saving a draft to active must enforce the constraint as an error
ItemCountValidationHelper.validateItemCounts(
context.getTarget(), data, false, context.getMessages());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,7 @@ attachment_note=Note
attachment=Attachment
#XTIT: Header label for Attachments
attachments=Attachments
#XMSG: Error message when too few attachments are provided (min items validation)
attachment_minItems=A minimum of {0} attachments is required.
#XMSG: Error message when too many attachments are provided (max items validation)
attachment_maxItems=A maximum of {0} attachments is allowed.
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@

import com.sap.cds.feature.attachments.handler.applicationservice.CreateAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.DeleteAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.ItemsCountValidationHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.ReadAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.applicationservice.UpdateAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.draftservice.DraftActiveAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.draftservice.DraftCancelAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.draftservice.DraftPatchAttachmentsHandler;
import com.sap.cds.feature.attachments.handler.draftservice.DraftSaveItemsCountValidationHandler;
import com.sap.cds.feature.attachments.service.AttachmentService;
import com.sap.cds.feature.attachments.service.handler.DefaultAttachmentsServiceHandler;
import com.sap.cds.feature.attachments.service.malware.DefaultAttachmentMalwareScanner;
Expand Down Expand Up @@ -108,7 +110,7 @@ void handlersAreRegistered() {

cut.eventHandlers(configurer);

var handlerSize = 8;
var handlerSize = 10;
verify(configurer, times(handlerSize)).eventHandler(handlerArgumentCaptor.capture());
checkHandlers(handlerArgumentCaptor.getAllValues(), handlerSize);
}
Expand All @@ -128,7 +130,7 @@ void handlersAreRegisteredWithoutOutboxService() {

cut.eventHandlers(configurer);

var handlerSize = 8;
var handlerSize = 10;
verify(configurer, times(handlerSize)).eventHandler(handlerArgumentCaptor.capture());
checkHandlers(handlerArgumentCaptor.getAllValues(), handlerSize);
}
Expand All @@ -143,6 +145,8 @@ private void checkHandlers(List<EventHandler> handlers, int handlerSize) {
isHandlerForClassIncluded(handlers, DraftPatchAttachmentsHandler.class);
isHandlerForClassIncluded(handlers, DraftCancelAttachmentsHandler.class);
isHandlerForClassIncluded(handlers, DraftActiveAttachmentsHandler.class);
isHandlerForClassIncluded(handlers, ItemsCountValidationHandler.class);
isHandlerForClassIncluded(handlers, DraftSaveItemsCountValidationHandler.class);
}

@Test
Expand Down
Loading
Loading