Discuss unique ProjectionConstants API

Description

We're currently exposing both a org.hibernate.search.backend.elasticsearch.ProjectionConstants and a org.hibernate.search.engine.ProjectionConstants.

It's not nice to have two classes in the public API which have the same name, as the package difference isn't very clear when reading code.

Should we simply expose the Elasticsearch specific projection constants in the parent class?

Should we make these options somehow better errorprone, maybe via typesafety, to prevent using a constant which isn't accepted?

At the very least we should verify that the non-Elasticsearch backend recognises the constants currently defined on org.hibernate.search.backend.elasticsearch.ProjectionConstants and warns about a possible mistake. I believe it would currently attempt to load a stored field having the name of the constant value.

Activity

Show:

Sanne GrinoveroFebruary 24, 2016 at 3:29 PM
Edited

I don't like exposing the ES constants on the default projection constants interface, there should be no knowledge about ES be present in core.

It's just a string slightly smiling face But ok, +1 to simply rename it.

Gunnar MorlingFebruary 24, 2016 at 11:33 AM

We can rename it to ElasticsearchProjectionConstants. I don't like exposing the ES constants on the default projection constants interface, there should be no knowledge about ES be present in core.

We may raise a warning/error when spotting an unknown __HSearch_XYZ projection constant, but I don't consider this very critical.

Fixed

Details

Assignee

Reporter

Fix versions

Priority

Created February 23, 2016 at 6:31 PM
Updated May 21, 2016 at 6:43 PM
Resolved May 12, 2016 at 12:55 PM

Flag notifications