feat : expose endpoint to save post along with details, comments and tags #1647
Conversation
WalkthroughThis pull request introduces significant enhancements to a Spring Boot JPA project by integrating MapStruct for object mapping, adding new request and response models, and implementing a post creation workflow. Changes include modifications to the Maven configuration, updates to entity classes, the creation of mapper interfaces, and the establishment of a new REST API endpoint for post creation with validation and transactional support. Changes
Sequence DiagramsequenceDiagram
Client->>PostController: createPostByUserName(postRequest, userName)
PostController->>PostService: createPost(postRequest, userName)
PostService->>PostMapper: Convert request to entity
PostMapper->>PostMapperDecorator: Process tags and comments
PostMapperDecorator->>TagRepository: Check existing tags
PostService->>PostRepository: Save post entity
PostRepository-->>PostService: Saved post
PostController-->>Client: 201 Created with Location header
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
type: dependency-upgrade
| public abstract class PostMapperDecorator implements PostMapper { | ||
|
|
||
| @Autowired | ||
| private TagRepository tagRepository; |
There was a problem hiding this comment.
@coderabbitai, how to use constructor injection instead of field injection in the Decorator of PostMapper defined using MapStruct
There was a problem hiding this comment.
@rajadilipkolli Here's how to implement constructor injection in your MapStruct decorator:
public abstract class PostMapperDecorator implements PostMapper {
private final PostMapper delegate;
private final TagRepository tagRepository;
protected PostMapperDecorator(PostMapper delegate, TagRepository tagRepository) {
this.delegate = delegate;
this.tagRepository = tagRepository;
}
}Remove the @Autowired field injection and use the constructor injection instead. MapStruct will automatically wire the delegate implementation and the repository through the constructor.
🔗 Analysis chain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find PostMapper interface
ast-grep --pattern 'interface PostMapper {
$$$
}'
# Find MapStruct configuration for PostMapper
rg -l "@Mapper" | xargs rg "PostMapper"
Length of output: 770
✏️ Learnings added
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1647
File: jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java:16-16
Timestamp: 2025-01-14T05:18:20.953Z
Learning: In MapStruct decorators, use constructor injection instead of field injection by adding constructor parameters for both the delegate mapper and any required dependencies. MapStruct will automatically wire the delegate implementation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
@coderabbitai, receiving error as below, please fix
Specified decorator type has no default constructor nor a constructor with a single parameter accepting the decorated mapper type.
There was a problem hiding this comment.
@rajadilipkolli Here's the corrected constructor pattern for the MapStruct decorator:
public abstract class PostMapperDecorator implements PostMapper {
private final PostMapper delegate;
private final TagRepository tagRepository;
@Autowired
protected PostMapperDecorator(PostMapper delegate) {
this.delegate = delegate;
this.tagRepository = null; // Will be injected by Spring
}
@Autowired
public void setTagRepository(TagRepository tagRepository) {
this.tagRepository = tagRepository;
}
}This follows MapStruct's requirement of having a constructor with a single parameter for the delegate mapper, while still allowing Spring to inject the TagRepository through setter injection.
✏️ Learnings added
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1647
File: jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java:16-16
Timestamp: 2025-01-14T05:26:52.862Z
Learning: In MapStruct decorators, the constructor must either be a default constructor or have a single parameter accepting the delegate mapper type. Additional dependencies should be injected through setter injection or field injection.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (10)
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/TagRepository.java (1)
9-9: LGTM! Method follows Spring Data JPA conventions.The
findByTagNamemethod is well-designed:
- Uses Optional for null-safety
- Follows Spring Data JPA naming conventions for automatic query generation
- Clear and focused purpose aligned with the domain model
Consider adding a database index on the
tag_namecolumn if you expect frequent queries, especially if the tags table grows large:CREATE INDEX idx_tag_name ON tags(tag_name);jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java (1)
14-32: Consider adding security and rate limiting documentation.As this API handles user-generated content, consider documenting:
- Authentication requirements using
@SecurityRequirement- Rate limiting constraints
- Maximum request size limits
Example addition:
@SecurityRequirement(name = "bearer-jwt") @Tag(name = "Post API") interface PostAPI { // existing code }Also consider adding rate limiting metadata:
@RateLimiter(name = "post-creation") ResponseEntity<PostResponse> createPostByUserName(...)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/PostService.java (1)
5-8: Add documentation and validation constraints.The interface would benefit from:
- Javadoc describing the method's purpose, parameters, and behavior
@NotNullconstraints on parameters@Transactionalannotation if implementing classes need transaction managementpublic interface PostService { + /** + * Creates a new post for the specified user. + * + * @param postRequest the post details to create + * @param userName the name of the user creating the post + * @return the created post response + * @throws IllegalArgumentException if the request is invalid + */ + @Transactional + PostResponse createPost(@NotNull PostRequest postRequest, @NotNull String userName); }jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/request/PostRequest.java (1)
7-13: Add additional validation constraints for better data integrity.Consider adding the following validations:
- Size constraints for
titleandcontentto prevent extremely long inputs@NotNullfor the lists to prevent null values (if required)@Validfor nested objects to trigger validation ofPostCommentRequestandTagRequestpublic record PostRequest( - @NotBlank(message = "Title of post is mandatory") String title, - @NotBlank(message = "Content of post can't be Blank") String content, + @NotBlank(message = "Title of post is mandatory") + @Size(max = 255, message = "Title must not exceed 255 characters") + String title, + @NotBlank(message = "Content of post can't be Blank") + @Size(max = 10000, message = "Content must not exceed 10000 characters") + String content, boolean published, LocalDateTime publishedAt, + @Valid List<PostCommentRequest> comments, + @Valid List<TagRequest> tags) {}jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/impl/JPAPostServiceImpl.java (1)
22-27: Add error handling and logging for better observability.The service method should:
- Validate the userName parameter
- Handle potential repository exceptions
- Add logging for debugging and monitoring
+ private static final Logger log = LoggerFactory.getLogger(JPAPostServiceImpl.class); + @Override @Transactional public void createPost(PostRequest postRequest, String userName) { + if (StringUtils.isBlank(userName)) { + throw new IllegalArgumentException("Username cannot be blank"); + } + log.debug("Creating post with title '{}' for user '{}'", postRequest.title(), userName); Post post = this.postMapper.postRequestToEntity(postRequest, userName); - this.postRepository.save(post); + try { + post = this.postRepository.save(post); + log.info("Successfully created post with id {} for user '{}'", post.getId(), userName); + } catch (DataAccessException e) { + log.error("Failed to create post for user '{}': {}", userName, e.getMessage()); + throw new ServiceException("Failed to create post", e); + } }jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostController.java (1)
23-35: Return created resource details in the response.Consider returning the created post details in the response body for better client experience.
@PostMapping("/{user_name}/posts/") @Override - public ResponseEntity<Object> createPostByUserName( + public ResponseEntity<PostResponse> createPostByUserName( @RequestBody @Valid PostRequest postRequest, @PathVariable("user_name") String userName) { - this.jpaPostService.createPost(postRequest, userName); + Post createdPost = this.jpaPostService.createPost(postRequest, userName); + PostResponse response = postMapper.toPostResponse(createdPost); URI location = ServletUriComponentsBuilder.fromCurrentRequest() - .path("{title}") - .buildAndExpand(postRequest.title()) + .path("{id}") + .buildAndExpand(createdPost.getId()) .toUri(); - return ResponseEntity.created(location).build(); + return ResponseEntity.created(location).body(response); }jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapper.java (2)
11-15: Consider adding null value mapping strategy for collections.The current null value check strategy might not handle empty collections appropriately.
@Mapper( componentModel = MappingConstants.ComponentModel.SPRING, injectionStrategy = InjectionStrategy.CONSTRUCTOR, - nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS) + nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS, + collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED)
18-18: Add mapping annotations for the main mapping method.The
postRequestToEntitymethod lacks mapping annotations which could lead to ambiguous mappings.- Post postRequestToEntity(PostRequest postRequest, String userName); + @Mapping(source = "userName", target = "details.createdBy") + @Mapping(source = "postRequest.comments", target = "comments") + @Mapping(source = "postRequest.tags", target = "tags") + Post postRequestToEntity(PostRequest postRequest, String userName);jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java (2)
15-16: Consider using constructor injection instead of field injection.Replace
@Autowiredfield injection with constructor injection for better testability and to make dependencies explicit.- @Autowired - private TagRepository tagRepository; + private final TagRepository tagRepository; + + public PostMapperDecorator(TagRepository tagRepository) { + this.tagRepository = tagRepository; + }
36-42: Consider adding batch processing for tag creation.The current implementation performs database operations within a loop. For better performance with multiple tags, consider collecting new tags and performing a batch insert.
- tagRequests.forEach(tagRequest -> { - // Check if the tag already exists - Optional<Tag> tagOptional = this.tagRepository.findByTagName(tagRequest.name()); - Tag tag = tagOptional.orElseGet(() -> tagRequestToTag(tagRequest)); - // Use the managed Tag entity to create and associate PostTag - post.addTag(tag); - }); + Map<String, TagRequest> newTagRequests = tagRequests.stream() + .collect(Collectors.toMap(TagRequest::name, Function.identity())); + + // Find existing tags in a single query + List<Tag> existingTags = tagRepository.findByTagNameIn(newTagRequests.keySet()); + Map<String, Tag> tagMap = existingTags.stream() + .collect(Collectors.toMap(Tag::getTagName, Function.identity())); + + // Create new tags in batch + List<Tag> newTags = newTagRequests.entrySet().stream() + .filter(e -> !tagMap.containsKey(e.getKey())) + .map(e -> tagRequestToTag(e.getValue())) + .collect(Collectors.toList()); + if (!newTags.isEmpty()) { + tagRepository.saveAll(newTags); + newTags.forEach(tag -> tagMap.put(tag.getTagName(), tag)); + } + + // Associate all tags with the post + tagRequests.forEach(tagRequest -> post.addTag(tagMap.get(tagRequest.name())));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
jpa/boot-jpa-jooq-sample/pom.xml(3 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/entities/PostComment.java(3 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapper.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/request/PostCommentRequest.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/request/PostRequest.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/request/TagRequest.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/response/PostResponse.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/TagRepository.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/PostService.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/impl/JPAPostServiceImpl.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostController.java(1 hunks)jpa/boot-jpa-jooq-sample/src/test/java/com/example/learning/web/controller/PostControllerIntTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/response/PostResponse.java
- jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/request/TagRequest.java
- jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/model/request/PostCommentRequest.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/TagRepository.java (1)
7-10: Verify error handling in the service layer.Since this repository is used for tag existence checks during post creation, ensure proper error handling is implemented in the service layer when tag lookup fails.
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java (1)
1-13: LGTM! Well-organized imports.The package structure and imports are clean and follow best practices.
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/PostService.java (1)
3-3: Verify PostRequest model validation.Ensure that the PostRequest model has proper validation constraints for required fields and data integrity.
✅ Verification successful
PostRequest model has proper validation constraints
The PostRequest model includes appropriate validation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check PostRequest class for validation annotations ast-grep --pattern 'class PostRequest { $$$ @$_($$$) private $_ $_; $$$ }'Length of output: 84
Script:
#!/bin/bash # Find PostRequest.java file fd PostRequest.java # Examine content for validation annotations rg -A 2 "@(NotNull|NotEmpty|NotBlank|Size|Min|Max|Pattern|Email|Valid)" --type javaLength of output: 65876
jpa/boot-jpa-jooq-sample/pom.xml (1)
141-147: Add Lombok's annotation processor to avoid conflicts.If the project uses Lombok, its annotation processor should be added to avoid conflicts with MapStruct.
<annotationProcessorPaths> <path> <groupId>org.mapstruct</groupId> <artifactId>mapstruct-processor</artifactId> <version>${org.mapstruct.version}</version> </path> + <path> + <groupId>org.projectlombok</groupId> + <artifactId>lombok</artifactId> + <version>${lombok.version}</version> + </path> + <path> + <groupId>org.projectlombok</groupId> + <artifactId>lombok-mapstruct-binding</artifactId> + <version>0.2.0</version> + </path> </annotationProcessorPaths>
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java
Outdated
Show resolved
Hide resolved
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java
Outdated
Show resolved
Hide resolved
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/PostService.java
Show resolved
Hide resolved
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostController.java
Show resolved
Hide resolved
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java
Show resolved
Hide resolved
...jpa-jooq-sample/src/test/java/com/example/learning/web/controller/PostControllerIntTest.java
Outdated
Show resolved
Hide resolved
| this.mockMvcTester | ||
| .post() | ||
| .uri("/api/users/{user_name}/posts/", "junit") | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .accept(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(postRequest)) | ||
| .assertThat() | ||
| .hasStatus(HttpStatus.CREATED) | ||
| .hasHeader(HttpHeaders.LOCATION, "http://localhost/api/users/junit/posts/newPostTitle"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add assertions for the created entity.
The test only verifies the HTTP response. Add assertions to verify that the post and its associated entities were correctly persisted in the database.
.hasStatus(HttpStatus.CREATED)
- .hasHeader(HttpHeaders.LOCATION, "http://localhost/api/users/junit/posts/newPostTitle");
+ .hasHeader(HttpHeaders.LOCATION, "http://localhost/api/users/junit/posts/newPostTitle")
+ .andDo(result -> {
+ // Verify post was created
+ Post savedPost = postRepository.findByTitleAndUserName("newPostTitle", "junit")
+ .orElseThrow();
+ assertThat(savedPost.getComments()).hasSize(2);
+ assertThat(savedPost.getTags()).hasSize(3);
+ });Committable suggestion skipped: line range outside the PR's diff.
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/entities/PostComment.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java (2)
15-19: 🛠️ Refactor suggestionRefactor dependency injection pattern for MapStruct compatibility.
The current field injection pattern is not compatible with MapStruct's requirements. MapStruct decorators must have either a default constructor or a constructor with a single parameter accepting the delegate mapper.
Replace the field injection with this pattern:
public abstract class PostMapperDecorator implements PostMapper { - @Autowired - private TagRepository tagRepository; + private final PostMapper delegate; + private TagRepository tagRepository; + + @Autowired + protected PostMapperDecorator(PostMapper delegate) { + this.delegate = delegate; + } + + @Autowired + public void setTagRepository(TagRepository tagRepository) { + this.tagRepository = tagRepository; + }
59-65: 🛠️ Refactor suggestionAdd validation and ordering for comments.
The current implementation lacks several important validations:
- No validation of comment content
- No chronological ordering of comments
- Missing transaction boundary
Consider this improved implementation:
+ @Transactional private void addPostCommentsToPost(List<PostCommentRequest> comments, Post post) { if (comments == null) { return; } + + // Validate comments + comments.forEach(comment -> { + if (comment.content() == null || comment.content().trim().isEmpty()) { + throw new IllegalArgumentException("Comment content cannot be empty"); + } + if (comment.publishedAt() != null && comment.publishedAt().isAfter(LocalDateTime.now())) { + throw new IllegalArgumentException("Comment cannot be published in the future"); + } + }); + + // Sort comments by publishedAt before adding + comments.stream() + .sorted(Comparator.comparing(PostCommentRequest::publishedAt)) + .forEach(comment -> post.addComment(postCommentRequestToPostComment(comment))); - comments.forEach(comment -> post.addComment(postCommentRequestToPostComment(comment)));jpa/boot-jpa-jooq-sample/src/test/java/com/example/learning/web/controller/PostControllerIntTest.java (1)
22-58: 🛠️ Refactor suggestionEnhance test coverage with additional assertions.
The test should verify all aspects of the created post:
- Post entity and its attributes
- Associated comments
- All associated tags
Add these assertions after the HTTP response check:
.hasHeader(HttpHeaders.LOCATION, "http://localhost/api/users/junit/posts/newPostTitle"); + + // Verify post and its relationships + assertThat(postRepository.findByTitleAndUserName("newPostTitle", "junit")) + .isPresent() + .get() + .satisfies(savedPost -> { + assertThat(savedPost.getContent()).isEqualTo("newPostContent"); + assertThat(savedPost.getIsPublished()).isTrue(); + assertThat(savedPost.getPublishedOn()).isEqualTo(LocalDateTime.parse("2025-01-15T10:00:00")); + + // Verify comments + assertThat(savedPost.getComments()) + .hasSize(2) + .extracting("title", "content") + .containsExactly( + tuple("commentTitle1", "Nice Post1"), + tuple("commentTitle2", "Nice Post2") + ); + + // Verify tags + assertThat(savedPost.getTags()) + .hasSize(3) + .extracting("tagName") + .containsExactlyInAnyOrder("junit", "spring", "Java"); + });
🧹 Nitpick comments (9)
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/TagRepository.java (1)
11-13: Consider adding parameter annotations and documentation.The finder methods are well-designed and follow Spring Data JPA conventions. Consider these enhancements:
- Add
@NonNullannotations to parameters to enforce null safety- Add Javadoc to document the methods' purposes and behavior
Example enhancement:
/** * Finds a tag by its name. * @param tagName the name of the tag to find * @return the tag if found, empty optional otherwise */ Optional<Tag> findByTagName(@NonNull String tagName); /** * Finds all tags whose names are in the given set. * @param tagNameSet set of tag names to find * @return list of matching tags */ List<Tag> findByTagNameIn(@NonNull Set<String> tagNameSet);jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java (1)
33-57: Consider adding transaction boundary and tag validation.While the implementation is efficient with batch operations, consider these improvements:
- Add transaction boundary to ensure atomicity
- Add validation for tag names (e.g., length, format)
Example implementation:
+ @Transactional private void addPostTagsToPost(List<TagRequest> tagRequests, Post post) { if (tagRequests == null) { return; } + + // Validate tag names + tagRequests.forEach(tag -> { + if (tag.name() == null || tag.name().trim().isEmpty()) { + throw new IllegalArgumentException("Tag name cannot be empty"); + } + if (tag.name().length() > 50) { // adjust max length as needed + throw new IllegalArgumentException("Tag name too long"); + } + });jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/exception/PostAlreadyExistsException.java (1)
6-17: Add serialization support to the exception class.While the implementation is good, consider adding serialVersionUID for proper serialization support, as this is a serializable class extending RuntimeException.
public class PostAlreadyExistsException extends RuntimeException { + private static final long serialVersionUID = 1L; private static final HttpStatus httpStatus = HttpStatus.CONFLICT;jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/config/GlobalExceptionHandler.java (3)
50-50: Consider externalizing the error type URI.The hardcoded URI should be externalized to a configuration property for better maintainability and flexibility.
52-52: Consider using ISO-8601 formatted string for timestamp.While Instant works, using an ISO-8601 formatted string might be more widely compatible across different JSON parsers and clients.
- problemDetail.setProperty("timestamp", Instant.now()); + problemDetail.setProperty("timestamp", Instant.now().toString());
30-41: Consider adding validation error count to the response.For better error reporting, consider including the total count of validation errors in the response.
List<ApiValidationError> validationErrorsList = methodArgumentNotValidException.getAllErrors().stream() // ... existing mapping code ... .toList(); problemDetail.setProperty("violations", validationErrorsList); + problemDetail.setProperty("totalViolations", validationErrorsList.size());jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/impl/JPAPostServiceImpl.java (1)
30-39: Consider enhancing error handling and logging.While the implementation is good, consider these improvements:
- Validate input parameters (postRequest and userName) for null values
- Add debug logging for the case when post already exists
- Add trace logging for the saved post details
@Transactional public void createPost(PostRequest postRequest, String userName) { + Objects.requireNonNull(postRequest, "PostRequest must not be null"); + Objects.requireNonNull(userName, "UserName must not be null"); + Optional<Post> byTitleIgnoreCase = this.postRepository.findByTitleIgnoreCase(postRequest.title()); if (byTitleIgnoreCase.isPresent()) { + log.debug("Post with title '{}' already exists", postRequest.title()); throw new PostAlreadyExistsException(postRequest.title()); } else { log.debug("Creating post with title '{}' for user '{}'", postRequest.title(), userName); Post post = this.postMapper.postRequestToEntity(postRequest, userName); - this.postRepository.save(post); + Post savedPost = this.postRepository.save(post); + log.trace("Created post with id: {}", savedPost.getId()); } }jpa/boot-jpa-jooq-sample/src/test/java/com/example/learning/web/controller/PostControllerIntTest.java (2)
60-113: Improve test maintainability and coverage.
- Extract duplicate assertion logic into a helper method
- Add tests for other validation scenarios:
- Invalid dates (future/past)
- Null fields
- Invalid comment data
- Invalid tag data
+ private void assertBadRequest(PostRequest request, String expectedViolation) throws Exception { + this.mockMvcTester + .post() + .uri("/api/users/{user_name}/posts/", "junit") + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request)) + .assertThat() + .hasStatus(HttpStatus.BAD_REQUEST) + .bodyJson() + .convertTo(ProblemDetail.class) + .satisfies(problemDetail -> { + assertThat(problemDetail.getTitle()).isEqualTo("Constraint Violation"); + assertThat(problemDetail.getStatus()).isEqualTo(400); + assertThat(problemDetail.getDetail()).isEqualTo(expectedViolation); + assertThat(problemDetail.getInstance()).isEqualTo(URI.create("/api/users/junit/posts/")); + }); + } @Test void shouldFailForInvalidInput() throws Exception { - // Test with blank title - PostRequest invalidRequest = new PostRequest( + // Test various validation scenarios + assertBadRequest( + new PostRequest("", "content", true, LocalDateTime.parse("2025-01-15T10:00:00"), List.of(), List.of()), + "Title must not be blank" + ); + + assertBadRequest( + new PostRequest("title", "", true, LocalDateTime.parse("2025-01-15T10:00:00"), List.of(), List.of()), + "Content must not be blank" + ); + + assertBadRequest( + new PostRequest("title", "content", true, null, List.of(), List.of()), + "Published date must not be null" + );
115-164: Enhance duplicate title test assertions.The test should verify the actual properties in the problem detail instead of just checking the size.
assertThat(problemDetail.getProperties()) .isNotNull() .isNotEmpty() - .hasSize(2); + .containsExactlyInAnyOrderEntriesOf(Map.of( + "timestamp", LocalDateTime.now().toString(), + "errorCategory", "DATA_INTEGRITY_VIOLATION" + ));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
jpa/boot-jpa-jooq-sample/pom.xml(4 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/config/GlobalExceptionHandler.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/exception/PostAlreadyExistsException.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapper.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/PostCommentRepository.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/PostRepository.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/TagRepository.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/impl/JPAPostServiceImpl.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java(1 hunks)jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostController.java(1 hunks)jpa/boot-jpa-jooq-sample/src/test/java/com/example/learning/web/controller/PostControllerIntTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/PostCommentRepository.java
🚧 Files skipped from review as they are similar to previous changes (3)
- jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostAPI.java
- jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/web/controller/PostController.java
- jpa/boot-jpa-jooq-sample/pom.xml
🧰 Additional context used
📓 Learnings (2)
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java (2)
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1647
File: jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java:16-16
Timestamp: 2025-01-14T05:26:53.024Z
Learning: In MapStruct decorators, the constructor must either be a default constructor or have a single parameter accepting the delegate mapper type. Additional dependencies should be injected through setter injection or field injection.
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1647
File: jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java:16-16
Timestamp: 2025-01-14T05:18:21.158Z
Learning: In MapStruct decorators, use constructor injection instead of field injection by adding constructor parameters for both the delegate mapper and any required dependencies. MapStruct will automatically wire the delegate implementation.
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapper.java (2)
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1647
File: jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java:16-16
Timestamp: 2025-01-14T05:26:53.024Z
Learning: In MapStruct decorators, the constructor must either be a default constructor or have a single parameter accepting the delegate mapper type. Additional dependencies should be injected through setter injection or field injection.
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1647
File: jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java:16-16
Timestamp: 2025-01-14T05:18:21.158Z
Learning: In MapStruct decorators, use constructor injection instead of field injection by adding constructor parameters for both the delegate mapper and any required dependencies. MapStruct will automatically wire the delegate implementation.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (10)
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/TagRepository.java (3)
4-6: LGTM! Imports are correctly organized.The added imports are necessary for the new method signatures and follow standard Java conventions.
9-10: LGTM! Repository interface is properly defined.The interface correctly extends JpaRepository with appropriate entity type and ID parameters.
11-13: Verify the integration with PostMapperDecorator.Let's verify the usage of these repository methods in the PostMapperDecorator class.
✅ Verification successful
✓ Repository methods are properly integrated
The methods are correctly used in the codebase:
findByTagNameInis efficiently used in PostMapperDecorator for batch tag operationsfindByTagNameis properly utilized in tests for verification🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of the new repository methods echo "Searching for findByTagName usage:" rg "findByTagName" -A 3 echo -e "\nSearching for findByTagNameIn usage:" rg "findByTagNameIn" -A 3Length of output: 5864
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java (1)
20-31: LGTM! Well-structured mapping implementation.The method follows good practices with proper null checking and clear separation of concerns for mapping child entities.
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/repository/PostRepository.java (1)
9-9: LGTM! Well-designed repository method.The method follows Spring Data JPA naming conventions and appropriately uses Optional for the return type. Case-insensitive search is a good practice for title comparisons.
jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/service/impl/JPAPostServiceImpl.java (1)
23-26: LGTM! Good use of constructor injection.Constructor injection is the recommended approach for required dependencies.
jpa/boot-jpa-jooq-sample/src/test/java/com/example/learning/web/controller/PostControllerIntTest.java (1)
20-20: Add missing test scenarios.Based on previous review comments, add test cases for:
- Error scenarios (invalid user, malformed JSON)
- Duplicate tag handling
Let's verify the current test coverage:
#!/bin/bash # Check test coverage for PostController find . -type f -name "PostController.java" -exec echo "Found PostController at: {}" \; # Look for error handling methods ast-grep --pattern 'class PostController { $$$ @ExceptionHandler($_) $$$ }' # Look for tag handling methods ast-grep --pattern 'class PostController { $$$ $_(Tag $_) { $$$ } $$$ }'jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapper.java (3)
1-16: LGTM! Well-organized imports.The package structure and imports are clean and well-organized, following Java conventions.
27-45: LGTM! Well-structured mapping configurations.The mapping methods are well-defined with:
- Appropriate ignoring of JPA-managed fields
- Clear and consistent field mappings
- Proper handling of relationships
25-25: Document the base mapping method and verify its implementation.The
postRequestToEntitymethod lacks specific mapping configurations. Since it's likely implemented in the decorator, let's verify its implementation and suggest adding documentation.Let's check the decorator's implementation:
Consider adding Javadoc to clarify the method's purpose:
+ /** + * Maps a PostRequest to a Post entity, handling nested comments and tags. + * @param postRequest the request containing post data + * @param userName the creator's username + * @return fully populated Post entity + */ Post postRequestToEntity(PostRequest postRequest, String userName);✅ Verification successful
Documentation would improve code clarity, but implementation follows best practices
The
postRequestToEntitymethod is properly implemented as part of a well-structured MapStruct mapper that:
- Uses decorator pattern for complex mapping logic
- Follows consistent mapping patterns
- Properly handles nested objects
Consider adding the suggested documentation to improve clarity:
/** * Maps a PostRequest to a Post entity, handling nested comments and tags. * @param postRequest the request containing post data * @param userName the creator's username * @return fully populated Post entity */ Post postRequestToEntity(PostRequest postRequest, String userName);🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the implementation of postRequestToEntity in the decorator ast-grep --pattern 'class PostMapperDecorator { $$$ public Post postRequestToEntity($$$) { $$$ } $$$ }'Length of output: 117
Script:
#!/bin/bash # Find all files related to PostMapper fd -t f -e java . | rg -l "PostMapper" # Search for the implementation pattern with different class names ast-grep --pattern 'Post postRequestToEntity(PostRequest $_, String $_) { $$$ }' # Look for MapStruct configuration rg -l "@Mapper|@DecoratedWith" --type javaLength of output: 1955
Script:
#!/bin/bash # Check the content of PostMapper.java cat "jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapper.java" # Search for PostMapperImpl or similar implementation files fd -t f -e java . | rg -l "PostMapper(Impl|Decorator)"Length of output: 2106
| @Mapper( | ||
| componentModel = MappingConstants.ComponentModel.SPRING, | ||
| injectionStrategy = InjectionStrategy.CONSTRUCTOR, | ||
| nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS, | ||
| collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED) | ||
| @DecoratedWith(PostMapperDecorator.class) |
There was a problem hiding this comment.
💡 Codebase verification
Field injection in decorator contradicts mapper's constructor injection pattern
The PostMapperDecorator implementation has several issues that contradict the best practices set in the mapper configuration:
- Uses field injection (
@Autowired private TagRepository) instead of constructor injection - Missing delegate mapper injection which is typically required for MapStruct decorators
- Could lead to testing difficulties and potential runtime issues
Suggested improvements:
- Replace field injection with constructor injection
- Add delegate mapper as a constructor parameter
- Example structure:
private final PostMapper delegate;
private final TagRepository tagRepository;
public PostMapperDecorator(PostMapper delegate, TagRepository tagRepository) {
this.delegate = delegate;
this.tagRepository = tagRepository;
}🔗 Analysis chain
Verify PostMapperDecorator implementation based on learnings.
The mapper configuration follows best practices with constructor injection, null checking, and appropriate collection mapping strategy. However, based on the retrieved learnings, ensure that the PostMapperDecorator follows these guidelines:
- Uses constructor injection for both delegate mapper and dependencies
- Avoids field injection
Let's verify the decorator implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check PostMapperDecorator implementation
ast-grep --pattern 'class PostMapperDecorator {
$$$
@Autowired
$$$
}'
# Check for constructor
ast-grep --pattern 'class PostMapperDecorator {
$$$
PostMapperDecorator($_) {
$$$
}
$$$
}'
Length of output: 182
Script:
#!/bin/bash
# Find decorator file
rg -l "PostMapperDecorator"
# Once found, let's see its implementation
rg -A 10 "class PostMapperDecorator"
Length of output: 1542
Script:
#!/bin/bash
# View complete decorator implementation
cat "jpa/boot-jpa-jooq-sample/src/main/java/com/example/learning/mapper/PostMapperDecorator.java"
Length of output: 2502
|
Summary by CodeRabbit
New Features
Improvements
Technical Updates
OffsetDateTimetoLocalDateTime