grails / grails-core

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

Grails 3 unable to bind request parameter to multilevel command objects #11259

Open vineeln opened 5 years ago

vineeln commented 5 years ago

We are 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 {
    MyStudent student
    Map<String,CourseCmd> courses;
}
class MyStudent {
    Long id;
}
class CourseCmd {
    CourseDomain course
}
class CourseDomain {
    Long id
}

was hoping that it will bind to

StudentEnrollmentCmd -> student -> id (this works) StudentEnrollmentCmd -> courses -> course -> id (this doesn't work)

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 thrown:" + ex.message );

        // student.id is bound correctly
        obj.student.id == 1
        // the following doesn't work
        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.

Environment Information

Example Spec

Note: we illustrated the same problem in #11245, which wrongly started of as a LazyMap binding issue, that issue can be closed.

vineeln commented 5 years ago

@graemerocher any thoughts on this issue ?

olavgg commented 5 years ago

I think you have to start the courses index with 0 For example courses[0].course.id=10

osscontributor commented 5 years ago

I think you have to start the courses index with 0

Why do you think that?

olavgg commented 5 years ago

Because we do it with 0 and that works If I change it to 10, I get a null pointer exception.

osscontributor commented 5 years ago

With a Map, that surprises me, but I believe you.

Thanks for the feedback.

osscontributor commented 5 years ago

I wouldn't expect the code shown above to work with a 0 or with a 10.

osscontributor commented 5 years ago

I think you have to start the courses index with 0

I have confirmed that this is not the case.

olavgg commented 5 years ago

Ah I didn't see that it was a map, thought is was a List. sorry :(

osscontributor commented 5 years ago

Ah I didn't see that it was a map, thought is was a List. sorry :(

Right. It also isn't true of List. You can start with any non-negative integer. We have a number of tests around that. For example, https://github.com/grails/grails-core/blob/b2fd8c84e2406d15f18ff54070f5783149e084e3/grails-databinding/src/test/groovy/grails/databinding/CollectionBindingSpec.groovy#L89-L108 starts with 2, which leads to [0] and [1] being null.

olavgg commented 5 years ago

Interesting, I didn't know about that, you're right that it creates null objects in that collection if you dont add parameters in correct order.

For example curl -XPOST http://localhost:8080/default/save -d 'books[2].title=Hello' -d books[5].title=Grails Will bind to [null. null, BookCmd(Hello), null, null BookCmd(Grails)]

Learned something new there :-) Thanks

@vineeln I don't think that example you have will work, but you can always bind it manually with @BindUsing

Example

class StudentEnrollmentCmd {
    MyStudent student

    @BindUsing({ StudentEnrollmentCmd obj, SimpleMapDataBindingSource source ->
        def map = source['courses'] as Map
        def tempCourses = new HashMap<String, CourseCmd>()
        tempCourses.put("10", new CourseCmd())
        return tempCourses
    })
    Map<String,CourseCmd> courses
}
osscontributor commented 5 years ago

Interesting, I didn't know about that, you're right that it creates null objects in that collection if you dont add parameters in correct order.

I don't think it is about "correct" order. It is ok to have a List with null entries and it is ok for those null entries to be at any position so there isn't anything incorrect about a List like [null, null, someObject, null, someOtherObject, null, etc...].

vineeln commented 5 years ago

@olavgg let me try the workaround.