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 : expose endpoint to save post along with details, comments and tags #1647

Merged
merged 3 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
32 changes: 32 additions & 0 deletions jpa/boot-jpa-jooq-sample/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<properties>
<java.version>21</java.version>
<springdoc-openapi.version>2.8.3</springdoc-openapi.version>
<org.mapstruct.version>1.6.3</org.mapstruct.version>
<testcontainers-jooq-codegen-maven-plugin.version>0.0.4</testcontainers-jooq-codegen-maven-plugin.version>
<spotless.version>2.44.1</spotless.version>

Expand All @@ -43,6 +44,10 @@
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-web</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-validation</artifactId>
</dependency>

<dependency>
<groupId>org.liquibase</groupId>
Expand All @@ -59,6 +64,12 @@
<version>1.0.6</version>
</dependency>

<dependency>
<groupId>org.mapstruct</groupId>
<artifactId>mapstruct</artifactId>
<version>${org.mapstruct.version}</version>
</dependency>

<dependency>
<groupId>org.springdoc</groupId>
<artifactId>springdoc-openapi-starter-webmvc-ui</artifactId>
Expand Down Expand Up @@ -119,6 +130,27 @@
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>build-info</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>org.mapstruct</groupId>
<artifactId>mapstruct-processor</artifactId>
<version>${org.mapstruct.version}</version>
</path>
</annotationProcessorPaths>
<release>${java.version}</release>
</configuration>
</plugin>
<plugin>
<groupId>org.testcontainers</groupId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package com.example.learning.config;

import com.example.learning.exception.PostAlreadyExistsException;
import java.net.URI;
import java.time.Instant;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ProblemDetail;
import org.springframework.validation.FieldError;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseStatus;

@ControllerAdvice
@Order(Ordered.HIGHEST_PRECEDENCE)
public class GlobalExceptionHandler {

@ExceptionHandler(MethodArgumentNotValidException.class)
@ResponseStatus(HttpStatus.BAD_REQUEST)
ProblemDetail onException(MethodArgumentNotValidException methodArgumentNotValidException) {
ProblemDetail problemDetail =
ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(400), "Invalid request content.");
problemDetail.setTitle("Constraint Violation");
List<ApiValidationError> validationErrorsList = methodArgumentNotValidException.getAllErrors().stream()
.map(objectError -> {
FieldError fieldError = (FieldError) objectError;
return new ApiValidationError(
fieldError.getObjectName(),
fieldError.getField(),
fieldError.getRejectedValue(),
Objects.requireNonNull(fieldError.getDefaultMessage(), ""));
})
.sorted(Comparator.comparing(ApiValidationError::field))
.toList();
problemDetail.setProperty("violations", validationErrorsList);
return problemDetail;
}

@ExceptionHandler(PostAlreadyExistsException.class)
ProblemDetail onException(PostAlreadyExistsException postAlreadyExistsException) {
ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(
postAlreadyExistsException.getHttpStatus(), postAlreadyExistsException.getMessage());
problemDetail.setTitle("Duplicate entry");
problemDetail.setType(URI.create("https://api.boot-jpa-jooq.com/errors/already-exists"));
problemDetail.setProperty("errorCategory", "Generic");
problemDetail.setProperty("timestamp", Instant.now().toString());
return problemDetail;
}

record ApiValidationError(String object, String field, Object rejectedValue, String message) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import jakarta.persistence.Table;
import java.io.Serial;
import java.io.Serializable;
import java.time.OffsetDateTime;
import java.time.LocalDateTime;
import java.util.Objects;
import org.hibernate.Hibernate;

Expand All @@ -33,7 +33,7 @@ public class PostComment extends Auditable implements Serializable {

private boolean published;

private OffsetDateTime publishedAt;
private LocalDateTime publishedAt;
rajadilipkolli marked this conversation as resolved.
Show resolved Hide resolved

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "post_id", nullable = false)
Expand Down Expand Up @@ -77,11 +77,11 @@ public PostComment setPublished(boolean published) {
return this;
}

