Improvements/addons to Hibernate with Panache

[Loïc MATHIEU]

Hello,

I wanted to make some improvements/addons to Hibernate with Panache regarding opened issues and some other ideas.

For the moment, I add all those improvements in a single branch so you can check the result (WIP)

Here are the list of what I wanted to add: https://github.com/quarkusio/quarkus/compare/master...loicmathieu:feat/panache-improvements

1. Optional Support:
Issues: https://github.com/quarkusio/quarkus/issues/3967 and  https://github.com/quarkusio/quarkus/issues/1368
Proposed implementation:

PanacheEntity.maybeFind/findOptionaly(Object id) 

Or we can change findById() to return an optional and use getOne/findOneById() to return the Entity or null, this will be a breaking change but is more understandable

2. LockModeType support: 

Issue: https://github.com/quarkusio/quarkus/issues/2744
Proposed implementation:

PanacheQuery.withLock(LockModeType lockModeType);


3. Query hint: 

Issue: parts of https://github.com/quarkusio/quarkus/issues/3483
Proposed implementation:

PanacheQuery.withHint(String hintName, Object value);

We can also add a PanacheQuery.withFlushMode(FlushModeType flushModeType) that will complete the coverage of what can be done on a JpaQuery from PanacheQuery.

4. Range query : 

Issue: https://github.com/quarkusio/quarkus/issues/3870
Proposed implementation:

PanacheQuery.range(int startIdx, int lastIdx)


5. JPA Query: 

Issue: parts of https://github.com/quarkusio/quarkus/issues/3483

Proposed implementation:
PanacheEntity.find(Query query, String... params)
PanacheEntity.find(Query query, Map<String, Object> params)
PanacheEntity.find(Query query, Map<String, Parameters params)
Same for list/stream/delete/count methods
An other implementation will be to support named queries (via @NamedQuery), the implementation could be to lookup if the query string is equals one of the registered named query before using it ...
Or we can do both ;)


6. Specification/Example: 

Issues: https://github.com/quarkusio/quarkus/issues/3965 and part of https://github.com/quarkusio/quarkus/issues/2303
Person p = new Person();
p.lastName = "Mathieu";
PanacheQuery<Person> mathieus = Person.findByExample(Example.of(p));

The implementation can use Example or Specification semantic, or we can directly write Person.findByExample(p) if we don't care to enforce a semantic by having a carrier Example/Specification class.
PanacheEntity.findByExample(Example example)
Same for list/stream/delete/count methods

So, what do you think of it ?

I can open a PR with the current branch I'm working on ... or 6 different PR to be able to discuss each point separatly.

Or maybe the best is to open an umbrella issue with all those 6 proposals ...

[Stephane Epardaud]

This all sounds great, but I think we should wait until after 1.0 is release as we're too busy ATM to properly look at that.

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAJLxjVGwYDCXE2f0QH-%3Df92LeHt27Z%2Bv%2BnrfdQUd7ddrxnxrzA%40mail.gmail.com.

--

Stéphane Épardaud

[Loïc MATHIEU]

OK, at least the code is somewhere :)

Maybe opening an umbrella issue with this content to avoid forgetting it ?

[Stephane Epardaud]

Sure, thanks.

--

Stéphane Épardaud

[Loïc MATHIEU]

https://github.com/quarkusio/quarkus/issues/4809

The only one that bothers me is the Optional case for findById. I really think we should break the compatibility and moving the current findById(1) to return an Optional instead of the entity.

As it's a breaking change if we agree on it we must do it before 1.0

[George Gastaldi]

That makes sense. +1 to break sooner than later.

Best Regards,

George Gastaldi

Principal Software Engineer

Red Hat

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAJLxjVEeF0euJk%3DS3gEruoa0C85es3%2BHg4_36mDKutkYXAKrqg%40mail.gmail.com.

[Stephane Epardaud]

I'm really not sold on findById returning an Optional, I'd rather this be another method.

--

Stéphane Épardaud

[Loïc MATHIEU]

It's the way Spring Data do it:

Optional<Peson> o = Person.findById(1);

Person p = Person.findOneById(1);

But we can also do

Optional<Peson> o = Person.findOptionalyById(1);

//or

Optional<Peson> o = Person.maybeFindById(1);

Person p = Person.findById(1);

If we choose not to change the current behaviour, no hurry. But I will sponsor a change of behaviour because I don't like maybeFindById() not findOptionalyById() but if there is a better challenger I can recall my vote ;)

[Emmanuel Bernard]

I have specific better plans for query by example I'd like to use (or invalidate) before going the traditional way. It is discussed in a GitHub issue somewhere.

[Emmanuel Bernard]

yes two methods are likely safer.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKU9E9vqVJk7%3DWsOZNgjfBPirg%3D_n%3D_aswsRYAbFRbAHDDoYxg%40mail.gmail.com.

[Emmanuel Bernard]

I don't see a strong argument to break existing code for the Spring way of naming things. They don't feel significantly clearer than your proposals (actually the opposite).

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAJLxjVH4cGazN_G%2B_bmZhke9KCS20X8F%3DPHyd4W7mMAv8fWGaA%40mail.gmail.com.

[Georgios Andrianakis]

I agree, I don't see any real advantage of the Spring Data way of doing things in this case.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/B8D1AC52-F922-4ADE-9633-4860E6CA5EDA%40redhat.com.

[Loïc MATHIEU]

Hello,

Now that 1.0 is on track I would like to re-open discussions for these improvements / addons to Panache.

A lot of the corresponding issues are being labelled as stale by the bot as they was on-hold regarding decision to implements them or not waiting for 1.0 to be released ...

So please, can you give guidance on the following umbrella issue wether or not some of these improvements would be consider for inclusion: https://github.com/quarkusio/quarkus/issues/4809

I have implementation ready for almost all the corresponding improvements :) I can batch them or send them one by one regarding the decision to include them or not ...