Skip to content

Commit

Permalink
Improve resiliency to auto-formatting in libs, modules (#48619)
Browse files Browse the repository at this point in the history 
Backport of #48448. Make a number of changes so that code in the libs and
modules directories are more resilient to automatic formatting. This covers:

* Remove string concatenation where JSON fits on a single line
* Move some comments around to they aren't auto-formatted to a strange
  place
  • Loading branch information
@pugnascotia
pugnascotia authored Oct 29, 2019
1 parent 028084c commit 3c77c50
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 161 deletions.
179 changes: 57 additions & 122 deletions ...ontent/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,7 @@ public void testRandomOrder() throws Exception {
}

public void testMissingAllConstructorArgs() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"mineral\": 1\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1 }");
ConstructingObjectParser<HasCtorArguments, Void> objectParser = randomBoolean() ? HasCtorArguments.PARSER
: HasCtorArguments.PARSER_VEGETABLE_OPTIONAL;
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> objectParser.apply(parser, null));
Expand All @@ -108,31 +105,20 @@ public void testMissingAllConstructorArgs() throws IOException {
}

public void testMissingAllConstructorArgsButNotRequired() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"mineral\": 1\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1 }");
HasCtorArguments parsed = HasCtorArguments.PARSER_ALL_OPTIONAL.apply(parser, null);
assertEquals(1, parsed.mineral);
}

public void testMissingSecondConstructorArg() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"mineral\": 1,\n"
+ " \"animal\": \"cat\"\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"animal\": \"cat\" }");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> HasCtorArguments.PARSER.apply(parser, null));
assertEquals("Required [vegetable]", e.getMessage());
}

public void testMissingSecondConstructorArgButNotRequired() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"mineral\": 1,\n"
+ " \"animal\": \"cat\"\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"animal\": \"cat\" }");
@SuppressWarnings("unchecked")
HasCtorArguments parsed = randomFrom(HasCtorArguments.PARSER_VEGETABLE_OPTIONAL, HasCtorArguments.PARSER_ALL_OPTIONAL).apply(parser,
null);
Expand All @@ -141,35 +127,27 @@ public void testMissingSecondConstructorArgButNotRequired() throws IOException {
}

public void testMissingFirstConstructorArg() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"mineral\": 1,\n"
+ " \"vegetable\": 2\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"vegetable\": 2 }");
@SuppressWarnings("unchecked")
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> randomFrom(HasCtorArguments.PARSER, HasCtorArguments.PARSER_VEGETABLE_OPTIONAL).apply(parser, null));
assertEquals("Required [animal]", e.getMessage());
}

public void testMissingFirstConstructorArgButNotRequired() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"mineral\": 1,\n"
+ " \"vegetable\": 2\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"vegetable\": 2 }");
HasCtorArguments parsed = HasCtorArguments.PARSER_ALL_OPTIONAL.apply(parser, null);
assertEquals(1, parsed.mineral);
assertEquals((Integer) 2, parsed.vegetable);
}

public void testBadParam() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"animal\": \"cat\",\n"
+ " \"vegetable\": 2,\n"
+ " \"a\": \"supercalifragilisticexpialidocious\"\n"
+ "}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
// The following JSON needs to include newlines, in order to affect the line numbers
// included in the exception
"{\n" + " \"animal\": \"cat\",\n" + " \"vegetable\": 2,\n" + " \"a\": \"supercalifragilisticexpialidocious\"\n" + "}"
);
XContentParseException e = expectThrows(XContentParseException.class,
() -> randomFrom(HasCtorArguments.ALL_PARSERS).apply(parser, null));
assertThat(e.getMessage(), containsString("[has_required_arguments] failed to parse field [a]"));
Expand All @@ -179,12 +157,12 @@ public void testBadParam() throws IOException {
}

