Allow ObjectParsers to specify required sets of fields (#49661)
ConstructingObjectParser can be used to specify required fields, but it is still difficult to configure "sets" of fields where only one of the set is required (requiring hand-rolled logic in each ConstructingObjectParser, or adding special validation methods to objects that are called after building the object). This commit adds a new method on ObjectParser which allows the parsers to register required sets. E.g. ["foo", "bar"] can be registered, which means "foo", "bar" or both must be configured by the user otherwise an exception is thrown. This pattern crops up in many places in our parsers; a good example are the aggregation "field" and "script" fields. One or both must be configured on all aggregations, omitting both should result in an exception. This was previously handled far downstream resulting in an aggregation exception, when it should be a parse exception.
This commit is contained in:
parent
86d5211c05
commit
0372d6d239
|
@ -37,6 +37,8 @@ import java.util.function.Consumer;
|
||||||
public abstract class AbstractObjectParser<Value, Context>
|
public abstract class AbstractObjectParser<Value, Context>
|
||||||
implements BiFunction<XContentParser, Context, Value>, ContextParser<Context, Value> {
|
implements BiFunction<XContentParser, Context, Value>, ContextParser<Context, Value> {
|
||||||
|
|
||||||
|
final List<String[]> requiredFieldSets = new ArrayList<>();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Declare some field. Usually it is easier to use {@link #declareString(BiConsumer, ParseField)} or
|
* Declare some field. Usually it is easier to use {@link #declareString(BiConsumer, ParseField)} or
|
||||||
* {@link #declareObject(BiConsumer, ContextParser, ParseField)} rather than call this directly.
|
* {@link #declareObject(BiConsumer, ContextParser, ParseField)} rather than call this directly.
|
||||||
|
@ -211,6 +213,61 @@ public abstract class AbstractObjectParser<Value, Context>
|
||||||
declareField(consumer, (p, c) -> parseArray(p, () -> itemParser.parse(p, c)), field, type);
|
declareField(consumer, (p, c) -> parseArray(p, () -> itemParser.parse(p, c)), field, type);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Declares a set of fields that are required for parsing to succeed. Only one of the values
|
||||||
|
* provided per String[] must be matched.
|
||||||
|
*
|
||||||
|
* E.g. <code>declareRequiredFieldSet("foo", "bar");</code> means at least one of "foo" or
|
||||||
|
* "bar" fields must be present. If neither of those fields are present, an exception will be thrown.
|
||||||
|
*
|
||||||
|
* Multiple required sets can be configured:
|
||||||
|
*
|
||||||
|
* <pre><code>
|
||||||
|
* parser.declareRequiredFieldSet("foo", "bar");
|
||||||
|
* parser.declareRequiredFieldSet("bizz", "buzz");
|
||||||
|
* </code></pre>
|
||||||
|
*
|
||||||
|
* requires that one of "foo" or "bar" fields are present, and also that one of "bizz" or
|
||||||
|
* "buzz" fields are present.
|
||||||
|
*
|
||||||
|
* In JSON, it means any of these combinations are acceptable:
|
||||||
|
*
|
||||||
|
* <ul>
|
||||||
|
* <li><code>{"foo":"...", "bizz": "..."}</code></li>
|
||||||
|
* <li><code>{"bar":"...", "bizz": "..."}</code></li>
|
||||||
|
* <li><code>{"foo":"...", "buzz": "..."}</code></li>
|
||||||
|
* <li><code>{"bar":"...", "buzz": "..."}</code></li>
|
||||||
|
* <li><code>{"foo":"...", "bar":"...", "bizz": "..."}</code></li>
|
||||||
|
* <li><code>{"foo":"...", "bar":"...", "buzz": "..."}</code></li>
|
||||||
|
* <li><code>{"foo":"...", "bizz":"...", "buzz": "..."}</code></li>
|
||||||
|
* <li><code>{"bar":"...", "bizz":"...", "buzz": "..."}</code></li>
|
||||||
|
* <li><code>{"foo":"...", "bar":"...", "bizz": "...", "buzz": "..."}</code></li>
|
||||||
|
* </ul>
|
||||||
|
*
|
||||||
|
* The following would however be rejected:
|
||||||
|
*
|
||||||
|
* <table>
|
||||||
|
* <caption>failure cases</caption>
|
||||||
|
* <tr><th>Provided JSON</th><th>Reason for failure</th></tr>
|
||||||
|
* <tr><td><code>{"foo":"..."}</code></td><td>Missing "bizz" or "buzz" field</td></tr>
|
||||||
|
* <tr><td><code>{"bar":"..."}</code></td><td>Missing "bizz" or "buzz" field</td></tr>
|
||||||
|
* <tr><td><code>{"bizz": "..."}</code></td><td>Missing "foo" or "bar" field</td></tr>
|
||||||
|
* <tr><td><code>{"buzz": "..."}</code></td><td>Missing "foo" or "bar" field</td></tr>
|
||||||
|
* <tr><td><code>{"foo":"...", "bar": "..."}</code></td><td>Missing "bizz" or "buzz" field</td></tr>
|
||||||
|
* <tr><td><code>{"bizz":"...", "buzz": "..."}</code></td><td>Missing "foo" or "bar" field</td></tr>
|
||||||
|
* <tr><td><code>{"unrelated":"..."}</code></td> <td>Missing "foo" or "bar" field, and missing "bizz" or "buzz" field</td></tr>
|
||||||
|
* </table>
|
||||||
|
*
|
||||||
|
* @param requiredSet
|
||||||
|
* A set of required fields, where at least one of the fields in the array _must_ be present
|
||||||
|
*/
|
||||||
|
public void declareRequiredFieldSet(String... requiredSet) {
|
||||||
|
if (requiredSet.length == 0) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
this.requiredFieldSets.add(requiredSet);
|
||||||
|
}
|
||||||
|
|
||||||
private interface IOSupplier<T> {
|
private interface IOSupplier<T> {
|
||||||
T get() throws IOException;
|
T get() throws IOException;
|
||||||
}
|
}
|
||||||
|
|
|
@ -28,6 +28,7 @@ import java.util.Arrays;
|
||||||
import java.util.EnumSet;
|
import java.util.EnumSet;
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
|
import java.util.Iterator;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
|
@ -272,6 +273,8 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
|
||||||
FieldParser fieldParser = null;
|
FieldParser fieldParser = null;
|
||||||
String currentFieldName = null;
|
String currentFieldName = null;
|
||||||
XContentLocation currentPosition = null;
|
XContentLocation currentPosition = null;
|
||||||
|
List<String[]> requiredFields = new ArrayList<>(this.requiredFieldSets);
|
||||||
|
|
||||||
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
|
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
|
||||||
if (token == XContentParser.Token.FIELD_NAME) {
|
if (token == XContentParser.Token.FIELD_NAME) {
|
||||||
currentFieldName = parser.currentName();
|
currentFieldName = parser.currentName();
|
||||||
|
@ -285,11 +288,32 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
|
||||||
unknownFieldParser.acceptUnknownField(this, currentFieldName, currentPosition, parser, value, context);
|
unknownFieldParser.acceptUnknownField(this, currentFieldName, currentPosition, parser, value, context);
|
||||||
} else {
|
} else {
|
||||||
fieldParser.assertSupports(name, parser, currentFieldName);
|
fieldParser.assertSupports(name, parser, currentFieldName);
|
||||||
|
|
||||||
|
// Check to see if this field is a required field, if it is we can
|
||||||
|
// remove the entry as the requirement is satisfied
|
||||||
|
Iterator<String[]> iter = requiredFields.iterator();
|
||||||
|
while (iter.hasNext()) {
|
||||||
|
String[] requriedFields = iter.next();
|
||||||
|
for (String field : requriedFields) {
|
||||||
|
if (field.equals(currentFieldName)) {
|
||||||
|
iter.remove();
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
parseSub(parser, fieldParser, currentFieldName, value, context);
|
parseSub(parser, fieldParser, currentFieldName, value, context);
|
||||||
}
|
}
|
||||||
fieldParser = null;
|
fieldParser = null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (requiredFields.isEmpty() == false) {
|
||||||
|
StringBuilder message = new StringBuilder();
|
||||||
|
for (String[] fields : requiredFields) {
|
||||||
|
message.append("Required one of fields ").append(Arrays.toString(fields)).append(", but none were specified. ");
|
||||||
|
}
|
||||||
|
throw new IllegalArgumentException(message.toString());
|
||||||
|
}
|
||||||
return value;
|
return value;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -43,6 +43,7 @@ import static org.hamcrest.Matchers.containsString;
|
||||||
import static org.hamcrest.Matchers.equalTo;
|
import static org.hamcrest.Matchers.equalTo;
|
||||||
import static org.hamcrest.Matchers.hasSize;
|
import static org.hamcrest.Matchers.hasSize;
|
||||||
import static org.hamcrest.Matchers.instanceOf;
|
import static org.hamcrest.Matchers.instanceOf;
|
||||||
|
import static org.hamcrest.Matchers.nullValue;
|
||||||
|
|
||||||
public class ObjectParserTests extends ESTestCase {
|
public class ObjectParserTests extends ESTestCase {
|
||||||
|
|
||||||
|
@ -757,6 +758,89 @@ public class ObjectParserTests extends ESTestCase {
|
||||||
assertThat(o.fields.get("test_nested"), instanceOf(Map.class));
|
assertThat(o.fields.get("test_nested"), instanceOf(Map.class));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public void testRequiredFieldSet() throws IOException {
|
||||||
|
class TestStruct {
|
||||||
|
private Long a;
|
||||||
|
private Long b;
|
||||||
|
|
||||||
|
private void setA(long value) {
|
||||||
|
this.a = value;
|
||||||
|
}
|
||||||
|
|
||||||
|
private void setB(long value) {
|
||||||
|
this.b = value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"a\": \"123\"}");
|
||||||
|
ObjectParser<TestStruct, Void> objectParser = new ObjectParser<>("foo", true, TestStruct::new);
|
||||||
|
objectParser.declareLong(TestStruct::setA, new ParseField("a"));
|
||||||
|
objectParser.declareLong(TestStruct::setB, new ParseField("b"));
|
||||||
|
objectParser.declareRequiredFieldSet(new String[]{"a", "b"});
|
||||||
|
|
||||||
|
TestStruct obj = objectParser.apply(parser, null);
|
||||||
|
assertThat(obj.a, equalTo(123L));
|
||||||
|
assertThat(obj.b, nullValue());
|
||||||
|
|
||||||
|
parser = createParser(JsonXContent.jsonXContent, "{\"b\": \"123\"}");
|
||||||
|
objectParser = new ObjectParser<>("foo", true, TestStruct::new);
|
||||||
|
objectParser.declareLong(TestStruct::setA, new ParseField("a"));
|
||||||
|
objectParser.declareLong(TestStruct::setB, new ParseField("b"));
|
||||||
|
objectParser.declareRequiredFieldSet(new String[]{"a", "b"});
|
||||||
|
|
||||||
|
obj = objectParser.apply(parser, null);
|
||||||
|
assertThat(obj.a, nullValue());
|
||||||
|
assertThat(obj.b, equalTo(123L));
|
||||||
|
|
||||||
|
parser = createParser(JsonXContent.jsonXContent, "{\"a\": \"123\", \"b\": \"456\"}");
|
||||||
|
objectParser = new ObjectParser<>("foo", true, TestStruct::new);
|
||||||
|
objectParser.declareLong(TestStruct::setA, new ParseField("a"));
|
||||||
|
objectParser.declareLong(TestStruct::setB, new ParseField("b"));
|
||||||
|
objectParser.declareRequiredFieldSet(new String[]{"a", "b"});
|
||||||
|
|
||||||
|
obj = objectParser.apply(parser, null);
|
||||||
|
assertThat(obj.a, equalTo(123L));
|
||||||
|
assertThat(obj.b, equalTo(456L));
|
||||||
|
}
|
||||||
|
|
||||||
|
public void testMultipleRequiredFieldSet() throws IOException {
|
||||||
|
class TestStruct {
|
||||||
|
private Long a;
|
||||||
|
private Long b;
|
||||||
|
private Long c;
|
||||||
|
private Long d;
|
||||||
|
|
||||||
|
private void setA(long value) {
|
||||||
|
this.a = value;
|
||||||
|
}
|
||||||
|
|
||||||
|
private void setB(long value) {
|
||||||
|
this.b = value;
|
||||||
|
}
|
||||||
|
|
||||||
|
private void setC(long value) {
|
||||||
|
this.c = value;
|
||||||
|
}
|
||||||
|
|
||||||
|
private void setD(long value) {
|
||||||
|
this.d = value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"unrelated\": \"123\"}");
|
||||||
|
ObjectParser<TestStruct, Void> objectParser = new ObjectParser<>("foo", true, TestStruct::new);
|
||||||
|
objectParser.declareLong(TestStruct::setA, new ParseField("a"));
|
||||||
|
objectParser.declareLong(TestStruct::setB, new ParseField("b"));
|
||||||
|
objectParser.declareLong(TestStruct::setC, new ParseField("c"));
|
||||||
|
objectParser.declareLong(TestStruct::setD, new ParseField("d"));
|
||||||
|
objectParser.declareRequiredFieldSet(new String[]{"a", "b"});
|
||||||
|
objectParser.declareRequiredFieldSet(new String[]{"c", "d"});
|
||||||
|
|
||||||
|
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> objectParser.apply(parser, null));
|
||||||
|
assertThat(e.getMessage(), equalTo("Required one of fields [a, b], but none were specified. " +
|
||||||
|
"Required one of fields [c, d], but none were specified. "));
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected NamedXContentRegistry xContentRegistry() {
|
protected NamedXContentRegistry xContentRegistry() {
|
||||||
return new NamedXContentRegistry(Arrays.asList(
|
return new NamedXContentRegistry(Arrays.asList(
|
||||||
|
|
Loading…
Reference in New Issue