Talent-Catalog / talentcatalog

https://tctalent.org
GNU Affero General Public License v3.0
13 stars 4 forks source link

Move away from using CriteriaAPI for our JPA database queries #55

Open sadatmalik opened 1 year ago

sadatmalik commented 1 year ago

As a developer I need to to move away from using the CriteriaAPI because of its inflexibility, in particular it makes it difficult for us to run stats on the results of saved searches.

A step forward has been delivered in #1160 where the extractSQL method of SearchCandidateRequest produces SQL which replicates the query produced by the CriteriaAPI.

Missing

To completely replace CriteriaAPI with using extractSQL we still need:

sadatmalik commented 1 year ago

JC - "For queries we use the JPA Criteria API. I find it complicated - but that is what we use. I think the alternative of QueryDsl is conceptually simpler - and closer to good old SQL (or the related JPQL) which everyone understands. I just wanted to post my notes on the two alternatives. As I said to Sam, when I asked the developer Dan Zwolenski who did our original port to Spring/Angular why he chose the CriteriaAPI instead of QueryDsl, he said that he preferred it because it didn’t do code generation (eg like Lombok does). Fair enough. I don’t think it is a big issue for us - but good to know the history and that there is an alternative."

John C - " I got my acronyms mixed up. JPQL is NOT what I suggest we should move to - what I suggest we should move to is QueryDsl. The problem with JPQL is that it is just strings. QueryDsl is object based - like CriteriaAPI but much less obscure. I have replaced “JPQL” with “QueryDsl” in all the above discussion - including my reported conversation with Dan Zwolenski. Here is a good comparison - https://spring.io/blog/2011/04/26/advanced-spring-data-jpa-specifications-and-querydsl Our CriteriaAPI code works - so “if it ain’t broke, don’t fix it” is relevant - but it is truly some of the most obscure code in our source - and it sits right at the heart of what we do - searching."

Nother option is JooQ. See https://stackshare.io/stackups/jooq-vs-querydsl

SM - "I do also rather like the intuitive SQL-like api that we would get from QueryDSL, in contrast to CriteriaAPI which tends to be harder to read and can become a bit obscure, to say the least! Also agree that it is a good practice for us as a team - especially as we foray further into open sourcing - to simplify where possible any especially awkward parts of code - particularly if they are central to what we do and areas that we may want to build on in the future. It's a balancing act I guess - keep developing and releasing new functionality while also continuing to make the existing code simpler to understand, maintain, and extend."

sadatmalik commented 1 year ago

Useful links: https://spring.io/blog/2011/04/26/advanced-spring-data-jpa-specifications-and-querydsl https://www.baeldung.com/rest-api-search-language-spring-data-querydsl https://www.baeldung.com/intro-to-querydsl

camerojo commented 5 months ago

Delivered in #1160

Saved Search Search

Add to SavedSearchService a method like...

extractPredicateSQL(savedSearch): String

That would extract the SQL representing the filter parameters in the given savedSearch.

That String could be used to generate the associated search query - or it could also be used to limit results of a stats query based on the results of the search.

CandidateStatsService

Probably should move stats out of the repository - it is messy and has limitations.

Instead construct queries using EntityManager and predicateSQL retrieved as shown above inside a new CandidateStatsService

eg public long getAccountsByPermissionUsingJPQL() throws ParseException { Query query = entityManager.createQuery("SELECT COUNT(*) FROM Account a WHERE a.permission = ?1"); query.setParameter(1, permissionRepository.findByType("admin")); return (long) query.getSingleResult(); } from https://www.baeldung.com/spring-data-jpa-row-count

camerojo commented 5 months ago

Note that you can get the string from a CriteriaAPI query.

See https://stackoverflow.com/questions/6276122/can-i-get-the-sql-string-from-a-jpa-query-object