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

Detachment : Allow detached objects to have a StateManager #94

Open
andyjefferson opened this issue Jun 10, 2016 · 13 comments
Open

Detachment : Allow detached objects to have a StateManager #94

andyjefferson opened this issue Jun 10, 2016 · 13 comments

Comments

@andyjefferson
Copy link
Member

The (only) reason that this could be useful is so that you can allow fetch of unloaded fields while detached (but while the PMF/EMF is still open).

@chrisco484
Copy link

chrisco484 commented Jun 10, 2016

It may be surprising but that is a use case we have run into a lot and
being able to do that would be of great benefit.

We often get 'attempt to access "blah" but "blah" was not detached'
exceptions and so to fix we have to configure fetch groups in the model
that satisfy the needs of a UI form that needs access to those related
objects.

If detached objects could load refrenced object on demand that would
save a lot of mucking around with fetch groups maybe....

@nlmarco
Copy link

nlmarco commented Jun 10, 2016

If you need to access data at a later time, why do you detach your object in the first place? I mean the word "detach" itself already states clearly that there is no database connection, anymore.

If you want lazy-loading of data, you do not want detachment.

Actually, what you request IMHO contradicts the clean specification of the lifecycle of objects (with everything belonging to it, including transactions) as laid out in the JDO spec (and JPA probably too, since they copied most of JDO).

@nlmarco
Copy link

nlmarco commented Jun 10, 2016

@andyjefferson I didn't notice in the first place that this was your own suggestion. I thought it came from a user. Sorry. You have far more insight and likely thought well about this :-)

Still, I'd like to state that the current behaviour has advantages. If using detachment, now, the code clearly specifies (=> fetch-groups) what data exactly is loaded and what is not. If sth. is silently lazy-loaded, this might IMHO easily cause an inconsistent state, because other transactions might have modified the object (or its relations) already. Therefore, I kindly ask you to make this new behaviour optional - and disabled by default.

@andyjefferson
Copy link
Member Author

@nlmarco I thought the description I put for this issue "The (only) reason that this could be useful ..." was quite clear ;-) Yes, this is a minority use-case (IMHO also) and would be optional, with JDO/JPA spec behaviour being the standard. I registered the issue here as a placeholder for one day when it gets implemented ...

@chrisco484
Copy link

chrisco484 commented Jun 10, 2016

@nlmarco said:

If you want lazy-loading of data, you do not want detachment.

As you would know, in many cases, detachment is not optional when you have a UI form editing a model object as that form remains open across multiple http requests, each http request creating a new PM/EM and throwing it away after the response to the request is completed. In this case we can't avoid detachment and lazy loading would be desirable for the reasons described below:

In our experience, with sophisticated object graphs, getting the fetch groups working sufficiently and optimally isn't trivial. Often the objects referenced from the model object being edited are not being edited themselves but the UI form code may directly or indirectly cause navigation along these references eminating from the detached object. That's where you get issues if you haven't done what we call 'pre-fetching' any objects that your UI form might navigate to.

That's where I think lazy loading from a detached object would be useful - in many of our scenarios it would avoid having to establish the fetch groups and execute the code to perform this prefetch.

Fetch groups for the specific purpose of being used by a particular UI form also have a bit of code smell because they are defined in the .jdo files (model layer) but they are put there purely for a specific form's requirements (UI layer) and often named according to the form they are intended to be used with. In our product this breaks our otherwise 100% separation of concerns between model and UI.

@andyjefferson Ideally any objects thus lazily fetched would also become instantly detached. Would that be the intention?

@nlmarco I do understand your concerns over data inconsistency and so yes this should definitely be an optional feature which defaults to 'off'. However a data inconsistency would probably cause problems (maybe different problems) in both cases (either 'pre-fetched' or 'lazily fetched') when you go to reattach then commit an detached object graph if some of the associated objects had been changed in the DB by, for example, another user during the detachment period.

@renataogarcia
Copy link
Member

@chrisco484 Why do you need to close the the PM? Why not keep the PM and the TX opened?

@chrisco484
Copy link

chrisco484 commented Jun 11, 2016

For many, many years that's how we thought we could do it and so we did it that way but trust me - keeping PMs alive across multiple HTTP requests is a mirage - it seems like a nice place to aim for because you can avoid detachment but in the end it's waaaay more trouble than it's worth. Much better to have everything detached at the end of every HTTP request and the PM closed.

