opendev/gerrit
Code Issues Proposed changes

Check project state in ChangesCollection

Browse Source 
The majority of code in {Project,Ref,Change}Control is now about
permissions, but not all. Exceptions include checks for a project's
state. This is confusing, because users are presented with an exception
telling them that they lack some kind of permission while the real
reason for the failed operation is that the project's current state
doesn't permit the operation.

This is part of a series of commits to remove all project state checks
from *Control classes and make explicit checks instead.

For all change resources, the project has to at least be in a readable
state. This can be easily checked in ChangesCollection, which is what
this commit does.

Change-Id: I4b3506af16a711a67c8e54635b93896189c15e3a
This commit is contained in:
Patrick Hiesel
2018-01-18 15:06:00 +01:00
parent 97035388c6
commit 979fd39578
7 changed files with 36 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
java/com/google/gerrit/sshd/ChangeArgumentParser.java
View File

@@ -15,7 +15,7 @@
package com.google.gerrit.sshd;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -32,6 +32,7 @@ import com.google.gerrit.server.restapi.change.ChangesCollection;
import com.google.gerrit.sshd.BaseCommand.UnloggedFailure;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -62,13 +63,13 @@ public class ChangeArgumentParser {
}
public void addChange(String id, Map<Change.Id, ChangeResource> changes)
throws UnloggedFailure, OrmException, PermissionBackendException {
throws UnloggedFailure, OrmException, PermissionBackendException, IOException {
addChange(id, changes, null);
}
public void addChange(
String id, Map<Change.Id, ChangeResource> changes, ProjectState projectState)
throws UnloggedFailure, OrmException, PermissionBackendException {
throws UnloggedFailure, OrmException, PermissionBackendException, IOException {
addChange(id, changes, projectState, true);
}
@@ -77,7 +78,7 @@ public class ChangeArgumentParser {
Map<Change.Id, ChangeResource> changes,
ProjectState projectState,
boolean useIndex)
throws UnloggedFailure, OrmException, PermissionBackendException {
throws UnloggedFailure, OrmException, PermissionBackendException, IOException {
List<ChangeNotes> matched = useIndex ? changeFinder.find(id) : changeFromNotesFactory(id);
List<ChangeNotes> toAdd = new ArrayList<>(changes.size());
boolean canMaintainServer;
@@ -109,7 +110,7 @@ public class ChangeArgumentParser {
ChangeResource changeResource;
try {
changeResource = changesCollection.parse(cId);
} catch (ResourceNotFoundException e) {
} catch (RestApiException e) {
throw new UnloggedFailure(1, "\"" + id + "\" no such change");
}
changes.put(cId, changeResource);