micronaut-projects / micronaut-data

Ahead of Time Data Repositories
Apache License 2.0
465 stars 197 forks source link

An association is not fetched when using findOne(QuerySpecification) with a Data JDBC repository annotated with @Join #1690

Closed ex-orc closed 2 years ago

ex-orc commented 2 years ago

Expected Behavior

given two entities, with one referencing the other using a @OneToOne relation:

@Entity
public class Genre {
    @Id
    @GeneratedValue
    private Long id;
    private String name;

    // getters and setters are skipped for brevity
}

@Entity
public class Book {
    @Id
    @GeneratedValue
    private Long id;
    private String title;
    @OneToOne(fetch = FetchType.EAGER)
    private Genre genre;

    // getters and setters are skipped for brevity
}

and an empty JDBC repository annotated with @Join

@JdbcRepository(dialect = Dialect.H2)
@Join(value = "genre", type = Join.Type.LEFT_FETCH)
public interface BookRepository extends CrudRepository<Book, Long>, JpaSpecificationExecutor<Book> {
}

when a book is retrieved using findOne(PredicateSpecification) or findOne(QuerySpecification)

var bookLoadedWithCriteriaApi = bookRepository.findOne((root, criteriaBuilder) -> criteriaBuilder.and()).orElse(null);

then the referenced genre object is expected to be populated because of the @Join annotation on the BookRepository above.

In reality, the genre object is not populated, only the id is set correctly:

    @Test
    void criteriaApiJoin() {
        var genre = new Genre();
        genre.setName("dystopia");
        genreRepository.save(genre);

        var book = new Book();
        book.setTitle("1984");
        book.setGenre(genre);
        bookRepository.save(book);

        var bookLoadedUsingFindAll = bookRepository.findAll().iterator().next();
        var bookLoadedWithCriteriaApi = bookRepository.findOne((root, criteriaBuilder) -> criteriaBuilder.and()).orElse(null);

        assertNotNull(bookLoadedUsingFindAll.getGenre().getName()); // works
        assertNotNull(bookLoadedWithCriteriaApi.getGenre().getName()); // fails!!!
    }

Actual Behaviour

the referenced object is not populated

Steps To Reproduce

create two entities and an empty repository:

@Entity
public class Genre {
    @Id
    @GeneratedValue
    private Long id;

    private String name;

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

@Entity
public class Book {
    @Id
    @GeneratedValue
    private Long id;
    private String title;
    @OneToOne(fetch = FetchType.EAGER)
    private Genre genre;

    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 Genre getGenre() {
        return genre;
    }

    public void setGenre(Genre genre) {
        this.genre = genre;
    }
}

@JdbcRepository(dialect = Dialect.H2)
@Join(value = "genre", type = Join.Type.LEFT_FETCH)
public interface BookRepository extends CrudRepository<Book, Long>, JpaSpecificationExecutor<Book> {
}

and run the following test:

    @Test
    void criteriaApiJoin() {
        var genre = new Genre();
        genre.setName("dystopia");
        genreRepository.save(genre);

        var book = new Book();
        book.setTitle("1984");
        book.setGenre(genre);
        bookRepository.save(book);

        var bookLoadedUsingFindAll = bookRepository.findAll().iterator().next();
        var bookLoadedWithCriteriaApi = bookRepository.findOne((root, criteriaBuilder) -> criteriaBuilder.and()).orElse(null);

        assertNotNull(bookLoadedUsingFindAll.getGenre().getName()); // works
        assertNotNull(bookLoadedWithCriteriaApi.getGenre().getName()); // fails!!!
    }

The test will fail.

Environment Information

No response

Example Application

No response

Version

3.6.0

dstepanov commented 2 years ago

I did a small debugging, it looks like joinPaths aren't set in SqlResultEntityTypeMapper when the criteria are invoked.

I suggested on the chat to us this:

            ((PersistentEntityFrom) root).join("genre", io.micronaut.data.annotation.Join.Type.LEFT_FETCH);

To specify join fetch of the entity as I'm not sure ordinary join is enough.

BTW We also miss JPA Criteria fetch API's implemented; for now, that should be the correct way to join an association that should also be fetched - included in the resulting entity.

ex-orc commented 2 years ago

Thank you, but I tried this, and it didn't work. See here: https://gitter.im/micronautfw/questions?at=62f3c4f0647d633cf649b88e

Also, calling the join() method feels superfluous as we already have @Join annotation which works fine with the no-arg findAll() method. So I guess your suggestion is more like a temporary fix, which would be nice to get rid of in one of the following Micronaut releases.

radovanradic commented 2 years ago

Using predicate specification like Denis suggested

public static PredicateSpecification<Book> joinGenre() {
        return (root, criteriaBuilder) -> {
            ((PersistentEntityFrom) root).join("genre", io.micronaut.data.annotation.Join.Type.LEFT_FETCH);
            return null;
        };
    }

will be available as a workaround/solution for this issue after 3.7.3 micronaut-data release

ex-orc commented 2 years ago

@dstepanov @radovanradic Thank you for providing a workaround! But I think that the proper fix for this issue is to make sure that findAll(QuerySpecification) respects the @Join annotation in the same way as the no-argument findAll(), without requiring the above predicate specification with this explicit typecast.

dstepanov commented 2 years ago

Please create a feature request

ex-orc commented 2 years ago

@dstepanov But that feature request would be an exact copy of this issue! :) I even provided a test that fails due to the incorrect behaviour, and I believe it will still fail in 3.7.3 because the test doesn't contain the line with the workaround. Can't we just reopen this case and fix it so that the provided test works without a workaround?

radovanradic commented 2 years ago

@ex-orc Thanks for the feedback. We have added additional fix that should resolve issues you reported and your tests should pass now.