On major issue is that you can't cluster your app and so can't scale because clustering requires serializable model objects (session data is serizalized from server to server or to disk if memory gets tight) and serializing a PC detaches it whether you like it or not and the same PM wouldn't be available on any server that your user's session might happen to be migrated to.

We've broken detachment into two categories:
Immutable detachable objects - objects are attached to UI elements that don't change their values but need a reference to them.
Mutable detachable objects - objects are attached to UI elements that can change (mutate) their values.

For immutables we have a special genericized immutable model reference class that simply dereference the PC - no need for DN detachment here - we just throw away the reference altogether and leave the object with the OID of the previously referenced object so that it can reload if required later.

For mutables we have a special genericized mutable model reference class that performs a DN detachment when the http request's response is complete. It also has a flag called 'stayDetached' which will avoid reattachment until explicitly told to do so - i.e. when the user submits and form validation is successful. At this point reattachment will cause the mutated object to be saved to the DB.

We can could open source these classes if anyone else would find them useful.

They have some generic (non UI) base classes/interfaces and then we have some derived classes specialized for the most excellent Wicket Java UI framework.

@renataogarcia
Copy link
Member

renataogarcia commented Jun 11, 2016

I see... given that you want scalability and availability(since you are replicating your sessions) wouldn't make sense to use a stateless architecture e.g.: not keep the PC in the web session?

Also, with your current approach why not just reattach the PC, apply the changes from the request and commit if you are ready to save or rollback if it's not the case? In that way you don't need special handling in your app and let DN take care of the persistence concerns?

It seems to me that what you want/need wouldn't be solved by this enhancement, given that it would require the pm to be opened anyway.

@chrisco484
Copy link

I see... given that you want scalability and availability(since you are replicating your sessions) wouldn't make sense to use a stateless architecture e.g.: not keep the PC in the web session?

With sticky sessions (new requests are sent to the same server as last request if possible) there's no real performance hit to have stateful sessions as they don't migrate between servers in a cluster frequently - only in low memory scenarios or if shutting down a server.

Also, with your current approach why not just reattach the PC, apply the changes from the request and commit if you are ready to save or rollback if it's not the case? In that way you don't need special handling in your app and let DN take care of the persistence concerns?

That's basically what we're doing if I understand you correctly. We have a mutable detached PC and we reattach when the user submits the form and it passes validation then we commit to save the changes.

The great thing about detached objects is that you can use them as DTOs in your UI without having to write separate DTO classes - a massive developer productivity boost. With the productivity boost this has given us over the 'old school way' of having a separate but similar DTO class hierachy to use in the UI when editing our domain model objects I can only assume that the whole detach/attach feature was designed to service this need and make DTOs redundant avoiding a lot of repetitive code that needs to be kept in sync with the domain model.

It seems to me that what you want/need wouldn't be solved by this enhancement, given that it would require the pm to be opened anyway.

We use "open session/PM in view" pattern so there is always a PM available when all this activity is happening.

@renataogarcia
Copy link
Member

😕 Hmm... so if you are already re-attaching why would you want the ability to load unloaded fields while detached then?

@chrisco484
Copy link

chrisco484 commented Jun 11, 2016

why would you want the ability to load unloaded fields while detached then?

Complex UI forms with lots of different components and modal subforms. They can stay open across many HTTP requests (via AJAX).

The subject of the main form is a detached (mutable) domain model object. Users exploring in the UI by opening child modal forms or by non modal panels that change on the main form according to user choices - all these things can trigger navigation of relationships and thus cause exceptions if not prefetched - that's where allowing lazy loading of unloaded fields on a detached object would be useful.

The Wicket Java UI Framework gives us the power to do all of the funky, powerful user interface things that the cool kids do but it lets smart people do it in Java as it manages all the browser side JavaScript - treating JavaScript almost like machine code: low level, typeless language which you should never write code in directly if possible :)

@renataogarcia
Copy link
Member

So you only re-attach when saving and not when doing the intermediate requests that triggers navigation of relationships?

@chrisco484
Copy link

chrisco484 commented Jun 11, 2016

Yes, we definitely defer reattach until we're ready to commit the changes.

Some forms are editing new objects that are not yet persistent. We don't want to make them persistent until the user has said "OK" to the creation of the new objects. If we were to reattach at the start of each HTTP request that would necessitate making those new objects persistent at that point because reattach = make persistent.

We use JDO in "JPA mode" i.e. all objects are automatically detached on commit. We went from complex problems to simpler ones after we started using JDO in "JPA mode". So at the end of every request there is no PM and nothing attached (by necessity as the PM is closed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants