From ceeada23c32f7f01604999c15e24c1e9d453e56e Mon Sep 17 00:00:00 2001 From: markharwood Date: Wed, 23 Sep 2020 09:25:07 +0100 Subject: [PATCH] Search case insensitivity - complete the version dance now the back port is complete (#62764) Change the versions used in serialization now the back port is complete. Re-enbaled BWC tests. Relates to #61546 --- build.gradle | 4 +- .../index/query/PrefixQueryBuilder.java | 4 +- .../index/query/TermQueryBuilder.java | 4 +- .../index/query/WildcardQueryBuilder.java | 4 +- .../index/query/PrefixQueryBuilderTests.java | 24 ++++-------- .../index/query/TermQueryBuilderTests.java | 13 +------ .../query/WildcardQueryBuilderTests.java | 38 ++++++++----------- 7 files changed, 33 insertions(+), 58 deletions(-) diff --git a/build.gradle b/build.gradle index 88a3e8b02e73..53a3a164c39d 100644 --- a/build.gradle +++ b/build.gradle @@ -174,8 +174,8 @@ tasks.register("verifyVersions") { * after the backport of the backcompat code is complete. */ -boolean bwc_tests_enabled = false -final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/62764" /* place a PR link here when committing bwc changes */ +boolean bwc_tests_enabled = true +final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */ if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false") diff --git a/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java index e2273a9dbf19..cd45c0dc10ae 100644 --- a/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java @@ -84,7 +84,7 @@ public PrefixQueryBuilder(StreamInput in) throws IOException { fieldName = in.readString(); value = in.readString(); rewrite = in.readOptionalString(); - if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + if (in.getVersion().onOrAfter(Version.V_7_10_0)) { caseInsensitive = in.readBoolean(); } } @@ -94,7 +94,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeString(fieldName); out.writeString(value); out.writeOptionalString(rewrite); - if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + if (out.getVersion().onOrAfter(Version.V_7_10_0)) { out.writeBoolean(caseInsensitive); } } diff --git a/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java index 25270acd12ed..e1ff338f4010 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java @@ -104,14 +104,14 @@ public boolean caseInsensitive() { */ public TermQueryBuilder(StreamInput in) throws IOException { super(in); - if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + if (in.getVersion().onOrAfter(Version.V_7_10_0)) { caseInsensitive = in.readBoolean(); } } @Override protected void doWriteTo(StreamOutput out) throws IOException { super.doWriteTo(out); - if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + if (out.getVersion().onOrAfter(Version.V_7_10_0)) { out.writeBoolean(caseInsensitive); } } diff --git a/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java index 18876a5828dc..4b309bd880aa 100644 --- a/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java @@ -94,7 +94,7 @@ public WildcardQueryBuilder(StreamInput in) throws IOException { fieldName = in.readString(); value = in.readString(); rewrite = in.readOptionalString(); - if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + if (in.getVersion().onOrAfter(Version.V_7_10_0)) { caseInsensitive = in.readBoolean(); } } @@ -104,7 +104,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeString(fieldName); out.writeString(value); out.writeOptionalString(rewrite); - if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + if (out.getVersion().onOrAfter(Version.V_7_10_0)) { out.writeBoolean(caseInsensitive); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java index f90e5e4815b0..fa94900d8efd 100644 --- a/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.query; import org.apache.lucene.index.Term; +import org.apache.lucene.search.AutomatonQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.MultiTermQuery; import org.apache.lucene.search.PrefixQuery; @@ -44,13 +45,6 @@ protected PrefixQueryBuilder doCreateTestQueryBuilder() { if (randomBoolean()) { query.rewrite(getRandomRewriteMethod()); } - //TODO code below is commented out while we do the Version dance for PR 61596. Steps are - // 1) Commit PR 61596 with this code commented out in master - // 2) Backport PR 61596 to 7.x, uncommented - // 3) New PR on master to uncomment this code now that 7.x has support for case insensitive flag. -// if (randomBoolean()) { -// query.caseInsensitive(true); -// } return query; } @@ -77,8 +71,10 @@ private static PrefixQueryBuilder randomPrefixQuery() { @Override protected void doAssertLuceneQuery(PrefixQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { - assertThat(query, Matchers.anyOf(instanceOf(PrefixQuery.class), instanceOf(MatchNoDocsQuery.class))); - if (context.fieldMapper(queryBuilder.fieldName()) != null) { // The field is mapped + assertThat(query, Matchers.anyOf(instanceOf(PrefixQuery.class), instanceOf(MatchNoDocsQuery.class), + instanceOf(AutomatonQuery.class))); + if (context.fieldMapper(queryBuilder.fieldName()) != null + && queryBuilder.caseInsensitive() == false) { // The field is mapped and case sensitive PrefixQuery prefixQuery = (PrefixQuery) query; String expectedFieldName = expectedFieldName(queryBuilder.fieldName()); @@ -108,13 +104,9 @@ public void testBlendedRewriteMethod() throws IOException { public void testFromJson() throws IOException { String json = - "{ \"prefix\" : { \"user\" : { \"value\" : \"ki\", \"boost\" : 2.0 " - //TODO code below is commented out while we do the Version dance for PR 61596. Steps are - // 1) Commit PR 61596 with this code commented out in master - // 2) Backport PR 61596 to 7.x, uncommented - // 3) New PR on master to uncomment this code now that 7.x has support for case insensitive flag. -// " \"case_insensitive\" : true\n" + - + "{ \"prefix\" : { \"user\" : { \"value\" : \"ki\",\n" + + " \"case_insensitive\" : true,\n" + + " \"boost\" : 2.0" + "} }}"; PrefixQueryBuilder parsed = (PrefixQueryBuilder) parseQuery(json); diff --git a/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java index 8a85b90cd8dd..b6d4f8f64760 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java @@ -89,13 +89,6 @@ protected TermQueryBuilder doCreateTestQueryBuilder() { @Override protected TermQueryBuilder createQueryBuilder(String fieldName, Object value) { TermQueryBuilder result = new TermQueryBuilder(fieldName, value); - //TODO code below is commented out while we do the Version dance for PR 61596. Steps are - // 1) Commit PR 61596 with this code commented out in master - // 2) Backport PR 61596 to 7.x, uncommented - // 3) New PR on master to uncomment this code now that 7.x has support for case insensitive flag. -// if (randomBoolean()) { -// result.caseInsensitive(true); -// } return result; } @@ -142,12 +135,8 @@ public void testFromJson() throws IOException { " \"term\" : {\n" + " \"exact_value\" : {\n" + " \"value\" : \"Quick Foxes!\",\n" + + " \"case_insensitive\" : true,\n" + " \"boost\" : 1.0\n" + - //TODO code below is commented out while we do the Version dance for PR 61596. Steps are - // 1) Commit PR 61596 with this code commented out in master - // 2) Backport PR 61596 to 7.x, uncommented - // 3) New PR on master to uncomment this code now that 7.x has support for case insensitive flag. -// " \"case_insensitive\" : true\n" + " }\n" + " }\n" + "}"; diff --git a/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java index f2706c2ad9b3..deeced8dd1f4 100644 --- a/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.AutomatonQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.WildcardQuery; @@ -41,13 +42,6 @@ protected WildcardQueryBuilder doCreateTestQueryBuilder() { if (randomBoolean()) { query.rewrite(randomFrom(getRandomRewriteMethod())); } - //TODO code below is commented out while we do the Version dance for PR 61596. Steps are - // 1) Commit PR 61596 with this code commented out in master - // 2) Backport PR 61596 to 7.x, uncommented - // 3) New PR on master to uncomment this code now that 7.x has support for case insensitive flag. -// if (randomBoolean()) { -// query.caseInsensitive(true); -// } return query; } @@ -78,14 +72,18 @@ protected void doAssertLuceneQuery(WildcardQueryBuilder queryBuilder, Query quer String expectedFieldName = expectedFieldName(queryBuilder.fieldName()); if (expectedFieldName.equals(TEXT_FIELD_NAME)) { - assertThat(query, instanceOf(WildcardQuery.class)); - WildcardQuery wildcardQuery = (WildcardQuery) query; - - assertThat(wildcardQuery.getField(), equalTo(expectedFieldName)); - assertThat(wildcardQuery.getTerm().field(), equalTo(expectedFieldName)); - // wildcard queries get normalized - String text = wildcardQuery.getTerm().text().toLowerCase(Locale.ROOT); - assertThat(text, equalTo(text)); + if (queryBuilder.caseInsensitive()) { + assertThat(query, instanceOf(AutomatonQuery.class)); + } else { + assertThat(query, instanceOf(WildcardQuery.class)); + WildcardQuery wildcardQuery = (WildcardQuery) query; + + assertThat(wildcardQuery.getField(), equalTo(expectedFieldName)); + assertThat(wildcardQuery.getTerm().field(), equalTo(expectedFieldName)); + // wildcard queries get normalized + String text = wildcardQuery.getTerm().text().toLowerCase(Locale.ROOT); + assertThat(text, equalTo(text)); + } } else { Query expected = new MatchNoDocsQuery("unknown field [" + expectedFieldName + "]"); assertEquals(expected, query); @@ -110,13 +108,9 @@ public void testEmptyValue() throws IOException { } public void testFromJson() throws IOException { - String json = "{ \"wildcard\" : { \"user\" : { \"wildcard\" : \"ki*y\", \"boost\" : 2.0" - //TODO code below is commented out while we do the Version dance for PR 61596. Steps are - // 1) Commit PR 61596 with this code commented out in master - // 2) Backport PR 61596 to 7.x, uncommented - // 3) New PR on master to uncomment this code now that 7.x has support for case insensitive flag. -// " \"case_insensitive\" : true\n" + - + String json = "{ \"wildcard\" : { \"user\" : { \"wildcard\" : \"ki*y\"," + + " \"case_insensitive\" : true,\n" + + " \"boost\" : 2.0" + " } }}"; WildcardQueryBuilder parsed = (WildcardQueryBuilder) parseQuery(json); checkGeneratedJson(json, parsed);