[JIRA] (HSEARCH-4705) Merge the various implementations ElasticsearchTestDialect
by Yoann Rodière (JIRA)
Yoann Rodière ( https://hibernate.atlassian.net/secure/ViewProfile.jspa?accountId=557058%... ) *created* an issue
Hibernate Search ( https://hibernate.atlassian.net/browse/HSEARCH?atlOrigin=eyJpIjoiOTI4YzA4... ) / Task ( https://hibernate.atlassian.net/browse/HSEARCH-4705?atlOrigin=eyJpIjoiOTI... ) HSEARCH-4705 ( https://hibernate.atlassian.net/browse/HSEARCH-4705?atlOrigin=eyJpIjoiOTI... ) Merge the various implementations ElasticsearchTestDialect ( https://hibernate.atlassian.net/browse/HSEARCH-4705?atlOrigin=eyJpIjoiOTI... )
Issue Type: Task Assignee: Unassigned Components: build, tests Created: 21/Sep/2022 05:36 AM Fix Versions: 7.0-backlog Priority: Major Reporter: Yoann Rodière ( https://hibernate.atlassian.net/secure/ViewProfile.jspa?accountId=557058%... )
Nowadays we can check the version of Elasticsearch we’re testing against with ElasticsearchTestDialect.getActualVersion().
So, we could easily merge all the various implementations by using if/else blocks in each methods, e.g.
@Override
public boolean normalizesStringArgumentToWildcardPredicateForAnalyzedStringField() {
// In ES 7.7 through 7.11, wildcard predicates on analyzed fields get their pattern normalized,
// but that was deemed a bug and fixed in 7.12.2+: https://github.com/elastic/elasticsearch/pull/53127
ElasticsearchVersion version = getActualVersion();
return version.major() == 7 && 7 <= version.minor() && version.minor <= 11;
}
Bonus points if we don’t use the exact code above, but cleaner util methods, e.g. isActualVersionBetween("elastic:7.7.0", "elastic:7.12.1").
While a bit dirty, this has the advantage of:
* Being extra clear: one look at the method implementation, and you know what happens with each version of Elasticsearch.
* Being very flexible: we can add exceptions for specific micros of Elasticsearch.
* More future proof: when addign workarounds to ignore bugs of Elasticsearch, we can willingly target a specific micro of Elasticsearch. That way, if the bug was fixed, the workaround will be ignored automatically, and if it wasn’t, we will get a reminder of that bug and will have to expand the workaround explicitly.
One we’ve merged all ElasticsearchTestDialect implementations together:
* We can simplify Maven profiles, which will no longer need to specify the dialect they need
* We can move most methods to ElasticsearchBackendFeatures , because they’re only useful there. Some methods will probably stay useful somewhere else, e.g. TestElasticsearchClient.
( https://hibernate.atlassian.net/browse/HSEARCH-4705#add-comment?atlOrigin... ) Add Comment ( https://hibernate.atlassian.net/browse/HSEARCH-4705#add-comment?atlOrigin... )
Get Jira notifications on your phone! Download the Jira Cloud app for Android ( https://play.google.com/store/apps/details?id=com.atlassian.android.jira.... ) or iOS ( https://itunes.apple.com/app/apple-store/id1006972087?pt=696495&ct=EmailN... ) This message was sent by Atlassian Jira (v1001.0.0-SNAPSHOT#100207- sha1:a65942b )
2 years, 3 months