Correctly set HTTP error status codes when list objects fails

This makes the associated nomulus tool commands correctly return error
exit codes when the server-side component fails. This improves
scriptability.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=140543216
This commit is contained in:
mcilwain 2016-11-29 16:30:19 -08:00 committed by Ben McIlwain
parent b35ac10b7e
commit 5f32d1bbeb
8 changed files with 31 additions and 11 deletions

View file

@ -15,6 +15,8 @@
package google.registry.tools.server; package google.registry.tools.server;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import com.google.common.base.Function; import com.google.common.base.Function;
import com.google.common.base.Functions; import com.google.common.base.Functions;
@ -120,6 +122,8 @@ public abstract class ListObjectsAction<T extends ImmutableObject> implements Ru
if (message == null) { if (message == null) {
message = e.getClass().getName(); message = e.getClass().getName();
} }
response.setStatus(
e instanceof IllegalArgumentException ? SC_BAD_REQUEST : SC_INTERNAL_SERVER_ERROR);
response.setPayload(ImmutableMap.of( response.setPayload(ImmutableMap.of(
"error", message, "error", message,
"status", "error")); "status", "error"));

View file

@ -40,7 +40,7 @@ public class ListActionTestCase {
private FakeJsonResponse response; private FakeJsonResponse response;
void runAction( private void runAction(
ListObjectsAction<?> action, ListObjectsAction<?> action,
Optional<String> fields, Optional<String> fields,
Optional<Boolean> printHeaderRow, Optional<Boolean> printHeaderRow,
@ -51,7 +51,6 @@ public class ListActionTestCase {
action.printHeaderRow = printHeaderRow; action.printHeaderRow = printHeaderRow;
action.fullFieldNames = fullFieldNames; action.fullFieldNames = fullFieldNames;
action.run(); action.run();
assertThat(response.getStatus()).isEqualTo(SC_OK);
} }
void testRunSuccess( void testRunSuccess(
@ -62,6 +61,7 @@ public class ListActionTestCase {
String ... expectedLinePatterns) { String ... expectedLinePatterns) {
assertThat(expectedLinePatterns).isNotNull(); assertThat(expectedLinePatterns).isNotNull();
runAction(action, fields, printHeaderRow, fullFieldNames); runAction(action, fields, printHeaderRow, fullFieldNames);
assertThat(response.getStatus()).isEqualTo(SC_OK);
assertThat(response.getResponseMap().get("status")).isEqualTo("success"); assertThat(response.getResponseMap().get("status")).isEqualTo("success");
Object obj = response.getResponseMap().get("lines"); Object obj = response.getResponseMap().get("lines");
assertThat(obj).isInstanceOf(List.class); assertThat(obj).isInstanceOf(List.class);
@ -78,9 +78,11 @@ public class ListActionTestCase {
Optional<String> fields, Optional<String> fields,
Optional<Boolean> printHeaderRow, Optional<Boolean> printHeaderRow,
Optional<Boolean> fullFieldNames, Optional<Boolean> fullFieldNames,
String expectedErrorPattern) { String expectedErrorPattern,
int expectedResponseStatus) {
assertThat(expectedErrorPattern).isNotNull(); assertThat(expectedErrorPattern).isNotNull();
runAction(action, fields, printHeaderRow, fullFieldNames); runAction(action, fields, printHeaderRow, fullFieldNames);
assertThat(response.getStatus()).isEqualTo(expectedResponseStatus);
assertThat(response.getResponseMap().get("status")).isEqualTo("error"); assertThat(response.getResponseMap().get("status")).isEqualTo("error");
Object obj = response.getResponseMap().get("error"); Object obj = response.getResponseMap().get("error");
assertThat(obj).isInstanceOf(String.class); assertThat(obj).isInstanceOf(String.class);

View file

@ -17,6 +17,7 @@ package google.registry.tools.server;
import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.createTlds;
import static google.registry.testing.DatastoreHelper.persistActiveDomain; import static google.registry.testing.DatastoreHelper.persistActiveDomain;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
@ -50,7 +51,8 @@ public class ListDomainsActionTest extends ListActionTestCase {
null, null,
null, null,
null, null,
"^Must specify TLDs to query$"); "^Must specify TLDs to query$",
SC_BAD_REQUEST);
} }
@Test @Test
@ -61,7 +63,8 @@ public class ListDomainsActionTest extends ListActionTestCase {
null, null,
null, null,
null, null,
"^TLD %%%badtld%%% does not exist$"); "^TLD %%%badtld%%% does not exist$",
SC_BAD_REQUEST);
} }
@Test @Test
@ -229,6 +232,7 @@ public class ListDomainsActionTest extends ListActionTestCase {
Optional.of("badfield"), Optional.of("badfield"),
null, null,
null, null,
"^Field 'badfield' not found - recognized fields are:"); "^Field 'badfield' not found - recognized fields are:",
SC_BAD_REQUEST);
} }
} }

