search

gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.51k stars 372 forks source link

ValueProxy in EntityProxy updated but when EntityProxy is sent to server it's valueProxy is a reinitialised object with null values #7446

Closed dankurka closed 9 years ago

dankurka commented 9 years ago

Originally reported on Google Code with ID 7449

Found in GWT Release trunk (Subversion trunk@11088)

Encountered on OS / Browser - Win7 / FF11:

Detailed description (please be as specific as possible):

In reference to code example below.  The setRole() appears to work as the editableUser
details in it's data as having a role of adminRole.  Debugging and break pointing in
the save method on the server side shows that the the User object has a Role object
containing no data i.e. a new initialised instance of Role when it should have the
newly set AdminRole data - this is the issue.  

In the code after the setRole I have added a commented out line of getRole() if I add
this line in then the code above works and the save method is passed a User object
with the Admin Role as expected.  

It maybe of help to know that when the getRole() is commented out the state.editedProxies.values()
in method makePayloadOperations() in class AbstractRequestContext contains 1 UserProxyAutoBean
and 1 RoleProxyAutoBean.  Whereas when the line is NOT commented out state.editedProxies.values()
in method makePayloadOperations() in class AbstractRequestContext contains 1 UserProxyAutoBean
and 2 RoleProxyAutoBeans

Shortest code snippet which demonstrates issue (please indicate where
actual result differs from expected result):

public void onGoToEditUser(String email) {
        factory.userRequest().getByEmail("user17@validus-ivc.co.uk").with("role").to(new
Receiver<UserProxy>() {
            @Override
            public void onSuccess(final UserProxy user) {
                assert "User Role".equals(user.getRole().getDisplayName());
                // Retrieve all roles
                factory.roleRequest().getAll().to(new Receiver<List<RoleProxy>>() {
                    @Override
                    public void onSuccess(final List<RoleProxy> roles) {
                        final RoleProxy adminRole = roles.get(1);
                        assert "Admin Role".equals(adminRole.getDisplayName());
                        final UserRequest context = factory.userRequest();
                        final UserProxy editableUser = context.edit(user);
                        editableUser.setRole(adminRole);
                        //editableUser.getRole();
                        context.save(editableUser).with("role").to(new Receiver<UserProxy>()
{
                            @Override
                            public void onSuccess(final UserProxy user) {
                                // Does not get here as issue with null id of Role
on server side causes issue with hibernate
                                assert "Admin Role".equals(user.getRole().getDisplayName());
                            }
                        }).fire();
                     }
                  }).fire();
              }
          }).fire();
}

Note - there are some hibernate / spring config annotations in here but I've removed
these as don't believe they are relevant
@Service(value = UserServiceImpl.class)
public interface UserRequest extends RequestContext {
    Request<UserProxy> getByEmail(String email);

    Request<UserProxy> save(UserProxy user);
}

public class UserServiceImpl {
....
    public User getByEmail(String email) {
        if ( log.isInfoEnabled() ) {
            log.info("getUserByEmail(email=" + email + ")");
        }

        return userDao.get(email);
    }

    public User save(User user) {
        if ( log.isInfoEnabled() ) {
            log.info("save(User[email=" + user.getEmail() + "])");
        }

        userDao.update(user);
        return user;
    }
}

@Service(value = RoleService.class)
public interface RoleRequest extends RequestContext {
    Request<List<RoleProxy>> getAll();
}

public class RoleServiceImpl {
....
    public List<Role> getAll() {
        if ( log.isInfoEnabled() ) {
            log.info("getAll()");
        }

        return roleDao.getAll();
    }
}

Note - I've tried with setters as well but that made no difference.
@ProxyFor(value = Role.class)
public interface RoleProxy extends ValueProxy {
    Long getId();

    String getDisplayName();

    String getDescription();

    boolean isEnabled();
}

public class Role implements Serializable {
    private Long                id;
    private String              displayName;
    private String              description;
    private boolean             enabled;

    ... getters and setters ....
}

public class User {
    private Long                id;
    private int                 version;
    private String              email;
    private String              title;
    private String              firstName;
    private String              initial;
    private String              lastName;
    private Role                role;
... getters and setters ....
}

@ProxyFor(value = User.class, locator = UserLocator.class)
public interface UserProxy extends EntityProxy
{
    Long getId();

    int getVersion();

    String getEmail();
    void setEmail(String email);

    String getTitle();
    void setTitle(String title);

    String getFirstName();
    void setFirstName(String firstname);

    String getInitial();
    void setInitial(String initial);

    String getLastName();
    void setLastName(String lastname);

