From 1f45d652acf79d20a3286cb9e014df52f86c3001 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 27 Mar 2024 12:02:46 +0100 Subject: [PATCH 01/27] Add version enum --- .../xpack/esql/version/EsqlVersion.java | 36 +++++++++++++++++++ .../xpack/esql/version/EsqlVersionTests.java | 25 +++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java new file mode 100644 index 0000000000000..85c2fe1bf7848 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.version; + +import org.elasticsearch.common.VersionId; + +public enum EsqlVersion implements VersionId { + // Breaking changes go here until the next version is released. + NIGHTLY(Integer.MAX_VALUE, Integer.MAX_VALUE, "\uD83D\uDE34"), + PARTY_POPPER(2024, 4, "\uD83C\uDF89"); + + EsqlVersion(int year, int month, String emoji) { + this.year = year; + this.month = month; + this.emoji = emoji; + } + + private int year; + private int month; + private String emoji; + + @Override + public String toString() { + return this == NIGHTLY ? "nightly." + emoji : String.format("%d.%02d.%s", year, month, emoji); + } + + @Override + public int id() { + return this == NIGHTLY ? Integer.MAX_VALUE : (100*year + month); + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java new file mode 100644 index 0000000000000..cd78ab1b719c3 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.version; + +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.equalTo; +import static org.elasticsearch.xpack.esql.version.EsqlVersion.*; + +public class EsqlVersionTests extends ESTestCase { + public void testVersionString() { + assertThat(NIGHTLY.toString(), equalTo("nightly.\uD83D\uDE34")); + assertThat(PARTY_POPPER.toString(), equalTo("2024.04.\uD83C\uDF89")); + } + + public void testVersionId() { + assertThat(NIGHTLY.id(), equalTo(Integer.MAX_VALUE)); + assertThat(PARTY_POPPER.id(), equalTo(202404)); + } +} From c4d5a8ed04d07eb07e3175f63e5c2cdb66146e8c Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 27 Mar 2024 15:17:59 +0100 Subject: [PATCH 02/27] Parse + validate esql.version --- .../core/esql/action/EsqlQueryRequest.java | 2 ++ .../esql/action/EsqlQueryRequestBuilder.java | 2 ++ .../xpack/esql/action/EsqlQueryRequest.java | 35 +++++++++++++++++-- .../esql/action/EsqlQueryRequestBuilder.java | 6 ++++ .../xpack/esql/action/RequestXContent.java | 7 ++-- .../xpack/esql/version/EsqlVersion.java | 24 ++++++++++++- .../xpack/esql/version/EsqlVersionTests.java | 24 ++++++++++++- 7 files changed, 94 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java index 9faa78d3b34f9..c7a5b583fed42 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java @@ -21,6 +21,8 @@ protected EsqlQueryRequest(StreamInput in) throws IOException { super(in); } + public abstract String esqlVersion(); + public abstract String query(); public abstract QueryBuilder filter(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequestBuilder.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequestBuilder.java index a0a2bbc3bed19..acd44165cad65 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequestBuilder.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequestBuilder.java @@ -35,6 +35,8 @@ public final ActionType action() { return action; } + public abstract EsqlQueryRequestBuilder esqlVersion(String esqlVersion); + public abstract EsqlQueryRequestBuilder query(String query); public abstract EsqlQueryRequestBuilder filter(QueryBuilder filter); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index e5ff790619d14..d7ac90de2ead7 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -20,6 +20,7 @@ import org.elasticsearch.tasks.TaskId; import org.elasticsearch.xpack.esql.parser.TypedParamValue; import org.elasticsearch.xpack.esql.plugin.QueryPragmas; +import org.elasticsearch.xpack.esql.version.EsqlVersion; import java.io.IOException; import java.util.List; @@ -35,6 +36,7 @@ public class EsqlQueryRequest extends org.elasticsearch.xpack.core.esql.action.E private boolean async; + private String esqlVersion; private String query; private boolean columnar; private boolean profile; @@ -65,17 +67,46 @@ public EsqlQueryRequest(StreamInput in) throws IOException { @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; + if (Strings.hasText(esqlVersion) == false) { + // TODO: make this required + // validationException = addValidationError("[" + RequestXContent.ESQL_VERSION_FIELD + "] is required", validationException); + } else { + EsqlVersion version = EsqlVersion.parse(esqlVersion); + if (version == null) { + validationException = addValidationError( + "[" + RequestXContent.ESQL_VERSION_FIELD + "] has invalid value [" + esqlVersion + "]", + validationException + ); + } else if (version == EsqlVersion.NIGHTLY && Build.current().isSnapshot() == false) { + validationException = addValidationError( + "[" + RequestXContent.ESQL_VERSION_FIELD + "] with value [" + esqlVersion + "] only allowed in snapshot builds", + validationException + ); + } + } if (Strings.hasText(query) == false) { - validationException = addValidationError("[query] is required", validationException); + validationException = addValidationError("[" + RequestXContent.QUERY_FIELD + "] is required", validationException); } if (Build.current().isSnapshot() == false && pragmas.isEmpty() == false) { - validationException = addValidationError("[pragma] only allowed in snapshot builds", validationException); + validationException = addValidationError( + "[" + RequestXContent.PRAGMA_FIELD + "] only allowed in snapshot builds", + validationException + ); } return validationException; } public EsqlQueryRequest() {} + public void esqlVersion(String esqlVersion) { + this.esqlVersion = esqlVersion; + } + + @Override + public String esqlVersion() { + return esqlVersion; + } + public void query(String query) { this.query = query; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestBuilder.java index 7df5c95cbc953..511fbd9f1c275 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestBuilder.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestBuilder.java @@ -29,6 +29,12 @@ private EsqlQueryRequestBuilder(ElasticsearchClient client, EsqlQueryRequest req super(client, EsqlQueryAction.INSTANCE, request); } + @Override + public EsqlQueryRequestBuilder esqlVersion(String esqlVersion) { + request.esqlVersion(esqlVersion); + return this; + } + @Override public EsqlQueryRequestBuilder query(String query) { request.query(query); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index 8db940d5a4779..a90fc4d5240d4 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -46,10 +46,12 @@ final class RequestXContent { PARAM_PARSER.declareString(constructorArg(), TYPE); } - private static final ParseField QUERY_FIELD = new ParseField("query"); + // TODO: Maybe esql_version would be better? This looks like there's a JSON attribute `esql` with subattribute `version`. + static final ParseField ESQL_VERSION_FIELD = new ParseField("esql.version"); + static final ParseField QUERY_FIELD = new ParseField("query"); private static final ParseField COLUMNAR_FIELD = new ParseField("columnar"); private static final ParseField FILTER_FIELD = new ParseField("filter"); - private static final ParseField PRAGMA_FIELD = new ParseField("pragma"); + static final ParseField PRAGMA_FIELD = new ParseField("pragma"); private static final ParseField PARAMS_FIELD = new ParseField("params"); private static final ParseField LOCALE_FIELD = new ParseField("locale"); private static final ParseField PROFILE_FIELD = new ParseField("profile"); @@ -72,6 +74,7 @@ static EsqlQueryRequest parseAsync(XContentParser parser) { } private static void objectParserCommon(ObjectParser parser) { + parser.declareString(EsqlQueryRequest::esqlVersion, ESQL_VERSION_FIELD); parser.declareString(EsqlQueryRequest::query, QUERY_FIELD); parser.declareBoolean(EsqlQueryRequest::columnar, COLUMNAR_FIELD); parser.declareObject(EsqlQueryRequest::filter, (p, c) -> AbstractQueryBuilder.parseTopLevelQuery(p), FILTER_FIELD); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 85c2fe1bf7848..43deebab57169 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.esql.version; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.VersionId; public enum EsqlVersion implements VersionId { @@ -24,6 +25,27 @@ public enum EsqlVersion implements VersionId { private int month; private String emoji; + /** + * Version prefix that we accept when parsing. If a version string starts with the given prefix, we consider the version string valid. + * E.g. "2024.04.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04" and "2024.04foobar". + */ + String parsingPrefix() { + return this == NIGHTLY ? "nightly" : String.format("%d.%02d", year, month); + } + + public static EsqlVersion parse(String versionString) { + EsqlVersion parsed = null; + if (Strings.hasText(versionString)) { + versionString = versionString.toLowerCase(); + for (EsqlVersion version : EsqlVersion.values()) { + if (versionString.startsWith(version.parsingPrefix())) { + return version; + } + } + } + return parsed; + } + @Override public String toString() { return this == NIGHTLY ? "nightly." + emoji : String.format("%d.%02d.%s", year, month, emoji); @@ -31,6 +53,6 @@ public String toString() { @Override public int id() { - return this == NIGHTLY ? Integer.MAX_VALUE : (100*year + month); + return this == NIGHTLY ? Integer.MAX_VALUE : (100 * year + month); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index cd78ab1b719c3..c24eb31369bcc 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -9,8 +9,11 @@ import org.elasticsearch.test.ESTestCase; -import static org.hamcrest.Matchers.equalTo; +import java.util.Arrays; + import static org.elasticsearch.xpack.esql.version.EsqlVersion.*; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; public class EsqlVersionTests extends ESTestCase { public void testVersionString() { @@ -22,4 +25,23 @@ public void testVersionId() { assertThat(NIGHTLY.id(), equalTo(Integer.MAX_VALUE)); assertThat(PARTY_POPPER.id(), equalTo(202404)); } + + public void testParsingPrefix() { + for (EsqlVersion version : EsqlVersion.values()) { + String[] versionSegments = version.toString().split("\\."); + String[] parsingPrefixSegments = Arrays.copyOf(versionSegments, versionSegments.length - 1); + + String expectedParsingPrefix = String.join(".", parsingPrefixSegments); + assertThat(version.parsingPrefix(), equalTo(expectedParsingPrefix)); + } + } + + public void testParsing() { + for (EsqlVersion version : EsqlVersion.values()) { + int suffixLength = randomIntBetween(0, 10); + String validVersionString = version.parsingPrefix() + randomUnicodeOfLength(suffixLength); + + assertThat(EsqlVersion.parse(validVersionString), is(version)); + } + } } From 26b9fe664552755bf285b4b8620c53676461c9a5 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 27 Mar 2024 16:28:51 +0100 Subject: [PATCH 03/27] Add tests --- .../xpack/esql/action/EsqlQueryRequest.java | 1 + .../xpack/esql/version/EsqlVersion.java | 4 +- .../esql/action/EsqlQueryRequestTests.java | 83 ++++++++++++++++++- .../xpack/esql/version/EsqlVersionTests.java | 6 +- 4 files changed, 86 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index d7ac90de2ead7..5199e9e78ae6a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -69,6 +69,7 @@ public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; if (Strings.hasText(esqlVersion) == false) { // TODO: make this required + // "https://github.com/elastic/elasticsearch/issues/104890" // validationException = addValidationError("[" + RequestXContent.ESQL_VERSION_FIELD + "] is required", validationException); } else { EsqlVersion version = EsqlVersion.parse(esqlVersion); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 43deebab57169..779486f3d12f3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -29,7 +29,7 @@ public enum EsqlVersion implements VersionId { * Version prefix that we accept when parsing. If a version string starts with the given prefix, we consider the version string valid. * E.g. "2024.04.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04" and "2024.04foobar". */ - String parsingPrefix() { + public String versionStringNoEmoji() { return this == NIGHTLY ? "nightly" : String.format("%d.%02d", year, month); } @@ -38,7 +38,7 @@ public static EsqlVersion parse(String versionString) { if (Strings.hasText(versionString)) { versionString = versionString.toLowerCase(); for (EsqlVersion version : EsqlVersion.values()) { - if (versionString.startsWith(version.parsingPrefix())) { + if (versionString.startsWith(version.versionStringNoEmoji())) { return version; } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 5b16691bcee77..09ae8d2685caa 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -24,9 +24,11 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.esql.parser.TypedParamValue; +import org.elasticsearch.xpack.esql.version.EsqlVersion; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -44,20 +46,23 @@ public void testParseFields() throws IOException { boolean columnar = randomBoolean(); Locale locale = randomLocale(random()); QueryBuilder filter = randomQueryBuilder(); + EsqlVersion esqlVersion = randomFrom(EsqlVersion.values()); List params = randomParameters(); boolean hasParams = params.isEmpty() == false; StringBuilder paramsString = paramsString(params, hasParams); String json = String.format(Locale.ROOT, """ { + "esql.version": "%s", "query": "%s", "columnar": %s, "locale": "%s", "filter": %s - %s""", query, columnar, locale.toLanguageTag(), filter, paramsString); + %s""", esqlVersion, query, columnar, locale.toLanguageTag(), filter, paramsString); EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + assertEquals(esqlVersion.toString(), request.esqlVersion()); assertEquals(query, request.query()); assertEquals(columnar, request.columnar()); assertEquals(locale.toLanguageTag(), request.locale().toLanguageTag()); @@ -75,6 +80,7 @@ public void testParseFieldsForAsync() throws IOException { boolean columnar = randomBoolean(); Locale locale = randomLocale(random()); QueryBuilder filter = randomQueryBuilder(); + EsqlVersion esqlVersion = randomFrom(EsqlVersion.values()); List params = randomParameters(); boolean hasParams = params.isEmpty() == false; @@ -86,6 +92,7 @@ public void testParseFieldsForAsync() throws IOException { Locale.ROOT, """ { + "esql.version": "%s", "query": "%s", "columnar": %s, "locale": "%s", @@ -94,6 +101,7 @@ public void testParseFieldsForAsync() throws IOException { "wait_for_completion_timeout": "%s", "keep_alive": "%s" %s""", + esqlVersion, query, columnar, locale.toLanguageTag(), @@ -106,6 +114,7 @@ public void testParseFieldsForAsync() throws IOException { EsqlQueryRequest request = parseEsqlQueryRequestAsync(json); + assertEquals(esqlVersion.toString(), request.esqlVersion()); assertEquals(query, request.query()); assertEquals(columnar, request.columnar()); assertEquals(locale.toLanguageTag(), request.locale().toLanguageTag()); @@ -149,10 +158,78 @@ public void testRejectUnknownFields() { }""", "unknown field [asdf]"); } - public void testMissingQueryIsNotValidation() throws IOException { + public void testKnownVersionIsValid() throws IOException { + for (EsqlVersion version : EsqlVersion.values()) { + int suffixLength = randomIntBetween(0, 10); + String validVersionString = version.versionStringNoEmoji() + randomAlphaOfLength(suffixLength); + + String json = String.format(Locale.ROOT, """ + { + "esql.version": "%s", + "query": "ROW x = 1" + } + """, validVersionString); + + EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + assertNull(request.validate()); + + request = parseEsqlQueryRequestAsync(json); + assertNull(request.validate()); + } + } + + private static String invalidVersionString() { + String[] invalidVersionString = new String[1]; + + do { + int length = randomIntBetween(0, 10); + invalidVersionString[0] = randomAlphaOfLength(length); + } while (Arrays.stream(EsqlVersion.values()) + .anyMatch(version -> invalidVersionString[0].startsWith(version.versionStringNoEmoji()))); + + return invalidVersionString[0]; + } + + public void testUnknownVersionIsNotValid() throws IOException { + String invalidVersionString = invalidVersionString(); + + String json = String.format(Locale.ROOT, """ + { + "esql.version": "%s", + "query": "ROW x = 1" + } + """, invalidVersionString); + + EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + assertNotNull(request.validate()); + assertThat(request.validate().getMessage(), containsString("[esql.version] has invalid value [" + invalidVersionString + "]")); + + request = parseEsqlQueryRequestAsync(json); + assertNotNull(request.validate()); + assertThat(request.validate().getMessage(), containsString("[esql.version] has invalid value [" + invalidVersionString + "]")); + } + + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104890") + public void testMissingVersionIsNotValid() throws IOException { + String json = """ + { + "columnar": true, + "query": "row x = 1" + }"""; + EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + assertNotNull(request.validate()); + assertThat(request.validate().getMessage(), containsString("[esql.version] is required")); + + request = parseEsqlQueryRequestAsync(json); + assertNotNull(request.validate()); + assertThat(request.validate().getMessage(), containsString("[esql.version] is required")); + } + + public void testMissingQueryIsNotValid() throws IOException { String json = """ { - "columnar": true + "columnar": true, + "esql.version": "nightly" }"""; EsqlQueryRequest request = parseEsqlQueryRequestSync(json); assertNotNull(request.validate()); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index c24eb31369bcc..4d64fd45a3067 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -26,20 +26,20 @@ public void testVersionId() { assertThat(PARTY_POPPER.id(), equalTo(202404)); } - public void testParsingPrefix() { + public void testVersionStringNoEmoji() { for (EsqlVersion version : EsqlVersion.values()) { String[] versionSegments = version.toString().split("\\."); String[] parsingPrefixSegments = Arrays.copyOf(versionSegments, versionSegments.length - 1); String expectedParsingPrefix = String.join(".", parsingPrefixSegments); - assertThat(version.parsingPrefix(), equalTo(expectedParsingPrefix)); + assertThat(version.versionStringNoEmoji(), equalTo(expectedParsingPrefix)); } } public void testParsing() { for (EsqlVersion version : EsqlVersion.values()) { int suffixLength = randomIntBetween(0, 10); - String validVersionString = version.parsingPrefix() + randomUnicodeOfLength(suffixLength); + String validVersionString = version.versionStringNoEmoji() + randomUnicodeOfLength(suffixLength); assertThat(EsqlVersion.parse(validVersionString), is(version)); } From 887759ae0070b15f5f485c27437e26a762f20515 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 27 Mar 2024 16:37:36 +0100 Subject: [PATCH 04/27] Refactor EsqlVersion.toString --- .../java/org/elasticsearch/xpack/esql/version/EsqlVersion.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 779486f3d12f3..bee08b80e4518 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -48,7 +48,7 @@ public static EsqlVersion parse(String versionString) { @Override public String toString() { - return this == NIGHTLY ? "nightly." + emoji : String.format("%d.%02d.%s", year, month, emoji); + return versionStringNoEmoji() + "." + emoji; } @Override From da6c0e183eaa509d896c48a293c131df808c5298 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 27 Mar 2024 16:39:23 +0100 Subject: [PATCH 05/27] Make checkstyle happy --- .../xpack/esql/version/EsqlVersionTests.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index 4d64fd45a3067..f6abd775aef54 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -11,19 +11,18 @@ import java.util.Arrays; -import static org.elasticsearch.xpack.esql.version.EsqlVersion.*; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; public class EsqlVersionTests extends ESTestCase { public void testVersionString() { - assertThat(NIGHTLY.toString(), equalTo("nightly.\uD83D\uDE34")); - assertThat(PARTY_POPPER.toString(), equalTo("2024.04.\uD83C\uDF89")); + assertThat(EsqlVersion.NIGHTLY.toString(), equalTo("nightly.\uD83D\uDE34")); + assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.\uD83C\uDF89")); } public void testVersionId() { - assertThat(NIGHTLY.id(), equalTo(Integer.MAX_VALUE)); - assertThat(PARTY_POPPER.id(), equalTo(202404)); + assertThat(EsqlVersion.NIGHTLY.id(), equalTo(Integer.MAX_VALUE)); + assertThat(EsqlVersion.PARTY_POPPER.id(), equalTo(202404)); } public void testVersionStringNoEmoji() { From 8e6de6335b9972988dd88d3958c7fa004f857c2a Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 27 Mar 2024 16:53:36 +0100 Subject: [PATCH 06/27] Avoid forbidden APIs --- .../org/elasticsearch/xpack/esql/version/EsqlVersion.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index bee08b80e4518..e0003990302e3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -30,13 +30,13 @@ public enum EsqlVersion implements VersionId { * E.g. "2024.04.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04" and "2024.04foobar". */ public String versionStringNoEmoji() { - return this == NIGHTLY ? "nightly" : String.format("%d.%02d", year, month); + return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d", year, month); } public static EsqlVersion parse(String versionString) { EsqlVersion parsed = null; if (Strings.hasText(versionString)) { - versionString = versionString.toLowerCase(); + versionString = Strings.toLowercaseAscii(versionString); for (EsqlVersion version : EsqlVersion.values()) { if (versionString.startsWith(version.versionStringNoEmoji())) { return version; From 98b816fc98da1decbfc7f8ee202737b97eb6c4f7 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Thu, 28 Mar 2024 14:00:05 +0100 Subject: [PATCH 07/27] Use proper emojis --- .../org/elasticsearch/xpack/esql/version/EsqlVersion.java | 4 ++-- .../elasticsearch/xpack/esql/version/EsqlVersionTests.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index e0003990302e3..fe0555a7c5bed 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -12,8 +12,8 @@ public enum EsqlVersion implements VersionId { // Breaking changes go here until the next version is released. - NIGHTLY(Integer.MAX_VALUE, Integer.MAX_VALUE, "\uD83D\uDE34"), - PARTY_POPPER(2024, 4, "\uD83C\uDF89"); + NIGHTLY(Integer.MAX_VALUE, Integer.MAX_VALUE, "😴"), + PARTY_POPPER(2024, 4, "🎉"); EsqlVersion(int year, int month, String emoji) { this.year = year; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index f6abd775aef54..6ae99da3d21a6 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -16,8 +16,8 @@ public class EsqlVersionTests extends ESTestCase { public void testVersionString() { - assertThat(EsqlVersion.NIGHTLY.toString(), equalTo("nightly.\uD83D\uDE34")); - assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.\uD83C\uDF89")); + assertThat(EsqlVersion.NIGHTLY.toString(), equalTo("nightly.😴")); + assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.🎉")); } public void testVersionId() { From d5bef793217adf212b31a85918bc7954841fe57a Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Thu, 28 Mar 2024 14:57:39 +0100 Subject: [PATCH 08/27] Add disambiguation counter to version --- .../xpack/esql/version/EsqlVersion.java | 14 +++++++++++--- .../xpack/esql/version/EsqlVersionTests.java | 4 ++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index fe0555a7c5bed..a243593192b97 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -16,21 +16,29 @@ public enum EsqlVersion implements VersionId { PARTY_POPPER(2024, 4, "🎉"); EsqlVersion(int year, int month, String emoji) { + this(year, month, 1, emoji); + } + + EsqlVersion(int year, int month, int numberThisMonth, String emoji) { + assert 0 < numberThisMonth && numberThisMonth < 100; this.year = year; this.month = month; + this.numberThisMonth = numberThisMonth; this.emoji = emoji; } private int year; private int month; + // In case we really have to release more than one version in a given month, disambiguates between versions in the month. + private int numberThisMonth; private String emoji; /** * Version prefix that we accept when parsing. If a version string starts with the given prefix, we consider the version string valid. - * E.g. "2024.04.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04" and "2024.04foobar". + * E.g. "2024.04.01.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04.01". */ public String versionStringNoEmoji() { - return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d", year, month); + return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d.%02d", year, month, numberThisMonth); } public static EsqlVersion parse(String versionString) { @@ -53,6 +61,6 @@ public String toString() { @Override public int id() { - return this == NIGHTLY ? Integer.MAX_VALUE : (100 * year + month); + return this == NIGHTLY ? Integer.MAX_VALUE : (10000 * year + 100 * month + numberThisMonth); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index 6ae99da3d21a6..8c3f0cee8c97d 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -17,12 +17,12 @@ public class EsqlVersionTests extends ESTestCase { public void testVersionString() { assertThat(EsqlVersion.NIGHTLY.toString(), equalTo("nightly.😴")); - assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.🎉")); + assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.01.🎉")); } public void testVersionId() { assertThat(EsqlVersion.NIGHTLY.id(), equalTo(Integer.MAX_VALUE)); - assertThat(EsqlVersion.PARTY_POPPER.id(), equalTo(202404)); + assertThat(EsqlVersion.PARTY_POPPER.id(), equalTo(20240401)); } public void testVersionStringNoEmoji() { From f0a2aae186f6b434a9f2898568966d6a6d0aa058 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Thu, 28 Mar 2024 15:37:24 +0100 Subject: [PATCH 09/27] Make parsing stricter --- .../xpack/esql/version/EsqlVersion.java | 42 +++++++++++++------ .../esql/action/EsqlQueryRequestTests.java | 19 ++------- .../xpack/esql/version/EsqlVersionTests.java | 21 ++++++++-- 3 files changed, 49 insertions(+), 33 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index a243593192b97..fb78c236ecdfa 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -10,11 +10,32 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.VersionId; +import java.util.LinkedHashMap; +import java.util.Map; + public enum EsqlVersion implements VersionId { - // Breaking changes go here until the next version is released. + /** + * Breaking changes go here until the next version is released. + */ NIGHTLY(Integer.MAX_VALUE, Integer.MAX_VALUE, "😴"), PARTY_POPPER(2024, 4, "🎉"); + static final Map VERSION_MAP_WITH_AND_WITHOUT_EMOJI = versionMapWithAndWithoutEmoji(); + + private static Map versionMapWithAndWithoutEmoji() { + Map stringToVersion = new LinkedHashMap<>(EsqlVersion.values().length * 2); + + for (EsqlVersion version : EsqlVersion.values()) { + EsqlVersion existingVersionForKey = null; + existingVersionForKey = stringToVersion.put(version.versionStringWithoutEmoji(), version); + assert existingVersionForKey == null; + existingVersionForKey = stringToVersion.put(version.toString(), version); + assert existingVersionForKey == null; + } + + return stringToVersion; + } + EsqlVersion(int year, int month, String emoji) { this(year, month, 1, emoji); } @@ -37,26 +58,21 @@ public enum EsqlVersion implements VersionId { * Version prefix that we accept when parsing. If a version string starts with the given prefix, we consider the version string valid. * E.g. "2024.04.01.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04.01". */ - public String versionStringNoEmoji() { + public String versionStringWithoutEmoji() { return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d.%02d", year, month, numberThisMonth); } + public String emoji() { + return emoji; + } + public static EsqlVersion parse(String versionString) { - EsqlVersion parsed = null; - if (Strings.hasText(versionString)) { - versionString = Strings.toLowercaseAscii(versionString); - for (EsqlVersion version : EsqlVersion.values()) { - if (versionString.startsWith(version.versionStringNoEmoji())) { - return version; - } - } - } - return parsed; + return VERSION_MAP_WITH_AND_WITHOUT_EMOJI.get(versionString); } @Override public String toString() { - return versionStringNoEmoji() + "." + emoji; + return versionStringWithoutEmoji() + "." + emoji; } @Override diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 09ae8d2685caa..dd06ad2a9e804 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -25,10 +25,10 @@ import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.esql.parser.TypedParamValue; import org.elasticsearch.xpack.esql.version.EsqlVersion; +import org.elasticsearch.xpack.esql.version.EsqlVersionTests; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -160,8 +160,7 @@ public void testRejectUnknownFields() { public void testKnownVersionIsValid() throws IOException { for (EsqlVersion version : EsqlVersion.values()) { - int suffixLength = randomIntBetween(0, 10); - String validVersionString = version.versionStringNoEmoji() + randomAlphaOfLength(suffixLength); + String validVersionString = randomBoolean() ? version.versionStringWithoutEmoji() : version.toString(); String json = String.format(Locale.ROOT, """ { @@ -178,20 +177,8 @@ public void testKnownVersionIsValid() throws IOException { } } - private static String invalidVersionString() { - String[] invalidVersionString = new String[1]; - - do { - int length = randomIntBetween(0, 10); - invalidVersionString[0] = randomAlphaOfLength(length); - } while (Arrays.stream(EsqlVersion.values()) - .anyMatch(version -> invalidVersionString[0].startsWith(version.versionStringNoEmoji()))); - - return invalidVersionString[0]; - } - public void testUnknownVersionIsNotValid() throws IOException { - String invalidVersionString = invalidVersionString(); + String invalidVersionString = EsqlVersionTests.invalidVersionString(); String json = String.format(Locale.ROOT, """ { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index 8c3f0cee8c97d..8ab3448706110 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -31,16 +31,29 @@ public void testVersionStringNoEmoji() { String[] parsingPrefixSegments = Arrays.copyOf(versionSegments, versionSegments.length - 1); String expectedParsingPrefix = String.join(".", parsingPrefixSegments); - assertThat(version.versionStringNoEmoji(), equalTo(expectedParsingPrefix)); + assertThat(version.versionStringWithoutEmoji(), equalTo(expectedParsingPrefix)); } } public void testParsing() { for (EsqlVersion version : EsqlVersion.values()) { - int suffixLength = randomIntBetween(0, 10); - String validVersionString = version.versionStringNoEmoji() + randomUnicodeOfLength(suffixLength); + String versionStringWithoutEmoji = version.versionStringWithoutEmoji(); - assertThat(EsqlVersion.parse(validVersionString), is(version)); + assertThat(EsqlVersion.parse(versionStringWithoutEmoji), is(version)); + assertThat(EsqlVersion.parse(versionStringWithoutEmoji + "." + version.emoji()), is(version)); } + + assertNull(EsqlVersion.parse(invalidVersionString())); + } + + public static String invalidVersionString() { + String[] invalidVersionString = new String[1]; + + do { + int length = randomIntBetween(0, 10); + invalidVersionString[0] = randomAlphaOfLength(length); + } while (EsqlVersion.VERSION_MAP_WITH_AND_WITHOUT_EMOJI.containsKey(invalidVersionString[0])); + + return invalidVersionString[0]; } } From 9e97e43716fb74c3678ea171635f6bc78c3fceca Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Thu, 28 Mar 2024 15:43:25 +0100 Subject: [PATCH 10/27] Rename esql.version -> version --- .../xpack/esql/action/RequestXContent.java | 2 +- .../esql/action/EsqlQueryRequestTests.java | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index a90fc4d5240d4..1cb7a671bd4be 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -47,7 +47,7 @@ final class RequestXContent { } // TODO: Maybe esql_version would be better? This looks like there's a JSON attribute `esql` with subattribute `version`. - static final ParseField ESQL_VERSION_FIELD = new ParseField("esql.version"); + static final ParseField ESQL_VERSION_FIELD = new ParseField("version"); static final ParseField QUERY_FIELD = new ParseField("query"); private static final ParseField COLUMNAR_FIELD = new ParseField("columnar"); private static final ParseField FILTER_FIELD = new ParseField("filter"); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index dd06ad2a9e804..4b55727b97050 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -53,7 +53,7 @@ public void testParseFields() throws IOException { StringBuilder paramsString = paramsString(params, hasParams); String json = String.format(Locale.ROOT, """ { - "esql.version": "%s", + "version": "%s", "query": "%s", "columnar": %s, "locale": "%s", @@ -92,7 +92,7 @@ public void testParseFieldsForAsync() throws IOException { Locale.ROOT, """ { - "esql.version": "%s", + "version": "%s", "query": "%s", "columnar": %s, "locale": "%s", @@ -164,7 +164,7 @@ public void testKnownVersionIsValid() throws IOException { String json = String.format(Locale.ROOT, """ { - "esql.version": "%s", + "version": "%s", "query": "ROW x = 1" } """, validVersionString); @@ -182,18 +182,18 @@ public void testUnknownVersionIsNotValid() throws IOException { String json = String.format(Locale.ROOT, """ { - "esql.version": "%s", + "version": "%s", "query": "ROW x = 1" } """, invalidVersionString); EsqlQueryRequest request = parseEsqlQueryRequestSync(json); assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[esql.version] has invalid value [" + invalidVersionString + "]")); + assertThat(request.validate().getMessage(), containsString("[version] has invalid value [" + invalidVersionString + "]")); request = parseEsqlQueryRequestAsync(json); assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[esql.version] has invalid value [" + invalidVersionString + "]")); + assertThat(request.validate().getMessage(), containsString("[version] has invalid value [" + invalidVersionString + "]")); } @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104890") @@ -205,18 +205,18 @@ public void testMissingVersionIsNotValid() throws IOException { }"""; EsqlQueryRequest request = parseEsqlQueryRequestSync(json); assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[esql.version] is required")); + assertThat(request.validate().getMessage(), containsString("[version] is required")); request = parseEsqlQueryRequestAsync(json); assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[esql.version] is required")); + assertThat(request.validate().getMessage(), containsString("[version] is required")); } public void testMissingQueryIsNotValid() throws IOException { String json = """ { "columnar": true, - "esql.version": "nightly" + "version": "nightly" }"""; EsqlQueryRequest request = parseEsqlQueryRequestSync(json); assertNotNull(request.validate()); From 11ba353a75fb413515c5637eea7b86449cb9b12b Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Thu, 28 Mar 2024 15:46:33 +0100 Subject: [PATCH 11/27] Update docs/changelog/106824.yaml --- docs/changelog/106824.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/106824.yaml diff --git a/docs/changelog/106824.yaml b/docs/changelog/106824.yaml new file mode 100644 index 0000000000000..0a2001df5039a --- /dev/null +++ b/docs/changelog/106824.yaml @@ -0,0 +1,5 @@ +pr: 106824 +summary: "ESQL: Introduce language versioning to REST API" +area: ES|QL +type: enhancement +issues: [] From a29fc710b69b53b47c522f2bb69c3b5d5642d6f4 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Thu, 28 Mar 2024 16:09:10 +0100 Subject: [PATCH 12/27] Remove obsolete TODO --- .../org/elasticsearch/xpack/esql/action/RequestXContent.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index 1cb7a671bd4be..63d230b24c690 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -45,8 +45,7 @@ final class RequestXContent { PARAM_PARSER.declareField(constructorArg(), (p, c) -> parseFieldsValue(p), VALUE, ObjectParser.ValueType.VALUE); PARAM_PARSER.declareString(constructorArg(), TYPE); } - - // TODO: Maybe esql_version would be better? This looks like there's a JSON attribute `esql` with subattribute `version`. + static final ParseField ESQL_VERSION_FIELD = new ParseField("version"); static final ParseField QUERY_FIELD = new ParseField("query"); private static final ParseField COLUMNAR_FIELD = new ParseField("columnar"); From 85576a716f849f20f7b635627b3a849e795b5402 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Thu, 28 Mar 2024 16:18:52 +0100 Subject: [PATCH 13/27] Spotless --- .../org/elasticsearch/xpack/esql/action/RequestXContent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index 63d230b24c690..ef82f666ce904 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -45,7 +45,7 @@ final class RequestXContent { PARAM_PARSER.declareField(constructorArg(), (p, c) -> parseFieldsValue(p), VALUE, ObjectParser.ValueType.VALUE); PARAM_PARSER.declareString(constructorArg(), TYPE); } - + static final ParseField ESQL_VERSION_FIELD = new ParseField("version"); static final ParseField QUERY_FIELD = new ParseField("query"); private static final ParseField COLUMNAR_FIELD = new ParseField("columnar"); From 5920994909684c33a643492535eee3c710cf1048 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Tue, 2 Apr 2024 15:06:09 +0200 Subject: [PATCH 14/27] Refactor month and numberThisMonth --- .../xpack/esql/version/EsqlVersion.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index fb78c236ecdfa..4ed0e80a7206d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -17,7 +17,7 @@ public enum EsqlVersion implements VersionId { /** * Breaking changes go here until the next version is released. */ - NIGHTLY(Integer.MAX_VALUE, Integer.MAX_VALUE, "😴"), + NIGHTLY(Integer.MAX_VALUE, Byte.MAX_VALUE, "😴"), PARTY_POPPER(2024, 4, "🎉"); static final Map VERSION_MAP_WITH_AND_WITHOUT_EMOJI = versionMapWithAndWithoutEmoji(); @@ -40,18 +40,18 @@ private static Map versionMapWithAndWithoutEmoji() { this(year, month, 1, emoji); } - EsqlVersion(int year, int month, int numberThisMonth, String emoji) { - assert 0 < numberThisMonth && numberThisMonth < 100; + EsqlVersion(int year, int month, int revision, String emoji) { + assert 0 < revision && revision < 100; + assert 0 < month && month < 13; this.year = year; - this.month = month; - this.numberThisMonth = numberThisMonth; + this.month = (byte) month; + this.revision = (byte) revision; this.emoji = emoji; } private int year; - private int month; - // In case we really have to release more than one version in a given month, disambiguates between versions in the month. - private int numberThisMonth; + private byte month; + private byte revision; private String emoji; /** @@ -59,7 +59,7 @@ private static Map versionMapWithAndWithoutEmoji() { * E.g. "2024.04.01.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04.01". */ public String versionStringWithoutEmoji() { - return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d.%02d", year, month, numberThisMonth); + return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d.%02d", year, month, revision); } public String emoji() { @@ -77,6 +77,6 @@ public String toString() { @Override public int id() { - return this == NIGHTLY ? Integer.MAX_VALUE : (10000 * year + 100 * month + numberThisMonth); + return this == NIGHTLY ? Integer.MAX_VALUE : (10000 * year + 100 * month + revision); } } From a633632e24c961574e32e7ff434c34dc7ca3cb72 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Tue, 2 Apr 2024 16:02:37 +0200 Subject: [PATCH 15/27] Add validation tests for non-snapshot builds --- .../xpack/esql/action/EsqlQueryRequest.java | 10 ++++- .../xpack/esql/version/EsqlVersion.java | 2 +- .../esql/action/EsqlQueryRequestTests.java | 39 +++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index 5199e9e78ae6a..5da85bc3d001b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -47,6 +47,7 @@ public class EsqlQueryRequest extends org.elasticsearch.xpack.core.esql.action.E private TimeValue waitForCompletionTimeout = DEFAULT_WAIT_FOR_COMPLETION; private TimeValue keepAlive = DEFAULT_KEEP_ALIVE; private boolean keepOnCompletion; + private boolean onSnapshotBuild = Build.current().isSnapshot(); static EsqlQueryRequest syncEsqlQueryRequest() { return new EsqlQueryRequest(false); @@ -78,7 +79,7 @@ public ActionRequestValidationException validate() { "[" + RequestXContent.ESQL_VERSION_FIELD + "] has invalid value [" + esqlVersion + "]", validationException ); - } else if (version == EsqlVersion.NIGHTLY && Build.current().isSnapshot() == false) { + } else if (version == EsqlVersion.NIGHTLY && onSnapshotBuild == false) { validationException = addValidationError( "[" + RequestXContent.ESQL_VERSION_FIELD + "] with value [" + esqlVersion + "] only allowed in snapshot builds", validationException @@ -88,7 +89,7 @@ public ActionRequestValidationException validate() { if (Strings.hasText(query) == false) { validationException = addValidationError("[" + RequestXContent.QUERY_FIELD + "] is required", validationException); } - if (Build.current().isSnapshot() == false && pragmas.isEmpty() == false) { + if (onSnapshotBuild == false && pragmas.isEmpty() == false) { validationException = addValidationError( "[" + RequestXContent.PRAGMA_FIELD + "] only allowed in snapshot builds", validationException @@ -206,4 +207,9 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId, // Pass the query as the description return new CancellableTask(id, type, action, query, parentTaskId, headers); } + + // Setter for tests + void onSnapshotBuild(boolean onSnapshotBuild) { + this.onSnapshotBuild = onSnapshotBuild; + } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 4ed0e80a7206d..53ea556ecd458 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -17,7 +17,7 @@ public enum EsqlVersion implements VersionId { /** * Breaking changes go here until the next version is released. */ - NIGHTLY(Integer.MAX_VALUE, Byte.MAX_VALUE, "😴"), + NIGHTLY(Integer.MAX_VALUE, 12, 99, "😴"), PARTY_POPPER(2024, 4, "🎉"); static final Map VERSION_MAP_WITH_AND_WITHOUT_EMOJI = versionMapWithAndWithoutEmoji(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 4b55727b97050..153061ad2a8d3 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -196,6 +196,27 @@ public void testUnknownVersionIsNotValid() throws IOException { assertThat(request.validate().getMessage(), containsString("[version] has invalid value [" + invalidVersionString + "]")); } + public void testNightlyVersionIsOnlyValidOnSnapshot() throws IOException { + String esqlVersion = randomBoolean() ? "nightly" : "nightly.😴"; + String json = String.format(Locale.ROOT, """ + { + "version": "%s", + "query": "ROW x = 1" + } + """, esqlVersion); + + EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + request.onSnapshotBuild(true); + assertNull(request.validate()); + + request.onSnapshotBuild(false); + assertNotNull(request.validate()); + assertThat( + request.validate().getMessage(), + containsString("[version] with value [" + esqlVersion + "] only allowed in snapshot builds") + ); + } + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104890") public void testMissingVersionIsNotValid() throws IOException { String json = """ @@ -227,6 +248,24 @@ public void testMissingQueryIsNotValid() throws IOException { assertThat(request.validate().getMessage(), containsString("[query] is required")); } + public void testPragmasOnlyValidOnSnapshot() throws IOException { + String json = String.format(Locale.ROOT, """ + { + "version": "2024.04.01", + "query": "ROW x = 1", + "pragma": {"foo": "bar"} + } + """); + + EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + request.onSnapshotBuild(true); + assertNull(request.validate()); + + request.onSnapshotBuild(false); + assertNotNull(request.validate()); + assertThat(request.validate().getMessage(), containsString("[pragma] only allowed in snapshot builds")); + } + public void testTask() throws IOException { String query = randomAlphaOfLength(10); int id = randomInt(); From be3f255acc8d53b022b4b0352cecb35d134fcaf8 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Tue, 2 Apr 2024 17:18:31 +0200 Subject: [PATCH 16/27] Use randomization to test both sync/async --- .../esql/action/EsqlQueryRequestTests.java | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 153061ad2a8d3..be7d9a94bcffd 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -169,7 +169,7 @@ public void testKnownVersionIsValid() throws IOException { } """, validVersionString); - EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNull(request.validate()); request = parseEsqlQueryRequestAsync(json); @@ -187,11 +187,7 @@ public void testUnknownVersionIsNotValid() throws IOException { } """, invalidVersionString); - EsqlQueryRequest request = parseEsqlQueryRequestSync(json); - assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[version] has invalid value [" + invalidVersionString + "]")); - - request = parseEsqlQueryRequestAsync(json); + EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNotNull(request.validate()); assertThat(request.validate().getMessage(), containsString("[version] has invalid value [" + invalidVersionString + "]")); } @@ -205,7 +201,7 @@ public void testNightlyVersionIsOnlyValidOnSnapshot() throws IOException { } """, esqlVersion); - EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); request.onSnapshotBuild(true); assertNull(request.validate()); @@ -224,11 +220,7 @@ public void testMissingVersionIsNotValid() throws IOException { "columnar": true, "query": "row x = 1" }"""; - EsqlQueryRequest request = parseEsqlQueryRequestSync(json); - assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[version] is required")); - - request = parseEsqlQueryRequestAsync(json); + EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNotNull(request.validate()); assertThat(request.validate().getMessage(), containsString("[version] is required")); } @@ -239,11 +231,7 @@ public void testMissingQueryIsNotValid() throws IOException { "columnar": true, "version": "nightly" }"""; - EsqlQueryRequest request = parseEsqlQueryRequestSync(json); - assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[query] is required")); - - request = parseEsqlQueryRequestAsync(json); + EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNotNull(request.validate()); assertThat(request.validate().getMessage(), containsString("[query] is required")); } @@ -257,7 +245,7 @@ public void testPragmasOnlyValidOnSnapshot() throws IOException { } """); - EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); request.onSnapshotBuild(true); assertNull(request.validate()); @@ -363,6 +351,10 @@ private static void assertParserErrorMessage(String json, String message) { assertThat(e.getMessage(), containsString(message)); } + static EsqlQueryRequest parseEsqlQueryRequest(String json, boolean sync) throws IOException { + return sync ? parseEsqlQueryRequestSync(json) : parseEsqlQueryRequestAsync(json); + } + static EsqlQueryRequest parseEsqlQueryRequestSync(String json) throws IOException { var request = parseEsqlQueryRequest(json, RequestXContent::parseSync); assertFalse(request.async()); From 5b15c30caa12d94d12d43d45b4121b89fdbf4e45 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Tue, 2 Apr 2024 17:22:41 +0200 Subject: [PATCH 17/27] Add empty version string to test --- .../xpack/esql/action/EsqlQueryRequestTests.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index be7d9a94bcffd..be9967dd656ff 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -215,11 +215,14 @@ public void testNightlyVersionIsOnlyValidOnSnapshot() throws IOException { @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104890") public void testMissingVersionIsNotValid() throws IOException { - String json = """ + String missingVersion = randomBoolean() ? "" : ", \"version\": \"\""; + String json = String.format(""" { "columnar": true, "query": "row x = 1" - }"""; + %s + }""", missingVersion); + EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNotNull(request.validate()); assertThat(request.validate().getMessage(), containsString("[version] is required")); @@ -237,13 +240,13 @@ public void testMissingQueryIsNotValid() throws IOException { } public void testPragmasOnlyValidOnSnapshot() throws IOException { - String json = String.format(Locale.ROOT, """ + String json = """ { "version": "2024.04.01", "query": "ROW x = 1", "pragma": {"foo": "bar"} } - """); + """; EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); request.onSnapshotBuild(true); From c6f06dbe27bb8333e3fd548c16785eab403227c7 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 11:43:38 +0200 Subject: [PATCH 18/27] Address feedback --- .../core/esql/action/EsqlQueryRequest.java | 1 + .../xpack/esql/version/EsqlVersion.java | 38 +++++++++++++------ .../esql/action/EsqlQueryRequestTests.java | 4 +- .../xpack/esql/version/EsqlVersionTests.java | 22 ++++++++++- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java index c7a5b583fed42..dcd89c200db26 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java @@ -21,6 +21,7 @@ protected EsqlQueryRequest(StreamInput in) throws IOException { super(in); } + // Use the unparsed version String, so we don't have to serialize a version object. public abstract String esqlVersion(); public abstract String query(); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 53ea556ecd458..3745e13e0560b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -26,16 +26,20 @@ private static Map versionMapWithAndWithoutEmoji() { Map stringToVersion = new LinkedHashMap<>(EsqlVersion.values().length * 2); for (EsqlVersion version : EsqlVersion.values()) { - EsqlVersion existingVersionForKey = null; - existingVersionForKey = stringToVersion.put(version.versionStringWithoutEmoji(), version); - assert existingVersionForKey == null; - existingVersionForKey = stringToVersion.put(version.toString(), version); - assert existingVersionForKey == null; + putVersionAssertNoDups(stringToVersion, version.versionStringWithoutEmoji(), version); + putVersionAssertNoDups(stringToVersion, version.toString(), version); } return stringToVersion; } + private static void putVersionAssertNoDups(Map stringToVersion, String versionString, EsqlVersion version) { + EsqlVersion existingVersionForKey = stringToVersion.put(versionString, version); + if (existingVersionForKey != null) { + throw new AssertionError("Duplicate esql version with version string [" + versionString + "]"); + } + } + EsqlVersion(int year, int month, String emoji) { this(year, month, 1, emoji); } @@ -54,22 +58,34 @@ private static Map versionMapWithAndWithoutEmoji() { private byte revision; private String emoji; - /** - * Version prefix that we accept when parsing. If a version string starts with the given prefix, we consider the version string valid. - * E.g. "2024.04.01.🎉" will be interpreted as {@link EsqlVersion#PARTY_POPPER}, but so will "2024.04.01". - */ - public String versionStringWithoutEmoji() { - return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d.%02d", year, month, revision); + public int year() { + return year; + } + + public byte month() { + return month; + } + + public byte revision() { + return revision; } public String emoji() { return emoji; } + /** + * Accepts a version string with the emoji suffix or without it. + * E.g. both "2024.04.01.🎉" and "2024.04.01" will be interpreted as {@link EsqlVersion#PARTY_POPPER}. + */ public static EsqlVersion parse(String versionString) { return VERSION_MAP_WITH_AND_WITHOUT_EMOJI.get(versionString); } + public String versionStringWithoutEmoji() { + return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d.%02d", year, month, revision); + } + @Override public String toString() { return versionStringWithoutEmoji() + "." + emoji; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index be9967dd656ff..385e7243eb505 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -178,7 +178,7 @@ public void testKnownVersionIsValid() throws IOException { } public void testUnknownVersionIsNotValid() throws IOException { - String invalidVersionString = EsqlVersionTests.invalidVersionString(); + String invalidVersionString = EsqlVersionTests.randomInvalidVersionString(); String json = String.format(Locale.ROOT, """ { @@ -216,7 +216,7 @@ public void testNightlyVersionIsOnlyValidOnSnapshot() throws IOException { @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104890") public void testMissingVersionIsNotValid() throws IOException { String missingVersion = randomBoolean() ? "" : ", \"version\": \"\""; - String json = String.format(""" + String json = String.format(Locale.ROOT, """ { "columnar": true, "query": "row x = 1" diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index 8ab3448706110..e6c344edcf1bb 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -10,6 +10,8 @@ import org.elasticsearch.test.ESTestCase; import java.util.Arrays; +import java.util.Comparator; +import java.util.List; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -23,6 +25,22 @@ public void testVersionString() { public void testVersionId() { assertThat(EsqlVersion.NIGHTLY.id(), equalTo(Integer.MAX_VALUE)); assertThat(EsqlVersion.PARTY_POPPER.id(), equalTo(20240401)); + + for (EsqlVersion version : EsqlVersion.values()) { + assertTrue(EsqlVersion.NIGHTLY.onOrAfter(version)); + if (version != EsqlVersion.NIGHTLY) { + assertTrue(version.before(EsqlVersion.NIGHTLY)); + } else { + assertTrue(version.onOrAfter(EsqlVersion.NIGHTLY)); + } + } + + List versionsSortedAsc = Arrays.stream(EsqlVersion.values()) + .sorted(Comparator.comparing(EsqlVersion::year).thenComparing(EsqlVersion::month).thenComparing(EsqlVersion::revision)) + .toList(); + for (int i = 0; i < versionsSortedAsc.size() - 1; i++) { + assertTrue(versionsSortedAsc.get(i).before(versionsSortedAsc.get(i + 1))); + } } public void testVersionStringNoEmoji() { @@ -43,10 +61,10 @@ public void testParsing() { assertThat(EsqlVersion.parse(versionStringWithoutEmoji + "." + version.emoji()), is(version)); } - assertNull(EsqlVersion.parse(invalidVersionString())); + assertNull(EsqlVersion.parse(randomInvalidVersionString())); } - public static String invalidVersionString() { + public static String randomInvalidVersionString() { String[] invalidVersionString = new String[1]; do { From eb795fbbce0e46bbe9edaee1b0cf9ab941dbcf90 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 12:00:53 +0200 Subject: [PATCH 19/27] Improve assertions --- .../elasticsearch/xpack/esql/version/EsqlVersion.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 3745e13e0560b..7298cc59b8298 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -45,8 +45,15 @@ private static void putVersionAssertNoDups(Map stringToVers } EsqlVersion(int year, int month, int revision, String emoji) { - assert 0 < revision && revision < 100; - assert 0 < month && month < 13; + if ((1 <= revision && revision <= 99) == false) { + throw new AssertionError("Version revision number must be between 1 and 99 but was [" + revision + "]"); + } + if ((1 <= month && month <= 12) == false) { + throw new AssertionError("Version month must be between 1 and 12 but was [" + month + "]"); + } + if ((emoji.codePointCount(0, emoji.length()) == 1) == false) { + throw new AssertionError("Version emoji must be a single unicode character but was [" + emoji + "]"); + } this.year = year; this.month = (byte) month; this.revision = (byte) revision; From 8a9c0f8148ee0aca468b5abde6ec2c9ba7f17ce5 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 12:05:47 +0200 Subject: [PATCH 20/27] Rename nightly -> snapshot --- .../xpack/esql/action/EsqlQueryRequest.java | 2 +- .../xpack/esql/version/EsqlVersion.java | 6 +++--- .../xpack/esql/action/EsqlQueryRequestTests.java | 6 +++--- .../xpack/esql/version/EsqlVersionTests.java | 12 ++++++------ 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index 5da85bc3d001b..b904f0b83a49d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -79,7 +79,7 @@ public ActionRequestValidationException validate() { "[" + RequestXContent.ESQL_VERSION_FIELD + "] has invalid value [" + esqlVersion + "]", validationException ); - } else if (version == EsqlVersion.NIGHTLY && onSnapshotBuild == false) { + } else if (version == EsqlVersion.SNAPSHOT && onSnapshotBuild == false) { validationException = addValidationError( "[" + RequestXContent.ESQL_VERSION_FIELD + "] with value [" + esqlVersion + "] only allowed in snapshot builds", validationException diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 7298cc59b8298..bb2483e05936b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -17,7 +17,7 @@ public enum EsqlVersion implements VersionId { /** * Breaking changes go here until the next version is released. */ - NIGHTLY(Integer.MAX_VALUE, 12, 99, "😴"), + SNAPSHOT(Integer.MAX_VALUE, 12, 99, "📷"), PARTY_POPPER(2024, 4, "🎉"); static final Map VERSION_MAP_WITH_AND_WITHOUT_EMOJI = versionMapWithAndWithoutEmoji(); @@ -90,7 +90,7 @@ public static EsqlVersion parse(String versionString) { } public String versionStringWithoutEmoji() { - return this == NIGHTLY ? "nightly" : Strings.format("%d.%02d.%02d", year, month, revision); + return this == SNAPSHOT ? "snapshot" : Strings.format("%d.%02d.%02d", year, month, revision); } @Override @@ -100,6 +100,6 @@ public String toString() { @Override public int id() { - return this == NIGHTLY ? Integer.MAX_VALUE : (10000 * year + 100 * month + revision); + return this == SNAPSHOT ? Integer.MAX_VALUE : (10000 * year + 100 * month + revision); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 385e7243eb505..8c0451c94e323 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -192,8 +192,8 @@ public void testUnknownVersionIsNotValid() throws IOException { assertThat(request.validate().getMessage(), containsString("[version] has invalid value [" + invalidVersionString + "]")); } - public void testNightlyVersionIsOnlyValidOnSnapshot() throws IOException { - String esqlVersion = randomBoolean() ? "nightly" : "nightly.😴"; + public void testSnapshotVersionIsOnlyValidOnSnapshot() throws IOException { + String esqlVersion = randomBoolean() ? "snapshot" : "snapshot.📷"; String json = String.format(Locale.ROOT, """ { "version": "%s", @@ -232,7 +232,7 @@ public void testMissingQueryIsNotValid() throws IOException { String json = """ { "columnar": true, - "version": "nightly" + "version": "snapshot" }"""; EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNotNull(request.validate()); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index e6c344edcf1bb..ea55a33e7e0ec 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -18,20 +18,20 @@ public class EsqlVersionTests extends ESTestCase { public void testVersionString() { - assertThat(EsqlVersion.NIGHTLY.toString(), equalTo("nightly.😴")); + assertThat(EsqlVersion.SNAPSHOT.toString(), equalTo("snapshot.📷")); assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.01.🎉")); } public void testVersionId() { - assertThat(EsqlVersion.NIGHTLY.id(), equalTo(Integer.MAX_VALUE)); + assertThat(EsqlVersion.SNAPSHOT.id(), equalTo(Integer.MAX_VALUE)); assertThat(EsqlVersion.PARTY_POPPER.id(), equalTo(20240401)); for (EsqlVersion version : EsqlVersion.values()) { - assertTrue(EsqlVersion.NIGHTLY.onOrAfter(version)); - if (version != EsqlVersion.NIGHTLY) { - assertTrue(version.before(EsqlVersion.NIGHTLY)); + assertTrue(EsqlVersion.SNAPSHOT.onOrAfter(version)); + if (version != EsqlVersion.SNAPSHOT) { + assertTrue(version.before(EsqlVersion.SNAPSHOT)); } else { - assertTrue(version.onOrAfter(EsqlVersion.NIGHTLY)); + assertTrue(version.onOrAfter(EsqlVersion.SNAPSHOT)); } } From 0b5fb1fe820112cbfa84a1f39103386161012c98 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 14:11:44 +0200 Subject: [PATCH 21/27] Add latest version to error message when missing --- .../xpack/esql/action/EsqlQueryRequest.java | 9 +++++- .../xpack/esql/version/EsqlVersion.java | 32 +++++++++++-------- .../esql/action/EsqlQueryRequestTests.java | 5 ++- .../xpack/esql/version/EsqlVersionTests.java | 4 +++ 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index b904f0b83a49d..5eeeb94bd449c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -71,7 +71,14 @@ public ActionRequestValidationException validate() { if (Strings.hasText(esqlVersion) == false) { // TODO: make this required // "https://github.com/elastic/elasticsearch/issues/104890" - // validationException = addValidationError("[" + RequestXContent.ESQL_VERSION_FIELD + "] is required", validationException); + // validationException = addValidationError( + // "[" + // + RequestXContent.ESQL_VERSION_FIELD + // + "] is required, latest available version is [" + // + EsqlVersion.latestReleased() + // + "]", + // validationException + // ); } else { EsqlVersion version = EsqlVersion.parse(esqlVersion); if (version == null) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index bb2483e05936b..6c2f12ee24f2c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -10,6 +10,8 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.VersionId; +import java.util.Arrays; +import java.util.Comparator; import java.util.LinkedHashMap; import java.util.Map; @@ -40,6 +42,23 @@ private static void putVersionAssertNoDups(Map stringToVers } } + /** + * Accepts a version string with the emoji suffix or without it. + * E.g. both "2024.04.01.🎉" and "2024.04.01" will be interpreted as {@link EsqlVersion#PARTY_POPPER}. + */ + public static EsqlVersion parse(String versionString) { + return VERSION_MAP_WITH_AND_WITHOUT_EMOJI.get(versionString); + } + + public static EsqlVersion latestReleased() { + return Arrays.stream(EsqlVersion.values()).filter(v -> v != SNAPSHOT).max(Comparator.comparingInt(EsqlVersion::id)).get(); + } + + private int year; + private byte month; + private byte revision; + private String emoji; + EsqlVersion(int year, int month, String emoji) { this(year, month, 1, emoji); } @@ -60,11 +79,6 @@ private static void putVersionAssertNoDups(Map stringToVers this.emoji = emoji; } - private int year; - private byte month; - private byte revision; - private String emoji; - public int year() { return year; } @@ -81,14 +95,6 @@ public String emoji() { return emoji; } - /** - * Accepts a version string with the emoji suffix or without it. - * E.g. both "2024.04.01.🎉" and "2024.04.01" will be interpreted as {@link EsqlVersion#PARTY_POPPER}. - */ - public static EsqlVersion parse(String versionString) { - return VERSION_MAP_WITH_AND_WITHOUT_EMOJI.get(versionString); - } - public String versionStringWithoutEmoji() { return this == SNAPSHOT ? "snapshot" : Strings.format("%d.%02d.%02d", year, month, revision); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 8c0451c94e323..d0a767e19022c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -225,7 +225,10 @@ public void testMissingVersionIsNotValid() throws IOException { EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[version] is required")); + assertThat( + request.validate().getMessage(), + containsString("[version] is required, latest available version is [" + EsqlVersion.latestReleased() + "]") + ); } public void testMissingQueryIsNotValid() throws IOException { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index ea55a33e7e0ec..5ef5e3aef93cd 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -17,6 +17,10 @@ import static org.hamcrest.Matchers.is; public class EsqlVersionTests extends ESTestCase { + public void testLatestReleased() { + assertThat(EsqlVersion.latestReleased(), is(EsqlVersion.PARTY_POPPER)); + } + public void testVersionString() { assertThat(EsqlVersion.SNAPSHOT.toString(), equalTo("snapshot.📷")); assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.01.🎉")); From 962f16be8042693e0638394026a23678e8178f4c Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 14:19:52 +0200 Subject: [PATCH 22/27] Add latest version to error message if invalid --- .../xpack/esql/action/EsqlQueryRequest.java | 8 +++++++- .../xpack/esql/action/EsqlQueryRequestTests.java | 11 ++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index 5eeeb94bd449c..d7ba6eeb6d46e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -83,7 +83,13 @@ public ActionRequestValidationException validate() { EsqlVersion version = EsqlVersion.parse(esqlVersion); if (version == null) { validationException = addValidationError( - "[" + RequestXContent.ESQL_VERSION_FIELD + "] has invalid value [" + esqlVersion + "]", + "[" + + RequestXContent.ESQL_VERSION_FIELD + + "] has invalid value [" + + esqlVersion + + "], latest available version is [" + + EsqlVersion.latestReleased() + + "]", validationException ); } else if (version == EsqlVersion.SNAPSHOT && onSnapshotBuild == false) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index d0a767e19022c..d31dbc5fda286 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -189,7 +189,16 @@ public void testUnknownVersionIsNotValid() throws IOException { EsqlQueryRequest request = parseEsqlQueryRequest(json, randomBoolean()); assertNotNull(request.validate()); - assertThat(request.validate().getMessage(), containsString("[version] has invalid value [" + invalidVersionString + "]")); + assertThat( + request.validate().getMessage(), + containsString( + "[version] has invalid value [" + + invalidVersionString + + "], latest available version is [" + + EsqlVersion.latestReleased() + + "]" + ) + ); } public void testSnapshotVersionIsOnlyValidOnSnapshot() throws IOException { From 3b7c0319a11646e7d7488f80787fbf6a9922c180 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 14:29:16 +0200 Subject: [PATCH 23/27] Add latest version in error if using snapshot --- .../xpack/esql/action/EsqlQueryRequest.java | 32 ++++++++----------- .../esql/action/EsqlQueryRequestTests.java | 8 ++++- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index d7ba6eeb6d46e..227f96e820da9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -71,30 +71,14 @@ public ActionRequestValidationException validate() { if (Strings.hasText(esqlVersion) == false) { // TODO: make this required // "https://github.com/elastic/elasticsearch/issues/104890" - // validationException = addValidationError( - // "[" - // + RequestXContent.ESQL_VERSION_FIELD - // + "] is required, latest available version is [" - // + EsqlVersion.latestReleased() - // + "]", - // validationException - // ); + // validationException = addValidationError(invalidVersion("is required"), validationException); } else { EsqlVersion version = EsqlVersion.parse(esqlVersion); if (version == null) { - validationException = addValidationError( - "[" - + RequestXContent.ESQL_VERSION_FIELD - + "] has invalid value [" - + esqlVersion - + "], latest available version is [" - + EsqlVersion.latestReleased() - + "]", - validationException - ); + validationException = addValidationError(invalidVersion("has invalid value [" + esqlVersion + "]"), validationException); } else if (version == EsqlVersion.SNAPSHOT && onSnapshotBuild == false) { validationException = addValidationError( - "[" + RequestXContent.ESQL_VERSION_FIELD + "] with value [" + esqlVersion + "] only allowed in snapshot builds", + invalidVersion("with value [" + esqlVersion + "] only allowed in snapshot builds"), validationException ); } @@ -111,6 +95,16 @@ public ActionRequestValidationException validate() { return validationException; } + private static String invalidVersion(String reason) { + return "[" + + RequestXContent.ESQL_VERSION_FIELD + + "] " + + reason + + ", latest available version is [" + + EsqlVersion.latestReleased() + + "]"; + } + public EsqlQueryRequest() {} public void esqlVersion(String esqlVersion) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index d31dbc5fda286..048997760f1af 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -218,7 +218,13 @@ public void testSnapshotVersionIsOnlyValidOnSnapshot() throws IOException { assertNotNull(request.validate()); assertThat( request.validate().getMessage(), - containsString("[version] with value [" + esqlVersion + "] only allowed in snapshot builds") + containsString( + "[version] with value [" + + esqlVersion + + "] only allowed in snapshot builds, latest available version is [" + + EsqlVersion.latestReleased() + + "]" + ) ); } From 20118c63e4506bec5291c0c554822bed120ff61d Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 15:03:16 +0200 Subject: [PATCH 24/27] Remove emoji from error message Might cause trouble for people on a terminal. --- .../elasticsearch/xpack/esql/action/EsqlQueryRequest.java | 2 +- .../xpack/esql/action/EsqlQueryRequestTests.java | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index 227f96e820da9..54ae2f4c90fc1 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -101,7 +101,7 @@ private static String invalidVersion(String reason) { + "] " + reason + ", latest available version is [" - + EsqlVersion.latestReleased() + + EsqlVersion.latestReleased().versionStringWithoutEmoji() + "]"; } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 048997760f1af..44066ff3d091d 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -195,7 +195,7 @@ public void testUnknownVersionIsNotValid() throws IOException { "[version] has invalid value [" + invalidVersionString + "], latest available version is [" - + EsqlVersion.latestReleased() + + EsqlVersion.latestReleased().versionStringWithoutEmoji() + "]" ) ); @@ -222,7 +222,7 @@ public void testSnapshotVersionIsOnlyValidOnSnapshot() throws IOException { "[version] with value [" + esqlVersion + "] only allowed in snapshot builds, latest available version is [" - + EsqlVersion.latestReleased() + + EsqlVersion.latestReleased().versionStringWithoutEmoji() + "]" ) ); @@ -242,7 +242,9 @@ public void testMissingVersionIsNotValid() throws IOException { assertNotNull(request.validate()); assertThat( request.validate().getMessage(), - containsString("[version] is required, latest available version is [" + EsqlVersion.latestReleased() + "]") + containsString( + "[version] is required, latest available version is [" + EsqlVersion.latestReleased().versionStringWithoutEmoji() + "]" + ) ); } From 037f1f7021aa8d96a61d12a14732f93d231dd28a Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 17:43:15 +0200 Subject: [PATCH 25/27] Throw IAE instead of AssertionError --- .../xpack/esql/version/EsqlVersion.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index 6c2f12ee24f2c..fbd70641047e1 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -28,17 +28,17 @@ private static Map versionMapWithAndWithoutEmoji() { Map stringToVersion = new LinkedHashMap<>(EsqlVersion.values().length * 2); for (EsqlVersion version : EsqlVersion.values()) { - putVersionAssertNoDups(stringToVersion, version.versionStringWithoutEmoji(), version); - putVersionAssertNoDups(stringToVersion, version.toString(), version); + putVersionCheckNoDups(stringToVersion, version.versionStringWithoutEmoji(), version); + putVersionCheckNoDups(stringToVersion, version.toString(), version); } return stringToVersion; } - private static void putVersionAssertNoDups(Map stringToVersion, String versionString, EsqlVersion version) { + private static void putVersionCheckNoDups(Map stringToVersion, String versionString, EsqlVersion version) { EsqlVersion existingVersionForKey = stringToVersion.put(versionString, version); if (existingVersionForKey != null) { - throw new AssertionError("Duplicate esql version with version string [" + versionString + "]"); + throw new IllegalArgumentException("Duplicate esql version with version string [" + versionString + "]"); } } @@ -65,13 +65,13 @@ public static EsqlVersion latestReleased() { EsqlVersion(int year, int month, int revision, String emoji) { if ((1 <= revision && revision <= 99) == false) { - throw new AssertionError("Version revision number must be between 1 and 99 but was [" + revision + "]"); + throw new IllegalArgumentException("Version revision number must be between 1 and 99 but was [" + revision + "]"); } if ((1 <= month && month <= 12) == false) { - throw new AssertionError("Version month must be between 1 and 12 but was [" + month + "]"); + throw new IllegalArgumentException("Version month must be between 1 and 12 but was [" + month + "]"); } if ((emoji.codePointCount(0, emoji.length()) == 1) == false) { - throw new AssertionError("Version emoji must be a single unicode character but was [" + emoji + "]"); + throw new IllegalArgumentException("Version emoji must be a single unicode character but was [" + emoji + "]"); } this.year = year; this.month = (byte) month; From ab3be62022d722ef73dee6a63461ee38e85090cd Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 17:45:42 +0200 Subject: [PATCH 26/27] Use rocket emoji for first released version --- .../org/elasticsearch/xpack/esql/version/EsqlVersion.java | 4 ++-- .../elasticsearch/xpack/esql/version/EsqlVersionTests.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java index fbd70641047e1..9f96ba0e64e17 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/version/EsqlVersion.java @@ -20,7 +20,7 @@ public enum EsqlVersion implements VersionId { * Breaking changes go here until the next version is released. */ SNAPSHOT(Integer.MAX_VALUE, 12, 99, "📷"), - PARTY_POPPER(2024, 4, "🎉"); + ROCKET(2024, 4, "🚀"); static final Map VERSION_MAP_WITH_AND_WITHOUT_EMOJI = versionMapWithAndWithoutEmoji(); @@ -44,7 +44,7 @@ private static void putVersionCheckNoDups(Map stringToVersi /** * Accepts a version string with the emoji suffix or without it. - * E.g. both "2024.04.01.🎉" and "2024.04.01" will be interpreted as {@link EsqlVersion#PARTY_POPPER}. + * E.g. both "2024.04.01.🚀" and "2024.04.01" will be interpreted as {@link EsqlVersion#ROCKET}. */ public static EsqlVersion parse(String versionString) { return VERSION_MAP_WITH_AND_WITHOUT_EMOJI.get(versionString); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index 5ef5e3aef93cd..902824c0aacc2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -18,17 +18,17 @@ public class EsqlVersionTests extends ESTestCase { public void testLatestReleased() { - assertThat(EsqlVersion.latestReleased(), is(EsqlVersion.PARTY_POPPER)); + assertThat(EsqlVersion.latestReleased(), is(EsqlVersion.ROCKET)); } public void testVersionString() { assertThat(EsqlVersion.SNAPSHOT.toString(), equalTo("snapshot.📷")); - assertThat(EsqlVersion.PARTY_POPPER.toString(), equalTo("2024.04.01.🎉")); + assertThat(EsqlVersion.ROCKET.toString(), equalTo("2024.04.01.🚀")); } public void testVersionId() { assertThat(EsqlVersion.SNAPSHOT.id(), equalTo(Integer.MAX_VALUE)); - assertThat(EsqlVersion.PARTY_POPPER.id(), equalTo(20240401)); + assertThat(EsqlVersion.ROCKET.id(), equalTo(20240401)); for (EsqlVersion version : EsqlVersion.values()) { assertTrue(EsqlVersion.SNAPSHOT.onOrAfter(version)); From acb270362128204260bc0fd666b26885abad576a Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 3 Apr 2024 18:29:48 +0200 Subject: [PATCH 27/27] Fix test --- .../org/elasticsearch/xpack/esql/version/EsqlVersionTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java index 902824c0aacc2..cd4fd77a8dd22 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/version/EsqlVersionTests.java @@ -72,7 +72,7 @@ public static String randomInvalidVersionString() { String[] invalidVersionString = new String[1]; do { - int length = randomIntBetween(0, 10); + int length = randomIntBetween(1, 10); invalidVersionString[0] = randomAlphaOfLength(length); } while (EsqlVersion.VERSION_MAP_WITH_AND_WITHOUT_EMOJI.containsKey(invalidVersionString[0]));