Merge "Address post-submit review of CheckAccess"

Documentation/rest-api-config.txt

@@ -1319,7 +1319,7 @@ an access check.
 |`status`|The HTTP status code for the access.
 200 means success, 403 means denied and 404 means the project does not
 exist.
-|`message`|A clarifying message.
+|`message`|A clarifying message if `status` is not 200.
 =========================================
 
 [[access-check-input]]

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CheckAccessIT.java

@@ -42,7 +42,7 @@ public class CheckAccessIT extends AbstractDaemonTest {
   private AccountGroup privilegedGroup;
 
   @Before
-  public void setup() throws Exception {
+  public void setUp() throws Exception {
     normalProject = createProject("normal");
     secretProject = createProject("secret");
     secretRefProject = createProject("secretRef");

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/Server.java

@@ -79,7 +79,7 @@ public interface Server {
     }
 
     @Override
-    public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
+    public AccessCheckInfo checkAccess(AccessCheckInput in) {
       throw new NotImplementedException();
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/api/config/ServerImpl.java

@@ -33,6 +33,7 @@ import com.google.gerrit.server.config.GetServerInfo;
 import com.google.gerrit.server.config.SetDiffPreferences;
 import com.google.gerrit.server.config.SetPreferences;
 import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -132,7 +133,7 @@ public class ServerImpl implements Server {
   public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
     try {
       return checkAccess.get().apply(new ConfigResource(), in);
-    } catch (IOException | PermissionBackendException e) {
+    } catch (OrmException | IOException | PermissionBackendException e) {
       throw new RestApiException("Cannot check access", e);
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/config/CheckAccess.java

@@ -44,7 +44,7 @@ import javax.servlet.http.HttpServletResponse;
 @Singleton
 public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckInput> {
   private final Provider<IdentifiedUser> currentUser;
-  private final AccountResolver resolver;
+  private final AccountResolver accountResolver;
   private final Provider<ReviewDb> db;
   private final IdentifiedUser.GenericFactory userFactory;
   private final ProjectCache projectCache;
@@ -59,7 +59,7 @@ public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckIn
       ProjectCache projectCache,
       PermissionBackend permissionBackend) {
     this.currentUser = currentUser;
-    this.resolver = resolver;
+    this.accountResolver = resolver;
     this.db = db;
     this.userFactory = userFactory;
     this.projectCache = projectCache;
@@ -68,30 +68,24 @@ public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckIn
 
   @Override
   public AccessCheckInfo apply(ConfigResource unused, AccessCheckInput input)
-      throws PermissionBackendException, RestApiException, IOException {
+      throws OrmException, PermissionBackendException, RestApiException, IOException {
     permissionBackend.user(currentUser.get()).check(GlobalPermission.ADMINISTRATE_SERVER);
 
     if (input == null) {
       throw new BadRequestException("input is required");
     }
-    if (input.account == null) {
-      throw new BadRequestException("must set account in input");
+    if (Strings.isNullOrEmpty(input.account)) {
+      throw new BadRequestException("input requires 'account'");
     }
-    if (input.project == null) {
-      throw new BadRequestException("must set project in input");
+    if (Strings.isNullOrEmpty(input.project)) {
+      throw new BadRequestException("input requires 'project'");
     }
 
-    Account match;
-    try {
-      match = resolver.find(db.get(), input.account);
-    } catch (OrmException e) {
-      throw new IOException(e);
-    }
+    Account match = accountResolver.find(db.get(), input.account);
     if (match == null) {
       throw new BadRequestException(String.format("cannot find account %s", input.account));
     }
 
-    IdentifiedUser user = userFactory.create(match.getId());
     AccessCheckInfo info = new AccessCheckInfo();
     info.result = new Result();
 
@@ -102,6 +96,7 @@ public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckIn
       return info;
     }
 
+    IdentifiedUser user = userFactory.create(match.getId());
     try {
       permissionBackend.user(user).project(key).check(ProjectPermission.ACCESS);
     } catch (AuthException | PermissionBackendException e) {