Move GeoLocation object to Spring Data Commons. [DATACMNS-437]

[spring-projects-issues]

Petar Tahchiev opened DATACMNS-437 and commented

Hi guys,

I'm trying to implement a solution that can support Spring-Data-SOLR and Spring-Data-Elasticsearch on the fly (no recompilation needed). My problem is with the geolocation parameters. If I want to export a Geolocation parameter to the solrIndex I need to define it of type:

org.springframework.data.solr.core.geo.GeoLocation.class

and if I want to export it to the elasticsearchIndex I need to define it of type:

org.elasticsearch.common.geo.GeoPoint.class

The problem here is that I need to ask the user to chose the solr or elasticsearch and then get a specific version of my API :( .. It would be so much better if both SDS and SDE were both using one class defined in Spring-data-commons. Or if that's not possible, it would be a lot better if their classes implemented a common interface defined in Spring-data-commons. One of the key principals of Spring is using interfaces, but I don't see it here. It could be something like this:

public interface GeoLocation {
     BigDecimal getLatitude();

     BigDecimal getLongitude();
}

---

Issue Links:

• DATAGRAPH-454 Support for Common Geospatial Types and Derived spatial Queries ("is depended on by")

• DATAMONGO-858 Add support for common geospatial structures ("is depended on by")

• DATACMNS-475 Add Jackson module to deserialize new geo types

• DATASOLR-142 Support geo types of spring data commons

Referenced from: pull request https://github.com/spring-projects/spring-data-commons/pull/68

0 votes, 5 watchers

[spring-projects-issues]

Oliver Drotbohm commented

That's a great suggestion, Petar. So far we went with the store specific implementations as premature abstraction is the root of all evil ;). Seriously, we didn't want to introduce abstractions, without being sure where to draw the lines in terms of API and functionality.

However, now that a few implementations have emerged it's probably a good time to evaluate possibilities to extract common abstractions for people to use in their interfaces

[spring-projects-issues]

Petar Tahchiev commented

Hi Oli,

my understanding is that both projects (and any other search-related spring data project) must follow the same abstraction, which is not the case right now. At the moment I see some major differences (just a few but major ones). For instance this one: https://jira.springsource.org/browse/DATAES-20 which I provided a patch for, half a year ago, is still not fixed. Another one is the ability to specify index name in elasticsearch (@Document(indexName = "pointofservice-index", type = "geo-class-point-type")), while the same functionality is missing for solr. Also the class for the @Facet annotation and the @Query annotation is different in SDS and SDE (though it has the same name!). So this means I can't create a single repository and reuse it by just changing jars in the classpath.

[spring-projects-issues]

Oliver Drotbohm commented

Petar Tahchiev Let's not side track this ticket into completely off topics. If you have filed a ticket for Spring Data Elasticsearch, it's the perfect place to ask for the current state of the affairs. SD ES is a community project and these guys work on their own pace, we're not controlling everything here.

Still, it's probably a good chance to make sure you understand the foundational goals of the Spring Data projects. Or even more precise, to understand the non-goals of it. Being able to write completely store agnostic repository interfaces is not a goal of ours. First and foremost this is due to the fact that the individual stores provide different functionality which you want to make use of. Plus, the devil is usually in the details. Indexing is available for a lot of stores but they work remarkably different in all of them. In some cases the index is effectively what the data get's stored in (Solr, Elasticsearch). In others it's a secondary index to speed up access to the data only.

A consequence of that is that the customization and configuration options for the seemingly same things are incredibly different. Hence, it just doesn't make sense to try to provide a single unifying @Index, @Document or @Query annotation. Still the same naming of the annotations is by purpose and in general the annotations work in a similar fashion. You remember you used @Query in JPA, so you think: "Ah, there should probably be one for MongoDB as well!" - and there is. And both take the store specific query string as value attribute.

The value add that Spring Data provides here is that the programming model stays the same with each store. You know how templates work, you know how repositories work. That's why you can quickly pick up a new supported store.

The other aspect you mentioned is trying to create a single repository layer and the just switching stores. That's not an approach we actually want to propagate. At least it's not a primary task of ours to help you achieve that as it will - by definition - end up in bad compromises especially regarding the domain model. If you switch from a JPA based mapping to a MongoDB one, exchanging the mapping annotations is not enough. You will have tweak your customizations, even more important - you should rethink your domain modeling, as certain ways of modeling work superb on a relational store but are absolutely no-goes for Mongo and vice versa.

The bottom line here is that you still will have to know how the store works, just as you have to know about relational persistence even if you work with Hibernate. We're pretty much only providing a unified set of affordances to modeling a data access layer

[spring-projects-issues]

Oliver Drotbohm commented

Initial feedback provided at the pull request

[spring-projects-issues]

Thomas Darimont commented

I added some comments to the pull request. Please review

[spring-projects-issues]

Thomas Darimont commented

I applied the requested changes, please revise

[spring-projects-issues]

Thomas Darimont commented

Adjusted PR according to code review - please revise

[spring-projects-issues]

Mark Diskin commented

Is a reason why you introduced your own classes for Point, Circle, Polygon,.. instead of using an existing framework such as jts (base code for geolatte, hibernate spatial,etc..) that provides there and much, much more? Hopefully you'll reconsider before the 1.8 is released.

Thanks Mark

[spring-projects-issues]

Oliver Drotbohm commented

Yes, there is. All the available libraries out there are actually designed in the context of doing geo-spatial calculations. The types we need are targeted towards persistence. This effectively means we'd pull in a lot of additional internals that we would have to take care of when persisting the types. Plus we'd add a dependency with quite some weight that doesn't really bring any benefits. Another aspect to that is that whichever library we'd choose the incompatibility towards the other libraries would still exist.

So to summarize, here are the main drivers for the decision:

• Simple types, reusable amongst Spring Data projects
• Designed with the purpose of persistence, not general purpose geo calculations
• As little additional dependency as possible

[spring-projects-issues]

Mark Diskin commented

I respect your comments but the choice to provide basic or singular use-case support will have us maintaining our own logic aka data common code. I see it as a missed opportunity to build on some great work out there in the Java ecosystem at the same time not playing to Spring's strengths as the great abstracter for third-party libraries.

We'll continue to leverage the libraries below and see if we can blend it into spring data on our side; ideally it would be nice to see spring data support multiple providers via a plugin architecture where the "out of the box" geo support but switch it out via a plugin for a more complete solution.

jts - is a pure java library for official specification Geo types. geolatte (modules for noqsl, SQL, Hibernate,...) - Extends JTS and provides data persistence modules and REST GeoJson data serialization

[spring-projects-issues]

Oliver Drotbohm commented

But you actually get to the point: if we integrate with jts, Spatial4j users will be turned off and the other way round. Besides the fact that JTS hasn't seen an upgrade in over a year and a ~800 kb dependency for 5 value objects is a bit of a high price.

It might be worthwhile to mention that you're of course free to persist any geo type that you'd like to by using the store specific means to provide custom converters (simple Converter implementations in MongoDB for example). I wouldn't even mind to see tickets asking to let us ship them out of the box in case the library is on the class path, just as we do with JodaTime converters. The place for that code cannot be Spring Data Commons by definition as will have to adhere to the custom traits of the store (see e.g. DATAMONGO-858 for the way we persist our own types to MongoDB).

However the integration with 3rd party geo libraries is not in the scope of this tickets. The ticket's purpose is to provide a shareable API to do implement queries and types to use on repository interfaces, so that you don't have to alter your repos and queries in case you switch from one store to another. In that sense, the current step indeed provides an abstraction as you'll be able to query data written by using whatever geo library you like, as long as your converters persist the special-library geo types in the appropriate store format.

Does that make sense?

[spring-projects-issues]

Mark Diskin commented

I do understand and the converter might work. I say might because without doing more investigation into the classes for completeness will it (for example) support other Distance models such as Sphere's and Spheroid's using different models of the earth.

Also since the Spring Data JDBC is the red-headed stepchild :-) project and not part of the the Data Train releases would above about tickets and following the Mongo model still apply?

[spring-projects-issues]

Petar Tahchiev commented

Guys the Point class is using java.lang.Double. Wouldn't it be better if it was using BigDecimal