opendev/gerrit
Code Issues Proposed changes

Convert IncludedInResolver.Result to AutoValue

Browse Source 
Convert the tests to Truth as well.

Change-Id: Iddbe7b8f5e29e8bc4ff3e0780477ec38434061c1
This commit is contained in:
Dave Borowitz
2018-08-31 12:11:03 -07:00
parent 5944f628cf
commit 9398d54c03
4 changed files with 45 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
java/com/google/gerrit/extensions/api/changes/IncludedInInfo.java
View File

@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.gerrit.extensions.api.changes;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
@@ -23,9 +24,11 @@ public class IncludedInInfo {
public Map<String, Collection<String>> external;
public IncludedInInfo(
List<String> branches, List<String> tags, Map<String, Collection<String>> external) {
this.branches = branches;
this.tags = tags;
Collection<String> branches,
Collection<String> tags,
Map<String, Collection<String>> external) {
this.branches = new ArrayList<>(branches);
this.tags = new ArrayList<>(tags);
this.external = external;
}
}

4
java/com/google/gerrit/server/change/IncludedIn.java
View File

@@ -63,13 +63,13 @@ public class IncludedIn {
ListMultimap<String, String> external = MultimapBuilder.hashKeys().arrayListValues().build();
for (ExternalIncludedIn ext : externalIncludedIn) {
ListMultimap<String, String> extIncludedIns =
ext.getIncludedIn(project.get(), rev.name(), d.getTags(), d.getBranches());
ext.getIncludedIn(project.get(), rev.name(), d.tags(), d.branches());
if (extIncludedIns != null) {
external.putAll(extIncludedIns);
}
}
return new IncludedInInfo(
d.getBranches(), d.getTags(), (!external.isEmpty() ? external.asMap() : null));
d.branches(), d.tags(), (!external.isEmpty() ? external.asMap() : null));
}
}
}

53
java/com/google/gerrit/server/change/IncludedInResolver.java
View File

