From 31aa5c1293a2c8b118602e4784af6ce6cec3199d Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 13 Apr 2026 22:08:21 +0200 Subject: [PATCH 1/2] Enhance DefaultAttachmentsServiceHandler test coverage Add five focused unit tests for edge cases and explicit behavior verification in AttachmentsServiceImplHandlerTest: - Explicit contentId mapping from attachmentIds map - Empty attachmentIds map handled gracefully (contentId is null) - ChangeSetListener registration verified with non-null context - Missing ChangeSetContext throws NullPointerException - Focused assertion on SCANNING status and isInternalStored flag --- .../AttachmentsServiceImplHandlerTest.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java index cbfd67a5d..75afa72e1 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java @@ -4,6 +4,8 @@ package com.sap.cds.feature.attachments.service.handler; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -23,6 +25,7 @@ import com.sap.cds.services.handler.annotations.On; import com.sap.cds.services.handler.annotations.ServiceName; import com.sap.cds.services.impl.changeset.ChangeSetContextImpl; +import java.util.Collections; import java.util.Map; import java.util.Objects; import org.junit.jupiter.api.AfterEach; @@ -163,6 +166,92 @@ void malwareScannerRegisteredForEndOfTransaction() { verify(malwareScanProvider).getChangeSetListener(entity, "contentId"); } + @Test + void createAttachment_setsContentIdFromAttachmentIds() { + var expectedContentId = "unique-attachment-id-123"; + var createContext = AttachmentCreateEventContext.create(); + createContext.setAttachmentIds( + Map.of(Attachments.ID, expectedContentId, "someOtherKey", "irrelevant-value")); + createContext.setData(MediaData.create()); + createContext.setAttachmentEntity(mock(CdsEntity.class)); + ChangeSetContextImpl.open(false); + + cut.createAttachment(createContext); + + assertThat(createContext.getContentId()) + .as("contentId must be set from the Attachments.ID key in attachmentIds map") + .isEqualTo(expectedContentId); + } + + @Test + void createAttachment_emptyAttachmentIds_handlesGracefully() { + var createContext = AttachmentCreateEventContext.create(); + createContext.setAttachmentIds(Collections.emptyMap()); + createContext.setData(MediaData.create()); + createContext.setAttachmentEntity(mock(CdsEntity.class)); + ChangeSetContextImpl.open(false); + + cut.createAttachment(createContext); + + assertThat(createContext.getContentId()) + .as("contentId should be null when Attachments.ID key is missing from attachmentIds") + .isNull(); + assertThat(createContext.isCompleted()).isTrue(); + } + + @Test + void afterCreateAttachment_registersChangeSetListener() { + var listener = mock(ChangeSetListener.class); + var entity = mock(CdsEntity.class); + when(malwareScanProvider.getChangeSetListener(entity, "scan-id")).thenReturn(listener); + var createContext = AttachmentCreateEventContext.create(); + createContext.setAttachmentIds(Map.of(Attachments.ID, "scan-id")); + createContext.setData(MediaData.create()); + createContext.setAttachmentEntity(entity); + ChangeSetContextImpl.open(false); + + cut.createAttachment(createContext); + cut.afterCreateAttachment(createContext); + + var changeSetContext = createContext.getChangeSetContext(); + assertThat(changeSetContext).isNotNull(); + verify(malwareScanProvider).getChangeSetListener(entity, "scan-id"); + } + + @Test + void afterCreateAttachment_noChangeSetContext_throws() { + var listener = mock(ChangeSetListener.class); + var entity = mock(CdsEntity.class); + when(malwareScanProvider.getChangeSetListener(any(), any())).thenReturn(listener); + var createContext = AttachmentCreateEventContext.create(); + createContext.setAttachmentIds(Map.of(Attachments.ID, "some-id")); + createContext.setData(MediaData.create()); + createContext.setAttachmentEntity(entity); + + cut.createAttachment(createContext); + + assertThatThrownBy(() -> cut.afterCreateAttachment(createContext)) + .isInstanceOf(NullPointerException.class); + } + + @Test + void createAttachment_verifyStatusAndInternalStored() { + var createContext = AttachmentCreateEventContext.create(); + createContext.setAttachmentIds(Map.of(Attachments.ID, "any-id")); + createContext.setData(MediaData.create()); + createContext.setAttachmentEntity(mock(CdsEntity.class)); + ChangeSetContextImpl.open(false); + + cut.createAttachment(createContext); + + assertThat(createContext.getData().getStatus()) + .as("status must be set to SCANNING") + .isEqualTo(StatusCode.SCANNING); + assertThat(createContext.getIsInternalStored()) + .as("isInternalStored must be true for default DB storage") + .isTrue(); + } + private void closeChangeSetContext() throws Exception { var context = ChangeSetContextImpl.getCurrent(); if (Objects.nonNull(context)) { From 24d635e0fa2ab8238fc31c3eb64fc4cfe07cb7d6 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 13 Apr 2026 22:26:12 +0200 Subject: [PATCH 2/2] Address review: remove duplicate tests, clean up unused variable --- .../AttachmentsServiceImplHandlerTest.java | 58 +------------------ 1 file changed, 2 insertions(+), 56 deletions(-) diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java index 75afa72e1..e91c404de 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java @@ -166,23 +166,6 @@ void malwareScannerRegisteredForEndOfTransaction() { verify(malwareScanProvider).getChangeSetListener(entity, "contentId"); } - @Test - void createAttachment_setsContentIdFromAttachmentIds() { - var expectedContentId = "unique-attachment-id-123"; - var createContext = AttachmentCreateEventContext.create(); - createContext.setAttachmentIds( - Map.of(Attachments.ID, expectedContentId, "someOtherKey", "irrelevant-value")); - createContext.setData(MediaData.create()); - createContext.setAttachmentEntity(mock(CdsEntity.class)); - ChangeSetContextImpl.open(false); - - cut.createAttachment(createContext); - - assertThat(createContext.getContentId()) - .as("contentId must be set from the Attachments.ID key in attachmentIds map") - .isEqualTo(expectedContentId); - } - @Test void createAttachment_emptyAttachmentIds_handlesGracefully() { var createContext = AttachmentCreateEventContext.create(); @@ -199,30 +182,11 @@ void createAttachment_emptyAttachmentIds_handlesGracefully() { assertThat(createContext.isCompleted()).isTrue(); } - @Test - void afterCreateAttachment_registersChangeSetListener() { - var listener = mock(ChangeSetListener.class); - var entity = mock(CdsEntity.class); - when(malwareScanProvider.getChangeSetListener(entity, "scan-id")).thenReturn(listener); - var createContext = AttachmentCreateEventContext.create(); - createContext.setAttachmentIds(Map.of(Attachments.ID, "scan-id")); - createContext.setData(MediaData.create()); - createContext.setAttachmentEntity(entity); - ChangeSetContextImpl.open(false); - - cut.createAttachment(createContext); - cut.afterCreateAttachment(createContext); - - var changeSetContext = createContext.getChangeSetContext(); - assertThat(changeSetContext).isNotNull(); - verify(malwareScanProvider).getChangeSetListener(entity, "scan-id"); - } - @Test void afterCreateAttachment_noChangeSetContext_throws() { - var listener = mock(ChangeSetListener.class); var entity = mock(CdsEntity.class); - when(malwareScanProvider.getChangeSetListener(any(), any())).thenReturn(listener); + when(malwareScanProvider.getChangeSetListener(any(), any())) + .thenReturn(mock(ChangeSetListener.class)); var createContext = AttachmentCreateEventContext.create(); createContext.setAttachmentIds(Map.of(Attachments.ID, "some-id")); createContext.setData(MediaData.create()); @@ -234,24 +198,6 @@ void afterCreateAttachment_noChangeSetContext_throws() { .isInstanceOf(NullPointerException.class); } - @Test - void createAttachment_verifyStatusAndInternalStored() { - var createContext = AttachmentCreateEventContext.create(); - createContext.setAttachmentIds(Map.of(Attachments.ID, "any-id")); - createContext.setData(MediaData.create()); - createContext.setAttachmentEntity(mock(CdsEntity.class)); - ChangeSetContextImpl.open(false); - - cut.createAttachment(createContext); - - assertThat(createContext.getData().getStatus()) - .as("status must be set to SCANNING") - .isEqualTo(StatusCode.SCANNING); - assertThat(createContext.getIsInternalStored()) - .as("isInternalStored must be true for default DB storage") - .isTrue(); - } - private void closeChangeSetContext() throws Exception { var context = ChangeSetContextImpl.getCurrent(); if (Objects.nonNull(context)) {