Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nested DTO mapping #2540

Open
belovaf opened this issue Feb 2, 2022 · 10 comments
Open

Support nested DTO mapping #2540

belovaf opened this issue Feb 2, 2022 · 10 comments
Assignees

Comments

@belovaf
Copy link

belovaf commented Feb 2, 2022

Ebean perfectly supports fetching partial entities.
But it feels very error prone to return a partially loaded object for further processing.
Someone can accidentally access unloaded field and trigger lazy loading query.
I think it is a good idea to introduce a dto in such cases to prevent additional queries.
If someone wants to access an unloaded field he will be forced to update query and dto.

But currently ebean supports only plain dtos. This is not convenient to construct nested projections from a flat resultset manually.

It is possible to define new entity with @View annotation and map it to the same table, but it is not feels right.

May be a better option exists, can you explain, please, how do you solve such problems?

@rbygrave
Copy link
Member

rbygrave commented Feb 2, 2022

better option exists, can you explain, please, how do you solve such problems?

Use query.setDisableLazyLoading(true)

I think that is what you are looking for.

@belovaf
Copy link
Author

belovaf commented Feb 2, 2022

I saw this flag, but for nullable fields it becomes indistinguishable when this property is unloaded and when this property is null.
Can i at least set query.setFailOnLazyLoading(true) somehow?

@rbygrave
Copy link
Member

rbygrave commented Feb 2, 2022

but for nullable fields it becomes indistinguishable when this property is unloaded and when this property is null.

Only if you don't know if the property was part of the projection AND you don't care to use Ebean's API / BeanState to determine if the property was loaded.

Can i at least set query.setFailOnLazyLoading(true) somehow?

You need to explain more fully what the use case is, what you are trying to achieve and why setDisableLazyLoading(true) is not sufficient. That is, explain why you think a query.setFailOnLazyLoading(true) feature would help your use case.

@belovaf
Copy link
Author

belovaf commented Feb 2, 2022

Sure! For read use cases we use the following steps:

  1. Select and fetch only needed fields from database as ebean entities.
  2. Convert ebean entities with mapstruct (mapping framework) to nested projection dto classes.
  3. Serialize dto as json and return it to a user.

But it is hard to verify, that projection classes contains only selected fields.
In case when selected and dto fields don't match I have 2 options:

  1. Lazy load missing fields
  2. Set missing fields as nulls

I don't really like both options. I want at least to prohibit lazy load for this query and fail fast behavior which allows me to detect such problems in my tests relatively fast.

But ideal solution for me looks like this:

  1. I describe to ebean my projection classes with annotations
  2. Select and fetch only needed fields from database as ebean projections directly without conversions.

Performance of this solution can be much better than select entities, because projections are read only and don't need any additional state.
And I will not need to enumerate selected fields, ebean can determine what to select from projection classes.

Other sql/orm frameworks have similar functionality:
QueryDSL: @QueryProjection
Blaze persistence: @EntityView

@rbygrave
Copy link
Member

rbygrave commented Feb 2, 2022

and fail fast behavior which allows me to detect such problems in my tests relatively fast.

So one problem/issue is that we want to have a failing fast mechanism when doing the partial query + mapping it to DTO's + when the mapping is incorrect (e.g. the DTO property name and entity property names don't match and this isn't in the mapping).

For this case we want to detect this and throw an exception rather than lazy load or leave null. Something like query.setThrowOnLazyLoad(true). Assuming I got that right yes that makes sense to me.

Performance of this solution can be much better than select entities, because projections are read only and don't need any additional state.

While this is absolutely true of Hibernate generally this is not the case with Ebean. More specifically with Hibernate (using dynamic proxies) - 1. Hibernate doesn't really support partially loaded entities and 2. For dirty detection it needs to store the values additionally in the session - this means it does use a lot more memory regardless of whether the entity is read only or not. So yes, for Hibernate DTO queries are considered a lot cheaper than entity queries for those reasons and this approach is promoted a LOT (and so then a lot of people generally assume Ebean has similar issues which it doesn't).

This isn't the case for Ebean because we do dirty detection on the beans themselves and this isn't any extra cost until you mutate the bean itself. That is, there is no extra state or memory consumption or cost with Ebean. For Ebean there is very little difference between a DTO query and an entity query in terms of cost. Hibernate with enhancement similarly reduces their problem but they still don't support partial objects yet so still push DTO queries.

