Skip to content

Commit

Permalink
Fix Issue #17.
Browse files Browse the repository at this point in the history
Dates are no longer parsed leniently by default by the extractors.
This avoids unexpected behavior when parsing dates such as
"1999-99-99".

Lenient parsing is still possible, but must be explicitly configured.
  • Loading branch information
sriddell committed Sep 24, 2014
1 parent c2e77ad commit 427a8da
Show file tree
Hide file tree
Showing 20 changed files with 335 additions and 14 deletions.
17 changes: 16 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2418,7 +2418,7 @@ You can specify paths that should be parsed into java.util.Date instances:

This can simplifying conversion of strings to Dates without dealing with Grails data-binding and custom property editors.

By default, the extractor will use a default java.text.SimpleDateFormat to parse date fields. You can specify which formats to use:
By default, the extractor will use a default java.text.SimpleDateFormat to parse date fields, with lenient parsing set to false. You can specify which formats to use:

resource 'purchase-orders' config {
representation {
Expand All @@ -2434,6 +2434,21 @@ Each date format will be tried in order, until one sucessfully parses the date s

You can configure date formats in a single location by placing them on an [extractor template](#json-extractor-templates) and then inheriting them.

If you want to allow the SimpleDateFormatter to use lenient parsing, where heuristics are used to allow a date such as 1999-99-99 to be sucessfully parsed, you can set lenientDates = true. For example:

resource 'purchase-orders' config {
representation {
mediaTypes = ["application/json"]
jsonExtractor {
property 'signupDate' date true
dateFormats = ["yyyy-MM-dd'T'HH:mm:ss.SSSZ", "yyyyy.MMMMM.dd GGG hh:mm aaa"]
lenientDates = true
}
}
}

Note that both the dateFormats and lenientDates settings apply the same to all properties identified as dates in a jsonExtractor.

###Providing default values.
As your system evolves, you may introduce new, required fields to domain objects. If you are using versioned APIs, the new field cannot be added to existing representation(s) without breaking them, so when a caller uses one of these representations, it will be necessary to add a default value. This can, of course, be done at the service layer, but only if the service layer can provide an appropriate default - it is more likely that the service will treat the missing value as a validation exception. The declarative extractor can be configured to provide a default value for any missing key.

Expand Down
3 changes: 2 additions & 1 deletion RELEASE-NOTES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#1.0.0
* BREAKING CHANGE. The service layer contract and RestfulServiceAdapter interface have been changed so that all methods signatures are consistent. The resource id will no longer be passed as a separate parameter to the update and delete methods, as it is also available via the params map. (Issue #8) If you have services that expect the id to be passed explicitly, you can continue to support this behavior with use of RestfulServiceAdapter that extracts the id from the params and passes it in as a separate argument.
* BREAKING CHANGE. The service layer contract and RestfulServiceAdapter interface have been changed so that all method signatures are consistent. The resource id will no longer be passed as a separate parameter to the update and delete methods, as it is also available via the params map. (Issue #8) If you have services that expect the id to be passed explicitly, you can continue to support this behavior with use of RestfulServiceAdapter that extracts the id from the params and passes it in as a separate argument.
* BREAKING CHANGE. The JSON and XML extractors will no longer parse dates in a lenient fashion by default. In previous releases, a date such as '1999-99-99' would have sucessfully parsed (lenient parsing by default). In 1.0.0, it will throw an exception, resulting in a 400 response. You can retain the lenient behavior if necessary by setting lenientDates = true on the extractor. See "Parsing Dates" in the README for details.

#0.10.0
* BREAKING CHANGE. By default, on a DELETE, the controller will ignore the Content-Type and request body, and send an empty content map to the service. This is the opposite of previous behavior. The original behavior can be obtained with the bodyExtractedOnDelete setting. See the delete method documentation in the README.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class JSONExtractorConfig implements MergeableConfig {
List<String> dottedFlattenedPaths = []
Closure shortObjectClosure
boolean isShortObjectClosureSet = false
Boolean lenientDates = null
List<String> dottedDatePaths = []
List<String> dateFormats = []

Expand Down Expand Up @@ -87,6 +88,9 @@ class JSONExtractorConfig implements MergeableConfig {
config.dateFormats.add(it)
}
}
if (other.lenientDates != null) {
config.lenientDates = other.lenientDates
}

config
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class JSONExtractorDelegate {
this
}


JSONExtractorDelegate setDateFormats(Collection formats) {
formats.each {
try {
Expand All @@ -59,6 +60,11 @@ class JSONExtractorDelegate {
this
}

JSONExtractorDelegate setLenientDates(boolean b) {
config.lenientDates = b
this
}

class PropertyOptions {
String propertyName
PropertyOptions(String propertyName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class JSONExtractorFactory {
if (config.isShortObjectClosureSet) extractor.shortObjectClosure = config.shortObjectClosure
extractor.dottedDatePaths = config.dottedDatePaths
extractor.dateFormats = config.dateFormats
if (config.lenientDates != null) extractor.lenientDates = config.lenientDates

extractor
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class XMLExtractorConfig implements MergeableConfig {
boolean isShortObjectClosureSet = false
List<String> dottedDatePaths = []
List<String> dateFormats = []
Boolean lenientDates

XMLExtractorConfig() {
}
Expand Down Expand Up @@ -86,6 +87,10 @@ class XMLExtractorConfig implements MergeableConfig {
}
}

if (other.lenientDates != null) {
config.lenientDates = other.lenientDates
}

config
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ class XMLExtractorDelegate {
this
}

XMLExtractorDelegate setLenientDates(boolean b) {
config.lenientDates = b
this
}

class PropertyOptions {
String propertyName
PropertyOptions(String propertyName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class XMLExtractorFactory {
if (config.isShortObjectClosureSet) extractor.shortObjectClosure = config.shortObjectClosure
extractor.dottedDatePaths = config.dottedDatePaths
extractor.dateFormats = config.dateFormats
if (config.lenientDates != null) extractor.lenientDates = config.lenientDates

extractor
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class BasicJSONExtractor implements JSONExtractor {
}
for (String format : formats) {
SimpleDateFormat df = new SimpleDateFormat(format)
df.setLenient(getLenientDates())
try {
return df.parse(value.toString())
} catch (ParseException e) {
Expand Down Expand Up @@ -165,8 +166,8 @@ class BasicJSONExtractor implements JSONExtractor {
/**
* Returns a List of String denoting paths whose
* values should be parsed as dates.
* The conversion is not lenient - if the value cannot
* be parsed as a date, an exception is thrown.
* If the value cannot be parsed as a date,
* an exception is thrown.
* Parses according to {@link #getDateFormats() getDateFormats},
* or default SimpleDateFormat if no formats are specified.
**/
Expand All @@ -182,6 +183,16 @@ class BasicJSONExtractor implements JSONExtractor {
[]
}

/**
* Return true if lenient date parsing should be used.
* If true, the default lenient behavior of SimpleDateFormatter
* is used.
* Default is to return false.
**/
protected boolean getLenientDates() {
false
}

/**
* Returns a closure that can convert a 'short object'
* representation to a map containing the id represented
Expand All @@ -192,7 +203,7 @@ class BasicJSONExtractor implements JSONExtractor {
}

/**
* Returns a closure that can conver date representations
* Returns a closure that can convert date representations
* into Dates.
**/
protected Closure getDateClosure() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class DeclarativeJSONExtractor extends BasicJSONExtractor {
List<String> dottedDatePaths = []
Closure shortObjectClosure
List<String> dateFormats = []
boolean lenientDates = false


/**
Expand Down Expand Up @@ -112,6 +113,17 @@ class DeclarativeJSONExtractor extends BasicJSONExtractor {
dateFormats.clone()
}

/**
* Return true if lenient date parsing should be used.
* If true, the default lenient behavior of SimpleDateFormatter
* is used.
* Default is to return false.
**/
@Override
protected boolean getLenientDates() {
lenientDates
}

/**
* Returns a closure that can convert a 'short object'
* representation to a map containing the id represented
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class BasicXMLExtractor extends MapExtractor {
}
for (String format : formats) {
SimpleDateFormat df = new SimpleDateFormat(format)
df.setLenient(getLenientDates())
try {
return df.parse(value.toString())
} catch (ParseException e) {
Expand Down Expand Up @@ -183,6 +184,16 @@ class BasicXMLExtractor extends MapExtractor {
[]
}

/**
* Return true if lenient date parsing should be used.
* If true, the default lenient behavior of SimpleDateFormatter
* is used.
* Default is to return false.
**/
protected boolean getLenientDates() {
false
}

/**
* Returns a closure that can convert a 'short object'
* representation to a map containing the id represented
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class DeclarativeXMLExtractor extends BasicXMLExtractor {
List<String> dottedDatePaths = []
Closure shortObjectClosure
List<String> dateFormats = []
boolean lenientDates = false

/**
* Returns a map of rename rules. The keys
Expand Down Expand Up @@ -109,6 +110,17 @@ class DeclarativeXMLExtractor extends BasicXMLExtractor {
dateFormats.clone()
}

/**
* Return true if lenient date parsing should be used.
* If true, the default lenient behavior of SimpleDateFormatter
* is used.
* Default is to return false.
**/
@Override
protected boolean getLenientDates() {
lenientDates
}

/**
* Returns a closure that can convert a 'short object'
* representation to a map containing the id represented
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,19 @@ class JSONExtractorConfigSpec extends Specification {
closure == config.shortObjectClosure
}

def "Test lenient dates"() {
setup:
def src = {
lenientDates = true
}

when:
def config = invoke(src)

then:
true == config.lenientDates
}

def "Test repeated property clears previous settings"() {
setup:
def src = {
Expand Down Expand Up @@ -168,7 +181,8 @@ class JSONExtractorConfigSpec extends Specification {
dottedShortObjectPaths:['short1'],
shortObjectClosure:c1,
dottedDatePaths:['date1'],
dateFormats:['yyyy.MM.dd']
dateFormats:['yyyy.MM.dd'],
lenientDates: false

)
def two = new JSONExtractorConfig(
Expand All @@ -178,7 +192,8 @@ class JSONExtractorConfigSpec extends Specification {
dottedShortObjectPaths:['short2'],
shortObjectClosure:c2,
dottedDatePaths:['date2'],
dateFormats:['yyyy/MM/dd']
dateFormats:['yyyy/MM/dd'],
lenientDates: true
)

when:
Expand All @@ -192,6 +207,7 @@ class JSONExtractorConfigSpec extends Specification {
c2 == config.shortObjectClosure
['date1','date2'] == config.dottedDatePaths
['yyyy.MM.dd','yyyy/MM/dd'] == config.dateFormats
true == config.lenientDates
}

def "Test merging configurations does not alter either object"() {
Expand All @@ -205,7 +221,8 @@ class JSONExtractorConfigSpec extends Specification {
dottedShortObjectPaths:['short1'],
shortObjectClosure:c1,
dottedDatePaths:['date1'],
dateFormats:['yyyy.MM.dd']
dateFormats:['yyyy.MM.dd'],
lenientDates:false
)
def two = new JSONExtractorConfig(
dottedRenamedPaths:['two':'newTwo','three':'newThree'],
Expand All @@ -214,7 +231,8 @@ class JSONExtractorConfigSpec extends Specification {
dottedShortObjectPaths:['short2'],
shortObjectClosure:c2,
dottedDatePaths:['date2'],
dateFormats:['yyyy/MM/dd']
dateFormats:['yyyy/MM/dd'],
lenientDates:true
)

when:
Expand All @@ -228,6 +246,7 @@ class JSONExtractorConfigSpec extends Specification {
c1 == one.shortObjectClosure
['date1'] == one.dottedDatePaths
['yyyy.MM.dd'] == one.dateFormats
false == one.lenientDates

['two':'newTwo','three':'newThree'] == two.dottedRenamedPaths
['flat2'] == two.dottedFlattenedPaths
Expand All @@ -236,6 +255,7 @@ class JSONExtractorConfigSpec extends Specification {
c2 == two.shortObjectClosure
['date2'] == two.dottedDatePaths
['yyyy/MM/dd'] == two.dateFormats
true == two.lenientDates
}

def "Test merging with short object closure set only on the left"() {
Expand All @@ -252,6 +272,18 @@ class JSONExtractorConfigSpec extends Specification {
true == config.isShortObjectClosureSet
}

def "Test merging with lenient dates set only on the left"() {
setup:
def one = new JSONExtractorConfig(lenientDates:true)
def two = new JSONExtractorConfig()

when:
def config = one.merge(two)

then:
true == config.lenientDates
}

private JSONExtractorConfig invoke( Closure c ) {
JSONExtractorDelegate delegate = new JSONExtractorDelegate()
c.delegate = delegate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class RestConfigJSONExtractorSpec extends Specification {
property 'person.customer' shortObject true
property 'lastName' defaultValue 'Smith'
property 'date' date true
lenientDates = true
dateFormats = ['yyyy.MM.dd', 'yyyy/MM/dd']
shortObject { def v -> 'short' }
}
Expand All @@ -65,6 +66,7 @@ class RestConfigJSONExtractorSpec extends Specification {
['lastName':'Smith'] == mConfig.dottedValuePaths
'short' == shortObject
['date'] == mConfig.dottedDatePaths
true == mConfig.lenientDates
['yyyy.MM.dd', 'yyyy/MM/dd'] == mConfig.dateFormats
}

Expand All @@ -81,6 +83,7 @@ class RestConfigJSONExtractorSpec extends Specification {
property 'person.customer' shortObject true
property 'lastName' defaultValue 'Smith'
property 'date' date true
lenientDates = true
dateFormats = ['yyyy.MM.dd', 'yyyy/MM/dd']
shortObject { def v -> 'short' }
}
Expand All @@ -102,6 +105,7 @@ class RestConfigJSONExtractorSpec extends Specification {
'short' == shortObject
['date'] == extractor.dottedDatePaths
['yyyy.MM.dd','yyyy/MM/dd'] == extractor.dateFormats
true == extractor.lenientDates
}

def "Test json extractor creation from merged configuration"() {
Expand Down

0 comments on commit 427a8da

Please sign in to comment.