grails / grails-core

The Grails Web Application Framework
http://grails.org
Apache License 2.0
2.79k stars 949 forks source link

Command object LazyMap databinding not working in grails 3.3.7 #11245

Open swzaidi opened 5 years ago

swzaidi commented 5 years ago

We have application running on grails 2.2.4/java 1.7 and we are upgrading to grails 3.3.7/java1.8. Running the application on grails 3.3.7 we are noticing in a particular case the parameters to command object databinding is not working, same code just runs fine in grails 2.2.4

I have created a repo in github with a sample app reproducing the issue , containing the codebase for both grails 2.2.4(data-binding works fine) and grails 3.3.7(data-binding issue, with error in command object).

GitHub repo URL is https://github.com/swzaidi/sample

I will also inline the HTTP request data and relevant code snippet from sample app for your quick reference.

HTTP Request data

student.id: 1000 courses[1000].course.id: 1000 courses[1001].course.id: 1001 class EnrollmentController {

def save(EnrollmentCommand cmd) {

    println "+++++++ cmd has errors" + cmd.hasErrors()
    println "+++++++ cmd errors " + cmd.errors

}

} class EnrollmentCommand {

Student student
Map<String, CourseCommand> courses = MapUtils.lazyMap(
        new HashMap<String, CourseCommand>(),
        FactoryUtils.instantiateFactory( CourseCommand ) )

}

class CourseCommand { Course course } DataBinding error in grails 3.3.7

Error in object 'student.EnrollmentCommand': codes []; arguments []; default message [No such property: course.id for class: student.CourseCommand Possible solutions: course] DataBinding works fine in grails 2.2.4. Please see the below screenshot

intellij debugger screenshot

I will appreciate any insight on this issue and any idea on how to resolve it will be great.

Thanks, Shiraz

Environment Information

Example Application

GitHub repo URL is https://github.com/swzaidi/sample

graemerocher commented 5 years ago

What is MapUtils.lazyMap? Seems unrelated to Grails

swzaidi commented 5 years ago

yeah, it doesn't seem to be related to LazyMap". I am trying to post form data as

"student.id=1&courses[10].course.id=10"

The command object classes we are trying to bind are as below ...

class StudentEnrollmentCmd {
    Student student
    Map<String,CourseCmd> courses;
}

class CourseCmd {
    CourseDomain course
}

class CourseDomain {
    Long id
}

was hoping that it will bind to

"StudentEnrollmentCmd -> courses -> course.id"

which seems to work in grails 2.2.4 but fails in 3.3.7 with the following exception

No such property: course.id for class: student.CourseCmd

Here is the test case which illustrates the problem

void 'databinding from request parameters'() {
        given:
        // request with simple formdata: student.id=1&courses[10].course.id=10
        MockHttpServletRequest request = buildMockRequestWithParams('POST',['student.id':'1','courses[10].course.id':'10']);
        DataBindingSource source = bindingSourceCreator.createDataBindingSource(null,null,request);

        // databinder & command object
        def binder = new SimpleDataBinder()
        def obj = new StudentEnrollmentCmd()

        when:
        binder.bind(obj,source)

        then:
        // this should not throw an exception, but throws an exception
        MissingPropertyException ex = thrown()
        System.out.println ( "Exception again:" + ex.message );

        // the following should work, but does not work
        obj.student.id == 1
        obj.courses['10'].course.id == 10
    }

Here is the link to full spec... https://github.com/swzaidi/sample/blob/master/grails3.3.7/src/test/groovy/student/DataBindingSpec.groovy

Looking for some help on how to pass the form data so that it binds to above command objects properly.

demon101 commented 5 years ago

@swzaidi could you have a look at https://github.com/grails/grails-core/pull/11080

The fix was merged to master and 3.3 branch and will be released at 3.3.10

swzaidi commented 5 years ago

How can we use release 3.3.10 milestone? It seems it not available as of now

vineeln commented 5 years ago

@demon101 while we wait for 3.3.10 release, would it be possible for you to verify that this test works https://github.com/swzaidi/sample/blob/master/grails3.3.7/src/test/groovy/student/DataBindingSpec.groovy in 3.3.10 branch ?

demon101 commented 5 years ago

@vineeln fix already on the 3.3.x branch. Could you create PR from your branch, please. Test will be started on Trevis automatically

vineeln commented 5 years ago

i tried running this spec on 3.3.x branch its still the same issue, the issue here is not related to LazyMap binding as the initial comment suggested, instead we are unable to bind multilevel properties in object graph. We should probably close this Issue & create a new one ..