Skip to content

Commit

Permalink
handle feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
quanpham-axonivy committed Dec 16, 2024
1 parent 995f8b6 commit 46c9534
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 41 deletions.
1 change: 1 addition & 0 deletions marketplace-build/dev/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,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
Expand Down
1 change: 1 addition & 0 deletions marketplace-build/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,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
Expand Down
1 change: 1 addition & 0 deletions marketplace-build/release/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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

Expand Down
Original file line number Diff line number Diff line change
@@ -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</%s>%n";
public static final String ENTRY_START = " <LogEntry>\n";
public static final String ENTRY_END = " </LogEntry>\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 = "<Logs>\n";
public static final String LOG_END = "</Logs>";
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";

}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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();
}
}
Expand All @@ -54,11 +52,11 @@ public void logMethodCall(JoinPoint joinPoint) throws MissingHeaderException {
private Map<String, String> 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))
);
}

Expand All @@ -72,14 +70,14 @@ private synchronized void saveLogToDailyFile(Map<String, String> headersMap) {
content.append(new String(Files.readAllBytes(logFile.toPath())));
}
if (content.isEmpty()) {
content.append("<Logs>\n");
content.append(LoggingConstants.LOG_START);
}
int lastLogIndex = content.lastIndexOf("</Logs>");
int lastLogIndex = content.lastIndexOf(LoggingConstants.LOG_END);
if (lastLogIndex != -1) {
content.delete(lastLogIndex, content.length());
}
content.append(buildLogEntry(headersMap));
content.append("</Logs>\n");
content.append(LoggingConstants.LOG_END);

writeToFile(logFile, content.toString());
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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("&", "&amp;")
.replace("<", "&lt;")
Expand All @@ -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])
Expand All @@ -43,9 +45,9 @@ public static String getArgumentsString(String[] paramNames, Object[] args) {
public static String buildLogEntry(Map<String, String> headersMap) {
StringBuilder logEntry = new StringBuilder();
Map<String, String> map = new TreeMap<>(headersMap);
logEntry.append(" <LogEntry>\n");
map.forEach((key, value) -> logEntry.append(String.format(" <%s>%s</%s>%n", key, value, key)));
logEntry.append(" </LogEntry>\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();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -33,37 +35,42 @@ 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("<Logs>"), "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
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;
}

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();

}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -56,15 +56,13 @@ void testBuildLogEntry() {
""".indent(2);

var result = LoggingUtils.buildLogEntry(given);

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");
}

Expand All @@ -73,7 +71,6 @@ 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");
}
Expand Down

0 comments on commit 46c9534

Please sign in to comment.