public OffsetDateTime getPublishedAt() {
public LocalDateTime getPublishedAt() {
return publishedAt;
}

public PostComment setPublishedAt(OffsetDateTime publishedAt) {
public PostComment setPublishedAt(LocalDateTime publishedAt) {
this.publishedAt = publishedAt;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.example.learning.exception;

import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;

public class PostAlreadyExistsException extends RuntimeException {

private static final long serialVersionUID = 1L;

private static final HttpStatus httpStatus = HttpStatus.CONFLICT;

public PostAlreadyExistsException(String title) {
super("Post with title : " + title + " already exists");
}

public HttpStatusCode getHttpStatus() {
return httpStatus;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.example.learning.mapper;

import com.example.learning.entities.Post;
import com.example.learning.entities.PostComment;
import com.example.learning.entities.Tag;
import com.example.learning.model.request.PostCommentRequest;
import com.example.learning.model.request.PostRequest;
import com.example.learning.model.request.TagRequest;
import org.mapstruct.CollectionMappingStrategy;
import org.mapstruct.DecoratedWith;
import org.mapstruct.InjectionStrategy;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.MappingConstants;
import org.mapstruct.NullValueCheckStrategy;

@Mapper(
componentModel = MappingConstants.ComponentModel.SPRING,
injectionStrategy = InjectionStrategy.CONSTRUCTOR,
nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS,
collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED)
@DecoratedWith(PostMapperDecorator.class)
Comment on lines +17 to +22
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

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

public interface PostMapper {

Post postRequestToEntity(PostRequest postRequest, String userName);

@Mapping(target = "modifiedAt", ignore = true)
@Mapping(target = "id", ignore = true)
@Mapping(target = "createdAt", ignore = true)
@Mapping(target = "comments", ignore = true)
@Mapping(target = "tags", ignore = true)
@Mapping(source = "userName", target = "details.createdBy")
Post postDtoToPostIgnoringChild(PostRequest postRequest, String userName);

@Mapping(target = "post", ignore = true)
@Mapping(target = "modifiedAt", ignore = true)
@Mapping(target = "id", ignore = true)
@Mapping(target = "createdAt", ignore = true)
@Mapping(source = "review", target = "content")
PostComment postCommentRequestToPostComment(PostCommentRequest postCommentRequest);

@Mapping(target = "id", ignore = true)
@Mapping(source = "name", target = "tagName")
@Mapping(source = "description", target = "tagDescription")
Tag tagRequestToTag(TagRequest tagRequest);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package com.example.learning.mapper;

import com.example.learning.entities.Post;
import com.example.learning.entities.Tag;
import com.example.learning.model.request.PostCommentRequest;
import com.example.learning.model.request.PostRequest;
import com.example.learning.model.request.TagRequest;
import com.example.learning.repository.TagRepository;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.springframework.beans.factory.annotation.Autowired;

public abstract class PostMapperDecorator implements PostMapper {

@Autowired
private TagRepository tagRepository;
Copy link
Owner Author

Choose a reason for hiding this comment

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

@coderabbitai, how to use constructor injection instead of field injection in the Decorator of PostMapper defined using MapStruct

Copy link
Contributor

Choose a reason for hiding this comment

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

@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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor

Choose a reason for hiding this comment

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

@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.


@Override
public Post postRequestToEntity(PostRequest postRequest, String userName) {
if (postRequest == null) {
return null;
}

Post post = postDtoToPostIgnoringChild(postRequest, userName);
addPostCommentsToPost(postRequest.comments(), post);
addPostTagsToPost(postRequest.tags(), post);

return post;
}

private void addPostTagsToPost(List<TagRequest> tagRequests, Post post) {
if (tagRequests == null) {
return;
}

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())));
}

private void addPostCommentsToPost(List<PostCommentRequest> comments, Post post) {
if (comments == null) {
return;
}

comments.forEach(comment -> post.addComment(postCommentRequestToPostComment(comment)));
}
rajadilipkolli marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.example.learning.model.request;

import java.time.LocalDateTime;

public record PostCommentRequest(String title, String review, boolean published, LocalDateTime publishedAt) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.example.learning.model.request;

import jakarta.validation.constraints.NotBlank;
import java.time.LocalDateTime;
import java.util.List;

public record PostRequest(
@NotBlank(message = "Title of post is mandatory") String title,
@NotBlank(message = "Content of post can't be Blank") String content,
boolean published,
LocalDateTime publishedAt,
List<PostCommentRequest> comments,
List<TagRequest> tags) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package com.example.learning.model.request;

public record TagRequest(String name, String description) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package com.example.learning.model.response;

public class PostResponse {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.example.learning.repository;

import com.example.learning.entities.PostComment;
import org.springframework.data.jpa.repository.JpaRepository;

public interface PostCommentRepository extends JpaRepository<PostComment, Long> {}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package com.example.learning.repository;

import com.example.learning.entities.Post;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;

public interface PostRepository extends JpaRepository<Post, Long> {}
public interface PostRepository extends JpaRepository<Post, Long> {

Optional<Post> findByTitleIgnoreCase(String title);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
package com.example.learning.repository;

import com.example.learning.entities.Tag;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import org.springframework.data.jpa.repository.JpaRepository;

public interface TagRepository extends JpaRepository<Tag, Long> {}
public interface TagRepository extends JpaRepository<Tag, Long> {

/**
* 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(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(Set<String> tagNameSet);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.example.learning.service;

import com.example.learning.model.request.PostRequest;

public interface PostService {

void createPost(PostRequest postRequest, String userName);
rajadilipkolli marked this conversation as resolved.
Show resolved Hide resolved
}
Loading
Loading