Search code examples
javahibernatejpa

Problem with proper usage of orphan removal


  • I use spring boot 2.7.18 with java 17 and maven.
  • Please treat it purely for educational purposes (don't shout at me that that's not how filtering and updating should work, I know it).
  • I've created that simple example after reading the article about 'best way to handle hibernate MultipleBagFetchException' and stumbled upon the problem with proper orphan removal, hence the question.

I have following entities:

@Entity
@Getter
@Setter
@Table(name = "post")
@SequenceGenerator(name = "post_seq", sequenceName = "post_id_seq", allocationSize = 1)
public class Post {

    @Id
    @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "post_seq")
    Long id;

    @ToString.Exclude
    @EqualsAndHashCode.Exclude
    @OneToMany(mappedBy = "post", cascade = CascadeType.ALL, orphanRemoval = true)
    List<Comment> comments;

    @ToString.Exclude
    @EqualsAndHashCode.Exclude
    @OneToMany(fetch = FetchType.EAGER, mappedBy = "post", cascade = CascadeType.ALL, orphanRemoval = true)
    List<Metadata> metadata;

    @Enumerated(EnumType.STRING)
    PostStatus status;
}

and

@Entity
@Getter
@Setter
@Table(name = "metadata")
@SequenceGenerator(name = "post_metadata_seq", sequenceName = "post_metadata_id_seq", allocationSize = 1)
public class Metadata {

    @Id
    @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "post_metadata_seq")
    Long id;

    @ManyToOne
    @JoinColumn(name = "post_id")
    Post post;

    @Column(name = "\"key\"")
    String key;

    @Column(name = "\"value\"")
    String value;
}

and

@Entity
@Getter
@Setter
@Table(name = "comment")
@SequenceGenerator(name = "post_comment_seq", sequenceName = "post_comment_id_seq", allocationSize = 1)
public class Comment {

    @Id
    @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "post_comment_seq")
    Long id;

    @ManyToOne
    @JoinColumn(name = "post_id")
    Post post;

    String text;
}

and corresponding DTOs:

@Builder(toBuilder = true)
@Getter
@Jacksonized
public class PostDTO {
    Long id;
    MetadataDTO metadata;
    List<CommentDTO> comments;
    PostStatus status;
}

and

@ToString
@Value(staticConstructor = "of")
public class MetadataDTO {

    Map<String, String> metadata;

    @JsonCreator
    @Builder(toBuilder = true)
    public MetadataDTO(@JsonProperty("metadata") final Map<String, String> metadata) {
        this.metadata = Optional.ofNullable(metadata)
                .map(HashMap::new)
                .map(Collections::unmodifiableMap)
                .orElse(Map.of());
    }
}

service:

@Service
@RequiredArgsConstructor
public class PostService {

    public final PersistablePostMapper persistablePostMapper;
    public final PostRepository postRepository;
    public final EntityManager entityManager;

    @Transactional
    public PostDTO saveAll(final PostDTO postDTO) throws BadRequestException {
        String referenceId = postDTO.getMetadata().getMetadata().get("reference_id");
        List<Long> alreadyClosedPostIds = findAllByReferenceId(Long.valueOf(referenceId)).stream()
                .filter(p -> PostStatus.CLOSED.equals(p.getStatus()))
                .map(PostDTO::getId)
                .toList();
        if (alreadyClosedPostIds.contains(postDTO.getId())) {
            throw new BadRequestException();
        }

        return save(postDTO);
    }

    public List<PostDTO> findAllByReferenceId(final Long referenceId) {
        List<Post> posts = entityManager.createQuery("""
                        select distinct p
                        from Post p
                        left join fetch p.metadata m
                        where m.key=:key and m.value=:value""", Post.class)
                .setParameter("key", "reference_id")
                .setParameter("value", String.valueOf(referenceId))
                .setHint(QueryHints.PASS_DISTINCT_THROUGH, false)
                .getResultList();

        posts = entityManager.createQuery("""
                                select distinct p
                                from Post p
                                left join fetch p.comments l
                                where p in :posts"""
                        , Post.class)
                .setParameter("posts", posts)
                .setHint(QueryHints.PASS_DISTINCT_THROUGH, false)
                .getResultList();
        return posts.stream().map(persistablePostMapper::mapToPost).collect(Collectors.toList());
    }

    public PostDTO save(final PostDTO postDTO) {
        Post persistablePost = persistablePostMapper.mapToPersistablePost(postDTO);
        Post savedPersistablePost = postRepository.save(persistablePost);

        return persistablePostMapper.mapToPost(savedPersistablePost);
    }
}

controller:

@RequiredArgsConstructor
@RestController
@RequestMapping("/api/posts")
public class PostController {

    private final PostService postService;

    @PostMapping
    PostDTO createOrUpdatePosts(@RequestBody final PostDTO postDTO) throws BadRequestException {
        return postService.saveAll(postDTO);
    }
}

and the test:

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles(profiles = {"test"})
@AutoConfigureMockMvc
class PostControllerTest {

