From 6234a49fb320388a5a1f489be9deb541772e7d7c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 4 Apr 2017 00:17:05 -0400 Subject: [PATCH] Fix initialization issue in ElasticsearchException If a test touches ElasticsearchExceptionHandle before the class initialzer for ElasticsearchException has run, a circular class initialization problem can arise. Namely, the class initializer for ElasticsearchExceptionHandle depends on the class initializer for ElasticsearchExceptionHandle which depends on the class initializer for all the classes that extend ElasticsearchException, but these classes can not be loaded because ElasticsearchException has not finished its class initializer. There are tests that can trigger this before ElasticsearchException has been loaded due to an unlucky ordering of test execution. This commit addresses this issue by making ElasticsearchExceptionHandle private, and then exposing methods that provide the necessary values from ElasticsearchExceptionHandle. Touching these methods will force the class initializer for ElasticsearchException to run first. --- .../elasticsearch/ElasticsearchException.java | 27 ++++++++++++++++++- .../ExceptionSerializationTests.java | 16 ++++++----- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/ElasticsearchException.java b/core/src/main/java/org/elasticsearch/ElasticsearchException.java index 5d5a98ce3a9..b6ed2cb7a2b 100644 --- a/core/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/core/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -23,6 +23,7 @@ import org.elasticsearch.action.support.replication.ReplicationOperation; import org.elasticsearch.cluster.action.shard.ShardStateAction; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -712,7 +713,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte * in id order below. If you want to remove an exception leave a tombstone comment and mark the id as null in * ExceptionSerializationTests.testIds.ids. */ - enum ElasticsearchExceptionHandle { + private enum ElasticsearchExceptionHandle { INDEX_SHARD_SNAPSHOT_FAILED_EXCEPTION(org.elasticsearch.index.snapshots.IndexShardSnapshotFailedException.class, org.elasticsearch.index.snapshots.IndexShardSnapshotFailedException::new, 0, UNKNOWN_VERSION_ADDED), DFS_PHASE_EXECUTION_EXCEPTION(org.elasticsearch.search.dfs.DfsPhaseExecutionException.class, @@ -1006,6 +1007,30 @@ public class ElasticsearchException extends RuntimeException implements ToXConte } } + /** + * Returns an array of all registered handle IDs. These are the IDs for every registered + * exception. + * + * @return an array of all registered handle IDs + */ + static int[] ids() { + return Arrays.stream(ElasticsearchExceptionHandle.values()).mapToInt(h -> h.id).toArray(); + } + + /** + * Returns an array of all registered pairs of handle IDs and exception classes. These pairs are + * provided for every registered exception. + * + * @return an array of all registered pairs of handle IDs and exception classes + */ + static Tuple>[] classes() { + @SuppressWarnings("unchecked") + final Tuple>[] ts = + Arrays.stream(ElasticsearchExceptionHandle.values()) + .map(h -> Tuple.tuple(h.id, h.exceptionClass)).toArray(Tuple[]::new); + return ts; + } + static { ID_TO_SUPPLIER = unmodifiableMap(Arrays .stream(ElasticsearchExceptionHandle.values()).collect(Collectors.toMap(e -> e.id, e -> e.constructor))); diff --git a/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java b/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java index 2cd87c0470e..cca3fb352b8 100644 --- a/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java +++ b/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java @@ -43,6 +43,7 @@ import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.breaker.CircuitBreakingException; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NotSerializableExceptionWrapper; @@ -680,15 +681,15 @@ public class ExceptionSerializationTests extends ESTestCase { } public void testThatIdsArePositive() { - for (ElasticsearchException.ElasticsearchExceptionHandle handle : ElasticsearchException.ElasticsearchExceptionHandle.values()) { - assertThat("negative id", handle.id, greaterThanOrEqualTo(0)); + for (final int id : ElasticsearchException.ids()) { + assertThat("negative id", id, greaterThanOrEqualTo(0)); } } public void testThatIdsAreUnique() { - Set ids = new HashSet<>(); - for (ElasticsearchException.ElasticsearchExceptionHandle handle : ElasticsearchException.ElasticsearchExceptionHandle.values()) { - assertTrue("duplicate id", ids.add(handle.id)); + final Set ids = new HashSet<>(); + for (final int id: ElasticsearchException.ids()) { + assertTrue("duplicate id", ids.add(id)); } } @@ -848,8 +849,9 @@ public class ExceptionSerializationTests extends ESTestCase { } } - for (ElasticsearchException.ElasticsearchExceptionHandle handle : ElasticsearchException.ElasticsearchExceptionHandle.values()) { - assertEquals((int) reverse.get(handle.exceptionClass), handle.id); + for (final Tuple> tuple : ElasticsearchException.classes()) { + assertNotNull(tuple.v1()); + assertEquals((int) reverse.get(tuple.v2()), (int)tuple.v1()); } for (Map.Entry> entry : ids.entrySet()) {