diff --git a/java/google/registry/tools/server/ListObjectsAction.java b/java/google/registry/tools/server/ListObjectsAction.java index 8902e4e3e..904718213 100644 --- a/java/google/registry/tools/server/ListObjectsAction.java +++ b/java/google/registry/tools/server/ListObjectsAction.java @@ -14,9 +14,8 @@ package google.registry.tools.server; +import static com.google.common.base.MoreObjects.firstNonNull; 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.Functions; @@ -36,6 +35,7 @@ import com.google.common.collect.Ordering; import google.registry.model.ImmutableObject; import google.registry.request.JsonResponse; import google.registry.request.Parameter; +import google.registry.util.FormattingLogger; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -53,6 +53,8 @@ import javax.inject.Inject; */ public abstract class ListObjectsAction implements Runnable { + private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); + public static final String FIELDS_PARAM = "fields"; public static final String PRINT_HEADER_ROW_PARAM = "printHeaderRow"; public static final String FULL_FIELD_NAMES_PARAM = "fullFieldNames"; @@ -117,13 +119,11 @@ public abstract class ListObjectsAction implements Ru response.setPayload(ImmutableMap.of( "lines", lines, "status", "success")); - } catch (Exception e) { - String message = e.getMessage(); - if (message == null) { - message = e.getClass().getName(); - } - response.setStatus( - e instanceof IllegalArgumentException ? SC_BAD_REQUEST : SC_INTERNAL_SERVER_ERROR); + } catch (IllegalArgumentException e) { + String message = firstNonNull(e.getMessage(), e.getClass().getName()); + logger.warning(e, message); + // Don't return a non-200 response, since that will cause RegistryTool to barf instead of + // letting ListObjectsCommand parse the JSON response and return a clean error. response.setPayload(ImmutableMap.of( "error", message, "status", "error")); diff --git a/java/google/registry/tools/server/ListRegistrarsAction.java b/java/google/registry/tools/server/ListRegistrarsAction.java index 4cd74ff90..afc9ac302 100644 --- a/java/google/registry/tools/server/ListRegistrarsAction.java +++ b/java/google/registry/tools/server/ListRegistrarsAction.java @@ -45,6 +45,7 @@ public final class ListRegistrarsAction extends ListObjectsAction { @Override public ImmutableBiMap getFieldAliases() { return ImmutableBiMap.of( + "billingId", "billingIdentifier", "clientId", "clientIdentifier", "premiumNames", "blockPremiumNames"); } diff --git a/javatests/google/registry/tools/server/ListActionTestCase.java b/javatests/google/registry/tools/server/ListActionTestCase.java index 75ef4f596..4c719ae73 100644 --- a/javatests/google/registry/tools/server/ListActionTestCase.java +++ b/javatests/google/registry/tools/server/ListActionTestCase.java @@ -78,11 +78,10 @@ public class ListActionTestCase { Optional fields, Optional printHeaderRow, Optional fullFieldNames, - String expectedErrorPattern, - int expectedResponseStatus) { + String expectedErrorPattern) { assertThat(expectedErrorPattern).isNotNull(); runAction(action, fields, printHeaderRow, fullFieldNames); - assertThat(response.getStatus()).isEqualTo(expectedResponseStatus); + assertThat(response.getStatus()).isEqualTo(SC_OK); assertThat(response.getResponseMap().get("status")).isEqualTo("error"); Object obj = response.getResponseMap().get("error"); assertThat(obj).isInstanceOf(String.class); diff --git a/javatests/google/registry/tools/server/ListDomainsActionTest.java b/javatests/google/registry/tools/server/ListDomainsActionTest.java index 4bcf5b753..b5902cc7c 100644 --- a/javatests/google/registry/tools/server/ListDomainsActionTest.java +++ b/javatests/google/registry/tools/server/ListDomainsActionTest.java @@ -17,7 +17,6 @@ package google.registry.tools.server; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTlds; 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.collect.ImmutableSet; @@ -51,8 +50,7 @@ public class ListDomainsActionTest extends ListActionTestCase { null, null, null, - "^Must specify TLDs to query$", - SC_BAD_REQUEST); + "^Must specify TLDs to query$"); } @Test @@ -63,8 +61,7 @@ public class ListDomainsActionTest extends ListActionTestCase { null, null, null, - "^TLD %%%badtld%%% does not exist$", - SC_BAD_REQUEST); + "^TLD %%%badtld%%% does not exist$"); } @Test @@ -232,7 +229,6 @@ public class ListDomainsActionTest extends ListActionTestCase { Optional.of("badfield"), null, null, - "^Field 'badfield' not found - recognized fields are:", - SC_BAD_REQUEST); + "^Field 'badfield' not found - recognized fields are:"); } } diff --git a/javatests/google/registry/tools/server/ListHostsActionTest.java b/javatests/google/registry/tools/server/ListHostsActionTest.java index 7047e20d8..a4f5e2efd 100644 --- a/javatests/google/registry/tools/server/ListHostsActionTest.java +++ b/javatests/google/registry/tools/server/ListHostsActionTest.java @@ -16,7 +16,6 @@ package google.registry.tools.server; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.persistActiveHost; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import com.google.common.base.Optional; import google.registry.testing.FakeClock; @@ -104,7 +103,6 @@ public class ListHostsActionTest extends ListActionTestCase { Optional.of("badfield"), null, null, - "^Field 'badfield' not found - recognized fields are:", - SC_BAD_REQUEST); + "^Field 'badfield' not found - recognized fields are:"); } } diff --git a/javatests/google/registry/tools/server/ListPremiumListsActionTest.java b/javatests/google/registry/tools/server/ListPremiumListsActionTest.java index 34109a5bf..3ebcdaa24 100644 --- a/javatests/google/registry/tools/server/ListPremiumListsActionTest.java +++ b/javatests/google/registry/tools/server/ListPremiumListsActionTest.java @@ -15,7 +15,6 @@ package google.registry.tools.server; import static google.registry.testing.DatastoreHelper.persistPremiumList; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import com.google.common.base.Optional; import google.registry.model.registry.label.PremiumList; @@ -87,7 +86,6 @@ public class ListPremiumListsActionTest extends ListActionTestCase { Optional.of("badfield"), null, null, - "^Field 'badfield' not found - recognized fields are:", - SC_BAD_REQUEST); + "^Field 'badfield' not found - recognized fields are:"); } } diff --git a/javatests/google/registry/tools/server/ListRegistrarsActionTest.java b/javatests/google/registry/tools/server/ListRegistrarsActionTest.java index 991452a88..352080ce1 100644 --- a/javatests/google/registry/tools/server/ListRegistrarsActionTest.java +++ b/javatests/google/registry/tools/server/ListRegistrarsActionTest.java @@ -16,7 +16,6 @@ package google.registry.tools.server; import static google.registry.testing.DatastoreHelper.createTlds; 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.collect.ImmutableSet; @@ -96,7 +95,6 @@ public class ListRegistrarsActionTest extends ListActionTestCase { Optional.of("badfield"), null, null, - "^Field 'badfield' not found - recognized fields are:", - SC_BAD_REQUEST); + "^Field 'badfield' not found - recognized fields are:"); } } diff --git a/javatests/google/registry/tools/server/ListReservedListsActionTest.java b/javatests/google/registry/tools/server/ListReservedListsActionTest.java index 9c2c54739..9290feab1 100644 --- a/javatests/google/registry/tools/server/ListReservedListsActionTest.java +++ b/javatests/google/registry/tools/server/ListReservedListsActionTest.java @@ -17,7 +17,6 @@ package google.registry.tools.server; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.persistReservedList; import static google.registry.testing.DatastoreHelper.persistResource; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import com.google.common.base.Optional; import google.registry.model.registry.Registry; @@ -88,7 +87,6 @@ public class ListReservedListsActionTest extends ListActionTestCase { Optional.of("badfield"), null, null, - "^Field 'badfield' not found - recognized fields are:", - SC_BAD_REQUEST); + "^Field 'badfield' not found - recognized fields are:"); } } diff --git a/javatests/google/registry/tools/server/ListTldsActionTest.java b/javatests/google/registry/tools/server/ListTldsActionTest.java index e0decbe76..46af004e4 100644 --- a/javatests/google/registry/tools/server/ListTldsActionTest.java +++ b/javatests/google/registry/tools/server/ListTldsActionTest.java @@ -15,7 +15,6 @@ package google.registry.tools.server; import static google.registry.testing.DatastoreHelper.createTld; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import com.google.common.base.Optional; import google.registry.testing.FakeClock; @@ -76,7 +75,6 @@ public class ListTldsActionTest extends ListActionTestCase { Optional.of("badfield"), null, null, - "^Field 'badfield' not found - recognized fields are:", - SC_BAD_REQUEST); + "^Field 'badfield' not found - recognized fields are:"); } }