So then how about the following:
1. Add a multi-valued setting to define various categories of JPA compliance. E.g. `hibernate.jpa.compliance` with multi-selectable values such as: 1. query (strict jpql compliance) 2. txn (transaction handling per spec) 3. close (multiple calls to EMF and EM #close methods) 4. list (no bags) 5. others? 6. all (there should be some form of specifying all) 2. Add @Bag as an explicit declaration of a bag, even if `hibernate.jpa.compliance=list` is specified - that setting just controls how List with no @OrderColumn is interpreted. I vote to delay adding that until 6.0 3. Retain current behavior for "double close" calls unless "close" compliance has been specified. 4. Keep current behavior unless "txn" compliance has been specified On Mon, Nov 27, 2017 at 4:54 AM andrea boriero <and...@hibernate.org> wrote: > On 24 November 2017 at 17:39, Steve Ebersole <st...@hibernate.org> wrote: > >> Andrea, SF is a EMF. Unwrapping simply returns the same instance. >> > > yes but has you pointed out due to the bootstrapping the behaviour of the > SF will be strict JPA compliant. > >> >> Another thing I was discussing with Andrea in chat is possibly making >> these multi-valued, or having multiple values for this. I can't imagine >> the FQN case is really all that appealing to a user. I'm fairly certain a >> user would rather simply say "yeah, treat transactions according the JPA >> spec" as opposed to "here is a class I will provide that will tell will >> treat transactions according to the JPA spec". >> >> We have started to identify some cases where we deviate from the spec[1], >> such as: >> * Strict query compliance. As I mentioned earlier we do have such a >> setting already for this in particular >> * List versus Bag determination from mappings. >> * Closed EMF (SF) handling >> * EntityTransaction status checking - JPA says we should throw exceptions >> whereas we just ignore the call. >> >> We need to decide also which of these we want to just change outright >> versus controlling via a setting. >> >> * Setting >> * Setting, or introduce a new @Bag annotation - the annotation option is >> actually pretty appealing since often times the bag behavior is so >> unexpected from users... >> > > @Bag seems really a good idea to me but that means changing the current > default behaviour, forcing users to change the code, so not sure if we need > also a setting. > > >> * I think we should just change the behavior of calling EMF#close on a >> closed EMF. Any application that happens to be relying on us no-op'ing >> this call can easily change that to protect the call with an `#isOpen` >> check. In fact I think we should change all of these to match the JPA >> expectations such that it is an error to call any of the following: #close, >> #getCache, #getMetamodel, #getCriteriaBuilder, #getProperties, >> #getPersistenceUnitUtil, #createEntityManager. To me these all seem pretty >> reasonable. And in fact I think we used to handle this all properly from >> the EMF side. I think we just lost that behavior when we changed to have >> our contracts extend the JPA ones since we kept the legacy Hibernate >> behavior in SessionFactory. >> > > I do not like the EMF#close behaviour, probably a prefer a separate > setting for this. > > >> * This one I am very undecided. I can see very valid arguments for each. >> > > probably for such case a setting may be a good option. > >> >> [1] we really ought to start keeping a list of these. I have started >> adding them to the migration guide. Just as a list of things we need to >> support configuring or switch to the JPA "way". >> >> On Fri, Nov 17, 2017 at 11:06 AM andrea boriero <and...@hibernate.org> >> wrote: >> >>> I think for 5.3 it's still fine to rely on isJpaBootstrap may be >>> documenting that a SF obtained from unwrapping an EMF will conform to the >>> JPA spec in term of exceptions. >>> >>> On 16 November 2017 at 21:09, Vlad Mihalcea <mihalcea.v...@gmail.com> >>> wrote: >>> >>>> When I said multiple modes, I was thinking of defining all these >>>> situations >>>> In some interface which declares methods like: >>>> >>>> boolean throwsExceptionWhenClosingAClosedEMF() >>>> >>>> The interface can have two implementations for Strict JPA and Native >>>> mode. >>>> >>>> However, the setting could take the FQN of the interface >>>> implementation, so >>>> a user can define those compatibility methods according to their needs. >>>> >>>> E.g. Maybe someone wants the Strict JPA mode but with just 2 >>>> differences; >>>> >>>> - don't throw exception when closing the ENG twice >>>> - use the native Hibernate FlushMode.AUTO instead of the JPA one. >>>> >>>> Vlad >>>> >>>> On 16 Nov 2017 10:49 pm, "Steve Ebersole" <st...@hibernate.org> wrote: >>>> >>>> > There is already a similar setting, although specific to query >>>> language: >>>> > `hibernate.query.jpaql_strict_compliance` - so there is precedence for >>>> > such a solution. >>>> > >>>> > I'm not sure about the "with multiple modes" aspect though. What are >>>> > these other enumerated mode values? >>>> > >>>> > >>>> > On Thu, Nov 16, 2017 at 2:15 PM Vlad Mihalcea < >>>> mihalcea.v...@gmail.com> >>>> > wrote: >>>> > >>>> >> Where the JPA way is questionable, let's add one configuration: >>>> >> hibernate.jpa.compliance with multiple modes: >>>> >> >>>> >> - strict: we do whatever the JPA standard says we should do, like >>>> >> throwing an exception when trying to close the EMF twice >>>> >> - native: we bend the rule where we don't agree with the standard >>>> >> >>>> >> Maybe we should expose all those cases and group them in some >>>> interface >>>> >> to allow the user to customize the level of compliance they need. >>>> >> >>>> >> Vlad >>>> >> >>>> >> On Thu, Nov 16, 2017 at 10:06 PM, Steve Ebersole < >>>> st...@hibernate.org> >>>> >> wrote: >>>> >> >>>> >>> It was added deprecated. Meaning I added it knowing it would go >>>> away >>>> >>> and I wanted to avoid users using it. >>>> >>> >>>> >>> BTW, I am talking about a 5.3 release specifically covering 5.2 + >>>> JPA >>>> >>> 2.2. Yes there is a longer term aspect as well with 6.0 and beyond. >>>> >>> >>>> >>> Its specifically the "where the JPA way is questionable" aspect I am >>>> >>> asking about. Like to me, it really never makes sense to throw an >>>> >>> exception when I close something that is already closed. So how do >>>> we >>>> >>> handle cases like this? >>>> >>> >>>> >>> >>>> >>> On Thu, Nov 16, 2017 at 1:51 PM Vlad Mihalcea < >>>> mihalcea.v...@gmail.com> >>>> >>> wrote: >>>> >>> >>>> >>>> Hi Steve, >>>> >>>> >>>> >>>> I think that for 5.2 was ok to have the isJpaBootstrap method to >>>> avoid >>>> >>>> breaking compatibility for the native bootstrap. >>>> >>>> For 6.0, maybe it's easier if we just align to the JPA spec where >>>> it >>>> >>>> makes sense, >>>> >>>> and only provide a separation where the JPA way is questionable. >>>> >>>> >>>> >>>> I noticed that the isJpaBootstrap method is deprecated. Was it >>>> >>>> intended to be removed in 6.0? >>>> >>>> >>>> >>>> Vlad >>>> >>>> >>>> >>>> On Thu, Nov 16, 2017 at 6:21 PM, Steve Ebersole < >>>> st...@hibernate.org> >>>> >>>> wrote: >>>> >>>> >>>> >>>>> Part of 5.2 was merging the JPA contracts into the corresponding >>>> >>>>> Hibernate >>>> >>>>> ones. So, e.g., we no longer "wrap" a SessionFactory in an impl >>>> of >>>> >>>>> EntityManagerFactory - instead, SessionFactory now extends >>>> >>>>> EntityManagerFactory. >>>> >>>>> >>>> >>>>> This caused a few problems that we handled as they came up. In >>>> >>>>> working on >>>> >>>>> the JPA 2.2 compatibility testing, I see that there are a few more >>>> >>>>> still >>>> >>>>> that we need to resolve. Mostly they relate to JPA expecting >>>> >>>>> exceptions in >>>> >>>>> certain cases where Hibernate has historically been lenient. >>>> E.g., JPA >>>> >>>>> says that calling EntityManagerFactory#close on an EMF that is >>>> already >>>> >>>>> closed should result in an exception. Historically, calling >>>> >>>>> SessionFactory#close on a SF that is already closed is simply >>>> ignored. >>>> >>>>> Philosophical debates aside[1], we need to decide how we want to >>>> handle >>>> >>>>> this situation such that we can throw the JPA-expected exceptions >>>> when >>>> >>>>> needed. Do we simply change SF#close to match the JPA >>>> expectation? >>>> >>>>> Or do >>>> >>>>> we somehow >>>> >>>>> make SF#close aware of JPA versus "native" use? This latter >>>> option >>>> >>>>> was the >>>> >>>>> intent of `SessionFactoryOptions#isJpaBootstrap` and we can >>>> certainly >>>> >>>>> continue to use that as the basis of the solution here for other >>>> cases. >>>> >>>>> >>>> >>>>> This `#isJpaBootstrap` flag is controlled by the JPA bootstrap >>>> code. >>>> >>>>> So if >>>> >>>>> the EMF is created in either of the 2 JPA-defined bootstrap >>>> mechanisms, >>>> >>>>> that flag is set to true. It's an ok solution, but it does have >>>> some >>>> >>>>> limitations - mainly, there was previously a distinction between >>>> >>>>> SF#close >>>> >>>>> being called versus EMF#close being called (they were different >>>> >>>>> classes, so >>>> >>>>> they could react differently). Therefore, regardless of bootstrap >>>> >>>>> mechanism, if the user unwrapped the EMF to a SF, they would >>>> always >>>> >>>>> get the >>>> >>>>> legacy SF behavior. >>>> >>>>> >>>> >>>>> So long story short, so we want to consider an alternative >>>> approach to >>>> >>>>> deciding what to do in "some"[2] of these cases? Again, we >>>> clearly >>>> >>>>> need >>>> >>>>> these to throw the spec-mandated exceptions in certain "strict >>>> >>>>> compliance" >>>> >>>>> situations. The question really is how to do that. Should we: >>>> >>>>> >>>> >>>>> 1. just completely change the behavior to align with the spec? >>>> >>>>> 2. change the behavior to match the spec *conditionally*, >>>> where that >>>> >>>>> condition could be: >>>> >>>>> 1. `#isJpaBootstrap` >>>> >>>>> 2. some setting >>>> >>>>> 3. some extension contract >>>> >>>>> 4. something else? >>>> >>>> >>>> >>>> >>>> >>>>> >>>> >>>>> Thoughts? >>>> >>>>> >>>> >>>>> >>>> >>>>> [1] It's not relevant e.g. that I think JPA is wrong here. We >>>> need to >>>> >>>>> comply with the spec, at least in certain cases ;) >>>> >>>>> >>>> >>>>> [2] I say "some" here, because I think the spec is correct in some >>>> >>>>> cases - >>>> >>>>> for example, I think its clearly correct that a closed EMF throws >>>> an >>>> >>>>> exception when `#createEntityManager` is called. Personally I >>>> think >>>> >>>>> its >>>> >>>>> questionable whether closing an already closed EMF should be an >>>> >>>>> exception. >>>> >>>>> >>>> >>>> _______________________________________________ >>>> >>>>> hibernate-dev mailing list >>>> >>>>> hibernate-dev@lists.jboss.org >>>> >>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>> >>>>> >>>> >>>> >>>> >>>> >>>> >> >>>> _______________________________________________ >>>> hibernate-dev mailing list >>>> hibernate-dev@lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>> >>> >>> _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev