From 6ccd298bff44bd6692702d66e742ce5cad07ca4e Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 30 Mar 2026 17:38:34 +0200 Subject: [PATCH 1/3] fix --- .../DefaultAttachmentMalwareScanner.java | 71 ++++++++++++------- .../DefaultAttachmentMalwareScannerTest.java | 68 +++++++++++++++--- 2 files changed, 103 insertions(+), 36 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java index 552161aab..0f122d1c2 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java @@ -73,32 +73,51 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { contentId, attachmentEntity.getQualifiedName()); - List selectionResult = selectData(attachmentEntity, contentId); - - selectionResult.forEach( - result -> { - long rowCount = result.result().rowCount(); - if (rowCount <= 0) { - logger.debug( - "No attachments {} found in entity {}, nothing to scan.", - contentId, - result.entity.getQualifiedName()); - return; - } - - if (rowCount > 1) { - logger.warn( - "More than one attachment {} found in entity {}.", - contentId, - result.entity.getQualifiedName()); - throw new IllegalStateException( - "More than one attachment with contentId %s.".formatted(contentId)); - } - - Attachments attachment = result.result().single(Attachments.class); - MalwareScanResultStatus status = scanDocument(attachment); - updateData(result.entity, contentId, status); - }); + List selectionResults = selectData(attachmentEntity, contentId); + + MalwareScanResultStatus status = findAndScanAttachment(selectionResults, contentId); + + if (status == null) { + logger.debug("Attachment {} not found in any entity, skipping update.", contentId); + return; + } + + // Update ALL candidate entities. This ensures the scan result is persisted + // even if the attachment moved between draft and active tables during the scan. + for (SelectionResult result : selectionResults) { + updateData(result.entity, contentId, status); + } + } + + private MalwareScanResultStatus findAndScanAttachment( + List selectionResults, String contentId) { + return selectionResults.stream() + .filter(result -> validateAndFilter(result, contentId)) + .findFirst() + .map(result -> scanDocument(result.result().single(Attachments.class))) + .orElse(null); + } + + private boolean validateAndFilter(SelectionResult result, String contentId) { + long rowCount = result.result().rowCount(); + if (rowCount <= 0) { + logger.debug( + "No attachments {} found in entity {}, nothing to scan.", + contentId, + result.entity.getQualifiedName()); + return false; + } + + if (rowCount > 1) { + logger.warn( + "More than one attachment {} found in entity {}.", + contentId, + result.entity.getQualifiedName()); + throw new IllegalStateException( + "More than one attachment with contentId %s.".formatted(contentId)); + } + + return true; } private List selectData(CdsEntity attachmentEntity, String contentId) { diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java index 41c68ce0f..41d07f641 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java @@ -181,7 +181,7 @@ void contentTakenFromTheDatabaseSelect() { cut.scanAttachment(entity.orElseThrow(), ""); - verify(malwareScanClient, times(2)).scanContent(content); + verify(malwareScanClient, times(1)).scanContent(content); verifyNoInteractions(attachmentService); } @@ -197,8 +197,8 @@ void contentTakenFromTheAttachmentService() { cut.scanAttachment(entity.orElseThrow(), ""); - verify(attachmentService, times(2)).readAttachment(contentId); - verify(malwareScanClient, times(2)).scanContent(content); + verify(attachmentService, times(1)).readAttachment(contentId); + verify(malwareScanClient, times(1)).scanContent(content); } @Test @@ -232,9 +232,10 @@ void noDataReturnedForUpdateNothingDoneForNonDraftEntity() { cut.scanAttachment(entity.orElseThrow(), "ID"); - verify(persistenceService).run(updateCaptor.capture()); - var update = updateCaptor.getValue(); - assertThat(update.ref().toString()).contains(entity.get().getQualifiedName()); + verify(persistenceService, times(2)).run(updateCaptor.capture()); + var updateList = updateCaptor.getAllValues(); + assertThat(updateList).hasSize(2); + assertThat(updateList.get(0).ref().toString()).contains(entity.get().getQualifiedName()); } @Test @@ -251,12 +252,11 @@ void clientNotCalledIfNoInstanceBound() { cut.scanAttachment(entity.orElseThrow(), "ID"); verifyNoInteractions(malwareScanClient); - verify(persistenceService).run(updateCaptor.capture()); + verify(persistenceService, times(2)).run(updateCaptor.capture()); var updateList = updateCaptor.getAllValues(); assertThat(updateList) - .hasSize(1) - .first() - .satisfies( + .hasSize(2) + .allSatisfy( update -> { assertThat(update.entries()).hasSize(1); assertThat(update.entries().get(0)) @@ -264,6 +264,54 @@ void clientNotCalledIfNoInstanceBound() { }); } + @Test + void scanResultWrittenToAllEntitiesEvenIfDraftDeletedDuringScanning() { + var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); + var content = mock(InputStream.class); + var draftData = Attachments.create(); + draftData.setContent(content); + // Phase 1: draft has the row, active does not + when(persistenceService.run(any(CqnSelect.class))).thenReturn(result); + when(result.rowCount()).thenReturn(1L).thenReturn(0L); + when(result.single(Attachments.class)).thenReturn(draftData); + when(malwareScanClient.scanContent(any())).thenReturn(MalwareScanResultStatus.CLEAN); + // Phase 2: simulate draft deleted (0 rows updated), active now has the row (1 row updated) + var draftUpdateResult = mock(Result.class); + when(draftUpdateResult.rowCount()).thenReturn(0L); + var activeUpdateResult = mock(Result.class); + when(activeUpdateResult.rowCount()).thenReturn(1L); + when(persistenceService.run(any(CqnUpdate.class))) + .thenReturn(draftUpdateResult) + .thenReturn(activeUpdateResult); + + cut.scanAttachment(entity.orElseThrow(), "ID"); + + // Scan should happen once from draft content + verify(malwareScanClient).scanContent(content); + // Updates should be attempted on both entities + verify(persistenceService, times(2)).run(updateCaptor.capture()); + var updates = updateCaptor.getAllValues(); + assertThat(updates).hasSize(2); + updates.forEach( + update -> { + assertThat(update.entries()).hasSize(1); + assertThat(update.entries().get(0)).containsEntry(Attachments.STATUS, StatusCode.CLEAN); + }); + } + + @Test + void noScanOrUpdateWhenAttachmentNotFoundInAnyEntity() { + var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); + var emptyResult = mock(Result.class); + when(emptyResult.rowCount()).thenReturn(0L); + when(persistenceService.run(any(CqnSelect.class))).thenReturn(emptyResult); + + cut.scanAttachment(entity.orElseThrow(), "ID"); + + verifyNoInteractions(malwareScanClient); + verify(persistenceService, times(0)).run(any(CqnUpdate.class)); + } + @Test void mapStatus() { assertEquals( From f742fddbea51a3ccd5de867b741d197b2af4dce8 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 30 Mar 2026 17:46:08 +0200 Subject: [PATCH 2/3] Rename misleading test to match new behavior --- .../service/malware/DefaultAttachmentMalwareScannerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java index 41d07f641..343cd6b15 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScannerTest.java @@ -218,7 +218,7 @@ void contentTakenFromTheAttachmentServiceForNonDraft() { } @Test - void noDataReturnedForUpdateNothingDoneForNonDraftEntity() { + void updateAttemptedForAllEntitiesEvenWhenActiveHasNoData() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); when(persistenceService.run(any(CqnSelect.class))).thenReturn(result); when(result.rowCount()).thenReturn(1L).thenReturn(0L); From 69693d690bd3c04b8897e6ea485a2da513cfe40c Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 31 Mar 2026 11:32:49 +0200 Subject: [PATCH 3/3] address review comment --- .../service/malware/DefaultAttachmentMalwareScanner.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java index 0f122d1c2..7b53c6484 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java @@ -75,7 +75,7 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { List selectionResults = selectData(attachmentEntity, contentId); - MalwareScanResultStatus status = findAndScanAttachment(selectionResults, contentId); + MalwareScanResultStatus status = findAndScanAttachments(selectionResults, contentId); if (status == null) { logger.debug("Attachment {} not found in any entity, skipping update.", contentId); @@ -89,7 +89,7 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { } } - private MalwareScanResultStatus findAndScanAttachment( + private MalwareScanResultStatus findAndScanAttachments( List selectionResults, String contentId) { return selectionResults.stream() .filter(result -> validateAndFilter(result, contentId))