From 7824f305ac18afea335042a91a945bd24b3964be Mon Sep 17 00:00:00 2001 From: Makson Lee Date: Fri, 20 Jul 2018 10:33:56 +0800 Subject: [PATCH] Implement REST endpoint to list files in a commit Change-Id: Id42b51dad374a2018a325a06f546226afc887ebd --- Documentation/rest-api-projects.txt | 45 ++++++++++ .../gerrit/extensions/common/FileInfo.java | 16 ++++ .../gerrit/server/change/FileInfoJson.java | 8 +- .../project/FilesInCommitCollection.java | 44 ++++++++- .../rest/ProjectsRestApiBindingsIT.java | 5 +- .../rest/project/ListCommitFilesIT.java | 90 +++++++++++++++++++ 6 files changed, 201 insertions(+), 7 deletions(-) create mode 100644 javatests/com/google/gerrit/acceptance/rest/project/ListCommitFilesIT.java diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 4a5cd1df03..2db67a31ff 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -2512,6 +2512,51 @@ describes the resulting cherry-picked change. } ---- +[[list-files]] +=== List Files +-- +'GET /projects/link:#project-name[\{project-name\}]/commits/link:#commit-id[\{commit-id\}]/files/' +-- + +Lists the files that were modified, added or deleted in a commit. + +.Request +---- + GET /projects/work%2Fmy-project/commits/a8a477efffbbf3b44169bb9a1d3a334cbbd9aa96/files/ HTTP/1.0 +---- + +As result a map is returned that maps the link:rest-api-changes.html#file-id[file path] to a +link:rest-api-changes.html#file-info[FileInfo] entry. The entries in the map are +sorted by file path. + +.Response +---- + HTTP/1.1 200 OK + Content-Disposition: attachment + Content-Type: application/json; charset=UTF-8 + + )]}' + { + "/COMMIT_MSG": { + "status": "A", + "lines_inserted": 7, + "size_delta": 551, + "size": 551 + }, + "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java": { + "lines_inserted": 5, + "lines_deleted": 3, + "size_delta": 98, + "size": 23348 + } + } +---- + +The integer-valued request parameter `parent` changes the response to return a +list of the files which are different in this commit compared to the given +parent commit. This is useful for supporting review of merge commits. The value +is the 1-based index of the parent's position in the commit object. + [[dashboard-endpoints]] == Dashboard Endpoints diff --git a/java/com/google/gerrit/extensions/common/FileInfo.java b/java/com/google/gerrit/extensions/common/FileInfo.java index a81290848b..747f9e2f92 100644 --- a/java/com/google/gerrit/extensions/common/FileInfo.java +++ b/java/com/google/gerrit/extensions/common/FileInfo.java @@ -14,6 +14,8 @@ package com.google.gerrit.extensions.common; +import java.util.Objects; + public class FileInfo { public Character status; public Boolean binary; @@ -22,4 +24,18 @@ public class FileInfo { public Integer linesDeleted; public long sizeDelta; public long size; + + public boolean equals(Object o) { + if (o instanceof FileInfo) { + FileInfo fileInfo = (FileInfo) o; + return Objects.equals(status, fileInfo.status) + && Objects.equals(binary, fileInfo.binary) + && Objects.equals(oldPath, fileInfo.oldPath) + && Objects.equals(linesInserted, fileInfo.linesInserted) + && Objects.equals(linesDeleted, fileInfo.linesDeleted) + && Objects.equals(sizeDelta, fileInfo.sizeDelta) + && Objects.equals(size, fileInfo.size); + } + return false; + } } diff --git a/java/com/google/gerrit/server/change/FileInfoJson.java b/java/com/google/gerrit/server/change/FileInfoJson.java index f4b7457c8f..56cc8df4d6 100644 --- a/java/com/google/gerrit/server/change/FileInfoJson.java +++ b/java/com/google/gerrit/server/change/FileInfoJson.java @@ -20,6 +20,7 @@ import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.server.patch.PatchList; import com.google.gerrit.server.patch.PatchListCache; @@ -68,7 +69,12 @@ public class FileInfoJson { private Map toFileInfoMap(Change change, PatchListKey key) throws PatchListNotAvailableException { - PatchList list = patchListCache.get(key, change.getProject()); + return toFileInfoMap(change.getProject(), key); + } + + public Map toFileInfoMap(Project.NameKey project, PatchListKey key) + throws PatchListNotAvailableException { + PatchList list = patchListCache.get(key, project); Map files = new TreeMap<>(); for (PatchListEntry e : list.getPatches()) { diff --git a/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java b/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java index 53411b8be3..09f973b50f 100644 --- a/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java +++ b/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java @@ -14,34 +14,46 @@ package com.google.gerrit.server.restapi.project; +import com.google.gerrit.extensions.client.DiffPreferencesInfo; import com.google.gerrit.extensions.registration.DynamicMap; 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.RestReadView; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Patch; +import com.google.gerrit.server.change.FileInfoJson; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.patch.PatchListKey; +import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.project.CommitResource; import com.google.gerrit.server.project.FileResource; import com.google.inject.Inject; +import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; +import org.eclipse.jgit.revwalk.RevCommit; +import org.kohsuke.args4j.Option; @Singleton public class FilesInCommitCollection implements ChildCollection { private final DynamicMap> views; + private final Provider list; private final GitRepositoryManager repoManager; @Inject FilesInCommitCollection( - DynamicMap> views, GitRepositoryManager repoManager) { + DynamicMap> views, + Provider list, + GitRepositoryManager repoManager) { this.views = views; + this.list = list; this.repoManager = repoManager; } @Override public RestView list() throws ResourceNotFoundException { - throw new ResourceNotFoundException(); + return list.get(); } @Override @@ -57,4 +69,32 @@ public class FilesInCommitCollection implements ChildCollection> views() { return views; } + + public static final class ListFiles implements RestReadView { + @Option(name = "--parent", metaVar = "parent-number") + int parentNum; + + private final FileInfoJson fileInfoJson; + + @Inject + public ListFiles(FileInfoJson fileInfoJson) { + this.fileInfoJson = fileInfoJson; + } + + @Override + public Object apply(CommitResource resource) throws PatchListNotAvailableException { + RevCommit commit = resource.getCommit(); + PatchListKey key; + + if (parentNum > 0) { + key = + PatchListKey.againstParentNum( + parentNum, commit, DiffPreferencesInfo.Whitespace.IGNORE_NONE); + } else { + key = PatchListKey.againstCommit(null, commit, DiffPreferencesInfo.Whitespace.IGNORE_NONE); + } + + return fileInfoJson.toFileInfoMap(resource.getProjectState().getNameKey(), key); + } + } } diff --git a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java index a86a7394fa..6563de32da 100644 --- a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java @@ -144,10 +144,7 @@ public class ProjectsRestApiBindingsIT extends AbstractRestApiBindingsTest { ImmutableList.of( RestCall.get("/projects/%s/commits/%s"), RestCall.get("/projects/%s/commits/%s/in"), - RestCall.builder(GET, "/projects/%s/commits/%s/files") - // GET /projects//branches//files is not implemented - .expectedResponseCode(SC_NOT_FOUND) - .build(), + RestCall.get("/projects/%s/commits/%s/files"), RestCall.post("/projects/%s/commits/%s/cherrypick")); /** diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListCommitFilesIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListCommitFilesIT.java new file mode 100644 index 0000000000..3ac2d10a3b --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/project/ListCommitFilesIT.java @@ -0,0 +1,90 @@ +// Copyright (C) 2018 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.acceptance.rest.project; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.acceptance.GitUtil.getChangeId; +import static com.google.gerrit.acceptance.GitUtil.pushHead; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.extensions.common.FileInfo; +import com.google.gson.reflect.TypeToken; +import java.lang.reflect.Type; +import java.util.Map; +import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Test; + +public class ListCommitFilesIT extends AbstractDaemonTest { + private static String SUBJECT_1 = "subject 1"; + private static String SUBJECT_2 = "subject 2"; + private static String FILE_A = "a.txt"; + private static String FILE_B = "b.txt"; + + @Test + public void listCommitFiles() throws Exception { + commitBuilder().add(FILE_B, "2").message(SUBJECT_1).create(); + pushHead(testRepo, "refs/heads/master", false); + + RevCommit a = commitBuilder().add(FILE_A, "1").rm(FILE_B).message(SUBJECT_2).create(); + String id = getChangeId(testRepo, a).get(); + pushHead(testRepo, "refs/for/master", false); + + RestResponse r = + userRestSession.get("/projects/" + project.get() + "/commits/" + a.name() + "/files/"); + r.assertOK(); + Type type = new TypeToken>() {}.getType(); + Map files1 = newGson().fromJson(r.getReader(), type); + r.consume(); + + r = userRestSession.get("/changes/" + id + "/revisions/" + a.name() + "/files"); + r.assertOK(); + Map files2 = newGson().fromJson(r.getReader(), type); + r.consume(); + + assertThat(files1).isEqualTo(files2); + } + + @Test + public void listMergeCommitFiles() throws Exception { + PushOneCommit.Result result = createMergeCommitChange("refs/for/master"); + + RestResponse r = + userRestSession.get( + "/projects/" + + project.get() + + "/commits/" + + result.getCommit().name() + + "/files/?parent=2"); + r.assertOK(); + Type type = new TypeToken>() {}.getType(); + Map files1 = newGson().fromJson(r.getReader(), type); + r.consume(); + + r = + userRestSession.get( + "/changes/" + + result.getChangeId() + + "/revisions/" + + result.getCommit().name() + + "/files/?parent=2"); + r.assertOK(); + Map files2 = newGson().fromJson(r.getReader(), type); + r.consume(); + + assertThat(files1).isEqualTo(files2); + } +}