Option to expose empty value instead of NullValueInNestedPathException [SPR-1671]

[spring-projects-issues]

Rob Monie opened SPR-1671 and commented

When using nested paths such as "organisation.address.state" with a property editor for State intended to convert a String ID to a State object and back again, if address is null, a 'NullValueInNestedPathException' is thrown from BeanWrapperImpl. Is this the intended behaviour ?

Obviously if part of the nested path is null it is impossible to traverse the object graph to determine what property editor to use for the actual State property. However, if address is null then State is sure to be null also, so a generic handler of nulls could be possibly be applied.

The current workaround is to manually populate empty objects in the object graph in formBackingObject(). This does however require that these 'empty' placeholder objects need to be cleaned up before a save to prevent complications with incorrect data being pushed to the database.

---

Affects: 1.2.6

Issue Links:

• BeanWrapperImpl doesn't delegate Map-creation to propertyEdtior [SPR-3565] #8248 BeanWrapperImpl doesn't delegate Map-creation to propertyEdtior
• mvc: Dynamic List use case - allow collections/arrays to grow [SPR-4728] #9405 mvc: Dynamic List use case - allow collections/arrays to grow
• Data Binder: Add support for auto-instantiating null values in nested paths [SPR-6032] #10701 Data Binder: Add support for auto-instantiating null values in nested paths
• Null property values in nested paths relating to <spring:bind> tag, <form:xxx> tags, and ServletRequestDataBinder [SPR-1860] #6554 Null property values in nested paths relating to spring:bind tag, form:xxx tags, and ServletRequestDataBinder
• <form> tag library should WARN instead of blowup when command missing [SPR-2607] #7296 tag library should WARN instead of blowup when command missing

38 votes, 37 watchers

[spring-projects-issues]

Juergen Hoeller commented

This is actually intended behavior: BeanWrapper does not support handling properties that have an incomplete nested path.
We do not consider such a non-existent nested value to be null by default, in contrast to (for example) the JSP EL.

However, you should be able to register property editors for such paths. They just shouldn't get applied in case of a null value in the path...

Juergen

[spring-projects-issues]

Rob Monie commented

Thanks Juergen. I know this isn't the place to be asking questions but i'm a little confused as to the intended way of dealing with this. As soon as I try to bind a nested path with a null value in any position other than the final object in the graph, a NullValueInNestedPathException is thrown. Regardless of whether this is the intended behaviour of BeanWrapper, this seems odd. I am definately able to register property editors for these types of paths but if a null value exists in the path, an exception is thrown well before the property editor can be applied.

Are you confirming that my workaround described above is the recommended way of approaching this problem or am I missing something that would negate the need for populating empty placeholder objects to enable traversal of the object graph ? This topic isn't covered in any of the books or reference material I have available to me.

[spring-projects-issues]

Juergen Hoeller commented

I guess this mainly applies to retrieving the current value of a property, doesn't it - since we really can't do much about an attempt to set a nested property with a null value in the path.

So essentially, you'd like to be able to treat a 'NullValueInNestedPathException' such that a plain empty value is exposed instead? Similar to what the <c:out> tag does in JSTL?

We could only support that for specific output mechanisms, for example the spring:bind tag. And even there it should be optional, since else there's the risk of swallowing an exception that you might not be aware of. The problem is that a subsequent attempt to set that field will fail - and of course that's what the spring:bind tag is there for in the first place.

Hence, I'd argue that you should use the JSTL <c:out> tag whereever you'd like to treat incomplete nested paths simply as empty String values...

In general, pre-populating the object graph to actually have the accessed fields in there is the recommended solution. I do see that it can be a nuisance to clean up the object graph afterwards. Nevertheless, this looks like the only proper way that cleanly allows for populating fields as well.

Juergen

[spring-projects-issues]

Rob Monie commented

Thanks Juergen. I appreciate your response!

Rob

[spring-projects-issues]

Peter Mularien commented

+1 vote for fixing this issue. It is a nuisance to have to populate an object (especially a complex one) with empty object trees, only to have to clean them up later. I actually disagree with Juergen:

"Hence, I'd argue that you should use the JSTL <c:out> tag whereever you'd like to treat incomplete nested paths simply as empty String values... "

Instead, I would expect that spring:bind would take its cue on behavior from the JSTL out tag, and instead simply log a warning if the requested bind path is null/not present, and set the BindStatus to null. This way references to "status.foo" will simply resolve (as consistent with JSTL semantics) to null/empty, and output within the spring:bind would function as expected, without errors.

I would put this suggestion for improvement under the heading of "the [Spring] framework should predictably and non-destructively handle simple error conditions in pages". It would remove unneeded complexity (referenced by Rob) from the developer of the controller, and would improve consistency with other tags in JSTL.

I'd be happy to contribute a patch if Juergen agrees with this behavior.

[spring-projects-issues]

Juergen Hoeller commented

As outlined above, the problem is not so much the retrieval of a value for display - we could quite easily use a fallback to an empty String there, similar to the JSTL's <c:out>. The problem is rather that spring:bind's main purpose is to populate the target field on submit of the page - and if the bind path contains a null value, this is gonna fail in any case... Hence, such a fallback would only make sense for read-only fields that the user cannot edit. But why wouldn't you simply use <c:out> or some other standard means to display such a field? Just for consistency with the code for read-write fields?

Juergen

[spring-projects-issues]

Peter Mularien commented

Right, display is (should be) trivial, and should function (or be configurable to function) like c:out. Right now, if I try to spring:bind a nested path expression that traverses a relationship where an intervening object is currently null (the "address" portion of the example in the bug report), I get a NullValueInNestedPathException thrown. This is thrown by the getPropertyValue method of BeanWrapperImpl.

