Skip to content

Commit 6ec459b

Browse files
xuezhou25xuezhou25
committed
delete InvalidArgumentException and change illegalArgumentException to SettingsException
Signed-off-by: Xue Zhou <[email protected]>
1 parent d0d7465 commit 6ec459b

File tree

15 files changed

+95
-127
lines changed

15 files changed

+95
-127
lines changed

‎client/rest-high-level/src/test/java/org/opensearch/client/ClusterClientIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,10 @@ public void testClusterUpdateSettingNonExistent() {
159159
highLevelClient().cluster()::putSettingsAsync
160160
)
161161
);
162-
assertThat(exception.status(), equalTo(RestStatus.INTERNAL_SERVER_ERROR));
162+
assertThat(exception.status(), equalTo(RestStatus.BAD_REQUEST));
163163
assertThat(
164164
exception.getMessage(),
165-
equalTo("OpenSearch exception [type=invalid_argument_exception, reason=transient setting [" + setting + "], not recognized]")
165+
equalTo("OpenSearch exception [type=settings_exception, reason=transient setting [" + setting + "], not recognized]")
166166
);
167167
}
168168

‎client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,8 +1437,7 @@ public void testIndexPutSettings() throws IOException {
14371437
assertThat(
14381438
exception.getMessage(),
14391439
startsWith(
1440-
"OpenSearch exception [type=invalid_argument_exception, "
1441-
+ "reason=final index setting [index.number_of_shards], not updateable"
1440+
"OpenSearch exception [type=settings_exception, " + "reason=final index setting [index.number_of_shards], not updateable"
14421441
)
14431442
);
14441443
}
@@ -1471,11 +1470,11 @@ public void testIndexPutSettingNonExistent() throws IOException {
14711470
highLevelClient().indices()::putSettingsAsync
14721471
)
14731472
);
1474-
assertThat(exception.status(), equalTo(RestStatus.INTERNAL_SERVER_ERROR));
1473+
assertThat(exception.status(), equalTo(RestStatus.BAD_REQUEST));
14751474
assertThat(
14761475
exception.getMessage(),
14771476
equalTo(
1478-
"OpenSearch exception [type=invalid_argument_exception, "
1477+
"OpenSearch exception [type=settings_exception, "
14791478
+ "reason=unknown setting [index.no_idea_what_you_are_talking_about] please check that any required plugins are installed, "
14801479
+ "or check the breaking changes documentation for removed settings]"
14811480
)

‎server/src/main/java/org/opensearch/InvalidArgumentException.java‎

Lines changed: 0 additions & 28 deletions
This file was deleted.

‎server/src/main/java/org/opensearch/OpenSearchException.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,12 +1613,6 @@ private enum OpenSearchExceptionHandle {
16131613
org.opensearch.cluster.decommission.NodeDecommissionedException::new,
16141614
164,
16151615
V_3_0_0
1616-
),
1617-
INVALID_ARGUMENT_EXCEPTION(
1618-
org.opensearch.InvalidArgumentException.class,
1619-
org.opensearch.InvalidArgumentException::new,
1620-
165,
1621-
V_3_0_0
16221616
);
16231617

16241618
final Class<? extends OpenSearchException> exceptionClass;

‎server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.opensearch.common.regex.Regex;
5959
import org.opensearch.common.settings.IndexScopedSettings;
6060
import org.opensearch.common.settings.Settings;
61+
import org.opensearch.common.settings.SettingsException;
6162
import org.opensearch.common.unit.TimeValue;
6263
import org.opensearch.common.util.set.Sets;
6364
import org.opensearch.common.xcontent.NamedXContentRegistry;
@@ -220,7 +221,7 @@ ClusterState addComponentTemplate(
220221
) throws Exception {
221222
final ComponentTemplate existing = currentState.metadata().componentTemplates().get(name);
222223
if (create && existing != null) {
223-
throw new IllegalArgumentException("component template [" + name + "] already exists");
224+
throw new SettingsException("component template [" + name + "] already exists");
224225
}
225226

226227
CompressedXContent mappings = template.template().mappings();
@@ -256,7 +257,7 @@ ClusterState addComponentTemplate(
256257
}
257258
}
258259
if (globalTemplatesThatUseThisComponent.isEmpty() == false) {
259-
throw new IllegalArgumentException(
260+
throw new SettingsException(
260261
"cannot update component template ["
261262
+ name
262263
+ "] because the following global templates would resolve to specifying the ["

‎server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import org.apache.lucene.search.spell.LevenshteinDistance;
3939
import org.apache.lucene.util.CollectionUtil;
4040
import org.opensearch.ExceptionsHelper;
41-
import org.opensearch.InvalidArgumentException;
4241
import org.opensearch.common.collect.Tuple;
4342
import org.opensearch.common.regex.Regex;
4443

@@ -100,16 +99,14 @@ protected AbstractScopedSettings(
10099
Map<String, Setting<?>> keySettings = new HashMap<>();
101100
for (Setting<?> setting : settingsSet) {
102101
if (setting.getProperties().contains(scope) == false) {
103-
throw new InvalidArgumentException(
104-
"Setting " + setting + " must be a " + scope + " setting but has: " + setting.getProperties()
105-
);
102+
throw new SettingsException("Setting " + setting + " must be a " + scope + " setting but has: " + setting.getProperties());
106103
}
107104
validateSettingKey(setting);
108105

109106
if (setting.hasComplexMatcher()) {
110107
Setting<?> overlappingSetting = findOverlappingSetting(setting, complexMatchers);
111108
if (overlappingSetting != null) {
112-
throw new InvalidArgumentException(
109+
throw new SettingsException(
113110
"complex setting key: ["
114111
+ setting.getKey()
115112
+ "] overlaps existing setting key: ["
@@ -130,7 +127,7 @@ protected void validateSettingKey(Setting<?> setting) {
130127
if (isValidKey(setting.getKey()) == false
131128
&& (setting.isGroupSetting() && isValidGroupKey(setting.getKey()) || isValidAffixKey(setting.getKey())) == false
132129
|| setting.getKey().endsWith(".0")) {
133-
throw new InvalidArgumentException("illegal settings key: [" + setting.getKey() + "]");
130+
throw new SettingsException("illegal settings key: [" + setting.getKey() + "]");
134131
}
135132
}
136133

@@ -232,7 +229,7 @@ public synchronized Settings applySettings(Settings newSettings) {
232229
*/
233230
public synchronized <T> void addSettingsUpdateConsumer(Setting<T> setting, Consumer<T> consumer, Consumer<T> validator) {
234231
if (setting != get(setting.getKey())) {
235-
throw new InvalidArgumentException("Setting is not registered for key [" + setting.getKey() + "]");
232+
throw new SettingsException("Setting is not registered for key [" + setting.getKey() + "]");
236233
}
237234
addSettingsUpdater(setting.newUpdater(consumer, logger, validator));
238235
}
@@ -395,7 +392,7 @@ public void apply(Map<String, Settings> values, Settings current, Settings previ
395392
private void ensureSettingIsRegistered(Setting.AffixSetting<?> setting) {
396393
final Setting<?> registeredSetting = this.complexMatchers.get(setting.getKey());
397394
if (setting != registeredSetting) {
398-
throw new InvalidArgumentException("Setting is not registered for key [" + setting.getKey() + "]");
395+
throw new SettingsException("Setting is not registered for key [" + setting.getKey() + "]");
399396
}
400397
}
401398

@@ -411,7 +408,7 @@ public synchronized <T> void addAffixMapUpdateConsumer(
411408
) {
412409
final Setting<?> registeredSetting = this.complexMatchers.get(setting.getKey());
413410
if (setting != registeredSetting) {
414-
throw new InvalidArgumentException("Setting is not registered for key [" + setting.getKey() + "]");
411+
throw new SettingsException("Setting is not registered for key [" + setting.getKey() + "]");
415412
}
416413
addSettingsUpdater(setting.newAffixMapUpdater(consumer, logger, validator));
417414
}
@@ -444,10 +441,10 @@ public synchronized <A, B> void addSettingsUpdateConsumer(
444441
BiConsumer<A, B> validator
445442
) {
446443
if (a != get(a.getKey())) {
447-
throw new InvalidArgumentException("Setting is not registered for key [" + a.getKey() + "]");
444+
throw new SettingsException("Setting is not registered for key [" + a.getKey() + "]");
448445
}
449446
if (b != get(b.getKey())) {
450-
throw new InvalidArgumentException("Setting is not registered for key [" + b.getKey() + "]");
447+
throw new SettingsException("Setting is not registered for key [" + b.getKey() + "]");
451448
}
452449
addSettingsUpdater(Setting.compoundUpdater(consumer, validator, a, b, logger));
453450
}
@@ -544,7 +541,7 @@ public final void validate(
544541
* @param key the key of the setting to validate
545542
* @param settings the settings
546543
* @param validateDependencies true if dependent settings should be validated
547-
* @throws IllegalArgumentException if the setting is invalid
544+
* @throws SettingsException if the setting is invalid
548545
*/
549546
void validate(final String key, final Settings settings, final boolean validateDependencies) {
550547
validate(key, settings, validateDependencies, false);
@@ -557,7 +554,7 @@ void validate(final String key, final Settings settings, final boolean validateD
557554
* @param settings the settings
558555
* @param validateDependencies true if dependent settings should be validated
559556
* @param validateInternalOrPrivateIndex true if internal index settings should be validated
560-
* @throws IllegalArgumentException if the setting is invalid
557+
* @throws SettingsException if the setting is invalid
561558
*/
562559
void validate(
563560
final String key,
@@ -589,7 +586,7 @@ void validate(
589586
msg += " please check that any required plugins are installed, or check the breaking changes documentation for removed "
590587
+ "settings";
591588
}
592-
throw new InvalidArgumentException(msg);
589+
throw new SettingsException(msg);
593590
} else {
594591
Set<Setting.SettingDependency> settingsDependencies = setting.getSettingsDependencies(key);
595592
if (setting.hasComplexMatcher()) {
@@ -606,7 +603,7 @@ void validate(
606603
dependency.getKey(),
607604
setting.getKey()
608605
);
609-
throw new InvalidArgumentException(message);
606+
throw new SettingsException(message);
610607
}
611608
// validate the dependent setting value
612609
settingDependency.validate(setting.getKey(), setting.get(settings), dependency.get(settings));
@@ -615,11 +612,11 @@ void validate(
615612
// the only time that validateInternalOrPrivateIndex should be true is if this call is coming via the update settings API
616613
if (validateInternalOrPrivateIndex) {
617614
if (setting.isInternalIndex()) {
618-
throw new InvalidArgumentException(
615+
throw new SettingsException(
619616
"can not update internal setting [" + setting.getKey() + "]; this setting is managed via a dedicated API"
620617
);
621618
} else if (setting.isPrivateIndex()) {
622-
throw new InvalidArgumentException(
619+
throw new SettingsException(
623620
"can not update private setting [" + setting.getKey() + "]; this setting is managed by OpenSearch"
624621
);
625622
}
@@ -766,12 +763,12 @@ public Settings diff(Settings source, Settings defaultSettings) {
766763
*/
767764
public <T> T get(Setting<T> setting) {
768765
if (setting.getProperties().contains(scope) == false) {
769-
throw new InvalidArgumentException(
766+
throw new SettingsException(
770767
"settings scope doesn't match the setting scope [" + this.scope + "] not in [" + setting.getProperties() + "]"
771768
);
772769
}
773770
if (get(setting.getKey()) == null) {
774-
throw new InvalidArgumentException("setting " + setting.getKey() + " has not been registered");
771+
throw new SettingsException("setting " + setting.getKey() + " has not been registered");
775772
}
776773
return setting.get(this.lastSettingsApplied, settings);
777774
}
@@ -780,7 +777,7 @@ public <T> T get(Setting<T> setting) {
780777
* Updates a target settings builder with new, updated or deleted settings from a given settings builder.
781778
* <p>
782779
* Note: This method will only allow updates to dynamic settings. if a non-dynamic setting is updated an
783-
* {@link IllegalArgumentException} is thrown instead.
780+
* {@link SettingsException} is thrown instead.
784781
* </p>
785782
*
786783
* @param toApply the new settings to apply
@@ -844,17 +841,17 @@ private boolean updateSettings(Settings toApply, Settings.Builder target, Settin
844841
toRemove.add(key);
845842
// we don't set changed here it's set after we apply deletes below if something actually changed
846843
} else if (get(key) == null) {
847-
throw new InvalidArgumentException(type + " setting [" + key + "], not recognized");
844+
throw new SettingsException(type + " setting [" + key + "], not recognized");
848845
} else if (isDelete == false && canUpdate.test(key)) {
849846
get(key).validateWithoutDependencies(toApply); // we might not have a full picture here do to a dependency validation
850847
settingsBuilder.copy(key, toApply);
851848
updates.copy(key, toApply);
852849
changed |= toApply.get(key).equals(target.get(key)) == false;
853850
} else {
854851
if (isFinalSetting(key)) {
855-
throw new InvalidArgumentException("final " + type + " setting [" + key + "], not updateable");
852+
throw new SettingsException("final " + type + " setting [" + key + "], not updateable");
856853
} else {
857-
throw new InvalidArgumentException(type + " setting [" + key + "], not dynamically updateable");
854+
throw new SettingsException(type + " setting [" + key + "], not dynamically updateable");
858855
}
859856
}
860857
}

‎server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
202202
Map<String, Settings> groups = s.getAsGroups();
203203
for (String key : SimilarityService.BUILT_IN.keySet()) {
204204
if (groups.containsKey(key)) {
205-
throw new IllegalArgumentException(
205+
throw new SettingsException(
206206
"illegal value for [index.similarity." + key + "] cannot redefine built-in similarity"
207207
);
208208
}
@@ -247,7 +247,7 @@ public IndexScopedSettings copy(Settings settings, IndexMetadata metadata) {
247247
@Override
248248
protected void validateSettingKey(Setting setting) {
249249
if (setting.getKey().startsWith("index.") == false) {
250-
throw new IllegalArgumentException("illegal settings key: [" + setting.getKey() + "] must start with [index.]");
250+
throw new SettingsException("illegal settings key: [" + setting.getKey() + "] must start with [index.]");
251251
}
252252
super.validateSettingKey(setting);
253253
}

‎server/src/main/java/org/opensearch/common/settings/Setting.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -496,15 +496,10 @@ private T get(Settings settings, boolean validate) {
496496
}
497497
return parsed;
498498
} catch (OpenSearchParseException ex) {
499-
throw new IllegalArgumentException(ex.getMessage(), ex);
500-
} catch (NumberFormatException ex) {
499+
throw new SettingsException(ex.getMessage(), ex);
500+
} catch (Exception ex) {
501501
String err = "Failed to parse value" + (isFiltered() ? "" : " [" + value + "]") + " for setting [" + getKey() + "]";
502-
throw new IllegalArgumentException(err, ex);
503-
} catch (IllegalArgumentException ex) {
504-
throw ex;
505-
} catch (Exception t) {
506-
String err = "Failed to parse value" + (isFiltered() ? "" : " [" + value + "]") + " for setting [" + getKey() + "]";
507-
throw new IllegalArgumentException(err, t);
502+
throw new SettingsException(err, ex);
508503
}
509504
}
510505

‎server/src/main/java/org/opensearch/common/settings/SettingsException.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import org.opensearch.OpenSearchException;
3636
import org.opensearch.common.io.stream.StreamInput;
37+
import org.opensearch.rest.RestStatus;
3738

3839
import java.io.IOException;
3940

@@ -59,4 +60,9 @@ public SettingsException(StreamInput in) throws IOException {
5960
public SettingsException(String msg, Object... args) {
6061
super(msg, args);
6162
}
63+
64+
@Override
65+
public RestStatus status() {
66+
return RestStatus.BAD_REQUEST;
67+
}
6268
}

‎server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.opensearch.common.settings.ClusterSettings;
4646
import org.opensearch.common.settings.IndexScopedSettings;
4747
import org.opensearch.common.settings.Settings;
48+
import org.opensearch.common.settings.SettingsException;
4849
import org.opensearch.common.unit.TimeValue;
4950
import org.opensearch.common.xcontent.LoggingDeprecationHandler;
5051
import org.opensearch.common.xcontent.NamedXContentRegistry;
@@ -378,8 +379,8 @@ public void testAddComponentTemplate() throws Exception {
378379
assertThat(state.metadata().componentTemplates().get("foo"), equalTo(componentTemplate));
379380

380381
final ClusterState throwState = ClusterState.builder(state).build();
381-
IllegalArgumentException e = expectThrows(
382-
IllegalArgumentException.class,
382+
SettingsException e = expectThrows(
383+
SettingsException.class,
383384
() -> metadataIndexTemplateService.addComponentTemplate(throwState, true, "foo", componentTemplate)
384385
);
385386
assertThat(e.getMessage(), containsString("component template [foo] already exists"));
@@ -416,7 +417,7 @@ public void testAddComponentTemplate() throws Exception {
416417
);
417418
ComponentTemplate componentTemplate4 = new ComponentTemplate(template, 1L, new HashMap<>());
418419
expectThrows(
419-
IllegalArgumentException.class,
420+
SettingsException.class,
420421
() -> metadataIndexTemplateService.addComponentTemplate(throwState, true, "foo2", componentTemplate4)
421422
);
422423
}

0 commit comments

Comments
 (0)