It seems fine to me and I agree to delay the @Bag to 6.0. On 27 November 2017 at 21:29, Steve Ebersole <st...@hibernate.org> wrote:
> 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