opendev/gerrit
Code Issues Proposed changes

SubmoduleSectionParser: move existence check to SubmoduleOp

Browse Source 
This makes it easier for the SubmoduleSectionParser to be a utility
helper class such that it doesn't need to know about server internals.

This also moves the business logic to SubmoduleOp, which only uses helper
classes for parsing now.

Change-Id: Icacb85adb9d4801765033dfe0fdec3633efcd214
This commit is contained in:
Stefan Beller
2016-05-24 08:27:13 -07:00
parent 613cb309cc
commit 174ad0a479
4 changed files with 33 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSectionParserIT.java
View File

@@ -43,7 +43,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -66,7 +66,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch1 = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res1 = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res1 = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch1).parseAllSections();
Set<SubmoduleSubscription> expected1 = Sets.newHashSet(
@@ -78,7 +78,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch2 = new Branch.NameKey(
new Project.NameKey("project"), "somebranch");
Set<SubmoduleSubscription> res2 = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res2 = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch2).parseAllSections();
Set<SubmoduleSubscription> expected2 = Sets.newHashSet(
@@ -101,7 +101,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -124,7 +124,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res =new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res =new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -147,7 +147,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -170,7 +170,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -197,7 +197,8 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -226,7 +227,8 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -269,7 +271,8 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -293,7 +296,8 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
assertThat(res).isEmpty();
@@ -311,7 +315,8 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
assertThat(res).isEmpty();
@@ -330,7 +335,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(
@@ -353,7 +358,7 @@ public class SubmoduleSectionParserIT extends AbstractDaemonTest {
Branch.NameKey targetBranch = new Branch.NameKey(
new Project.NameKey("nested/project"), "master");
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(projectCache,
Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
cfg, THIS_SERVER, targetBranch).parseAllSections();
Set<SubmoduleSubscription> expected = Sets.newHashSet(

5
gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.SubmoduleSectionParser;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -73,7 +72,7 @@ public class GitModules {
this.canonicalWebUrl = canonicalWebUrl;
}
void load(ProjectCache cache) throws IOException {
void load() throws IOException {
Project.NameKey project = branch.getParentKey();
logDebug("Loading .gitmodules of {} for project {}", branch, project);
try {
@@ -97,7 +96,7 @@ public class GitModules {
try {
BlobBasedConfig bbc =
new BlobBasedConfig(null, or.repo, commit, GIT_MODULES);
subscriptions = new SubmoduleSectionParser(cache, bbc, canonicalWebUrl,
subscriptions = new SubmoduleSectionParser(bbc, canonicalWebUrl,
branch).parseAllSections();
} catch (ConfigInvalidException e) {
throw new IOException(

8
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
View File

@@ -149,8 +149,12 @@ public class SubmoduleOp {
getDestinationBranches(branch, s, orm);
for (Branch.NameKey targetBranch : branches) {
GitModules m = gitmodulesFactory.create(targetBranch, updateId, orm);
m.load(this.projectCache);
ret.addAll(m.subscribedTo(branch));
m.load();
for (SubmoduleSubscription ss : m.subscribedTo(branch)) {
if (projectCache.get(ss.getSubmodule().getParentKey()) != null) {
ret.add(ss);
}
}
}
}
logDebug("Calculated superprojects for " + branch + " are " + ret);

13
gerrit-server/src/main/java/com/google/gerrit/server/util/SubmoduleSectionParser.java
View File

@@ -17,7 +17,6 @@ package com.google.gerrit.server.util;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
import com.google.gerrit.server.project.ProjectCache;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -46,16 +45,13 @@ import java.util.Set;
*/
public class SubmoduleSectionParser {
private final ProjectCache projectCache;
private final Config bbc;
private final String canonicalWebUrl;
private final Branch.NameKey superProjectBranch;
public SubmoduleSectionParser(ProjectCache projectCache,
Config bbc,
public SubmoduleSectionParser(Config bbc,
String canonicalWebUrl,
Branch.NameKey superProjectBranch) {
this.projectCache = projectCache;
this.bbc = bbc;
this.canonicalWebUrl = canonicalWebUrl;
this.superProjectBranch = superProjectBranch;
@@ -76,7 +72,6 @@ public class SubmoduleSectionParser {
final String url = bbc.getString("submodule", id, "url");
final String path = bbc.getString("submodule", id, "path");
String branch = bbc.getString("submodule", id, "branch");
SubmoduleSubscription ss = null;
try {
if (url != null && url.length() > 0 && path != null && path.length() > 0
@@ -137,16 +132,14 @@ public class SubmoduleSectionParser {
project.length() - Constants.DOT_GIT_EXT.length());
}
Project.NameKey projectKey = new Project.NameKey(project);
if (projectCache.get(projectKey) != null) {
ss = new SubmoduleSubscription(
return new SubmoduleSubscription(
superProjectBranch,
new Branch.NameKey(projectKey, branch),
path);
}
}
} catch (URISyntaxException e) {
// Error in url syntax (in fact it is uri syntax)
}
return ss;
return null;
}
}