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

feat : use SQLStatementValidator and ValidationGroups #1593

Merged
merged 6 commits into from
Dec 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.example.custom.sequence.config;
package com.example.custom.sequence.config.db;

import io.hypersistence.utils.spring.repository.BaseJpaRepositoryImpl;
import org.springframework.context.annotation.Configuration;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.example.custom.sequence.config;
package com.example.custom.sequence.config.db;

import io.hypersistence.utils.logging.InlineQueryLogEntryCreator;
import javax.sql.DataSource;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.example.custom.sequence.config;
package com.example.custom.sequence.config.db;

import java.io.Serializable;
import java.lang.reflect.Member;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.example.custom.sequence.config;
package com.example.custom.sequence.config.db;

import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.METHOD;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.example.custom.sequence.entities;

import com.example.custom.sequence.config.StringPrefixedSequence;
import com.example.custom.sequence.config.db.StringPrefixedSequence;
import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
Expand Down Expand Up @@ -52,7 +52,7 @@
}

@Override
public boolean equals(Object o) {

Check warning on line 55 in jpa/boot-data-customsequence/src/main/java/com/example/custom/sequence/entities/Customer.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

'equals()' method which does not check class of parameter

`equals()` should check the class of its parameter
if (this == o) return true;
if (o == null || Hibernate.getClass(this) != Hibernate.getClass(o)) return false;
Customer customer = (Customer) o;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.example.custom.sequence.entities;

import com.example.custom.sequence.config.StringPrefixedSequence;
import com.example.custom.sequence.config.db.StringPrefixedSequence;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
Expand Down Expand Up @@ -38,7 +38,7 @@
private Customer customer;

@Override
public boolean equals(Object o) {

Check warning on line 41 in jpa/boot-data-customsequence/src/main/java/com/example/custom/sequence/entities/Order.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

'equals()' method which does not check class of parameter

`equals()` should check the class of its parameter
if (this == o) return true;
if (o == null || Hibernate.getClass(this) != Hibernate.getClass(o)) return false;
Order order = (Order) o;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ public CustomerResponse mapToResponse(Customer saved) {

public Customer mapToEntity(CustomerRequest customerRequest) {
Customer customer = new Customer(customerRequest.text());
if (customerRequest.orders() == null) {
return customer;
}
customerRequest
.orders()
.forEach(orderRequest -> customer.addOrder(orderMapper.mapToEntity(orderRequest)));
Expand All @@ -38,6 +41,9 @@ public Customer mapToEntity(CustomerRequest customerRequest) {

public void updateCustomerFromRequest(CustomerRequest customerRequest, Customer foundCustomer) {
foundCustomer.setText(customerRequest.text());
if (customerRequest.orders() == null) {
return;
}
List<Order> removedOrders = new ArrayList<>(foundCustomer.getOrders());
List<Order> ordersFromRequest =
customerRequest.orders().stream()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.example.custom.sequence.model.request;

import jakarta.validation.Valid;
import jakarta.validation.constraints.NotBlank;
import java.util.List;

public record CustomerRequest(
@NotBlank(message = "Text cannot be empty") String text, List<OrderRequest> orders) {}
@NotBlank(message = "Text cannot be empty") String text,
@Valid List<OrderRequest> orders) {}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
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) {}
@NotBlank(message = "Text cannot be empty") String text,
@NotBlank(
message = "CustomerId cannot be blank",
groups = ValidationGroups.GroupCheck.class)
String customerId) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.example.custom.sequence.model.request;

public interface ValidationGroups {

interface SkipGroupCheck {}

interface GroupCheck {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@ public Optional<CustomerResponse> updateCustomerById(
}

@Transactional
public void deleteCustomerById(String id) {
customerRepository.deleteById(id);
public Optional<CustomerResponse> deleteCustomerById(String id) {
Optional<CustomerResponse> optionalCustomer = findCustomerById(id);
optionalCustomer.ifPresent(
customerResponse -> customerRepository.deleteById(customerResponse.id()));
return optionalCustomer;
}

public Optional<Customer> findById(String customerId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

import com.example.custom.sequence.entities.Customer;
import com.example.custom.sequence.model.request.CustomerRequest;
import com.example.custom.sequence.model.request.ValidationGroups;
import com.example.custom.sequence.model.response.CustomerResponse;
import com.example.custom.sequence.model.response.PagedResult;
import com.example.custom.sequence.services.CustomerService;
import com.example.custom.sequence.utils.AppConstants;
import com.example.custom.sequence.web.api.CustomerAPI;
import jakarta.validation.groups.Default;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -60,13 +62,16 @@ public ResponseEntity<CustomerResponse> getCustomerById(@PathVariable String id)
@ResponseStatus(HttpStatus.CREATED)
@Override
public CustomerResponse createCustomer(
@RequestBody @Validated CustomerRequest customerRequest) {
@RequestBody @Validated(value = {Default.class, ValidationGroups.SkipGroupCheck.class})
CustomerRequest customerRequest) {
return customerService.saveCustomer(customerRequest);
}

@PutMapping("/{id}")
public ResponseEntity<CustomerResponse> updateCustomer(
@PathVariable String id, @RequestBody CustomerRequest customerRequest) {
@PathVariable String id,
@RequestBody @Validated(value = {Default.class, ValidationGroups.GroupCheck.class})
CustomerRequest customerRequest) {
return customerService
.updateCustomerById(id, customerRequest)
.map(ResponseEntity::ok)
Expand All @@ -76,12 +81,8 @@ public ResponseEntity<CustomerResponse> updateCustomer(
@DeleteMapping("/{id}")
public ResponseEntity<CustomerResponse> deleteCustomer(@PathVariable String id) {
return customerService
.findCustomerById(id)
.map(
customer -> {
customerService.deleteCustomerById(id);
return ResponseEntity.ok(customer);
})
.deleteCustomerById(id)
.map(ResponseEntity::ok)
.orElseGet(() -> ResponseEntity.notFound().build());
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package com.example.custom.sequence.web.controllers;

import com.example.custom.sequence.model.request.OrderRequest;
import com.example.custom.sequence.model.request.ValidationGroups;
import com.example.custom.sequence.model.response.OrderResponse;
import com.example.custom.sequence.model.response.PagedResult;
import com.example.custom.sequence.services.OrderService;
import com.example.custom.sequence.utils.AppConstants;
import jakarta.validation.groups.Default;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -53,7 +55,8 @@ public ResponseEntity<OrderResponse> getOrderById(@PathVariable String id) {

@PostMapping
public ResponseEntity<OrderResponse> createOrder(
@RequestBody @Validated OrderRequest orderRequest) {
@RequestBody @Validated(value = {Default.class, ValidationGroups.GroupCheck.class})
OrderRequest orderRequest) {
return orderService
.saveOrder(orderRequest)
.map(order -> ResponseEntity.status(HttpStatus.CREATED).body(order))
Expand All @@ -62,7 +65,9 @@ public ResponseEntity<OrderResponse> createOrder(

@PutMapping("/{id}")
public ResponseEntity<OrderResponse> updateOrder(
@PathVariable String id, @RequestBody OrderRequest orderRequest) {
@PathVariable String id,
@RequestBody @Validated(value = {Default.class, ValidationGroups.GroupCheck.class})
OrderRequest orderRequest) {
return orderService
.updateOrderById(id, orderRequest)
.map(ResponseEntity::ok)
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
com.example.custom.sequence.config.LazyConnectionDataSourceProxyConfig
com.example.custom.sequence.config.db.LazyConnectionDataSourceProxyConfig
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import static org.assertj.core.api.Assertions.assertThat;

import com.example.custom.sequence.common.ContainersConfig;
import com.example.custom.sequence.config.JpaConfig;
import com.example.custom.sequence.config.db.JpaConfig;
import com.zaxxer.hikari.HikariDataSource;
import javax.sql.DataSource;
import org.junit.jupiter.api.Test;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.assertj.MockMvcTester;

/**
* Base class for integration tests providing test infrastructure including: - Configured DataSource
Expand All @@ -23,7 +23,7 @@
@AutoConfigureMockMvc
public abstract class AbstractIntegrationTest {

@Autowired protected MockMvc mockMvc;
@Autowired protected MockMvcTester mockMvcTester;

@Autowired protected ObjectMapper objectMapper;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import com.example.custom.sequence.entities.Customer;
import com.example.custom.sequence.mapper.CustomerMapper;
import com.example.custom.sequence.model.request.CustomerRequest;
import com.example.custom.sequence.model.response.CustomerResponse;
import com.example.custom.sequence.model.response.PagedResult;
import com.example.custom.sequence.model.request.OrderRequest;
import com.example.custom.sequence.model.response.*;
import com.example.custom.sequence.repositories.CustomerRepository;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -88,6 +88,8 @@ void saveCustomer() {
void deleteCustomerById() {
// given
willDoNothing().given(customerRepository).deleteById("CUS_1");
given(customerRepository.findById("CUS_1")).willReturn(Optional.of(getCustomer()));
given(customerMapper.mapToResponse(getCustomer())).willReturn(getCustomerResponse());
// when
customerService.deleteCustomerById("CUS_1");
// then
Expand All @@ -102,10 +104,13 @@ private Customer getCustomer() {
}

private CustomerRequest getCustomerRequest() {
return new CustomerRequest("junitTest", List.of());
return new CustomerRequest("junitTest", List.of(new OrderRequest("ORD_1", "junitTest")));
}

private CustomerResponse getCustomerResponse() {
return new CustomerResponse("CUS_1", "junitTest", List.of());
return new CustomerResponse(
"CUS_1",
"junitTest",
List.of(new OrderResponseWithOutCustomer("ORD_1", "junitTest")));
Comment on lines +110 to +117
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for order-related data

While the helper methods now include order data, the test methods saveCustomer and findCustomerById don't verify this data in their assertions. This could miss potential mapping issues with the order-related fields.

Example enhancement for saveCustomer test:

 // then
 assertThat(persistedCustomer).isNotNull();
 assertThat(persistedCustomer.id()).isEqualTo("CUS_1");
 assertThat(persistedCustomer.text()).isEqualTo("junitTest");
+assertThat(persistedCustomer.orders())
+    .hasSize(1)
+    .element(0)
+    .satisfies(order -> {
+        assertThat(order.id()).isEqualTo("ORD_1");
+        assertThat(order.text()).isEqualTo("junitTest");
+    });

Similar assertions should be added to the findCustomerById test.

Committable suggestion skipped: line range outside the PR's diff.

}
}
Loading
Loading