groovy

Groovy Templates should treat missing variables as null rather than throw MissingPropertyException's

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Won't Fix
  • Affects Version/s: 1.8.3
  • Fix Version/s: None
  • Component/s: Templating
  • Labels:
    None
  • Number of attachments :
    0

Description

See Testing for nonexistent properties in groovy templates from the Groovy User list.

SimpleTemplateEngine should evaluate ${notAlwaysAvailable == null} to true if the variable is missing from the template binding. Instead it throws: Exception in thread "main" groovy.lang.MissingPropertyException: No such property: notAlwaysAvailable for class: SimpleTemplateScript4.

An exception in this case is not very useful, and is inconsistent with most other template implementations such as: the Java Unified Expression Language; the Spring Expression Language; FreeMarker; Velocity; and Ant, Maven and Spring property substitution.

The binding.variables.contains("notAlwaysAvailable") workaround is overly verbose, as is having to implement your own binding or template engine.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ian Brandt added a comment -

Pull request made.

Show
Ian Brandt added a comment - Pull request made .
Hide
Permalink
blackdrag blackdrag added a comment -

Ian, I think we cannot do this change. The problem I see is that those MissingPropertyExceptions are part of the MOP and that means builders and closures may get problems with such a default. We can think about opening SimpleTemplateEngine to allow people set their own binding and they can through subclassing reach the goal you are talking about here... but in general...

there would be a way though to reach the goal you have and keep builders working at the same time... there has to be a different Script class as base for the template and in there use getProperty to delegate to the meta class but then catch the MissingPropertyException, to then return the default. Also for the default there are 3 different possibilities I can imagaine right now: null, "", "null". Meaning the default should be configurable too.

What do you think Ian?

Show
blackdrag blackdrag added a comment - Ian, I think we cannot do this change. The problem I see is that those MissingPropertyExceptions are part of the MOP and that means builders and closures may get problems with such a default. We can think about opening SimpleTemplateEngine to allow people set their own binding and they can through subclassing reach the goal you are talking about here... but in general... there would be a way though to reach the goal you have and keep builders working at the same time... there has to be a different Script class as base for the template and in there use getProperty to delegate to the meta class but then catch the MissingPropertyException, to then return the default. Also for the default there are 3 different possibilities I can imagaine right now: null, "", "null". Meaning the default should be configurable too. What do you think Ian?
Hide
Permalink
Ian Brandt added a comment -

Jochen,

Certainly. However I didn't think I was changing the default behavior for Binding.getProperty of throwing a MissingPropertyException, only adding the option to change the behavior to be used specifically by those that need it. But perhaps your saying just having the option at that low a level doesn't sit right with you, and I'll of course defer to your expertise.

It looks a little trickier to move the behavior up to Script however. SimpleTemplateEngine.createTemplate(...) defers Script creation first to InvokerHelper.createScript(...) via groovyShell.parse(...). Then when the user calls .make(...).toString() on the returned SimpleTemplateEngine$SimpleTemplate another Script instance is created using the class from the first, via InvokerHelper.createScript(...) directly this time. I'm sure there's a reason for all that, but not being familiar with it I'm not clear on where I should try to hook a configurable Script instance or class into the mix? I'm also not sure how I'd work that into GStringTemplateEngine either, as it doesn't appear to use Script directly at all. With Binding it appeared straightforward to hook into both.

I'm all for offering the end user a choice for how to handle missing properties. Perhaps a better approach, for Script or Binding, would be to introduce a "MissingPropertyHandler". The default one would throw MissingPropertyException, but the user could set others for "", "null", null, another constant, or even something dynamic.

Just let me know how you think I might proceed and I'll give it another go.

Show
Ian Brandt added a comment - Jochen, Certainly. However I didn't think I was changing the default behavior for Binding.getProperty of throwing a MissingPropertyException, only adding the option to change the behavior to be used specifically by those that need it. But perhaps your saying just having the option at that low a level doesn't sit right with you, and I'll of course defer to your expertise. It looks a little trickier to move the behavior up to Script however. SimpleTemplateEngine.createTemplate(...) defers Script creation first to InvokerHelper.createScript(...) via groovyShell.parse(...) . Then when the user calls .make(...).toString() on the returned SimpleTemplateEngine$SimpleTemplate another Script instance is created using the class from the first, via InvokerHelper.createScript(...) directly this time. I'm sure there's a reason for all that, but not being familiar with it I'm not clear on where I should try to hook a configurable Script instance or class into the mix? I'm also not sure how I'd work that into GStringTemplateEngine either, as it doesn't appear to use Script directly at all. With Binding it appeared straightforward to hook into both. I'm all for offering the end user a choice for how to handle missing properties. Perhaps a better approach, for Script or Binding, would be to introduce a " MissingPropertyHandler ". The default one would throw MissingPropertyException , but the user could set others for "" , "null" , null , another constant, or even something dynamic. Just let me know how you think I might proceed and I'll give it another go.
Hide
Permalink
blackdrag blackdrag added a comment -

After looking at the issue again we came up with a different approach. Instead of providing a custom Binding of some kind, we use the backing map and let it provide the default. See https://github.com/groovy/groovy-core/commit/fb8fad7c22b8d36671fd679464cd3e062d7a52f5 for an example. This can be used for empty strings. It won't work for null though, bu you could use "null" as default text instead

Show
blackdrag blackdrag added a comment - After looking at the issue again we came up with a different approach. Instead of providing a custom Binding of some kind, we use the backing map and let it provide the default. See https://github.com/groovy/groovy-core/commit/fb8fad7c22b8d36671fd679464cd3e062d7a52f5 for an example. This can be used for empty strings. It won't work for null though, bu you could use "null" as default text instead

People

  • Assignee:
    blackdrag blackdrag
    Reporter:
    Ian Brandt
Vote (1)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: