Skip to content

Commit

Permalink
Move validation exception to enum from setting defn
Browse files Browse the repository at this point in the history
Signed-off-by: Rishabh Maurya <[email protected]>
  • Loading branch information
rishabhmaurya committed Sep 28, 2023
1 parent 4ab50d3 commit 8686dbe
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 35 deletions.
38 changes: 13 additions & 25 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,12 @@ public static IndexMergePolicy fromString(String text) {
return policy;
}
}
return null;
throw new IllegalArgumentException(
"The setting has unsupported policy specified: "
+ text
+ ". Please use one of: "
+ String.join(", ", Arrays.stream(IndexMergePolicy.values()).map(IndexMergePolicy::getValue).toArray(String[]::new))
);
}
}

Expand Down Expand Up @@ -601,34 +606,17 @@ public static IndexMergePolicy fromString(String text) {
Property.Dynamic
);

public static final Setting<String> INDEX_MERGE_POLICY = Setting.simpleString("index.merge.policy", DEFAULT_POLICY, policy -> {
if (IndexMergePolicy.fromString(policy) == null) {
throw new IllegalArgumentException(
"The "
+ IndexSettings.INDEX_MERGE_POLICY.getKey()
+ " has unsupported policy specified: "
+ policy
+ ". Please use one of: "
+ String.join(", ", Arrays.stream(IndexMergePolicy.values()).map(IndexMergePolicy::getValue).toArray(String[]::new))
);
}
}, Property.IndexScope);
public static final Setting<String> INDEX_MERGE_POLICY = Setting.simpleString(
"index.merge.policy",
DEFAULT_POLICY,
IndexMergePolicy::fromString,
Property.IndexScope
);

public static final Setting<String> TIME_INDEX_MERGE_POLICY = Setting.simpleString(
"indices.time_index.default_index_merge_policy",
DEFAULT_POLICY,
policy -> {
if (IndexMergePolicy.fromString(policy) == null) {
throw new IllegalArgumentException(
"The "
+ IndexSettings.TIME_INDEX_MERGE_POLICY.getKey()
+ " has unsupported policy specified: "
+ policy
+ ". Please use one of: "
+ String.join(", ", Arrays.stream(IndexMergePolicy.values()).map(IndexMergePolicy::getValue).toArray(String[]::new))
);
}
},
IndexMergePolicy::fromString,
Property.NodeScope
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,31 +188,25 @@ public void testInvalidMergePolicy() throws IOException {
IllegalArgumentException.class,
() -> IndexSettings.INDEX_MERGE_POLICY.get(invalidSettings)
);
assertThat(exc1.getMessage(), containsString(IndexSettings.INDEX_MERGE_POLICY.getKey() + " has unsupported policy specified: "));
assertThat(exc1.getMessage(), containsString(" has unsupported policy specified: "));
IllegalArgumentException exc2 = expectThrows(
IllegalArgumentException.class,
() -> indexSettings(invalidSettings).getMergePolicy(false)
);
assertThat(exc2.getMessage(), containsString(IndexSettings.INDEX_MERGE_POLICY.getKey() + " has unsupported policy specified: "));
assertThat(exc2.getMessage(), containsString(" has unsupported policy specified: "));

final Settings invalidSettings2 = Settings.builder().put(IndexSettings.TIME_INDEX_MERGE_POLICY.getKey(), "invalid").build();
IllegalArgumentException exc3 = expectThrows(
IllegalArgumentException.class,
() -> IndexSettings.TIME_INDEX_MERGE_POLICY.get(invalidSettings2)
);
assertThat(
exc3.getMessage(),
containsString(IndexSettings.TIME_INDEX_MERGE_POLICY.getKey() + " has unsupported policy specified: ")
);
assertThat(exc3.getMessage(), containsString(" has unsupported policy specified: "));

IllegalArgumentException exc4 = expectThrows(
IllegalArgumentException.class,
() -> new IndexSettings(newIndexMeta("test", Settings.EMPTY), invalidSettings2).getMergePolicy(true)
);
assertThat(
exc4.getMessage(),
containsString(IndexSettings.TIME_INDEX_MERGE_POLICY.getKey() + " has unsupported policy specified: ")
);
assertThat(exc4.getMessage(), containsString(" has unsupported policy specified: "));
}

public void testUpdateSettingsForLogByteSizeMergePolicy() throws IOException {
Expand Down

0 comments on commit 8686dbe

Please sign in to comment.