If we projected straight to a "nested DTO Graph" with Ebean we'd approximately be saving extra instantiation of entity beans + the mapping code execution (mapping entity -> dto).

Blaze @EntityView

Blaze here is mapping to interfaces and not to DTO's per say so these are Blaze specific "proxies". As I see it we would prefer plain old DTO's that have no attachment to a framework/library. For myself, I have always seen EntityView as addressing specific limitations with Hibernate that Ebean does not have rather than addressing the issue of "projecting to a complex DTO graph". Do I have the wrong impression about this?

QueryDSL @QueryProjection

You will have noticed that we literally don't need that annotation with Ebean. We can use constructors in our DTO's without needing anything explicit like that.

In general the issue wrt nested DTO mapping is that "nested" translates to cardinality *ToOne or *ToMany ... and we are now building a graph. If we want to build a "consistent" graph then we need to know "identity" and use a "persistence context" to de-duplicate instances etc. So we pretty quickly either need to have DTO's that start looking like entity beans (but ideally we don't annotate the DTO's at all) ... OR ... derive some meta data (property mapping) that maps Id properties and something like simulate them being "read only entities".

Hmmm.

@belovaf
Copy link
Author

belovaf commented Feb 2, 2022

Thank you very much for so fast, complete and helful responses!

You got first part absolutely correct, query.setThrowOnLazyLoad(true) will be relativeley easy fix for my problem and let me detect mapping problems much easier.

QueryDSL can map to plain dtos with dynamic constructor invocation like ebean. But i think that @QueryProjection is a big adventage, because it will generate class, where constructor parameters are expressions. So i can enumerate selected fields and get compile time checking for these expressions. Look at this code:

QUser user = QUser.user;
 List <UserInfo> result = querydsl.from(user)
     .where(user.valid.eq(true))
     .select(new QUserInfo(user.firstName, user.lastName))
     .fetch();

user.firstName is expression, not String.

So i think map to a plain dto is good, but if some annotation can integrate my dto with type safe query builder I will prefer this approach.

You are absolutely right about mapping to nested dtos, I think them will be look like read only entities.

I think, the main problem of partial objects is not technical (i don't care about several null fields) but is type safety.
If i create projection, I want to declare exact type, which would represent this projection.

Currently I'm trying to use theese approches with ebean:

  1. Additional entity with @view annotation, mapped to the same table.
  2. Additional interface, implemented by entity.
  3. Mapping to plain dtos with mapstruct.

All of them technically solve my problem, but have some limitations and drawbacks.

@rbygrave
Copy link
Member

rbygrave commented Feb 22, 2022

Just to note that the QueryDSL query could be written in Ebean with type safe projection and predicates like:

 List <UserInfo> result = new QUser()
     .valid.eq(true)
     .select(firstName, lastName)
     .asDto(UserInfo.class)  // turn it into a DtoQuery last
     .findList();

@belovaf
Copy link
Author

belovaf commented Feb 22, 2022

Yes, but the main difference is that asDto(UserInfo.class) will be checked for columns missmatch only at runtime.

select(new QUserInfo(user.firstName, user.lastName)) will be checked at compile time.

@rbygrave
Copy link
Member

Just to say that I have had a couple of thoughts on this and probably had an idea to try out with a view of supporting mapping into "nested dto's that contain ToOne and ToMany style relationships" (as opposed to "flat only dtos").

@belovaf If you put up an example repo of what QueryDSL was doing I'd take a look. I'd be especially interested in an example that "wasn't flat" / where the DTO's contained ToMany or ToOne or both.

So an update to say there is something here I'm keen to try out and hopefully I get a crack at that in the next month or so.

@rbygrave rbygrave pinned this issue Apr 6, 2022
@rbygrave
Copy link
Member

rbygrave commented May 3, 2022

The first part of the plan for this is PR #2626 - going into Ebean 13.6.0. This provides an "intercept object" optimised for the readOnly + disableLazyLoading case - that is, the case where we build an ORM graph and want to map that into a plain DTO graph (of arbitrary complexity, ie. include ToMany / ToOne etc).

@rbygrave rbygrave self-assigned this May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants