Skip to content

Commit 1829d42

Browse files
committed
Ask the collection branch the question its siblings already ask
Three places bind an element into a collection, and two of them keep the item when the element type is already assignable from it: the array branch above and the Map branch below both do it, and SimpleDataBinder does it too. Only the collection branch went straight to instantiating, which is why a raw collection lost its data there and nowhere else. Asking the same question there fixes it in the same shape as its neighbours, rather than adding a special case for Object, and needs no change to instantiateAndBindNestedOrUseMapConstructor: a raw collection's component type is Object, Object is assignable from everything, so the element is kept. The SimpleDataBinder guard from the first commit is dropped with this: that class already asks at line 373 and was never affected.
1 parent 17d24ee commit 1829d42

2 files changed

Lines changed: 11 additions & 21 deletions

File tree

grails-databinding-core/src/main/groovy/grails/databinding/SimpleDataBinder.groovy

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -426,16 +426,6 @@ class SimpleDataBinder implements DataBinder {
426426
}
427427

428428
protected Object instantiateAndBindOrUseMapConstructor(Class referencedType, Map values, DataBindingListener listener) {
429-
if (referencedType == null || referencedType == Object) {
430-
// A raw collection -- List/Set/Map written without a type argument -- reports Object as
431-
// its component type: Basic#componentType falls back to Object.class when a property
432-
// carries no generic signature. Object declares no properties, so instantiating one and
433-
// binding into it has nowhere to put the element's data and the element silently becomes
434-
// an empty Object. A value that is never used as a property source cannot mass-assign
435-
// anything, so it is kept as it stands, which is also how these collections bound before
436-
// deny-by-default.
437-
return values
438-
}
439429
def instance
440430
try {
441431
instance = referencedType.getDeclaredConstructor().newInstance()

grails-web-databinding/src/main/groovy/grails/web/databinding/GrailsWebDataBinder.groovy

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,17 @@ class GrailsWebDataBinder extends SimpleDataBinder {
545545
}
546546
}
547547
if (persistentInstance == null) {
548-
if (item instanceof Map || item instanceof DataBindingSource) {
548+
if (item == null || referencedType.isAssignableFrom(item.getClass())) {
549+
// Already of the element type, so there is nothing to instantiate
550+
// and nothing to bind into. A raw collection always lands here:
551+
// Basic#componentType falls back to Object.class when a property
552+
// carries no generic signature, and Object is assignable from
553+
// everything. The array branch above and the Map branch below ask
554+
// the same question before instantiating; only this one did not,
555+
// so a map element was replaced by an empty Object and its
556+
// contents were dropped.
557+
itemsWhichNeedBinding << item
558+
} else if (item instanceof Map || item instanceof DataBindingSource) {
549559
DataBindingSource itemBindingSource = item instanceof DataBindingSource ?
550560
(DataBindingSource) item : new SimpleMapDataBindingSource((Map) item)
551561
def instance = instantiateAndBindNestedOrUseMapConstructor(
@@ -634,16 +644,6 @@ class GrailsWebDataBinder extends SimpleDataBinder {
634644

635645
private Object instantiateAndBindNestedOrUseMapConstructor(Class referencedType, Object value,
636646
DataBindingSource source, List includeList, DataBindingListener listener) {
637-
if (referencedType == null || referencedType == Object) {
638-
// A raw collection -- List/Set/Map written without a type argument -- reports Object as
639-
// its component type: Basic#componentType falls back to Object.class when a property
640-
// carries no generic signature. Object declares no properties, so instantiating one and
641-
// binding into it has nowhere to put the element's data and the element silently becomes
642-
// an empty Object. A value that is never used as a property source cannot mass-assign
643-
// anything, so it is kept as it stands, which is also how these collections bound before
644-
// deny-by-default.
645-
return value
646-
}
647647
def instance
648648
try {
649649
instance = referencedType.getDeclaredConstructor().newInstance()

0 commit comments

Comments
 (0)