From de9cd8cb9b6df8291591eaa5d1e4db8a2378feee Mon Sep 17 00:00:00 2001 From: "AAVN\\pvquan" Date: Thu, 12 Dec 2024 15:05:13 +0700 Subject: [PATCH 1/3] MARP-1548 Suspicious installation counts --- marketplace-build/.env | 3 +- marketplace-build/dev/.env | 3 +- marketplace-build/release/.env | 3 +- marketplace-service/pom.xml | 6 +- .../market/constants/CommonConstants.java | 1 + .../ProductMarketplaceDataController.java | 16 ++-- .../com/axonivy/market/logging/Loggable.java | 11 +++ .../market/logging/LoggableAspect.java | 94 +++++++++++++++++++ .../com/axonivy/market/util/FileUtils.java | 31 ++++++ .../com/axonivy/market/util/LoggingUtils.java | 52 ++++++++++ .../src/main/resources/application.properties | 3 +- .../service/impl/SchedulingTasksTest.java | 3 +- .../axonivy/market/util/FileUtilsTest.java | 39 ++++++++ .../axonivy/market/util/LoggingUtilsTest.java | 46 +++++++++ 14 files changed, 296 insertions(+), 15 deletions(-) create mode 100644 marketplace-service/src/main/java/com/axonivy/market/logging/Loggable.java create mode 100644 marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java create mode 100644 marketplace-service/src/main/java/com/axonivy/market/util/FileUtils.java create mode 100644 marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java create mode 100644 marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java create mode 100644 marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java diff --git a/marketplace-build/.env b/marketplace-build/.env index d1b4b7edb..606142801 100644 --- a/marketplace-build/.env +++ b/marketplace-build/.env @@ -12,4 +12,5 @@ MARKET_GITHUB_OAUTH_APP_CLIENT_ID= MARKET_GITHUB_OAUTH_APP_CLIENT_SECRET= MARKET_JWT_SECRET_KEY= MARKET_CORS_ALLOWED_ORIGIN=* -MARKET_MONGO_LOG_LEVEL=DEBUG \ No newline at end of file +MARKET_MONGO_LOG_LEVEL=DEBUG +MARKET_LOG_PATH=logs \ No newline at end of file diff --git a/marketplace-build/dev/.env b/marketplace-build/dev/.env index e9d068e53..09f4215ad 100644 --- a/marketplace-build/dev/.env +++ b/marketplace-build/dev/.env @@ -12,4 +12,5 @@ MARKET_GITHUB_OAUTH_APP_CLIENT_ID= MARKET_GITHUB_OAUTH_APP_CLIENT_SECRET= MARKET_JWT_SECRET_KEY= MARKET_CORS_ALLOWED_ORIGIN=* -MARKET_MONGO_LOG_LEVEL=DEBUG \ No newline at end of file +MARKET_MONGO_LOG_LEVEL=DEBUG +MARKET_LOG_PATH=logs \ No newline at end of file diff --git a/marketplace-build/release/.env b/marketplace-build/release/.env index d8dfb5f8a..fd7ac7016 100644 --- a/marketplace-build/release/.env +++ b/marketplace-build/release/.env @@ -13,4 +13,5 @@ MARKET_GITHUB_OAUTH_APP_CLIENT_ID= MARKET_GITHUB_OAUTH_APP_CLIENT_SECRET= MARKET_JWT_SECRET_KEY= MARKET_CORS_ALLOWED_ORIGIN=* -MARKET_MONGO_LOG_LEVEL=DEBUG \ No newline at end of file +MARKET_MONGO_LOG_LEVEL=DEBUG +MARKET_LOG_PATH=logs \ No newline at end of file diff --git a/marketplace-service/pom.xml b/marketplace-service/pom.xml index 8bdb6e625..ec7ccc0ed 100644 --- a/marketplace-service/pom.xml +++ b/marketplace-service/pom.xml @@ -73,7 +73,11 @@ jaxb-api 2.3.1 - + + org.springframework.boot + spring-boot-starter-aop + + org.springframework.boot spring-boot-starter-validation diff --git a/marketplace-service/src/main/java/com/axonivy/market/constants/CommonConstants.java b/marketplace-service/src/main/java/com/axonivy/market/constants/CommonConstants.java index 2966383f4..e80c2b57d 100644 --- a/marketplace-service/src/main/java/com/axonivy/market/constants/CommonConstants.java +++ b/marketplace-service/src/main/java/com/axonivy/market/constants/CommonConstants.java @@ -6,6 +6,7 @@ @NoArgsConstructor(access = AccessLevel.PRIVATE) public class CommonConstants { public static final String REQUESTED_BY = "X-Requested-By"; + public static final String USER_AGENT = "user-agent"; public static final String SLASH = "/"; public static final String DOT_SEPARATOR = "."; public static final String PLUS = "+"; diff --git a/marketplace-service/src/main/java/com/axonivy/market/controller/ProductMarketplaceDataController.java b/marketplace-service/src/main/java/com/axonivy/market/controller/ProductMarketplaceDataController.java index 728251b33..cd4abe584 100644 --- a/marketplace-service/src/main/java/com/axonivy/market/controller/ProductMarketplaceDataController.java +++ b/marketplace-service/src/main/java/com/axonivy/market/controller/ProductMarketplaceDataController.java @@ -3,13 +3,12 @@ import com.axonivy.market.constants.GitHubConstants; import com.axonivy.market.enums.ErrorCode; import com.axonivy.market.github.service.GitHubService; +import com.axonivy.market.logging.Loggable; import com.axonivy.market.model.Message; import com.axonivy.market.model.ProductCustomSortRequest; import com.axonivy.market.service.ProductMarketplaceDataService; import com.axonivy.market.util.AuthorizationUtils; import io.swagger.v3.oas.annotations.Operation; -import io.swagger.v3.oas.annotations.Parameter; -import io.swagger.v3.oas.annotations.enums.ParameterIn; import io.swagger.v3.oas.annotations.tags.Tag; import jakarta.validation.Valid; import lombok.AllArgsConstructor; @@ -36,7 +35,7 @@ public class ProductMarketplaceDataController { private final GitHubService gitHubService; private final ProductMarketplaceDataService productMarketplaceDataService; - + @PostMapping(CUSTOM_SORT) @Operation(hidden = true) public ResponseEntity createCustomSortProducts( @@ -51,15 +50,14 @@ public ResponseEntity createCustomSortProducts( return new ResponseEntity<>(message, HttpStatus.OK); } + @Loggable + @Operation(hidden = true) @PutMapping(INSTALLATION_COUNT_BY_ID) - @Operation(summary = "Update installation count of product", - description = "By default, increase installation count when click download product files by users") public ResponseEntity syncInstallationCount( - @PathVariable(ID) @Parameter(description = "Product id (from meta.json)", example = "approval-decision-utils", - in = ParameterIn.PATH) String productId, - @RequestParam(name = DESIGNER_VERSION, required = false) @Parameter(in = ParameterIn.QUERY, - example = "v10.0.20") String designerVersion) { + @PathVariable(ID) String productId, + @RequestParam(name = DESIGNER_VERSION, required = false) String designerVersion) { int result = productMarketplaceDataService.updateInstallationCountForProduct(productId, designerVersion); return new ResponseEntity<>(result, HttpStatus.OK); } + } diff --git a/marketplace-service/src/main/java/com/axonivy/market/logging/Loggable.java b/marketplace-service/src/main/java/com/axonivy/market/logging/Loggable.java new file mode 100644 index 000000000..3194ee816 --- /dev/null +++ b/marketplace-service/src/main/java/com/axonivy/market/logging/Loggable.java @@ -0,0 +1,11 @@ +package com.axonivy.market.logging; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface Loggable { +} diff --git a/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java b/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java new file mode 100644 index 000000000..4f50ea59c --- /dev/null +++ b/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java @@ -0,0 +1,94 @@ +package com.axonivy.market.logging; + +import com.axonivy.market.constants.CommonConstants; +import com.axonivy.market.exceptions.model.MissingHeaderException; +import jakarta.servlet.http.HttpServletRequest; +import lombok.extern.log4j.Log4j2; +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.aspectj.lang.reflect.MethodSignature; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Component; +import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.web.context.request.ServletRequestAttributes; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.Objects; + +import static com.axonivy.market.util.FileUtils.createFile; +import static com.axonivy.market.util.FileUtils.writeToFile; +import static com.axonivy.market.util.LoggingUtils.*; + +@Log4j2 +@Aspect +@Component +public class LoggableAspect { + + @Value("${loggable.log-path}") + private String logFilePath; + + private static final String REQUESTED_BY = "marketplace-website"; + + @Before("@annotation(com.axonivy.market.logging.Loggable)") + public void logMethodCall(JoinPoint joinPoint) throws MissingHeaderException { + MethodSignature signature = (MethodSignature) joinPoint.getSignature(); + ServletRequestAttributes attributes = + (ServletRequestAttributes) RequestContextHolder.getRequestAttributes(); + if (attributes != null) { + HttpServletRequest request = attributes.getRequest(); + Map headersMap = extractHeaders(request, signature, joinPoint); + saveLogToDailyFile(headersMap); + + // block execution if request isn't from Market or Ivy Designer + if (!Objects.equals(headersMap.get(CommonConstants.REQUESTED_BY), REQUESTED_BY)) { + throw new MissingHeaderException(); + } + } + } + + private Map extractHeaders(HttpServletRequest request, MethodSignature signature, + JoinPoint joinPoint) { + return Map.of( + "method", escapeXml(String.valueOf(signature.getMethod())), + "timestamp", escapeXml(getCurrentTimestamp()), + "user-agent", escapeXml(request.getHeader(CommonConstants.USER_AGENT)), + "arguments", escapeXml(getArgumentsString(signature.getParameterNames(), joinPoint.getArgs())), + "x-requested-by", escapeXml(request.getHeader(CommonConstants.REQUESTED_BY)) + ); + } + + // Use synchronized to prevent race condition + private synchronized void saveLogToDailyFile(Map headersMap) { + try { + File logFile = createFile(generateFileName()); + + StringBuilder content = new StringBuilder(); + if (logFile.exists()) { + content.append(new String(Files.readAllBytes(logFile.toPath()))); + } + if (content.isEmpty()) { + content.append("\n"); + } + int lastLogIndex = content.lastIndexOf(""); + if (lastLogIndex != -1) { + content.delete(lastLogIndex, content.length()); + } + content.append(buildLogEntry(headersMap)); + content.append("\n"); + + writeToFile(logFile, content.toString()); + } catch (IOException e) { + log.error("Error writing log to file: {}", e.getMessage()); + } + } + + private String generateFileName() { + return Path.of(logFilePath, "log-" + getCurrentDate() + ".xml").toString(); + } + +} diff --git a/marketplace-service/src/main/java/com/axonivy/market/util/FileUtils.java b/marketplace-service/src/main/java/com/axonivy/market/util/FileUtils.java new file mode 100644 index 000000000..6eeb35248 --- /dev/null +++ b/marketplace-service/src/main/java/com/axonivy/market/util/FileUtils.java @@ -0,0 +1,31 @@ +package com.axonivy.market.util; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class FileUtils { + + public static File createFile(String fileName) throws IOException { + File file = new File(fileName); + File parentDir = file.getParentFile(); + if (parentDir != null && !parentDir.exists() && !parentDir.mkdirs()) { + throw new IOException("Failed to create directory: " + parentDir.getAbsolutePath()); + } + if (!file.exists() && !file.createNewFile()) { + throw new IOException("Failed to create file: " + file.getAbsolutePath()); + } + return file; + } + + public static void writeToFile(File file, String content) throws IOException { + try (FileWriter writer = new FileWriter(file, false)) { + writer.write(content); + } + } + +} diff --git a/marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java b/marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java new file mode 100644 index 000000000..f4bf370d2 --- /dev/null +++ b/marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java @@ -0,0 +1,52 @@ +package com.axonivy.market.util; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; + +import java.text.SimpleDateFormat; +import java.util.Map; +import java.util.TreeMap; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class LoggingUtils { + + public static String getCurrentDate() { + return new SimpleDateFormat("yyyy-MM-dd").format(System.currentTimeMillis()); + } + + public static String getCurrentTimestamp() { + return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").format(System.currentTimeMillis()); + } + + public static String escapeXml(String value) { + if (value == null) { + return ""; + } + return value.replace("&", "&") + .replace("<", "<") + .replace(">", ">") + .replace("\"", """) + .replace("'", "'"); + } + + public static String getArgumentsString(String[] paramNames, Object[] args) { + if (paramNames == null || paramNames.length == 0 || args == null || args.length == 0) { + return "No arguments"; + } + return IntStream.range(0, paramNames.length) + .mapToObj(i -> paramNames[i] + ": " + args[i]) + .collect(Collectors.joining(", ")); + } + + public static String buildLogEntry(Map headersMap) { + StringBuilder logEntry = new StringBuilder(); + Map map = new TreeMap<>(headersMap); + logEntry.append(" \n"); + map.forEach((key, value) -> logEntry.append(String.format(" <%s>%s%n", key, value, key))); + logEntry.append(" \n"); + return logEntry.toString(); + } + +} diff --git a/marketplace-service/src/main/resources/application.properties b/marketplace-service/src/main/resources/application.properties index 3e81b354f..a0ab0a261 100644 --- a/marketplace-service/src/main/resources/application.properties +++ b/marketplace-service/src/main/resources/application.properties @@ -16,4 +16,5 @@ market.github.oauth2-clientSecret=${MARKET_GITHUB_OAUTH_APP_CLIENT_SECRET} jwt.secret=${MARKET_JWT_SECRET_KEY} jwt.expiration=365 logging.level.org.springframework.data.mongodb.core.MongoTemplate=${MARKET_MONGO_LOG_LEVEL} -spring.jackson.serialization.indent_output=true \ No newline at end of file +spring.jackson.serialization.indent_output=true +loggable.log-path=${MARKET_LOG_PATH} \ No newline at end of file diff --git a/marketplace-service/src/test/java/com/axonivy/market/service/impl/SchedulingTasksTest.java b/marketplace-service/src/test/java/com/axonivy/market/service/impl/SchedulingTasksTest.java index 23daae4b9..be16eada3 100644 --- a/marketplace-service/src/test/java/com/axonivy/market/service/impl/SchedulingTasksTest.java +++ b/marketplace-service/src/test/java/com/axonivy/market/service/impl/SchedulingTasksTest.java @@ -13,7 +13,8 @@ @SpringBootTest(properties = {"MONGODB_USERNAME=user", "MONGODB_PASSWORD=password", "MONGODB_HOST=mongoHost", "MONGODB_DATABASE=product", "MARKET_GITHUB_OAUTH_APP_CLIENT_ID=clientId", "MARKET_GITHUB_OAUTH_APP_CLIENT_SECRET=clientSecret", "MARKET_JWT_SECRET_KEY=jwtSecret", - "MARKET_CORS_ALLOWED_ORIGIN=*", "MARKET_GITHUB_MARKET_BRANCH=master", "MARKET_MONGO_LOG_LEVEL=DEBUG"}) + "MARKET_CORS_ALLOWED_ORIGIN=*", "MARKET_GITHUB_MARKET_BRANCH=master", "MARKET_MONGO_LOG_LEVEL=DEBUG", + "MARKET_LOG_PATH=logs"}) class SchedulingTasksTest { @SpyBean diff --git a/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java b/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java new file mode 100644 index 000000000..05c201e72 --- /dev/null +++ b/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java @@ -0,0 +1,39 @@ +package com.axonivy.market.util; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; + +@ExtendWith(MockitoExtension.class) +class FileUtilsTest { + + private static final String FILE_PATH = "src/test/resources/test-file.xml"; + + @Test + void testCreateFile() throws IOException { + File createdFile = FileUtils.createFile(FILE_PATH); + Assertions.assertTrue(createdFile.exists(), "File should exist"); + Assertions.assertTrue(createdFile.isFile(), "Should be a file"); + + createdFile.delete(); + } + + @Test + void testWriteFile() throws IOException { + File createdFile = FileUtils.createFile(FILE_PATH); + String content = "Hello, world!"; + FileUtils.writeToFile(createdFile, content); + + String fileContent = Files.readString(createdFile.toPath()); + Assertions.assertEquals(content, fileContent, "File content should match the written content"); + + createdFile.delete(); + + } + +} diff --git a/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java b/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java new file mode 100644 index 000000000..01d58ad07 --- /dev/null +++ b/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java @@ -0,0 +1,46 @@ +package com.axonivy.market.util; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Map; + +@ExtendWith(MockitoExtension.class) +class LoggingUtilsTest { + + @Test + void testEscapeXml() { + String input = ""; + String expectedValue = "<Test'& "Method>"; + String result = LoggingUtils.escapeXml(input); + Assertions.assertEquals(expectedValue, result); + } + + @Test + void testGetArgumentsString() { + String expectedValue = "a: random, b: sample"; + String result = LoggingUtils.getArgumentsString(new String[]{"a", "b"}, new String[]{"random", "sample"}); + Assertions.assertEquals(expectedValue, result); + } + + @Test + void testBuildLogEntry() { + Map given = Map.of( + "method", "test", + "timestamp", "15:02:00" + ); + String expected = """ + + test + 15:02:00 + + """.indent(2); + + var result = LoggingUtils.buildLogEntry(given); + + Assertions.assertEquals(expected, result); + } + +} From 995f8b678c50d0554b17d8e5ab2dd8841a742d08 Mon Sep 17 00:00:00 2001 From: "AAVN\\pvquan" Date: Mon, 16 Dec 2024 13:50:23 +0700 Subject: [PATCH 2/3] update test coverage --- .../market/logging/LoggableAspect.java | 4 +- .../market/logging/LoggableAspectTest.java | 82 +++++++++++++++++++ .../axonivy/market/util/FileUtilsTest.java | 29 ++++++- .../axonivy/market/util/LoggingUtilsTest.java | 37 ++++++++- 4 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java diff --git a/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java b/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java index 4f50ea59c..f04fbb8e4 100644 --- a/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java +++ b/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java @@ -30,7 +30,7 @@ public class LoggableAspect { @Value("${loggable.log-path}") - private String logFilePath; + public String logFilePath; private static final String REQUESTED_BY = "marketplace-website"; @@ -58,7 +58,7 @@ private Map extractHeaders(HttpServletRequest request, MethodSig "timestamp", escapeXml(getCurrentTimestamp()), "user-agent", escapeXml(request.getHeader(CommonConstants.USER_AGENT)), "arguments", escapeXml(getArgumentsString(signature.getParameterNames(), joinPoint.getArgs())), - "x-requested-by", escapeXml(request.getHeader(CommonConstants.REQUESTED_BY)) + "X-Requested-By", escapeXml(request.getHeader(CommonConstants.REQUESTED_BY)) ); } diff --git a/marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java b/marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java new file mode 100644 index 000000000..583e0810c --- /dev/null +++ b/marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java @@ -0,0 +1,82 @@ +package com.axonivy.market.logging; + +import com.axonivy.market.constants.CommonConstants; +import com.axonivy.market.exceptions.model.MissingHeaderException; +import com.axonivy.market.util.LoggingUtils; +import jakarta.servlet.http.HttpServletRequest; +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.reflect.MethodSignature; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.web.context.request.ServletRequestAttributes; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class LoggableAspectTest { + + @Mock + private HttpServletRequest request; + + private LoggableAspect loggableAspect; + + @BeforeEach + void setUp() throws IOException { + MockitoAnnotations.openMocks(this); + loggableAspect = new LoggableAspect(); + loggableAspect.logFilePath = Files.createTempDirectory("logs").toString(); // Use temp directory for tests + } + + @Test + void testLogFileCreation() throws Exception { + mockRequestAttributes("marketplace-website", "test-agent"); + MethodSignature signature = mockMethodSignature(); + + loggableAspect.logMethodCall(mockJoinPoint(signature)); + + Path logFilePath = Path.of(loggableAspect.logFilePath, "log-" + LoggingUtils.getCurrentDate() + ".xml"); + assertTrue(Files.exists(logFilePath), "Log file should be created"); + + String content = Files.readString(logFilePath); + assertTrue(content.contains(""), "Log file should contain log entries"); + } + + @Test + void testMissingHeaderException() { + mockRequestAttributes("invalid-source", "mock-agent"); + MethodSignature signature = mockMethodSignature(); + + assertThrows(MissingHeaderException.class, () -> { + loggableAspect.logMethodCall(mockJoinPoint(signature)); + }); + } + + private JoinPoint mockJoinPoint(MethodSignature signature) { + JoinPoint joinPoint = mock(JoinPoint.class); + when(joinPoint.getSignature()).thenReturn(signature); + when(joinPoint.getArgs()).thenReturn(new Object[]{"arg1", "arg2"}); // Example arguments + return joinPoint; + } + + private void mockRequestAttributes(String requestedBy, String userAgent) { + when(request.getHeader(CommonConstants.REQUESTED_BY)).thenReturn(requestedBy); + when(request.getHeader(CommonConstants.USER_AGENT)).thenReturn(userAgent); + RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request)); + } + + private MethodSignature mockMethodSignature() { + MethodSignature signature = mock(MethodSignature.class); + when(signature.getMethod()).thenReturn(this.getClass().getMethods()[0]); // Use any available method + return signature; + } + +} diff --git a/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java b/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java index 05c201e72..b7db96817 100644 --- a/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java +++ b/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java @@ -1,6 +1,5 @@ package com.axonivy.market.util; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; @@ -9,6 +8,8 @@ import java.io.IOException; import java.nio.file.Files; +import static org.junit.jupiter.api.Assertions.*; + @ExtendWith(MockitoExtension.class) class FileUtilsTest { @@ -17,12 +18,32 @@ class FileUtilsTest { @Test void testCreateFile() throws IOException { File createdFile = FileUtils.createFile(FILE_PATH); - Assertions.assertTrue(createdFile.exists(), "File should exist"); - Assertions.assertTrue(createdFile.isFile(), "Should be a file"); + assertTrue(createdFile.exists(), "File should exist"); + assertTrue(createdFile.isFile(), "Should be a file"); createdFile.delete(); } + @Test + void testFailedToCreateDirectory() throws IOException { + File createdFile = new File("testDirAsFile"); + try { + if (!createdFile.exists()) { + assertTrue(createdFile.createNewFile(), "Setup failed: could not create file"); + } + + IOException exception = assertThrows(IOException.class, () -> { + FileUtils.createFile("testDirAsFile/subDir/testFile.txt"); + }); + assertTrue(exception.getMessage().contains("Failed to create directory"), + "Exception message does not contain expected text"); + } catch (IOException e) { + fail("Setup failed: " + e.getMessage()); + } finally { + createdFile.delete(); + } + } + @Test void testWriteFile() throws IOException { File createdFile = FileUtils.createFile(FILE_PATH); @@ -30,7 +51,7 @@ void testWriteFile() throws IOException { FileUtils.writeToFile(createdFile, content); String fileContent = Files.readString(createdFile.toPath()); - Assertions.assertEquals(content, fileContent, "File content should match the written content"); + assertEquals(content, fileContent, "File content should match the written content"); createdFile.delete(); diff --git a/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java b/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java index 01d58ad07..00d788744 100644 --- a/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java +++ b/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java @@ -5,19 +5,29 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; import java.util.Map; @ExtendWith(MockitoExtension.class) class LoggingUtilsTest { @Test - void testEscapeXml() { + void testEscapeXmlSuccess() { String input = ""; String expectedValue = "<Test'& "Method>"; String result = LoggingUtils.escapeXml(input); Assertions.assertEquals(expectedValue, result); } + @Test + void testEscapeXmlOnNullValue() { + String expectedValue = ""; + String result = LoggingUtils.escapeXml(null); + Assertions.assertEquals(expectedValue, result); + } + @Test void testGetArgumentsString() { String expectedValue = "a: random, b: sample"; @@ -25,6 +35,13 @@ void testGetArgumentsString() { Assertions.assertEquals(expectedValue, result); } + @Test + void testGetArgumentsStringOnNullValue() { + String expectedValue = "No arguments"; + String result = LoggingUtils.getArgumentsString(null, null); + Assertions.assertEquals(expectedValue, result); + } + @Test void testBuildLogEntry() { Map given = Map.of( @@ -43,4 +60,22 @@ void testBuildLogEntry() { Assertions.assertEquals(expected, result); } + @Test + void testGetCurrentDate() { + String expectedDate = LocalDate.now().toString(); + String actualDate = LoggingUtils.getCurrentDate(); + + Assertions.assertEquals(expectedDate, actualDate, "The returned date does not match the current date"); + } + + @Test + void testGetCurrentTimestamp() { + String expectedTimestamp = LocalDateTime.now() + .format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss")); + String actualTimestamp = LoggingUtils.getCurrentTimestamp(); + + Assertions.assertEquals(expectedTimestamp.substring(0, 19), actualTimestamp.substring(0, 19), + "The returned timestamp does not match the expected format or value"); + } + } From 935ed8b67aa05fbb6488b9727e8ccf6f3169e1a9 Mon Sep 17 00:00:00 2001 From: "AAVN\\pvquan" Date: Mon, 16 Dec 2024 16:30:00 +0700 Subject: [PATCH 3/3] handle feedback --- marketplace-build/dev/docker-compose.yml | 2 ++ marketplace-build/docker-compose.yml | 2 ++ marketplace-build/release/docker-compose.yml | 2 ++ .../market/constants/LoggingConstants.java | 22 +++++++++++++++ .../market/logging/LoggableAspect.java | 22 +++++++-------- .../com/axonivy/market/util/LoggingUtils.java | 18 +++++++------ .../market/logging/LoggableAspectTest.java | 27 ++++++++++++------- .../axonivy/market/util/FileUtilsTest.java | 11 +++----- .../axonivy/market/util/LoggingUtilsTest.java | 9 +++---- 9 files changed, 72 insertions(+), 43 deletions(-) create mode 100644 marketplace-service/src/main/java/com/axonivy/market/constants/LoggingConstants.java diff --git a/marketplace-build/dev/docker-compose.yml b/marketplace-build/dev/docker-compose.yml index 260fd1f6c..ae81c5613 100644 --- a/marketplace-build/dev/docker-compose.yml +++ b/marketplace-build/dev/docker-compose.yml @@ -24,6 +24,7 @@ services: volumes: - /home/axonivy/marketplace/data/market-installations.json:/app/data/market-installation.json - marketcache:/app/data/market-cache + - ./logs:/app/logs environment: - MONGODB_HOST=${SERVICE_MONGODB_HOST} - MONGODB_DATABASE=${SERVICE_MONGODB_DATABASE} @@ -36,6 +37,7 @@ services: - MARKET_JWT_SECRET_KEY=${MARKET_JWT_SECRET_KEY} - MARKET_CORS_ALLOWED_ORIGIN=${MARKET_CORS_ALLOWED_ORIGIN} - MARKET_MONGO_LOG_LEVEL=${MARKET_MONGO_LOG_LEVEL} + - MARKET_LOG_PATH=${MARKET_LOG_PATH} build: context: ../../marketplace-service dockerfile: Dockerfile diff --git a/marketplace-build/docker-compose.yml b/marketplace-build/docker-compose.yml index 74011c3a5..d57c8ad8e 100644 --- a/marketplace-build/docker-compose.yml +++ b/marketplace-build/docker-compose.yml @@ -24,6 +24,7 @@ services: volumes: - /home/axonivy/marketplace/data/market-installations.json:/app/data/market-installation.json - marketcache:/app/data/market-cache + - ./logs:/app/logs environment: - MONGODB_HOST=${SERVICE_MONGODB_HOST} - MONGODB_DATABASE=${SERVICE_MONGODB_DATABASE} @@ -36,6 +37,7 @@ services: - MARKET_JWT_SECRET_KEY=${MARKET_JWT_SECRET_KEY} - MARKET_CORS_ALLOWED_ORIGIN=${MARKET_CORS_ALLOWED_ORIGIN} - MARKET_MONGO_LOG_LEVEL=${MARKET_MONGO_LOG_LEVEL} + - MARKET_LOG_PATH=${MARKET_LOG_PATH} build: context: ../marketplace-service dockerfile: Dockerfile diff --git a/marketplace-build/release/docker-compose.yml b/marketplace-build/release/docker-compose.yml index f8fc73f25..018614b70 100644 --- a/marketplace-build/release/docker-compose.yml +++ b/marketplace-build/release/docker-compose.yml @@ -22,6 +22,7 @@ services: volumes: - /home/axonivy/marketplace/data/market-installations.json:/app/data/market-installation.json - marketcache:/app/data/market-cache + - ./logs:/app/logs environment: - MONGODB_HOST=${SERVICE_MONGODB_HOST} - MONGODB_DATABASE=${SERVICE_MONGODB_DATABASE} @@ -34,6 +35,7 @@ services: - MARKET_JWT_SECRET_KEY=${MARKET_JWT_SECRET_KEY} - MARKET_CORS_ALLOWED_ORIGIN=${MARKET_CORS_ALLOWED_ORIGIN} - MARKET_MONGO_LOG_LEVEL=${MARKET_MONGO_LOG_LEVEL} + - MARKET_LOG_PATH=${MARKET_LOG_PATH} networks: - marketplace-network diff --git a/marketplace-service/src/main/java/com/axonivy/market/constants/LoggingConstants.java b/marketplace-service/src/main/java/com/axonivy/market/constants/LoggingConstants.java new file mode 100644 index 000000000..2ce721583 --- /dev/null +++ b/marketplace-service/src/main/java/com/axonivy/market/constants/LoggingConstants.java @@ -0,0 +1,22 @@ +package com.axonivy.market.constants; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class LoggingConstants { + + public static final String ENTRY_FORMAT = " <%s>%s%n"; + public static final String ENTRY_START = " \n"; + public static final String ENTRY_END = " \n"; + public static final String DATE_FORMAT = "yyyy-MM-dd"; + public static final String TIMESTAMP_FORMAT = "yyyy-MM-dd HH:mm:ss"; + public static final String LOG_START = "\n"; + public static final String LOG_END = ""; + public static final String METHOD = "method"; + public static final String ARGUMENTS = "arguments"; + public static final String TIMESTAMP = "timestamp"; + public static final String NO_ARGUMENTS = "No arguments"; + public static final String MARKET_WEBSITE = "marketplace-website"; + +} diff --git a/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java b/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java index f04fbb8e4..059a19983 100644 --- a/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java +++ b/marketplace-service/src/main/java/com/axonivy/market/logging/LoggableAspect.java @@ -1,6 +1,7 @@ package com.axonivy.market.logging; import com.axonivy.market.constants.CommonConstants; +import com.axonivy.market.constants.LoggingConstants; import com.axonivy.market.exceptions.model.MissingHeaderException; import jakarta.servlet.http.HttpServletRequest; import lombok.extern.log4j.Log4j2; @@ -18,7 +19,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Map; -import java.util.Objects; import static com.axonivy.market.util.FileUtils.createFile; import static com.axonivy.market.util.FileUtils.writeToFile; @@ -32,8 +32,6 @@ public class LoggableAspect { @Value("${loggable.log-path}") public String logFilePath; - private static final String REQUESTED_BY = "marketplace-website"; - @Before("@annotation(com.axonivy.market.logging.Loggable)") public void logMethodCall(JoinPoint joinPoint) throws MissingHeaderException { MethodSignature signature = (MethodSignature) joinPoint.getSignature(); @@ -45,7 +43,7 @@ public void logMethodCall(JoinPoint joinPoint) throws MissingHeaderException { saveLogToDailyFile(headersMap); // block execution if request isn't from Market or Ivy Designer - if (!Objects.equals(headersMap.get(CommonConstants.REQUESTED_BY), REQUESTED_BY)) { + if (!LoggingConstants.MARKET_WEBSITE.equals(headersMap.get(CommonConstants.REQUESTED_BY))) { throw new MissingHeaderException(); } } @@ -54,11 +52,11 @@ public void logMethodCall(JoinPoint joinPoint) throws MissingHeaderException { private Map extractHeaders(HttpServletRequest request, MethodSignature signature, JoinPoint joinPoint) { return Map.of( - "method", escapeXml(String.valueOf(signature.getMethod())), - "timestamp", escapeXml(getCurrentTimestamp()), - "user-agent", escapeXml(request.getHeader(CommonConstants.USER_AGENT)), - "arguments", escapeXml(getArgumentsString(signature.getParameterNames(), joinPoint.getArgs())), - "X-Requested-By", escapeXml(request.getHeader(CommonConstants.REQUESTED_BY)) + LoggingConstants.METHOD, escapeXml(String.valueOf(signature.getMethod())), + LoggingConstants.TIMESTAMP, escapeXml(getCurrentTimestamp()), + CommonConstants.USER_AGENT, escapeXml(request.getHeader(CommonConstants.USER_AGENT)), + LoggingConstants.ARGUMENTS, escapeXml(getArgumentsString(signature.getParameterNames(), joinPoint.getArgs())), + CommonConstants.REQUESTED_BY, escapeXml(request.getHeader(CommonConstants.REQUESTED_BY)) ); } @@ -72,14 +70,14 @@ private synchronized void saveLogToDailyFile(Map headersMap) { content.append(new String(Files.readAllBytes(logFile.toPath()))); } if (content.isEmpty()) { - content.append("\n"); + content.append(LoggingConstants.LOG_START); } - int lastLogIndex = content.lastIndexOf(""); + int lastLogIndex = content.lastIndexOf(LoggingConstants.LOG_END); if (lastLogIndex != -1) { content.delete(lastLogIndex, content.length()); } content.append(buildLogEntry(headersMap)); - content.append("\n"); + content.append(LoggingConstants.LOG_END); writeToFile(logFile, content.toString()); } catch (IOException e) { diff --git a/marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java b/marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java index f4bf370d2..e69929bc1 100644 --- a/marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java +++ b/marketplace-service/src/main/java/com/axonivy/market/util/LoggingUtils.java @@ -1,7 +1,9 @@ package com.axonivy.market.util; +import com.axonivy.market.constants.LoggingConstants; import lombok.AccessLevel; import lombok.NoArgsConstructor; +import org.apache.commons.lang3.StringUtils; import java.text.SimpleDateFormat; import java.util.Map; @@ -13,16 +15,16 @@ public class LoggingUtils { public static String getCurrentDate() { - return new SimpleDateFormat("yyyy-MM-dd").format(System.currentTimeMillis()); + return new SimpleDateFormat(LoggingConstants.DATE_FORMAT).format(System.currentTimeMillis()); } public static String getCurrentTimestamp() { - return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").format(System.currentTimeMillis()); + return new SimpleDateFormat(LoggingConstants.TIMESTAMP_FORMAT).format(System.currentTimeMillis()); } public static String escapeXml(String value) { - if (value == null) { - return ""; + if (StringUtils.isEmpty(value)) { + return StringUtils.EMPTY; } return value.replace("&", "&") .replace("<", "<") @@ -33,7 +35,7 @@ public static String escapeXml(String value) { public static String getArgumentsString(String[] paramNames, Object[] args) { if (paramNames == null || paramNames.length == 0 || args == null || args.length == 0) { - return "No arguments"; + return LoggingConstants.NO_ARGUMENTS; } return IntStream.range(0, paramNames.length) .mapToObj(i -> paramNames[i] + ": " + args[i]) @@ -43,9 +45,9 @@ public static String getArgumentsString(String[] paramNames, Object[] args) { public static String buildLogEntry(Map headersMap) { StringBuilder logEntry = new StringBuilder(); Map map = new TreeMap<>(headersMap); - logEntry.append(" \n"); - map.forEach((key, value) -> logEntry.append(String.format(" <%s>%s%n", key, value, key))); - logEntry.append(" \n"); + logEntry.append(LoggingConstants.ENTRY_START); + map.forEach((key, value) -> logEntry.append(String.format(LoggingConstants.ENTRY_FORMAT, key, value, key))); + logEntry.append(LoggingConstants.ENTRY_END); return logEntry.toString(); } diff --git a/marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java b/marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java index 583e0810c..0270dfd60 100644 --- a/marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java +++ b/marketplace-service/src/test/java/com/axonivy/market/logging/LoggableAspectTest.java @@ -1,11 +1,13 @@ package com.axonivy.market.logging; import com.axonivy.market.constants.CommonConstants; +import com.axonivy.market.constants.LoggingConstants; import com.axonivy.market.exceptions.model.MissingHeaderException; import com.axonivy.market.util.LoggingUtils; import jakarta.servlet.http.HttpServletRequest; import org.aspectj.lang.JoinPoint; import org.aspectj.lang.reflect.MethodSignature; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; @@ -22,7 +24,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class LoggableAspectTest { +class LoggableAspectTest { @Mock private HttpServletRequest request; @@ -33,21 +35,26 @@ public class LoggableAspectTest { void setUp() throws IOException { MockitoAnnotations.openMocks(this); loggableAspect = new LoggableAspect(); - loggableAspect.logFilePath = Files.createTempDirectory("logs").toString(); // Use temp directory for tests + loggableAspect.logFilePath = Files.createTempDirectory("logs").toString(); + } + + @AfterEach + void tearDown() { + RequestContextHolder.resetRequestAttributes(); } @Test void testLogFileCreation() throws Exception { - mockRequestAttributes("marketplace-website", "test-agent"); + mockRequestAttributes(LoggingConstants.MARKET_WEBSITE, "test-agent"); MethodSignature signature = mockMethodSignature(); loggableAspect.logMethodCall(mockJoinPoint(signature)); - Path logFilePath = Path.of(loggableAspect.logFilePath, "log-" + LoggingUtils.getCurrentDate() + ".xml"); assertTrue(Files.exists(logFilePath), "Log file should be created"); String content = Files.readString(logFilePath); - assertTrue(content.contains(""), "Log file should contain log entries"); + assertTrue(content.contains(LoggingConstants.LOG_START), "Log file should contain log"); + assertTrue(content.contains(LoggingConstants.ENTRY_START), "Log file should contain log entry"); } @Test @@ -55,15 +62,15 @@ void testMissingHeaderException() { mockRequestAttributes("invalid-source", "mock-agent"); MethodSignature signature = mockMethodSignature(); - assertThrows(MissingHeaderException.class, () -> { - loggableAspect.logMethodCall(mockJoinPoint(signature)); - }); + assertThrows(MissingHeaderException.class, () -> + loggableAspect.logMethodCall(mockJoinPoint(signature)) + ); } private JoinPoint mockJoinPoint(MethodSignature signature) { JoinPoint joinPoint = mock(JoinPoint.class); when(joinPoint.getSignature()).thenReturn(signature); - when(joinPoint.getArgs()).thenReturn(new Object[]{"arg1", "arg2"}); // Example arguments + when(joinPoint.getArgs()).thenReturn(new Object[]{"arg1", "arg2"}); return joinPoint; } @@ -75,7 +82,7 @@ private void mockRequestAttributes(String requestedBy, String userAgent) { private MethodSignature mockMethodSignature() { MethodSignature signature = mock(MethodSignature.class); - when(signature.getMethod()).thenReturn(this.getClass().getMethods()[0]); // Use any available method + when(signature.getMethod()).thenReturn(this.getClass().getMethods()[0]); return signature; } diff --git a/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java b/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java index b7db96817..196152d82 100644 --- a/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java +++ b/marketplace-service/src/test/java/com/axonivy/market/util/FileUtilsTest.java @@ -20,21 +20,20 @@ void testCreateFile() throws IOException { File createdFile = FileUtils.createFile(FILE_PATH); assertTrue(createdFile.exists(), "File should exist"); assertTrue(createdFile.isFile(), "Should be a file"); - createdFile.delete(); } @Test - void testFailedToCreateDirectory() throws IOException { + void testFailedToCreateDirectory() { File createdFile = new File("testDirAsFile"); try { if (!createdFile.exists()) { assertTrue(createdFile.createNewFile(), "Setup failed: could not create file"); } - IOException exception = assertThrows(IOException.class, () -> { - FileUtils.createFile("testDirAsFile/subDir/testFile.txt"); - }); + IOException exception = assertThrows(IOException.class, () -> + FileUtils.createFile("testDirAsFile/subDir/testFile.txt") + ); assertTrue(exception.getMessage().contains("Failed to create directory"), "Exception message does not contain expected text"); } catch (IOException e) { @@ -49,10 +48,8 @@ void testWriteFile() throws IOException { File createdFile = FileUtils.createFile(FILE_PATH); String content = "Hello, world!"; FileUtils.writeToFile(createdFile, content); - String fileContent = Files.readString(createdFile.toPath()); assertEquals(content, fileContent, "File content should match the written content"); - createdFile.delete(); } diff --git a/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java b/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java index 00d788744..be0ad64fa 100644 --- a/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java +++ b/marketplace-service/src/test/java/com/axonivy/market/util/LoggingUtilsTest.java @@ -1,5 +1,6 @@ package com.axonivy.market.util; +import com.axonivy.market.constants.LoggingConstants; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -37,9 +38,8 @@ void testGetArgumentsString() { @Test void testGetArgumentsStringOnNullValue() { - String expectedValue = "No arguments"; String result = LoggingUtils.getArgumentsString(null, null); - Assertions.assertEquals(expectedValue, result); + Assertions.assertEquals(LoggingConstants.NO_ARGUMENTS, result); } @Test @@ -56,7 +56,6 @@ void testBuildLogEntry() { """.indent(2); var result = LoggingUtils.buildLogEntry(given); - Assertions.assertEquals(expected, result); } @@ -64,16 +63,14 @@ void testBuildLogEntry() { void testGetCurrentDate() { String expectedDate = LocalDate.now().toString(); String actualDate = LoggingUtils.getCurrentDate(); - Assertions.assertEquals(expectedDate, actualDate, "The returned date does not match the current date"); } @Test void testGetCurrentTimestamp() { String expectedTimestamp = LocalDateTime.now() - .format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss")); + .format(DateTimeFormatter.ofPattern(LoggingConstants.TIMESTAMP_FORMAT)); String actualTimestamp = LoggingUtils.getCurrentTimestamp(); - Assertions.assertEquals(expectedTimestamp.substring(0, 19), actualTimestamp.substring(0, 19), "The returned timestamp does not match the expected format or value"); }