406036: enhance generic remote API to push review details

Change-Id: I3b626b93c2475cb32313b523d9609edb8d3e44df
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=406036
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java
index ee24574..bf1d8da 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java
@@ -144,6 +144,11 @@
 	}
 
 	@Override
+	public void push(GerritChange remoteObject, IProgressMonitor monitor) throws CoreException {
+		//noop, push not supported in Gerrit
+	}
+
+	@Override
 	public boolean isCreateModelNeeded(IRepository parentObject, IReview modelObject) {
 		return super.isCreateModelNeeded(parentObject, modelObject) || modelObject.getModificationDate() == null;
 	}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritUserRemoteFactory.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritUserRemoteFactory.java
index ecf74dc..73b33ed 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritUserRemoteFactory.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritUserRemoteFactory.java
@@ -48,6 +48,11 @@
 	}
 
 	@Override
+	public void push(AccountInfo remoteObject, IProgressMonitor monitor) throws CoreException {
+		//noop, push not supported in Gerrit
+	}
+
+	@Override
 	public IUser createModel(IRepository repository, AccountInfo info) {
 		IUser user = IReviewsFactory.INSTANCE.createUser();
 		user.setId(info.getId() + "");
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetContentRemoteFactory.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetContentRemoteFactory.java
index fea25eb..b18636e 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetContentRemoteFactory.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetContentRemoteFactory.java
@@ -74,6 +74,11 @@
 		return content;
 	}
 
+	@Override
+	public void push(PatchSetContent remoteObject, IProgressMonitor monitor) throws CoreException {
+		//noop, push not supported in Gerrit
+	}
+
 	boolean addComments(IReviewItemSet set, IFileVersion version, List<PatchLineComment> comments,
 			AccountInfoCache accountInfoCache) {
 		version.getComments().clear();
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetDetailRemoteFactory.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetDetailRemoteFactory.java
index cf6896b..3f83848 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetDetailRemoteFactory.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetDetailRemoteFactory.java
@@ -47,6 +47,11 @@
 	}
 
 	@Override
+	public void push(PatchSetDetail remoteObject, IProgressMonitor monitor) throws CoreException {
+		//noop, push not supported in Gerrit
+	}
+
+	@Override
 	public boolean isAsynchronous() {
 		return false;
 	}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteObserver.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteObserver.java
index fce7809..6899aad 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteObserver.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteObserver.java
@@ -29,6 +29,8 @@
 
 	int responded;
 
+	int sent;
+
 	IStatus failure;
 
 	AbstractRemoteEmfFactory<P, T, L, ?, ?, C> factory;
@@ -51,16 +53,29 @@
 		}
 	}
 
+	@Override
+	public void sent(P parentObject, T modelObject) {
+		sent++;
+	}
+
+	@Override
+	public void sending(P parentObject, T modelObject) {
+	}
+
 	public void failed(P object, T child, IStatus status) {
 		failure = status;
 		throw new RuntimeException(failure.getException());
 	}
 
-	protected void waitForResponse(int response, int update) {
+	protected void waitForResponse(int expectedResponses, int expectedUpdate) {
+		waitForResponse(expectedResponses, expectedUpdate, 0);
+	}
+
+	protected void waitForResponse(int expectedResponses, int expectedUpdate, int expectedSents) {
 		long delay;
 		delay = 0;
 		while (delay < TEST_TIMEOUT) {
-			if (responded < response || updated < update) {
+			if (responded < expectedResponses || updated < expectedUpdate || sent < expectedSents) {
 				try {
 					Thread.sleep(10);
 					delay += 10;
@@ -76,8 +91,9 @@
 		} catch (InterruptedException e) {
 			Thread.currentThread().interrupt();
 		}
-		assertThat("Wrong # responses: " + responded + ", updated: " + updated, responded, is(response));
-		assertThat("Wrong # updates" + updated, updated, is(update));
+		assertThat("Wrong # responses: " + responded + ", updated: " + updated, responded, is(expectedResponses));
+		assertThat("Wrong # updates" + updated, updated, is(expectedUpdate));
+		assertThat("Wrong # sents" + sent, sent, is(expectedSents));
 		if (factory != null) {
 			assertThat(factory.getService().isActive(), is(false));
 		}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteService.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteService.java
index 8f80837..dd49612 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteService.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/TestRemoteService.java
@@ -46,4 +46,8 @@
 	public void dispose() {
 	}
 
+	@Override
+	public void send(AbstractRemoteConsumer process, boolean force) {
+		//noop -- send not supported or tested
+	}
 }
diff --git a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfFactoryTest.java b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfFactoryTest.java
index f597c1b..1497f2d 100644
--- a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfFactoryTest.java
+++ b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfFactoryTest.java
@@ -385,6 +385,12 @@
 			// ignore
 			return null;
 		}
+
+		@Override
+		public void push(TestRemoteObject remoteObject, IProgressMonitor monitor) throws CoreException {
+			// ignore
+
+		}
 	}
 
 	@Test
@@ -500,4 +506,24 @@
 				"Local Object 1");
 		assertThat(modelObject.getInstanceTypeName(), is("newData"));
 	}
