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
LazyInitializationException on an entity with ManyToMany, pagination and mapstruct #5629
Comments
+1 |
The lazy initialization is a right approach. For such problem, a service layer would do its duty, fetching all data needed for the client which can have more than one DB calls. You don't need to fetch children entities only if you in the Hibernate session. A Hibernate session won't be kept alive in the modern web application architecture after fetching a parent entity. |
If you change the line: If "Job" has service, it works correctly |
the question here is:
|
I think that the original issue is that we try to serialize a "many" collection which is both an issue with lazy loading and for performance if the collection gets important. For instance on OneToMany relationship we hide the collection from the response. |
any result? |
Please @Codefans-fan don't spam everyone if you don't work on the issue. |
The problem remains intact when using a service. And with no service it fails because the transaction is done. A solution could be to have in
There is a discussion about this: mapstruct/mapstruct#778 |
JobMapper can be like this:
and in JobResource.getAllJobs method: return new ResponseEntity<>(jobMapper.toDtoLazy(page.getContent()), headers, HttpStatus.OK); |
@ctamisier I don't think we can address all use cases, your approach with toDtoLazy works for one relationship, what if you have several ones and some of them are bi-directional or creating a cycle? I think this is the case where you must write ad-hoc mapper code and MapStruct just gets in your way. I like @cbornet RESTful approach, it would produce a very intuitive swagger doc. @deepu105 and @sendilkumarn this is the kind of tips that would be very useful in a JHipster book ;) |
Other advantages of the REST link:
For the form, it could be done as an HATEOAS-compliant [ {
"id": 1001,
"jobTitle": "some job",
"links": [ {
"rel": "tasks",
"href": "http://localhost:8080/api/jobs/1001/tasks"
} ]
},
{
"id": 1003,
"jobTitle": "some other job",
"links": [ {
"rel": "tasks",
"href": "http://localhost:8080/api/jobs/1003/tasks"
} ]
}
] GET /api/tasks (not paginated) [ {
"id": 1002,
"title": "some task",
"links": [ {
"rel": "jobs",
"href": "http://localhost:8080/api/tasks/1002/jobs"
} ]
} ] GET /api/tasks/1002/jobs (paginated) where task 1002 is linked to job 1001 but not to job 1003 [ {
"id": 1001,
"jobTitle": "some job",
"links": [ {
"rel": "tasks",
"href": "http://localhost:8080/api/jobs/1001/tasks"
} ]
} ] |
yes very good. |
@ctamisier if you mean eager fetching for performance reason because the client is sure to need the info, then this shouldn't be the default and I'm not sure we should generate something for that. |
@jhipster/developers if we agree on the links, I can do the work for the backend but I would probably need help for the front... For instance, on the grid, the "many" value should probably be a link to the child grid filtered on the parent instead of the list of child IDs as we do currently. |
For the backend, you don't need to do a lot of thing, after #5540 , the entity listing will be improved with filtering, so you can call : /api/tasks?jobId.equals=1001 and it will return the matching "tasks". |
@gzsombor that's the point of outputting a link : no automatic fetching of the child collections, only one HTTP query, only one Hibernate call. |
But the browser still needs to issue a separate HTTP query to fetch the entities, isn't it ? |
It depends on what you display in the browser 😃 |
@gzsombor will the criteria filtering work with many-to-many relationships ? |
I just realized that many-to-many are currently fetched eagerly in the repository (
|
Yes, LAZY should be by default (it is currently the case) but the use findAllWithEagerRelationships and findOneWithEagerRelationships is required depending the cases: If we have:
If we have:
True when we don't use DTO otherwise the methods in *Service class are All this to say that I would keep |
N+1 queries are not necessary bad when your related entities can be easily cached and don't change often. |
Yes you're right. |
@cbornet : in the current patch, the many-to-many relations are not covered yet (only the one-to-one and many-to-one), but it's fairly simple to add them. |
@ctamisier @Query("select distinct test_many_to_many from TestManyToMany test_many_to_many left join fetch test_many_to_many.testEntities left join fetch test_many_to_many.testMapstructs left join fetch test_many_to_many.testServiceClasses left join fetch test_many_to_many.testServiceImpls left join fetch test_many_to_many.testInfiniteScrolls left join fetch test_many_to_many.testPagers left join fetch test_many_to_many.testPaginations left join fetch test_many_to_many.testCustomTableNames")
List<TestManyToMany> findAllWithEagerRelationships(); I'm pretty sure that's inefficient. |
The only configuration that does not make much sense to me is DTO without
service layer, then there can still be use cases for this.
So I would keep the options and either solve the issue or at least warn the
user.
Le 6 août 2017 11:58 AM, "Deepu K Sasidharan" <notifications@github.com> a
écrit :
… @jdubois <https://github.com/jdubois> @cbornet
<https://github.com/cbornet> @gzsombor <https://github.com/gzsombor>
@gmarziou <https://github.com/gmarziou> @ctamisier
<https://github.com/ctamisier> sorry I didnt read the entire thread its
very long so sorry if this was already discussed. Is the issue happening
only when there is no service layer? we laos had quite a number of issues
in the past for such a combination. Would it make sense to enable service
layer if DTO is choosen? I.e make service layer mandatory for DTO option.
It might solve some head aches. I dont see why DTO needs to be used without
a service layer as they are already in the service layer and we even
recommend using service with DTO. WDYT?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5629 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AATVo5x0OWirRwPtl1TOrMEPQ1a-0XaBks5sVY5MgaJpZM4NChK8>
.
|
My position is still the same :
|
I started to look at what would need to be generated to have child collections fetch endpoints.
CarResource.java /**
* GET /people/:id/owned-cars : get the owned-cars for the owner "id".
*
* @param id the id of the person for which to get the owned-cars
* @return the list of owned-cars for the person "id"
*/
@GetMapping("/people/{id}/owned-cars")
@Timed
public ResponseEntity<List<CarDTO>> getCarsByOwnerId(@PathVariable Long id, @ApiParam Pageable pageable) {
log.debug("REST request to get owned-cars for person : {}", id);
Person person = new Person();
person.setId(id);
Page<Car> page = carRepository.findByOwner(person, pageable);
HttpHeaders headers = PaginationUtil.generatePaginationHttpHeaders(page, "/api/people/" + id + "/owned-cars");
return new ResponseEntity<>(carMapper.toDto(page.getContent()), headers, HttpStatus.OK);
}
/**
* GET /people/:id/driven-cars : get the driven-cars for the driver "id".
*
* @param id the id of the person for which to get the driven-cars
* @return the list of driven-cars for the person "id"
*/
@GetMapping("/people/{id}/driven-cars")
@Timed
public ResponseEntity<List<CarDTO>> getCarsByDriverId(@PathVariable Long id, @ApiParam Pageable pageable) {
log.debug("REST request to get driven-cars for person : {}", id);
Person person = new Person();
person.setId(id);
Page<Car> page = carRepository.findByDriver(person, pageable);
HttpHeaders headers = PaginationUtil.generatePaginationHttpHeaders(page, "/api/people/" + id + "/cars");
return new ResponseEntity<>(carMapper.toDto(page.getContent()), headers, HttpStatus.OK);
}
As for hypermedia links if we want to add them for documentation/ease-of-use, it's easy to add them using spring-hateoas. @GetMapping("/people/{id}")
@Timed
public ResponseEntity<PersonDTO> getPerson(@PathVariable Long id) {
log.debug("REST request to get Person : {}", id);
Person person = personRepository.findOne(id);
PersonDTO personDTO = personMapper.toDto(person);
if (personDTO != null) {
personDTO.getLinks().add(linkTo((methodOn(CarResource.class).getCarsByDriverId(person.getId(), null))).withRel("drivenCars"));
personDTO.getLinks().add(linkTo((methodOn(CarResource.class).getCarsByOwnerId(person.getId(), null))).withRel("ownedCars"));
}
return ResponseUtil.wrapOrNotFound(Optional.ofNullable(personDTO));
} which will output for {
"id": 1,
"name": "qrehgtrh",
"links": [
{
"rel": "drivenCars",
"href": "http://localhost:8080/api/people/1/driven-cars"
},
{
"rel": "ownedCars",
"href": "http://localhost:8080/api/people/1/owned-cars"
}
]
} |
Sorry, I've started implementing the many-to-many stuff, the library part is done, here. After it's merged, and a new release is made, then the generator can be adjusted. For the two-way references between a many-to-many relation (or with any other type of relation), is not solved automatically. Maybe we could re-generate both the entities in this case? Other than, I don't get why this hateoas thing is good. Yes, with generating that many links, you could have a very generic client, which could automatically download a lots of entities, without knowing the meaning of them. Which is nice, if you want to crawl your site :-) But if you develop an actual client, then you will know, that people entity have 'drivenCars' and 'ownedCars' property, and in this case, following REST rules, you could be expecting them reachable under {entity URL}/driverCars and {entity URL}/ownedCars, instead of this additional indirection. If not, I would consult the generated swagger documentation :) |
@gzsombor the intent with the links is not to provide HATEOAS / REST level 3. It's just to provide nice-to-have auto-documentation for the humans, not expecting smart clients to use them at runtime. |
Hi, Is it normal that jHipster does not define all relationship as LAZY? Was this choice to reflect JPA definition of relationships? Thank you. |
@Blackdread please don't pollute the thread, for general questions please use stackoverflow |
Sorry for my previous comment @Blackdread - I see that in fact you are an Hibernate master (in ticket #6105 ) and that I should listen to you more :-) Anyway: this as been opened for too long, and I'm not sure we can find an easy solution. I just don't have enough time at the moment to work on this - @gzsombor @cbornet you are probably the 2 best experts here, what do you think? Should we close this? |
There is really a bug so I don't think we can close this without any fix... |
It's been a long time. I don't know whether this bug is fixed or not. Put it in the entity that have ManytoMany relationship. It will reduce considerably the number of query in case the children entities is too large. Hope this help. |
@apovn no it has not been fixed, the bug is still opened. But nobody works on this... |
I don't know why nobody works on this because it throws an exception if I don't add the code as I mentioned in previous comment. In my code: Customer and Ticket have ManyToMany relationship. Do you have a solution for this problem, @jdubois ? |
Nobody is working on it because that doesn't seem to interest anyone... People just have better things to do, that's all. |
ok, i understand. |
The better solution is to fix the transaction handling :
Basically, you need to access your database from a code block, which is in a 'JPA/hibernate session' |
Yes, that would make sense ! |
I like it that we offer the options to have "no DTOs and a service layer", and "no service layer and DTOs". But indeed, it doesn't make much sense to have DTOs without a service layer, and this seems to constantly cause trouble to people. |
@jdubois what I can do is issue the warning if the options are "incompatible" at JDL-parsing time. |
Yes I would also prefer to suppress the DTO and filtering option when
service is not selected. It will reduce lot of headaches for us
Thanks & Regards,
Deepu
…On Mon, Dec 4, 2017 at 1:59 PM, Mathieu ABOU-AICHI ***@***.*** > wrote:
@jdubois <https://github.com/jdubois> what I can do is issue the warning
if the options are "incompatible" as JDL-parsing time.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5629 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABDlF1-MheiTIQwNp5PXoBoT5aXUxp2nks5s8-zNgaJpZM4NChK8>
.
|
I have disabled DTO if service is not selected this won't affect existing apps and JDL as currently it only disables the prompt. @MathieuAA it would be nice if we can do the warning from JDL as well |
Overview of the issue
Hibernate throws an exception when trying to get all Job entities after creating a Job entity.
Motivation for or Use Case
This issue only appears when having an entity with a ManyToMany relationship, being paginated (pager, pagination or infinite-scroll) and dto generated with mapstruct.
It's a little specific but if you use the default JDL from JDL Studio you will have the issue.
Reproduce the error
Use the JDL below to generate entities.
Create a Job entity with no Task.
An exception will be thrown when displaying all Job.
Related issues
Found nothing related.
Suggest a Fix
Maybe by doing an eager load on the ManyToMany relationship like it's done when Job is not paginated.
JHipster Version(s)
4.3.0
JHipster configuration
JHipster Version(s)
JHipster configuration, a
.yo-rc.json
file generated in the root folderEntity configuration(s)
entityName.json
files generated in the.jhipster
directoryJob.json
Task.json
Browsers and Operating System
java version "1.8.0_92"
Java(TM) SE Runtime Environment (build 1.8.0_92-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.92-b14, mixed mode)
git version 2.10.1 (Apple Git-78)
node: v7.8.0
npm: 4.2.0
bower: 1.8.0
gulp:
[23:47:04] CLI version 1.2.2
[23:47:04] Local version 3.9.1
yeoman: 1.8.5
yarn: 0.22.0
Docker version 17.03.1-ce, build c6d412e
docker-compose version 1.11.2, build dfed245
Entity configuration(s)
entityName.json
files generated in the.jhipster
directoryJDL
Browsers and Operating System
Chrome 57 on OS X El Capitan
The text was updated successfully, but these errors were encountered: