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

Polish #1598

Merged
merged 8 commits into from
Dec 25, 2024
Merged

Polish #1598

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2137e67
polish
rajadilipkolli Dec 25, 2024
c996d81
feat : adds new testcases
rajadilipkolli Dec 25, 2024
90c8676
fix input type
rajadilipkolli Dec 25, 2024
8beedfa
fix wildcard import
rajadilipkolli Dec 25, 2024
d808beb
polish db operations
rajadilipkolli Dec 25, 2024
41615f6
Merge branch 'main' into polish
rajadilipkolli Dec 25, 2024
67f8292
fix : spotless
rajadilipkolli Dec 25, 2024
5705ca8
concise code, consider leveraging streams directly
rajadilipkolli Dec 25, 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
3 changes: 3 additions & 0 deletions ...ltimate-redis/src/test/java/com/example/ultimateredis/controller/RedisControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ void addRedisKeyValue() throws Exception {
.content(objectMapper.writeValueAsString(addRedisRequest))
.assertThat()
.hasStatus(HttpStatus.CREATED)
.hasContentType(MediaType.APPLICATION_JSON)
.bodyJson()
.convertTo(GenericResponse.class)
.satisfies(response -> assertThat(response.response()).isEqualTo(true));
Expand All @@ -42,6 +43,7 @@ void getFromCache() {
.param("key", "junit")
.assertThat()
.hasStatusOk()
.hasContentType(MediaType.APPLICATION_JSON)
.bodyJson()
.convertTo(GenericResponse.class)
.satisfies(response -> assertThat(response.response()).isEqualTo("JunitValue"));
Expand All @@ -61,6 +63,7 @@ void expireFromCache() {
.param("key", "junit")
.assertThat()
.hasStatusOk()
.hasContentType(MediaType.APPLICATION_JSON)
.bodyJson()
.convertTo(GenericResponse.class)
.satisfies(
Expand Down
6 changes: 3 additions & 3 deletions jpa/boot-read-replica-postgresql/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
<relativePath /> <!-- lookup parent from repository -->
</parent>
<groupId>com.example.demo</groupId>
<artifactId>boot-jpa-read-replica-postgresql</artifactId>
<artifactId>boot-read-replica-postgresql</artifactId>
<version>0.0.1-SNAPSHOT</version>
<name>boot-jpa-read-replica-postgresql</name>
<name>boot-read-replica-postgresql</name>
<description>Demo project for Spring Boot Read Replica</description>

<properties>
Expand Down Expand Up @@ -109,7 +109,7 @@
<configuration>
<java>
<googleJavaFormat>
<version>1.24.0</version>
<version>1.25.2</version>
<style>AOSP</style>
</googleJavaFormat>
</java>
Expand Down
22 changes: 20 additions & 2 deletions ...a-postgresql/src/main/java/com/example/demo/readreplica/controller/ArticleController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
import com.example.demo.readreplica.service.ArticleService;
import java.net.URI;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.servlet.support.ServletUriComponentsBuilder;

@RestController
@RequestMapping("/articles")
Expand All @@ -22,7 +24,7 @@ class ArticleController {
}

@GetMapping("/{id}")
ResponseEntity<ArticleDTO> findArticleById(@PathVariable Integer id) {
ResponseEntity<ArticleDTO> findArticleById(@PathVariable Long id) {
return this.articleService
.findArticleById(id)
.map(ResponseEntity::ok)
Expand All @@ -32,6 +34,22 @@ ResponseEntity<ArticleDTO> findArticleById(@PathVariable Integer id) {
@PostMapping("/")
ResponseEntity<Object> saveArticle(@RequestBody ArticleDTO articleDTO) {
Long articleId = this.articleService.saveArticle(articleDTO);
return ResponseEntity.created(URI.create("/articles/" + articleId)).build();
URI location =
ServletUriComponentsBuilder.fromCurrentRequest()
.path("{id}")
.buildAndExpand(articleId)
.toUri();
return ResponseEntity.created(location).build();
}

@DeleteMapping("/{id}")
ResponseEntity<Void> deleteArticle(@PathVariable Long id) {
boolean exists = this.articleService.existsById(id);
if (exists) {
this.articleService.deleteById(id);
return ResponseEntity.accepted().build();
} else {
return ResponseEntity.notFound().build();
}
}
}
18 changes: 17 additions & 1 deletion ...read-replica-postgresql/src/main/java/com/example/demo/readreplica/domain/ArticleDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,26 @@
package com.example.demo.readreplica.domain;

import static com.example.demo.readreplica.domain.CommentDTO.convertToComment;

import com.example.demo.readreplica.entities.Article;
import com.example.demo.readreplica.entities.Comment;
import java.time.LocalDateTime;
import java.util.List;

public record ArticleDTO(
String title,
LocalDateTime authored,
LocalDateTime published,
List<CommentDTO> commentDTOs) {}
List<CommentDTO> commentDTOs) {

public Article convertToArticle() {
Article article =
new Article().setAuthored(authored).setTitle(title).setPublished(published);
commentDTOs.forEach(
commentDTO -> {
Comment comment = convertToComment(commentDTO);
article.addComment(comment);
});
return article;
}
}
9 changes: 8 additions & 1 deletion ...read-replica-postgresql/src/main/java/com/example/demo/readreplica/domain/CommentDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
package com.example.demo.readreplica.domain;

public record CommentDTO(String comment) {}
import com.example.demo.readreplica.entities.Comment;

public record CommentDTO(String comment) {

static Comment convertToComment(CommentDTO commentDTO) {
return new Comment().setComment(commentDTO.comment());
}
}
4 changes: 1 addition & 3 deletions ...a-postgresql/src/main/java/com/example/demo/readreplica/repository/ArticleRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.transaction.annotation.Transactional;

public interface ArticleRepository extends JpaRepository<Article, Long> {

@Transactional(readOnly = true)
@Query("select a from Article a left join fetch a.comments where a.id = :articleId ")
Optional<Article> findByArticleId(@Param("articleId") Integer articleId);
Optional<Article> findByArticleId(@Param("articleId") Long articleId);
}
36 changes: 12 additions & 24 deletions ...replica-postgresql/src/main/java/com/example/demo/readreplica/service/ArticleService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
import com.example.demo.readreplica.domain.ArticleDTO;
import com.example.demo.readreplica.domain.CommentDTO;
import com.example.demo.readreplica.entities.Article;
import com.example.demo.readreplica.entities.Comment;
import com.example.demo.readreplica.repository.ArticleRepository;
import java.util.List;
import java.util.Optional;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional(readOnly = true)
public class ArticleService {

private final ArticleRepository articleRepository;
Expand All @@ -19,17 +18,26 @@ public class ArticleService {
this.articleRepository = articleRepository;
}

public Optional<ArticleDTO> findArticleById(Integer id) {
public Optional<ArticleDTO> findArticleById(Long id) {
return this.articleRepository.findByArticleId(id).map(this::convertToArticleDTO);
}

public boolean existsById(Long id) {
return articleRepository.existsById(id);
}

@Transactional
public Long saveArticle(ArticleDTO articleDTO) {
Article article = convertToArticle(articleDTO);
Article article = articleDTO.convertToArticle();
Article savedArticle = this.articleRepository.save(article);
return savedArticle.getId();
}

@Transactional
public void deleteById(Long id) {
articleRepository.deleteById(id);
}

private ArticleDTO convertToArticleDTO(Article articleEntity) {
return new ArticleDTO(
articleEntity.getTitle(),
Expand All @@ -39,24 +47,4 @@ private ArticleDTO convertToArticleDTO(Article articleEntity) {
.map(comment -> new CommentDTO(comment.getComment()))
.toList());
}

private Article convertToArticle(ArticleDTO articleDTO) {
Article article = new Article();
article.setAuthored(articleDTO.authored());
article.setTitle(articleDTO.title());
article.setPublished(articleDTO.published());
convertToComment(articleDTO.commentDTOs()).forEach(article::addComment);
return article;
}

private List<Comment> convertToComment(List<CommentDTO> commentDTOs) {
return commentDTOs.stream()
.map(
commentDTO -> {
Comment comment = new Comment();
comment.setComment(commentDTO.comment());
return comment;
})
.toList();
}
}
3 changes: 3 additions & 0 deletions jpa/boot-read-replica-postgresql/src/main/resources/application.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ spring:
fail_on_pagination_over_collection_fetch: true
in_clause_parameter_padding: true
plan_cache_max_size: 4096
mvc:
problemdetails:
enabled: true
Comment on lines +48 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Problem Details enabled but error handling implementation is incomplete

The problem details configuration is enabled in application.yml, but the Article-related components lack proper error handling implementation:

  • No custom exceptions defined for Article-related error scenarios
  • No global exception handler found in the project
  • ArticleService returns Optional but there's no explicit error handling for not-found scenarios
  • No ResponseStatusException usage found in the controllers

To properly integrate with Problem Details:

  • Create a GlobalExceptionHandler class with @ControllerAdvice
  • Define custom exceptions for Article-related error scenarios (e.g., ArticleNotFoundException)
  • Handle Optional.empty() cases in the controller by throwing appropriate exceptions
  • Consider adding validation error handling
🔗 Analysis chain

Verify error handling integration with Article operations.

Let's verify that the problem details configuration is properly integrated with the Article-related error scenarios.

Let me gather more specific information about the error handling in this project.

Let me check for specific error handling implementations in the Article-related components.

Let me check one more aspect to verify the error handling integration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for proper error handling implementation in Article-related components

# Look for @ControllerAdvice or @RestControllerAdvice classes
echo "Checking for global exception handlers..."
rg -l "(@ControllerAdvice|@RestControllerAdvice)" --type java

# Check for custom exceptions in Article components
echo "Checking for custom exceptions..."
rg -l "Exception" --type java src/main/java/com/example/demo/readreplica/

# Look for @ExceptionHandler methods
echo "Checking for exception handlers..."
ast-grep --pattern 'class $_ {
  $$$
  @ExceptionHandler
  $$$
}'

Length of output: 3555

Script:

#!/bin/bash
# Check for exception handling in the current project
echo "Checking for exception handling in the current project..."
fd . --type f --extension java --exec rg -l "(@RestController|@Controller|@ExceptionHandler|Exception)" {}

# Check for specific Article-related error handling
echo "Looking for Article-related components..."
fd . --type f --extension java --exec rg -l "Article" {}

# Check application.yml for error handling configurations
echo "Checking application.yml configurations..."
cat jpa/boot-read-replica-postgresql/src/main/resources/application.yml

Length of output: 35315

Script:

#!/bin/bash
# Check for error handling in Article-related components
echo "Checking ArticleController implementation..."
rg -A 10 "class ArticleController" jpa/boot-read-replica-postgresql/src/main/java/com/example/demo/readreplica/controller/ArticleController.java

echo "\nChecking for exception handling in Article components..."
rg "throw|catch|@ExceptionHandler" jpa/boot-read-replica-postgresql/src/main/java/com/example/demo/readreplica/

echo "\nChecking for custom exceptions..."
rg "Exception" jpa/boot-read-replica-postgresql/src/main/java/com/example/demo/readreplica/

echo "\nChecking for global exception handler..."
fd . --type f --extension java --exec rg -l "GlobalExceptionHandler" jpa/boot-read-replica-postgresql/src/main/java/

Length of output: 4263

Script:

#!/bin/bash
# Check for service layer error handling
echo "Checking ArticleService implementation..."
rg -A 10 "class ArticleService" jpa/boot-read-replica-postgresql/src/main/java/com/example/demo/readreplica/service/ArticleService.java

# Check for any custom exceptions in the project
echo "Looking for custom exceptions in the project..."
fd . --type f --extension java --exec rg -l "extends (RuntimeException|Exception)" jpa/boot-read-replica-postgresql/src/main/java/

# Check for any ResponseStatusException usage
echo "Checking for ResponseStatusException usage..."
rg "ResponseStatusException" jpa/boot-read-replica-postgresql/src/main/java/com/example/demo/readreplica/

Length of output: 2479

threads:
virtual:
enabled: true
Expand Down
119 changes: 119 additions & 0 deletions ...resql/src/test/java/com/example/demo/readreplica/controller/ArticleControllerIntTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package com.example.demo.readreplica.controller;

import static org.assertj.core.api.Assertions.assertThat;

import com.example.demo.readreplica.domain.ArticleDTO;
import com.example.demo.readreplica.domain.CommentDTO;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.assertj.MockMvcTester;

@SpringBootTest
@AutoConfigureMockMvc
class ArticleControllerIntTest {

@Autowired private MockMvcTester mvcTester;

@Autowired private ObjectMapper objectMapper;

@Test
void findArticleById() {

mvcTester
.get()
.uri("/articles/1")
.assertThat()
.hasStatusOk()
.hasContentType(MediaType.APPLICATION_JSON)
.bodyJson()
.convertTo(ArticleDTO.class)
.satisfies(
articleDTO -> {
assertThat(articleDTO.title())
.isNotNull()
.isEqualTo("Waiter! There is a bug in my JSoup!");
assertThat(articleDTO.authored())
.isNotNull()
.isInstanceOf(LocalDateTime.class);
assertThat(articleDTO.published())
.isNotNull()
.isInstanceOf(LocalDateTime.class);
assertThat(articleDTO.commentDTOs())
.isNotNull()
.hasSize(2)
.hasOnlyElementsOfType(CommentDTO.class);
});
}

@Test
void shouldReturn404WhenFetchingNonExistingArticle() {
mvcTester.get().uri("/articles/99999").assertThat().hasStatus(HttpStatus.NOT_FOUND);
}

@Test
void saveRetrieveAndDeleteArticle() throws JsonProcessingException {
ArticleDTO articleDTO =
new ArticleDTO(
"junitTitle",
LocalDateTime.now().minusDays(1),
LocalDateTime.now(),
List.of(new CommentDTO("junitComment")));
AtomicReference<String> location = new AtomicReference<>();
mvcTester
.post()
.uri("/articles/")
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(articleDTO))
.assertThat()
.hasStatus(HttpStatus.CREATED)
.matches(
result -> {
location.set(result.getResponse().getHeader("Location"));
assertThat(location.get()).isNotBlank().contains("/articles/");
});

mvcTester
.get()
.uri(location.get())
.assertThat()
.hasStatusOk()
.hasContentType(MediaType.APPLICATION_JSON)
.bodyJson()
.convertTo(ArticleDTO.class)
.satisfies(
response -> {
assertThat(response.title()).isNotNull().isEqualTo("junitTitle");
assertThat(response.authored())
.isNotNull()
.isInstanceOf(LocalDateTime.class);
assertThat(response.authored().toLocalDate())
.isEqualTo(LocalDate.now().minusDays(1));
assertThat(response.published())
.isNotNull()
.isInstanceOf(LocalDateTime.class);
assertThat(response.published().toLocalDate())
.isEqualTo(LocalDate.now());
assertThat(response.commentDTOs())
.isNotNull()
.hasSize(1)
.hasOnlyElementsOfType(CommentDTO.class);
});

mvcTester.delete().uri(location.get()).assertThat().hasStatus(HttpStatus.ACCEPTED);
}

@Test
void cantDeleteArticleWhenArticleNotFound() {
mvcTester.delete().uri("/articles/99999").assertThat().hasStatus(HttpStatus.NOT_FOUND);
}
}
Loading