Skip to content

resolved the task #1241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AndriiAlieksieienko wants to merge 3 commits into
base: master
Choose a base branch
from
Open

resolved the task #1241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5814ab9
resolved the task
AlieksieienkoAndriy Apr 14, 2026
8fabfe9
improved the code
AlieksieienkoAndriy Apr 14, 2026
80af0e0
changed the log
AlieksieienkoAndriy Apr 14, 2026
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.idea/*
*.iml
target/*
logs/*
12 changes: 12 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@
<version>4.12</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
<version>2.22.1</version>
</dependency>

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<version>2.22.1</version>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/mate/academy/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,20 @@
import mate.academy.service.AuthenticationServiceImpl;
import mate.academy.service.OrderService;
import mate.academy.service.OrderServiceImpl;
import org.apache.logging.log4j.LogManager;
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

This class uses Log4j2, but without the required configuration file the File appender won't be created. Create src/main/resources/log4j2.xml with a Console appender, a File appender to "logs/app.log", and a root logger referencing both.

import org.apache.logging.log4j.Logger;

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

This logger usage is fine, but as above you must add the Log4j2 config so file logging works. Ensure the File appender's fileName is exactly "logs/app.log" and the root logger references both Console and File appenders.

public class Main {
private static final Logger logger = LogManager.getLogger(Main.class);
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

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" (description). I don't see a Log4j2 configuration file (e.g. log4j2.xml or log4j2.properties) in the project. Add a configuration with Console and File appenders so logs are written where expected and the File appender can write into the logs directory excluded by .gitignore.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

The project has logger usage here but there is no visible Log4j2 configuration file in the repository. Per the task description you must add a Log4j2 config (for example src/main/resources/log4j2.xml) that defines both a Console appender and a File appender writing to logs/app.log so that logs are written to the logs/ directory. Ensure the file appender path is logs/app.log and that logs/ remains ignored by .gitignore.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

Logging is used here but the project lacks the Log4j2 configuration file. Add src/main/resources/log4j2.xml defining a Console appender and a File appender whose fileName is exactly "logs/app.log", and make the root logger reference both appenders so logs go to console and file.


Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

There is an outstanding TODO in this class that wasn't removed/completed. The requirements state: "Remove all TODO comments before submitting solution. [CHECKLIST ITEM #1]" and "Complete all TODO statements" (description). Also the current log logger.info("login method was called") is not informative — follow checklist item #3 and include method parameters (avoid logging passwords).

public static void main(String[] args) {
logger.info("Application started");
AuthenticationService authenticationService = new AuthenticationServiceImpl();
User user;
try {
user = authenticationService.login("bob", "1234");
} catch (AuthenticationException e) {
e.printStackTrace();
logger.error("Login failed for user: {}", "bob", e);
return;
}
OrderService orderService = new OrderServiceImpl();
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/mate/academy/service/AuthenticationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

import mate.academy.exception.AuthenticationException;
import mate.academy.model.User;
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

This violates the project requirement: "Configure the logger in this project. Add appenders for File and Console". I cannot find a visible Log4j2 configuration (for example src/main/resources/log4j2.xml). Please add a Log4j2 config that defines both a Console appender and a File appender writing to logs/app.log and attaches them to the root logger. Note: .gitignore already contains logs/*, so the log file will be ignored as required.

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class AuthenticationServiceImpl implements AuthenticationService {
private static final Logger logger = LogManager.getLogger(AuthenticationServiceImpl.class);
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

The task requires configuring the logger and adding appenders for File and Console (e.g., provide a log4j2.xml or programmatic configuration). I don't see any Log4j2 configuration file in the project. Please add a config with a Console and a File appender (and set the file path to a folder ignored by .gitignore).


@Override
public User login(String login, String password) throws AuthenticationException {
//TODO: add corresponding log message about method login was called
logger.info("Login attempt for user: {}", login);
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

The project is missing the Log4j2 configuration file that sets up Console and File appenders (so file logging to logs/app.log won't work). Please add e.g. src/main/resources/log4j2.xml that defines a Console appender and a File appender whose fileName is exactly logs/app.log, and a root logger that references both appenders. This addresses the requirement: "Configure the logger in this project and add appenders for File and Console so that logging goes to both destinations." Also follow the previous review note: "Add a Log4j2 configuration file, e.g. src/main/resources/log4j2.xml, that defines: - A Console appender. - A File appender whose fileName is exactly logs/app.log so logs are written into the logs/ directory. - A root logger that references both appenders so all existing logger calls output to both console and file."

User user = findByLogin(login);
if (!user.getPassword().equals(password)) {
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

The log message for completeOrder is not informative — it should include context such as the userId (or other relevant params). This violates the checklist item: "Let's make our log messages informative". Consider: logger.info("completeOrder called for userId={}", userId);

throw new AuthenticationException("Username or password are incorrect");
throw new AuthenticationException("Invalid credentials for user: " + login);
}
return user;
}
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/mate/academy/service/OrderServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
import java.util.List;
import mate.academy.model.Order;
import mate.academy.model.Product;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

The class uses Log4j2 but without the required configuration file the File appender won't be created. Add src/main/resources/log4j2.xml (Console + File appenders, fileName="logs/app.log") so these logger calls also write to the file.

public class OrderServiceImpl implements OrderService {
private static final Logger logger = LogManager.getLogger(OrderServiceImpl.class);
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

This logger is fine in code, but the File appender is still missing at the project level. Create the required log4j2.xml under src/main/resources with a Console appender and a File appender pointing to logs/app.log and reference both from the root logger.


@Override
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

The project lacks a Log4j2 configuration (e.g., log4j2.xml or log4j2.properties) that defines Console and File appenders. The code creates loggers but without a config file the required appenders won't be set up — the task requires "Configure the logger in this project. Add appenders for File and Console."

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

The project uses Log4j2 but I don't see a Log4j2 configuration file (e.g. src/main/resources/log4j2.xml) that defines both a Console and a File appender. The task requires: "Configure the logger in this project. Add appenders for File and Console" and the previous review explicitly requested a visible Log4j2 config writing into the logs/ directory (e.g. logs/app.log). Please add a configuration file (log4j2.xml or properties) with a Console appender and a File appender pointing to logs/app.log so logging actually writes to console and the logs/ directory (which is already ignored by .gitignore).

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

Critical: there is no Log4j2 configuration file in src/main/resources. Add a log4j2.xml that defines a Console appender and a File appender whose fileName is exactly "logs/app.log", and make the root logger reference both appenders so logs go to console and to logs/app.log.

public Order completeOrder(Long userId) {
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 14, 2026

Choose a reason for hiding this comment

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

A TODO comment was left in the source (please remove all TODO comments before submitting). See checklist: "Let's remove all TODO comments before submitting solution." Also the log message here is too generic; make it informative (e.g., include the login parameter but never the password).

// TODO: add log message about method completeOrder was called
logger.info("Completing order for userId={}", 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 @@ -24,7 +28,8 @@ private List<Product> getAllProductsFromShoppingCart(Long userId) {
Product macBook = new Product("MacBook Air 2020", BigDecimal.valueOf(1399));
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
logger.info("Fetched {} products from shopping cart for userId={}",
products.size(), userId);
return products;
}
}
24 changes: 24 additions & 0 deletions src/main/resources/log4j2.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<Configuration status="info">
<Appenders>
<Console name="LogToConsole" target="SYSTEM_OUT">
<PatternLayout pattern="%d{HH:mm:ss.SSS} %-5level %c:%L - %msg%n"/>
</Console>

<File name="LogToFile" fileName="logs/app.log">
<PatternLayout>
<Pattern>%d %p %c:%L %m%n</Pattern>
</PatternLayout>
</File>
</Appenders>
<Loggers>
<Logger name="mate.academy" level="info" additivity="false">
<AppenderRef ref="LogToFile"/>
<AppenderRef ref="LogToConsole"/>
</Logger>
<Root level="error">
<AppenderRef ref="LogToFile"/>
<AppenderRef ref="LogToConsole"/>
</Root>
</Loggers>
</Configuration>
Loading