+
+	@Test
+	public void testRemotePush() {
+		TestRemoteFactory.remoteValue.clear();
+		TestManagerHarness harness = new TestManagerHarness(new TestPullCreateOnlyFactory()) {
+			@Override
+			RemoteEmfConsumer<EPackage, EClass, String, TestRemoteObject, String, Integer> createConsumer() {
+				return factory.getConsumerForRemoteKey(parent, "remoteKeyFor Object 1");
+			}
+		};
+		harness.consumer.retrieve(false);
+		harness.listener.waitForResponse(1, 1);
+		checkConsumer(harness.consumer, "remoteKeyFor Object 1", "Remote Object 1", "localKeyFor Object 1",
+				"Local Object 1");
+		assertThat(TestRemoteFactory.remoteValue.get(TestRemoteFactory.remote1), nullValue());
+		TestRemoteFactory.remote1.data = "pushData";
+		harness.consumer.send(false);
+		harness.listener.waitForResponse(2, 1, 1);
+		assertThat(TestRemoteFactory.remoteValue.get(TestRemoteFactory.remote1), is("pushData"));
+	}
 }
diff --git a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteServiceTest.java b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteServiceTest.java
index 4a0bc05..afe3b23 100644
--- a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteServiceTest.java
+++ b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteServiceTest.java
@@ -31,9 +31,13 @@
 
 	class Consumer extends AbstractRemoteConsumer {
 
-		boolean retrieve;
+		boolean pull;
 
-		boolean apply;
+		boolean applyModel;
+
+		boolean push;
+
+		boolean applyRemote;
 
 		boolean notify;
 
@@ -43,12 +47,22 @@
 
 		@Override
 		public void pull(boolean force, IProgressMonitor monitor) throws CoreException {
-			retrieve = true;
+			pull = true;
 		}
 
 		@Override
 		public void applyModel(boolean force) {
-			apply = true;
+			applyModel = true;
+		}
+
+		@Override
+		public void push(boolean force, IProgressMonitor monitor) throws CoreException {
+			push = true;
+		}
+
+		@Override
+		public void applyRemote(boolean force) {
+			applyRemote = true;
 		}
 
 		@Override
@@ -103,8 +117,8 @@
 		remoteService.retrieve(consumer, false);
 		consumer.waitForDone();
 		assertThat(consumer.status.getSeverity(), is(IStatus.OK));
-		assertThat(consumer.retrieve, is(true));
-		assertThat(consumer.apply, is(true));
+		assertThat(consumer.pull, is(true));
+		assertThat(consumer.applyModel, is(true));
 	}
 
 	@Test
@@ -115,8 +129,8 @@
 		remoteService.retrieve(consumer, false);
 		consumer.waitForDone();
 		assertThat(consumer.status.getSeverity(), is(IStatus.OK));
-		assertThat(consumer.retrieve, is(true));
-		assertThat(consumer.apply, is(true));
+		assertThat(consumer.pull, is(true));
+		assertThat(consumer.applyModel, is(true));
 	}
 
 	class BrokenConsumer extends Consumer {
@@ -133,8 +147,8 @@
 		remoteService.retrieve(consumer, false);
 		consumer.waitForDone();
 		assertThat(consumer.status.getSeverity(), is(IStatus.WARNING));
-		assertThat(consumer.retrieve, is(false));
-		assertThat(consumer.apply, is(false));
+		assertThat(consumer.pull, is(false));
+		assertThat(consumer.applyModel, is(false));
 	}
 
 	@Test
