Refactor GetFilesOwnersIT and make FilesOwnersResponse public
The members of the FilesOwnersResponse have no reason to be
private as they represent the JSON payload of the owners API
response.
Refactor also the GetFilesOwnersIT with separate assertions
on the response fields, making use of the Google Truth's
map-based assertions.
Change-Id: I1daa0619c13394890a295b588948f9c04071d269
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java
index 693813b..30c9c73 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java
@@ -22,8 +22,8 @@
/* Files to Owners response API representation */
public class FilesOwnersResponse {
- private final Map<String, Set<GroupOwner>> files;
- private final Map<Integer, Map<String, Integer>> ownersLabels;
+ public final Map<String, Set<GroupOwner>> files;
+ public final Map<Integer, Map<String, Integer>> ownersLabels;
public FilesOwnersResponse(
Map<Integer, Map<String, Integer>> ownersLabels, Map<String, Set<GroupOwner>> files) {
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
index b1117b9..707d1b1 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -76,7 +76,7 @@
}
@Override
- public Response<?> apply(RevisionResource revision)
+ public Response<FilesOwnersResponse> apply(RevisionResource revision)
throws AuthException, BadRequestException, ResourceConflictException, Exception {
PatchSet ps = revision.getPatchSet();
Change change = revision.getChange();
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
index b72bd3c..cf8d4d6 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
@@ -17,24 +17,21 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
-import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.acceptance.config.GlobalPluginConfig;
import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.server.change.RevisionResource;
import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
import com.googlesource.gerrit.owners.entities.GroupOwner;
import com.googlesource.gerrit.owners.entities.Owner;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.compress.utils.Sets;
import org.junit.Test;
@TestPlugin(name = "owners", httpModule = "com.googlesource.gerrit.owners.OwnersRestApiModule")
+@UseLocalDisk
public class GetFilesOwnersIT extends LightweightPluginDaemonTest {
private GetFilesOwners ownersApi;
@@ -44,98 +41,59 @@
super.setUpTestPlugin();
ownersApi = plugin.getSysInjector().getInstance(GetFilesOwners.class);
- }
- @Test
- @UseLocalDisk
- public void shouldReturnACorrectResponse() throws Exception {
// Add OWNERS file to root:
//
// inherited: true
// owners:
- // - Administrator
- merge(createChange(testRepo, "master", "Add OWNER file", "OWNERS", getOwnerFileContent(), ""));
-
- PushOneCommit.Result result = createChange();
-
- approve(result.getChangeId());
-
- RevisionResource revisionResource = parseCurrentRevisionResource(result.getChangeId());
-
- Response<?> resp = ownersApi.apply(revisionResource);
-
- assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
-
- FilesOwnersResponse responseValue = (FilesOwnersResponse) resp.value();
-
- FilesOwnersResponse expectedFilesOwnerResponse =
- new FilesOwnersResponse(
- new HashMap<Integer, Map<String, Integer>>() {
- {
- put(
- admin.id().get(),
- new HashMap<String, Integer>() {
- {
- put("Code-Review", 2);
- }
- });
- }
- },
- new HashMap<String, Set<GroupOwner>>() {
- {
- put("a.txt", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get())));
- }
- });
-
- assertThat(responseValue).isEqualTo(expectedFilesOwnerResponse);
+ // - admin
+ merge(
+ createChange(
+ testRepo,
+ "master",
+ "Add OWNER file",
+ "OWNERS",
+ "owners:\n" + "- " + admin.username() + "\n",
+ ""));
}
@Test
- @UseLocalDisk
+ public void shouldReturnExactFileOwners() throws Exception {
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get())));
+ }
+
+ @Test
+ public void shouldReturnOwnersLabels() throws Exception {
+ String changeId = createChange().getChangeId();
+ approve(changeId);
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().ownersLabels)
+ .containsExactly(admin.id().get(), ImmutableMap.builder().put("Code-Review", 2).build());
+ }
+
+ @Test
@GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
- public void shouldReturnResponseWithUnexpandedOwners() throws Exception {
- // Add OWNERS file to root:
- //
- // inherited: true
- // owners:
- // - Administrator
- merge(createChange(testRepo, "master", "Add OWNER file", "OWNERS", getOwnerFileContent(), ""));
+ public void shouldReturnResponseWithUnexpandedFileOwners() throws Exception {
+ String changeId = createChange().getChangeId();
- PushOneCommit.Result result = createChange();
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
- approve(result.getChangeId());
-
- RevisionResource revisionResource = parseCurrentRevisionResource(result.getChangeId());
-
- Response<?> resp = ownersApi.apply(revisionResource);
-
- assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
-
- FilesOwnersResponse responseValue = (FilesOwnersResponse) resp.value();
-
- FilesOwnersResponse expectedFilesOwnerResponse =
- new FilesOwnersResponse(
- new HashMap<Integer, Map<String, Integer>>() {
- {
- put(
- admin.id().get(),
- new HashMap<String, Integer>() {
- {
- put("Code-Review", 2);
- }
- });
- }
- },
- new HashMap<String, Set<GroupOwner>>() {
- {
- put("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
- }
- });
-
- assertThat(responseValue).isEqualTo(expectedFilesOwnerResponse);
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
}
- private String getOwnerFileContent() {
- return "owners:\n" + "- " + admin.username() + "\n";
+ private static <T> Response<T> assertResponseOk(Response<T> response) {
+ assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
+ return response;
}
}
