org.springframework.beans.DirectFieldAccessor fails to go through field paths recursively [SPR-9705]

[spring-projects-issues]

Lukas Kalinski opened SPR-9705 and commented

Shortly put: having the WebDataBinder to call initDirectFieldAccess() will result in failure of resolving nested paths (i.e., if class A has a member of type/class B, then referencing that member will throw a NotReadablePropertyException).

I would suggest that you visit the following page for a detailed explanation of the problem.

http://forum.springsource.org/showthread.php?112464-Direct-Field-Access-Probelm

Both me and the poster in the link above have noticed that in org.springframework.beans.DirectFieldAccessor, no recursive calls to get properties of properties are made, and therefore it's not very surprising (anymore) that this just don't work.

I've set the priority to "critical" (even if it in fact is kind of a blocker for me, as I refuse to use property access).

Anyway, thank you for a great framework!

Kind regards,
Lukas, Sweden

---

Affects: 3.1.2

Issue Links:

• JSR-303 validation doesn't work with nested properties in case of direct field access [SPR-10623] #15251 JSR-303 validation doesn't work with nested properties in case of direct field access
• Make initDirectFieldAccess work with array, List and Map [SPR-12805] #17402 Make initDirectFieldAccess work with array, List and Map
• DirectFieldBindingResult should support autoGrowNestedPaths [SPR-8692] #13334 DirectFieldBindingResult should support autoGrowNestedPaths

Referenced from: commits 8221c9a

10 votes, 11 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

Changed component type to "Core". Even though used through the WebDataBinder, the question is really about DirectFieldAccessor.

[spring-projects-issues]

Juergen Hoeller commented

I'm afraid this is a known limitation: DirectFieldAccessor never got nested property support in its initial incarnation, and nobody ever asked since direct field access seems to have been used for simple command objects only...

In any case, we will reconsider for Spring 3.2. That limitation may come as a surprise to quite a few people trying direct field binding for the first time.

Juergen

[spring-projects-issues]

Hans Desmet commented

When you apply Domain Driven Design, you have Entities which can contain Value objects.
These Value Objects can contain other Value Objects.

Making Value Objects immutable is a best practice.

A DirectFieldAccessor with nested property support is highly desirable for databinding
to these nested Value Objects

[spring-projects-issues]

Gerson commented

I fully agree with Hans. I always end up creating setters on domain objects only because of Spring's data binding. Adding nested property support to DirectFieldAccessor would be very nice feature!

[spring-projects-issues]

Maciej Walkowiak commented

I've submitted a pull request that adds nested fields traversing feature for DirectFieldAccessor: #543

[spring-projects-issues]

Andrzej L commented

It will be great if this improvement will be available also in 3.2.x version.

[spring-projects-issues]

Stéphane Nicoll commented

Maciej Walkowiak thanks for the patch. Is there a specific reason you decided to lazily instantiate any child reference that is not available yet?

This looks like a pretty hard and weird outcome. What if the field is null and should stay that way unless someone call the setter?

There are other problems, such as caching the value alongside the field for a nested field. I'll look into that and propose a solution.

[spring-projects-issues]

Maciej Walkowiak commented

Lazy child fields initialisation is required for writing nested properties:

@Test
public void testSettingNestedProperty() {
Person person = new Person();
	DirectFieldAccessor accessor = new DirectFieldAccessor(person);
	accessor.setPropertyValue("address.city", "London");

	assertThat(person.address, is(notNullValue()));
	assertThat(person.address.city, is("London"));
}

Of course, you are correct - it has a side effect when nested property is being read. I missed it - my bad.

Test like

@Test
public void shouldNotCreateObjectsWhenGettingProperty() {
	Person person = new Person();
	DirectFieldAccessor accessor = new DirectFieldAccessor(person);

	accessor.getPropertyValue("address.city");
	assertThat(person.address, is(nullValue()));
}

should be added to DirectFieldAccessorTests and code refactored to make it pass. I will take a look at that and hopefully find time during next couple of days to submit a fix.

[spring-projects-issues]

Stéphane Nicoll commented

As I said, I am working on it already. We shouldn't initialize intermediate paths either. Keeping the value in FieldHolder is wrong as well because its being cached and that value can change over time (if you keep the reference of the DirectFieldAccessor). Thanks.

[spring-projects-issues]

Maciej Walkowiak commented

Initialising intermediate paths for writing matched well use cases I came across - mainly binding forms to objects. I believe it's pretty handy.

Ok, I am not going then to fix it, and I am looking forward to see your solution. Thanks!

[spring-projects-issues]

Stéphane Nicoll commented

Thanks for the feedback. I realized about ConfigurablePropertyAccessor and how it's used in the binder API. I'll look how I can improve that. Thanks.

[spring-projects-issues]

Stéphane Nicoll commented

This has now been implemented. DirectFieldAccessor is now more in line with the more powerful BeanWrapper counter part.

The core tests have been shared to validate that these two implementations behave similarly. In particular, path traversal is now implemented and setAutoGrowNestedPaths is available for this implementation as well.

Note that BeanWrapper still handles more feature (that were not in the scope of this issue), such as Map and List access.