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 1 commit
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("run main");
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.

Log message is too generic. The checklist requires: "Make log messages informative. [CHECKLIST ITEM #3]" Consider adding context (e.g. inputs or status) instead of "run main" — for example include args length or environment info.

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 method contains a remaining TODO that must be removed/completed: "Remove all TODO comments before submitting solution. [CHECKLIST ITEM #1]". Also logger.info("method completeOrder was called") is not informative per: "Make log messages informative. [CHECKLIST ITEM #3]" — include userId or other context in the log.

AuthenticationService authenticationService = new AuthenticationServiceImpl();
User user;
try {
user = authenticationService.login("bob", "1234");
} catch (AuthenticationException e) {
e.printStackTrace();
logger.error("Can't login", e);
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 error log could be more informative per the checklist: "Make log messages informative. [CHECKLIST ITEM #3]" Consider including which user/login failed (without logging the password) to help debugging: e.g. logger.error("Can't login for user={}", login, e);.

return;
}
OrderService orderService = new OrderServiceImpl();
Expand Down
5 changes: 5 additions & 0 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,11 +2,16 @@

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 method was called");
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 remains in this class (see the preceding TODO line). This violates the checklist item: "Let's remove all TODO comments before submitting solution." Remove leftover TODO comments before submitting. Also the current log message is minimal — it doesn't include context/parameters. This violates the checklist item: "Let's make our log messages informative"; consider logging the login parameter, e.g. logger.info("login method was called. Params: login={}", login);

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");
Expand Down
6 changes: 6 additions & 0 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,16 @@
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("method completeOrder was called");
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 this method (remove all TODO comments as required: "Let's remove all TODO comments before submitting solution."). The log call is not informative — include the userId when logging that completeOrder was called (example: "completeOrder called. 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.

This startup log is minimal. Per checklist "Let's make our log messages informative" consider adding context (for example, include application args or environment info) while avoiding sensitive data.

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 +30,7 @@ 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
logger.info("successful fetched data from DB");
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 here as well (remove all TODO comments: "Let's remove all TODO comments before submitting solution."). The log message should be more informative — include context such as userId and number of fetched products (e.g., "Fetched {} products for userId={}"). Also follow the checklist guidance: "Let's make our log messages informative."

return products;
}
}
23 changes: 23 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,23 @@
<?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