From 0775ae6a0e12ef5d70fcf372d325915675a2e510 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 29 May 2019 15:08:42 +0300 Subject: [PATCH 1/6] Refactor Version class to make version bumps easier With thic change we only have to add one line to add a new version. The intent is to make it less error prone and easier to write a script to automate teh process. --- .../main/java/org/elasticsearch/Version.java | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index 7c48b38a8cf90..42f0d404c091a 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -34,8 +34,10 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; public class Version implements Comparable, ToXContentFragment { @@ -46,26 +48,29 @@ public class Version implements Comparable, ToXContentFragment { */ public static final int V_EMPTY_ID = 0; public static final Version V_EMPTY = new Version(V_EMPTY_ID, org.apache.lucene.util.Version.LATEST); - public static final int V_7_0_0_ID = 7000099; - public static final Version V_7_0_0 = new Version(V_7_0_0_ID, org.apache.lucene.util.Version.LUCENE_8_0_0); - public static final int V_7_0_1_ID = 7000199; - public static final Version V_7_0_1 = new Version(V_7_0_1_ID, org.apache.lucene.util.Version.LUCENE_8_0_0); - public static final int V_7_1_0_ID = 7010099; - public static final Version V_7_1_0 = new Version(V_7_1_0_ID, org.apache.lucene.util.Version.LUCENE_8_0_0); - public static final int V_7_1_1_ID = 7010199; - public static final Version V_7_1_1 = new Version(V_7_1_1_ID, org.apache.lucene.util.Version.LUCENE_8_0_0); - public static final int V_7_1_2_ID = 7010299; - public static final Version V_7_1_2 = new Version(V_7_1_2_ID, org.apache.lucene.util.Version.LUCENE_8_0_0); - public static final int V_7_2_0_ID = 7020099; - public static final Version V_7_2_0 = new Version(V_7_2_0_ID, org.apache.lucene.util.Version.LUCENE_8_0_0); - public static final int V_7_3_0_ID = 7030099; - public static final Version V_7_3_0 = new Version(V_7_3_0_ID, org.apache.lucene.util.Version.LUCENE_8_1_0); - public static final int V_8_0_0_ID = 8000099; - public static final Version V_8_0_0 = new Version(V_8_0_0_ID, org.apache.lucene.util.Version.LUCENE_8_1_0); + public static final Version V_7_0_0 = new Version(7000099, org.apache.lucene.util.Version.LUCENE_8_0_0); + public static final Version V_7_0_1 = new Version(7000199, org.apache.lucene.util.Version.LUCENE_8_0_0); + public static final Version V_7_1_0 = new Version(7010099, org.apache.lucene.util.Version.LUCENE_8_0_0); + public static final Version V_7_1_1 = new Version(7010199, org.apache.lucene.util.Version.LUCENE_8_0_0); + public static final Version V_7_2_0 = new Version(7020099, org.apache.lucene.util.Version.LUCENE_8_0_0); + public static final Version V_7_3_0 = new Version(7030099, org.apache.lucene.util.Version.LUCENE_8_1_0); + public static final Version V_8_0_0 = new Version(8000099, org.apache.lucene.util.Version.LUCENE_8_1_0); public static final Version CURRENT = V_8_0_0; + private static final Map idToVersion; static { + idToVersion = new HashMap<>(); + for (Field declaredField : Version.class.getFields()) { + if (declaredField.getType().equals(Version.class)) { + try { + Version version = (Version) declaredField.get(null); + idToVersion.put(version.id, version); + } catch (IllegalAccessException e) { + assert false : "version fields should be public"; + } + } + } assert CURRENT.luceneVersion.equals(org.apache.lucene.util.Version.LATEST) : "Version must be upgraded to [" + org.apache.lucene.util.Version.LATEST + "] is still set to [" + CURRENT.luceneVersion + "]"; } @@ -75,23 +80,10 @@ public static Version readVersion(StreamInput in) throws IOException { } public static Version fromId(int id) { + if (idToVersion.containsKey(id)) { + return idToVersion.get(id); + } switch (id) { - case V_8_0_0_ID: - return V_8_0_0; - case V_7_3_0_ID: - return V_7_3_0; - case V_7_2_0_ID: - return V_7_2_0; - case V_7_1_2_ID: - return V_7_1_2; - case V_7_1_1_ID: - return V_7_1_1; - case V_7_1_0_ID: - return V_7_1_0; - case V_7_0_1_ID: - return V_7_0_1; - case V_7_0_0_ID: - return V_7_0_0; case V_EMPTY_ID: return V_EMPTY; default: From 4f5fe60d75b751a17f73f9c1413707b4cd332101 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 29 May 2019 10:20:36 -0400 Subject: [PATCH 2/6] Stronger assertions --- .../main/java/org/elasticsearch/Version.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index 42f0d404c091a..e83c516498e3f 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.collect.ImmutableOpenIntMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; @@ -34,10 +35,8 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Objects; public class Version implements Comparable, ToXContentFragment { @@ -57,22 +56,40 @@ public class Version implements Comparable, ToXContentFragment { public static final Version V_8_0_0 = new Version(8000099, org.apache.lucene.util.Version.LUCENE_8_1_0); public static final Version CURRENT = V_8_0_0; - private static final Map idToVersion; + private static final ImmutableOpenIntMap idToVersion; static { - idToVersion = new HashMap<>(); - for (Field declaredField : Version.class.getFields()) { + final ImmutableOpenIntMap.Builder builder = ImmutableOpenIntMap.builder(); + + for (final Field declaredField : Version.class.getFields()) { if (declaredField.getType().equals(Version.class)) { + if (declaredField.getName().equals("CURRENT") || declaredField.getName().equals("V_EMPTY")) { + continue; + } + assert declaredField.getName().matches("V_\\d+_\\d+_\\d+") : declaredField.getName(); try { - Version version = (Version) declaredField.get(null); - idToVersion.put(version.id, version); - } catch (IllegalAccessException e) { + final Version version = (Version) declaredField.get(null); + if (Assertions.ENABLED) { + final String[] fields = declaredField.getName().split("_"); + final int major = Integer.valueOf(fields[1]) * 1000000; + final int minor = Integer.valueOf(fields[2]) * 10000; + final int revision = Integer.valueOf(fields[3]) * 100; + final int expectedId = major + minor + revision + 99; + assert version.id == expectedId : + "expected version [" + version + "] to have id [" + expectedId + "] but was [" + version.id + "]"; + } + final Version maybePrevious = builder.put(version.id, version); + assert maybePrevious == null : + "expected [" + version.id + "] to be uniquely mapped but saw [" + maybePrevious + "] and [" + version + "]"; + } catch (final IllegalAccessException e) { assert false : "version fields should be public"; } } } assert CURRENT.luceneVersion.equals(org.apache.lucene.util.Version.LATEST) : "Version must be upgraded to [" + org.apache.lucene.util.Version.LATEST + "] is still set to [" + CURRENT.luceneVersion + "]"; + + idToVersion = builder.build(); } public static Version readVersion(StreamInput in) throws IOException { From 72dd7902ffbd5db06cbef9f0d34bbcd766ca276d Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 29 May 2019 10:26:33 -0400 Subject: [PATCH 3/6] Iteration --- server/src/main/java/org/elasticsearch/Version.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index e83c516498e3f..ffdcbc1126f15 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -66,7 +66,8 @@ public class Version implements Comparable, ToXContentFragment { if (declaredField.getName().equals("CURRENT") || declaredField.getName().equals("V_EMPTY")) { continue; } - assert declaredField.getName().matches("V_\\d+_\\d+_\\d+") : declaredField.getName(); + assert declaredField.getName().matches("V_\\d+_\\d+_\\d+") + : "expected Version field [" + declaredField.getName() + "] to match V_\\d+_\\d+_\\d+"; try { final Version version = (Version) declaredField.get(null); if (Assertions.ENABLED) { @@ -82,7 +83,7 @@ public class Version implements Comparable, ToXContentFragment { assert maybePrevious == null : "expected [" + version.id + "] to be uniquely mapped but saw [" + maybePrevious + "] and [" + version + "]"; } catch (final IllegalAccessException e) { - assert false : "version fields should be public"; + assert false : "Version field [" + declaredField.getName() + "] should be public"; } } } From b11be62755925b8b60a48f40298fa27b76740fd1 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 29 May 2019 10:30:58 -0400 Subject: [PATCH 4/6] Do not use Version#toString here, it gives confusing messages --- server/src/main/java/org/elasticsearch/Version.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index ffdcbc1126f15..c1a52e7d41280 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -77,7 +77,7 @@ public class Version implements Comparable, ToXContentFragment { final int revision = Integer.valueOf(fields[3]) * 100; final int expectedId = major + minor + revision + 99; assert version.id == expectedId : - "expected version [" + version + "] to have id [" + expectedId + "] but was [" + version.id + "]"; + "expected version [" + declaredField.getName() + "] to have id [" + expectedId + "] but was [" + version.id + "]"; } final Version maybePrevious = builder.put(version.id, version); assert maybePrevious == null : From 24007a8349693324109a9edb7f39010977f2cae6 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 29 May 2019 10:31:41 -0400 Subject: [PATCH 5/6] Checkstyle --- server/src/main/java/org/elasticsearch/Version.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index c1a52e7d41280..ad43f40f6d7f2 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -63,27 +63,28 @@ public class Version implements Comparable, ToXContentFragment { for (final Field declaredField : Version.class.getFields()) { if (declaredField.getType().equals(Version.class)) { - if (declaredField.getName().equals("CURRENT") || declaredField.getName().equals("V_EMPTY")) { + final String fieldName = declaredField.getName(); + if (fieldName.equals("CURRENT") || fieldName.equals("V_EMPTY")) { continue; } - assert declaredField.getName().matches("V_\\d+_\\d+_\\d+") - : "expected Version field [" + declaredField.getName() + "] to match V_\\d+_\\d+_\\d+"; + assert fieldName.matches("V_\\d+_\\d+_\\d+") + : "expected Version field [" + fieldName + "] to match V_\\d+_\\d+_\\d+"; try { final Version version = (Version) declaredField.get(null); if (Assertions.ENABLED) { - final String[] fields = declaredField.getName().split("_"); + final String[] fields = fieldName.split("_"); final int major = Integer.valueOf(fields[1]) * 1000000; final int minor = Integer.valueOf(fields[2]) * 10000; final int revision = Integer.valueOf(fields[3]) * 100; final int expectedId = major + minor + revision + 99; assert version.id == expectedId : - "expected version [" + declaredField.getName() + "] to have id [" + expectedId + "] but was [" + version.id + "]"; + "expected version [" + fieldName + "] to have id [" + expectedId + "] but was [" + version.id + "]"; } final Version maybePrevious = builder.put(version.id, version); assert maybePrevious == null : "expected [" + version.id + "] to be uniquely mapped but saw [" + maybePrevious + "] and [" + version + "]"; } catch (final IllegalAccessException e) { - assert false : "Version field [" + declaredField.getName() + "] should be public"; + assert false : "Version field [" + fieldName + "] should be public"; } } } From 8cc603a7ecc3497e8957d97f65538d7b957ed3a2 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Thu, 13 Jun 2019 11:12:01 +0300 Subject: [PATCH 6/6] Add missing version --- server/src/main/java/org/elasticsearch/Version.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index ad43f40f6d7f2..1396d3ffd2c82 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -51,6 +51,7 @@ public class Version implements Comparable, ToXContentFragment { public static final Version V_7_0_1 = new Version(7000199, org.apache.lucene.util.Version.LUCENE_8_0_0); public static final Version V_7_1_0 = new Version(7010099, org.apache.lucene.util.Version.LUCENE_8_0_0); public static final Version V_7_1_1 = new Version(7010199, org.apache.lucene.util.Version.LUCENE_8_0_0); + public static final Version V_7_1_2 = new Version(7010299, org.apache.lucene.util.Version.LUCENE_8_0_0); public static final Version V_7_2_0 = new Version(7020099, org.apache.lucene.util.Version.LUCENE_8_0_0); public static final Version V_7_3_0 = new Version(7030099, org.apache.lucene.util.Version.LUCENE_8_1_0); public static final Version V_8_0_0 = new Version(8000099, org.apache.lucene.util.Version.LUCENE_8_1_0);