In the reverse case, which you mention (setting property), it is again BeanWrapperImpl which throws NullValueInNestedPath within the setPropertyValue method.

So the enhancement on the spring:bind side (getPropertyValue) is relatively trivial - BindTag would need to be enhanced to catch NullValueInNestedPath and set the status model object to null.

Handling setPropertyValue's throwing of the exception would have to take place in the databinder code. The question is - who is responsible for creation of the intervening object ("address", in this case)? If we assume that there were an obvious API, then the databinder could (be configured to), upon receiving of NullValueInNestedPath from the setPropertyValue API, create the intervening object(s) and try again.

One sees this technique being applied in several suggestions in the forums in the use case of array and list items not existing, where the pattern established by commons' ListUtils.lazyList is used.

Creating the intervening objects could easily be accomplished by use of a property editor, calling setValue on the property editor with a value of null - this could trigger the property editor author to ensure that his/her property editor returns a new object -- this is consistent with behavior established in the JavaBeans contract.

[spring-projects-issues]

Jon Are Storløkken commented

I'd certainly like to see this fixed. I came across this issue looking for a solution for my current problem with NullValueInNestedPathException.

In my case I've been trying to bind values to a domain object. I didn't use the bind tag, just set the name attribute on the input field. I'll look into bind tag next. But it seems that this won't solve the problem anyway.

It's impossible to bind values that originally contained null. So, these values cannot be updated without some kind of workaround... Although I came across this quite recently and might now have the full picture, I can't really see the rationale for the current implementation. Populating all non-leaf nodes prior to rendering would probably be the solution. This is just such a hassle, when it seems that an easier solution could be implemented.

[spring-projects-issues]

Aaron Dressin commented

"This is actually intended behavior: BeanWrapper does not support handling properties that have an incomplete nested path. "

I am interested in why this is intended. Other web frameworks with binding capabilities will instantiate the nested object graph as needed while binding. Why should the binding process care if a nested property was not explicitly instantiated? In my opinion it is VERY annoying to have to manage the creation of nested object graphs... esp. in large cases. My hope is that I can leave the object creation to the binding process, and handle any application-specific issues with the binded data to the validation process.

[spring-projects-issues]

sam commented

It makes me crazy. so strange!

[spring-projects-issues]

Rob Monie commented

As Juergen said, it makes sense to return null or an empty string rather than throwing an exception for display purposes although it will cause problems if you try to then bind against this field on form submission. I can't think of any way a framework could reliably instantiate objects in the graph for binding against unless they were limited to a no arg constructor or BeanWrapper imposed some other approach for instantiating objects. This might work for some apps but most of the apps i've worked on have required far more than simple model objects that can be instantiated in this way.

Since originally submitting this issue my colleagues and I have done a significant amount of work in this area writing an open source extension to Spring MVC called Spring Layout and have come up with some ways of managing these sorts of things but there's always some compromise in flexibility if you want simplicity. One approach that we use in our tags is to render any fields that throw a NullValueInNestedPathException as disabled. This also prevents the field from being submitted so Spring won't attempt to bind the fields to the command.

[spring-projects-issues]

Sergio Bossa commented

I can't think of any way a framework could reliably instantiate objects in the graph for binding against

In the past I've worked with some Apache Cocoon committers for solving the same kind of problem in the Cocoon Form binding framework: it was resolved by providing a factory interface to implement and associate to the property path that has to be instantiated in a custom way.

We could do the same in the BeanWrapper: associate factory objects to property paths and let the bean wrapper call them when an object in the path is null and need to be created.

What do you think?

Cheers,

Sergio B.

[spring-projects-issues]

Rob Monie commented

That sounds like a great idea to me. We usually use a factory for instantiating model objects for various reasons so i'd imagine that if we could register this against the bean wrapper we'd have a lot of smiling faces around the office. I'm thinking aloud here so I could be talking rubbish but if it was smart enough to also deal with paths that include collections and add items to lists etc it would be a godsend.

Imagine I have a graph something like 'user.addresses[0].city' and I add a field to the form bound against 'user.addresses[1].city' but that doesn't exist in the model yet. Currently we need to add pre binding to check for this kind of thing and put empty placeholders in so we have something to bind against. If BeanWrapper could catch the null value in nested path exception, invoke the factory method registered for that path and populate the graph for binding things would be so much cleaner.

[spring-projects-issues]

Burak Modali commented

I can't wait for this to be fixed. This issue is a big annoyance, especially if you have nested object graphs and using hibernate for persistance as LazyList workaround mentioned above cannot be used on domain classes persisted by hibernate. Hibernate throws concurrent object modification exceptions if i lazily instantiate a collection in my domain class definition.

So i ended up using VOs and using lazy lists in them and mapping nested VOs back to my domain object graph after form post just to workaround Spring MVC's data binding restrictions. Having done so, i had to create 'VO' versions of my business logic validators.

very annoying indeed.

[spring-projects-issues]

Derek Alexander commented

I'd vote for this again if I could :)

Instead, I'll propose a solution: How about "borrowing" Groovy's safe dereferrencing syntax (the ?. operator) ?

This would maintain compatibility for existing code (which shouldn't contain such an operator) and allow developers to choose the behaviour they want in new code.

For anyone who doesn't know Groovy, this syntax allows access to deep nested paths without risk of a null pointer exception.
If the reference before the operator is a null reference, the evaluation of the current expression stops, and null is returned.
(paraphrased from Groovy In Action page 184)

So for example, the existing syntax:
<form:select path="supervisor.id"> ...
would still cause a null pointer exception if the supervisor property of the command object is null

But with the safe dereferencing syntax:
<form:select path="supervisor?.id> ...
there would be no problem.