Split out Depends-On computation in helper classes
This splitting out simplifies the main logic a bit, but helps a lot
with getting simpler tests.
Change-Id: Icd74a29a97bb9ef6132c3748129f351fb2984efd
diff --git a/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java b/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java
index f9e23b7..22b2f56 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java
@@ -14,9 +14,6 @@
package com.googlesource.gerrit.plugins.zuul;
-import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
@@ -25,24 +22,19 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import com.googlesource.gerrit.plugins.zuul.util.CommitMessageFetcher;
+import com.googlesource.gerrit.plugins.zuul.util.DependsOnFetcher;
import com.googlesource.gerrit.plugins.zuul.util.NeededByFetcher;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@Singleton
public class GetCrd implements RestReadView<RevisionResource> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- private final CommitMessageFetcher commitMessageFetcher;
+ private final DependsOnFetcher dependsOnFetcher;
private final NeededByFetcher neededByFetcher;
@Inject
- GetCrd(CommitMessageFetcher commitMessageFetcher, NeededByFetcher neededByFetcher) {
- this.commitMessageFetcher = commitMessageFetcher;
+ GetCrd(DependsOnFetcher dependsOnFetcher, NeededByFetcher neededByFetcher) {
+ this.dependsOnFetcher = dependsOnFetcher;
this.neededByFetcher = neededByFetcher;
}
@@ -51,22 +43,10 @@
throws RepositoryNotFoundException, IOException, BadRequestException, AuthException,
PermissionBackendException {
CrdInfo out = new CrdInfo();
- Change.Key thisId = rsrc.getChange().getKey();
- // get depends on info
- out.dependsOn = new ArrayList<>();
- Project.NameKey p = rsrc.getChange().getProject();
- String rev = rsrc.getPatchSet().commitId().getName();
- String commitMsg = commitMessageFetcher.fetch(p, rev);
- Pattern pattern = Pattern.compile("[Dd]epends-[Oo]n:? (I[0-9a-f]{8,40})", Pattern.DOTALL);
- Matcher matcher = pattern.matcher(commitMsg);
- while (matcher.find()) {
- String otherId = matcher.group(1);
- logger.atFinest().log("Change %s depends on change %s", thisId, otherId);
- out.dependsOn.add(otherId);
- }
+ out.dependsOn = dependsOnFetcher.fetchForRevision(rsrc);
- out.neededBy = neededByFetcher.fetchForChangeKey(thisId);
+ out.neededBy = neededByFetcher.fetchForChangeKey(rsrc.getChange().getKey());
out.cycle = false;
for (String neededKey : out.neededBy) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnExtractor.java
new file mode 100644
index 0000000..92521a5
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnExtractor.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.zuul.util;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/** Extracts dependency information from texts. */
+public class DependsOnExtractor {
+ public List<String> extract(String commitMessage) {
+ List<String> dependsOn = new ArrayList<>();
+ Pattern pattern = Pattern.compile("[Dd]epends-[Oo]n:? (I[0-9a-f]{8,40})", Pattern.DOTALL);
+ Matcher matcher = pattern.matcher(commitMessage);
+ while (matcher.find()) {
+ String key = matcher.group(1);
+ dependsOn.add(key);
+ }
+ return dependsOn;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnFetcher.java b/src/main/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnFetcher.java
new file mode 100644
index 0000000..ee66de0
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnFetcher.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.zuul.util;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.change.RevisionResource;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.util.List;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+/** Fetches the Depends-On part of cross repository dependencies. */
+public class DependsOnFetcher {
+ private final CommitMessageFetcher commitMessageFetcher;
+ private final DependsOnExtractor dependsOnExtractor;
+
+ @Inject
+ public DependsOnFetcher(
+ CommitMessageFetcher commitMessageFetcher, DependsOnExtractor dependsOnExtractor) {
+ this.commitMessageFetcher = commitMessageFetcher;
+ this.dependsOnExtractor = dependsOnExtractor;
+ }
+
+ public List<String> fetchForRevision(RevisionResource rsrc)
+ throws RepositoryNotFoundException, IOException {
+ Project.NameKey p = rsrc.getChange().getProject();
+ String rev = rsrc.getPatchSet().commitId().getName();
+ String commitMsg = commitMessageFetcher.fetch(p, rev);
+ return dependsOnExtractor.extract(commitMsg);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/zuul/GetCrdTest.java b/src/test/java/com/googlesource/gerrit/plugins/zuul/GetCrdTest.java
index bfa3cdb..d5a101e 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/zuul/GetCrdTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/zuul/GetCrdTest.java
@@ -17,30 +17,23 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.server.change.RevisionResource;
-import com.googlesource.gerrit.plugins.zuul.util.CommitMessageFetcher;
+import com.googlesource.gerrit.plugins.zuul.util.DependsOnFetcher;
import com.googlesource.gerrit.plugins.zuul.util.NeededByFetcher;
-import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.List;
-import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
public class GetCrdTest {
- private NeededByFetcher neededByFetcher;
- private CommitMessageFetcher commitMessageFetcher;
private RevisionResource rsrc;
+ private DependsOnFetcher dependsOnFetcher;
+ private NeededByFetcher neededByFetcher;
@Test
public void testNoDependencies() throws Exception {
- String commitMessage = "subject";
- configureMocks(commitMessage, new ArrayList<>());
+ configureMocks(new ArrayList<>(), new ArrayList<>());
GetCrd getCrd = createGetCrd();
Response<CrdInfo> response = getCrd.apply(rsrc);
@@ -54,8 +47,10 @@
@Test
public void testSingleDependsOn() throws Exception {
- String commitMessage = "subject\nDepends-On: I00000000";
- configureMocks(commitMessage, new ArrayList<>());
+ ArrayList<String> dependsOn = new ArrayList<>();
+ dependsOn.add("I00000000");
+
+ configureMocks(dependsOn, new ArrayList<>());
GetCrd getCrd = createGetCrd();
Response<CrdInfo> response = getCrd.apply(rsrc);
@@ -69,9 +64,12 @@
@Test
public void testMultipleDependsOn() throws Exception {
- String commitMessage =
- "subject\nDepends-On: I00000000\nDepends-On: I00000002\nDepends-On: I00000004";
- configureMocks(commitMessage, new ArrayList<>());
+ ArrayList<String> dependsOn = new ArrayList<>();
+ dependsOn.add("I00000000");
+ dependsOn.add("I00000002");
+ dependsOn.add("I00000004");
+
+ configureMocks(dependsOn, new ArrayList<>());
GetCrd getCrd = createGetCrd();
Response<CrdInfo> response = getCrd.apply(rsrc);
@@ -85,10 +83,12 @@
@Test
public void testSingleNeededBy() throws Exception {
- String commitMessage = "subject";
+ List<String> dependsOn = new ArrayList<>();
+
List<String> neededBy = new ArrayList<>();
neededBy.add("I00000001");
- configureMocks(commitMessage, neededBy);
+
+ configureMocks(dependsOn, neededBy);
GetCrd getCrd = createGetCrd();
Response<CrdInfo> response = getCrd.apply(rsrc);
@@ -102,12 +102,14 @@
@Test
public void testMultipleNeededBy() throws Exception {
- String commitMessage = "subject";
+ List<String> dependsOn = new ArrayList<>();
+
List<String> neededBy = new ArrayList<>();
neededBy.add("I00000001");
neededBy.add("I00000003");
neededBy.add("I00000005");
- configureMocks(commitMessage, neededBy);
+
+ configureMocks(dependsOn, neededBy);
GetCrd getCrd = createGetCrd();
Response<CrdInfo> response = getCrd.apply(rsrc);
@@ -121,11 +123,15 @@
@Test
public void testMixed() throws Exception {
- String commitMessage = "subject\nDepends-On: I00000002\nDepends-On: I00000004";
+ List<String> dependsOn = new ArrayList<>();
+ dependsOn.add("I00000002");
+ dependsOn.add("I00000004");
+
List<String> neededBy = new ArrayList<>();
neededBy.add("I00000001");
neededBy.add("I00000003");
- configureMocks(commitMessage, neededBy);
+
+ configureMocks(dependsOn, neededBy);
GetCrd getCrd = createGetCrd();
Response<CrdInfo> response = getCrd.apply(rsrc);
@@ -139,10 +145,13 @@
@Test
public void testSimpleCycle() throws Exception {
- String commitMessage = "subject\nDepends-On: I00000001";
+ List<String> dependsOn = new ArrayList<>();
+ dependsOn.add("I00000001");
+
List<String> neededBy = new ArrayList<>();
neededBy.add("I00000001");
- configureMocks(commitMessage, neededBy);
+
+ configureMocks(dependsOn, neededBy);
GetCrd getCrd = createGetCrd();
Response<CrdInfo> response = getCrd.apply(rsrc);
@@ -154,36 +163,21 @@
assertThat(crdInfo.cycle).isTrue();
}
- public void configureMocks(String commitMessage, final List<String> neededBy) throws Exception {
- String commitId = "0123456789012345678901234567890123456789";
-
- Project.NameKey projectNameKey = Project.nameKey("projectFoo");
-
- PatchSet patchSet = mock(PatchSet.class);
- when(patchSet.commitId()).thenReturn(ObjectId.fromString(commitId));
-
+ public void configureMocks(final List<String> dependsOn, final List<String> neededBy)
+ throws Exception {
Change.Key changeKey = Change.key("I0123456789");
-
- Change change =
- new Change(
- changeKey,
- Change.id(4711),
- Account.id(23),
- BranchNameKey.create(projectNameKey, "branchBar"),
- new Timestamp(0));
-
+ Change change = new Change(changeKey, null, null, null, null);
rsrc = mock(RevisionResource.class);
when(rsrc.getChange()).thenReturn(change);
- when(rsrc.getPatchSet()).thenReturn(patchSet);
- commitMessageFetcher = mock(CommitMessageFetcher.class);
- when(commitMessageFetcher.fetch(projectNameKey, commitId)).thenReturn(commitMessage);
+ dependsOnFetcher = mock(DependsOnFetcher.class);
+ when(dependsOnFetcher.fetchForRevision(rsrc)).thenReturn(dependsOn);
neededByFetcher = mock(NeededByFetcher.class);
when(neededByFetcher.fetchForChangeKey(changeKey)).thenReturn(neededBy);
}
private GetCrd createGetCrd() {
- return new GetCrd(commitMessageFetcher, neededByFetcher);
+ return new GetCrd(dependsOnFetcher, neededByFetcher);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnExtractorTest.java b/src/test/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnExtractorTest.java
new file mode 100644
index 0000000..7e24229
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnExtractorTest.java
@@ -0,0 +1,82 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.zuul.util;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.entities.Change;
+import java.util.List;
+import org.junit.Test;
+
+public class DependsOnExtractorTest {
+ @Test
+ public void testExtractNoDependies() throws Exception {
+ DependsOnExtractor extractor = new DependsOnExtractor();
+
+ List<String> extracted = extractor.extract("foo");
+
+ assertThat(extracted).isEmpty();
+ }
+
+ @Test
+ public void testExtractSingleDependency() throws Exception {
+ DependsOnExtractor extractor = new DependsOnExtractor();
+ String commitMessage = "subject\n\nDepends-On: " + getChangeKey(1);
+
+ List<String> extracted = extractor.extract(commitMessage);
+
+ assertThat(extracted).containsExactly(getChangeKey(1));
+ }
+
+ @Test
+ public void testExtractMultipleDependencies() throws Exception {
+ DependsOnExtractor extractor = new DependsOnExtractor();
+ String commitMessage =
+ "subject\n\nDepends-On: "
+ + getChangeKey(1)
+ + "\nDepends-On: "
+ + getChangeKey(2)
+ + "\nDepends-On: "
+ + getChangeKey(3);
+
+ List<String> extracted = extractor.extract(commitMessage);
+
+ assertThat(extracted).containsExactly(getChangeKey(1), getChangeKey(2), getChangeKey(3));
+ }
+
+ @Test
+ public void testExtractLowerCase() throws Exception {
+ DependsOnExtractor extractor = new DependsOnExtractor();
+ String commitMessage = "subject\n\ndepends-on: " + getChangeKey(1);
+
+ List<String> extracted = extractor.extract(commitMessage);
+
+ assertThat(extracted).containsExactly(getChangeKey(1));
+ }
+
+ @Test
+ public void testExtractMixedCases() throws Exception {
+ DependsOnExtractor extractor = new DependsOnExtractor();
+ String commitMessage =
+ "subject\n\ndepends-On: " + getChangeKey(1) + "\nDepends-on: " + getChangeKey(2);
+
+ List<String> extracted = extractor.extract(commitMessage);
+
+ assertThat(extracted).containsExactly(getChangeKey(1), getChangeKey(2));
+ }
+
+ private String getChangeKey(int keyEnding) {
+ return Change.key("I0123456789abcdef0000000000000000000" + (10000 + keyEnding)).toString();
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnFetcherTest.java b/src/test/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnFetcherTest.java
new file mode 100644
index 0000000..3cc6652
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/zuul/util/DependsOnFetcherTest.java
@@ -0,0 +1,101 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.zuul.util;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.change.RevisionResource;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class DependsOnFetcherTest {
+ private CommitMessageFetcher commitMessageFetcher;
+ private DependsOnExtractor dependsOnExtractor;
+ private RevisionResource rsrc;
+
+ @Test
+ public void testExtractNoDependencies() throws Exception {
+ configureMocks(new ArrayList<>());
+
+ DependsOnFetcher fetcher = createFetcher();
+ List<String> dependsOn = fetcher.fetchForRevision(rsrc);
+
+ assertThat(dependsOn).isEmpty();
+ }
+
+ @Test
+ public void testExtractSingleDependency() throws Exception {
+ List<String> extracted = new ArrayList<>();
+ extracted.add("I00000001");
+ configureMocks(extracted);
+
+ DependsOnFetcher fetcher = createFetcher();
+ List<String> dependsOn = fetcher.fetchForRevision(rsrc);
+
+ assertThat(dependsOn).containsExactly("I00000001");
+ }
+
+ @Test
+ public void testExtractMultipleDependencies() throws Exception {
+ List<String> extracted = new ArrayList<>();
+ extracted.add("I00000001");
+ extracted.add("I00000002");
+ extracted.add("I00000003");
+ configureMocks(extracted);
+
+ DependsOnFetcher fetcher = createFetcher();
+ List<String> dependsOn = fetcher.fetchForRevision(rsrc);
+
+ assertThat(dependsOn).containsExactly("I00000001", "I00000002", "I00000003");
+ }
+
+ private void configureMocks(List<String> dependsOn)
+ throws RepositoryNotFoundException, IOException {
+ String commitId = "0123456789012345678901234567890123456789";
+
+ Project.NameKey projectNameKey = Project.nameKey("projectFoo");
+
+ PatchSet patchSet = mock(PatchSet.class);
+ when(patchSet.commitId()).thenReturn(ObjectId.fromString(commitId));
+
+ BranchNameKey branchNameKey = BranchNameKey.create(projectNameKey, "branchBar");
+
+ Change.Key changeKey = Change.key("I0123456789");
+ Change change = new Change(changeKey, null, null, branchNameKey, null);
+
+ rsrc = mock(RevisionResource.class);
+ when(rsrc.getChange()).thenReturn(change);
+ when(rsrc.getPatchSet()).thenReturn(patchSet);
+
+ commitMessageFetcher = mock(CommitMessageFetcher.class);
+ when(commitMessageFetcher.fetch(projectNameKey, commitId)).thenReturn("commitMsgFoo");
+
+ dependsOnExtractor = mock(DependsOnExtractor.class);
+ when(dependsOnExtractor.extract("commitMsgFoo")).thenReturn(dependsOn);
+ }
+
+ private DependsOnFetcher createFetcher() {
+ return new DependsOnFetcher(commitMessageFetcher, dependsOnExtractor);
+ }
+}