@@ -145,8 +159,8 @@
 		remoteService.retrieve(consumer, false);
 		consumer.waitForDone();
 		assertThat(consumer.status.getSeverity(), is(IStatus.ERROR));
-		assertThat(consumer.retrieve, is(false));
-		assertThat(consumer.apply, is(false));
+		assertThat(consumer.pull, is(false));
+		assertThat(consumer.applyModel, is(false));
 	}
 
 	Thread testThread;
@@ -155,58 +169,109 @@
 
 		@Override
 		public void modelExec(Runnable runnable, boolean block) {
-			testThread = new Thread(runnable, "Test Thread");
-			testThread.start();
+			if (block) {
+				runnable.run();
+			} else {
+				testThread = new Thread(runnable, "Model Thread");
+				testThread.start();
+			}
 		}
 	}
 
 	class ModelThreadConsumer extends Consumer {
 
-		Thread modelThread;
+		Thread applyModelThread;
 
-		Thread retrieveThread;
+		Thread pullThread;
+
+		Thread applyRemoteThread;
+
+		Thread pushThread;
 
 		@Override
 		public void pull(boolean force, IProgressMonitor monitor) throws CoreException {
-			retrieveThread = Thread.currentThread();
+			pullThread = Thread.currentThread();
 			super.pull(force, monitor);
 		}
 
 		@Override
 		public void applyModel(boolean force) {
-			modelThread = Thread.currentThread();
+			applyModelThread = Thread.currentThread();
 			super.applyModel(force);
 		}
+
+		@Override
+		public void push(boolean force, IProgressMonitor monitor) throws CoreException {
+			pushThread = Thread.currentThread();
+			super.push(force, monitor);
+		}
+
+		@Override
+		public void applyRemote(boolean force) {
+			applyRemoteThread = Thread.currentThread();
+			super.applyRemote(force);
+		}
 	}
 
 	@Test