public void testBadParamBeforeObjectBuilt() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"a\": \"supercalifragilisticexpialidocious\",\n"
+ " \"animal\": \"cat\"\n,"
+ " \"vegetable\": 2\n"
+ "}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
// The following JSON needs to include newlines, in order to affect the line numbers
// included in the exception
"{\n" + " \"a\": \"supercalifragilisticexpialidocious\",\n" + " \"animal\": \"cat\"\n," + " \"vegetable\": 2\n" + "}"
);
XContentParseException e = expectThrows(XContentParseException.class,
() -> randomFrom(HasCtorArguments.ALL_PARSERS).apply(parser, null));
assertThat(e.getMessage(), containsString("[has_required_arguments] failed to parse field [vegetable]"));
Expand Down Expand Up @@ -240,40 +218,25 @@ void setFoo(String foo) {
parser.declareString(ctorArgOptional ? optionalConstructorArg() : constructorArg(), new ParseField("yeah"));

// ctor arg first so we can test for the bug we found one time
XContentParser xcontent = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"yeah\": \"!\",\n"
+ " \"foo\": \"foo\"\n"
+ "}");
XContentParser xcontent = createParser(JsonXContent.jsonXContent, "{ \"yeah\": \"!\", \"foo\": \"foo\" }");
CalledOneTime result = parser.apply(xcontent, null);
assertTrue(result.fooSet);

// and ctor arg second just in case
xcontent = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"foo\": \"foo\",\n"
+ " \"yeah\": \"!\"\n"
+ "}");
xcontent = createParser(JsonXContent.jsonXContent, "{ \"foo\": \"foo\", \"yeah\": \"!\" }");
result = parser.apply(xcontent, null);
assertTrue(result.fooSet);

if (ctorArgOptional) {
// and without the constructor arg if we've made it optional
xcontent = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"foo\": \"foo\"\n"
+ "}");
xcontent = createParser(JsonXContent.jsonXContent, "{ \"foo\": \"foo\" }");
result = parser.apply(xcontent, null);
}
assertTrue(result.fooSet);
}

public void testIgnoreUnknownFields() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"test\" : \"foo\",\n"
+ " \"junk\" : 2\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"test\" : \"foo\", \"junk\" : 2 }");
class TestStruct {
public final String test;
TestStruct(String test) {
Expand All @@ -288,11 +251,7 @@ class TestStruct {
}

public void testConstructObjectUsingContext() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"animal\": \"dropbear\",\n"
+ " \"mineral\": -8\n"
+ "}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"animal\": \"dropbear\", \"mineral\": -8 }");
HasCtorArguments parsed = HasCtorArguments.PARSER_INT_CONTEXT.apply(parser, 42);
assertEquals(Integer.valueOf(42), parsed.vegetable);
assertEquals("dropbear", parsed.animal);
Expand Down Expand Up @@ -412,12 +371,10 @@ private static void declareSetters(ConstructingObjectParser<HasCtorArguments, ?>
}

public void testParseNamedObject() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": {\n"
+ " \"a\": {}"
+ "},\"named_in_constructor\": {\n"
+ " \"b\": {}"
+ "}}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": { \"a\": {} }, \"named_in_constructor\": { \"b\": {} } }"
);
NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null);
assertThat(h.named, hasSize(1));
assertEquals("a", h.named.get(0).name);
Expand All @@ -427,12 +384,10 @@ public void testParseNamedObject() throws IOException {
}

public void testParseNamedObjectInOrder() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}}"
+ "],\"named_in_constructor\": [\n"
+ " {\"b\": {}}"
+ "]}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": [ {\"a\": {}} ], \"named_in_constructor\": [ {\"b\": {}} ]}"
);
NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null);
assertThat(h.named, hasSize(1));
assertEquals("a", h.named.get(0).name);
Expand All @@ -442,12 +397,10 @@ public void testParseNamedObjectInOrder() throws IOException {
}

