Revert "Files REST endpoint: Don't parse non-existing files"
This reverts commit 0520e08cb6
. It broke
display of diffs for deleted files.
Also add a test to prevent it breaking again.
Bug: Issue 4450
Change-Id: Ib7546ed88c9cf7a5a0e16fc1eeff5a224feb4f25
This commit is contained in:
@@ -52,7 +52,6 @@ import com.google.gerrit.extensions.restapi.AuthException;
|
|||||||
import com.google.gerrit.extensions.restapi.BinaryResult;
|
import com.google.gerrit.extensions.restapi.BinaryResult;
|
||||||
import com.google.gerrit.extensions.restapi.ETagView;
|
import com.google.gerrit.extensions.restapi.ETagView;
|
||||||
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
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.extensions.restapi.UnprocessableEntityException;
|
||||||
import com.google.gerrit.reviewdb.client.Patch;
|
import com.google.gerrit.reviewdb.client.Patch;
|
||||||
import com.google.gerrit.server.change.GetRevisionActions;
|
import com.google.gerrit.server.change.GetRevisionActions;
|
||||||
@@ -567,16 +566,19 @@ public class RevisionIT extends AbstractDaemonTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void diffNonExistingFile() throws Exception {
|
public void diffDeletedFile() throws Exception {
|
||||||
PushOneCommit.Result r = createChange();
|
pushFactory.create(db, admin.getIdent(), testRepo)
|
||||||
|
.to("refs/heads/master");
|
||||||
exception.expect(ResourceNotFoundException.class);
|
PushOneCommit.Result r =
|
||||||
exception.expectMessage("non-existing");
|
pushFactory.create(db, admin.getIdent(), testRepo)
|
||||||
gApi.changes()
|
.rm("refs/for/master");
|
||||||
|
DiffInfo diff = gApi.changes()
|
||||||
.id(r.getChangeId())
|
.id(r.getChangeId())
|
||||||
.revision(r.getCommit().name())
|
.revision(r.getCommit().name())
|
||||||
.file("non-existing")
|
.file(FILE_NAME)
|
||||||
.diff();
|
.diff();
|
||||||
|
assertThat(diff.metaA.lines).isEqualTo(1);
|
||||||
|
assertThat(diff.metaB).isNull();
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@@ -47,7 +47,7 @@ public interface RevisionApi {
|
|||||||
Map<String, FileInfo> files() throws RestApiException;
|
Map<String, FileInfo> files() throws RestApiException;
|
||||||
Map<String, FileInfo> files(String base) throws RestApiException;
|
Map<String, FileInfo> files(String base) throws RestApiException;
|
||||||
Map<String, FileInfo> files(int parentNum) throws RestApiException;
|
Map<String, FileInfo> files(int parentNum) throws RestApiException;
|
||||||
FileApi file(String path) throws RestApiException;
|
FileApi file(String path);
|
||||||
MergeableInfo mergeable() throws RestApiException;
|
MergeableInfo mergeable() throws RestApiException;
|
||||||
MergeableInfo mergeableOtherBranches() throws RestApiException;
|
MergeableInfo mergeableOtherBranches() throws RestApiException;
|
||||||
|
|
||||||
|
@@ -321,13 +321,9 @@ class RevisionApiImpl implements RevisionApi {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public FileApi file(String path) throws RestApiException {
|
public FileApi file(String path) {
|
||||||
try {
|
|
||||||
return fileApi.create(files.parse(revision,
|
return fileApi.create(files.parse(revision,
|
||||||
IdString.fromDecoded(path)));
|
IdString.fromDecoded(path)));
|
||||||
} catch (IOException e) {
|
|
||||||
throw new RestApiException("Cannot retrieve file", e);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
@@ -49,7 +49,6 @@ import org.eclipse.jgit.lib.ObjectId;
|
|||||||
import org.eclipse.jgit.lib.ObjectReader;
|
import org.eclipse.jgit.lib.ObjectReader;
|
||||||
import org.eclipse.jgit.lib.Repository;
|
import org.eclipse.jgit.lib.Repository;
|
||||||
import org.eclipse.jgit.revwalk.RevCommit;
|
import org.eclipse.jgit.revwalk.RevCommit;
|
||||||
import org.eclipse.jgit.revwalk.RevTree;
|
|
||||||
import org.eclipse.jgit.revwalk.RevWalk;
|
import org.eclipse.jgit.revwalk.RevWalk;
|
||||||
import org.eclipse.jgit.treewalk.TreeWalk;
|
import org.eclipse.jgit.treewalk.TreeWalk;
|
||||||
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
|
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
|
||||||
@@ -69,15 +68,11 @@ import java.util.concurrent.TimeUnit;
|
|||||||
public class Files implements ChildCollection<RevisionResource, FileResource> {
|
public class Files implements ChildCollection<RevisionResource, FileResource> {
|
||||||
private final DynamicMap<RestView<FileResource>> views;
|
private final DynamicMap<RestView<FileResource>> views;
|
||||||
private final Provider<ListFiles> list;
|
private final Provider<ListFiles> list;
|
||||||
private final GitRepositoryManager repoManager;
|
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
Files(DynamicMap<RestView<FileResource>> views,
|
Files(DynamicMap<RestView<FileResource>> views, Provider<ListFiles> list) {
|
||||||
Provider<ListFiles> list,
|
|
||||||
GitRepositoryManager repoManager) {
|
|
||||||
this.views = views;
|
this.views = views;
|
||||||
this.list = list;
|
this.list = list;
|
||||||
this.repoManager = repoManager;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -91,21 +86,9 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public FileResource parse(RevisionResource rev, IdString id)
|
public FileResource parse(RevisionResource rev, IdString id) {
|
||||||
throws ResourceNotFoundException, IOException {
|
|
||||||
if (Patch.COMMIT_MSG.equals(id.get())) {
|
|
||||||
return new FileResource(rev, id.get());
|
return new FileResource(rev, id.get());
|
||||||
}
|
}
|
||||||
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> {
|
public static final class ListFiles implements RestReadView<RevisionResource> {
|
||||||
private static final Logger log = LoggerFactory.getLogger(ListFiles.class);
|
private static final Logger log = LoggerFactory.getLogger(ListFiles.class);
|
||||||
|
Reference in New Issue
Block a user