-	public void testExecuteModelThread() throws CoreException {
+	public void testExecuteModelThreadRetrieve() throws CoreException {
 		JobRemoteService remoteService = new ThreadedService();
 		ModelThreadConsumer consumer = new ModelThreadConsumer();
 		remoteService.retrieve(consumer, false);
 		consumer.waitForDone();
-		assertThat(consumer.modelThread.getName(), is("Test Thread"));
-		assertThat(consumer.retrieveThread.getName(), not("Test Thread"));
+		assertThat(consumer.applyModelThread.getName(), is("Model Thread"));
+		assertThat(consumer.pullThread.getName(), not("Model Thread"));
 		assertThat(consumer.status.getSeverity(), is(IStatus.OK));
-		assertThat(consumer.retrieve, is(true));
-		assertThat(consumer.apply, is(true));
+		assertThat(consumer.pull, is(true));
+		assertThat(consumer.applyModel, is(true));
 
-		assertThat(consumer.retrieveThread, not(Thread.currentThread()));
+		assertThat(consumer.pullThread, not(Thread.currentThread()));
 	}
 
 	@Test
-	public void testExecuteModelThreadSync() throws CoreException {
+	public void testExecuteModelThreadRetrieveSync() throws CoreException {
 		JobRemoteService remoteService = new ThreadedService();
 		ModelThreadConsumer consumer = new ModelThreadConsumer();
 		consumer.async = false;
 		remoteService.retrieve(consumer, false);
 		consumer.waitForDone();
-		assertThat(consumer.modelThread.getName(), is("Test Thread"));
-		assertThat(consumer.retrieveThread.getName(), not("Test Thread"));
+		assertThat(consumer.applyModelThread.getName(), is("main"));
+		assertThat(consumer.pullThread.getName(), not("Model Thread"));
 		assertThat(consumer.status.getSeverity(), is(IStatus.OK));
-		assertThat(consumer.retrieve, is(true));
-		assertThat(consumer.apply, is(true));
+		assertThat(consumer.pull, is(true));
+		assertThat(consumer.applyModel, is(true));
 
-		assertThat(consumer.retrieveThread, is(Thread.currentThread()));
+		assertThat(consumer.pullThread, is(Thread.currentThread()));
+	}
+
+	@Test
+	public void testExecuteModelThreadSend() throws CoreException {
+		JobRemoteService remoteService = new ThreadedService();
+		ModelThreadConsumer consumer = new ModelThreadConsumer();
+		remoteService.send(consumer, false);
+		consumer.waitForDone();
+		assertThat(consumer.applyRemoteThread.getName(), is("Model Thread"));
+		assertThat(consumer.pushThread.getName(), not("Model Thread"));
+		assertThat(consumer.status.getSeverity(), is(IStatus.OK));
+		assertThat(consumer.push, is(true));
+		assertThat(consumer.applyRemote, is(true));
+
+		assertThat(consumer.pullThread, not(Thread.currentThread()));
+	}
+
+	@Test
+	public void testExecuteModelThreadSendSync() throws CoreException {
+		JobRemoteService remoteService = new ThreadedService();
+		ModelThreadConsumer consumer = new ModelThreadConsumer();
+		consumer.async = false;
+		remoteService.send(consumer, false);
+		consumer.waitForDone();
+		assertThat(consumer.applyRemoteThread.getName(), is("main"));
+		assertThat(consumer.pushThread.getName(), not("Model Thread"));
+		assertThat(consumer.status.getSeverity(), is(IStatus.OK));
+		assertThat(consumer.push, is(true));
+		assertThat(consumer.applyRemote, is(true));
+
+		assertThat(consumer.pushThread, is(Thread.currentThread()));
 	}
 }
diff --git a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestIRemoteEmfObserver.java b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestIRemoteEmfObserver.java
index 4747556..34a7b4d 100644
--- a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestIRemoteEmfObserver.java
+++ b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestIRemoteEmfObserver.java
@@ -27,6 +27,8 @@
 
 	int responded;
 
+	int sent;
+
 	IStatus failure;
 
 	AbstractRemoteEmfFactory<P, T, ?, ?, L, C> factory;
@@ -42,6 +44,15 @@
 	public void updating(P parent, T object) {
 	}
 
+	public void sent(P parentObject, T modelObject) {
+		responded++;
+		sent++;
+	}
+
+	@Override
+	public void sending(P parentObject, T modelObject) {
+	}
+
 	public void updated(P object, T child, boolean modified) {
 		responded++;
 		if (modified) {
@@ -53,11 +64,15 @@
 		failure = status;
 	}
 
-	protected void waitForResponse(int response, int update) {
+	protected void waitForResponse(int expectedResponses, int expectedUpdate) {
+		waitForResponse(expectedResponses, expectedUpdate, 0);
+	}
+
+	protected void waitForResponse(int expectedResponses, int expectedUpdate, int expectedSents) {
 		long delay;
 		delay = 0;
 		while (delay < TEST_TIMEOUT) {
-			if (responded < response) {
+			if (responded < expectedResponses || updated < expectedUpdate || sent < expectedSents) {
 				try {
 					Thread.sleep(10);
 					delay += 10;
@@ -69,11 +84,13 @@
 		}
 		try {
 			//wait extra to ensure there aren't remaining jobs
-			Thread.sleep(100);
+			Thread.sleep(25);
 		} catch (InterruptedException e) {
+			Thread.currentThread().interrupt();
 		}
-		assertThat("Wrong # responses", responded, is(response));
-		assertThat("Wrong # updates", updated, is(update));
+		assertThat("Wrong # responses: " + responded + ", updated: " + updated, responded, is(expectedResponses));
+		assertThat("Wrong # updates" + updated, updated, is(expectedUpdate));
+		assertThat("Wrong # sents" + sent, sent, is(expectedSents));
 		if (factory != null) {
 			assertThat(factory.getService().isActive(), is(false));
 		}
diff --git a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestRemoteFactory.java b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestRemoteFactory.java
index cba5c59..78715f5 100644
--- a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestRemoteFactory.java
+++ b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/TestRemoteFactory.java
@@ -29,6 +29,8 @@
 
 	static Map<String, TestRemoteObject> remoteForKey = new HashMap<String, TestRemoteObject>();
 
+	static Map<TestRemoteObject, String> remoteValue = new HashMap<TestRemoteObject, String>();
+
 	Integer currentVal;
 
 	{
@@ -47,6 +49,16 @@
 	}
 
 	@Override
+	public void push(TestRemoteObject remoteObject, IProgressMonitor monitor) throws CoreException {
+		remoteValue.put(remoteObject, remoteObject.data);
+	}
+
+	@Override
+	public boolean isPushNeeded(EPackage parent, EClass object, TestRemoteObject remote) {
+		return true;
+	}
+
+	@Override
 	protected EClass createModel(EPackage parent, TestRemoteObject remoteObject) {
 		EClass clazz = EcoreFactory.eINSTANCE.createEClass();
 		clazz.setName(remoteObject.name.replaceAll("Remote", "Local"));
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteConsumer.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteConsumer.java
index 99f5620..917854f 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteConsumer.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteConsumer.java
@@ -36,6 +36,17 @@
 	public abstract void pull(boolean force, IProgressMonitor monitor) throws CoreException;
 
 	/**
+	 * Override to perform the request against remote API, taking the state (e.g. as member(s) field of an implementing
+	 * class) and updating the remote server with it.
+	 * 
+	 * @param force
+	 *            push to remote even when API doesn't require
+	 * @param monitor
+	 * @throws CoreException
+	 */
+	public abstract void push(boolean force, IProgressMonitor monitor) throws CoreException;
+
+	/**
 	 * Override to apply the remotely obtained state to a local model object. This method is expected to execute
 	 * <em>very</em> quickly, as the typical implementation will occur on the UI thread.
 	 * 
@@ -46,6 +57,14 @@
 	public abstract void applyModel(boolean force);
 
 	/**
+	 * Override to update the remote state with the current contents of the model object. This method is expected to
+	 * execute <em>very</em> quickly, as the typical implementation will occur on the UI thread.
+	 * 
+	 * @throws CoreException
+	 */
+	public abstract void applyRemote(boolean force);
+
+	/**
 	 * Provides notification of failure. See {@link AbstractRemoteService#retrieve(AbstractRemoteProcess, boolean)} for
 	 * details.
 	 */
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteService.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteService.java
index f0f4a4d..a33374d 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteService.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/AbstractRemoteService.java
@@ -28,7 +28,7 @@
 	 * <ol>
 	 * <li>The pull phase of the process is invoked. This invocation must be asynchronous if
 	 * {@link AbstractRemoteConsumer#isAsynchronous()} is true.</li>
-	 * <li>If a failure occurs or a core exception is thrown during the request phase,
+	 * <li>If a failure occurs or a core exception is thrown during the pull phase,
 	 * {@link AbstractRemoteConsumer#notifyDone(org.eclipse.core.runtime.IStatus)} is invoked with the exception.</li>
 	 * <li>Otherwise, when the request process returns, the {@link AbstractRemoteConsumer#applyModel(boolean)} phase of
 	 * the process is invoked. (In the case of the UI implementations, this might occur on the UI thread.)</li>
@@ -46,6 +46,31 @@
 	public abstract void retrieve(final AbstractRemoteConsumer process, boolean force);
 
 	/**
+	 * Implementors should invoke the the {@link AbstractRemoteConsumer#push(org.eclipse.core.runtime.IProgressMonitor)}
+	 * and {@link AbstractRemoteConsumer#applyRemote(boolean)} methods for the supplied process in the following well
+	 * defined way. This method is expected to return very quickly and <em>must not block</em> under any circumstances,
+	 * as it must be safely callable from the UI thread. Implementors should support one full cycle of invocation:
+	 * <ol>
+	 * <li>The updateRemote phase of the process is invoked. This method is expected to execute very quickly and
+	 * synchronously. (In the case of the UI implementations, this might occur on the UI thread.)</li>
+	 * <li>The push phase of the process is invoked.</li> This invocation must be asynchronous if
+	 * {@link AbstractRemoteConsumer#isAsynchronous()} is true.</li>
+	 * <li>If a failure occurs or a core exception is thrown during the push phase,
+	 * {@link AbstractRemoteConsumer#notifyDone(org.eclipse.core.runtime.IStatus)} is invoked with the exception.</li>
+	 * <li>If a failure occurs during the create phase, notifyDone may optionally be invoked to report the failure.</li>
+	 * <li>If both phases complete successfully,
+	 * {@link AbstractRemoteConsumer#notifyDone(org.eclipse.core.runtime.IStatus)} is invoked on the process with an OK
+	 * status.</li>
+	 * </ol>
+	 * 
+	 * @param process
+	 *            The consumer process to execute
+	 * @param force
+	 *            Invoke the push and apply processes even if the relevant APIs indicate that they are not needed.
+	 */
+	public abstract void send(final AbstractRemoteConsumer process, boolean force);
+
+	/**
 	 * Fail fast if we aren't in current model thread.
 	 */
 	public abstract void ensureModelThread();
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/JobRemoteService.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/JobRemoteService.java
index 6af6bc7..478cd16 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/JobRemoteService.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/JobRemoteService.java
@@ -53,7 +53,7 @@
 	@Override
 	public void retrieve(final AbstractRemoteConsumer process, final boolean force) {
 		if (process.isAsynchronous()) {
-			final Job job = new Job(process.getDescription()) {
+			final Job job = new Job("[Pull]" + process.getDescription()) {
 				@Override
 				protected IStatus run(IProgressMonitor monitor) {
 					try {
@@ -82,11 +82,6 @@
 				}
 			});
 			addJob(job);
-//			if (process.isSystemJob()) {
-//				job.setSystem(true);
-//			} else if (process.isUserJob()) {
-//				job.setUser(true);
-//			}
 			job.schedule();
 		} else {
 			try {
@@ -104,6 +99,68 @@
 		}
 	}
 
+	/**
+	 * Fully implements the {@link AbstractRemoteService#send(AbstractRemoteConsumer, boolean)} contract:
+	 * <ol>
+	 * <li>Invokes {@link AbstractRemoteConsumer#applyRemote(org.eclipse.core.runtime.IProgressMonitor)} inside of a
+	 * modelExec call, so that extending classes can manage thread context.</li>
+	 * <li>If {@link AbstractRemoteConsumer#isAsynchronous()}, creates and runs a job to
+	 * {@link AbstractRemoteConsumer#pull(org.eclipse.core.runtime.IProgressMonitor)} the remote API data. Otherwise,
+	 * simply calls pull.</li>
+	 * <li>If a failure occurs, calls {@link AbstractRemoteConsumer#notifyDone(org.eclipse.core.runtime.IStatus)}.</li>
+	 * <li>(No notification occurs in the case of an error while applying.)</li>
+	 * </ol>
+	 */
+	@Override
+	public void send(final AbstractRemoteConsumer process, final boolean force) {
+		if (process.isAsynchronous()) {
+			modelExec(new Runnable() {
+				public void run() {
+					process.applyRemote(force);
+					final Job job = new Job("[Push]" + process.getDescription()) {
+						@Override
+						protected IStatus run(IProgressMonitor monitor) {
+							try {
+								process.push(force, monitor);
+							} catch (CoreException e) {
+								return new Status(IStatus.WARNING, "org.eclipse.mylyn.reviews.core",
+										"Couldn't push model.", e);
+							} catch (OperationCanceledException e) {
+								return Status.CANCEL_STATUS;
+							}
+							return Status.OK_STATUS;
+						}
+					};
+					job.addJobChangeListener(new JobChangeAdapter() {
+						@Override
+						public void done(final IJobChangeEvent event) {
+							modelExec(new Runnable() {
+								public void run() {
+									process.notifyDone(event.getResult());
+								}
+							}, false);
+						}
+					});
+					addJob(job);
+					job.schedule();
+				}
+			}, false);
+		} else {
+			modelExec(new Runnable() {
+				public void run() {
+					process.applyRemote(force);
+				}
+			}, true);
+			try {
+				process.push(force, new NullProgressMonitor());
+			} catch (CoreException e) {
+				process.notifyDone(e.getStatus());
+				return;
+			}
+			process.notifyDone(Status.OK_STATUS);
+		}
+	}
+
 	private void addJob(final Job job) {
 		synchronized (jobs) {
 			jobs.add(job);
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java
index 62aabc1..866c526 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java
@@ -20,7 +20,6 @@
 import org.eclipse.emf.ecore.EAttribute;
 import org.eclipse.emf.ecore.EObject;
 import org.eclipse.emf.ecore.EReference;
-import org.eclipse.mylyn.reviews.core.model.IReview;
 import org.eclipse.mylyn.reviews.core.spi.remote.AbstractRemoteService;
 
 /**
@@ -396,6 +395,36 @@
 	}
 
 	/**
+	 * Override to perform actual push to remote API. This request is fully managed by remote service and could be
+	 * invoked directly, but is typically invoked through a consumer.
+	 * <em>This method may block or fail, and must not be called from UI thread.</em>
+	 * 
+	 * @param parentObject
+	 *            The object that contains the model object
+	 * @param remoteKey
+	 *            A unique identifier in the target API
+	 * @param monitor
+	 * @throws CoreException
+	 */
+	public abstract void push(RemoteType remoteObject, IProgressMonitor monitor) throws CoreException;
+
+	/**
+	 * Override to return true if the remote object state has changed and needs to update to remote API.
+	 * 
+	 * @param parentObject
+	 *            The object that contains the model object
+	 * @param modelObject
+	 *            The model object to test
+	 * @param remoteObject
+	 *            A unique identifier in the target API
+	 * @param monitor
+	 * @return
+	 */
+	public boolean isPushNeeded(EParentObjectType parent, EObjectType object, RemoteType remote) {
+		return false;
+	}
+
+	/**
 	 * Override to create an EObject from remote object. (Consumers should use
 	 * {@link #get(EParentObjectType parent, RemoteType remoteObject)}, which ensures that any cached objects will be
 	 * returned instead.)
@@ -427,8 +456,8 @@
 
 	/**
 	 * Updates the values for the supplied EMF object based on any values that have changed in the remote object since
-	 * the last call to {@link #retrieve(String, EObject, EReference, Object)} or {@link #update(Object)}. The object
-	 * must have been previously retrieved using this factory.
+	 * the last call to {@link #pull(EObject, Object, IProgressMonitor)} or
+	 * {@link #updateModel(EObject, Object, Object)}. The object must have been previously retrieved using this factory.
 	 * <em>Must be called from EMF safe (e.g. UI) thread and should have very fast execution time.</em>
 	 * 
 	 * @param parentObject
@@ -442,6 +471,21 @@
 	}
 
 	/**
+	 * Updates the values for the remote object based on any values that have changed in model object since the last
+	 * call to {@link #updateRemote(Object)}.
+	 * <em>Must be called from EMF safe (e.g. UI) thread and should have very fast execution time.</em>
+	 * 
+	 * @param parentObject
+	 *            the parent EMF object that the new child object is referenced from
+	 * @param modelObject
+	 *            The model object to update -- must currently exist in the parent
+	 * @return true if the remote object has changed or the object delta is unknown, false otherwise
+	 */
+	public boolean updateRemote(EParentObjectType parentObject, EObjectType modelObject, RemoteType remoteObject) {
+		return false;
+	}
+
+	/**
 	 * Override to return true if a remote object to model object update should occur, e.g. when the remote object state
 	 * is more recent then the model object state. Return true by default and generally doesn't need to be overridden as
 	 * most update operations should be inexpensive.
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/IRemoteEmfObserver.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/IRemoteEmfObserver.java
index 097660c..5dce709 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/IRemoteEmfObserver.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/IRemoteEmfObserver.java
@@ -60,6 +60,27 @@
 	void updating(EParentObjectType parentObject, EObjectType modelObject);
 
 	/**
+	 * Called whenever a remote API's state has been updated from the remote object
+	 * 
+	 * @param parentObject
+	 *            The parent of the supplied object
+	 * @param modelObject
+	 *            The sent object
+	 */
+	void sent(EParentObjectType parentObject, EObjectType modelObject);
+
+	/**
+	 * Called whenever a model object beings sending it's state to a remote object.
+	 * {@link RemoteEmfConsumer#send(boolean)}.
+	 * 
+	 * @param parentObject
+	 *            The parent of the supplied object
+	 * @param modelObject
+	 *            The object being sent
+	 */
+	void sending(EParentObjectType parentObject, EObjectType modelObject);
+
+	/**
 	 * Called whenever a failure has occurred while attempting to retrieve a remote object.
 	 * 
 	 * @param parentObject
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfConsumer.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfConsumer.java
index e3dd563..8adf3d8 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfConsumer.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfConsumer.java
@@ -17,6 +17,7 @@
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.emf.common.notify.Notification;
 import org.eclipse.emf.common.notify.NotificationChain;
 import org.eclipse.emf.common.notify.impl.AdapterImpl;
@@ -59,6 +60,8 @@
 
 	private boolean retrieving;
 
+	private boolean sending;
+
 	boolean userJob;
 
 	boolean systemJob;
@@ -91,6 +94,12 @@
 							case RemoteNotification.REMOTE_UPDATE:
 								listener.updated(parentObject, modelObject, remoteMessage.isModification());
 								break;
+							case RemoteNotification.REMOTE_SENDING:
+								listener.sending(parentObject, modelObject);
+								break;
+							case RemoteNotification.REMOTE_SEND:
+								listener.sent(parentObject, modelObject);
+								break;
 							case RemoteNotification.REMOTE_MEMBER_FAILURE:
 							case RemoteNotification.REMOTE_FAILURE:
 								listener.failed(parentObject, modelObject, remoteMessage.getStatus());
@@ -193,6 +202,44 @@
 		return pulling;
 	}
 
+	@Override
+	public void push(boolean force, IProgressMonitor monitor) throws CoreException {
+		if (remoteObject == null) {
+			throw new CoreException(new Status(IStatus.ERROR, "org.eclipse.mylyn.reviews.core",
+					"Tried to push without a remote object!"));
+		}
+		if (modelObject == null) {
+			throw new CoreException(new Status(IStatus.ERROR, "org.eclipse.mylyn.reviews.core",
+					"Tried to push without a model object!"));
+		}
+		if (modelObject instanceof EObject) {
+			getFactory().getService().modelExec(new Runnable() {
+				@Override
+				public void run() {
+					((EObject) modelObject).eNotify(new RemoteENotificationImpl((InternalEObject) modelObject,
+							RemoteNotification.REMOTE_SENDING, null, null));
+				}
+			});
+		}
+		if (factory.isPushNeeded(parentObject, modelObject, remoteObject)) {
+			factory.push(remoteObject, monitor);
+		}
+		if (modelObject instanceof EObject) {
+			getFactory().getService().modelExec(new Runnable() {
+				@Override
+				public void run() {
+					((EObject) modelObject).eNotify(new RemoteENotificationImpl((InternalEObject) modelObject,
+							RemoteNotification.REMOTE_SEND, null, null));
+				}
+			});
+		}
+	}
+
+	@Override
+	public void applyRemote(boolean force) {
+		factory.updateRemote(parentObject, modelObject, remoteObject);
+	}
+
 	/**
 	 * Apply the remote object to the local model object.
 	 * <em>This method must be called from the EMF managed (e.g.) UI thread.</em>
@@ -290,12 +337,29 @@
 	}
 
 	/**
+	 * Performs a complete remote send, updating the remote value and then pushing it to the Remote API.
+	 * <ol>
+	 * 
+	 * @param force
+	 *            Forces update and push, even if factory methods
+	 *            {@link AbstractRemoteEmfFactory#isPushNeeded(EObject, Object, Object)} returns false.
+	 */
+	public void send(boolean force) {
+		if (sending) {
+			return;
+		}
+		sending = true;
+		getFactory().getService().send(this, force);
+	}
+
+	/**
 	 * Notifies the consumer that a failure has occurred while performing a retrieval. (Consumers should generally
 	 * handle update and failure notifications through the {@link IRemoteEmfObserver#failed(IStatus)} method instead.)
 	 */
 	@Override
 	public void notifyDone(IStatus status) {
 		retrieving = false;
+		sending = false;
 	}
 
 	/**
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfObserver.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfObserver.java
index 88828ef..becc78b 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfObserver.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfObserver.java
@@ -50,6 +50,12 @@
 	public void updated(EParentObjectType parentObject, EObjectType modelObject, boolean modified) {
 	}
 
+	public void sent(EParentObjectType parentObject, EObjectType modelObject) {
+	}
+
+	public void sending(EParentObjectType parentObject, EObjectType modelObject) {
+	}
+
 	public void failed(EParentObjectType parentObject, EObjectType modelObject, IStatus status) {
 	}
 
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteNotification.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteNotification.java
index 7b372ef..4eb6ead 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteNotification.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteNotification.java
@@ -70,6 +70,17 @@
 	int REMOTE_FAILURE = 1014;
 
 	/**
+	 * An {@link Notification#getEventType event type} indicating that the object is in the process of sending an update
+	 * to the remote source.
+	 */
+	int REMOTE_SENDING = 1020;
+
+	/**
+	 * An {@link Notification#getEventType event type} indicating that the remote source has just been updated.
+	 */
+	int REMOTE_SEND = 1021;
+
+	/**
 	 * Returns true if a remote operation has just completed. This is true for REMOTE_MEMBER_UPDATE,
 	 * REMOTE_MEMBER_FAILURE, REMOTE_UPDATE and REMOTE_FAILURE.
 	 *