View file

@ -16,6 +16,7 @@ package google.registry.tools.server;
import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.persistActiveHost; import static google.registry.testing.DatastoreHelper.persistActiveHost;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import google.registry.testing.FakeClock; import google.registry.testing.FakeClock;
@ -103,6 +104,7 @@ public class ListHostsActionTest extends ListActionTestCase {
Optional.of("badfield"), Optional.of("badfield"),
null, null,
null, null,
"^Field 'badfield' not found - recognized fields are:"); "^Field 'badfield' not found - recognized fields are:",
SC_BAD_REQUEST);
} }
} }

View file

@ -15,6 +15,7 @@
package google.registry.tools.server; package google.registry.tools.server;
import static google.registry.testing.DatastoreHelper.persistPremiumList; import static google.registry.testing.DatastoreHelper.persistPremiumList;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import google.registry.model.registry.label.PremiumList; import google.registry.model.registry.label.PremiumList;
@ -86,6 +87,7 @@ public class ListPremiumListsActionTest extends ListActionTestCase {
Optional.of("badfield"), Optional.of("badfield"),
null, null,
null, null,
"^Field 'badfield' not found - recognized fields are:"); "^Field 'badfield' not found - recognized fields are:",
SC_BAD_REQUEST);
} }
} }

View file

@ -16,6 +16,7 @@ package google.registry.tools.server;
import static google.registry.testing.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.createTlds;
import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DatastoreHelper.persistResource;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
@ -95,6 +96,7 @@ public class ListRegistrarsActionTest extends ListActionTestCase {
Optional.of("badfield"), Optional.of("badfield"),
null, null,
null, null,
"^Field 'badfield' not found - recognized fields are:"); "^Field 'badfield' not found - recognized fields are:",
SC_BAD_REQUEST);
} }
} }

View file

@ -17,6 +17,7 @@ package google.registry.tools.server;
import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.persistReservedList; import static google.registry.testing.DatastoreHelper.persistReservedList;
import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DatastoreHelper.persistResource;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import google.registry.model.registry.Registry; import google.registry.model.registry.Registry;
@ -87,6 +88,7 @@ public class ListReservedListsActionTest extends ListActionTestCase {
Optional.of("badfield"), Optional.of("badfield"),
null, null,
null, null,
"^Field 'badfield' not found - recognized fields are:"); "^Field 'badfield' not found - recognized fields are:",
SC_BAD_REQUEST);
} }
} }

View file

@ -15,6 +15,7 @@
package google.registry.tools.server; package google.registry.tools.server;
import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTld;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import google.registry.testing.FakeClock; import google.registry.testing.FakeClock;
@ -75,6 +76,7 @@ public class ListTldsActionTest extends ListActionTestCase {
Optional.of("badfield"), Optional.of("badfield"),
null, null,
null, null,
"^Field 'badfield' not found - recognized fields are:"); "^Field 'badfield' not found - recognized fields are:",
SC_BAD_REQUEST);
} }
} }