From a75e5293292cde05e73179dd8ee29c679c126728 Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 11:28:09 +0200 Subject: [PATCH 01/11] Revert "Revert "scan attachments only in certain scan states"" This reverts commit bc3c472e42ea4163ccdf6a8f959b4602de2a9c32. --- .../DefaultAttachmentMalwareScanner.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 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 3a6f2369..9d5b6c87 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 @@ -81,13 +81,13 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { } if (rowCount > 1) { - logger.warn("More than one attachment found for document id: {} in entity: {}", contentId, + logger.warn("More than one attachment found for contentId: {} in entity: {}", contentId, result.entity.getQualifiedName()); throw new IllegalStateException("More than one attachment found for document id: " + contentId); } - CdsData cdsData = result.result().single(CdsData.class); - MalwareScanResultStatus status = scanDocument(cdsData); + Attachments attachment = result.result().single(Attachments.class); + MalwareScanResultStatus status = scanDocument(attachment); updateData(result.entity, contentId, status); }); @@ -110,20 +110,22 @@ private List selectData(CdsEntity attachmentEntity, String cont private Result readData(String contentId, CdsEntity entity) { CqnSelect select = Select.from(entity).columns(Attachments.CONTENT_ID, Attachments.CONTENT) - .where(entry -> entry.get(Attachments.CONTENT_ID).eq(contentId)); + .where(e -> e.get(Attachments.CONTENT_ID).eq(contentId).and(e.get(Attachments.STATUS) + .in(StatusCode.FAILED, StatusCode.UNSCANNED, StatusCode.INFECTED, StatusCode.SCANNING))); + +// CqnSelect select2 = Select.from(entity, r -> r.filter(e -> e.get(Attachments.CONTENT_ID).eq(contentId) +// .and(e.get(Attachments.STATUS).in(StatusCode.FAILED, StatusCode.UNSCANNED, StatusCode.INFECTED)))); return persistenceService.run(select); } - private MalwareScanResultStatus scanDocument(CdsData data) { + private MalwareScanResultStatus scanDocument(Attachments attachment) { if (malwareScanClient != null) { - String contentId = (String) data.get(Attachments.CONTENT_ID); - InputStream dbContent = (InputStream) data.get(Attachments.CONTENT); try { - InputStream content = Objects.nonNull(dbContent) ? dbContent - : attachmentService.readAttachment(contentId); + InputStream content = Objects.nonNull(attachment.getContent()) ? attachment.getContent() + : attachmentService.readAttachment(attachment.getContentId()); return malwareScanClient.scanContent(content); } catch (RuntimeException e) { - logger.error("Error while scanning document with document id: {}", contentId, e); + logger.error("Error while scanning document with contentId: {}", attachment.getContentId(), e); return MalwareScanResultStatus.FAILED; } } From 5b5506a0c1a420e8c5f40b8a2b364a35c4f6b9e2 Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 11:47:00 +0200 Subject: [PATCH 02/11] fixed some tests --- .../DefaultAttachmentMalwareScanner.java | 2 +- .../DefaultAttachmentMalwareScannerTest.java | 31 +++++++++---------- 2 files changed, 16 insertions(+), 17 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 9d5b6c87..18a10c42 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 @@ -83,7 +83,7 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { if (rowCount > 1) { logger.warn("More than one attachment found for contentId: {} in entity: {}", contentId, result.entity.getQualifiedName()); - throw new IllegalStateException("More than one attachment found for document id: " + contentId); + throw new IllegalStateException("More than one attachment found for contentId: " + contentId); } Attachments attachment = result.result().single(Attachments.class); 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 ba8f3b04..594f3a76 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 @@ -19,7 +19,6 @@ import org.junit.jupiter.params.provider.EnumSource; import org.mockito.ArgumentCaptor; -import com.sap.cds.CdsData; import com.sap.cds.Result; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.StatusCode; @@ -80,7 +79,7 @@ void correctSelectForNonDraftEntity() { @Test void correctSelectForDraftEntity() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); - mockSelectResult(CdsData.create(), MalwareScanResultStatus.CLEAN); + mockSelectResult(Attachments.create(), MalwareScanResultStatus.CLEAN); cut.scanAttachment(entity.orElseThrow(), "ID"); @@ -100,9 +99,9 @@ void fallbackToActiveEntityIfDraftHasNoData() { when(persistenceService.run(any(CqnSelect.class))).thenReturn(emptyResult).thenReturn(result); when(result.rowCount()).thenReturn(1L); var content = mock(InputStream.class); - var cdsData = CdsData.create(); + var cdsData = Attachments.create(); cdsData.put(Attachments.CONTENT, content); - when(result.single(CdsData.class)).thenReturn(cdsData); + when(result.single(Attachments.class)).thenReturn(cdsData); when(malwareScanClient.scanContent(any())).thenReturn(MalwareScanResultStatus.CLEAN); cut.scanAttachment(entity.orElseThrow(), "ID"); @@ -130,7 +129,7 @@ void exceptionIfTooManyResultsAreSelected() { @EnumSource(MalwareScanResultStatus.class) void dataAreUpdatedWithStatus(MalwareScanResultStatus status) { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); - mockSelectResult(CdsData.create(), status); + mockSelectResult(Attachments.create(), status); cut.scanAttachment(entity.orElseThrow(), "ID"); @@ -142,7 +141,7 @@ void dataAreUpdatedWithStatusFromFailingScanClient() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); when(persistenceService.run(any(CqnSelect.class))).thenReturn(result); when(result.rowCount()).thenReturn(1L); - when(result.single(CdsData.class)).thenReturn(CdsData.create()); + when(result.single(Attachments.class)).thenReturn(Attachments.create()); when(malwareScanClient.scanContent(any())).thenThrow(new ServiceException("Error reading attachment")); cut.scanAttachment(entity.orElseThrow(), "ID"); @@ -155,7 +154,7 @@ void dataAreUpdatedWithStatusFromFailingAttachmentService() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); when(persistenceService.run(any(CqnSelect.class))).thenReturn(result); when(result.rowCount()).thenReturn(1L); - when(result.single(CdsData.class)).thenReturn(CdsData.create()); + when(result.single(Attachments.class)).thenReturn(Attachments.create()); when(attachmentService.readAttachment(any())).thenThrow(new ServiceException("Error reading attachment")); cut.scanAttachment(entity.orElseThrow(), "ID"); @@ -167,7 +166,7 @@ void dataAreUpdatedWithStatusFromFailingAttachmentService() { void contentTakenFromTheDatabaseSelect() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); var content = mock(InputStream.class); - var data = CdsData.create(); + var data = Attachments.create(); data.put("content", content); mockSelectResult(data, MalwareScanResultStatus.CLEAN); @@ -181,7 +180,7 @@ void contentTakenFromTheDatabaseSelect() { void contentTakenFromTheAttachmentService() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); var contentId = "contentId"; - var data = CdsData.create(); + var data = Attachments.create(); data.put(Attachments.CONTENT_ID, contentId); mockSelectResult(data, MalwareScanResultStatus.CLEAN); var content = mock(InputStream.class); @@ -197,7 +196,7 @@ void contentTakenFromTheAttachmentService() { void contentTakenFromTheAttachmentServiceForNonDraft() { var entity = runtime.getCdsModel().findEntity(Attachment_.CDS_NAME); var contentId = "contentId"; - var data = CdsData.create(); + var data = Attachments.create(); data.put(Attachments.CONTENT_ID, contentId); mockSelectResult(data, MalwareScanResultStatus.CLEAN); var content = mock(InputStream.class); @@ -214,10 +213,10 @@ void noDataReturnedForUpdateNothingDoneForNonDraftEntity() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); when(persistenceService.run(any(CqnSelect.class))).thenReturn(result); when(result.rowCount()).thenReturn(1L).thenReturn(0L); - var originSelectionData = CdsData.create(); + var originSelectionData = Attachments.create(); originSelectionData.put(Attachments.CONTENT_ID, "first contentId"); originSelectionData.put(Attachments.CONTENT, mock(InputStream.class)); - when(result.single(CdsData.class)).thenReturn(originSelectionData).thenReturn(CdsData.create()); + when(result.single(Attachments.class)).thenReturn(originSelectionData).thenReturn(Attachments.create()); when(malwareScanClient.scanContent(any())).thenReturn(MalwareScanResultStatus.CLEAN); cut.scanAttachment(entity.orElseThrow(), "ID"); @@ -233,10 +232,10 @@ void clientNotCalledIfNoInstanceBound() { var entity = runtime.getCdsModel().findEntity(getTestServiceAttachmentName()); var secondResult = mock(Result.class); when(secondResult.rowCount()).thenReturn(0L); - when(secondResult.single(CdsData.class)).thenReturn(CdsData.create()); + when(secondResult.single(Attachments.class)).thenReturn(Attachments.create()); when(persistenceService.run(any(CqnSelect.class))).thenReturn(result).thenReturn(secondResult); when(result.rowCount()).thenReturn(1L); - when(result.single(CdsData.class)).thenReturn(CdsData.create()); + when(result.single(Attachments.class)).thenReturn(Attachments.create()); cut.scanAttachment(entity.orElseThrow(), "ID"); @@ -273,10 +272,10 @@ private String getTestServiceAttachmentName() { return com.sap.cds.feature.attachments.generated.test.cds4j.unit.test.testservice.Attachment_.CDS_NAME; } - private void mockSelectResult(CdsData cdsData, MalwareScanResultStatus status) { + private void mockSelectResult(Attachments cdsData, MalwareScanResultStatus status) { when(persistenceService.run(any(CqnSelect.class))).thenReturn(result); when(result.rowCount()).thenReturn(1L); - when(result.single(CdsData.class)).thenReturn(cdsData); + when(result.single(Attachments.class)).thenReturn(cdsData); when(malwareScanClient.scanContent(any())).thenReturn(status); } From b7271b1087f164d8a001298d47b3686f92509f88 Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 12:06:57 +0200 Subject: [PATCH 03/11] use not equal to compare status --- .../malware/DefaultAttachmentMalwareScanner.java | 16 ++++++---------- .../DefaultAttachmentMalwareScannerTest.java | 2 +- 2 files changed, 7 insertions(+), 11 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 18a10c42..5227794b 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 @@ -15,7 +15,6 @@ import org.slf4j.LoggerFactory; import com.google.common.annotations.VisibleForTesting; -import com.sap.cds.CdsData; import com.sap.cds.Result; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments; import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.StatusCode; @@ -94,7 +93,7 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { } private List selectData(CdsEntity attachmentEntity, String contentId) { - var result = new ArrayList(); + List result = new ArrayList<>(); try { CdsEntity entity = (CdsEntity) attachmentEntity.getTargetOf(Drafts.SIBLING_ENTITY); Result selectionResult = readData(contentId, entity); @@ -109,12 +108,9 @@ private List selectData(CdsEntity attachmentEntity, String cont } private Result readData(String contentId, CdsEntity entity) { - CqnSelect select = Select.from(entity).columns(Attachments.CONTENT_ID, Attachments.CONTENT) - .where(e -> e.get(Attachments.CONTENT_ID).eq(contentId).and(e.get(Attachments.STATUS) - .in(StatusCode.FAILED, StatusCode.UNSCANNED, StatusCode.INFECTED, StatusCode.SCANNING))); + CqnSelect select = Select.from(entity).columns(Attachments.CONTENT_ID, Attachments.CONTENT).where( + e -> e.get(Attachments.CONTENT_ID).eq(contentId).and(e.get(Attachments.STATUS).ne(StatusCode.CLEAN))); -// CqnSelect select2 = Select.from(entity, r -> r.filter(e -> e.get(Attachments.CONTENT_ID).eq(contentId) -// .and(e.get(Attachments.STATUS).in(StatusCode.FAILED, StatusCode.UNSCANNED, StatusCode.INFECTED)))); return persistenceService.run(select); } @@ -133,9 +129,9 @@ private MalwareScanResultStatus scanDocument(Attachments attachment) { } private void updateData(CdsEntity attachmentEntity, String contentId, MalwareScanResultStatus status) { - CdsData updateData = CdsData.create(); - updateData.put(Attachments.STATUS, mapStatus(status)); - updateData.put(Attachments.SCANNED_AT, Instant.now()); + Attachments updateData = Attachments.create(); + updateData.setStatus(mapStatus(status)); + updateData.setScannedAt(Instant.now()); logger.debug("CdsData shall be updated for entity: {}", attachmentEntity.getQualifiedName()); CqnUpdate update = Update.entity(attachmentEntity).data(updateData) .where(entry -> entry.get(Attachments.CONTENT_ID).eq(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 594f3a76..fbbf3e88 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 @@ -282,7 +282,7 @@ private void mockSelectResult(Attachments cdsData, MalwareScanResultStatus statu private void verifyKeyWhereCondition(CqnSelect select) { assertThat(select.where()).isPresent(); var selectWhere = select.where().get(); - assertThat(selectWhere.toString()).contains("[{\"ref\":[\"contentId\"]},\"=\",{\"val\":\"ID\"}]"); + assertThat(selectWhere.toString()).contains("[{\"ref\":[\"contentId\"]},\"=\",{\"val\":\"ID\"},\"and\",{\"ref\":[\"status\"]},\"<>\",{\"val\":\"Clean\"}]"); } } From 3aa5ce82b1ab076ca39e539f626b390c02c9e2e7 Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 12:19:31 +0200 Subject: [PATCH 04/11] improved logging --- .../service/malware/DefaultAttachmentMalwareScanner.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 5227794b..3249d926 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 @@ -67,7 +67,8 @@ public DefaultAttachmentMalwareScanner(PersistenceService persistenceService, At @Override public void scanAttachment(CdsEntity attachmentEntity, String contentId) { - logger.info("Service handler called to scan document for malware"); + logger.debug("Started scanning attachment with contentId: {} in entity: {}", contentId, + attachmentEntity.getQualifiedName()); List selectionResult = selectData(attachmentEntity, contentId); @@ -75,7 +76,7 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { long rowCount = result.result().rowCount(); if (rowCount <= 0) { - logger.info("No data found, nothing to scan for entity: {}", result.entity.getQualifiedName()); + logger.debug("No attachments found, nothing to scan for entity: {}", result.entity.getQualifiedName()); return; } @@ -132,7 +133,7 @@ private void updateData(CdsEntity attachmentEntity, String contentId, MalwareSca Attachments updateData = Attachments.create(); updateData.setStatus(mapStatus(status)); updateData.setScannedAt(Instant.now()); - logger.debug("CdsData shall be updated for entity: {}", attachmentEntity.getQualifiedName()); + CqnUpdate update = Update.entity(attachmentEntity).data(updateData) .where(entry -> entry.get(Attachments.CONTENT_ID).eq(contentId)); Result result = persistenceService.run(update); From 90eded08fb61f93b5e0c2b73475aa48e2e6a0dfb Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 14:24:47 +0200 Subject: [PATCH 05/11] improved debug logging --- .../EndTransactionMalwareScanRunner.java | 9 ++++++--- .../malware/DefaultAttachmentMalwareScanner.java | 14 +++++++------- .../EndTransactionMalwareScanRunnerTest.java | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java index 3eb944aa..89d9d82e 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java @@ -55,7 +55,8 @@ private void startScanning(CdsEntity attachmentEntityToScan, String contentId) { // ensure that DB transaction is still active until the content is completely read from InputStream and // scanned by malware scanner runtime.changeSetContext().run(changeSetCtx -> { - logger.info("Starting to scan attachment"); + logger.debug("Asynchronously starting scan of attachment {} of entity {}.", contentId, + attachmentEntityToScan.getQualifiedName()); attachmentMalwareScanner.scanAttachment(attachmentEntityToScan, contentId); }); }); @@ -63,9 +64,11 @@ private void startScanning(CdsEntity attachmentEntityToScan, String contentId) { }; CompletableFuture.supplyAsync(executeAdapterSupplier).whenComplete((result, exception) -> { if (Objects.nonNull(exception)) { - logger.error("Error during scanning attachment", exception); + logger.error("Error scanning attachment {} of entity {}.", contentId, + attachmentEntityToScan.getQualifiedName(), exception); } else { - logger.info("Scanning attachment completed."); + logger.debug("Scanning of attachment {} of entity {} was completed successfully.", contentId, + attachmentEntityToScan.getQualifiedName()); } }); } 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 3249d926..98b60a6b 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 @@ -67,8 +67,7 @@ public DefaultAttachmentMalwareScanner(PersistenceService persistenceService, At @Override public void scanAttachment(CdsEntity attachmentEntity, String contentId) { - logger.debug("Started scanning attachment with contentId: {} in entity: {}", contentId, - attachmentEntity.getQualifiedName()); + logger.debug("Started scanning attachment {} of entity {}.", contentId, attachmentEntity.getQualifiedName()); List selectionResult = selectData(attachmentEntity, contentId); @@ -76,14 +75,14 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { long rowCount = result.result().rowCount(); if (rowCount <= 0) { - logger.debug("No attachments found, nothing to scan for entity: {}", result.entity.getQualifiedName()); + logger.debug("No attachments found, nothing to scan for entity {}", result.entity.getQualifiedName()); return; } if (rowCount > 1) { - logger.warn("More than one attachment found for contentId: {} in entity: {}", contentId, + logger.warn("More than one attachment {} found in entity {}.", contentId, result.entity.getQualifiedName()); - throw new IllegalStateException("More than one attachment found for contentId: " + contentId); + throw new IllegalStateException("More than one attachment with contentId %s.".formatted(contentId)); } Attachments attachment = result.result().single(Attachments.class); @@ -137,8 +136,9 @@ private void updateData(CdsEntity attachmentEntity, String contentId, MalwareSca CqnUpdate update = Update.entity(attachmentEntity).data(updateData) .where(entry -> entry.get(Attachments.CONTENT_ID).eq(contentId)); Result result = persistenceService.run(update); - logger.info("Attachment has been updated, with result row count {} for entity {}", result.rowCount(), - attachmentEntity.getQualifiedName()); + + logger.debug("Updated scan status to {} of attachment with contentId {} -> Row count {} for entity {}", + updateData.getStatus(), contentId, result.rowCount(), attachmentEntity.getQualifiedName()); } @VisibleForTesting diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunnerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunnerTest.java index 2e676e7c..a5eb1e40 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunnerTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunnerTest.java @@ -155,7 +155,7 @@ private void verifyLogIsWritten() { observer.stop(); var errorList = observer.getLogEvents().stream().filter(event -> event.getLevel().equals(Level.ERROR)).toList(); assertThat(errorList).hasSize(1); - assertThat(errorList.get(0).getFormattedMessage()).contains("Error during scanning attachment"); + assertThat(errorList.get(0).getFormattedMessage()).contains("Error scanning attachment"); } } From e9e44a66ecac797990cb2bbd91bba60e2b9672cf Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 14:42:03 +0200 Subject: [PATCH 06/11] changed some info logs to debug and improved message texts --- .../transaction/EndTransactionMalwareScanRunner.java | 9 +++++---- .../service/malware/DefaultAttachmentMalwareScanner.java | 9 +++++---- .../service/malware/client/DefaultMalwareScanClient.java | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java index 89d9d82e..1c4e09bd 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java @@ -48,14 +48,15 @@ private void startScanning(CdsEntity attachmentEntityToScan, String contentId) { // get current request context RequestContextRunner runner = runtime.requestContext(); - logger.debug("Transaction completed. Starting to scan attachment asynchronously."); + logger.debug("Transaction completed. Starting to scan attachment {} in entity {} asynchronously.", contentId, + attachmentEntityToScan.getQualifiedName()); Supplier executeAdapterSupplier = () -> { // run malware scan asynchronously with current request context runner.run(resourceCtx -> { // ensure that DB transaction is still active until the content is completely read from InputStream and // scanned by malware scanner runtime.changeSetContext().run(changeSetCtx -> { - logger.debug("Asynchronously starting scan of attachment {} of entity {}.", contentId, + logger.debug("Started asynchronously scan of attachment {} in entity {}.", contentId, attachmentEntityToScan.getQualifiedName()); attachmentMalwareScanner.scanAttachment(attachmentEntityToScan, contentId); }); @@ -64,10 +65,10 @@ private void startScanning(CdsEntity attachmentEntityToScan, String contentId) { }; CompletableFuture.supplyAsync(executeAdapterSupplier).whenComplete((result, exception) -> { if (Objects.nonNull(exception)) { - logger.error("Error scanning attachment {} of entity {}.", contentId, + logger.error("Error scanning attachment {} in entity {}.", contentId, attachmentEntityToScan.getQualifiedName(), exception); } else { - logger.debug("Scanning of attachment {} of entity {} was completed successfully.", contentId, + logger.debug("Scanning of attachment {} in entity {} was completed successfully.", contentId, attachmentEntityToScan.getQualifiedName()); } }); 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 98b60a6b..ecd055bb 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,8 @@ public void scanAttachment(CdsEntity attachmentEntity, String contentId) { long rowCount = result.result().rowCount(); if (rowCount <= 0) { - logger.debug("No attachments found, nothing to scan for entity {}", result.entity.getQualifiedName()); + logger.debug("No attachments {} found, nothing to scan for entity {}", contentId, + result.entity.getQualifiedName()); return; } @@ -121,7 +122,7 @@ private MalwareScanResultStatus scanDocument(Attachments attachment) { : attachmentService.readAttachment(attachment.getContentId()); return malwareScanClient.scanContent(content); } catch (RuntimeException e) { - logger.error("Error while scanning document with contentId: {}", attachment.getContentId(), e); + logger.error("Error while scanning attachment {}.", attachment.getContentId(), e); return MalwareScanResultStatus.FAILED; } } @@ -137,8 +138,8 @@ private void updateData(CdsEntity attachmentEntity, String contentId, MalwareSca .where(entry -> entry.get(Attachments.CONTENT_ID).eq(contentId)); Result result = persistenceService.run(update); - logger.debug("Updated scan status to {} of attachment with contentId {} -> Row count {} for entity {}", - updateData.getStatus(), contentId, result.rowCount(), attachmentEntity.getQualifiedName()); + logger.debug("Updated scan status to {} of attachment {} in entity {} -> Row count {}.", updateData.getStatus(), + contentId, attachmentEntity.getQualifiedName(), result.rowCount()); } @VisibleForTesting diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java index e861f4f4..01434900 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java @@ -49,7 +49,7 @@ public DefaultMalwareScanClient(HttpClientProvider clientProvider) { @Override public MalwareScanResultStatus scanContent(InputStream content) { - logger.info("Start scanning document"); + logger.debug("Start scanning document"); HttpPost request = buildHttpRequest(content); return executeRequest(request); } From d58468f7af028019273a6b2f1971e5679d7d636a Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 15:17:21 +0200 Subject: [PATCH 07/11] removed not useful info logging --- .../service/malware/client/DefaultMalwareScanClient.java | 1 - 1 file changed, 1 deletion(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java index 01434900..55716036 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java @@ -97,7 +97,6 @@ private static MalwareScanResultStatus mapResponseToStatus(MalwareScanResult sca scanResult.isMalwareDetected(), scanResult.isEncryptedContentDetected()); return MalwareScanResultStatus.INFECTED; } else { - logger.info("Document is clean"); return MalwareScanResultStatus.CLEAN; } } From 37ca5bfbcad37abbcbc69182ffe735f0a2f50c58 Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 16:08:35 +0200 Subject: [PATCH 08/11] moved debug logging to provide more log details --- .../handler/applicationservice/ReadAttachmentsHandler.java | 4 ++-- .../service/malware/DefaultAttachmentMalwareScanner.java | 3 ++- .../service/malware/client/DefaultMalwareScanClient.java | 1 - 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java index 85ac1beb..89b80c45 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java @@ -86,7 +86,7 @@ public void processAfter(CdsReadEventContext context, List data) { logger.debug("Processing after read event for entity {}", context.getTarget().getName()); Converter converter = (path, element, value) -> { - logger.info("Processing after read event for entity {}", element.getName()); + logger.debug("Processing after read event for entity {}", element.getName()); String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); String status = (String) path.target().values().get(Attachments.STATUS); InputStream content = (InputStream) path.target().values().get(Attachments.CONTENT); @@ -107,7 +107,7 @@ public void processAfter(CdsReadEventContext context, List data) { private List getAttachmentAssociations(CdsModel model, CdsEntity entity, String associationName, List processedEntities) { - List associationNames = new ArrayList(); + List associationNames = new ArrayList<>(); if (ApplicationHandlerHelper.isMediaEntity(entity)) { associationNames.add(associationName); } 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 ecd055bb..c92b8c41 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) { long rowCount = result.result().rowCount(); if (rowCount <= 0) { - logger.debug("No attachments {} found, nothing to scan for entity {}", contentId, + logger.debug("No attachments {} found in entity {}, nothing to scan.", contentId, result.entity.getQualifiedName()); return; } @@ -120,6 +120,7 @@ private MalwareScanResultStatus scanDocument(Attachments attachment) { try { InputStream content = Objects.nonNull(attachment.getContent()) ? attachment.getContent() : attachmentService.readAttachment(attachment.getContentId()); + logger.debug("Start scanning attachment {}.", attachment.getContentId()); return malwareScanClient.scanContent(content); } catch (RuntimeException e) { logger.error("Error while scanning attachment {}.", attachment.getContentId(), e); diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java index 55716036..754e1b73 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClient.java @@ -49,7 +49,6 @@ public DefaultMalwareScanClient(HttpClientProvider clientProvider) { @Override public MalwareScanResultStatus scanContent(InputStream content) { - logger.debug("Start scanning document"); HttpPost request = buildHttpRequest(content); return executeRequest(request); } From 805d1b915fb4c21b353add3b5ef1b1b3bed4db28 Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 16:18:32 +0200 Subject: [PATCH 09/11] use qualified name for debug logging --- .../handler/applicationservice/ReadAttachmentsHandler.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java index 89b80c45..d3736c16 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java @@ -83,10 +83,9 @@ public void processAfter(CdsReadEventContext context, List data) { if (ApplicationHandlerHelper.noContentFieldInData(context.getTarget(), data)) { return; } - logger.debug("Processing after read event for entity {}", context.getTarget().getName()); + logger.debug("Processing after read event for entity {}", context.getTarget().getQualifiedName()); Converter converter = (path, element, value) -> { - logger.debug("Processing after read event for entity {}", element.getName()); String contentId = (String) path.target().values().get(Attachments.CONTENT_ID); String status = (String) path.target().values().get(Attachments.STATUS); InputStream content = (InputStream) path.target().values().get(Attachments.CONTENT); From 109c3c3a8ce6ec483566b9f3727af4bf34cb6d10 Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Wed, 16 Jul 2025 17:00:32 +0200 Subject: [PATCH 10/11] log selected attachments for scanning --- .../service/malware/DefaultAttachmentMalwareScanner.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 c92b8c41..a945b584 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 @@ -112,7 +112,11 @@ private Result readData(String contentId, CdsEntity entity) { CqnSelect select = Select.from(entity).columns(Attachments.CONTENT_ID, Attachments.CONTENT).where( e -> e.get(Attachments.CONTENT_ID).eq(contentId).and(e.get(Attachments.STATUS).ne(StatusCode.CLEAN))); - return persistenceService.run(select); + Result result = persistenceService.run(select); + result.streamOf(Attachments.class) + .forEach(attachment -> logger.debug("Found attachment {} in entity {} with status {}.", + attachment.getContentId(), entity.getQualifiedName(), attachment.getStatus())); + return result; } private MalwareScanResultStatus scanDocument(Attachments attachment) { From 0241ff87793e7ba0e89781dde9b26f230a5107fd Mon Sep 17 00:00:00 2001 From: Markus Ofterdinger Date: Thu, 17 Jul 2025 10:49:20 +0200 Subject: [PATCH 11/11] added scan status to select list --- .../service/malware/DefaultAttachmentMalwareScanner.java | 5 +++-- 1 file changed, 3 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 a945b584..ec1eaa0a 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 @@ -109,8 +109,9 @@ private List selectData(CdsEntity attachmentEntity, String cont } private Result readData(String contentId, CdsEntity entity) { - CqnSelect select = Select.from(entity).columns(Attachments.CONTENT_ID, Attachments.CONTENT).where( - e -> e.get(Attachments.CONTENT_ID).eq(contentId).and(e.get(Attachments.STATUS).ne(StatusCode.CLEAN))); + CqnSelect select = Select.from(entity).columns(Attachments.CONTENT_ID, Attachments.CONTENT, Attachments.STATUS) + .where(e -> e.get(Attachments.CONTENT_ID).eq(contentId) + .and(e.get(Attachments.STATUS).ne(StatusCode.CLEAN))); Result result = persistenceService.run(select); result.streamOf(Attachments.class)