From 90353ceb79f5dd132e7e73df31e11df56c120bec Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Thu, 25 Apr 2013 15:13:31 +0200 Subject: [PATCH] Fixing possible NoClassDefFoundError when trying to load nonexisting classes In order to handle exceptions correctly, when classes are not found, one needs to handle ClassNotFoundException as well as NoClassDefFoundError in order to be sure to have caught every possible case. We did not cater for the latter in ImmutableSettings yet. This fix is just executing the same logic for both exceptions instead of simply bubbling up NoClassDefFoundError. --- .../common/settings/ImmutableSettings.java | 18 ++++++++++++------ .../settings/ImmutableSettingsTests.java | 6 ++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java b/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java index 4021c4219a3..4f2fb4831f2 100644 --- a/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java +++ b/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java @@ -332,16 +332,22 @@ public class ImmutableSettings implements Settings { try { return (Class) getClassLoader().loadClass(fullClassName); } catch (ClassNotFoundException e1) { - fullClassName = prefixValue + toCamelCase(sValue).toLowerCase() + "." + Strings.capitalize(toCamelCase(sValue)) + suffixClassName; - try { - return (Class) getClassLoader().loadClass(fullClassName); - } catch (ClassNotFoundException e2) { - throw new NoClassSettingsException("Failed to load class setting [" + setting + "] with value [" + get(setting) + "]", e2); - } + return loadClass(prefixValue, sValue, suffixClassName, setting); + } catch (NoClassDefFoundError e1) { + return loadClass(prefixValue, sValue, suffixClassName, setting); } } } + private Class loadClass(String prefixValue, String sValue, String suffixClassName, String setting) { + String fullClassName = prefixValue + toCamelCase(sValue).toLowerCase() + "." + Strings.capitalize(toCamelCase(sValue)) + suffixClassName; + try { + return (Class) getClassLoader().loadClass(fullClassName); + } catch (ClassNotFoundException e2) { + throw new NoClassSettingsException("Failed to load class setting [" + setting + "] with value [" + get(setting) + "]", e2); + } + } + @Override public String[] getAsArray(String settingPrefix) throws SettingsException { return getAsArray(settingPrefix, Strings.EMPTY_ARRAY); diff --git a/src/test/java/org/elasticsearch/test/unit/common/settings/ImmutableSettingsTests.java b/src/test/java/org/elasticsearch/test/unit/common/settings/ImmutableSettingsTests.java index 417817fec43..b1fd9fda5ae 100644 --- a/src/test/java/org/elasticsearch/test/unit/common/settings/ImmutableSettingsTests.java +++ b/src/test/java/org/elasticsearch/test/unit/common/settings/ImmutableSettingsTests.java @@ -81,4 +81,10 @@ public class ImmutableSettingsTests { assertThat(settings.toDelimitedString(';'), equalTo("key1=value1;key2=value2;")); } + @Test(expectedExceptions = NoClassSettingsException.class) + public void testThatAllClassNotFoundExceptionsAreCaught() { + // this should be nGram in order to really work, but for sure not not throw a NoClassDefFoundError + Settings settings = settingsBuilder().put("type", "ngram").build(); + settings.getAsClass("type", null, "org.elasticsearch.index.analysis.", "TokenFilterFactory"); + } }