    RoleProxy getRole();
    void setRole(RoleProxy role);

    String getFullName();
}

Workaround if you have one:

add a getxxx() prior to calling server side code as detailed above.

Links to relevant GWT Developer Forum posts:

Link to patch posted at http://gwt-code-reviews.appspot.com:

Reported by nroberts.web on 2012-06-25 07:30:51

dankurka commented 9 years ago 
Looks a lot like issue 7186 (calling the getter would edit() the proxy); there could
be a side-effect of some other change made recently though.

Would you mind checking what goes on the wire? Is the RoleProxy being sent? with all
its properties? (copy/paste the request payload here too)

Reported by t.broyer on 2012-06-25 08:51:10

dankurka commented 9 years ago 

Looking at the editedProxies intercepted on the way to the server's save method then
without the getRole() line included the single RoleProxyAutoBean contains the original
Role values
state.editedProxies.values().table.(RoleProxy Instance).value.data.JSONObject is {"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0}

the UserProxy is
the state.editedProxies.values().table.(UserProxy Instance).value.data.JSONObject is
{"id":"666","lastName":"Jones","title":"Mr","email":"another@secretagent.co.uk","role":{"id":"2","enabled":true,"description":"Admin
Role","displayName":"Admin Role","version":0},"fullName":"Mr Fred Jones","firstName":"Fred","version":0}

So yes it does appear to be sending the roleproxy with it's values in the UserProxy
instance.

with the getRole() line included
the first of the TWO RoleProxyAutoBeans is {"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0}

and the second is {"id":"2","enabled":true,"description":"Admin Role","displayName":"Admin
Role","version":0}

The userProxy is {"id":"666","lastName":"Jones","title":"Mr","email":"another@secretagent.co.uk","role":{"id":"2","enabled":true,"description":"Admin
Role","displayName":"Admin Role","version":0},"fullName":"Mr Fred Jones","firstName":"Fred","version":0}

A unit test which replicates this issue produces the following output with dumpPayload
argument:

Without getRole()
25 Jun 2012 10:10:36,830 INFO  [                         ] UserServiceImpl        
       - getUserByEmail(email=another@secretagent.co.uk)
25 Jun 2012 10:10:36,834 INFO  [                         ] UserLocator            
       - find(id=666)
<<< {"S":[true],"O":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","V":"MC4w","P":{"id":"666","lastName":"Jones","title":"Mr","email":"another@secretagent.co.uk","role":{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"2","Y":1},"fullName":"Mr
Fred Jones","firstName":"Fred","version":0},"S":"IjY2NiI=","O":"UPDATE"},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0},"R":"2","Y":1}],"I":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","S":"IjY2NiI="}]}
>>> {"F":"uk.co.validus.validate.shared.request.VAlidateRequestFactory","I":[{"O":"joX8u7lnnMBAB9IeJvqpNHAAgYg="}]}
25 Jun 2012 10:10:38,023 INFO  [                         ] RoleServiceImpl        
       - getAll()
<<< {"S":[true],"O":[{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"2","enabled":true,"description":"Admin
Role","displayName":"Admin Role","version":0},"R":"2","Y":2},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0},"R":"2","Y":1}],"I":[[{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"2","Y":1},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"2","Y":2}]]}
>>> {"F":"uk.co.validus.validate.shared.request.VAlidateRequestFactory","O":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","V":"MC4w","P":{"role":{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","C":2,"R":"1"}},"S":"IjY2NiI=","O":"UPDATE"},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0},"R":"1","C":1,"O":"PERSIST"}],"I":[{"P":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","S":"IjY2NiI="}],"R":["role"],"O":"IojeDiqdNZ2uvxHs9MgmZEyYr1Q="}]}
25 Jun 2012 10:10:51,142 INFO  [                         ] UserLocator            
       - find(id=666)
25 Jun 2012 10:11:45,884 INFO  [                         ] UserServiceImpl        
       - save(User[email=another@secretagent.co.uk])
25 Jun 2012 10:11:57,405 INFO  [                         ] UserLocator            
       - find(id=666)
<<< {"S":[true],"O":[{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"1","C":1},{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","V":"MC4w","P":{"id":"666","lastName":"Jones","title":"Mr","email":"another@secretagent.co.uk","role":{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","C":2,"R":"1"},"fullName":"Mr
Fred Jones","firstName":"Fred","version":0},"S":"IjY2NiI=","O":"UPDATE"},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"enabled":false,"version":0},"R":"1","C":2}],"I":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","S":"IjY2NiI="}]}
25 Jun 2012 10:11:57,411 INFO  [                         ] TransactionalTestExecutionListener
- Rolled back transaction after test execution for test context [[TestContext@fda1a15
testClass = UserRequestTest, locations = array<String>['classpath:/uk/co/validus/validate/applicationContext-test-infra.xml',
'classpath:/uk/co/validus/validate/applicationContext-security.xml', 'classpath:/uk/co/validus/validate/server/applicationContext-hibernate.xml'],
testInstance = uk.co.validus.validate.shared.request.UserRequestTest@fc306ed, testMethod
= testUpdateUsersRole@UserRequestTest, testException = java.lang.AssertionError: expected:<Admin
Role> but was:<null>]]

With getRole()
25 Jun 2012 10:14:55,005 INFO  [                         ] UserServiceImpl        
       - getUserByEmail(email=another@secretagent.co.uk)
25 Jun 2012 10:14:55,071 INFO  [                         ] UserLocator            
       - find(id=666)
<<< {"S":[true],"O":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","V":"MC4w","P":{"id":"666","lastName":"Jones","title":"Mr","email":"another@secretagent.co.uk","role":{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"2","Y":1},"fullName":"Mr
Fred Jones","firstName":"Fred","version":0},"S":"IjY2NiI=","O":"UPDATE"},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0},"R":"2","Y":1}],"I":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","S":"IjY2NiI="}]}
>>> {"F":"uk.co.validus.validate.shared.request.VAlidateRequestFactory","I":[{"O":"joX8u7lnnMBAB9IeJvqpNHAAgYg="}]}
25 Jun 2012 10:14:55,143 INFO  [                         ] RoleServiceImpl        
       - getAll()
<<< {"S":[true],"O":[{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"2","enabled":true,"description":"Admin
Role","displayName":"Admin Role","version":0},"R":"2","Y":2},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0},"R":"2","Y":1}],"I":[[{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"2","Y":1},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"2","Y":2}]]}
>>> {"F":"uk.co.validus.validate.shared.request.VAlidateRequestFactory","O":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","V":"MC4w","P":{"role":{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","C":2,"R":"1"}},"S":"IjY2NiI=","O":"UPDATE"},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"1","enabled":true,"description":"User
Role","displayName":"User Role","version":0},"R":"1","C":1,"O":"PERSIST"},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"2","enabled":true,"description":"Admin
Role","displayName":"Admin Role","version":0},"R":"1","C":2,"O":"PERSIST"}],"I":[{"P":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","S":"IjY2NiI="}],"R":["role"],"O":"IojeDiqdNZ2uvxHs9MgmZEyYr1Q="}]}
25 Jun 2012 10:14:55,174 INFO  [                         ] UserLocator            
       - find(id=666)
25 Jun 2012 10:14:55,705 INFO  [                         ] UserServiceImpl        
       - save(User[email=another@secretagent.co.uk])
25 Jun 2012 10:14:55,706 INFO  [                         ] UserLocator            
       - find(id=666)
<<< {"S":[true],"O":[{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","R":"1","C":1},{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","V":"MC4w","P":{"id":"666","lastName":"Jones","title":"Mr","email":"another@secretagent.co.uk","role":{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","C":2,"R":"1"},"fullName":"Mr
Fred Jones","firstName":"Fred","version":0},"S":"IjY2NiI=","O":"UPDATE"},{"T":"pATBufCq5F8tp1wx04pkTLKP51k=","P":{"id":"2","enabled":true,"description":"Admin
Role","displayName":"Admin Role","version":0},"R":"1","C":2}],"I":[{"T":"u8MIkYN$z1SH6i01UkB4kVzZPJ0=","S":"IjY2NiI="}]}
25 Jun 2012 10:14:55,730 INFO  [                         ] TransactionalTestExecutionListener
- Rolled back transaction after test execution for test context [[TestContext@10033620
testClass = UserRequestTest, locations = array<String>['classpath:/uk/co/validus/validate/applicationContext-test-infra.xml',
'classpath:/uk/co/validus/validate/applicationContext-security.xml', 'classpath:/uk/co/validus/validate/server/applicationContext-hibernate.xml'],
testInstance = uk.co.validus.validate.shared.request.UserRequestTest@10033674, testMethod
= testUpdateUsersRole@UserRequestTest, testException = [null]]]
25 Jun 2012 10:14:55,742 INFO  [                         ] GenericApplicationContext
     - Closing org.springframework.context.support.GenericApplicationContext@fccc546:
startup date [Mon Jun 25 10:14:42 BST 2012]; root of context hierarchy
25 Jun 2012 10:14:55,745 INFO  [                         ] DefaultListableBeanFactory
    - Destroying singletons in org.springframework.beans.factory.support.DefaultListableBeanFactory@10051978:
defining beans [org.springframework.context.annotation.internalConfigurationAnnotationProcessor,org.springframework.context.annotation.internalAutowiredAnnotationProcessor,org.springframework.context.annotation.internalRequiredAnnotationProcessor,org.springframework.context.annotation.internalCommonAnnotationProcessor,org.springframework.context.annotation.internalPersistenceAnnotationProcessor,userDaoHibernateTest,userServiceImplSecurityTest,passwordEncoderTest,abstractJdbcUnit4TestBase,claimLocator,claimDao,claimService,emailService,roleDao,userDao,userPasswordPolicyDao,userLocator,referenceDataService,roleService,userDetailsService,userService,expiredPasswordController,resetPasswordController,securityService,xsrfTokenService,applicationInitListenerBean,updatePasswordController,settingsDao,propertyConfigurer1,propertyConfigurer2,dataSource,jdbcTemplate,org.springframework.security.filterChains,org.springframework.security.filterChainProxy,org.springframework.security.web.PortMapperImpl#0,org.springframework.security.config.authentication.AuthenticationManagerFactoryBean#0,org.springframework.security.authentication.ProviderManager#0,org.springframework.security.web.context.HttpSessionSecurityContextRepository#0,org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy#0,org.springframework.security.web.savedrequest.HttpSessionRequestCache#0,org.springframework.security.access.vote.AffirmativeBased#0,org.springframework.security.web.access.intercept.FilterSecurityInterceptor#0,org.springframework.security.web.access.DefaultWebInvocationPrivilegeEvaluator#0,org.springframework.security.authentication.AnonymousAuthenticationProvider#0,org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint#0,org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter#0,org.springframework.security.userDetailsServiceFactory,org.springframework.security.web.DefaultSecurityFilterChain#0,org.springframework.security.authentication.dao.DaoAuthenticationProvider#0,org.springframework.security.authentication.DefaultAuthenticationEventPublisher#0,org.springframework.security.authenticationManager,org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler#0,org.springframework.security.access.method.DelegatingMethodSecurityMetadataSource#0,org.springframework.security.access.vote.AffirmativeBased#1,org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor#0,org.springframework.security.methodSecurityMetadataSourceAdvisor,org.springframework.aop.config.internalAutoProxyCreator,passwordEncoder,xsrfSecurityFilter,messageSource,sessionFactory,transactionManager,org.springframework.transaction.annotation.AnnotationTransactionAttributeSource#0,org.springframework.transaction.interceptor.TransactionInterceptor#0,org.springframework.transaction.config.internalTransactionAdvisor,transactionTemplate,org.springframework.context.annotation.ConfigurationClassPostProcessor$ImportAwareBeanPostProcessor#0];
root of factory hierarchy

Thank you for your quick response please let me know if there is anything else I can
provide.

Reported by nroberts.web on 2012-06-25 09:54:05

dankurka commented 9 years ago 
Ah, so it *is* the same as issue 7186, but the implications with a ValueProxy are much
different from those with an EntityProxy.

The workaround is to explicitly edit() the proxy:

   editableUser.setRole(context.edit(adminRole));

Reported by t.broyer on 2012-06-25 11:08:34

dankurka commented 9 years ago 
Thank you for this information, we too had originally discovered this issue through
our driver / editor setup as our comboboxes are populated with lists from service calls
such as the retrieval of roles.  Adjusting our list and then saving caused this problem.

Please could you advise as to whether this is likely to be fixed in the GWT 2.5 release?

Also would you be so kind as to advise / point me in the direction of the best way
to cancel / destroy a RequestContext or an alternative approach i.e.
Get requestContext from factory
Using requestContext get mutable version of Proxy
Pass proxy and requestContext to driver's edit() method
User is shown screen but rather than saving any changes just exits the screen

Returning to the screen tries to repeat above process, get requestContext, etc... and
fails with - Attempting to edit an EntityProxy previously edited by another RequestContext

Thank you

Reported by nroberts.web on 2012-06-25 12:52:37

dankurka commented 9 years ago 
I can't tell if it'll make it into the 2.5 release. Given there exists an easy workaround,
it might very well be pushed to 2.5.1.

As for your other question, there's no way to "destroy" a RequestContext other than
clearing any reference to it so it can be garbage-collected.

Reported by t.broyer on 2012-06-25 16:17:33

dankurka commented 9 years ago 
Ok I'll wait and see if it makes it into the 2.5 release, thanks for your help.

Reported by nroberts.web on 2012-06-26 13:14:32