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.

Environment

None

Activity

Show:
Gunnar Morling
February 24, 2016, 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.

Sanne Grinovero
February 24, 2016, 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 But ok, +1 to simply rename it.

Fixed

Assignee

Gunnar Morling

Reporter

Sanne Grinovero

Labels

None

Suitable for new contributors

None

Feedback Requested

None

Components

Fix versions

Priority

Blocker
Configure