@@ -14,9 +14,13 @@
package com.google.gerrit.server.change;
import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
import static java.util.Comparator.comparing;
import static java.util.Comparator.naturalOrder;
import static java.util.stream.Collectors.toList;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
@@ -95,11 +99,9 @@ public class IncludedInResolver {
parseCommits(allTagsAndBranches);
Set<String> allMatchingTagsAndBranches = includedIn(tipsByCommitTime, 0);
Result detail = new Result();
detail.setBranches(getMatchingRefNames(allMatchingTagsAndBranches, branches));
detail.setTags(getMatchingRefNames(allMatchingTagsAndBranches, tags));
return detail;
return new AutoValue_IncludedInResolver_Result(
getMatchingRefNames(allMatchingTagsAndBranches, branches),
getMatchingRefNames(allMatchingTagsAndBranches, tags));
}
private boolean includedInOne(Collection<Ref> refs) throws IOException {
@@ -169,15 +171,14 @@ public class IncludedInResolver {
* Returns the short names of refs which are as well in the matchingRefs list as well as in the
* allRef list.
*/
private static List<String> getMatchingRefNames(
private static ImmutableSortedSet<String> getMatchingRefNames(
Set<String> matchingRefs, Collection<Ref> allRefs) {
List<String> refNames = Lists.newArrayListWithCapacity(matchingRefs.size());
for (Ref r : allRefs) {
if (matchingRefs.contains(r.getName())) {
refNames.add(Repository.shortenRefName(r.getName()));
}
}
return refNames;
return allRefs
.stream()
.map(Ref::getName)
.filter(matchingRefs::contains)
.map(Repository::shortenRefName)
.collect(toImmutableSortedSet(naturalOrder()));
}
/** Parse commit of ref and store the relation between ref and commit. */
@@ -209,28 +210,10 @@ public class IncludedInResolver {
commitToRef.keySet().stream().sorted(comparing(RevCommit::getCommitTime)).collect(toList());
}
public static class Result {
private List<String> branches;
private List<String> tags;
@AutoValue
public abstract static class Result {
public abstract ImmutableSortedSet<String> branches();
public Result() {}
public void setBranches(List<String> b) {
Collections.sort(b);
branches = b;
}
public List<String> getBranches() {
return branches;
}
public void setTags(List<String> t) {
Collections.sort(t);
tags = t;
}
public List<String> getTags() {
return tags;
}
public abstract ImmutableSortedSet<String> tags();
}
}

63
javatests/com/google/gerrit/server/change/IncludedInResolverTest.java
View File

@@ -14,10 +14,9 @@
package com.google.gerrit.server.change;
import static com.google.common.truth.Truth.assertThat;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.MergeCommand.FastForwardMode;
import org.eclipse.jgit.junit.RepositoryTestCase;
@@ -27,7 +26,6 @@ import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevTag;
import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -55,9 +53,6 @@ public class IncludedInResolverTest extends RepositoryTestCase {
private RevCommit commit_v1_3;
private RevCommit commit_v2_5;
private List<String> expTags = new ArrayList<>();
private List<String> expBranches = new ArrayList<>();
private RevWalk revWalk;
@Override
@@ -140,12 +135,8 @@ public class IncludedInResolverTest extends RepositoryTestCase {
IncludedInResolver.Result detail = resolve(commit_v2_5);
// Check that only tags and branches which refer the tip are returned
expTags.add(TAG_2_5);
expTags.add(TAG_2_5_ANNOTATED);
expTags.add(TAG_2_5_ANNOTATED_TWICE);
assertEquals(expTags, detail.getTags());
expBranches.add(BRANCH_2_5);
assertEquals(expBranches, detail.getBranches());
assertThat(detail.tags()).containsExactly(TAG_2_5, TAG_2_5_ANNOTATED, TAG_2_5_ANNOTATED_TWICE);
assertThat(detail.branches()).containsExactly(BRANCH_2_5);
}
@Test
@@ -154,22 +145,18 @@ public class IncludedInResolverTest extends RepositoryTestCase {
IncludedInResolver.Result detail = resolve(commit_initial);
// Check whether all tags and branches are returned
expTags.add(TAG_1_0);
expTags.add(TAG_1_0_1);
expTags.add(TAG_1_3);
expTags.add(TAG_2_0);
expTags.add(TAG_2_0_1);
expTags.add(TAG_2_5);
expTags.add(TAG_2_5_ANNOTATED);
expTags.add(TAG_2_5_ANNOTATED_TWICE);
assertEquals(expTags, detail.getTags());
expBranches.add(BRANCH_MASTER);
expBranches.add(BRANCH_1_0);
expBranches.add(BRANCH_1_3);
expBranches.add(BRANCH_2_0);
expBranches.add(BRANCH_2_5);
assertEquals(expBranches, detail.getBranches());
assertThat(detail.tags())
.containsExactly(
TAG_1_0,
TAG_1_0_1,
TAG_1_3,
TAG_2_0,
TAG_2_0_1,
TAG_2_5,
TAG_2_5_ANNOTATED,
TAG_2_5_ANNOTATED_TWICE);
assertThat(detail.branches())
.containsExactly(BRANCH_MASTER, BRANCH_1_0, BRANCH_1_3, BRANCH_2_0, BRANCH_2_5);
}
@Test
@@ -178,27 +165,15 @@ public class IncludedInResolverTest extends RepositoryTestCase {
IncludedInResolver.Result detail = resolve(commit_v1_3);
// Check whether all succeeding tags and branches are returned
expTags.add(TAG_1_3);
expTags.add(TAG_2_5);
expTags.add(TAG_2_5_ANNOTATED);
expTags.add(TAG_2_5_ANNOTATED_TWICE);
assertEquals(expTags, detail.getTags());
expBranches.add(BRANCH_1_3);
expBranches.add(BRANCH_2_5);
assertEquals(expBranches, detail.getBranches());
assertThat(detail.tags())
.containsExactly(TAG_1_3, TAG_2_5, TAG_2_5_ANNOTATED, TAG_2_5_ANNOTATED_TWICE);
assertThat(detail.branches()).containsExactly(BRANCH_1_3, BRANCH_2_5);
}
private IncludedInResolver.Result resolve(RevCommit commit) throws Exception {
return IncludedInResolver.resolve(db, revWalk, commit);
}
private void assertEquals(List<String> list1, List<String> list2) {
Collections.sort(list1);
Collections.sort(list2);
Assert.assertEquals(list1, list2);
}
private void createAndCheckoutBranch(ObjectId objectId, String branchName) throws IOException {
String fullBranchName = "refs/heads/" + branchName;
super.createBranch(objectId, fullBranchName);