From 5470227571177c99f600cea396fe8e87639f6811 Mon Sep 17 00:00:00 2001 From: felipe Date: Tue, 14 Apr 2015 13:33:52 +0200 Subject: [PATCH 1/2] fixed issue with dates handling --- .gitignore | 3 +- .../unmarshall/DomainClassUnmarshaller.groovy | 27 ++- .../DomainClassUnmarshallerSpec.groovy | 155 ++++++++++++++++++ 3 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 test/unit/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshallerSpec.groovy diff --git a/.gitignore b/.gitignore index c32d6033..b9bac1ee 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ web-app *.log *.zip plugin.xml -/docs \ No newline at end of file +/docs +.DS_Store \ No newline at end of file diff --git a/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy b/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy index b83bcc7f..dfb14555 100644 --- a/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy +++ b/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy @@ -23,6 +23,7 @@ import org.codehaus.groovy.runtime.DefaultGroovyMethods import org.elasticsearch.action.get.GetRequest import org.elasticsearch.action.get.GetResponse import org.elasticsearch.client.Client +import org.elasticsearch.common.joda.time.DateTime import org.elasticsearch.common.xcontent.XContentBuilder import org.elasticsearch.search.SearchHit import org.elasticsearch.search.SearchHits @@ -37,6 +38,7 @@ import org.springframework.beans.TypeConverter import org.springframework.util.Assert import java.beans.PropertyEditor +import java.text.SimpleDateFormat /** * Domain class unmarshaller. @@ -94,15 +96,34 @@ class DomainClassUnmarshaller { } } new DatabindingApi().setProperties(instance, rebuiltProperties) + populateUnboundDateTimeProperties(instance, rebuiltProperties) results << instance } results } - private void populateCyclicReference(instance, Map rebuiltProperties, DefaultUnmarshallingContext unmarshallingContext) { - for (CycleReferenceSource cr : unmarshallingContext.cycleRefStack) { - populateProperty(cr.cyclePath, rebuiltProperties, resolvePath(cr.sourcePath, instance, rebuiltProperties)) + private void populateUnboundDateTimeProperties(GroovyObject instance, Map rebuiltProperties){ + for(Map.Entry entry: rebuiltProperties.entrySet()){ + + if(instance.hasProperty(entry.key) && !instance.getProperty(entry.key) && entry.value) { + + if(instance.hasProperty(entry.key).type.simpleName == DateTime.simpleName){ + + instance."${entry.key}" = Class.forName(instance.hasProperty(entry.key).type.canonicalName, true, getClass().classLoader).parse(entry.value) + // the above hack is necessary because ElasticSearch has Joda classes under "org.elasticsearch..." package. So, + // if we tried converting the string value into a DateTime using instance."${entry.key}" = DateTime.parse(entry.value), + // we might run into trouble, since instance."${entry.key}" could evaluate to org.joda.DateTime, whereas + // DateTime.parse(entry.value) could evaluate to org.elasticsearch.DateTime + + +// instance."${entry.key}" = DateTime.parse(entry.value) + // ^^org.joda... | ^^org.elasticsearch.... + } + else if(instance.hasProperty(entry.key).type == Date && entry.value instanceof Date){ + instance."${entry.key}" = entry.value + } + } } } diff --git a/test/unit/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshallerSpec.groovy b/test/unit/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshallerSpec.groovy new file mode 100644 index 00000000..901c7df9 --- /dev/null +++ b/test/unit/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshallerSpec.groovy @@ -0,0 +1,155 @@ +package org.grails.plugins.elasticsearch.conversion.unmarshall + +import grails.test.mixin.TestMixin +import grails.test.mixin.web.ControllerUnitTestMixin +import org.elasticsearch.common.joda.time.DateTime +import spock.lang.Specification + +import java.text.SimpleDateFormat + +@TestMixin(ControllerUnitTestMixin) +class DomainClassUnmarshallerSpec extends Specification { + + DomainClassUnmarshaller unmarshaller + + def setup(){ + unmarshaller = new DomainClassUnmarshaller() + } + + def "when instance is empty and rebuiltProperties is empty, populateUnboundDateTimeProperties sets nothing"(){ + given: + def instance = new Sample() + def rebuiltProperties = [:] + + when: + unmarshaller.populateUnboundDateTimeProperties(instance, rebuiltProperties) + + then: + instance + !instance.identifier + !instance.phone + !instance.dateCreated + !instance.lastUpdated + } + + def "when instance is empty and rebuiltProperties only has properties that do not belong to the instance, populateUnboundDateTimeProperties sets nothing"(){ + given: + def instance = new Sample() + def rebuiltProperties = [name: "value", other: "value"] + + when: + unmarshaller.populateUnboundDateTimeProperties(instance, rebuiltProperties) + + then: + instance + !instance.identifier + !instance.phone + !instance.dateCreated + !instance.lastUpdated + } + + def "when instance is empty and rebuiltProperties only has properties that are not date related, populateUnboundDateTimeProperties sets nothing"(){ + given: + def instance = new Sample() + def rebuiltProperties = [identifier: "value", phone: "1234567890"] + + when: + unmarshaller.populateUnboundDateTimeProperties(instance, rebuiltProperties) + + then: + instance + !instance.identifier + !instance.phone + !instance.dateCreated + !instance.lastUpdated + } + + def "when instance is empty and rebuiltProperties has properties that are DateTime, populateUnboundDateTimeProperties sets only those dates"(){ + given: + def dateCreated = DateTime.now() + def lastUpdated = DateTime.now() + def instance = new Sample() + def rebuiltProperties = [identifier: "value", phone: "1234567890", dateCreated: dateCreated.toString(), lastUpdated: lastUpdated.toString()] + + when: + unmarshaller.populateUnboundDateTimeProperties(instance, rebuiltProperties) + + then: + instance + !instance.identifier + !instance.phone + dateCreated == instance.dateCreated + lastUpdated == instance.lastUpdated + } + + def "when instance is empty and rebuiltProperties has properties that are DateTime and Date, populateUnboundDateTimeProperties sets both types of dates"(){ + given: + def dateCreated = DateTime.now() + def lastUpdated = DateTime.now() + def lastSync = new Date() + def instance = new Sample() + def rebuiltProperties = [identifier: "value", phone: "1234567890", dateCreated: dateCreated.toString(), lastUpdated: lastUpdated.toString(), lastSync: lastSync] + + when: + unmarshaller.populateUnboundDateTimeProperties(instance, rebuiltProperties) + + then: + instance + !instance.identifier + !instance.phone + dateCreated == instance.dateCreated + lastUpdated == instance.lastUpdated + lastSync == instance.lastSync + } + + def "when instance is empty and rebuiltProperties has null properties that are DateTime, populateUnboundDateTimeProperties sets only the non-null dates"(){ + given: + def dateCreated = DateTime.now() + def instance = new Sample() + def rebuiltProperties = [identifier: "value", phone: "1234567890", dateCreated: dateCreated.toString(), lastUpdated: null] + + when: + unmarshaller.populateUnboundDateTimeProperties(instance, rebuiltProperties) + + then: + instance + !instance.identifier + !instance.phone + dateCreated == instance.dateCreated + !instance.lastUpdated + } + + def "when instance is has date properties, populateUnboundDateTimeProperties does not reset them"(){ + given: + def dateCreated = DateTime.now() + def lastUpdated = DateTime.now() + def lastSync = new Date() + def instance = new Sample(dateCreated: dateCreated, lastUpdated: lastUpdated, lastSync: lastSync) + def rebuiltProperties = [identifier: "value", phone: "1234567890", dateCreated: DateTime.now().toString(), lastUpdated: DateTime.now().toString()] + + when: + unmarshaller.populateUnboundDateTimeProperties(instance, rebuiltProperties) + + then: + instance + !instance.identifier + !instance.phone + dateCreated == instance.dateCreated + lastUpdated == instance.lastUpdated + lastSync == instance.lastSync + } + + + class Sample { + + Long identifier + String phone + DateTime dateCreated + DateTime lastUpdated + Date lastSync + + } + +} + + From 07a61191dc40fdf1228ada45bd63da1f6ca4e69f Mon Sep 17 00:00:00 2001 From: felipe Date: Tue, 14 Apr 2015 13:39:31 +0200 Subject: [PATCH 2/2] removed forgotten comments --- .../conversion/unmarshall/DomainClassUnmarshaller.groovy | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy b/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy index dfb14555..854482e3 100644 --- a/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy +++ b/src/groovy/org/grails/plugins/elasticsearch/conversion/unmarshall/DomainClassUnmarshaller.groovy @@ -115,10 +115,6 @@ class DomainClassUnmarshaller { // if we tried converting the string value into a DateTime using instance."${entry.key}" = DateTime.parse(entry.value), // we might run into trouble, since instance."${entry.key}" could evaluate to org.joda.DateTime, whereas // DateTime.parse(entry.value) could evaluate to org.elasticsearch.DateTime - - -// instance."${entry.key}" = DateTime.parse(entry.value) - // ^^org.joda... | ^^org.elasticsearch.... } else if(instance.hasProperty(entry.key).type == Date && entry.value instanceof Date){ instance."${entry.key}" = entry.value