Moved and renamed some classes of the malware scan implementation (#442)

mofterdinger · web-flow · commit 11359db32bb2 · 2025-06-03T10:43:18.000+02:00
diff --git a/.pipeline/config.yml b/.pipeline/config.yml
@@ -34,6 +34,6 @@ steps:
     options:
       - sonar.qualitygate.wait=true
       - sonar.java.source=17
-      - sonar.exclusions=**/node_modules/**,**/target/**
+      - sonar.exclusions=**/node_modules/**,**/target/**,**/test/**
       - sonar.coverage.jacoco.xmlReportPaths=cds-feature-attachments/target/site/jacoco/jacoco.xml
       - sonar.coverage.exclusions=cds-feature-attachments/src/test/**,cds-feature-attachments/src/gen/**,integration-tests/**
diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/configuration/Registration.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/configuration/Registration.java
@@ -36,9 +36,9 @@
 import com.sap.cds.feature.attachments.service.malware.AttachmentMalwareScanner;
 import com.sap.cds.feature.attachments.service.malware.DefaultAttachmentMalwareScanner;
 import com.sap.cds.feature.attachments.service.malware.client.DefaultMalwareScanClient;
-import com.sap.cds.feature.attachments.service.malware.client.HttpClientProviderFactory;
+import com.sap.cds.feature.attachments.service.malware.client.HttpClientProvider;
 import com.sap.cds.feature.attachments.service.malware.client.MalwareScanClient;
-import com.sap.cds.feature.attachments.service.malware.client.MalwareScanClientProviderFactory;
+import com.sap.cds.feature.attachments.service.malware.client.MalwareScanClientProvider;
 import com.sap.cds.services.ServiceCatalog;
 import com.sap.cds.services.cds.ApplicationService;
 import com.sap.cds.services.draft.DraftService;
@@ -143,14 +143,14 @@ private static MalwareScanClient buildMalwareScanClient(CdsEnvironment environme
 		if (bindingOpt.isPresent()) {
 			ServiceBinding binding = bindingOpt.get();
 			ConnectionPool connectionPool = getConnectionPool(environment);
-			HttpClientProviderFactory clientProviderFactory = new MalwareScanClientProviderFactory(binding,
+			HttpClientProvider clientProvider = new MalwareScanClientProvider(binding,
 					connectionPool);
 			if (logger.isInfoEnabled()) {
 				logger.info(
 						"Using Malware Scanning service binding with name '{}' and plan '{}' for malware scanning of attachments.",
 						binding.getName().orElse("unknown"), binding.getServicePlan().orElse("unknown"));
 			}
-			return new DefaultMalwareScanClient(clientProviderFactory);
+			return new DefaultMalwareScanClient(clientProvider);
 		}
 
 		logger.info("No Malware Scanning service binding found, malware scanning is disabled.");
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
@@ -21,7 +21,7 @@
 import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.StatusCode;
 import com.sap.cds.feature.attachments.service.AttachmentService;
 import com.sap.cds.feature.attachments.service.malware.client.MalwareScanClient;
-import com.sap.cds.feature.attachments.service.malware.client.model.MalwareScanResultStatus;
+import com.sap.cds.feature.attachments.service.malware.client.MalwareScanResultStatus;
 import com.sap.cds.ql.Select;
 import com.sap.cds.ql.Update;
 import com.sap.cds.ql.cqn.CqnSelect;
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
@@ -19,8 +19,6 @@
 
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.sap.cds.feature.attachments.service.malware.client.model.MalwareScanResult;
-import com.sap.cds.feature.attachments.service.malware.client.model.MalwareScanResultStatus;
 import com.sap.cds.services.ServiceException;
 
 /**
@@ -35,33 +33,29 @@ public class DefaultMalwareScanClient implements MalwareScanClient {
 
 	private final ObjectMapper mapper = buildObjectMapper();
 
-	private final HttpClientProviderFactory clientProviderFactory;
+	private final HttpClientProvider clientProvider;
 
 	/**
 	 * Constructs a new instance of {@link DefaultMalwareScanClient}.
 	 *
-	 * @param clientProviderFactory the required {@link HttpClientProviderFactory} to create the HTTP client to access
-	 *                              the Malware Scan Service.
+	 * @param clientProvider the required {@link HttpClientProvider} to create the HTTP client to access
+	 *                       the Malware Scan Service.
 	 *
 	 * @throws NullPointerException if the {@code clientProviderFactory} is {@code null}.
 	 */
-	public DefaultMalwareScanClient(HttpClientProviderFactory clientProviderFactory) {
-		this.clientProviderFactory = requireNonNull(clientProviderFactory, "clientProviderFactory must not be null");
+	public DefaultMalwareScanClient(HttpClientProvider clientProvider) {
+		this.clientProvider = requireNonNull(clientProvider, "clientProvider must not be null");
 	}
 
 	@Override
 	public MalwareScanResultStatus scanContent(InputStream content) {
 		logger.info("Start scanning document");
-		return scanContentWithClient(content);
-	}
-
-	private MalwareScanResultStatus scanContentWithClient(InputStream content) {
-		HttpClient httpClient = clientProviderFactory.getHttpClient();
 		HttpPost request = buildHttpRequest(content);
-		return executeRequest(httpClient, request);
+		return executeRequest(request);
 	}
 
-	private MalwareScanResultStatus executeRequest(HttpClient httpClient, HttpPost request) {
+	private MalwareScanResultStatus executeRequest(HttpPost request) {
+		HttpClient httpClient = clientProvider.getHttpClient();
 		try (CloseableHttpResponse response = (CloseableHttpResponse) httpClient.execute(request)) {
 			MalwareScanResult malwareScanResult = convertHttpResponseToJavaObject(response);
 			return mapResponseToStatus(malwareScanResult);
diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/HttpClientProvider.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/HttpClientProvider.java
@@ -6,13 +6,13 @@
 import org.apache.http.client.HttpClient;
 
 /**
- * Factory for creating a {@link HttpClient} to access the Malware Scan Service.
+ * A provider for getting a {@link HttpClient} to access the Malware Scan Service.
  */
-public interface HttpClientProviderFactory {
+public interface HttpClientProvider {
 
 	/**
 	 * Returns an {@link HttpClient} to send HTTP requests to the Malware Scan Service.
-	 * 
+	 *
 	 * @return an {@link HttpClient} to send HTTP requests to the Malware Scan Service.
 	 */
 	HttpClient getHttpClient();
diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanClient.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanClient.java
@@ -4,8 +4,6 @@
 package com.sap.cds.feature.attachments.service.malware.client;
 
 import java.io.InputStream;
-
-import com.sap.cds.feature.attachments.service.malware.client.model.MalwareScanResultStatus;
 import com.sap.cds.services.ServiceException;
 
 /**
diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanClientProvider.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanClientProvider.java
@@ -17,19 +17,19 @@
 import com.sap.cloud.sdk.cloudplatform.security.BasicCredentials;
 
 /**
- * The default factory for creating a {@link HttpClient} for the Malware Scan Service.
+ * The default provider for getting a {@link HttpClient} for the Malware Scan Service.
  */
-public final class MalwareScanClientProviderFactory implements HttpClientProviderFactory {
+public final class MalwareScanClientProvider implements HttpClientProvider {
 
 	private final HttpClient httpClient;
 
 	/**
-	 * Creates a new instance of {@link MalwareScanClientProviderFactory}.
+	 * Creates a new instance of {@link MalwareScanClientProvider}.
 	 *
 	 * @param binding       the required {@link ServiceBinding} to the Malware Scan Service
 	 * @param configuration the required {@link ConnectionPool} configuration
 	 */
-	public MalwareScanClientProviderFactory(ServiceBinding binding, ConnectionPool configuration) {
+	public MalwareScanClientProvider(ServiceBinding binding, ConnectionPool configuration) {
 		Map<String, Object> credentials = binding.getCredentials();
 
 		BasicCredentials basic = new BasicCredentials((String) credentials.get("username"),
diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanResult.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanResult.java
@@ -1,8 +1,9 @@
 /**************************************************************************
  * (C) 2019-2025 SAP SE or an SAP affiliate company. All rights reserved. *
  **************************************************************************/
-package com.sap.cds.feature.attachments.service.malware.client.model;
+package com.sap.cds.feature.attachments.service.malware.client;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.io.Serial;
 import java.io.Serializable;
 
@@ -19,11 +20,12 @@ public class MalwareScanResult implements Serializable {
 	private String mimeType;
 
 	/**
-	 * Default constructor with no parameters.
+	 * Default constructor with no parameters. Required for Jackson deserialization.
 	 */
 	public MalwareScanResult() {
 	}
 
+	@VisibleForTesting
 	MalwareScanResult(boolean malwareDetected, boolean encryptedContentDetected, long scanSize, String mimeType) {
 		this.malwareDetected = malwareDetected;
 		this.encryptedContentDetected = encryptedContentDetected;
@@ -33,7 +35,7 @@ public MalwareScanResult() {
 
 	/**
 	 * Indicates if malware was detected.
-	 * 
+	 *
 	 * @return <code>true</code> if malware was detected, otherwise <code>false</code>.
 	 */
 	public boolean isMalwareDetected() {
@@ -42,7 +44,7 @@ public boolean isMalwareDetected() {
 
 	/**
 	 * Indicates if encrypted content was detected.
-	 * 
+	 *
 	 * @return <code>true</code> if encrypted content was detected, otherwise <code>false</code>.
 	 */
 	public boolean isEncryptedContentDetected() {
@@ -51,7 +53,7 @@ public boolean isEncryptedContentDetected() {
 
 	/**
 	 * The size of the scanned file.
-	 * 
+	 *
 	 * @return the size of the scanned file.
 	 */
 	public long getScanSize() {
@@ -60,7 +62,7 @@ public long getScanSize() {
 
 	/**
 	 * The mime type of the file scanned.
-	 * 
+	 *
 	 * @return the mime type of the file scanned.
 	 */
 	public String getMimeType() {
diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanResultStatus.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanResultStatus.java
@@ -1,7 +1,7 @@
 /**************************************************************************
  * (C) 2019-2025 SAP SE or an SAP affiliate company. All rights reserved. *
  **************************************************************************/
-package com.sap.cds.feature.attachments.service.malware.client.model;
+package com.sap.cds.feature.attachments.service.malware.client;
 
 /**
  * The results returned from the Malware Scan backing service. The status of the scan can be one of the following:
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
@@ -1,6 +1,7 @@
 package com.sap.cds.feature.attachments.service.malware;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
@@ -26,7 +27,7 @@
 import com.sap.cds.feature.attachments.handler.helper.RuntimeHelper;
 import com.sap.cds.feature.attachments.service.AttachmentService;
 import com.sap.cds.feature.attachments.service.malware.client.MalwareScanClient;
-import com.sap.cds.feature.attachments.service.malware.client.model.MalwareScanResultStatus;
+import com.sap.cds.feature.attachments.service.malware.client.MalwareScanResultStatus;
 import com.sap.cds.ql.cqn.CqnSelect;
 import com.sap.cds.ql.cqn.CqnUpdate;
 import com.sap.cds.services.ServiceException;
@@ -250,11 +251,11 @@ void clientNotCalledIfNoInstanceBound() {
 
 	@Test
 	void mapStatus() {
-		assertThat(DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.CLEAN)).isEqualTo(StatusCode.CLEAN);
-		assertThat(DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.ENCRYPTED)).isEqualTo(StatusCode.INFECTED);
-		assertThat(DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.FAILED)).isEqualTo(StatusCode.FAILED);
-		assertThat(DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.INFECTED)).isEqualTo(StatusCode.INFECTED);
-		assertThat(DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.NO_SCANNER)).isEqualTo(StatusCode.CLEAN);
+		assertEquals(StatusCode.CLEAN, DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.CLEAN));
+		assertEquals(StatusCode.INFECTED,DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.ENCRYPTED));
+		assertEquals(StatusCode.FAILED, DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.FAILED));
+		assertEquals(StatusCode.INFECTED, DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.INFECTED));
+		assertEquals(StatusCode.CLEAN, DefaultAttachmentMalwareScanner.mapStatus(MalwareScanResultStatus.NO_SCANNER));
 	}
 
 	private void verifyPersistenceServiceCalledCorrectlyForReadAndUpdate(MalwareScanResultStatus expectedStatus) {
diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClientTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/DefaultMalwareScanClientTest.java
@@ -1,19 +1,19 @@
 package com.sap.cds.feature.attachments.service.malware.client;
 
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import com.sap.cds.services.ServiceException;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.Objects;
-
 import org.apache.http.Header;
 import org.apache.http.HeaderElement;
 import org.apache.http.HttpEntity;
@@ -29,19 +29,16 @@
 import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.ArgumentCaptor;
 
-import com.sap.cds.feature.attachments.service.malware.client.model.MalwareScanResultStatus;
-import com.sap.cds.services.ServiceException;
-
 class DefaultMalwareScanClientTest {
 
 	private DefaultMalwareScanClient cut;
-	private HttpClientProviderFactory clientProviderFactory;
+	private HttpClientProvider clientProvider;
 	private ArgumentCaptor<HttpEntityEnclosingRequestBase> requestCaptor;
 
 	@BeforeEach
 	void setup() {
-		clientProviderFactory = mock(HttpClientProviderFactory.class);
-		cut = new DefaultMalwareScanClient(clientProviderFactory);
+		clientProvider = mock(HttpClientProvider.class);
+		cut = new DefaultMalwareScanClient(clientProvider);
 
 		requestCaptor = ArgumentCaptor.forClass(HttpEntityEnclosingRequestBase.class);
 	}
@@ -54,7 +51,7 @@ void contentIsUsedInRequest() throws IOException {
 		cut.scanContent(content);
 
 		verify(httpClient).execute(requestCaptor.capture());
-		assertThat(requestCaptor.getValue().getEntity().getContent()).isEqualTo(content);
+		assertEquals(requestCaptor.getValue().getEntity().getContent(), content);
 	}
 
 	@Test
@@ -66,7 +63,7 @@ void correctMethodIsUsedInRequest() throws IOException {
 
 		verify(httpClient).execute(requestCaptor.capture());
 		var request = requestCaptor.getValue();
-		assertThat(request.getMethod()).isEqualTo("POST");
+		assertEquals("POST", request.getMethod());
 	}
 
 	@Test
@@ -75,7 +72,7 @@ void correctStatusReturnedForSuccessfulScan() throws IOException {
 
 		var response = cut.scanContent(mock(InputStream.class));
 
-		assertThat(response).isEqualTo(MalwareScanResultStatus.CLEAN);
+		assertEquals(MalwareScanResultStatus.CLEAN, response);
 	}
 
 	@ParameterizedTest
@@ -86,13 +83,13 @@ void correctStatusReturnedForNotCleanResponse(boolean malwareDetected,
 
 		var response = cut.scanContent(mock(InputStream.class));
 
-		assertThat(response).isEqualTo(MalwareScanResultStatus.INFECTED);
+		assertEquals(MalwareScanResultStatus.INFECTED, response);
 	}
 
 	@Test
 	void exceptionIsThrownIfRequestRespondWithException() throws IOException {
 		var httpClient = mock(HttpClient.class);
-		when(clientProviderFactory.getHttpClient()).thenReturn(httpClient);
+		when(clientProvider.getHttpClient()).thenReturn(httpClient);
 		when(httpClient.execute(any())).thenThrow(new IOException());
 
 		var inputStream = mock(InputStream.class);
@@ -130,7 +127,7 @@ void noResponseEntityReturnsClean() throws IOException {
 
 		var response = cut.scanContent(mock(InputStream.class));
 
-		assertThat(response).isEqualTo(MalwareScanResultStatus.CLEAN);
+		assertEquals(MalwareScanResultStatus.CLEAN, response);
 	}
 
 	private HttpClient mockHttpResponse(int httpStatus, boolean malwareDetected,
@@ -141,7 +138,7 @@ private HttpClient mockHttpResponse(int httpStatus, boolean malwareDetected,
 	private HttpClient mockHttpResponse(int httpStatus, boolean malwareDetected, boolean encryptedContentDetected,
 			String contentTypeString, boolean responseEntityExists) throws IOException {
 		var httpClient = mock(HttpClient.class);
-		when(clientProviderFactory.getHttpClient()).thenReturn(httpClient);
+		when(clientProvider.getHttpClient()).thenReturn(httpClient);
 		var response = mock(CloseableHttpResponse.class);
 		when(httpClient.execute(any())).thenReturn(response);
 		var statusLine = mock(StatusLine.class);
diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanClientProviderTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanClientProviderTest.java
@@ -1,6 +1,6 @@
 package com.sap.cds.feature.attachments.service.malware.client;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -15,8 +15,8 @@
 import com.sap.cds.services.runtime.CdsRuntime;
 import com.sap.cloud.environment.servicebinding.api.ServiceBinding;
 
-class MalwareScanClientProviderFactoryTest {
-	private MalwareScanClientProviderFactory cut;
+class MalwareScanClientProviderTest {
+	private MalwareScanClientProvider cut;
 	private ServiceBinding binding;
 	private CdsRuntime runtime;
 	private ConnectionPool connectionPoolConfig;
@@ -36,10 +36,10 @@ void clientProviderReturned() {
 		when(connectionPoolConfig.getMaxConnections()).thenReturn(10);
 		when(connectionPoolConfig.getMaxConnectionsPerRoute()).thenReturn(1);
 
-		cut = new MalwareScanClientProviderFactory(binding, connectionPoolConfig);
+		cut = new MalwareScanClientProvider(binding, connectionPoolConfig);
 
 		var client = cut.getHttpClient();
-		assertThat(client).isNotNull();
+		assertNotNull(client);
 	}
 
 	private void mockInput() {
@@ -50,5 +50,4 @@ private void mockInput() {
 		when(connectionPoolConfig.getMaxConnections()).thenReturn(20);
 		when(connectionPoolConfig.getMaxConnectionsPerRoute()).thenReturn(20);
 	}
-
 }
diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanResultTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/MalwareScanResultTest.java
diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/model/MalwareScanResultTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/malware/client/model/MalwareScanResultTest.java