    @Autowired
    protected MockMvc mockMvc;

    @Autowired
    protected ObjectMapper objectMapper;

    @Autowired
    protected PostService postService;

    private static final TypeReference<PostDTO> POST_TYPE_REFERENCE = new TypeReference<>() {
    };

    @Test
    void shouldUpdatePostComments() throws Exception {
        //given
        CommentDTO comment1 = CommentDTO.builder()
                .text("test1")
                .build();
        CommentDTO comment2 = CommentDTO.builder()
                .text("test2")
                .build();
        List<CommentDTO> commentsBeforeUpdate = List.of(comment1);
        List<CommentDTO> commentsAfterUpdate = List.of(comment1, comment2);
        PostDTO postWithOneComment = PostDTO.builder()
                .status(PostStatus.OPEN)
                .metadata(MetadataDTO.builder()
                        .metadata(Map.of(
                                "reference_id", "100",
                                "origin", "test"))
                        .build())
                .comments(commentsBeforeUpdate)
                .build();
        PostDTO savedPost = postService.save(postWithOneComment);

        List<PostDTO> postBeforeUpdate = postService.findAllByReferenceId(100L);

        //when
        MockHttpServletResponse response = mockMvc.perform(post("/api/posts")
                        .contentType(MediaType.APPLICATION_JSON)
                        .content(objectMapper.writeValueAsString(savedPost.toBuilder()
                                .comments(commentsAfterUpdate)
                                .build())))
                .andReturn().getResponse();

        //then
        assertThat(response.getStatus()).isEqualTo(HttpStatus.OK.value());
        PostDTO returnedPost = objectMapper.readValue(response.getContentAsString(), POST_TYPE_REFERENCE);
        PostDTO postAfterUpdate = postService.findAllByReferenceId(100L).get(0);

        assertThat(returnedPost).isEqualTo(postAfterUpdate);
        assertThat(postBeforeUpdate.size()).isEqualTo(1);
        assertThat(postBeforeUpdate.get(0).getComments()).isEqualTo(commentsBeforeUpdate);
        assertThat(postAfterUpdate.getComments()).isEqualTo(commentsAfterUpdate);
    }
}

  • The problem is that the test is not passing becuase of: java.lang.IllegalStateException: Duplicate key origin (attempted merging values test and test)
  • The SQLs that are generated shows that only one line and only one metadata is deleted (the one with "reference_id" and not the one with "origin")
  • That happens, probably because metadata in Post has orphanRemoval = true and for some reason, only "reference_id" is deleted and 'put' again (because of cascade.ALL).
  • This means that during "update" request in the above test, we have three metadata in the Post: 2x with key "origin" (the old one and the new one as orphanremoval did't delete it) and 1x with key "reference_id" (old one replaced with new one).
  • That causes the issue in: posts.stream().map(persistablePostMapper::mapToPost).collect(Collectors.toList()); where it maps metadata:

stacktrace in Duplicate key exception points here (to the toMap):

default MetadataDTO mapToMetadataDTO(final List<Metadata> persistableMetadata) {
    Map<String, String> metadata = persistableMetadata
            .stream()
            .filter(content -> content.getKey() != null && content.getValue() != null)
            .collect(Collectors.toMap(Metadata::getKey, Metadata::getValue));
    return MetadataDTO.builder()
            .metadata(metadata)
            .build();
}

Could someone explain me why orphan removal is not recreating/removing the whole metadata collection but only "reference_id" ? Why metadata with key "origin" is not recreated as well ?


Solution

  • In your case, the problem occurs because the existing metadata is not being properly removed, despite the orphanRemoval = true attribute being set on the metadata field of the Post entity.

    Hibernate's orphanRemoval only removes entities that have been dereferenced. Therefore, you need to explicitly remove entities that are no longer referenced from the existing metadata collection.

    There are two ways to address this issue.


    The first method is to add a setter method for the metadata field in the Post entity, and include logic to remove the existing metadata when setting a new metadata list.

    public void setMetadata(List<Metadata> metadata) {
        if (this.metadata == null) {
            this.metadata = new ArrayList<>();
        }
        this.metadata.clear();
        if (metadata != null) {
            this.metadata.addAll(metadata);
        }
    }
    

    The second method is to remove all existing metadata and replace it with new metadata in the save method of the PostService.

    public PostDTO save(final PostDTO postDTO) {
        Post persistablePost = persistablePostMapper.mapToPersistablePost(postDTO);
        
        if (persistablePost.getId() != null) {
            Post existingPost = postRepository.findById(persistablePost.getId()).orElseThrow();
            existingPost.getMetadata().clear();
            existingPost.getMetadata().addAll(persistablePost.getMetadata());
            persistablePost = existingPost;
        }
        
        Post savedPersistablePost = postRepository.save(persistablePost);
        return persistablePostMapper.mapToPost(savedPersistablePost);
    }
    

    I think this link will be helpful to you.

    Have a great day!