Skip to content

Conversation

@jojoldu
Copy link

@jojoldu jojoldu commented Jul 19, 2020

Hi @eXsio

QClass created with Entityql as below will fail when using SQLInsertClause.populate().

    @Test
    void sqlPopulateInsert() throws Exception {
        //given
        Book entity = Book.builder()
                .bookNo(1)
                .bookType(BookType.IT)
                .build();

        //when
        sqlQueryFactory.insert(qBook)
                .populate(entity, BeanMapper.DEFAULT)
                .execute();

        //then
        List<Book> results = bookRepository.findAll();
        assertThat(results).hasSize(1);
        assertThat(results.get(0).getId()).isEqualTo(1L);
        assertThat(results.get(0).getBookType()).isEqualTo(BookType.IT);
    }

This is the insert query I intended,

insert into book (book_type, book_no) values (?, ?)

Actually, it works like this:

insert into book values ()

The reason was that the property key and the column name were different, and there was an issue that could not be recognized.

In BeanMapper supported by Querydsl-sql, the property key and the column name had to match.

This problem is also recognized in Querydsl-sql, so AnnotationMapper is supported.
However, AnnotationMapper uses the @column annotation of querydsl, so it cannot be used in QClass created with Entityql.

So I created a Mapper class that uses JPA's @column annotation.
I changed the test code above to EntityMapper and it worked fine.

Below is a link to the code I tested.

link

I'll ask for confirmation.

@codecov
Copy link

codecov bot commented Jul 19, 2020

Codecov Report

Merging #5 (248f5e4) into master (6532fd5) will decrease coverage by 4.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #5      +/-   ##
============================================
- Coverage     92.76%   88.75%   -4.01%     
- Complexity      300      344      +44     
============================================
  Files            27       36       +9     
  Lines           677      907     +230     
  Branches         37       56      +19     
============================================
+ Hits            628      805     +177     
- Misses           36       83      +47     
- Partials         13       19       +6     
Impacted Files Coverage Δ Complexity Δ
...l/exsio/querydsl/entityql/mapper/EntityMapper.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ain/java/pl/exsio/querydsl/entityql/QExporter.java 81.95% <0.00%> (-15.19%) 9.00% <0.00%> (+5.00%) ⬇️
.../java/pl/exsio/querydsl/entityql/QPathFactory.java 94.91% <0.00%> (-2.06%) 28.00% <0.00%> (+2.00%) ⬇️
...main/java/pl/exsio/querydsl/entityql/QFactory.java 100.00% <0.00%> (ø) 13.00% <0.00%> (+3.00%)
...n/java/pl/exsio/querydsl/entityql/QJoinColumn.java 87.93% <0.00%> (ø) 23.00% <0.00%> (ø%)
...exsio/querydsl/entityql/ex/MissingIdException.java 100.00% <0.00%> (ø) 2.00% <0.00%> (+1.00%)
...ntityql/entity/scanner/runtime/QRuntimeColumn.java 100.00% <0.00%> (ø) 1.00% <0.00%> (?%)
.../entityql/entity/scanner/QRuntimeTableScanner.java 100.00% <0.00%> (ø) 7.00% <0.00%> (?%)
...entityql/entity/scanner/runtime/QRuntimeTable.java 50.00% <0.00%> (ø) 1.00% <0.00%> (?%)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6532fd5...248f5e4. Read the comment docs.

Copy link
Owner

@eXsio eXsio left a comment

Choose a reason for hiding this comment

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

Hello, Thank you for the contribution!

Would you be so kind to provide an automated test that proves this code works and shows how to use it?

Please make sure that the overall code coverage is not decreased.

@jojoldu
Copy link
Author

jojoldu commented Jul 19, 2020

@eXsio
OK.
I will write the test code as soon as possible and commit it.

@jojoldu
Copy link
Author

jojoldu commented Jul 20, 2020

@eXsio Hello.
Added a test case where an issue occurs.

A successful test case using EntityMapper added as PR was also added.

Please confirm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants