> 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.
Mentioning this just in case: the javadoc for the AutoCloseable interface recommends making implementations of #close idempotent (not throwing exceptions when called twice). Since Session extends AutoCloseable, and IIRC there was a ticket about making EntityManager also extend AutoCloseable, I thought this would be relevant. I personally think that such a behavior is nice as it makes resource management a little bit simpler, but in any case the interface does not mandate it: it's just a recommendation. So you sure can do what you want. Yoann Rodière Hibernate NoORM Team yo...@hibernate.org On 24 November 2017 at 18:42, Steve Ebersole <st...@hibernate.org> wrote: > I should have said: > > 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". ***Or have multiple settings > following how I started with `hibernate.query.jpaql_strict_compliance`*** > > On Fri, Nov 24, 2017 at 11:39 AM Steve Ebersole <st...@hibernate.org> > wrote: > > > Andrea, SF is a EMF. Unwrapping simply returns the same instance. > > > > 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... > > * 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. > > * This one I am very undecided. I can see very valid arguments for each. > > > > [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 > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev