opendev/gerrit
Code Issues Proposed changes

Merge changes I97680b9f,If0c17281

Browse Source 
* changes:
  Don't parse non-existing files in files REST collections
  Files REST endpoint: Don't parse non-existing files
This commit is contained in:
Edwin Kempin 2016-08-22 14:17:38 +00:00 committed by Gerrit Code Review
commit c253b09b64
7 changed files with 83 additions and 16 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;

6
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) {
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

18
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,9 +90,18 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
}
@Override
public FileResource parse(RevisionResource rev, IdString id) {
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> {
private static final Logger log = LoggerFactory.getLogger(ListFiles.class);

23
gerrit-server/src/main/java/com/google/gerrit/server/project/FileResource.java
View File

@ -14,16 +14,39 @@
package com.google.gerrit.server.project;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.TypeLiteral;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
import java.io.IOException;
public class FileResource implements RestResource {
public static final TypeLiteral<RestView<FileResource>> FILE_KIND =
new TypeLiteral<RestView<FileResource>>() {};
public static FileResource create(GitRepositoryManager repoManager,
ProjectControl project, ObjectId rev, String path)
throws ResourceNotFoundException, IOException {
try (Repository repo =
repoManager.openRepository(project.getProject().getNameKey());
RevWalk rw = new RevWalk(repo)) {
RevTree tree = rw.parseTree(rev);
if (TreeWalk.forPath(repo, path, tree) != null) {
return new FileResource(project, rev, path);
}
}
throw new ResourceNotFoundException(IdString.fromDecoded(path));
}
private final ProjectControl project;
private final ObjectId rev;
private final String path;

17
gerrit-server/src/main/java/com/google/gerrit/server/project/FilesCollection.java
View File

@ -19,19 +19,25 @@ import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.ObjectId;
import java.io.IOException;
@Singleton
public class FilesCollection implements
ChildCollection<BranchResource, FileResource> {
private final DynamicMap<RestView<FileResource>> views;
private final GitRepositoryManager repoManager;
@Inject
FilesCollection(DynamicMap<RestView<FileResource>> views) {
FilesCollection(DynamicMap<RestView<FileResource>> views,
GitRepositoryManager repoManager) {
this.views = views;
this.repoManager = repoManager;
}
@Override
@ -40,11 +46,10 @@ public class FilesCollection implements
}
@Override
public FileResource parse(BranchResource parent, IdString id) {
return new FileResource(
parent.getControl(),
ObjectId.fromString(parent.getRevision()),
id.get());
public FileResource parse(BranchResource parent, IdString id)
throws ResourceNotFoundException, IOException {
return FileResource.create(repoManager, parent.getControl(),
ObjectId.fromString(parent.getRevision()), id.get());
}
@Override

13
gerrit-server/src/main/java/com/google/gerrit/server/project/FilesInCommitCollection.java
View File

@ -19,17 +19,23 @@ import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@Singleton
public class FilesInCommitCollection implements
ChildCollection<CommitResource, FileResource> {
private final DynamicMap<RestView<FileResource>> views;
private final GitRepositoryManager repoManager;
@Inject
FilesInCommitCollection(DynamicMap<RestView<FileResource>> views) {
FilesInCommitCollection(DynamicMap<RestView<FileResource>> views,
GitRepositoryManager repoManager) {
this.views = views;
this.repoManager = repoManager;
}
@Override
@ -39,8 +45,9 @@ public class FilesInCommitCollection implements
@Override
public FileResource parse(CommitResource parent, IdString id)
throws ResourceNotFoundException {
return new FileResource(parent.getProject(), parent.getCommit(), id.get());
throws ResourceNotFoundException, IOException {
return FileResource.create(repoManager, parent.getProject(),
parent.getCommit(), id.get());
}
@Override