From c315d54c2a6695301512fecdccb8f7de22e3ccfe Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Thu, 16 Jul 2015 12:41:01 -0400 Subject: [PATCH] Fix serialization of IndexFormatTooNewException and IndexFormatTooOldException This is essentially an ugly hack to get us by until a proper solution is possible with Lucene 5.3 Closes #12277 --- .../common/io/stream/StreamInput.java | 26 ++++-- .../common/io/stream/StreamOutput.java | 88 ++++++++++++++++++- .../ElasticsearchExceptionTests.java | 14 ++- 3 files changed, 110 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java b/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java index 8b6fdbdb45c..9c7041abedc 100644 --- a/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java +++ b/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java @@ -497,16 +497,26 @@ public abstract class StreamInput extends InputStream { final String name = readString(); return (T) readException(this, name); case 1: - // this sucks it would be nice to have a better way to construct those? - String msg = readOptionalString(); - final int idx = msg.indexOf(" (resource="); - final String resource = msg.substring(idx + " (resource=".length(), msg.length()-1); - msg = msg.substring(0, idx); - return (T) readStackTrace(new CorruptIndexException(msg, resource, readThrowable()), this); // Lucene 5.3 will have getters for all these + String msg1 = readOptionalString(); + String resource1 = readOptionalString(); + return (T) readStackTrace(new CorruptIndexException(msg1, resource1, readThrowable()), this); case 2: - return (T) readStackTrace(new IndexFormatTooNewException(readOptionalString(), -1, -1, -1), this); // Lucene 5.3 will have getters for all these + String resource2 = readOptionalString(); + int version2 = readInt(); + int minVersion2 = readInt(); + int maxVersion2 = readInt(); + return (T) readStackTrace(new IndexFormatTooNewException(resource2, version2, minVersion2, maxVersion2), this); case 3: - return (T) readStackTrace(new IndexFormatTooOldException(readOptionalString(), -1, -1, -1), this); // Lucene 5.3 will have getters for all these + String resource3 = readOptionalString(); + if (readBoolean()) { + int version3 = readInt(); + int minVersion3 = readInt(); + int maxVersion3 = readInt(); + return (T) readStackTrace(new IndexFormatTooOldException(resource3, version3, minVersion3, maxVersion3), this); + } else { + String version3 = readOptionalString(); + return (T) readStackTrace(new IndexFormatTooOldException(resource3, version3), this); + } case 4: return (T) readStackTrace(new NullPointerException(readOptionalString()), this); case 5: diff --git a/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java b/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java index 6670e36fc62..d00ca4446c2 100644 --- a/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java +++ b/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java @@ -31,6 +31,7 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.text.Text; import org.joda.time.ReadableInstant; @@ -43,6 +44,8 @@ import java.util.Date; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * @@ -453,19 +456,100 @@ public abstract class StreamOutput extends OutputStream { } } + static { + assert Version.CURRENT.luceneVersion == org.apache.lucene.util.Version.LUCENE_5_2_1: "Remove these regex once we upgrade to Lucene 5.3 and get proper getters for these expections"; + } + private final static Pattern CORRUPT_INDEX_EXCEPTION_REGEX = Regex.compile("^(.+) \\(resource=(.+)\\)$", ""); + private final static Pattern INDEX_FORMAT_TOO_NEW_EXCEPTION_REGEX = Regex.compile("Format version is not supported \\(resource (.+)\\): (-?\\d+) \\(needs to be between (-?\\d+) and (-?\\d+)\\)", ""); + private final static Pattern INDEX_FORMAT_TOO_OLD_EXCEPTION_REGEX_1 = Regex.compile("Format version is not supported \\(resource (.+)\\): (-?\\d+)(?: \\(needs to be between (-?\\d+) and (-?\\d+)\\)). This version of Lucene only supports indexes created with release 4.0 and later\\.", ""); + private final static Pattern INDEX_FORMAT_TOO_OLD_EXCEPTION_REGEX_2 = Regex.compile("Format version is not supported \\(resource (.+)\\): (.+). This version of Lucene only supports indexes created with release 4.0 and later\\.", ""); + + private static int parseIntSafe(String val, int defaultVal) { + try { + return Integer.parseInt(val); + } catch (NumberFormatException ex) { + return defaultVal; + } + } + public void writeThrowable(Throwable throwable) throws IOException { if (throwable == null) { writeBoolean(false); } else { writeBoolean(true); boolean writeCause = true; + boolean writeMessage = true; if (throwable instanceof CorruptIndexException) { writeVInt(1); + // Lucene 5.3 will have getters for all these + // we should switch to using getters instead of trying to parse the message: + // writeOptionalString(((CorruptIndexException)throwable).getDescription()); + // writeOptionalString(((CorruptIndexException)throwable).getResource()); + Matcher matcher = CORRUPT_INDEX_EXCEPTION_REGEX.matcher(throwable.getMessage()); + if (matcher.find()) { + writeOptionalString(matcher.group(1)); // message + writeOptionalString(matcher.group(2)); // resource + } else { + // didn't match + writeOptionalString("???"); // message + writeOptionalString("???"); // resource + } + writeMessage = false; } else if (throwable instanceof IndexFormatTooNewException) { writeVInt(2); + // Lucene 5.3 will have getters for all these + // we should switch to using getters instead of trying to parse the message: + // writeOptionalString(((CorruptIndexException)throwable).getResource()); + // writeInt(((IndexFormatTooNewException)throwable).getVersion()); + // writeInt(((IndexFormatTooNewException)throwable).getMinVersion()); + // writeInt(((IndexFormatTooNewException)throwable).getMaxVersion()); + Matcher matcher = INDEX_FORMAT_TOO_NEW_EXCEPTION_REGEX.matcher(throwable.getMessage()); + if (matcher.find()) { + writeOptionalString(matcher.group(1)); // resource + writeInt(parseIntSafe(matcher.group(2), -1)); // version + writeInt(parseIntSafe(matcher.group(3), -1)); // min version + writeInt(parseIntSafe(matcher.group(4), -1)); // max version + } else { + // didn't match + writeOptionalString("???"); // resource + writeInt(-1); // version + writeInt(-1); // min version + writeInt(-1); // max version + } + writeMessage = false; writeCause = false; } else if (throwable instanceof IndexFormatTooOldException) { writeVInt(3); + // Lucene 5.3 will have getters for all these + // we should switch to using getters instead of trying to parse the message: + // writeOptionalString(((CorruptIndexException)throwable).getResource()); + // writeInt(((IndexFormatTooNewException)throwable).getVersion()); + // writeInt(((IndexFormatTooNewException)throwable).getMinVersion()); + // writeInt(((IndexFormatTooNewException)throwable).getMaxVersion()); + Matcher matcher = INDEX_FORMAT_TOO_OLD_EXCEPTION_REGEX_1.matcher(throwable.getMessage()); + if (matcher.find()) { + // version with numeric version in constructor + writeOptionalString(matcher.group(1)); // resource + writeBoolean(true); + writeInt(parseIntSafe(matcher.group(2), -1)); // version + writeInt(parseIntSafe(matcher.group(3), -1)); // min version + writeInt(parseIntSafe(matcher.group(4), -1)); // max version + } else { + matcher = INDEX_FORMAT_TOO_OLD_EXCEPTION_REGEX_2.matcher(throwable.getMessage()); + if (matcher.matches()) { + writeOptionalString(matcher.group(1)); // resource + writeBoolean(false); + writeOptionalString(matcher.group(2)); // version + } else { + // didn't match + writeOptionalString("???"); // resource + writeBoolean(true); + writeInt(-1); // version + writeInt(-1); // min version + writeInt(-1); // max version + } + } + writeMessage = false; writeCause = false; } else if (throwable instanceof NullPointerException) { writeVInt(4); @@ -520,7 +604,9 @@ public abstract class StreamOutput extends OutputStream { return; } - writeOptionalString(throwable.getMessage()); + if (writeMessage) { + writeOptionalString(throwable.getMessage()); + } if (writeCause) { writeThrowable(throwable.getCause()); } diff --git a/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index 70face4b34f..b9995e83bef 100644 --- a/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -295,9 +295,10 @@ public class ElasticsearchExceptionTests extends ElasticsearchTestCase { new EOFException("dadada"), new ElasticsearchSecurityException("nono!"), new NumberFormatException("not a number"), - new CorruptIndexException("baaaam", "this is my resource"), - new IndexFormatTooNewException("tooo new", 1, 1, 1), - new IndexFormatTooOldException("tooo new", 1, 1, 1), + new CorruptIndexException("baaaam booom", "this is my resource"), + new IndexFormatTooNewException("tooo new", 1, 2, 3), + new IndexFormatTooOldException("tooo new", 1, 2, 3), + new IndexFormatTooOldException("tooo new", "very old version"), new ArrayIndexOutOfBoundsException("booom"), new StringIndexOutOfBoundsException("booom"), new FileNotFoundException("booom"), @@ -315,12 +316,7 @@ public class ElasticsearchExceptionTests extends ElasticsearchTestCase { StreamInput in = StreamInput.wrap(out.bytes()); ElasticsearchException e = in.readThrowable(); assertEquals(e.getMessage(), ex.getMessage()); - if (t instanceof IndexFormatTooNewException || t instanceof IndexFormatTooOldException) { - // these don't work yet - missing ctors - assertNotEquals(e.getCause().getMessage(), ex.getCause().getMessage()); - } else { - assertEquals(ex.getCause().getClass().getName(), e.getCause().getMessage(), ex.getCause().getMessage()); - } + assertEquals(ex.getCause().getClass().getName(), e.getCause().getMessage(), ex.getCause().getMessage()); if (ex.getCause().getClass() != Throwable.class) { // throwable is not directly mapped assertEquals(e.getCause().getClass(), ex.getCause().getClass()); } else {