Skip to content
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.

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

first_try #1069

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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/*
src/main/resources/log4j2.xml

Choose a reason for hiding this comment

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

The line src/main/resources/log4j2.xml is set to be ignored. If this file contains sensitive information or is environment-specific, it's appropriate to ignore it. However, if it's a crucial part of the application's configuration that should be versioned, consider removing this line.

Choose a reason for hiding this comment

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

The log4j2.xml file is ignored. Make sure this is intentional, as this file is often crucial for configuring logging in different environments. If this file is needed for deployment or testing, consider removing it from .gitignore.

9 changes: 9 additions & 0 deletions logs/app.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
pattern 2025-01-06 00:34:46,015 INFO mate.academy.service.AuthenticationServiceImpl Method login was called : bob
pattern 2025-01-06 00:34:46,023 INFO mate.academy.service.OrderServiceImpl Method completeOrder was called : 2
pattern 2025-01-06 00:34:46,026 INFO mate.academy.service.OrderServiceImpl Data was successfully fetched from DB
pattern 2025-01-06 00:41:54,983 INFO mate.academy.service.AuthenticationServiceImpl Method login was called : bob
pattern 2025-01-06 00:41:54,992 INFO mate.academy.service.OrderServiceImpl Method completeOrder was called : 2
pattern 2025-01-06 00:41:54,995 INFO mate.academy.service.OrderServiceImpl Data was successfully fetched from DB
pattern 2025-01-06 00:50:08,730 INFO mate.academy.service.AuthenticationServiceImpl Method login was called : bob
pattern 2025-01-06 00:50:08,752 INFO mate.academy.service.OrderServiceImpl Method completeOrder was called : 2
pattern 2025-01-06 00:50:08,756 INFO mate.academy.service.OrderServiceImpl Data was successfully fetched from DB
15 changes: 12 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>mate-academy</groupId>
<artifactId>jv-logger</artifactId>
<version>1.0-SNAPSHOT</version>

<properties>
<jdk.version>17</jdk.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
Expand All @@ -18,13 +16,25 @@
</properties>

<dependencies>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
<version>2.20.0</version>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<version>2.20.0</version>
</dependency>

<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.12</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
Expand Down Expand Up @@ -63,4 +73,3 @@
</pluginManagement>
</build>
</project>

6 changes: 5 additions & 1 deletion src/main/java/mate/academy/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@
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.core.Logger;

Choose a reason for hiding this comment

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

Consider using org.apache.logging.log4j.Logger instead of org.apache.logging.log4j.core.Logger. The core package is typically used for internal Log4j components, and using the standard Logger interface is recommended for application logging.

Choose a reason for hiding this comment

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

It is recommended to use org.apache.logging.log4j.Logger instead of org.apache.logging.log4j.core.Logger. The core package is intended for internal use and may not be stable for public use.


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

Choose a reason for hiding this comment

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

Casting LogManager.getLogger(Main.class) to Logger is unnecessary if you use org.apache.logging.log4j.Logger. Consider changing the logger declaration to private static final Logger logger = LogManager.getLogger(Main.class);.


public static void main(String[] args) {
AuthenticationService authenticationService = new AuthenticationServiceImpl();
User user;
try {
user = authenticationService.login("bob", "1234");
} catch (AuthenticationException e) {
e.printStackTrace();
logger.error("Failed login attempt for user: bob");

Choose a reason for hiding this comment

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

The log message could be more informative by including details such as the reason for the failure or additional context. Consider adding more information to help with debugging and monitoring.

return;
}
OrderService orderService = new OrderServiceImpl();
Expand Down
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 {
private static final Logger logger =
LogManager.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 : {}", login);

Choose a reason for hiding this comment

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

The log message 'Method login was called : {}' is not very informative. According to the checklist, log messages should be more descriptive. Consider changing it to something like 'Method login was called. Params: login={}'.

Choose a reason for hiding this comment

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

The log message 'Method login was called' is informative, but consider adding more context, such as the timestamp or additional user information, to make it more useful for debugging.

User user = findByLogin(login);
if (!user.getPassword().equals(password)) {
throw new AuthenticationException("Username or password are incorrect");
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/mate/academy/service/OrderServiceImpl.java
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;

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

@Override
public Order completeOrder(Long userId) {
// TODO: add log message about method completeOrder was called
logger.info("Method completeOrder was called : {}", userId);

Choose a reason for hiding this comment

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

The log message 'Method completeOrder was called : {}' is not very informative. According to the checklist, log messages should be more descriptive. Consider changing it to something like 'Method completeOrder was called. Params: userId={}'.

Choose a reason for hiding this comment

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

The log message 'Method completeOrder was called' is informative, but consider adding more context, such as the timestamp or additional user information, to make it more useful for debugging.

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,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("Data was successfully fetched from DB");

Choose a reason for hiding this comment

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

The log message 'Data was successfully fetched from DB' is informative, but since this is a dummy implementation, consider clarifying in the log that this is simulated data for testing purposes.

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">
<Appenders>
<Console name="Console" target="SYSTEM_OUT">
<PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} - %msg%n"/>
</Console>
<File name="File" fileName="logs/app.log">
<PatternLayout>
<Pattern>pattern %d %p %c %m%n</Pattern>
</PatternLayout>
</File>
</Appenders>
<Loggers>
<Logger name="mate.academy" level="info" additivity="false">
<AppenderRef ref="File"/>
<AppenderRef ref="Console"/>
</Logger>
<Root level="error">
<AppenderRef ref="Console"/>
<AppenderRef ref="File"/>
</Root>
</Loggers>
</Configuration>
Loading