Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.idea/*
*.iml
target/*
log/
7 changes: 7 additions & 0 deletions log/app.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
2026-07-07 19:10:23 [main] INFO mate.academy.service.AuthenticationServiceImpl - Method login was called for user: bob
2026-07-07 19:10:23 [main] INFO mate.academy.service.AuthenticationServiceImpl - User bob successfully logged in.
2026-07-07 19:10:23 [main] INFO mate.academy.service.OrderServiceImpl - Method completeOrder was called for user Id: 2
2026-07-07 19:12:49 [main] INFO mate.academy.service.AuthenticationServiceImpl - Method login was called for user: bob
2026-07-07 19:12:49 [main] INFO mate.academy.service.AuthenticationServiceImpl - User bob successfully logged in.
2026-07-07 19:12:49 [main] INFO mate.academy.service.OrderServiceImpl - Method completeOrder was called for user Id: 2
2026-07-07 19:12:49 [main] INFO mate.academy.service.OrderServiceImpl - Successfully fetched data from DB for user Id: 2. Retracted 3 products.
22 changes: 22 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,29 @@
<version>4.12</version>
<scope>test</scope>
</dependency>


<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>2.0.12</version>
</dependency>

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<version>2.23.1</version>
</dependency>

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-slf4j2-impl</artifactId>
<version>2.23.1</version>
</dependency>

</dependencies>


<build>
<plugins>
<plugin>
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/mate/academy/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ public static void main(String[] args) {
try {
user = authenticationService.login("bob", "1234");
} catch (AuthenticationException e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #1: Let's remove all TODO comments before submitting solution. You still have a TODO comment here; implement the logging as required and then remove the TODO line.

e.printStackTrace();
return;
throw new RuntimeException("Authentification failed for user bob");
}
OrderService orderService = new OrderServiceImpl();
orderService.completeOrder(user.getUserId());
Expand Down
28 changes: 22 additions & 6 deletions src/main/java/mate/academy/service/AuthenticationServiceImpl.java
Original file line number Diff line number Diff line change
@@ -1,23 +1,39 @@
package mate.academy.service;

import java.util.Objects;
import mate.academy.exception.AuthenticationException;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task requires a proper logger configuration with both File and Console appenders so logs go to console and a file under an ignored directory like log/. There is no such configuration file (e.g., logback.xml or log4j2.xml) in the repo, which violates the core requirement: "Configure the logger in this project. Add appenders for File and Console."

import mate.academy.model.User;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class AuthenticationServiceImpl implements AuthenticationService {

private static final Logger logger = LoggerFactory.getLogger(AuthenticationServiceImpl.class);

@Override
public User login(String login, String password) throws AuthenticationException {
//TODO: add corresponding log message about method login was called
logger.info("Method login was called for user: {}", login);
User user = findByLogin(login);
Comment thread
3xactCoder marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log message is informative but could better follow the checklist guidance about parameterized messages. Also, you might want a small space after the comma for readability: logger.info("Method completeOrder was called for user Id: {}", userId);. More importantly, consider whether you want to include additional context (e.g., number of products) to make the log more useful.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no space after the comma between {} and userId in this log call, which was part of the previous medium-level feedback about tidy and consistent log formatting. Adjusting to logger.info("Method completeOrder was called for user Id: {}", userId); will improve readability and parameterization.

if (!user.getPassword().equals(password)) {

Comment on lines +10 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task requires: "Configure the logger in this project. Add appenders for File and Console" and "If you have System.out.println() then replace them all with corresponding logger methods" plus "Replace all e.printStackTrace(); with corresponding logger methods." While this file has no System.out.println or printStackTrace, it also does not use a logger at all, and in the catch block you immediately throw a RuntimeException without logging the failure. Consider adding a logger to Main and logging the authentication failure (including the exception) before handling or rethrowing to align with the project-wide logging requirement and make the error easier to trace.

if (user == null || !Objects.equals(user.getPassword(), password)) {

logger.warn("Failed login attempt for user: {}", login);
throw new AuthenticationException("Username or password are incorrect");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log message does not follow checklist item Let's make our log messages informative. It hard-codes the username and doesn’t include any parameters; consider using a parameterized message with the login value and logging it before or along with the login attempt (e.g., logger.info("User {} tries to login", login) or similar) rather than a fixed string.

logger.info("User {} successfully logged in.", login);
Comment on lines +18 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method currently never throws AuthenticationException when login fails and will log "User ... successfully logged in" even when user is null or the password is wrong. That breaks the intended authentication behavior: on invalid credentials, it should log the failed attempt and then throw AuthenticationException instead of proceeding to success logging and returning null. Adjust the control flow so that failed login attempts lead to an exception and skip the success log.

return user;
}
Comment on lines 24 to 25

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When logging the authentication failure, you are not including the exception e in the log call, which makes debugging harder. Also, the message is slightly misspelled and not parameterized. To improve observability and align with the project-wide logging requirement, log the failure with the exception (e.g., logger.error("Authentication failed for user {}", "bob", e);) before throwing, instead of a plain message without the cause.


private User findByLogin(String login) {
User user = new User(login, "1234");
// this user identifier should be set by DB. We will use dummy data for this example
user.setUserId(2L);
return user;
if ("bob".equals(login)) {
User user = new User(login, "1234");
user.setUserId(2L);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log message has a trailing space before the closing quote: "... Id: {}. ". The previous review asked to clean up such minor issues so log output is tidy; removing the extra space (and keeping the period if you want) will make the message consistent.

return user;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message has a trailing space inside the format string ("... {}. "), which is slightly inconsistent and unnecessary. While minor, cleaning it to "Successfully fetched data from DB for user with Id: {}." will keep logs tidy and more professional.

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #1: "Let's remove all TODO comments before submitting solution." The comment // TODO: add log message about successful fetched data from DB should be removed now that the log statement is present.

return null;
}
Comment on lines +33 to 34

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log message concatenates strings and parameters incorrectly: "Successfully fetched data from DB for user Id:" + " {}. Retracted {} products." with userId and products.size() passed as arguments. This will not substitute the placeholders correctly and also makes the message less clear. This conflicts with checklist item #3: "Let's make our log messages informative" (compare against the "Good example" which uses a single format string with placeholders). Consider using a single format string like logger.info("Successfully fetched data from DB for user Id: {}. Retrieved {} products.", userId, products.size());.

}



8 changes: 8 additions & 0 deletions src/main/java/mate/academy/service/OrderServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@
import java.util.List;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .gitignore correctly ignores the log/ directory for log files, but make sure that your actual log file path used by the logger configuration will reside under this log/ directory; otherwise the requirement "so that your file with logs is NOT pushed to GitHub" would not be fully satisfied.

import mate.academy.model.Order;
import mate.academy.model.Product;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class OrderServiceImpl implements OrderService {

private static final Logger logger = LoggerFactory.getLogger(OrderServiceImpl.class);

@Override
public Order completeOrder(Long userId) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The info log for the login method is informative and parameterized, which satisfies the checklist, but you can improve readability by adding a space after the colon: "Method login was called for user: {}". The earlier review explicitly mentioned tidying spacing/parameterization of these "method ... was called" logs.

// TODO: add log message about method completeOrder was called

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item: "Let's remove all TODO comments before submitting solution." You’ve added the log call, but the TODO comment itself still needs to be removed.

logger.info("Method completeOrder was called for user Id: {}",userId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, here you have no space after the comma between the literal and the placeholder ("... Id: {}",userId). For cleaner, consistent logging per the checklist and prior comments, add a space: "... Id: {}", userId.

List<Product> products = getAllProductsFromShoppingCart(userId);
Order order = new Order(products, userId);
// NOTE: In production ready code this order identifier should be generated by DB
Expand All @@ -25,6 +31,8 @@ private List<Product> getAllProductsFromShoppingCart(Long userId) {
Product xiaomi = new Product("Xiaomi 12", BigDecimal.valueOf(499));
List<Product> products = List.of(iphone, macBook, xiaomi);
// TODO: add log message about successful fetched data from DB

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item: "Let's remove all TODO comments before submitting solution." You added a log message, but the TODO comment is still present and should be deleted.

logger.info("Successfully fetched data from DB for user Id:"
+ " {}. Retracted {} products.", userId, products.size());
return products;
}
}
23 changes: 23 additions & 0 deletions src/main/resources/log4j2.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
<Configuration status="WARN">
<Properties>
<Property name="basePath">logs</Property>
</Properties>

<Appenders>
<Console name="ConsoleAppender" target="SYSTEM_OUT">
<PatternLayout pattern="%d{yyyy-MM-dd HH:mm:ss} [%t] %-5level %logger{36} - %msg%n"/>
</Console>

<File name="FileAppender" fileName="log/app.log" append="true">
<PatternLayout pattern="%d{yyyy-MM-dd HH:mm:ss} [%t] %-5level %logger{36} - %msg%n"/>
</File>
</Appenders>

<Loggers>
<Root level="info">
<AppenderRef ref="ConsoleAppender"/>
<AppenderRef ref="FileAppender"/>
</Root>
</Loggers>
</Configuration>
Loading