opendev/gerrit
Code Issues Proposed changes

Files REST endpoint: Don't parse non-existing files

Browse Source 
Rather fail with ResourceNotFoundException.

This is e.g. important for the GetDiff REST endpoint as it returns an
empty diff for non-existing files. As result the diff screen shows an
empty diff for any non-existing file, but users expect to see an
error.

This change may break existing users of the extension API since the
RevisionApi#file(String) is now throwing RestApiException.

Change-Id: If0c172818b553129145f5ccd2f99e19b94472cce
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-08-22 12:56:20 +02:00
parent 470292b065
commit 0520e08cb6
4 changed files with 39 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

@ -52,6 +52,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ETagView;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.server.change.GetRevisionActions;
@ -565,6 +566,19 @@ public class RevisionIT extends AbstractDaemonTest {
assertThat(diff.metaB.lines).isEqualTo(1);
}
@Test
public void diffNonExistingFile() throws Exception {
PushOneCommit.Result r = createChange();
exception.expect(ResourceNotFoundException.class);
exception.expectMessage("non-existing");
gApi.changes()
.id(r.getChangeId())
.revision(r.getCommit().name())
.file("non-existing")
.diff();
}
@Test
public void diffOnMergeCommitChange() throws Exception {
PushOneCommit.Result r = createMergeCommitChange("refs/for/master");

2
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
View File

@ -47,7 +47,7 @@ public interface RevisionApi {
Map<String, FileInfo> files() throws RestApiException;
Map<String, FileInfo> files(String base) throws RestApiException;
Map<String, FileInfo> files(int parentNum) throws RestApiException;
FileApi file(String path);
FileApi file(String path) throws RestApiException;
MergeableInfo mergeable() throws RestApiException;
MergeableInfo mergeableOtherBranches() throws RestApiException;

10
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
View File

@ -321,9 +321,13 @@ class RevisionApiImpl implements RevisionApi {
}
@Override
public FileApi file(String path) {
return fileApi.create(files.parse(revision,
IdString.fromDecoded(path)));
public FileApi file(String path) throws RestApiException {
try {
return fileApi.create(files.parse(revision,
IdString.fromDecoded(path)));
} catch (IOException e) {
throw new RestApiException("Cannot retrieve file", e);
}
}
@Override

20
gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java
View File

@ -48,6 +48,7 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
@ -67,11 +68,15 @@ import java.util.concurrent.TimeUnit;
public class Files implements ChildCollection<RevisionResource, FileResource> {
private final DynamicMap<RestView<FileResource>> views;
private final Provider<ListFiles> list;
private final GitRepositoryManager repoManager;
@Inject
Files(DynamicMap<RestView<FileResource>> views, Provider<ListFiles> list) {
Files(DynamicMap<RestView<FileResource>> views,
Provider<ListFiles> list,
GitRepositoryManager repoManager) {
this.views = views;
this.list = list;
this.repoManager = repoManager;
}
@Override
@ -85,8 +90,17 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
}
@Override
public FileResource parse(RevisionResource rev, IdString id) {
return new FileResource(rev, id.get());
public FileResource parse(RevisionResource rev, IdString id)
throws ResourceNotFoundException, IOException {
try (Repository repo = repoManager.openRepository(rev.getProject());
RevWalk rw = new RevWalk(repo)) {
RevTree tree = rw.parseTree(
ObjectId.fromString(rev.getPatchSet().getRevision().get()));
if (TreeWalk.forPath(repo, id.get(), tree) != null) {
return new FileResource(rev, id.get());
}
}
throw new ResourceNotFoundException(id);
}
public static final class ListFiles implements RestReadView<RevisionResource> {