search

ngageoint / elasticgeo

ElasticGeo provides a GeoTools data store that allows geospatial features from an Elasticsearch index to be published via OGC services using GeoServer.
GNU General Public License v3.0
169 stars 85 forks source link

Highlighting Support #93

Open dclemenzi opened 5 years ago

dclemenzi commented 5 years ago

91 Implemented support for highlights

dclemenzi commented 5 years ago

Yeah I will put together a unit test and documentation.

sjudeng commented 5 years ago

Can you rebase this on the latest master branch when you get a chance?

dclemenzi commented 5 years ago

Sorry other priorities have come up and I have not gotten back to the unit tests that you request. Also I have been rethinking how the aggregation and highlighting were implemented. Not sure passing database proprietary JSON to the query as a hint is the best approach. Been thinking about adopting a custom function such as highlight(PropertyName) or aggregate(PropertyName, ...) so the interface will be common across multiple database implementations. The FilterToElastic would of course need to translate the function. Maybe there are other solutions already available in the geotool libraries, have not really looked. Thoughts?

sjudeng commented 5 years ago

Adding as a custom function sounds like an interesting idea but I haven't looked at it either. The use of viewparams was modeled after what's done in SolrDataStore. Let me know whether you want to continue with this PR or hold it while you explore the possible alternate implementation.

dclemenzi commented 5 years ago

We have used custom functions before to handle other edge cases so I know it would work. Just not sure if its the correct solution. I am sure others have run into the same issue using geotool APIs so I would not be surprised if there was not some approach already within geotools. When I get around to implementing something more generic (highlighting and aggregations) it would probably be best to continue supporting the hints (maybe as deprecated) to avoid breaking anyone depending on it. We are still using version 19.3 of geotools so we are not using the master branch so merging this has no impact on us. Its up to you whether you want to accept this implementation or wait for a more generic approach. Can't say when I will get back to this.

sjudeng commented 5 years ago

Sounds good. Thanks for the rebase.