Incompatibility between RequestFactory and JPA collections with orphanRemoval=true

[Ryan McFall]

After much stepping through GWT and Hibernate source code, I believe I
have come upon an incompatibility in the way that RequestFactory and
Hibernate/JPA interact.

I have a domain object named Survey, and a proxy named SurveyProxy.
Survey contains a Set of SurveyPermission objects, as follows:

public class Survey {
private Set<SurveyPermission> surveyPermissions;

@OneToMany(mappedBy="survey", orphanRemoval=true)
@Cascade(value={CascadeType.ALL})
public Set<SurveyPermission> getSurveyPermissions () {
return surveyPermissions;
}
}

SurveyProxy contains a method to retrieve the set of SurveyPermissions
owned by the Survey:
public Set<SurveyPermissionProxy> getSurveyPermissions ();

In my client code, I load a Survey object, and then send it back to
the server (unmodified), and then perform a save on the object:

Session session = HibernateUtil.getCurrentSession();
session.save(survey);
session.flush();

When I do this, I get an exception from Hibernate:
A collection with cascade="all-delete-orphan" was no longer referenced
by the owning entity instance:
edu.hope.cs.surveys.dao.pojo.Survey.surveyPermissions

Research on the web shows that the cause of this is when the setter
for a collection is called, when Hibernate is expecting to manage the
collection itself.

Going through the RequestFactoryServlet code, I see that
SimpleRequestProcessor's processOperationMessages (final RequestState
state, RequestMessage re) method (called by RequestFactoryServlet in
doPost) calls
bean.accept ()
which basically uses the visitor pattern to call setters for each
property referenced by the proxy.

But this is problematic for Hibernate, because now some outside code
has called the setter for one the collections it is managing, and at
least when orphanRemoval is set to true, Hibernate does not like this.

I'm afraid the answer to this question is "Yes, this is a problem, you
can't use collections with orphanRemoval set to true." If anyone
familiar enough with the RequestFactory implementation and Hibernate
semantics can verify that this is a problem, then I can file a bug
report and try to work around it in my code.

Thanks!
Ryan

[Thomas Broyer]

If you haven't changed the collection in any way, and Survey is mapped as an EntityProxy (not a ValueProxy), the setter shouldn't be called.

But back to your issue: if Hibernate is supposed to manage the collection, you either shouldn't have a setter (probably won't do well with RequestFactory though), or implement your setter as field.retainAll(newValue) (i.e. modifying the set in-place rather than swapping it with a new one)

[Ryan McFall]

I haven't changed the collection, and SurveyProxy is an EntityProxy,
not a value proxy. Here's the code from SimpleRequestProcessor that
handles this:
public boolean visitReferenceProperty(String propertyName, AutoBean<?>
value, PropertyContext ctx) {
// containsKey to distinguish null from unknown
if (flatValueMap.containsKey(propertyName)) {
Class<?> elementType = ctx instanceof CollectionPropertyContext ?
((CollectionPropertyContext) ctx).getElementType() : null;
Object newValue = EntityCodex.decode(state, ctx.getType(),
elementType, flatValueMap.get(propertyName));
Object resolved = state.getResolver().resolveDomainValue
(newValue, false);
service.setProperty(domain, propertyName,
service.resolveDomainClass(ctx.getType()), resolved);
}
return false;
}

There's nothing here that checks whether the collection has been
changed. So unless this is supposed to happen on the client side (so
that the survey permissions object doesn't exist in the payload at
all, and therefore doesn't get visited), I don't see any way for this
to happen. Should client side code be handling this? I can debug
through there and try to figure out what's happening if need be.

Thanks for your help!
Ryan

[Thomas Broyer]

On Monday, June 20, 2011 7:44:56 PM UTC+2, Ryan McFall wrote:

I haven't changed the collection, and SurveyProxy is an EntityProxy,
not a value proxy.  Here's the code from SimpleRequestProcessor that
handles this:
public boolean visitReferenceProperty(String propertyName, AutoBean<?>
value, PropertyContext ctx) {
  // containsKey to distinguish null from unknown
  if (flatValueMap.containsKey(propertyName)) {
    Class<?> elementType = ctx instanceof CollectionPropertyContext ?
((CollectionPropertyContext) ctx).getElementType() : null;
    Object newValue = EntityCodex.decode(state, ctx.getType(),
elementType, flatValueMap.get(propertyName));
    Object resolved = state.getResolver().resolveDomainValue
(newValue, false);
    service.setProperty(domain, propertyName,
service.resolveDomainClass(ctx.getType()), resolved);
  }
  return false;
}

There's nothing here that checks whether the collection has been
changed.  So unless this is supposed to happen on the client side (so
that the survey permissions object doesn't exist in the payload at
all, and therefore doesn't get visited), I don't see any way for this
to happen.  Should client side code be handling this?

Absolutely! That's one of the features of RequestFactory to only send diffs from client to server.

 

I can debug
through there and try to figure out what's happening if need be.

It might be http://code.google.com/p/google-web-toolkit/issues/detail?id=5952

But I still believe you should (independently of RequestFactory) rewrite your setter, otherwise the issue will surface later as some other code will call it.

[Ryan McFall]

I'm trying your suggestion on the setter at the moment; I'm a little
nervous about this due to the volume of posts on the web which make it
sound like trying to do anything intelligent in the setter can lead to
the error I encountered. But your logic makes sense, so it may work.
All my server-side unit tests pass with this in place, so that's at
least optimistic.

I looked at the bug you referenced, which seems possibly to be the
source of the problem. I don't have a many-to-many collection, but
the problem with .equals versus .deepEquals seems like it would exist
whether the collection was many-to-many or one-to-many.

Thanks again for the thoughts - it's great having someone respond so
quickly!
Ryan

> It might behttp://code.google.com/p/google-web-toolkit/issues/detail?id=5952

[Ryan McFall]

Implementing the setSurveyPermissions method as you suggested seems to
have helped a bit. However, I'm still a bit confused as to what the
client-side behavior I'm seeing should be.

As I mentioned, I retrieve and then send an object, and don't make any
modifications to the object on the client. But when I look at the
operations in SimpleRequestProcessor.processOperationMessages, I see
the following as the result of the call to req.getOperations.
Basically, this looks like every object referenced by the Survey proxy
object is being sent over the network and some of them are marked as
needing update. Am I right in that interpretation?

If so, I'm not sure why this is happening. Does the client keep track
of method calls to determine what needs to be updated, or something
else?

Ryan

{"T":"edu.hope.cs.surveys.dao.ISurveyInfo","V":"MC4w","P":{"tags":
[{"T":"edu.hope.cs.surveys.dao.ITag","S":"IjIi"},
{"T":"edu.hope.cs.surveys.dao.ITag","S":"IjEi"}],"surveyPermissions":
[{"T":"edu.hope.cs.surveys.dao.ISurveyPermission","S":"IjMyODki"},
{"T":"edu.hope.cs.surveys.dao.ISurveyPermission","S":"IjMyODgi"},
{"T":"edu.hope.cs.surveys.dao.ISurveyPermission","S":"IjMyOTAi"}],"completionRoutines":
[{"T":"edu.hope.cs.surveys.completion.client.ICompletionRoutine","S":"Ijci"},
{"T":"edu.hope.cs.surveys.completion.client.ICompletionRoutine","S":"Ijgi"}],"thisSurveysQuestions":
[{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","S":"IjEi"},
{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","S":"IjIi"},
{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","S":"IjMi"},
{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","S":"IjQi"}],"authenticationRoutine":
{"T":"edu.hope.cs.surveys.authentication.client.IAuthenticationRoutine","S":"IjIi"},"eligibilityRoutine":
{"T":"edu.hope.cs.surveys.eligibility.client.IEligibilityRoutine","S":"IjUi"}},"S":"IjEi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.authentication.client.IAuthenticationRoutine","V":"MC4w","S":"IjIi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.eligibility.client.IEligibilityRoutine","V":"MC4w","P":
{"parameterDescriptions":
[{"T":"edu.hope.cs.surveys.dao.IRoutineParameterDescription","S":"IjEi"},
{"T":"edu.hope.cs.surveys.dao.IRoutineParameterDescription","S":"IjIi"}]},"S":"IjUi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.IRoutineParameterDescription","V":"MC4w","S":"IjEi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.IRoutineParameterDescription","V":"MC4w","S":"IjIi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.completion.client.ICompletionRoutine","V":"MC4w","S":"Ijci","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.completion.client.ICompletionRoutine","V":"MC4w","S":"Ijgi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","V":"MC4w","P":
{"thisQuestionsChoices":
[{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","S":"IjEi"}],"choiceGroup":
{"T":"edu.hope.cs.surveys.dao.ChoiceGroupProxy","S":"IjEi"}},"S":"IjEi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceGroupProxy","V":"MC4w","P":
{"choices":
[{"T":"edu.hope.cs.surveys.dao.pojo.ChoiceGroupItemProxy","S":"IjEi"},
{"T":"edu.hope.cs.surveys.dao.pojo.ChoiceGroupItemProxy","S":"IjIi"}]},"S":"IjEi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.pojo.ChoiceGroupItemProxy","V":"MC4w","P":
{"choice":
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","S":"IjEi"}},"S":"IjEi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","V":"MC4w","S":"IjEi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.pojo.ChoiceGroupItemProxy","V":"MC4w","P":
{"choice":
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","S":"IjIi"}},"S":"IjIi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","V":"MC4w","S":"IjIi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","V":"MC4w","P":
{"choice":
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","S":"IjEyIg=="}},"S":"IjEi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","V":"MC4w","S":"IjEyIg==","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","V":"MC4w","P":
{"thisQuestionsChoices":
[{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","S":"IjIi"},
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","S":"IjMi"},
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","S":"IjQi"},
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","S":"IjUi"}]},"S":"IjIi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","V":"MC4w","P":
{"choice":
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","S":"IjMi"}},"S":"IjIi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","V":"MC4w","S":"IjMi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","V":"MC4w","P":
{"choice":
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","S":"IjQi"}},"S":"IjMi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","V":"MC4w","S":"IjQi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","V":"MC4w","P":
{"choice":
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","S":"IjUi"}},"S":"IjQi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","V":"MC4w","S":"IjUi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.SurveyChoiceProxy","V":"MC4w","P":
{"choice":
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","S":"IjYi"}},"S":"IjUi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ChoiceProxy","V":"MC4w","S":"IjYi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","V":"MC4w","S":"IjMi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ISurveyQuestion","V":"MC4w","S":"IjQi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ISurveyPermission","V":"MC4w","S":"IjMyODki","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ISurveyPermission","V":"MC4w","S":"IjMyODgi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ISurveyPermission","V":"MC4w","S":"IjMyOTAi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ITag","V":"MC4w","S":"IjIi","O":"UPDATE"}
{"T":"edu.hope.cs.surveys.dao.ITag","V":"MC4w","S":"IjEi","O":"UPDATE"}

[Thomas Broyer]

On Monday, June 20, 2011 9:08:31 PM UTC+2, Ryan McFall wrote:

Implementing the setSurveyPermissions method as you suggested seems to
have helped a bit.  However, I'm still a bit confused as to what the
client-side behavior I'm seeing should be.

As I mentioned, I retrieve and then send an object, and don't make any
modifications to the object on the client.  But when I look at the
operations in SimpleRequestProcessor.processOperationMessages, I see
the following as the result of the call to req.getOperations.
Basically, this looks like every object referenced by the Survey proxy
object is being sent over the network and some of them are marked as
needing update.  Am I right in that interpretation?

Actually, only the "stable IDs" of the referenced objects are sent (T=type, S=server ID, they are IdMessage objects; top level objects –the ones with a "P" property and/or "UPDATE" operation– are OperationMessage).

I interpret this as meaning it thinks the collection has changed, which could very well be a manifestation of the issue I already linked to.

If so, I'm not sure why this is happening.  Does the client keep track
of method calls to determine what needs to be updated, or something
else?

When you RequestContext#edit() a proxy, a mutable copy is created; and when you fire(), the diff is built by comparing with the immutable version (using AutoBeanUtils.diff that's mentionned in the issue report).

I'm not sure what the UPDATE means here (I don't remember).

[Ryan McFall]

Thanks for the help interpreting the JSON. After I posted the
original message I noted that it looked like only objects referenced
in collections were being updated.

The interesting part of your description of how the diffs are built is
that I took the call to RequestContext#edit on the proxy out, to see
what would happen if I tried to save the original proxy object. The
JSON string I posted actually came from doing that, so something is
definitely strange. If I do execute the edit method, the generated
JSON is the same.

Do you know offhand in which class the diff is built, so that I can
look to see if the bug that you pointed out is indeed the source of
the problem?

Thanks,
Ryan

[Thomas Broyer]

See http://code.google.com/p/google-web-toolkit/source/browse/trunk/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java

Actually, passing a proxy to a method implicitly edit()s it (see retainArg, called from addInvocation, called by the generated implementation of your RequestContext methods), that's why you don't see a difference.

The diff is then done in in makeOperationMessage, called by makePayloadOperations for each proxy in editedProxies (either create()d, edit()d, or used as an invocatino argument).

This is almost the same code as in isChanged, which seems to confirm that what you're seeing is due to http://code.google.com/p/google-web-toolkit/issues/detail?id=5952