public void testParseNamedObjectTwoFieldsInArray() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}, \"b\": {}}"
+ "],\"named_in_constructor\": [\n"
+ " {\"c\": {}}"
+ "]}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": [ {\"a\": {}, \"b\": {}}], \"named_in_constructor\": [ {\"c\": {}} ]}"
);
XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null));
assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]"));
assertThat(e.getCause().getMessage(),
Expand All @@ -456,12 +409,10 @@ public void testParseNamedObjectTwoFieldsInArray() throws IOException {
}

public void testParseNamedObjectTwoFieldsInArrayConstructorArg() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}}"
+ "],\"named_in_constructor\": [\n"
+ " {\"c\": {}, \"d\": {}}"
+ "]}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": [ {\"a\": {}}], \"named_in_constructor\": [ {\"c\": {}, \"d\": {}} ]}"
);
XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null));
assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named_in_constructor]"));
assertThat(e.getCause().getMessage(),
Expand All @@ -470,12 +421,7 @@ public void testParseNamedObjectTwoFieldsInArrayConstructorArg() throws IOExcept
}

public void testParseNamedObjectNoFieldsInArray() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {}"
+ "],\"named_in_constructor\": [\n"
+ " {\"a\": {}}"
+ "]}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {} ], \"named_in_constructor\": [ {\"a\": {}} ]}");
XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null));
assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]"));
assertThat(e.getCause().getMessage(),
Expand All @@ -484,12 +430,7 @@ public void testParseNamedObjectNoFieldsInArray() throws IOException {
}

public void testParseNamedObjectNoFieldsInArrayConstructorArg() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}}"
+ "],\"named_in_constructor\": [\n"
+ " {}"
+ "]}");
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {\"a\": {}} ], \"named_in_constructor\": [ {} ]}");
XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null));
assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named_in_constructor]"));
assertThat(e.getCause().getMessage(),
Expand All @@ -498,12 +439,10 @@ public void testParseNamedObjectNoFieldsInArrayConstructorArg() throws IOExcepti
}

public void testParseNamedObjectJunkInArray() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " \"junk\""
+ "],\"named_in_constructor\": [\n"
+ " {\"a\": {}}"
+ "]}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": [ \"junk\" ], \"named_in_constructor\": [ {\"a\": {}} ]}"
);
XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null));
assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]"));
assertThat(e.getCause().getMessage(),
Expand All @@ -512,12 +451,10 @@ public void testParseNamedObjectJunkInArray() throws IOException {
}

public void testParseNamedObjectJunkInArrayConstructorArg() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}}"
+ "],\"named_in_constructor\": [\n"
+ " \"junk\""
+ "]}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": [ {\"a\": {}} ], \"named_in_constructor\": [ \"junk\" ]}"
);
XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null));
assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named_in_constructor]"));
assertThat(e.getCause().getMessage(),
Expand All @@ -526,11 +463,10 @@ public void testParseNamedObjectJunkInArrayConstructorArg() throws IOException {
}

public void testParseNamedObjectInOrderNotSupported() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}}"
+ "],\"named_in_constructor\": {\"b\": {}}"
+ "}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": [\n" + " {\"a\": {}}" + "],\"named_in_constructor\": {\"b\": {}}" + "}"
);

// Create our own parser for this test so we can disable support for the "ordered" mode specified by the array above
@SuppressWarnings("unchecked")
Expand All @@ -547,11 +483,10 @@ public void testParseNamedObjectInOrderNotSupported() throws IOException {
}

public void testParseNamedObjectInOrderNotSupportedConstructorArg() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": {\"a\": {}}"
+ ",\"named_in_constructor\": [\n"
+ " {\"b\": {}}"
+ "]}");
XContentParser parser = createParser(
JsonXContent.jsonXContent,
"{\"named\": {\"a\": {}}, \"named_in_constructor\": [ {\"b\": {}} ]}"
);

// Create our own parser for this test so we can disable support for the "ordered" mode specified by the array above
@SuppressWarnings("unchecked")
Expand Down
Loading

0 comments on commit 3c77c50

Please sign in to comment.