From 2e7bc29031cd1b32ebde4a54a0b77520e5c0640f Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Fri, 20 Sep 2024 15:54:19 +0200 Subject: [PATCH] Fix: Find one with join issue (#3137) --- .../data/jdbc/h2/joinissue/Author.java | 18 +++++ .../jdbc/h2/joinissue/AuthorRepository.java | 39 ++++++++++ .../data/jdbc/h2/joinissue/AuthorTest.groovy | 72 +++++++++++++++++++ .../data/jdbc/h2/joinissue/Book.java | 24 +++++++ .../data/jdbc/h2/joinissue/Director.java | 62 ++++++++++++++++ .../jdbc/h2/joinissue/DirectorRepository.java | 23 ++++++ .../jdbc/h2/joinissue/DirectorSpec.groovy | 48 +++++++++++++ .../data/jdbc/h2/joinissue/Movie.java | 60 ++++++++++++++++ .../mapper/sql/SqlResultEntityTypeMapper.java | 26 ++++--- 9 files changed, 362 insertions(+), 10 deletions(-) create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Author.java create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorRepository.java create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorTest.groovy create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Book.java create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Director.java create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorRepository.java create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorSpec.groovy create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Movie.java diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Author.java b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Author.java new file mode 100644 index 0000000000..be55c1492d --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Author.java @@ -0,0 +1,18 @@ +package io.micronaut.data.jdbc.h2.joinissue; + +import io.micronaut.data.annotation.GeneratedValue; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.Relation; + +import java.util.Set; + +@MappedEntity("ji_author") +public record Author( + @Id + @GeneratedValue + Long id, + String name, + @Relation(value = Relation.Kind.ONE_TO_MANY, cascade = Relation.Cascade.ALL, mappedBy = "author") + Set books) { +} diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorRepository.java b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorRepository.java new file mode 100644 index 0000000000..e83ce9600a --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorRepository.java @@ -0,0 +1,39 @@ +package io.micronaut.data.jdbc.h2.joinissue; + +import io.micronaut.data.annotation.Join; +import io.micronaut.data.annotation.Query; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.repository.CrudRepository; + +import java.util.List; +import java.util.Optional; + +@JdbcRepository(dialect = Dialect.H2) +@Join(value = "books", type = Join.Type.LEFT_FETCH) +public interface AuthorRepository extends CrudRepository { + + Optional queryByName(String name); + + List queryByNameContains(String partialName); + + Optional findByNameContains(String partialName); //findByNameContainsIgnoreCase has the same issue + + //Note: findFirstByNameContains returns only the first row and therefore only one book. + + /* + SELECT author_.`id`, + author_.`name`, + author_books_.`id` AS books_id, + author_books_.`title` AS books_title, + author_books_.`author` AS books_author + FROM ( + SELECT id,name FROM author + WHERE (`name` LIKE CONCAT('%',:partialName,'%')) + LIMIT 1) author_ + LEFT JOIN book author_books_ ON author_.id=author_books_.author; + */ + @Query("SELECT author_.`id`,author_.`name`,author_books_.`id` AS books_id,author_books_.`title` AS books_title,author_books_.`author` AS books_author FROM (SELECT id,name FROM ji_author WHERE (`name` LIKE CONCAT('%',:partialName,'%')) LIMIT 1) author_ LEFT JOIN ji_book author_books_ ON author_.id=author_books_.author;") + Optional getOneByNameContains(String partialName); + +} diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorTest.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorTest.groovy new file mode 100644 index 0000000000..fbeb19b699 --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/AuthorTest.groovy @@ -0,0 +1,72 @@ +package io.micronaut.data.jdbc.h2.joinissue + +import io.micronaut.data.jdbc.h2.H2DBProperties +import io.micronaut.test.extensions.spock.annotation.MicronautTest +import jakarta.inject.Inject +import spock.lang.Specification + +@MicronautTest +@H2DBProperties +class AuthorTest extends Specification { + + @Inject + AuthorRepository authorRepository + + void test() { + given: + var authorList = List.of( + new Author(null, "Joe Doe", + Set.of(new Book(null, "History of nothing"))), + new Author(null, "Jane Doe", + Set.of(new Book(null, "History of everything"), + new Book(null, "Doing awesome things")))) + + authorRepository.saveAll(authorList) + + when: + Author author = authorRepository.queryByName("Joe Doe").orElse(null) + then: + author.name() == "Joe Doe" + author.books().size() == 1 + + when: + List list = authorRepository.queryByNameContains("Doe") + then: + list.size() == 2 + list.get(0).name() == "Joe Doe" + list.get(0).books().size() == 1 + list.get(1).name() == "Jane Doe" + list.get(1).books().size() == 2 + + when: + author = authorRepository.getOneByNameContains("Doe").orElse(null) + then: + author.name() == "Joe Doe" + author.books().size() == 1 + + when: + author = authorRepository.getOneByNameContains("ne Doe").orElse(null) + then: + author.name() == "Jane Doe" + author.books().size() == 2 + + when: + author = this.authorRepository.findByNameContains("Doe").orElse(null) + then: + author.name() == "Joe Doe" + author.books().size() == 1 + + when: + author = this.authorRepository.findByNameContains("e Doe").orElse(null) + then: + author.name() == "Joe Doe" + author.books().size() == 1 + + when: + author = this.authorRepository.findByNameContains("ne Doe").orElse(null) + then: + author.name() == "Jane Doe" + author.books().size() == 2 + } + +} diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Book.java b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Book.java new file mode 100644 index 0000000000..f56e5ce890 --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Book.java @@ -0,0 +1,24 @@ +package io.micronaut.data.jdbc.h2.joinissue; + +import io.micronaut.core.annotation.Nullable; +import io.micronaut.data.annotation.GeneratedValue; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; +import io.micronaut.data.annotation.Relation; + +@MappedEntity("ji_book") +public record Book( + @Id + @GeneratedValue + Long id, + String title, + @Nullable + @Relation(value = Relation.Kind.MANY_TO_ONE) + @MappedProperty("author") + Author author) { + public Book(Long id, String title) { + this(id, title, null); + } +} + diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Director.java b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Director.java new file mode 100644 index 0000000000..12c441450c --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Director.java @@ -0,0 +1,62 @@ +package io.micronaut.data.jdbc.h2.joinissue; + +import io.micronaut.core.annotation.Nullable; +import io.micronaut.data.annotation.GeneratedValue; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.Relation; + +import java.util.Set; + +@MappedEntity("ji_director") +public class Director { + + @Id + @GeneratedValue + private Long id; + + private String name; + + @Relation(value = Relation.Kind.ONE_TO_MANY, cascade = Relation.Cascade.ALL, mappedBy = "director") + Set movies; + + public Director(String name, @Nullable Set movies) { + this.name = name; + this.movies = movies; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Set getMovies() { + return movies; + } + + public void setMovies(Set movies) { + this.movies = movies; + } + +} + +// @Override +// public String toString() { +// return "Director{" + +// "id=" + id + +// ", name='" + name + '\'' + +// ", movies=" + movies + +// '}'; +// } +//} diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorRepository.java b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorRepository.java new file mode 100644 index 0000000000..305fdabbfe --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorRepository.java @@ -0,0 +1,23 @@ +package io.micronaut.data.jdbc.h2.joinissue; + +import io.micronaut.data.annotation.Join; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.repository.CrudRepository; + +import java.util.List; +import java.util.Optional; + +@JdbcRepository(dialect = Dialect.H2) +public interface DirectorRepository extends CrudRepository { + + @Join(value = "movies", type= Join.Type.LEFT_FETCH) + Optional queryByName(String name); + + @Join(value = "movies", type = Join.Type.LEFT_FETCH) + Optional findByNameContains(String partialName); + + @Join(value = "movies", type = Join.Type.LEFT_FETCH) + List queryByNameContains(String partialName); + +} diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorSpec.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorSpec.groovy new file mode 100644 index 0000000000..2ec14f05df --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/DirectorSpec.groovy @@ -0,0 +1,48 @@ +package io.micronaut.data.jdbc.h2.joinissue + +import io.micronaut.data.jdbc.h2.H2DBProperties +import io.micronaut.test.extensions.spock.annotation.MicronautTest +import jakarta.inject.Inject +import spock.lang.Specification + +@MicronautTest +@H2DBProperties +class DirectorSpec extends Specification { + + @Inject + DirectorRepository directorRepository + + def 'test'() { + given: + var directorList = List.of( + new Director("John Jones", + Set.of(new Movie("Random Movie"))), + new Director("Ann Jones", + Set.of(new Movie("Super Hero Movie"), + new Movie("Anther Movie with Heroes")))) + + directorRepository.saveAll(directorList) + + when: + var director = directorRepository.queryByName("John Jones").orElse(null) + then: + director.getName() == "John Jones" + director.getMovies().size() == 1 + + when: + var list = directorRepository.queryByNameContains("n Jones") + then: + list.size() == 2 + list.get(0).getName() == "John Jones" + list.get(0).getMovies().size() == 1 + list.get(1).getName() == "Ann Jones" + list.get(1).getMovies().size() == 2 + + when: + director = directorRepository.findByNameContains("n Jones").orElse(null) + then: + director.getName() == "John Jones" + director.getMovies().size() == 1 + + } +} diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Movie.java b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Movie.java new file mode 100644 index 0000000000..5617be6db8 --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/joinissue/Movie.java @@ -0,0 +1,60 @@ +package io.micronaut.data.jdbc.h2.joinissue; + +import io.micronaut.core.annotation.Nullable; +import io.micronaut.data.annotation.GeneratedValue; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; +import io.micronaut.data.annotation.Relation; + +@MappedEntity("ji_movie") +public class Movie { + + @Id + @GeneratedValue + private Long id; + + private String title; + + @Nullable + @Relation(Relation.Kind.MANY_TO_ONE) + @MappedProperty("director") + private Director director; + + public Movie(String title) { + this.title = title; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } + + public Director getDirector() { + return director; + } + + public void setDirector(Director director) { + this.director = director; + } + + @Override + public String toString() { + return "Movie{" + + "id=" + id + + ", title='" + title + '\'' + + ", director=" + director + + '}'; + } +} diff --git a/data-runtime/src/main/java/io/micronaut/data/runtime/mapper/sql/SqlResultEntityTypeMapper.java b/data-runtime/src/main/java/io/micronaut/data/runtime/mapper/sql/SqlResultEntityTypeMapper.java index b7c0a23ed1..4c41ce6768 100644 --- a/data-runtime/src/main/java/io/micronaut/data/runtime/mapper/sql/SqlResultEntityTypeMapper.java +++ b/data-runtime/src/main/java/io/micronaut/data/runtime/mapper/sql/SqlResultEntityTypeMapper.java @@ -263,13 +263,20 @@ public PushingMapper readOneMapper() { return new PushingMapper<>() { final MappingContext ctx = MappingContext.of(entity, startingPrefix); + Object entityId; R entityInstance; @Override public void processRow(RS row) { - if (entityInstance == null) { + Object id = readEntityId(row, ctx); + if (id == null) { + throw new IllegalStateException("Entity needs to have an ID when JOINs are used!"); + } + if (entityId == null) { + entityId = id; entityInstance = readEntity(row, ctx, null, null); - } else { + } else if (entityId.equals(id)) { + // We want only one entity, everything else should be skipped readChildren(row, entityInstance, null, ctx); } } @@ -328,15 +335,14 @@ public void processRow(RS row) { Object id = readEntityId(row, ctx); if (id == null) { throw new IllegalStateException("Entity needs to have an ID when JOINs are used!"); + } + MappingContext prevCtx = idEntities.get(id); + if (prevCtx != null) { + readChildren(row, prevCtx.entity, null, prevCtx); } else { - MappingContext prevCtx = idEntities.get(id); - if (prevCtx != null) { - readChildren(row, prevCtx.entity, null, prevCtx); - } else { - ctx.entity = readEntity(row, ctx, null, id); - idEntities.put(id, ctx); - allProcessed.add(ctx); - } + ctx.entity = readEntity(row, ctx, null, id); + idEntities.put(id, ctx); + allProcessed.add(ctx); } }