From a179ac89b02509c1ccb2de6117e8d820121dd8d1 Mon Sep 17 00:00:00 2001 From: Cherian Mathew Date: Thu, 29 Oct 2015 12:12:27 +0100 Subject: [PATCH] #5297 Implement monitor feedback, Add corresponding tests #5297 Add inconsistent monitor feedback test --- .../etaxonomy/taxeditor/io/ImportManager.java | 3 +- .../taxeditor/model/AbstractUtility.java | 24 +++-- .../operation/IFeedbackGenerator.java | 23 +++++ .../util/ProgressMonitorClientManager.java | 55 +++++++++++- .../httpinvoker/RemotingSessionAwareTest.java | 1 + .../service/ProgressMonitorServiceTest.java | 89 ++++++++++++++++--- 6 files changed, 167 insertions(+), 28 deletions(-) create mode 100644 eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/operation/IFeedbackGenerator.java diff --git a/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/io/ImportManager.java b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/io/ImportManager.java index f2dcc0140..abdad0532 100644 --- a/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/io/ImportManager.java +++ b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/io/ImportManager.java @@ -241,7 +241,8 @@ public class ImportManager extends AbstractIOManager implem uuid, 1000, false, - ImportManager.this); + ImportManager.this, + null); } }); diff --git a/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/model/AbstractUtility.java b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/model/AbstractUtility.java index cdfbe19ae..e700d2e35 100644 --- a/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/model/AbstractUtility.java +++ b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/model/AbstractUtility.java @@ -66,6 +66,7 @@ import eu.etaxonomy.cdm.api.service.IProgressMonitorService; import eu.etaxonomy.cdm.common.monitor.IRemotingProgressMonitor; import eu.etaxonomy.cdm.model.common.IEnumTerm; import eu.etaxonomy.taxeditor.operation.AbstractPostOperation; +import eu.etaxonomy.taxeditor.operation.IFeedbackGenerator; import eu.etaxonomy.taxeditor.operation.IPostMoniteredOperationEnabled; import eu.etaxonomy.taxeditor.operation.IPostOperationEnabled; import eu.etaxonomy.taxeditor.operation.RemotingCdmHandler; @@ -420,7 +421,8 @@ public abstract class AbstractUtility { final UUID uuid, final int pollInterval, final boolean cancelable, - final IPostMoniteredOperationEnabled postOp) { + final IPostMoniteredOperationEnabled postOp, + final IFeedbackGenerator feedbackGenerator) { try { // get the remoting monitor the first time to make sure that the @@ -439,9 +441,14 @@ public abstract class AbstractUtility { // run the monitor until the operation is finished IRemotingProgressMonitor remotingMonitor; try { - remotingMonitor = CdmStore.getProgressMonitorClientManager().pollMonitor(label, uuid, pollInterval, postOp, monitor); - } catch (InterruptedException ie) { - return new Status(Status.ERROR, TaxeditorStorePlugin.PLUGIN_ID, "Operation Interrupted", ie); + remotingMonitor = CdmStore.getProgressMonitorClientManager().pollMonitor(label, + uuid, + pollInterval, + postOp, + feedbackGenerator, + monitor); + } catch (Exception ex) { + return new Status(Status.ERROR, TaxeditorStorePlugin.PLUGIN_ID, "Operation Interrupted", ex); } final StringBuilder reportSb = new StringBuilder(); // collect reports @@ -469,15 +476,6 @@ public abstract class AbstractUtility { } }; -// job.addJobChangeListener(new JobChangeAdapter() { -// @Override -// public void done(IJobChangeEvent event) { -// if(event.getJob().) { -// logger.warn("in jobc change listener"); -// } -// } -// }); - // configure the job job.setProperty(IProgressConstants.KEEP_PROPERTY, true); job.setUser(true); diff --git a/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/operation/IFeedbackGenerator.java b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/operation/IFeedbackGenerator.java new file mode 100644 index 000000000..a7cee6ae7 --- /dev/null +++ b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/operation/IFeedbackGenerator.java @@ -0,0 +1,23 @@ +// $Id$ +/** +* Copyright (C) 2015 EDIT +* European Distributed Institute of Taxonomy +* http://www.e-taxonomy.eu +* +* The contents of this file are subject to the Mozilla Public License Version 1.1 +* See LICENSE.TXT at the top of this package for the full license terms. +*/ +package eu.etaxonomy.taxeditor.operation; + +import java.io.Serializable; + +/** + * @author cmathew + * @date 28 Oct 2015 + * + */ +public interface IFeedbackGenerator { + + public Serializable generateFeedback(); + +} diff --git a/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/util/ProgressMonitorClientManager.java b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/util/ProgressMonitorClientManager.java index 62f00179d..c50686fa1 100644 --- a/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/util/ProgressMonitorClientManager.java +++ b/eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/util/ProgressMonitorClientManager.java @@ -10,6 +10,8 @@ package eu.etaxonomy.taxeditor.util; import java.text.DecimalFormat; +import java.util.Arrays; +import java.util.List; import java.util.UUID; import org.apache.log4j.Logger; @@ -18,6 +20,7 @@ import org.eclipse.core.runtime.IProgressMonitor; import eu.etaxonomy.cdm.api.application.CdmApplicationState; import eu.etaxonomy.cdm.api.service.IProgressMonitorService; import eu.etaxonomy.cdm.common.monitor.IRemotingProgressMonitor; +import eu.etaxonomy.taxeditor.operation.IFeedbackGenerator; import eu.etaxonomy.taxeditor.operation.IPostMoniteredOperationEnabled; /** @@ -31,6 +34,29 @@ import eu.etaxonomy.taxeditor.operation.IPostMoniteredOperationEnabled; public class ProgressMonitorClientManager { private static final Logger logger = Logger.getLogger(ProgressMonitorClientManager.class); + /** + * Polls the progress monitor service for the progress status of a monitor + * corresponding to the given uuid. + * + * @param label for the operation + * @param uuid of the remoting monitor already started on the server + * @param pollInterval in milliseconds + * @param cancelable flag which determines whether the operation can be cancelled + * @param postOp callback for running post operation logic + * @param feedbackGenerator feedback generator corresponding to the + * 'wait on feedback' request made on the remoting monitor + * @param monitor to be updated + * @return a final progress monitor after the operation is completed + * @throws InterruptedException + */ + public IRemotingProgressMonitor pollMonitor(final String label, + final UUID uuid, + final int pollInterval, + final IPostMoniteredOperationEnabled postOp, + IFeedbackGenerator feedbackGenerator, + IProgressMonitor monitor) throws InterruptedException { + return pollMonitor(label, uuid, pollInterval, postOp, Arrays.asList(feedbackGenerator), monitor); + } /** * Polls the progress monitor service for the progress status of a monitor * corresponding to the given uuid. @@ -41,6 +67,9 @@ public class ProgressMonitorClientManager { * @param pollInterval in milliseconds * @param cancelable flag which determines whether the operation can be cancelled * @param postOp callback for running post operation logic + * @param feedbackGenerators list of feedback generators corresponding to the + * size and exact order of the 'wait on feedback' requests made on the + * remoting monitor * @param monitor to be updated * @return a final progress monitor after the operation is completed * @throws InterruptedException @@ -49,12 +78,14 @@ public class ProgressMonitorClientManager { final UUID uuid, final int pollInterval, final IPostMoniteredOperationEnabled postOp, + List feedbackGenerators, IProgressMonitor monitor) throws InterruptedException { IProgressMonitorService progressMonitorService = CdmApplicationState.getCurrentAppConfig().getProgressMonitorService(); IRemotingProgressMonitor remotingMonitor = progressMonitorService.getRemotingMonitor(uuid); try { final int START_DELAY=10; - // wait about 10 seconds for the remoting monitor to be initialised (i.e. for the begin task method to be called) + // wait about 10 seconds for the remoting monitor to be initialised + // (i.e. for the begin task method to be called ON THE REMOTING MONITOR) for(int i=0;i feedbackGenerators.size()) { + throw new IllegalStateException("Remoting monitor waiting on feedback that does not exist"); + } + progressMonitorService.setFeedback(uuid, feedbackGenerators.get(feedbackCount).generateFeedback()); + feedbackCount++; + } + } serverTotalWorkDone = (int) remotingMonitor.getWorkDone(); - logger.warn("Work done from start: " + serverTotalWorkDone); + logger.info("Work done from start: " + serverTotalWorkDone); String percentage = new DecimalFormat("#.##").format(remotingMonitor.getPercentage()); + // set dialog text monitor.setTaskName(label + " " + percentage + "% done "); monitor.subTask(remotingMonitor.getSubTask()); int worked = serverTotalWorkDone - editorTotalWorkDone; @@ -92,9 +138,12 @@ public class ProgressMonitorClientManager { } editorTotalWorkDone = serverTotalWorkDone; } + if(remotingMonitor.getResult() instanceof Exception) { + throw new IllegalStateException((Exception)remotingMonitor.getResult()); + } return remotingMonitor; } finally { - if(postOp != null) { + if(postOp != null && remotingMonitor.isDone()) { postOp.postOperation(remotingMonitor); } } diff --git a/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/httpinvoker/RemotingSessionAwareTest.java b/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/httpinvoker/RemotingSessionAwareTest.java index 83734e9b0..1949ce3b8 100644 --- a/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/httpinvoker/RemotingSessionAwareTest.java +++ b/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/httpinvoker/RemotingSessionAwareTest.java @@ -51,6 +51,7 @@ public abstract class RemotingSessionAwareTest extends BaseRemotingTest { cacher = getCacher(sessionOwner); UserDetails extraUser = null; + try { extraUser = userService.loadUserByUsername(extraUsername); } catch (UsernameNotFoundException unfe) { diff --git a/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/service/ProgressMonitorServiceTest.java b/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/service/ProgressMonitorServiceTest.java index e132f71a2..b4d2c3344 100644 --- a/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/service/ProgressMonitorServiceTest.java +++ b/eu.etaxonomy.taxeditor.test/src/test/java/eu/etaxonomy/taxeditor/service/ProgressMonitorServiceTest.java @@ -9,6 +9,10 @@ */ package eu.etaxonomy.taxeditor.service; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.UUID; import org.apache.log4j.Logger; @@ -22,6 +26,7 @@ import eu.etaxonomy.cdm.api.service.ITestService; import eu.etaxonomy.cdm.common.monitor.IRemotingProgressMonitor; import eu.etaxonomy.cdm.common.monitor.RemotingProgressMonitor; import eu.etaxonomy.taxeditor.httpinvoker.RemotingSessionAwareTest; +import eu.etaxonomy.taxeditor.operation.IFeedbackGenerator; import eu.etaxonomy.taxeditor.operation.IPostMoniteredOperationEnabled; import eu.etaxonomy.taxeditor.store.CdmStore; @@ -47,16 +52,17 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { @Test public void testMonitLongRunningMethod() throws InterruptedException { - UUID uuid = testService.monitLongRunningMethod(null); + UUID uuid = testService.monitLongRunningMethod(null, null); int pollInterval = 1000; RemotingProgressMonitor expectedMonitor = new RemotingProgressMonitor(); expectedMonitor.setResult("Success"); expectedMonitor.addReport("Report"); expectedMonitor.done(); - IRemotingProgressMonitor remotingMonitor = CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", + CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", uuid, pollInterval, new MockPostMoniteredOperationEnabled(expectedMonitor, uuid), + (IFeedbackGenerator)null, new NullProgressMonitor()); } @@ -64,7 +70,7 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { public void testMonitLongRunningMethodByChangingUser() throws InterruptedException { IllegalStateException ise = new IllegalStateException("IllegalStateException"); - UUID uuid = testService.monitLongRunningMethod(ise); + UUID uuid = testService.monitLongRunningMethod(ise, null); authenticateExtraUser(); IRemotingProgressMonitor monitor = progressMonitorService.getRemotingMonitor(uuid); Assert.assertNull(monitor); @@ -74,23 +80,24 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { @Test public void testMonitLongRunningMethodWithException() throws InterruptedException { IllegalStateException ise = new IllegalStateException("IllegalStateException"); - UUID uuid = testService.monitLongRunningMethod(ise); + UUID uuid = testService.monitLongRunningMethod(ise, null); int pollInterval = 1000; RemotingProgressMonitor expectedMonitor = new RemotingProgressMonitor(); expectedMonitor.setResult(ise); expectedMonitor.setIsFailed(true); expectedMonitor.done(); - IRemotingProgressMonitor remotingMonitor = CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", + CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", uuid, pollInterval, new MockPostMoniteredOperationEnabled(expectedMonitor, uuid), + (IFeedbackGenerator)null, new NullProgressMonitor()); } @Test - public void testMonitLongRunningMethodWithInterrupt() throws InterruptedException { + public void testMonitLongRunningMethodWithInterrupt() { IllegalStateException ise = new IllegalStateException("Interrupted Exception"); - final UUID uuid = testService.monitLongRunningMethod(ise); + final UUID uuid = testService.monitLongRunningMethod(ise, null); final int pollInterval = 1000; final RemotingProgressMonitor expectedMonitor = new RemotingProgressMonitor(); expectedMonitor.setResult(ise); @@ -102,16 +109,16 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { @Override public void run() { try { - IRemotingProgressMonitor remotingMonitor = CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", + CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", uuid, pollInterval, null, + (IFeedbackGenerator)null, new NullProgressMonitor()); } catch (InterruptedException e) { } } - }; thread.start(); while(!progressMonitorService.isMonitorThreadRunning(uuid)) {} @@ -128,7 +135,7 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { @Test public void testMonitLongRunningMethodWithCancellation() throws InterruptedException { - final UUID uuid = testService.monitLongRunningMethod(null); + final UUID uuid = testService.monitLongRunningMethod(null, null); final int pollInterval = 1000; final RemotingProgressMonitor expectedMonitor = new RemotingProgressMonitor(); expectedMonitor.setResult("Cancelled"); @@ -139,10 +146,11 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { @Override public void run() { try { - IRemotingProgressMonitor remotingMonitor = CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", + CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", uuid, pollInterval, null, + (IFeedbackGenerator)null, new NullProgressMonitor()); } catch (InterruptedException e) { @@ -159,6 +167,49 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { } + @Test + public void testMonitLongRunningMethodWithWaitForFeedback() throws InterruptedException { + + List feedbacks = Arrays.asList("feedback1", "feedback2"); + List feebackGenerators = new ArrayList(); + final RemotingProgressMonitor expectedMonitor = new RemotingProgressMonitor(); + expectedMonitor.setResult("Success"); + for(String feedback : feedbacks) { + feebackGenerators.add(new MockFeedbackGenerator(feedback)); + expectedMonitor.addReport(feedback); + } + expectedMonitor.addReport("Report"); + expectedMonitor.done(); + + final UUID uuid = testService.monitLongRunningMethod(null, feedbacks); + final int pollInterval = 1000; + + CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", + uuid, + pollInterval, + new MockPostMoniteredOperationEnabled(expectedMonitor, uuid), + feebackGenerators, + new NullProgressMonitor()); + + feebackGenerators.remove(1); + + final UUID newUuid = testService.monitLongRunningMethod(null, feedbacks); + + try { + CdmStore.getProgressMonitorClientManager().pollMonitor("Testing Progress Monitor", + newUuid, + pollInterval, + new MockPostMoniteredOperationEnabled(expectedMonitor, newUuid), + feebackGenerators, + new NullProgressMonitor()); + Assert.fail("Exception due to inconsistent number of feedback generators not thrown"); + } catch(IllegalStateException ise) { + + } + + + } + class MockPostMoniteredOperationEnabled implements IPostMoniteredOperationEnabled { @@ -188,6 +239,22 @@ public class ProgressMonitorServiceTest extends RemotingSessionAwareTest { Assert.assertEquals(expectedMonitor.isDone(), monitor.isDone()); Assert.assertTrue(!progressMonitorService.isMonitorThreadRunning(monitorUuid)); } + } + + class MockFeedbackGenerator implements IFeedbackGenerator { + + private String feedback; + + public MockFeedbackGenerator(String feedback) { + this.feedback = feedback; + } + /** + * {@inheritDoc} + */ + @Override + public Serializable generateFeedback() { + return feedback; + } } } -- 2.34.1