From cbe76e86158bff08a34caca4e750d6c16b82e7ab Mon Sep 17 00:00:00 2001 From: mmuller Date: Mon, 7 Nov 2016 12:21:48 -0800 Subject: [PATCH] Don't throw an exception on orphaned contacts Return Optional.absent() instead of throwing NotFoundException when a user has a contact record but the Registrar entity is missing. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=138423965 --- .../registry/ui/server/registrar/SessionUtils.java | 8 +++++++- .../registry/ui/server/registrar/SessionUtilsTest.java | 10 +++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/java/google/registry/ui/server/registrar/SessionUtils.java b/java/google/registry/ui/server/registrar/SessionUtils.java index cb07e1c73..ed85614c6 100644 --- a/java/google/registry/ui/server/registrar/SessionUtils.java +++ b/java/google/registry/ui/server/registrar/SessionUtils.java @@ -24,6 +24,7 @@ import com.google.appengine.api.users.UserService; import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.FluentIterable; +import com.googlecode.objectify.Key; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; import google.registry.util.FormattingLogger; @@ -121,7 +122,12 @@ public class SessionUtils { if (contact == null) { return Optional.absent(); } - return Optional.of(ofy().load().key(contact.getParent()).safe()); + Optional result = Optional.fromNullable(ofy().load().key(contact.getParent()).now()); + if (!result.isPresent()) { + logger.severefmt( + "A contact record exists for non-existent registrar: %s.", Key.create(contact)); + } + return result; } /** @see #hasAccessToRegistrar(Registrar, String) */ diff --git a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java index ce589bf6e..554dc7099 100644 --- a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java +++ b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java @@ -16,6 +16,7 @@ package google.registry.ui.server.registrar; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.AppEngineRule.THE_REGISTRAR_GAE_USER_ID; +import static google.registry.testing.DatastoreHelper.deleteResource; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -103,10 +104,17 @@ public class SessionUtilsTest { verify(session).invalidate(); } + @Test + public void testCheckRegistrarConsoleLogin_orphanedContactIsDenied() throws Exception { + deleteResource(Registrar.loadByClientId("TheRegistrar")); + when(userService.getCurrentUser()).thenReturn(jart); + assertThat(sessionUtils.checkRegistrarConsoleLogin(req)).isFalse(); + } + @Test public void testCheckRegistrarConsoleLogin_notLoggedIn_throwsIse() throws Exception { thrown.expect(IllegalStateException.class); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req)).isNull(); + boolean unused = sessionUtils.checkRegistrarConsoleLogin(req); } @Test