Refactor GerritClient to Version Specific Implementations

Change-Id: I70d3330f8d9a6e6a4689cba260fe43e8e8c0fb3b

7 files changed, 57 insertions, 56 deletions

diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/GerritSynchronizationTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/GerritSynchronizationTest.java
index 4c9eaa425..2efd9b3e2 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/GerritSynchronizationTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/GerritSynchronizationTest.java
@@ -85,8 +85,8 @@ public class GerritSynchronizationTest extends TestCase {
 //		GerritCorePlugin.getDefault().getConnector().setFactoryProviderConfigurer(configurer);
 		GerritUiPlugin.getDefault();
 		TasksUiPlugin.getDefault()
-		.getPreferenceStore()
-		.setValue(ITasksUiPreferenceConstants.REPOSITORY_SYNCH_SCHEDULE_ENABLED, false);
+				.getPreferenceStore()
+				.setValue(ITasksUiPreferenceConstants.REPOSITORY_SYNCH_SCHEDULE_ENABLED, false);
 		// cancel any parallel query synchronization jobs
 		Job.getJobManager().cancel(ITasksCoreConstants.JOB_FAMILY_SYNCHRONIZATION);
 
@@ -95,14 +95,14 @@ public class GerritSynchronizationTest extends TestCase {
 		harness = GerritFixture.current().harness();
 		repository = GerritFixture.current().singleRepository();
 		GerritCorePlugin.getDefault()
-		.getConnector()
-		.setFactoryProviderConfigurer(new RemoteUiFactoryProviderConfigurer());
+				.getConnector()
+				.setFactoryProviderConfigurer(new RemoteUiFactoryProviderConfigurer());
 		client = GerritCorePlugin.getDefault().getConnector().getClient(repository);
 		AbstractRemoteEditFactoryProvider abstractRemoteEditFactoryProvider = (AbstractRemoteEditFactoryProvider) client.getFactoryProvider();
 		GerritCorePlugin.getDefault()
-		.getConnector()
-		.getFactoryProviderConfigurer()
-		.configure(abstractRemoteEditFactoryProvider);
+				.getConnector()
+				.getFactoryProviderConfigurer()
+				.configure(abstractRemoteEditFactoryProvider);
 
 		assertThat(abstractRemoteEditFactoryProvider.getService(), instanceOf(RemoteUiService.class));
 		taskList = TasksUiPlugin.getTaskList();
@@ -175,7 +175,7 @@ public class GerritSynchronizationTest extends TestCase {
 
 	private String addComment(ITask task) throws GerritException {
 		taskDataManager.setTaskRead(task, true);
-		GerritClient client = new GerritClient(null, harness.location());
+		GerritClient client = GerritClient.create(null, harness.location());
 		String message = "new comment, time: " + System.currentTimeMillis(); //$NON-NLS-1$
 		client.publishComments(task.getTaskId(), 1, message, Collections.<ApprovalCategoryValue.Id> emptySet(), null);
 		return message;
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java
index 0f0b24bc2..1ee214afb 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java
@@ -71,7 +71,8 @@ public class GerritClientTest extends TestCase {
 	public class TestGerritClient extends GerritClient {
 
 		public TestGerritClient(TaskRepository repository, AbstractWebLocation location) {
-			super(repository, location);
+			super(repository, GerritFixture.current().getGerritVersion());
+			initialize(location, null, null, null, null);
 		}
 
 		@Override
@@ -186,7 +187,7 @@ public class GerritClientTest extends TestCase {
 		WebLocation location = harness.location();
 		GerritAuthenticationState authState = new GerritAuthenticationState();
 		authState.setCookie(new Cookie(WebUtil.getHost(location.getUrl()), "xrsfKey", "invalid"));
-		client = new GerritClient(null, location, null, authState, "invalid");
+		client = GerritClient.create(null, location, null, authState, "invalid", null);
 		client.getAccount(null);
 	}
 
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritHarness.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritHarness.java
index 638fa0974..c4eb25423 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritHarness.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritHarness.java
@@ -49,7 +49,7 @@ public class GerritHarness {
 
 	public GerritClient client(PrivilegeLevel privilegeLevel) {
 		TaskRepository repository = TasksUiPlugin.getRepositoryManager().getRepository(fixture.getRepositoryUrl());
-		return new GerritClient(repository, location(privilegeLevel));
+		return GerritClient.create(repository, location(privilegeLevel));
 	}
 
 	public WebLocation location() {
@@ -79,7 +79,7 @@ public class GerritHarness {
 				return WebUtil.getProxyForUrl(fixture.getRepositoryUrl());
 			}
 		});
-		return new GerritClient(repository, location);
+		return GerritClient.create(repository, location);
 	}
 
 	public void dispose() {
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritRemoteTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritRemoteTest.java
index 9cb3733f4..4e9d0a44b 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritRemoteTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritRemoteTest.java
@@ -13,6 +13,8 @@ package org.eclipse.mylyn.internal.gerrit.core.remote;
 
 import junit.framework.TestCase;
 
+import org.eclipse.mylyn.internal.gerrit.core.client.GerritException;
+import org.eclipse.mylyn.internal.gerrit.core.client.GerritVersion;
 import org.eclipse.mylyn.reviews.core.model.IReview;
 import org.junit.After;
 import org.junit.Before;
@@ -44,4 +46,19 @@ public class GerritRemoteTest extends TestCase {
 		reviewHarness.dispose();
 	}
 
+	boolean isVersion24x() throws GerritException {
+		return GerritVersion.isVersion24x(reviewHarness.getClient().getVersion());
+	}
+
+	boolean isVersion26OrLater() throws GerritException {
+		return GerritVersion.isVersion26OrLater(reviewHarness.getClient().getVersion());
+	}
+
+	boolean isVersion28OrLater() throws GerritException {
+		return GerritVersion.isVersion28OrLater(reviewHarness.getClient().getVersion());
+	}
+
+	boolean isVersion29OrLater() throws GerritException {
+		return GerritVersion.isVersion29OrLater(reviewHarness.getClient().getVersion());
+	}
 }
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
index 222ea63ed..a527faa3e 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
@@ -43,8 +43,8 @@ import org.eclipse.mylyn.commons.sdk.util.CommonTestUtil.PrivilegeLevel;
 import org.eclipse.mylyn.gerrit.tests.core.client.rest.ChangeInfoTest;
 import org.eclipse.mylyn.gerrit.tests.support.GerritProject.CommitResult;
 import org.eclipse.mylyn.internal.gerrit.core.client.GerritChange;
+import org.eclipse.mylyn.internal.gerrit.core.client.GerritClient29;
 import org.eclipse.mylyn.internal.gerrit.core.client.GerritException;
-import org.eclipse.mylyn.internal.gerrit.core.client.GerritVersion;
 import org.eclipse.mylyn.internal.gerrit.core.client.compat.ChangeDetailX;
 import org.eclipse.mylyn.internal.gerrit.core.client.compat.GerritSystemAccount;
 import org.eclipse.mylyn.internal.gerrit.core.client.compat.PatchSetPublishDetailX;
@@ -189,7 +189,7 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 
 	@Test
 	public void testDependencies() throws Exception {
-		boolean isversion29OrLater = reviewHarness.getClient().isVersion29OrLater(new NullProgressMonitor());
+		boolean isversion29OrLater = isVersion29OrLater();
 		String changeIdDep1 = "I" + StringUtils.rightPad(System.currentTimeMillis() + "", 40, "a");
 		CommitCommand commandDep1 = reviewHarness.createCommitCommand(changeIdDep1);
 		reviewHarness.addFile("testFile1.txt", "test 2");
@@ -197,9 +197,8 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 		String resultIdDep1 = StringUtils.trimToEmpty(StringUtils.substringAfterLast(resultDep1.push.getMessages(), "/"));
 		assertThat("Bad Push: " + resultDep1.push.getMessages(), resultIdDep1.length(), greaterThan(0));
 
-		TestRemoteObserverConsumer<IRepository, IReview, String, GerritChange, String, Date> consumerDep1 //
-		= retrieveForRemoteKey(reviewHarness.getProvider().getReviewFactory(), reviewHarness.getRepository(),
-				resultIdDep1, true);
+		TestRemoteObserverConsumer<IRepository, IReview, String, GerritChange, String, Date> consumerDep1 = retrieveForRemoteKey(
+				reviewHarness.getProvider().getReviewFactory(), reviewHarness.getRepository(), resultIdDep1, true);
 		IReview reviewDep1 = consumerDep1.getModelObject();
 
 		assertThat(reviewDep1.getParents().size(), is(1));
@@ -334,7 +333,7 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 	public void testAddSomeInvalidReviewers() throws Exception {
 		List<String> reviewers = Arrays.asList(new String[] { "tests", "foo" });
 		int userid = 1000001; //user id for tests
-		if (reviewHarness.getClient().isVersion29OrLater(new NullProgressMonitor())) {
+		if (isVersion29OrLater()) {
 			//use "admin " since this is a valid user in 2.9
 			reviewers = Arrays.asList(new String[] { "admin", "foo" });
 			userid = 1000000; //user id for admin
@@ -351,7 +350,7 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 		assertThat(getReview().getReviewerApprovals().isEmpty(), is(true));
 		List<String> reviewers = Arrays.asList(new String[] { "tests" });
 		int userid = 1000001; //user id for tests
-		if (reviewHarness.getClient().isVersion29OrLater(new NullProgressMonitor())) {
+		if (isVersion29OrLater()) {
 			//Need a user and not the review owner
 			reviewers = Arrays.asList(new String[] { "admin" });
 			userid = 1000000; //user id for admin
@@ -367,7 +366,7 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 	public void testAddReviewersByEmail() throws Exception {
 		List<String> reviewers = Arrays.asList(new String[] { "tests@mylyn.eclipse.org" });
 		int userid = 1000001; //user id for tests
-		if (reviewHarness.getClient().isVersion29OrLater(new NullProgressMonitor())) {
+		if (isVersion29OrLater()) {
 			//Need a user and not the review owner
 			reviewers = Arrays.asList(new String[] { "admin@mylyn.eclipse.org" });
 			userid = 1000000; //user id for admin
@@ -442,7 +441,7 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 		int reviewId = Integer.parseInt(reviewHarness.getShortId());
 
 		ChangeInfo changeInfo = reviewHarness.getClient().getChangeInfo(reviewId, new NullProgressMonitor());
-		if (reviewHarness.getClient().isVersion29OrLater(new NullProgressMonitor())) {
+		if (isVersion29OrLater()) {
 			ChangeInfoTest.assertHasCodeReviewLabels(changeInfo, true);
 		} else {
 			ChangeInfoTest.assertHasCodeReviewLabels(changeInfo, false);
@@ -598,7 +597,7 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 
 		IComment commentByGerrit = comments.get(offset + 1);
 
-		if (reviewHarness.getClient().isVersion29OrLater(new NullProgressMonitor())) {
+		if (isVersion29OrLater()) {
 			assertNotNull(commentByGerrit.getAuthor());
 			assertThat(commentByGerrit.getAuthor().getId(),
 					is(String.valueOf(GerritSystemAccount.GERRIT_SYSTEM.getId())));
@@ -613,14 +612,14 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 
 	private void createBranchIfNonExistent(String branchName) throws GerritException {
 		if (!branchExists(branchName)) {
-			reviewHarness.getAdminClient().createRemoteBranch("org.eclipse.mylyn.test", branchName, null,
-					new NullProgressMonitor());
+			((GerritClient29) reviewHarness.getAdminClient()).createRemoteBranch("org.eclipse.mylyn.test", branchName,
+					null, new NullProgressMonitor());
 		}
 	}
 
 	private boolean branchExists(String branchName) throws GerritException {
-		BranchInfo[] branches = reviewHarness.getAdminClient().getRemoteProjectBranches("org.eclipse.mylyn.test",
-				new NullProgressMonitor());
+		BranchInfo[] branches = ((GerritClient29) reviewHarness.getAdminClient()).getRemoteProjectBranches(
+				"org.eclipse.mylyn.test", new NullProgressMonitor());
 		for (BranchInfo branch : branches) {
 			String branchRef = StringUtils.trimToEmpty(StringUtils.substringAfterLast(branch.getRef(), "/"));
 			if (branchRef.equals(branchName)) {
@@ -631,9 +630,8 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 	}
 
 	private PatchSetDetail retrievePatchSetDetail(ReviewHarness reviewHarness, String patchSetId) {
-		TestRemoteObserverConsumer<IReview, IReviewItemSet, String, PatchSetDetail, PatchSetDetail, String> itemSetObserver //
-		= retrieveForLocalKey(reviewHarness.getProvider().getReviewItemSetFactory(), reviewHarness.getReview(),
-				patchSetId, false);
+		TestRemoteObserverConsumer<IReview, IReviewItemSet, String, PatchSetDetail, PatchSetDetail, String> itemSetObserver = retrieveForLocalKey(
+				reviewHarness.getProvider().getReviewItemSetFactory(), reviewHarness.getReview(), patchSetId, false);
 		PatchSetDetail detail = itemSetObserver.getRemoteObject();
 		return detail;
 	}
@@ -653,22 +651,6 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 		return isVersion28OrLater() ? 1 : 0;
 	}
 
-	private boolean isVersion29OrLater() throws GerritException {
-		return GerritVersion.isVersion29OrLater(reviewHarness.getClient().getVersion(new NullProgressMonitor()));
-	}
-
-	private boolean isVersion28OrLater() throws GerritException {
-		return GerritVersion.isVersion28OrLater(reviewHarness.getClient().getVersion(new NullProgressMonitor()));
-	}
-
-	private boolean isVersion26OrLater() throws GerritException {
-		return GerritVersion.isVersion26OrLater(reviewHarness.getClient().getVersion(new NullProgressMonitor()));
-	}
-
-	private boolean isVersion24x() throws GerritException {
-		return GerritVersion.isVersion24x(reviewHarness.getClient().getVersion(new NullProgressMonitor()));
-	}
-
 	private boolean supportsBranchCreation() throws GerritException {
 		return isVersion29OrLater();
 	}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
index cde3e096c..f8bbc37f3 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
@@ -193,7 +193,7 @@ public class PatchSetRemoteFactoryTest extends GerritRemoteTest {
 
 		// compare deleted image
 		patchScript = loadPatchSetContent(fileName, detail2, detail4);
-		boolean isVersion29OrLater = reviewHarness.getClient().isVersion29OrLater(new NullProgressMonitor());
+		boolean isVersion29OrLater = isVersion29OrLater();
 		if (isVersion29OrLater) {
 			//In Gerrit 2.9, if the file (test.png)is not in the target environment (Detail 4), it returns NULL
 			assertThat(patchScript, nullValue());
@@ -336,7 +336,8 @@ public class PatchSetRemoteFactoryTest extends GerritRemoteTest {
 	@Test
 	public void testLoadPatchSet() throws Exception {
 		// given
-		GerritChange change = reviewHarness.getClient().getChange(reviewHarness.getShortId(), new NullProgressMonitor());
+		GerritChange change = reviewHarness.getClient()
+				.getChange(reviewHarness.getShortId(), new NullProgressMonitor());
 		List<PatchSetDetail> details = change.getPatchSetDetails();
 		assertThat(details, notNullValue());
 		assertThat(details.size(), is(1));
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java
index 08ea8af7b..e08e50570 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java
@@ -209,10 +209,10 @@ class ReviewHarness {
 		assertThat(targetCommit.toString(), is(commitId));
 
 		git.checkout()
-		.setCreateBranch(true)
-		.setName("change" + "/" + getReview().getId() + "/" + number)
-		.setStartPoint(targetCommit)
-		.call();
+				.setCreateBranch(true)
+				.setName("change" + "/" + getReview().getId() + "/" + number)
+				.setStartPoint(targetCommit)
+				.call();
 	}
 
 	GerritClient getClient() {
@@ -225,14 +225,14 @@ class ReviewHarness {
 
 			WebLocation location = new WebLocation(GerritFixture.current().getRepositoryUrl(),
 					credentials.getUserName(), credentials.getPassword(), new IProxyProvider() {
-				public Proxy getProxyForHost(String host, String proxyType) {
-					return WebUtil.getProxyForUrl(GerritFixture.current().getRepositoryUrl());
-				}
-			});
+						public Proxy getProxyForHost(String host, String proxyType) {
+							return WebUtil.getProxyForUrl(GerritFixture.current().getRepositoryUrl());
+						}
+					});
 
 			TaskRepository repository = TasksUiPlugin.getRepositoryManager().getRepository(
 					GerritFixture.current().getRepositoryUrl());
-			adminClient = new GerritClient(repository, location);
+			adminClient = GerritClient.create(repository, location);
 		}
 		return adminClient;
 	}