Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,26 @@ public ServiceException putExtraField(String fieldName, Number value) {
return putExtraFieldInternal(fieldName, value);
}

/**
* Associates to this exception an extra field as a field name/value pair. If a field
* with the same name was previously set, the old value is replaced by the specified
* value.<br>
* This unsafe version accepts any POJO as is:
* <ul>
* <li>the object should be serializable</li>
* <li>no defensive copy nor conversion are made. So {@code value} should not be modified
* or reused after the call of this method.</li>
* </ul>
* These constraints must be taken into consideration when overriding this method.
* @return this
* @throws NullPointerException if {@code fieldName} is {@code null}.
* @throws IllegalArgumentException if {@code fieldName} is one of the reserved field
* names {@link #EXTRA_FIELDS_RESERVED_NAMES}.
*/
protected ServiceException putExtraFieldUnsafe(String fieldName, Object value) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular reason to set this method as protected while the other flavors of putExtraField are public ? This one fulfills exactly the same need as the other.
Same way, suffixing its name by unsafe is really puzzling since it has exactly the same purpose of the others. Plus, this suffix is not really invitation to use it). I would set its signature as the following :

Suggested change
protected ServiceException putExtraFieldUnsafe(String fieldName, Object value) {
public ServiceException putExtraField(String fieldName, Object value) {

return putExtraFieldInternal(fieldName, value);
}

private ServiceException putExtraFieldInternal(String fieldName, Object value) {
Preconditions.checkNotNull(fieldName);
Preconditions.checkArgument(!EXTRA_FIELDS_RESERVED_NAMES.contains(fieldName), "The field name '%s' is reserved", fieldName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
import org.skyscreamer.jsonassert.JSONAssert;
import org.springframework.mock.web.MockHttpServletResponse;

import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* Tests for {@link RestResponseResultWriter}.
*/
Expand Down Expand Up @@ -269,4 +274,79 @@ public void writeError_extraFields() throws Exception {
writer.writeError(serviceException);
JSONAssert.assertEquals(expectedError, response.getContentAsString(), true);
}

@Test
public void writeError_extraFieldsUnsafe() throws Exception {

MockHttpServletResponse response = new MockHttpServletResponse();
RestResponseResultWriter writer = new RestResponseResultWriter(response, null, true /* prettyPrint */,
true /* addContentLength */, true /* enableExceptionCompatibility */);

TestServiceExceptionExtraFieldUnsafe serviceException = new TestServiceExceptionExtraFieldUnsafe(400, "customMessage", "customReason", "customDomain");

// Extra field array
Boolean[] booleans = new Boolean[] { TRUE, FALSE, TRUE };

// Extra field List
List<String> stringList = Arrays.asList("First", "Second", "Last");

// Extra field Map
Map<Integer, TestValue> map = new HashMap<>();
map.put(1, new TestValue("Alice", 7, TestEnum.VALUE1));
map.put(2, new TestValue("Bob", 12, TestEnum.VALUE2));
map.put(3, new TestValue("Clark", 31, TestEnum.VALUE3));

serviceException.putExtraFieldUnsafe("someExtraNull", null)
.putExtraFieldUnsafe("someExtraArray", booleans)
.putExtraFieldUnsafe("someExtraList", stringList)
.putExtraFieldUnsafe("someExtraMap", map);

String expectedError = "{\"error\": {\"errors\": [{" +
" \"domain\": \"customDomain\"," +
" \"reason\": \"customReason\"," +
" \"message\": \"customMessage\"," +
" \"someExtraNull\": null," +
" \"someExtraArray\": [true, false, true]," +
" \"someExtraList\": [\"First\", \"Second\", \"Last\"]," +
" \"someExtraMap\": {" +
" \"1\": {\"name\": \"Alice\", \"age\": 7, \"testEnum\": \"VALUE1\"}," +
" \"2\": {\"name\": \"Bob\", \"age\": 12, \"testEnum\": \"VALUE2\"}," +
" \"3\": {\"name\": \"Clark\", \"age\": 31, \"testEnum\": \"VALUE3\"}" +
" }" +
" }]," +
" \"code\": 400," +
" \"message\": \"customMessage\"" +
"}}";

writer.writeError(serviceException);
JSONAssert.assertEquals(expectedError, response.getContentAsString(), true);
}

enum TestEnum {
VALUE1, VALUE2, VALUE3;
}

class TestValue {
public String name;
public int age;
public TestEnum testEnum;

TestValue(String name, int age, TestEnum testEnum) {
this.name = name;
this.age = age;
this.testEnum = testEnum;
}
}

class TestServiceExceptionExtraFieldUnsafe extends ServiceException {

TestServiceExceptionExtraFieldUnsafe(int statusCode, String statusMessage, String reason, String domain) {
super(statusCode, statusMessage, reason, domain);
}

@Override
public TestServiceExceptionExtraFieldUnsafe putExtraFieldUnsafe(String fieldName, Object value) {
return (TestServiceExceptionExtraFieldUnsafe) super.putExtraFieldUnsafe(fieldName, value);
}
}
}