Skip to content

Complete task: add logger to main class #1240

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
neuklearna wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Complete task: add logger to main class #1240

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9731285
Complete task: add logger to main class
neuklearna Apr 11, 2026
909a8b8
Add informative log messages with context parametrs
neuklearna Apr 11, 2026
bdfdc1f
Fix log messages: add context parameters and correct log
neuklearna Apr 12, 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/
11 changes: 11 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,17 @@
<version>4.12</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<version>2.24.3</version>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
<version>2.24.3</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;
import org.apache.logging.log4j.Logger;

public class Main {

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

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 11, 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 any Log4j2 configuration file (e.g. log4j2.xml or log4j2.properties) in the project — without it the appenders (File and Console) are not configured. Add a configuration file that defines Console and File appenders and appropriate loggers/levels (and ensure the log file path is inside the ignored logs/ directory).

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

This violates checklist guidance: "Let's make our log messages informative" (checklist). logger.info("login"); is too terse and matches the bad example pattern. Consider including context such as the login parameter (but never log the password), e.g. logger.info("Login attempt. Params: login={}", login); and log success/failure with more details.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Project is missing a Log4j2 configuration under src/main/resources that defines appenders and a root logger. This violates the requirement: "There is no Log4j2 configuration file (e.g., log4j2.xml or log4j2.properties under src/main/resources)... Add such a config that defines at least: a Console appender, a File appender writing to the already-ignored logs/ directory, and a root logger referencing both appenders at an appropriate level." Add a log4j2.xml (or properties) with a Console appender, a File appender writing into logs/, and a root logger referencing both.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Missing Log4j2 configuration file. The task requires: "Configure the logger in this project. Add appenders for File and Console" and additionally (from the detailed requirements) you must "Add a Log4j2 configuration file (log4j2.xml or log4j2.properties) under src/main/resources that: - Defines at least a Console appender. - Defines a File appender writing into the logs/ directory. - Defines a root logger that references both the Console and File appenders at an appropriate level." There is no file under src/main/resources configuring appenders/root logger. Add the config file so logs go to console and to logs/ (the logs/ directory is already ignored in .gitignore).

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Add an INFO-level log on successful login in AuthenticationServiceImpl.login. The task guidance asked: "In AuthenticationServiceImpl’s login method: Add an informative DEBUG-level entry log such as logger.debug("Method login was called. Params: login={}", login); Add a clear INFO-level log message on successful login." You already have a DEBUG entry (good), but there is no INFO log emitted when login succeeds—please add an informative info-level message (e.g. logger.info("User logged in. login={}, userId={}", login, user.getUserId()); ) before returning the user so successful auth events are recorded.


public static void main(String[] args) {
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

This violates checklist guidance: "Let's make our log messages informative" (checklist). logger.info("Order is complete"); lacks context — include at least the userId and ideally the created orderId when available, e.g. logger.info("Order completed. userId={}. orderId={}", userId, order.getOrderId());.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

This log message lacks full context required by the checklist: "Let's make our log messages informative" and you should "include key context such as login, userId, orderId, and counts". You're logging the order completion before the orderId is set and without product count. Move or add a log after the order is created and include orderId and number of products (for example: logger.info("Order completed. userId={} orderId={} productsCount={}", userId, order.getOrderId(), products.size());).

AuthenticationService authenticationService = new AuthenticationServiceImpl();
User user;
try {
user = authenticationService.login("bob", "1234");
} catch (AuthenticationException e) {
e.printStackTrace();
logger.error(e.getMessage());
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

In the catch block you log only the exception message. The checklist requires you to pass the exception object so the stack trace is recorded: "Also, where you log exceptions (for example, in the catch block in Main), pass the exception object to the logger so the stack trace is recorded, e.g., logger.error("Failed to login. login={}", login, e);." Include the login context as well (capture the login in a variable before the try or log the literal) and pass e to the logger.

return;
}
OrderService orderService = new OrderServiceImpl();
Expand Down
7 changes: 6 additions & 1 deletion 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;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class AuthenticationServiceImpl implements AuthenticationService {

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 12, 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 a Log4j2 configuration file under src/main/resources (e.g. log4j2.xml or log4j2.properties). The task requires a configuration that defines at least a Console appender and a File appender that writes into the logs/ directory, and a root logger referencing both appenders at an appropriate level. Please add such a config file so logging actually routes to console and the logs/ file.

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

@Override
public User login(String login, String password) throws AuthenticationException {
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Project is missing a Log4j2 configuration file under src/main/resources that defines Console and File appenders (writing to the already-ignored logs/ directory) and a root logger referencing both appenders. This violates: "There is no Log4j2 configuration file (e.g., log4j2.xml or log4j2.properties under src/main/resources)... Add such a config that defines at least: a Console appender, a File appender writing to the already-ignored logs/ directory, and a root logger referencing both appenders at an appropriate level." Add e.g. src/main/resources/log4j2.xml with a Console appender, a File appender (file in logs/), and root logger referencing both.

//TODO: add corresponding log message about method login was called
logger.info("Login called={}",login);
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

The login log uses terse wording Login called={}. The checklist asks to make logs informative: "* Let's make our log messages informative". Prefer a clearer message and include parameters like in the good example (e.g. logger.debug("Login attempt. Params: login={}", login);) and consider using DEBUG for method entry and INFO for successful login.

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

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

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 make our log messages informative" — logger.info("Order is complete") lacks context. Include relevant info such as userId and orderId (when available), e.g. logger.info("Order completed: userId={} orderId={}", userId, orderId).

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

This log is emitted before the order id is assigned and doesn't include the order identifier or product count. The task requires informative logs with key context (orderId, counts). See: "Log messages should include key context (such as login, userId, orderId, and counts) to satisfy the requirement that logs be informative. [CHECKLIST ITEM #3]" Move or add the order-completed log after order.setOrderId(...) and include the orderId and product count (e.g. logger.info("Order completed. userId={} orderId={} productsCount={}", userId, order.getOrderId(), products.size());).

throw new AuthenticationException("Username or password are incorrect");
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,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 11, 2026

Choose a reason for hiding this comment

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

There is no Log4j2 configuration file under src/main/resources. The task description requires: "Configure the logger in this project. Add appenders for File and Console." Add a log4j2.xml or log4j2.properties that defines at least a Console appender, a File appender (writing to logs/), and a root logger referencing both.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Missing Log4j2 configuration file under src/main/resources. The task requires adding a log4j2.xml or log4j2.properties that defines at least a Console appender, a File appender writing into the logs/ directory, and a root logger that references both appenders at an appropriate level. Without that config the File appender won't be created and file logging to logs/ will not work.

public class OrderServiceImpl implements OrderService {

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

@Override
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

The project uses Log4j loggers but I don't see a Log4j configuration that adds File and Console appenders. The task description explicitly requires: "Configure the logger in this project. Add appenders for File and Console". Add a log4j2 configuration (e.g. log4j2.xml or log4j2.properties under resources) registering a Console appender and a File appender (write files to the ignored logs/ folder).

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

This logs only a short message without context. This violates the checklist item: "Let's make our log messages informative". Include meaningful context (for example: that login() was called and which login value was provided — do not log passwords), e.g. logger.info("login called. Params: login={}", login);.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

The login entry log includes the login but consider using a more informative message and the appropriate level. Per checklist "Let's make our log messages informative", prefer e.g. logger.debug("Method login was called. Params: login={}", login); (DEBUG for method entry) or logger.info("User login attempt. login={}", login); if you want it at INFO.

public Order completeOrder(Long userId) {
// TODO: add log message about method completeOrder was called
logger.info("Order completed for userId={}",userId);
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

This logs the order as "completed" before the orderId is set and without product count. To be informative (checklist: "Let's make our log messages informative"), either move/add a log after order.setOrderId(...) that includes orderId and number of products, e.g.: logger.info("Order completed. userId={} orderId={} productsCount={}", userId, order.getOrderId(), products.size());

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 +29,7 @@ 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("All products in shopping cart for userId={}",userId);
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Current message is generic. Per checklist "Let's make our log messages informative" include product count and/or product names, e.g.: logger.info("All products in shopping cart for userId={} productsCount={}", userId, products.size());.

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="WARN">

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

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

<Loggers>
<Root level="debug">
<AppenderRef ref="ConsoleAppender"/>
<AppenderRef ref="FileAppender"/>
</Root>
</Loggers>

</Configuration>
Loading