Add findByPublisher query

lawynnj commented 3 years ago

Summary of change

Add fetchByPublisher method to book repository data access model. Add @DataJpa test for fetchByPublisher.

Related issue

Closes #57

Pull request checklist

Please keep this checklist in & ensure you have done the following:

[x] Read, understood and adhered to our contributing document.
- [x] Ensure that you were first assigned to a relevant issue before creating this pull request
- [ ] Ensure code changes pass all tests
[x] Read, understood and adhered to our style guide. A lot of our code reviews are spent on ensuring compliance with our style guide, so it would save a lot of time if this was adhered to from the outset.
[x] Filled in the summary, context (if applicable) and related issue section. Replace the square brackets and its placeholder content with your contents. For an example, see any merged in pull request
[x] Created a branch that has a descriptive name (what your branch is for in a few words and includes the issue number at the end, e.g. test-reading-goal-123
[x] Set this pull request to 'draft' if you are still working on it
[x] Resolved any merge conflicts

For any of the optional checkboxes (e.g. the screenshots one), still check it if it does not apply.

If in doubt, get in touch with us via our Slack workspace or by creating a new Q&A discussion on GitHub

knjk04 commented 3 years ago

@lawynnj How are you getting on with this?

lawynnj commented 3 years ago

@knjk04 It's still in progress. I will be adding a test shortly

knjk04 commented 3 years ago

@lawynnj Apologies for the delay, this slipped my mind. I've added the patch (#106). If you bring your branch level with main, this should no longer be an issue. If it is, let us know

lawynnj commented 3 years ago

@knjk04 I've added the @DataJpa test. The test I added passes locally, but fails in the Azure pipeline. I'm not familiar with the Java/Azure ecosystem at all so I might take some time identifying what the issue is.

This is where the failure occurs when analyzing the gradle output (it looks like pre-existing tests also fail):


> Task :test

BookRepository should > find by publisher FAILED
    org.springframework.dao.DataIntegrityViolationException at BookRepositoryTest.java:137
        Caused by: org.hibernate.exception.ConstraintViolationException at BookRepositoryTest.java:137
            Caused by: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException at BookRepositoryTest.java:137

BookRepository should > find book by title case insensitive FAILED
    org.springframework.dao.DataIntegrityViolationException at BookRepositoryTest.java:178
        Caused by: org.hibernate.exception.ConstraintViolationException at BookRepositoryTest.java:178
            Caused by: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException at BookRepositoryTest.java:178

BookRepository should > find saved books FAILED
    org.springframework.dao.DataIntegrityViolationException at BookRepositoryTest.java:68
        Caused by: org.hibernate.exception.ConstraintViolationException at BookRepositoryTest.java:68
            Caused by: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException at BookRepositoryTest.java:68

BookRepository should > find book with isbn FAILED
    org.springframework.dao.DataIntegrityViolationException at BookRepositoryTest.java:85
        Caused by: org.hibernate.exception.ConstraintViolationException at BookRepositoryTest.java:85
            Caused by: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException at BookRepositoryTest.java:85

BookRepository should > find by author FAILED
    org.springframework.dao.DataIntegrityViolationException at BookRepositoryTest.java:115
        Caused by: org.hibernate.exception.ConstraintViolationException at BookRepositoryTest.java:115
            Caused by: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException at BookRepositoryTest.java:115

BookRepository should > findBookByTitle() FAILED
    org.springframework.dao.DataIntegrityViolationException at BookRepositoryTest.java:158
        Caused by: org.hibernate.exception.ConstraintViolationException at BookRepositoryTest.java:158
            Caused by: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException at BookRepositoryTest.java:158
2021-05-25 06:34:00.061  INFO 2001 --- [extShutdownHook] j.LocalContainerEntityManagerFactoryBean : Closing JPA EntityManagerFactory for persistence unit 'default'

37 tests completed, 6 failed

> Task :test FAILED
9 actionable tasks: 8 executed, 1 up-to-date

Before I do a deep dive into this, have you encountered this before? Do you have any suggestions?

knjk04 commented 3 years ago

@lawynnj I've come across that exception before. If I remember correctly off the top of my head, it may be due to an annotation that we have (so a constraint) that is being violated (e.g. @NotNull)

lawynnj commented 3 years ago

@knjk04 I inspected the previous builds and it looks like this error also occured in this PR https://github.com/Project-Books/books-api/pull/106, thus the changes introduced in this PR are not a direct cause of the error. I think we should create a separate issue to investigate and fix the PR builds failing. What do you think?

In the mean time, here are some potential causes:

The azure pipeline configuration is not set up to run the database during testing
The database is configured and running, but is not seeded

knjk04 commented 3 years ago

@lawynnj Sure, that sounds reasonable. I'll test this locally when you're ready for a review

lawynnj commented 3 years ago

@knjk04 It's ready for review. The tests are passing locally when running ./gradlew test

knjk04 commented 3 years ago

Thanks for working on this!

The tests pass on my PC, but fail on my laptop. See https://gist.github.com/knjk04/f72a94054bd6e44da147846f50528412

I'm using Oracle JDK 11.0.8 on Windows. Are you able to reproduce this?

knjk04 commented 3 years ago

@lawynnj Following up on my previous question

lawynnj commented 3 years ago

@knjk04 What OS and JDK versions do you have on your PC? Are the JDK version on your laptop and PC the same?

It's working on my machine: openjdk 11.0.11 2021-04-20 OpenJDK Runtime Environment (build 11.0.11+9-Ubuntu-0ubuntu2.18.04) OpenJDK 64-Bit Server VM (build 11.0.11+9-Ubuntu-0ubuntu2.18.04, mixed mode, sharing)

knjk04 commented 3 years ago

@lawynnj Apologies for the late reply. I'll get back to you on this

knjk04 commented 2 years ago

@lawynnj I'm so sorry, this slipped my mind. I'll merge this in. Thanks a lot!

Project-Books / books-api