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

[vpavic]

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.

[wilkinsona]

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

[wilkinsona]

Closing in favour of the PR (#7125)

[odrotbohm]

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.

[vpavic]

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.

[odrotbohm]

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.

[vladmihalcea]

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.

[vpavic]

@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.

[odrotbohm]

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.

[nfrankel]

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.

[odrotbohm]

…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.

[vpavic]

@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.

[odrotbohm]

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.

[nfrankel]

…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.

[odrotbohm]

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

[nfrankel]

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?

[odrotbohm]

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.

[vladmihalcea]

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.

[philwebb]

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.

[vpavic]

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.

[qerub]

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.

[vladmihalcea]

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.

[jkubrynski]

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

[snicoll]

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

[philwebb]

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

[philwebb]

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

[philwebb]

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

[snicoll]

That's not happening on startup.

[jkubrynski]

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 :)

[vpavic]

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.

[odrotbohm]

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.

[jkubrynski]

@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.

[odrotbohm]

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

[jkubrynski]

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

[odrotbohm]

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?

[wilkinsona]

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.

[jkubrynski]

@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.

[odrotbohm]

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.

[vpavic]

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.

[wilkinsona]

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?

[jkubrynski]

@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:
   • with OSIV enabled the entity loaded in the second method is THE SAME Java object which has been saved by the first method
   • with OSIV disabled the entity loaded in the second method is DIFFERENT Java object than the one saved by the first method

[beikov]

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.

[wilkinsona]

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]

@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.

[jkubrynski]

@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.

[wilkinsona]

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.

[jkubrynski]

@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.

[wilkinsona]

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.

[jkubrynski]

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.

[wilkinsona]

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.

[jkubrynski]

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.