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.

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

resolve #1060

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

resolve #1060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5623697
resolve
DankevichMisha Nov 30, 2024
7e864bd
resolve
DankevichMisha Nov 30, 2024
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
DankevichMisha marked this conversation as resolved.
Show resolved Hide resolved
Copy link

Choose a reason for hiding this comment

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

The syntax for ignoring the logs directory is incorrect. It should be logs/ instead of /logs. This ensures that log files are not pushed to GitHub.

15 changes: 14 additions & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@
</properties>

<dependencies>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<version>2.23.1</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.12</version>
<version>4.13.1</version>
<scope>test</scope>
</dependency>
</dependencies>
Expand All @@ -46,6 +51,14 @@
<linkXRef>false</linkXRef>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>8</source>
<target>8</target>
</configuration>
</plugin>
</plugins>
<pluginManagement>
<plugins>
Expand Down
6 changes: 5 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,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.Logger;

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

Choose a reason for hiding this comment

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

The logger configuration is missing appenders for both File and Console. According to the task description, you need to configure the logger with these appenders .


public static void main(String[] args) {
AuthenticationService authenticationService = new AuthenticationServiceImpl();
User user;
DankevichMisha marked this conversation as resolved.
Show resolved Hide resolved
try {
user = authenticationService.login("bob", "1234");
Copy link

Choose a reason for hiding this comment

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

The log message in the login method should be more informative. Consider including parameters or context to make the logs more useful, as suggested in the checklist .

} catch (AuthenticationException e) {
e.printStackTrace();
logger.error("Error during login", e);
return;
}
OrderService orderService = new OrderServiceImpl();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/mate/academy/model/Order.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ public class Order {
private List<Product> products;
private Long userId; // the identifier of user who complete the order

public Order() {
public Order(Long orderId) {
DankevichMisha marked this conversation as resolved.
Show resolved Hide resolved
Copy link

Choose a reason for hiding this comment

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

The orderId field is not initialized in this constructor. You should set this.orderId = orderId; to properly initialize it.

}

public Order(List<Product> products, Long userId) {
Expand Down
6 changes: 5 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,15 @@

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("Login method was called");
Copy link

Choose a reason for hiding this comment

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

The log message 'Login method was called' is not very informative. Consider including the login parameter or other context to make the log more useful, as suggested in the checklist.

User user = findByLogin(login);
DankevichMisha marked this conversation as resolved.
Show resolved Hide resolved
if (!user.getPassword().equals(password)) {
DankevichMisha marked this conversation as resolved.
Show resolved Hide resolved
throw new AuthenticationException("Username or password are incorrect");
Expand Down
8 changes: 6 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;

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("CompleteOrder method was called");
Copy link

Choose a reason for hiding this comment

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

The log message 'CompleteOrder method was called' is not very informative. Consider including the userId parameter or other context to make the log more useful, as suggested in the checklist.

List<Product> products = getAllProductsFromShoppingCart(userId);
DankevichMisha marked this conversation as resolved.
Show resolved Hide resolved
Order order = new Order(products, userId);
DankevichMisha marked this conversation as resolved.
Show resolved Hide resolved
// 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("Successful fetched data from DB");
Copy link

Choose a reason for hiding this comment

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

The log message 'Successful fetched data from DB' could be more informative. Consider including details about the userId or the number of products fetched to provide more context.

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" ?>
<Configurations status="info">
<Appenders>
<Console name="LogToConsole" target="SYSTEM_OUT">
<PatternLayout pattern="%d{HH:mm:ss.SSS} %-5level %logger{36} - %msg%n"/>
</Console>
<File name="LogToFile" fileName="logs/app.log">
<PatternLayout>
<Pattern>%d %p %c %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>
</Configurations>