Fix for JSONObject marshalling. #21

Merged
merged 1 commit into from Apr 2, 2014

Conversation

Projects
None yet
2 participants

fdearle commented Apr 1, 2014

The 0.0.2.5 release broke the case where a JSONObject is an indexed field of a DomainClass. This change reorders the test for GeoPoint so that non domainClasses are not affected by th e0.0.2.5 change.

Owner

noamt commented Apr 1, 2014

Does the test also unmarshall the json object? it doesn't seem so to unmarshall it

fdearle commented Apr 1, 2014

No you are right it does not. We did enough to replicate the NPE in the
tests and then fixed that, which is why I'm saying you may not want to use
the yes changes we added.

Regards

Fergal

On Tue, Apr 1, 2014 at 4:10 PM, Noam Y. Tenne notifications@github.comwrote:

Does the test also unmarshall the json object? it doesn't seem so in the
test

Reply to this email directly or view it on GitHubhttps://github.com/noamt/elasticsearch-gorm-plugin/pull/21#issuecomment-39216847
.

Owner

noamt commented Apr 1, 2014

So the fix is just for the marshalling, then?

fdearle commented Apr 1, 2014

The issue we were seeing was purely during indexing. SO it was at the point
that the JSON was being marshalled. At that point in time the
delegateMarshalling method was mistakenly treating the JSON field as a
domain class.

So:

def propertyMapping =
elasticSearchContextHolder.getMappingContext(domainClass) failed with an
NPE due to domainCLass being null at tha point.

Regards

Fergal

On Tue, Apr 1, 2014 at 4:28 PM, Noam Y. Tenne notifications@github.comwrote:

So the fix is just for the marshalling, then?

Reply to this email directly or view it on GitHubhttps://github.com/noamt/elasticsearch-gorm-plugin/pull/21#issuecomment-39219052
.

noamt added a commit that referenced this pull request Apr 2, 2014

noamt added a commit that referenced this pull request Apr 2, 2014

@noamt noamt merged commit 7274e51 into noamt:master Apr 2, 2014

Owner

noamt commented Apr 2, 2014

OK, I've deployed a test snapshot; wanna try it out before I release?

Add this Maven repository to your build config:
http://noams.artifactoryonline.com/noams/grails-elasticsearch-plugin-snapshots

And depend on the plugin:
:elasticsearch:0.0.2.x-SNAPSHOT

Note that the plugin name has changed from elasticsearch-gorm to elasticsearch and will be so as of the next release

fdearle commented Apr 2, 2014

Noam,

That looks good thanks.

Regards

Fergal

On Wed, Apr 2, 2014 at 8:45 AM, Noam Y. Tenne notifications@github.comwrote:

OK, I've deployed a test snapshot; wanna try it out before I release?

Add this Maven repository to your build config:

http://noams.artifactoryonline.com/noams/grails-elasticsearch-plugin-snapshots

And depend on the plugin:
:elasticsearch:0.0.2.x-SNAPSHOT

Note that the plugin name has changed from elasticsearch-gorm to
elasticsearch and will be so as of the next release

Reply to this email directly or view it on GitHubhttps://github.com/noamt/elasticsearch-grails-plugin/pull/21#issuecomment-39303126
.

Owner

noamt commented Apr 2, 2014

Released. Please note that the plugin is now published under http://grails.org/plugin/elasticsearch

fdearle commented Apr 2, 2014

Thanks much Noam.

I owe you a beer. So you can collect at GR8 in Copenhagen in June :-)

Fergal

On Wed, Apr 2, 2014 at 12:03 PM, Noam Y. Tenne notifications@github.comwrote:

Released. Please note that the plugin is now published under
http://grails.org/plugin/elasticsearch

Reply to this email directly or view it on GitHubhttps://github.com/noamt/elasticsearch-grails-plugin/pull/21#issuecomment-39317360
.

Owner

noamt commented Apr 2, 2014

With pleasure, see you there! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment