Spring-boot: Log a warning on startup when spring.jpa.open-in-view is enabled but user has not explicitly opted in

Created on 5 Oct 2016  ·  96Comments  ·  Source: spring-projects/spring-boot

Considering OSIV/OEMIV is widely considered an anti-pattern, OpenEntityManagerInViewInterceptor should IMO not be enabled by default. Rather than that it should be opt-in.

If this proposal isn't accepted at the very least the default behavior should be stressed properly in the documentation. Currently the only reference is in configuration properties appendix.

enhancement

Most helpful comment

I agree. I think we should change the default in 2.0.

All 96 comments

I agree. I think we should change the default in 2.0.

Closing in favour of the PR (#7125)

Considering OSIV/OEMIV is widely considered an anti-pattern, …

Would anyone care to elaborate? I definitely see it's a controversial topic, but I think that "widely considered anti-pattern" is quite a bit of a stretch. It had been an anti-pattern in the days (< Spring 2.0) when O(S|E)IV automatically meant opening a transaction, too, but whenever I see someone (esp. from the CDI camp) arguing O(S|E)IV is a bad idea, the next step that usually follows is exposing a request scoped EntityManager which is _exactly_ the same thing with the same advantages and problems.

So I'd love to learn what's wrong with the default setting? Judging from the projects I see, I guess everyone would then just turn it on manually.

IMO if you have a transactional unit-of-work whose result of execution requires further fetching from database that occurs after the aforementioned unit-of-work has ended I don't know how adding behavior that supports such data access pattern can be described not being an anti-pattern. Same for not having well defined transactional boundaries. Anyway not to go any further here my thoughts on this topic have been covered in @vladmihalcea's blog post better than I could express them myself. I respect that everyone has their preferences when it comes to data access but I cannot agree this is not an anti-pattern.

So I'd love to learn what's wrong with the default setting? Judging from the projects I see, I guess everyone would then just turn it on manually.

Well, my experience is just the opposite. :smile:

What's wrong with the current default settings is that adds behavior that's too opinionated to address the complex subject of lazy loading of associations. Even worse, it does so completely silently, because as described in my original comment, its only mention is in the configuration properties appendix. I'd also argue this is bad for novice developers as it completely hides lazy loading concerns from them, rather then have them face such concerns and decide how to address them after educating on the topic.

If someone wants OSIV, they'll enable it with a single configuration property first time they face the LazyInitializationException.

IMO if you have a transactional unit-of-work whose result of execution requires further fetching from database that occurs after the aforementioned unit-of-work has ended I don't know how adding behavior that supports such data access pattern can be described not being an anti-pattern.

I can't really follow the sentence here. That's circular reasoning, isn't it? Because you describe it as anti-pattern you can't see how it cannot be described as such. Hm.

I just re-read Vlad's blog and still remain unconvinced. Actually, I agree with everything he writes there. Still, there's nothing that inherently _breaks_ code when an O(S|E)IVF is in place. Yes, its usage can cause suboptimal query performance and all other kinds of suboptimal effects if you don't know what you're doing. The thing is: if you start to argue that way, you basically have to shun JPA entirely. Yes, it's quite easy to suboptimally configure mappings, query executions etc. Still, I'd fix those problems when they actually occur to become some.

Quite the opposite picture without an O(S|E)IVF: fundamental things you naively (read: not being a JPA/Hibernate expert like Vlad, you, me (?)) expect to work — like traversing a property (e.g. while rendering a view) — will just stop working. Now you can of course go ahead and raise the experts finger, tell people to read Vlad's blog and then do the "right thing"™. I'd argue most people will rather do one of two things: activate O(S|E)IVF again and just live with the slight decrease in performance, or — even worse — rather extend their transaction boundaries as this might seem as the even more easy thing to do. I mean, why would you use an OR mapper if not for convenience. If you want to optimize the heck out of your relational data access, you're probably down to SQL anyway.

I personally have never seen any real problems being caused by the EntityManager (and thus the connection) being held open during view rendering. If that really becomes a performance problem: there's an easy way to turn that off. Not being able to traverse a model in the view and being forced into DTOing every entity is a much more cumbersome default. Note that one of Boot's core goals is to provide a great developer experience. Following the "but it might not be the most optimal way of working", we would never have started with things like Spring Data (JPA), as you can clearly create non-optimal queries OOTB if you're not deeply into the store characteristics.

I don't think O(S|E)IV is too opinionated, as it's not the thing that imposes the opinion. _JPA_ does in the first place. O(S|E)IV just causes fundamental things to continue to work as non-JPA experts expect them to work. I am not even saying O(S|E)IV is an optimal solution but the current setup is trading a working and potentially suboptimal OOTB experience over an experience that requires very deep understanding and looks like it's not working OOTB.

So I end up wondering what real benefit hanging the default has I am not aware of any real problems stemming from the current default that have been brought up here and we'd basically just cater a different opinion, with the very likely chance that if the default was changed, the other camp would just again ask for it to be reverted again.

To use or not to use OSIV is an architectural decision, and, in the good spirit of DevOps, the DBA must be involved when making all these decisions because OSIV can become a performance issue only in a production environment, not on a developer machine.

From a developer perspective, OSIV is very attractive since it can boost productivity. However, if you're not involved in tuning the enterprise system and scaling it with the ever increasing incoming throughput, you'll have a very optimistic view on OSIV or temporary Session anti-pattern.

Both Spring and Hibernate offer some opinionated options, but, in the end, it is the responsibility of the DevOps team to decide whether a given feature makes sense for their particular system.

Therefore, this option should be explicitly activated.

@olivergierke I'm sorry but your take on this with statements like _if you start to argue that way, you basically have to shun JPA entirely_ sounds like it's all or nothing with JPA. IMO where it (JPA/Hibernate) excels the most are the write scenarios, persisting complex graphs with added value in features like locking mechanisms. OTOH with reads you have so many options to choose from, and I (as well as many others) prefer to be explicit with what I fetch and how exactly I fetch it from the database. Such things should IMO not be dictated by your mappings.

Also I have a problem with statement that _If that really becomes a performance problem: there's an easy way to turn that off_ - you won't get yourself out of that kind of a problem by simply turning off OSIV. If you had relied on OSIV while developing your app and you have to turn it off at some point down the road, that will almost certainly require a fair share of refactoring of your data access related code.

What I was trying to get across that JPA is already a trade-off. A quite huge one, especially favoring developer convenience over performance / efficiency (as otherwise you'd use more low-level technology like SQL in the first place). So blaming O(S|E)IV for producing suboptimal results _in some cases_ is quite a stretch.

In case you find any of the effects O(S|E)IV are really becoming a problem in your application, you can switch of the defaults, still use a manually configured O(S|E)IV bound to the paths that just work fine with it and tweak the problematic scenarios. Heck, depending on what problem you really run into, you can still use Vlad's great advice to optimize mappings and queries even with the O(S|E)IV present.

I'd always rather act on problems when they appear rather than hastily DTOing my entire codebase in anticipatory obedience. Isn't the latter a great example of premature optimization? Also, always remember that with the current setup there's nothing preventing you from doing all the things you'd like. We solely argue OOTB developer experience, a default, and I'd argue that this should be less invasive to the overall code design than what it'd be if what's suggested is applied.

I read the whole thread and I can only agree that OSIV is an anti-pattern.

My point here is not about performance but about architectural design. I mean it's called Open Session in View for a reason, right? Not only do we handle database-related stuff in the view layer - which is wrong from a conceptual point of view, what happens if an exception happens while the response stream is being written? The page has only partially being served to the client so the stack trace gets written here as well... There's no way to handle nicely it.

That's the original sin of OSIV: it breaks in a very bad way one of the basics of software development - separation of concerns.

(Bragging rights: I might even have been one of the first to describe OSIV as an anti-pattern as such back in 2010 when it was all "it's described in the Hibernate book so you're plain wrong").

My 2 cents.

…what happens if an exception happens while the response stream is being written?

How is that an O(S|E)IV specific problem? Exceptions can be caused by anything.

That's the original sin of OSIV: it breaks in a very bad way one of the basics of software development - separation of concerns.

That feels quite constructed to me. Following that train of thought, you can argue that an @Transactional is violating this concern as well as you mix a technical thing with your business logic. It feels like you're trying to decouple for the reason of decoupling, and by that actually create incentives for users to create much more technical code than probably necessary. At some point you have to connect things to things, don't you?

So again, what hazardous problems are caused by _O(S|E)IV being the default_, that are introduced by O(S|E)IV actually, and not some upstream technology decision? I can probably stop repeating that I totally see the potential downsides of using it 🙃. I just think the benefits of letting it be the default — again, nobody's forcing anyone to actually use it — introduces totally outweigh the drawbacks that could occur under certain conditions.

@olivergierke I don't think that _hastily DTOing my entire codebase in anticipatory obedience_ was anywhere suggested as preferred approach so I don't see why it's necessary to go to extremes like that, similarly as with _you basically have to shun JPA entirely_.

Speaking of invasive, IMO if anything's invasive it's enabling OSIV by default. With _off_ by default at first sight of LazyInitializationException, and that will happen early in your development lifecycle, you'll be faced with a decision whether to add a single config property to enable OSIV or not. Otherwise you're hiding away an important aspect of underlying peristence technology and architectural decision, and by doing so increase the chances it will take some time for someone will realize things don't work exactly as they thought they do.

Following that train of thought, there would never be an argument for activating things by default. You're basically arguing that we should enable JPA and effectively leave the user with property traversal on domain objects being broken.

…what happens if an exception happens while the response stream is being written?
How is that an O(S|E)IV specific problem? Exceptions can be caused by anything.

The exception is not the point, the fact that it cannot be handled cleanly is. If an exception occurs in one of the "deep" layers, it can bubble up to an exception handler. If the same occurs while writing the response stream, it cannot as part of the page as already been committed.

And that problem can only appear when using an O(S|E)IV? 😳

And that problem can only appear when using an O(S|E)IV? 😳

I let you check the logical fallacies list.

I don't understand why this issue is so important that different valid arguments from different people get just discarded so easily. I've said my piece, others as well. As far as I understand, the decision has been made already?

I don't understand why this issue is so important…

Context is key: this ticket asks for a different default in O(S|E)IV filter, hence we collect arguments for that. That an exception during the rendering phase of a view is a general problem, totally unrelated to whether O(S|E)IV is enabled by default or not, right?

We're getting drawn into arguments of the consequences of using or not using O(S|E)IV. But that's not really the context of this ticket, is it? The default will not decide whether people ultimately end up using a certain approach or not. It's about whether the current default is in line with Boot's goals.

OSIV is the "blue pill". You give it to Spring Boot users, and, by the time they realize that hiding the LazyInitializationException is causing performance issues, it's going to take a lot of effort to switch from OSIV to fetch data on a per-business use case basis.

However, I understand why you'd enable OSIV by default. The easier to use, the more adoption you'll get for Spring Boot.

We could have done the same with the hibernate.enable_lazy_load_no_trans configuration property. We could just enable it by default and make Hibernate behave like EclipseLink - hiding the LazyInitializationException under the carpet.

But we didn't, and that hasn't affected Hibernate popularity.

For JPA and Hibernate consultants, enabling OSIV by default in Spring Boot is actually a gift because they will have more opportunities for performance tuning Spring/Hibernate applications.

So, it's indeed a tough decision.

After a lot (and I do mean a lot) of discussion we've decided to leave things as they are. The primary reason is that people upgrading are likely to face very subtle issues that only manifest themselves in certain circumstances.

If we were starting from a clean slate we may well have picked a different default, but we think the pain of changing the default (even at a major release) is going to cause more bugs than leaving things as they are.

Well, this is disappointing but it is what it is. Not sure I agree with the argument that it'll cause more bugs since requests that benefited from OEMIV will start failing hard with LazyInitializationException once you disable it.

I've opened #7944 to improve the documentation and emphasize the default behavior.

Just a relevant story from the trenches: I was recently hit by this issue when JPA entitites where unexpectedly cached between @Transactional service operations due to the thread-bound entity manager introduced by this interceptor.

I'm adding this article which is relevant to the caveats of OSIV. The article is in Czech, but Google Translate does a good job to translate it to English.

I think that we should also remember that currently, Spring Boot is often a starting point for people learning Java, especially on the enterprise level. Enabling OSIV by default shows them it's a good solution, that should be used. It's similar to mocking static or private methods. At the beginning, people consider it as a limitation, but as we know it's generally not true - it just enforces you to follow god design practices. The same situation is with OSIV.

I agree there should be the easy way to enable it, but IMHO it should be disabled by default, to show users Spring Boot is not promoting it.

Also, there won't be better moment to change it than version 2.0

@jkubrynski this issue is closed. Please see https://github.com/spring-projects/spring-boot/issues/7107#issuecomment-271709902

Sorry @snicoll, I asked @jkubrynski to comment on this issue.

Since this conversation took off again on Twitter I'm reopening this to triple check if we're doing the right thing.

Perhaps we can use a failure analyzer to report on the LazyInitializationException with advice.

That's not happening on startup.

What you can do is to extend PersistenceExceptionTranslator to catch LazyInitializationException and check if there is DispatcherServlet in the stack trace, and then log information that you can walk around this issue by setting spring.jpa.open-in-view to true.

Migration to major version is (probably) the only moment when people care about reading the migration guide. Also as far as I remember there are already some defaults that are going to change in 2.0.

@vladmihalcea from the Hibernate core team complains about this approach, so please consider promoting good design as a default :)

First of all thanks @philwebb for giving this one more round of consideration.

A lot of good arguments for dropping OSIV as the default have been outlined by many in this thread while the main argument for staying with the current default seems to be the concern for migration/backward compatibility.

I'd just like to stress that I find it hard to believe one could miss the occurrence of LazyInitializationException during testing phase (which should certainly be done thoroughly when you upgrade to a new major version of a part of your technology stack) and have it slip into production. And when you do face the LazyInitializationException it's just a matter of adding a single property to restore the OSIV if one prefers it.

OTOH the problems that stem from OSIV usage are much harder to track and would certainly require much more effort to address when encountered.

A lot of good arguments for dropping OSIV as the default have been outlined by many in this thread while the main argument for staying with the current default seems to be the concern for migration/backward compatibility.

No, it's not. Unless you just skip over all the arguments I made in the discussion. You might be inclined to just ignore them, that doesn't mean the only argument you can easily dismiss is the only one that was made.

Last comment from my side on this ticket as I think I've heard and replied to enough of "a lot of good arguments by a lot of people" description of basically repeating vague "it's an anti-pattern" and "problems are much harder to track" (really? which ones? what's hard about them?) for 5 screen pages. From the Twitter (when did we actually start questioning our decisions just because someone thinks something is wrong on Twitter? 🤔) conversation that re-opened the discussion: "Do you know it's an anti-pattern?", "It was a huge mistake". Not. A. Single. Argument. To. Back. Those. Claims. Repeating it's bad. Repeating it's bad.

I've also not heard a s single mention of the downsides of not using the OSIV and what users need to do to work around those: boilerplate DTOs all over the place? Encoding knowledge about what the UI needs in the transactional service layer? Isn't that an architectural problem? Compared to the solutions we have available for OSIV usage (dedicated queries, a properly prepared view model in controllers), what do opponents of OSIV suggest to mitigate problems that arise from the non-usage? And please, don't even bother to reply unless you're actually willing to answer these questions.

Gosh, I am not even saying OSIV a great thing to start with. All I am saying is that enabling it by default causes less problems than not enabling it. Because a typical Spring Boot application will — almost by definition — run into the LLE, just because JPA works the way JPA works. Boot's not gonna change that. Let Jackson render entities — boom. Let Thymeleaf render entities — boom. So — assuming we flip the default back —we end up with a Boot default that will let the user run into an LLE when they just add a controller and a Thymeleaf template and the first thing we basically do is letting them deal with some intricacy of JPA. "Hey, go and introduce a different architectural problem by teaching your service layer which relations you need in your view… or flip that bloody property!". Definitely the message I want to get when starting a project. Not. That's a massive degrade in first user experience.

My core argument is: with OSIV you run into problems outside a transactional boundary that are fundamentally the same ones you can run into within the transactional boundaries (e.g. sub-optimal queries). Thus solving those problems is a skill you will need anyway in all parts of you application (optimizing queries to stay with the example). You're going to work with database experts, you're going to solve those issues changing your code when needed. You add extra code to make obvious that this is something you want to take care of explicitly, for a reason.

Without an OSIV you create a completely new class of problem outside the transactional boundary and entities start to behave completely different when being handed outside that boundary. That makes the leaky abstraction more problematic, especially as it's not — and at the same time it is — something you solve when the problem appears, because it appears from the first line of code you write. That's pulling developers into technological details when for them that actually shouldn't be a problem, when it shouldn't be something they should have to think about. People have to write boilerplate code from the very beginning, completely hiding for which of the cases this would've actually been necessary.

A very important aspect: all the blogs discussing the downsides of OSIV look at it from a pure database access point of view: how the Session behaves, sub-optimal queries etc. And they rightfully do so as no discussion in the world can make them go away and developers have to be aware of those. However, none of them discuss the downsides and architectural and coding consequences of not using OSIV. Boot is not a data access framework. Boot is an application framework that covers a much broader picture and thus I think it has to be okay, if — given a much broader context — a decision regarding a default is made in a different way than if you looked a problem in much more narrow scope. Boot has to balance a lot more aspects than just: is this the most optimal way to get a query issued. If you're using JPA you've already made that tradeoff to a large degree as you favour developer convenience over optimizing the last percentage out of your query execution time.

In four years of Boot applications, and over ten of Spring Framework applications, I have never seen a single one that favoured not registering the OEMIV(F⁄I) and dealing with the consequences of that, over registering it and dealing with the consequences that brings. Not. A. Single. One. And that's been a couple of hundreds.

So it starts to feel really stupid that we honestly spend so much time on a topic that — thanks to Boot — is basically about just flipping a single property value in application.properties. I've laid out all the arguments I had in a lot of comments above. None of them were countered by any arguments but: "But it's an antipattern!". So I've decided to rather live with having to flip that switch instead of getting further drawn into a non-discussion.

One final remark: whether a JPA based application works decently and is architecturally sound has way more to do with the scope of the aggregates that developers model than any OSIV default could ever have. So why not just spend the amount of time spent to broadly postulate "OSIV is bad!" with exactly that: educating developers on how to build a decent domain model using JPA? Laying out the downsides of an OSIV and how to deal with them is definitely part of that. Suggesting you could just not use it and would end up with all roses and unicorns is just dishonest.

Thanks!

PS: Oh, by the way. Has anyone ever bothered to check the most upvoted SO question to that topic? There are pretty decent comments to the accepted answer that make obvious that it is a tradeoff.

PPS: Has anyone already told @joshlong that his demos won't actually work the way they worked anymore as he's going to always have to set that property, probably augmenting the // Why JPA why? comment on the default constructor needed by entities with a # Why Boot, why? on that property 😉. Just kidding.

@olivergierke For arguments you can take a look at the https://vladmihalcea.com/2016/05/30/the-open-session-in-view-anti-pattern/ (mentioned as well in the SO question you've posted).

You're saying that for 10 years of using Spring Framework you have never seen application not using OSIV. I use Spring also for 10 years, and I have never seen application using OSIV - is it really an argument?

I use @Transactional to control EM behaviour while using OSIV does all the creation outside of my control. In fact, the EM instance is shared where I don't expect it, causing for example that entities are present by sided-effect, just because two different methods share the same EM.

I am not going to respond to anything that — again — just fails to answer (ignores?) any of the questions I raised.

Which questions? You ask about the arguments -> I gave you the arguments.

But maybe just because of my incompetence I've spent the whole day trying to solve a bug caused by entities being shared where it shouldn't be. Thanks to OSIV enabled by default. Believe me - it's harder to track then LLE

If you don't bother to read my comment in the first place, why discuss at all? Editing a comment to contain more information than when I originally replied to is not a good way of discussion either.

But to make things easy for you, here you go…

"it's an anti-pattern" and "problems are much harder to track" (really? which ones? what's hard about them?)

… and …

I've also not heard a s single mention of the downsides of not using the OSIV and what users need to do to work around those: boilerplate DTOs all over the place? Encoding knowledge about what the UI needs in the transactional service layer? Isn't that an architectural problem? Compared to the solutions we have available for OSIV usage (dedicated queries, a properly prepared view model in controllers), what do opponents of OSIV suggest to mitigate problems that arise from the non-usage? And please, don't even bother to reply unless you're actually willing to answer these questions.

Thanks!

Believe me - it's harder to track then LLE

"Believe me"? What kind of "argument" is that?

I realise that this is a topic that people feel strongly about, but can we please try to be civil? Let's stick to discussing arguments for and against without implying that the participants are dishonest or stupid.

@olivergierke You have objected to others ignoring points that you are making, yet you seem to suggest we dismiss one purely because it was made on Twitter. I think it's very healthy to question design decisions periodically, especially one, like this, that is far from clear-cut.

Educating people on good design with JPA is something that can be done at any time. Changing our default OSIV configuration is not. Boot 2.0 is a rare opportunity to make that change so the time taken to discuss it in depth is, I think, well spent.

With that said, I'd like to hear more from the proponents of disabling OSIV by default on how you have avoided or dealt with the problems that @olivergierke has highlighted.

@olivergierke you're very selective. Have you read the @vladmihalcea post about why OSIV is an antipattern? I don't see you're arguing with its content. You asked which problems caused by OSIV are hard to track -> all caused by entities being shared in persistent context, where you don't expect different methods are using the same EntityManager.

You are also taking about "downsides" of not using OSIV. I don't want to argue if exposing domain entities by REST API is a good idea, but you still can enable OSIV if you want. But still - LLE is really obvious as most developers know how to handle it - by redesigning the service layer or by applying OSIV walkaround.

I am not selective. I have acknowledged the problems of OSIV in every comment I made. Actually, that's exactly my point. I am arguing that despite the well accepted problems OSIV imposes if looked at from a pure database centric point of view, it's reasonable to let it be the default because the downsides of switching the default have very wide implications (Boot user experience, architecturally) and you already can switch to the model you propose right now if you want to work in that model. I've repeatedly stated exactly that. Trying to create the impression I'd want to ignore those downsides is inappropriate. I look at them in a bigger context and then draw different conclusions.

An EntityManager is — by JPA spec (section 7.2) — a non-thread safe instance. If you consume it via DI, that means it has to be a Thread-scoped instance. That in turn means, in a servlet environment — that's again Thread-based by definition — the EntityManager is shared by definition. That's not even something Boot specific. EntityManager instances have always worked that way in Spring, CDI, simply because that's how they have to work. Serious question: how come you don't expect it to be shared?

Speaking about selectivity I think exactly that has been applied by the OSIV opponents by consistently failing to consider the implications of a changed default. In particular I'd love to hear:

  1. How are users supposed to handle the case that simple Jackson marshalling or Thymeleaf view rendering that includes traversing lazily-loaded relations are now broken by default? Especially as the most obvious solution would be returning to the current default, which implies the current one is actually the better one considering the entire application stack.
  2. How does this new "things don't work out of the box anymore" affect the overall user experience of a Boot application? Which problems do we expose to users at which point in the application development lifecycle (considering both simple sample projects as well as long lived applications).
  3. What architectural consequences has the vaguely (read: not at all) defined "redesigning the service layer" and are we sure we want to require users to deal with those with the new default from the first line of JPA-based Boot application code they write?

Ideally, I'd love to see an example project that shows the described use cases with the current default flipped and the code that has to be written to accommodate the lack of OSIV. That would allow the Boot team to decide whether this could be the default application design model that they want to promote to users.

@wilkinsona — I'm sorry if I seemed harsh, but it felt like a decision had been made with this comment. Re-opening the question after a complaint on Twitter that doesn't actually bring new arguments to the table, creates the impression that simply complaining is enough. That's what made me wonder.

With that said, I'd like to hear more from the proponents of disabling OSIV by default on how you have avoided or dealt with the problems that @olivergierke has highlighted.

@wilkinsona Our preference is to have explicit and well defined transactional boundaries which are marked by use of @Transactional. Together with this we use @Query on our Spring Data JPA Repository interfaces to specify join fetch according to needs of specific use cases/consumers. In rare occasions we also use projections. All in all very similar to Boot's own spring-boot-sample-data-jpa sample app.

The fact that this is such a hot topic IMO supports the approach to have the user decide on use of OSIV rather than sweeping the decision (together with potential problems) under the carpet by having it on by default. I believe this is in particular important for less experienced users, who might not be too familiar with the entire stack that comes with spring-boot-starter-data-jpa and Boot shouldn't hide lazy loading concerns from them.

Thanks, @vpavic. I understand your preference, but what I'm really interested in seeing is how you've gone about making that preference a reality. Boot's Data JPA sample is rather simplistic and, even if spring.jpa.open-in-view is set to false its single endpoint continues to work as it returns the name of a single entity.

It's pretty easy to make the sample break with open-in-view set to false:

diff --git a/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/domain/Hotel.java b/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/domain/Hotel.java
index f3cb6ab8bb..e2ca2b2d88 100644
--- a/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/domain/Hotel.java
+++ b/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/domain/Hotel.java
@@ -78,4 +78,9 @@ public class Hotel implements Serializable {
        public String getZip() {
                return this.zip;
        }
+
+       public Set<Review> getReviews() {
+               return this.reviews;
+       }
+
 }
diff --git a/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/web/SampleController.java b/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/web/SampleController.java
index a7fd42aee1..0798ec5872 100644
--- a/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/web/SampleController.java
+++ b/spring-boot-samples/spring-boot-sample-data-jpa/src/main/java/sample/data/jpa/web/SampleController.java
@@ -16,7 +16,9 @@

 package sample.data.jpa.web;

+import sample.data.jpa.domain.Hotel;
 import sample.data.jpa.service.CityService;
+import sample.data.jpa.service.HotelService;

 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Controller;
@@ -30,6 +32,9 @@ public class SampleController {
        @Autowired
        private CityService cityService;

+       @Autowired
+       private HotelService hotelService;
+
        @GetMapping("/")
        @ResponseBody
        @Transactional(readOnly = true)
@@ -37,4 +42,12 @@ public class SampleController {
                return this.cityService.getCity("Bath", "UK").getName();
        }

+       @GetMapping("/test")
+       @ResponseBody
+       @Transactional(readOnly = true)
+       public Hotel test() {
+               return this.hotelService.getHotel(this.cityService.getCity("Bath", "UK"),
+                               "The Bath Priory Hotel");
+       }
+
 }
diff --git a/spring-boot-samples/spring-boot-sample-data-jpa/src/main/resources/application.properties b/spring-boot-samples/spring-boot-sample-data-jpa/src/main/resources/application.properties
index 330237435d..27ca52899c 100644
--- a/spring-boot-samples/spring-boot-sample-data-jpa/src/main/resources/application.properties
+++ b/spring-boot-samples/spring-boot-sample-data-jpa/src/main/resources/application.properties
@@ -1,3 +1,3 @@
 spring.h2.console.enabled=true
-
+spring.jpa.open-in-view=false
 logging.level.org.hibernate.SQL=debug

With these changes in place, calling /test blows up:

$ curl localhost:8080/test
{"timestamp":1493212428796,"status":500,"error":"Internal Server Error","exception":"org.springframework.http.converter.HttpMessageNotWritableException","message":"Could not write JSON document: failed to lazily initialize a collection of role: sample.data.jpa.domain.Hotel.reviews, could not initialize proxy - no Session (through reference chain: sample.data.jpa.domain.Hotel[\"reviews\"]); nested exception is com.fasterxml.jackson.databind.JsonMappingException: failed to lazily initialize a collection of role: sample.data.jpa.domain.Hotel.reviews, could not initialize proxy - no Session (through reference chain: sample.data.jpa.domain.Hotel[\"reviews\"])","path":"/test"}

That error message isn't particularly helpful, particularly for a beginner. If nothing else it highlights the need for something like @jkubrynski proposed above.

@vpavic I'd really like to see the patterns that you use in a real-world app applied to the sample. What approach would you take in the service layer to create something that can be safely passed to a user of the service? How do you deal with the apparent overhead of all the extra types that would, presumably, be needed?

@olivergierke I think you miss very serious side effect of the OSIV. Assume I've two methods called from the controller - both are transactional.

  1. Method save() which persists the entity
  2. Method load() which loads the entity
    And now something I think you're missing:
  3. with OSIV enabled the entity loaded in the second method is THE SAME Java object which has been saved by the first method
  4. with OSIV disabled the entity loaded in the second method is DIFFERENT Java object than the one saved by the first method

I really don't want to advertise but this intense discussion about DTO boilerplate and OSIV just fits so perfectly that I must step in and say that I had the same problems in the past. I solved them by creating a library for declarative DTO definitions and persistence model binding. It works on top of JPA and is essentially a way of _teaching the service layer what data is needed in the UI_. Maybe someone will find that library useful for doing IMO the right thing and separating the representation and the persistence model.

Thank you, @beikov. Would anyone else who is in favour of disabling OSIV by default care to share some examples of how they deal with the problems created by doing so?

@vpavic I'd really like to see the patterns that you use in a real-world app applied to the sample. What approach would you take in the service layer to create something that can be safely passed to a user of the service? How do you deal with the apparent overhead of all the extra types that would, presumably, be needed?

@wilkinsona Sorry I didn't have time to revisit the sample with your changes but certainly plan to do so. I can also show you some real-world examples at Spring IO next week.

@wilkinsona Generally in the real-world applications that are more complex than database viewers you expose DTO instead of entities. Considering this pattern it's no real difference if the DTO is prepared in transactional service or in the controller. That means, in fact we don't need the session in the controller. Also please consider my recent comment which shows the serious problem of using OSIV.

Considering this pattern it's no real difference if the DTO is prepared in transactional service or in the controller

It could be argued that the service layer should have no knowledge of the form that an object should take when it's sent to a client by a controller. If you want to make that separation and avoid the use of OSIV, you now have to map things twice. That could be an excellent choice for someone who's experienced and understands why the additional mapping is beneficial, but it could also be a poor choice for a beginner who wants to get started quickly. Striking that balance is, IMO, what's made this issue so contentious.

@wilkinsona I still don't get one thing -> we're talking about the design, while "silently" enabling OSIV can cause serious issues on production, what people (even experienced ones) are not aware of.

while "silently" enabling OSIV can cause serious issues on production, what people (even experienced ones) are not aware of.

That's the other side of the same coin. Disabling OSIV by default could be an excellent choice for complex applications where it may cause performance problems in production, but it could also be a poor choice for a beginner who wants to get started quickly.

I've been trying to encourage people to share examples of how they do things without OSIV as I'd love to see an easy-to-use, concise, and beginner-proof approach to building an app without OSIV.

We take the getting started experience very seriously so don't want to inflict lots of boilerplate, or some additional mapping library on beginners to avoid a problem that they shouldn't have to care about just yet. On the other hand, we don't want a smooth getting started experience at the cost of creating problems for people as their app grows or it reaches production. Right now, I haven't seen a suggestion that satisfies both so we're left trying to pick the lesser of two evils.

IMHO important thing is that it's easy to find out issues caused by "no OSIV" -> google for the exception. On the other hand, it's really hard to realise that you're having issues because OSIV is enabled. It was never enabled by default in Spring and it was a giant surprise for me when I realised there is OSIV in the stacktrace. OSIV causes that EntityManger is silently shared over the @Transactional boundaries. In practice, you can load the entity in one transaction, modify it outside of the transaction and then it still will be persisted. In fact, many inexperienced people just won't realise why the code is working.

it's easy to find out issues caused by "no OSIV" -> google for the exception

I agree with that, but unless we can offer a solution that's better than people setting the property manually we haven't really made any progress by disabling OSIV.

For a solution to be better, it needs to be easy to understand and implement. My suspicion is that, faced with a choice between setting a property or doing lots of research to understand the problem and then introducing a mapping layer or taking care to only expose eagerly-loaded associations, the vast majority of people will just set the property and then forget about it. At that point, there's less friction and the outcome would be the same if we'd just enabled OSIV by default.

If you think that situation when the code works but nobody knows why is a good approach, then OSIV definitely should be enabled. However - understanding transactions is a must even for a junior developer. I don't want to argue if anyone should or shouldn't use OSIV. I just think it has a giant impact on the performance as well as on business logic that is should be enabled only on demand.

If you think that situation when the code works but nobody knows why is a good approach, then OSIV definitely should be enabled. However - understanding transactions is a must even for a junior developer.

When you're a beginner there are _lots_ of things that you don't understand and lots of code that works and you don't know why. The question here is really about when we should require a beginner to understand something that's quite intricate. Gaining that understanding will undoubtedly slow them down at a time when what they would really like is to keep their momentum up by seeing something that works.

If we can guide a beginner through a smooth, quick, and easy to understand series of steps to get an app off the ground without OSIV, then we can consider it. Your suggestion above is an excellent first step, but I don't think just telling a user to set spring.jpa.open-in-view=true is the right thing to do. Almost all beginners will hit the problem and if we just tell them all to enable OSIV then I think we might as well have just done that for them in the first place.

Some options for things that we could suggest:

  • Hide any lazily-loaded associations by making them package private so they can't be called outside the service layer. This requires a certain package structure that might not be in place.
  • Load everything eagerly
  • Map the entities to something else and return those from the service layer
  • Go nuclear and set spring.jpa.open-in-view=true

Excluding the nuclear option, I think loading eagerly is going to require the least effort to get an app that works.

@vladmihalcea is there any feature in Hibernate that allows me to call entityManager.pleaseLoadAllProperies(entity)?

Only with join fetch

But then you must know you want to fetch all when retrieving the entity. No way to do it on loaded entity?

You can use Hibernate#initialize along with some Reflection for multi-level entities.

So maybe @wilkinsona we could provide a bean available in Spring Boot which will handle eager loading of a given entity?

@Transactional
@Service
public class ProductService {

  public Product findProduct(Long productId) {
    Product product = productRepository.findById(productId);
    someMagicBean.fetchProperties(product);
    return product;
  }
}

There are also two nice feature requests in Spring Data JPA ,which would be very useful in this case, and looks like quick wins. I'll also provide better performance eliminating N+1 problem:

https://jira.spring.io/browse/DATAJPA-198
https://jira.spring.io/browse/DATAJPA-208

Frankly speaking I do not understand why some people want to force spring team to change default.
Here are first two sentences from Spring Boot web page:
Spring Boot makes it easy to create stand-alone, production-grade Spring based Applications that you can "just run".

You see the ideas of "easy" and "just run". This is absolutely fine to have the default optimized for developer productivity and ease of use. It is much more logical to require advanced users to change settings for non trivial use cases rather than make new users to make additional steps to "just run" and harder to do simple and trivial things.

And sorry but
someMagicBean.fetchProperties(product);
looks confusing and excessive. This is the line you will choose answering "Which One Doesn't Belong here" question.

There are no just good or bad things. Something is good for one purpose but exact same thing may be bad for other purpose.

And speaking about junior developers even if you say that they should understand the concept of transaction please don't force them to learn how to set up and configure framework so that their application just worked. And I may argue that there are many others things in spring that following same logic even junior developers have to understand. And if you change all the defaults to favor optimal or "academically right" vs "easy and simple" usage of spring boot will loose a lot of its appeal.

Please close this ticket and stop posting comments about patterns. It is not about definition of patterns and anti patterns. Use academic papers for this. Think about "Making Easy Things Easy & Hard Things Possible" principle. And again I think that it is much more logical to require advanced users to change settings for non trivial use cases rather than make new users to make additional steps to "just run" in simple and trivial situations.

@sveryovka That's what EclipseLink does. We could have done the same in Hibernate, but we didn't do that. Remember that Hibernate also provides the hibernate.enable_lazy_load_no_trans configuration property, but it's disabled by default. That's because we want our users to make the right choices from the very beginning.

Did this choice affect our user base? Not at all. Hibernate has a solid market share. So, it's not that bad to impose good practices, right?

@vladmihalcea
Frankly speaking this issue already has to many posts and I have not seen good proposal how everything (not just persistence tier) should work. What is the rerecorded best practice? Use DTOs? But I may argue that DTO which just duplicates entity is absolutely superfluous (anti-pattern) for simple apps. And DTOs where invented in times of j2ee 4 entity beans. And some main reasons for their invention no longer exist.

I am arguing that the right choice depends on what you are actually trying to do. If you are building large and complex app you have one right choice. If you are building quick proof of concept or trivial app OSIV is absolutely right choice for your task. Since in this case time to market and developer productivity is the most important factor for you.

Very famous example is facebook and their usage of php. There are many videos where they talk about it. And their message is simple - in many cases time to market is the most important thing and when you want to make everything right and take your time to do this you may just miss your opportunity and then it does not matter whether you've used good practices or not. The point I am trying to make is that in many situations usage of best practices in not most important thing. In many cases ease of use and developer productivity are much more important. So why not to force productivity? And facebook is definitely not the only example.

Regardless of what the default setting is there will be use cases where it will be wrong. So why not make default setting favor developer productivity over other factors? Few years ago common complain about spring and java was that it is much easier to develop same app using ror or node or python or whatever because spring required to much of ceremony and efforts to start and configure it to do even basic things. And unfortunately many people were not choosing java because of this (believe me I know what I am talking about). Spring boot has fixed this and it is still possible to use any approach you want. So what is the problem?

What is the rerecorded best practice? Use DTOs? But I may argue that DTO which just duplicates entity is absolutely superfluous (anti-pattern) for simple apps. And DTOs where invented in times of j2ee 4 entity beans. And some main reasons for their invention no longer exist.

Why do you think JPA offers DTO projections then? This is difference between fetching an entity and a DTO projection. Watch my High-Performance Hibernate presentation for more details.

If you are building large and complex app you have one right choice. If you are building quick proof of concept or trivial app OSIV is absolutely right choice for your task.

So, you want Spring Boot to be just a solution for rapid prototyping, and not for a serious enterprise application?

Few years ago common complain about spring and java was that it is much easier to develop same app using ror or node or python or whatever because spring required to much of ceremony and efforts to start and configure it to do even basic things.

Ruby on Rails is very convenient from a development perspective, yet, Twitter switched to Java because it scales better, especially on the back-end side, to quote this link).

@vladmihalcea
Vlad,

I thoght that I was very specific that even today with current default it is possible not to use OSIV with spring boot. Isn't it? So it is not one way or the other. It is just about default setting. Isn't it? I never wrote that it should be always OSIV. This issue is only about default value of switch: "on" or "off" and it is always possible to change the value.

For some reason from my paragraph

I am arguing that the right choice depends on what you are actually trying to do. If you are building large and complex app you have one right choice. If you are building quick proof of concept or trivial app OSIV is absolutely right choice for your task.

you chose not to copy the first sentance which is my main message:

I am arguing that the right choice depends on what you are actually trying to do.

The same statement I had in my initial comment

There are no just good or bad things. Something is good for one purpose but exact same thing may be bad for other purpose.

Are you accepting the statements that

the right choice depends on what you are actually trying to do.

?

@sveryovka OSIV is like an entropy. It's very easy to enable it, and really hard to remove. People making use of OSIV without any understanding how it works will be screwed when they will realise what happened. Especially that OSIV brings some unclear backside operations, like the one described in my comment.

@jkubrynski

Same question to you are you accepting statement that

the right choice depends on what you are actually trying to do.

?

@sveryovka the problem is that when we choose to disable OSIV by default, people will make a decision if they want to use it very soon - after the first LazyInitializationException. Leaving OSIV enabled by default will cause people realising they use it after 2-3 years, when removing it will be a giant project.

@jkubrynski

For some reason you avoid answering directly. Is this just because you don't want to agree with me on anything even if it is just some generic stetement? Or do you really don't think that

the right choice depends on what you are actually trying to do.

?

I don't want to speculate about who sees what and when. I don't want to speculate about adoption rate and so on.

I am trying to speak about basic logical stetements which can be used to dispute any value of any defelaut preference. And one of the most important from my standpoint is that

There are no just good or bad things. Something is good for one purpose but exact same thing may be bad for other purpose.

No speculations. Can you please answer "yes" or "no"?

There is no "yes" or "no" answer in IT. Everything is based on the context. Spring Boot context is "enterprise application". And in this context OSIV should not be used.

@jkubrynski
I will take your

There is no "yes" or "no" answer in IT.

as you agreeing to my

There are no just good or bad things. Something is good for one purpose but exact same thing may be bad for other purpose.

Since this is basically the same. But again you decided not to admit that you agree with me at least on this basic statement even though I was not saying about "Spring Boot context".

The next logical question you have already started to talk about yourself is what you get when you just start with spring boot (start.spring.io) default project stub.

When you have on/off switch defined by some property and if the default value is a tradeoff between "enterprice readiness" and "ease of use and developer productivity" what is more impornat as default? In any case it is possible to change value manually. And again it is not one way or another it is just about what is turned on/off by default.

Your point of view is that "enterprice readiness" is more impornat to be turned on as default. My point of view is that "ease of use and developer productivity" is more impornat to be turned on by default. And again in any case it is possible to change property value in config file.

I do not know how decisions are made for spring boot. But somebody has to make this decision.

@sveryovka

the right choice depends on what you are actually trying to do.

Exactly! That's why OSIV should be disabled by default and enabled explicitly. By default, I assume that Spring Boot is meant for serious enterprise applications lead by responsible developers. If you want a throw-away prototype, you can enable OSIV explicitly and knock yourself out.

Makes sense?

To get a better sense of what the developer experience would look like in practice with OSIV disabled, @jkubrynski @vladmihalcea would you be able to share an example repo that demonstrates the best practices you're advocating?

Perhaps the relationship between Movie, Theater, and Showtime where a movie can play at many theaters at many showtimes. Feel free to substitute your own domain but something that demonstrates these types of relationships.

Hopefully this will paint a clearer picture of what the startup experience will look like and could better inform the decision making process.

@andersonkyle. Sure, check out these 454 pages that show you how to design such a Persistence Layer so that it's both effective and efficient.

@vladmihalcea I think your answer summarizes it very well why people prefer to just use open-session-in-view!
1-3 pages long blog post with a small demo app might help! But if anyone has to read 454 pages to avoid this setting then I'm pretty sure 98% of the people will skip the book!

@martin-g Well, my readers' testimonials have a different opinion on this matter.

For better or worse, if a beginner is presented with a choice of reading 454 pages or setting a single property, I agree with @martin-g that the vast majority of them will set the property. That is in no way a criticism of your book, @vladmihalcea. It is, however, indicative of a rather steep learning curve with JPA and Hibernate.

In the absence of a concise, beginner-friendly explanation of what to do instead of using OSIV, I think we have no choice but to leave it enabled by default. Without it, I think the getting started experience is poor and the learning curve too steep.

@wilkinsona JPA and Hibernate, just like any data access framework, have a steep learning curve because, after all, you still need to know how RDBMS, JDBC work behind the scenes, how indexing works, how to read an Execution Plan, etc. However, I can see why you chose this property to be enabled be default. I understand that usability is a very important feature of Spring Boot.

My comment about reading my book was targeting this comment:

something that demonstrates these types of relationships.

It was not about OSIV. You don't need to read 454 pages to know how to use LAZY fetching properly. This article is enough and takes less than 5 minutes even for a junior developer.

Well, my readers' testimonials have a different opinion on this matter.

@vladmihalcea I was not talking about the book as a whole, but about the topic in this issue - OSIV.
Until someone shows a relatively small and easy to grasp example of a Spring Boot application without OSIV many new projects will be created with OSIV enabled!

Until someone shows a relatively small and easy to grasp example of a Spring Boot application without OSIV many new projects will be created with OSIV enabled!

You don't need a specific Spring Boot example. Spring Boot is just Srping after all, and OSIV hasn't been the default approach on any Spring-based project I worked on over the past 10 years. This article shows how to handle LAZY associations the right way, no matter if you're using Spring or Java EE.

Spring Boot is just Srping after all

Yes, in plain Spring you have add the additional Servlet Filter yourself. Not as simple as in Boot but again very easy!

My experience is exactly the opposite - I haven't seen a Spring application with Hibernate that doesn't use OSIV :-/
I don't say that using OSIV is better! I just say that it is widely spread (according to my experience!).
In any application that strives for performance I'd just use JdbcTemplate.
Maybe org.springframework.orm.hibernate5.HibernateTemplate should be promoted ?! (I haven't used it so far).

@wilkinsona Is Spring Boot targeted for beginners or to create production-ready applications? If it's a project for sandboxing and 5-minute live-coding on presentations then leaving OSIV enabled is a great idea. But deploying OSIV to production is a terrible idea.

For the read concerns you can use a projection tool like e.g. Blaze-Persistence Entity Views or if you only need simple projections Spring Data Projections should be enough too.
I am currently working on also handling the write concern with Entity Views, but this is still in development. If you provide me a sample app using OSIV I can adapt it and show you how to implement a DTO approach.

Is Spring Boot targeted for beginners or to create production-ready applications?
@jkubrynski I'd guess that's a rhetorical question, but I think it's worth addressing anyway.

Spring Boot is obviously targeted at both, which is why this isn't an easy decision to make. There's no point in making something great for production if people give up and choose a different stack because the getting started experience is poor.

The ideal here would be for the getting started experience to have a gentle learning curve and for the curve to continue to be gentle as you move towards production. Right now, the curve either has to be really steep at the beginning (OSIV disabled by default), or potentially really steep later on (OSIV enabled by default and it causes performance problems). Potentially really steep later on is the lesser of those two evils, IMO.

Perhaps we can strike more of a balance while still leaving OSIV enabled by default? I wonder if it would be possible to detect when it's prevented a LazyInitializationException and log a warning. That way people would get an easy getting started experience, but would also be warned about potential problems.

The ideal here would be for the getting started experience to have a gentle learning curve and for the curve to continue to be gentle as you move towards production. Right now, the curve either has to be really steep at the beginning (OSIV disabled by default), or potentially really steep later on (OSIV enabled by default and it causes performance problems). Potentially really steep later on is the lesser of those two evils, IMO.

@wilkinsona I understand and agree that Boot should target both groups, and that striking a balance is often times difficult, however I strongly disagree with the part in bold.

If you consider the topic of project's life cycle I think it should be obvious that the damage is much bigger if anyone runs into problems due to use of OSIV at later stages of the project. Especially considering the projects of enterprise-ish size, the effort to move off OSIV can be quite costly and potentially catastrophic to the project (delay of delivery dates, production issues, etc.). I don't know about others who participated in this discussion, but on every project I've worked on in my career the maintenance phase remained among my responsibilities as well so I care a lot about these things.

Regarding the concise and beginner-friendly example of OSIV-less approach, I think the 2 or 3 @vladmihalcea's blog posts that have been linked here do explain things in concise and beginner-friendly way. Do you perhaps insist on code based example that would be a part of Boot's codebase as a sample project?

Just to make it clear, I'm not trying to force anyone not to use OSIV or promote the data access approach that I use. It's just that I believe that having it enabled is a really bad default that has severe architectural implications, and as such it should be left for users to be explicit about whether they want it or not. I cannot recall any other of Boot's opinions that has such architectural impact. In fact, there are a number of other places where Boot has moved or is moving from having something on by default to requiring users to enable it using configuration property (e.g. spring.session.store-type, database initializers) and I don't believe that made Boot any less beginner friendly.

In my opinion having OSIV enabled without being aware of it is a problem. Why not leaving it on by default in 2.0, printing a big warning to the console that prompts the developer to explicitly opt-in? Everything accompanied by some concise docs that explain the implications of OSIV=true/false. The resulting behavior would be:

JPA is not enabled: No warning
JPA is enabled, no OSIV setting declared (OSIV is on): Warning is emitted
JPA is enabled, OSIV=true: No warning
JPA is enabled, OSIV=false: No warning

@aahlenst Thanks for the suggestion. Let's see if we can implement what you've proposed in M6.

IMHO, enabling OSIV by default does disservice to beginners. It hides the way Hibernate/JPA really works with collection mappings. one-to-many and many-to-many relations which are lazy by default will behave like eagerly fetched. Beginners who are familiar with fetching modes would be confused by absence of LazyInitializationException and maybe dig more , but many of them will be happy that things just work magically and likely pretty confused on some future project that does't have OSIV configured.
I always felt about OSIV as of violation of "separation of concerns" principle, but it's not even the point here.
The point is OSIV should be explicitly enabled only by someone who knows what it is and what it does. Let the beginners learn Hibernate/JPA pitfalls early, don't hide it.

It's worth adding this StackOverflow question where someone asks for explicitly disabling OSIV because the magically handling the LazyInitializationException can hide bugs.

Let me use this opportunity to point out that we've now reached a point where a user has declared a class with a field and an accessor method returning that field. That user is now actually surprised that this code does what it's declared to do and does not throw an arbitrary technical exception.

And this is considered the "wrong" behavior and we use that as an argument to actually change that.

a user has declared a class with a field and an accessor method returning that field

@olivergierke Are you talking about the same StackOverflow question?

There is no mapping being shown there.

There is no mapping being shown there.

Let's see if we find a Hibernate / JPA expert that can educate us on what code looks like that usually throws LIEs (funny abbreviation, eh?). Anyone?

I'm not sure if I qualify as a Hibernate / JPA expert, but I wrote my 2 cents on this article on my blog, the best way to handle the LazyInitializationException LazyInitializationException.

Let me use this opportunity to point out that we've now reached a point where a user has declared a class with a field and an accessor method returning that field. That user is now actually surprised that this code does what it's declared to do and does not throw an arbitrary technical exception.

I have a way better analogy:

  1. The user has a banking account with a balance of 50$.
  2. The user wants to withdraw 60$, and instead of getting an InsufficientFundsException, they will get the 60$ without knowing that there's an overdraft fee for the extra 10$.
  3. The longer it takes for the user to realize they have a debt to pay, the costlier it will become.

The whole point of this thread was to let the user know about this issue, as long as OSIV is still enabled by default.

Hiding a LIE (funny abbreviation, eh?) is not the same as telling the truth,

Anyway, I don't think this the discussion is going anywhere, so there is no point in continuing it.
I'll focus on making people aware of this issue so that they can take the best decision whether they want to have OSIV enabled or not.

That’s neither a better analogy nor an analogy at all to what the original reporter in that StackOverflow post asked. But it’s in line with a lot of the “making up things from thin air” presented in this thread. An LIE is not a business exception, it’s not triggered by any implementation of a business rule. It’s a technical distraction from your actual business problem.

My point was not about how to handle LIEs, but that code you can perfectly reason about is considered “broken” if it works the way it’d work in plain Java, without JPA in place. OEMIV is one means to solve that with well-known pros and cons. Heck, OEMIV is not even about handling exceptions, it’s about preventing them. It’s not about correctness, it’s about efficiency, convenience and optimizations. That’s a world of a difference. The thread here wants to imply something is horribly broke, where it’s actually about a trade-off between convenience and efficiency. We need proper data points to evaluate that balance but more on that below. The issue that triggered my comment was, that – I guess without a lot of people not realizing - the SO post was more proof of the underlying problem than that it was how to apply one of the solution options.

Anyway, I don't think this the discussion is going anywhere, so there is no point in continuing it.

We’ve reached said point a while ago when the proponents of the suggested change boasted that an alternative implementation to what the current default allows would be oh so very simple, but then constantly fail to provide any meaningful example of what this would look like, even after repeated requests to provide some code. However, that chance is still given. Unfortunately, so far, we’ve seen nothing but blowing smoke instead. I can point to a gazillion of SO threads in which users were caught by surprise about LIEs and where OEMIV was a decent solution for. But yeah, that’s not moving this discussion forward a bit. I guess my ironic remark was just my way of pointing out the pointlessness of that.

Shortcutting this: I guess, we should move forward by finally seeing examples what an alternative user application implementation would look like, or resolve the discussion.

finally seeing examples what an alternative user application implementation would look like.

It's simple.

Instead of:

@Transactional
public List<PostComment> getPostComments(String review) {
    return entityManager.createQuery(
        "select pc " +
        "from PostComment pc " +
        "where pc.review = :review", PostComment.class)
    .setParameter("review", review)
    .getResultList();
}

They would write:

@Transactional
public List<PostComment> getPostComments(String review) {
    return entityManager.createQuery(
        "select pc " +
        "from PostComment pc " +
        "join fetch pc.post " +
        "where pc.review = :review", PostComment.class)
    .setParameter("review", review)
    .getResultList();
}

It's just one extra JOIN FETCH for each @ManyToOne or @OneToOne associations being needed further up the call stack.

Or they can use Entity Graph, if don't write the JPQL or Criteria API query themselves and rely on Spring Data for that.

For collections, they can JOIN FETCH up to one collection, otherwise a Cartesian Product is generated.

So, any extra @OneToMany or @ManyToMany would require to be navigated before returning from the @Transactional block.

However, initializing more than one collection can easily lead to an N+1 query issue, and that's the case even if OSIV is disabled or not.

And what in the world is preventing anyone from writing the query the way you suggested with the current default? I think it’s a decent way of selectively optimizing queries while still retaining the convenience of everything not explicitly optimized still working as the written code suggests (getPost() returning the Post no matter whether it’s currently loaded or not).

And what in the world is preventing anyone from writing the query the way you suggested with the current default?

Why would they if it works magically like this:

List<PostComment> findByReview(String review);

With a single Spring Data method I can:

  • fetch the List<PostComment> by review when I only need data from PostComment
  • fetch the List<PostComment> by review when I need data from PostComment and from the Post as well

Why would a developer want to write separate methods and think about fetching data efficiently when their priority is addressing issues from the current Sprint log?

I have locked this issue, for now at least, as the discussion no longer seems to be productive. We’ll add the logging suggested above by @aahlenst.

Why would a developer want to write separate methods and think about fetching data efficiently when their priority is addressing issues from the current Sprint log?

Yes, and they shouldn't!
There is profiling, logging and monitoring for this.

If it is a not a high load service, then N+1 problem can never be noticed. Yes, it can work suboptimal, but who cares if it runs 10 queries/hour (1 main query and another 9 because of N+1 "problem" :smile:)?

If it is a high load service, than it should have a profiling, monitoring, logging etc, so this performance degradation will be noticed, then SQL log will be enabled and N+1 problem will be noticed. And only then some decision will be made:

  • Always use JOIN FETCH (for example, if the performance will be acceptable even in case when we don't need related data).
  • Write another method with JOIN FETCH and use one or another according to which fields of model will be needed (it is preferable from performance point of view, but is bad from convinience, clean-code and "service layer should have no knowledge about view" point of view. All these getUser, getUserWithPorts, getUserWithPortsAndLines etc :( ).
  • Leave it alone because this N+1 execution path appears only in very specific (rare) cases (users rarely click "show more" button).
  • Refactor to use specialized DTOs (can be part of another approach).
  • Another approach.

And all of them are possible without disabling OSIV.
I prefer optimizing the bottlenecks, not all possible paths.
Or else why should I use ORM in first place if I would always manually craft explicit queries for each and every use-case? Even these cases, when optimization is not required at all and lazy fetching is acceptable.

Maybe it will be more convinient to use something like JOOQ than JPA then?


Another concern is what to return from these "specialized" methods?
Like in getPostComments example above. If this is the only method returning List<PostComment> then we can just make post field EAGER. If not, then there is another method without join fetch pc.post, like getPostCommentsWithoutPost, right?

Should we really return List<PostComment> then? If we get the list using getPostComments, then we can call list.get(0).getPost() method. If we get the list using getPostCommentsWithoutPost, then (without OSIV) we can't do this (LIE) despite the fact that both methods return the same models. So as a developer I should always be aware that PostComment can containt the post or not contain the post. :man_facepalming: And I can never be sure if I can call getPost() method on it. It could be not clear where this list comes from in the code, so sometimes it could be very hard to understand what kind of PostComments (with or without post) in the concrete list. Partially filled models are more evil then OSIV IMHO.

So to prevent this we forced to always use DTOs then. Different DTOs of same model for different use-cases (feel the pain :imp: ).

And if we decide to do this, then OSIV just does nothing. Nothing good, but also nothing bad. Lazy loading will not happen anyway. Open session will still be open, but DTOs doing nothing with it.

So, my opinion is that, there is no one and only one "right" way to do the things. All have its drawbacks.

And if we want totally ignore the ORM's ability to lazily load relations, then maybe we should introduce a hibernate.use-the-joins-luke flag. :) and throw LIE on any not loaded field access attempt even when em is still open. :)

Was this page helpful?
0 / 5 - 0 ratings