-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat : use vlads BaseJpaRepository and polish #1592
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.example.custom.sequence.config; | ||
|
||
import io.hypersistence.utils.spring.repository.BaseJpaRepositoryImpl; | ||
import org.springframework.context.annotation.Configuration; | ||
import org.springframework.data.jpa.repository.config.EnableJpaRepositories; | ||
|
||
@Configuration | ||
@EnableJpaRepositories( | ||
repositoryBaseClass = BaseJpaRepositoryImpl.class, | ||
basePackages = "com.example.custom.sequence.repositories") | ||
public class JpaConfig {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,80 @@ | ||
package com.example.custom.sequence.mapper; | ||
|
||
import com.example.custom.sequence.entities.Customer; | ||
import com.example.custom.sequence.entities.Order; | ||
import com.example.custom.sequence.model.request.CustomerRequest; | ||
import com.example.custom.sequence.model.response.CustomerResponse; | ||
import com.example.custom.sequence.repositories.OrderRepository; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import org.springframework.stereotype.Service; | ||
|
||
@Service | ||
public class CustomerMapper { | ||
|
||
private final OrderMapper orderMapper; | ||
private final OrderRepository orderRepository; | ||
|
||
public CustomerMapper(OrderMapper orderMapper, OrderRepository orderRepository) { | ||
this.orderMapper = orderMapper; | ||
this.orderRepository = orderRepository; | ||
} | ||
|
||
public CustomerResponse mapToResponse(Customer saved) { | ||
return new CustomerResponse(saved.getId(), saved.getText()); | ||
return new CustomerResponse( | ||
saved.getId(), | ||
saved.getText(), | ||
orderMapper.mapToResponseListWithOutCustomer(saved.getOrders())); | ||
} | ||
|
||
public Customer mapToEntity(CustomerRequest customerRequest) { | ||
Customer customer = new Customer(customerRequest.text()); | ||
customerRequest | ||
.orders() | ||
.forEach(orderRequest -> customer.addOrder(orderMapper.mapToEntity(orderRequest))); | ||
return customer; | ||
} | ||
|
||
public void updateCustomerFromRequest(CustomerRequest customerRequest, Customer foundCustomer) { | ||
foundCustomer.setText(customerRequest.text()); | ||
List<Order> removedOrders = new ArrayList<>(foundCustomer.getOrders()); | ||
List<Order> ordersFromRequest = | ||
customerRequest.orders().stream() | ||
.map( | ||
orderRequest -> | ||
orderMapper.mapToEntityWithCustomer( | ||
orderRequest, foundCustomer)) | ||
.collect(Collectors.toList()); | ||
removedOrders.removeAll(ordersFromRequest); | ||
|
||
for (Order removedOrder : removedOrders) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coderabbitai, please write integrationTests in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rajadilipkolli Below is an example of how you might write three integration tests in CustomerControllerIT to cover typical scenarios—creation, update, and validation error. Adjust field names and URLs as needed to match your actual setup: Javaimport static org.assertj.core.api.Assertions.assertThat; import com.example.custom.sequence.model.request.CustomerRequest; @AutoConfigureMockMvc
} ✏️ Learnings added
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coderabbitai, In above examples include various orderRequests like (newer one, updated one, delete ones )as of now all are empty There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rajadilipkolli Below is an illustrative example in CustomerControllerIT showing how you might handle various order requests (new, updated, and deleted) within a single update scenario. Feel free to adapt field names, request body details, and endpoints as needed to align with your code: Javaimport static org.assertj.core.api.Assertions.assertThat; import com.example.custom.sequence.model.request.CustomerRequest; @AutoConfigureMockMvc
} ✏️ Learnings added
|
||
foundCustomer.removeOrder(removedOrder); | ||
} | ||
|
||
List<Order> newOrders = new ArrayList<>(ordersFromRequest); | ||
newOrders.removeAll(foundCustomer.getOrders()); | ||
|
||
ordersFromRequest.removeAll(newOrders); | ||
|
||
for (Order existingOrder : ordersFromRequest) { | ||
existingOrder.setCustomer(foundCustomer); | ||
// manually set the id of the existing order to avoid creating a new order instead of | ||
// updating the existing one | ||
for (Order foundOrder : foundCustomer.getOrders()) { | ||
if (foundOrder.getText().equals(existingOrder.getText())) { | ||
existingOrder.setId(foundOrder.getId()); | ||
break; | ||
} | ||
} | ||
Order mergedOrder = orderRepository.merge(existingOrder); | ||
foundCustomer | ||
.getOrders() | ||
.set(foundCustomer.getOrders().indexOf(mergedOrder), mergedOrder); | ||
} | ||
|
||
for (Order newOrder : newOrders) { | ||
foundCustomer.addOrder(newOrder); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,8 +2,11 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.example.custom.sequence.entities.Customer; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.example.custom.sequence.entities.Order; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.example.custom.sequence.model.response.CustomerResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.example.custom.sequence.model.request.OrderRequest; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.example.custom.sequence.model.response.CustomerResponseWithOutOrder; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.example.custom.sequence.model.response.OrderResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.example.custom.sequence.model.response.OrderResponseWithOutCustomer; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.springframework.stereotype.Service; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@Service | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -14,6 +17,28 @@ public OrderResponse getOrderResponse(Order order) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return new OrderResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
order.getId(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
order.getText(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
new CustomerResponse(customer.getId(), customer.getText())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
new CustomerResponseWithOutOrder(customer.getId(), customer.getText())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public Order mapToEntity(OrderRequest orderRequest) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Order order = new Order(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
order.setText(orderRequest.text()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return order; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public List<OrderResponse> mapToResponseList(List<Order> orders) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return orders.stream().map(this::getOrderResponse).toList(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public List<OrderResponseWithOutCustomer> mapToResponseListWithOutCustomer(List<Order> orders) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return orders.stream() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map(order -> new OrderResponseWithOutCustomer(order.getId(), order.getText())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.toList(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public Order mapToEntityWithCustomer(OrderRequest orderRequest, Customer foundCustomer) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Order order = mapToEntity(orderRequest); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
order.setCustomer(foundCustomer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return order; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+23
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation for robustness The mapping methods are well-structured and follow good practices, but could benefit from additional validation: Consider adding these safety checks: public Order mapToEntity(OrderRequest orderRequest) {
+ if (orderRequest == null) {
+ throw new IllegalArgumentException("OrderRequest cannot be null");
+ }
+ if (orderRequest.text() == null || orderRequest.text().trim().isEmpty()) {
+ throw new IllegalArgumentException("Order text cannot be null or empty");
+ }
Order order = new Order();
order.setText(orderRequest.text());
return order;
}
public List<OrderResponse> mapToResponseList(List<Order> orders) {
+ if (orders == null) {
+ throw new IllegalArgumentException("Orders list cannot be null");
+ }
return orders.stream().map(this::getOrderResponse).toList();
}
public Order mapToEntityWithCustomer(OrderRequest orderRequest, Customer foundCustomer) {
+ if (foundCustomer == null) {
+ throw new IllegalArgumentException("Customer cannot be null");
+ }
Order order = mapToEntity(orderRequest);
order.setCustomer(foundCustomer);
return order;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package com.example.custom.sequence.model.request; | ||
|
||
import jakarta.validation.constraints.NotBlank; | ||
import java.util.List; | ||
|
||
public record CustomerRequest( | ||
@NotBlank(message = "Text cannot be empty") String text, List<OrderRequest> orders) {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.example.custom.sequence.model.request; | ||
|
||
import jakarta.validation.constraints.NotBlank; | ||
import jakarta.validation.constraints.NotEmpty; | ||
|
||
public record OrderRequest( | ||
@NotEmpty(message = "Text cannot be empty") String text, | ||
@NotBlank(message = "CustomerId cannot be blank") String customerId) {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,17 @@ | ||
package com.example.custom.sequence.model.response; | ||
|
||
public record CustomerResponse(String id, String text) {} | ||
import java.util.List; | ||
|
||
/** | ||
* Response DTO representing customer information with their associated orders. | ||
* | ||
* @param id Customer's unique identifier | ||
* @param text Customer's descriptive text | ||
* @param orderResponses List of associated orders, never null but may be empty | ||
*/ | ||
public record CustomerResponse( | ||
String id, String text, List<OrderResponseWithOutCustomer> orderResponses) { | ||
public CustomerResponse { | ||
orderResponses = orderResponses == null ? List.of() : orderResponses; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
package com.example.custom.sequence.model.response; | ||
|
||
public record CustomerResponseWithOutOrder(String id, String text) {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
package com.example.custom.sequence.model.response; | ||
|
||
public record OrderResponse(String id, String text, CustomerResponse customer) {} | ||
public record OrderResponse(String id, String text, CustomerResponseWithOutOrder customer) {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.example.custom.sequence.model.response; | ||
|
||
/** | ||
* Response model representing an order without its associated customer details. Used in scenarios | ||
* where customer information is not needed in the response. | ||
* | ||
* @param id The unique identifier of the order | ||
* @param orderDescription The description or details of the order | ||
*/ | ||
public record OrderResponseWithOutCustomer(String id, String orderDescription) {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,21 @@ | ||
package com.example.custom.sequence.repositories; | ||
|
||
import com.example.custom.sequence.entities.Order; | ||
import io.hypersistence.utils.spring.repository.BaseJpaRepository; | ||
import java.util.Optional; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
import org.springframework.data.jpa.repository.EntityGraph; | ||
import org.springframework.data.jpa.repository.Query; | ||
import org.springframework.data.repository.ListPagingAndSortingRepository; | ||
import org.springframework.data.repository.query.Param; | ||
|
||
public interface OrderRepository extends JpaRepository<Order, String> { | ||
public interface OrderRepository | ||
extends BaseJpaRepository<Order, String>, ListPagingAndSortingRepository<Order, String> { | ||
|
||
@Query("select o from Order o join fetch o.customer where o.id = :id") | ||
Optional<Order> findById(@Param("id") String id); | ||
|
||
void deleteAllInBatch(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Exercise caution with deleteAllInBatch operation - potential data integrity risk The
Instead, use the standard 🔗 Analysis chainExercise caution with deleteAllInBatch operation The
Let me gather more information about potential foreign key relationships by checking the Order entity and related entities. Let me check one more specific detail about the Order entity and its relationships. Let me check one final detail about the Customer entity to understand the complete relationship structure. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for foreign key relationships to Order entity
ast-grep --pattern 'class $_ {
$$$
@ManyToOne
$$$
Order $_;
$$$
}'
# Check for usage of deleteAllInBatch to ensure it's used in appropriate contexts
rg "deleteAllInBatch" --type java
Length of output: 3869 Script: #!/bin/bash
# Check Order entity definition
rg -A 10 "class Order" --type java
# Check for @OneToMany relationships that might reference Order
ast-grep --pattern 'class $_ {
$$$
@OneToMany
$$$
$_ Order
$$$
}'
# Check for any JPA cascade configurations related to Order
rg "@OneToMany.*Order|@ManyToOne.*Order" --type java
Length of output: 34985 Script: #!/bin/bash
# Check the Order entity in the custom sequence project for its relationships
rg -A 15 "class Order" jpa/boot-data-customsequence/src/main/java/com/example/custom/sequence/entities/Order.java
# Check if there are any entities with relationships to Order
rg "@ManyToOne.*Order|@OneToMany.*Order" jpa/boot-data-customsequence/src/main/java/com/example/custom/sequence/entities/
Length of output: 643 Script: #!/bin/bash
# Check the Customer entity for any relationships with Order
rg -A 15 "class Customer" jpa/boot-data-customsequence/src/main/java/com/example/custom/sequence/entities/Customer.java
# Check for any cascade configurations in the Customer-Order relationship
rg "cascade.*ALL" jpa/boot-data-customsequence/src/main/java/com/example/custom/sequence/entities/
Length of output: 877 |
||
|
||
@EntityGraph(attributePaths = "customer") | ||
Optional<Order> findByIdAndCustomer_Id(String id, String customerId); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent potential NullPointerException in equals method
The current implementation could throw NullPointerException if the customer field is null. Additionally, comparing by text and customer ID instead of entity ID is unusual for JPA entities and could lead to unexpected behavior.
Consider this safer implementation:
If you really need to compare by text and customer, add null checks: