From 225795b8abafdd00b2dd153c09898975847c689f Mon Sep 17 00:00:00 2001
From: pkupczyk <pkupczyk>
Date: Tue, 25 Sep 2012 14:49:57 +0000
Subject: [PATCH] BIS-185 / Make long-running method calls in IDataStoreService
use service conversations BIS-196 / Make service conversations timeout
configurable - junit tests and bugfixes
SVN: 26801
---
.../systemtests/ServiceConversationTest.java | 108 +++++++++++++++++-
.../BaseServiceConversationServerManager.java | 4 +-
2 files changed, 107 insertions(+), 5 deletions(-)
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/ServiceConversationTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/ServiceConversationTest.java
index e6efff359d9..dad63f2ce86 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/ServiceConversationTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/ServiceConversationTest.java
@@ -24,8 +24,13 @@ import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.nio.SelectChannelConnector;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
+import org.hamcrest.Description;
import org.jmock.Expectations;
import org.jmock.Mockery;
+import org.jmock.api.Action;
+import org.jmock.api.Invocation;
+import org.jmock.lib.action.DoAllAction;
+import org.jmock.lib.action.ReturnValueAction;
import org.springframework.beans.MutablePropertyValues;
import org.springframework.beans.factory.support.GenericBeanDefinition;
import org.springframework.remoting.RemoteAccessException;
@@ -45,6 +50,7 @@ import ch.systemsx.cisd.common.concurrent.ConcurrencyUtilities;
import ch.systemsx.cisd.common.conversation.annotation.Conversational;
import ch.systemsx.cisd.common.conversation.annotation.Progress;
import ch.systemsx.cisd.common.conversation.client.ServiceConversationClientDetails;
+import ch.systemsx.cisd.common.conversation.context.ServiceConversationsThreadContext;
import ch.systemsx.cisd.common.conversation.manager.BaseServiceConversationClientManager;
import ch.systemsx.cisd.common.conversation.manager.BaseServiceConversationServerManager;
import ch.systemsx.cisd.common.conversation.manager.IServiceConversationClientManagerRemote;
@@ -178,6 +184,20 @@ public class ServiceConversationTest
assertNoMoreConversations();
}
+ @Test
+ public void testMethodWithNullReturnValue()
+ {
+ context.checking(new Expectations()
+ {
+ {
+ one(serviceOnServerSideWrapper1.getService()).methodWithObjectReturnValue();
+ will(returnValue(null));
+ }
+ });
+ Assert.assertNull(getServiceOnClientSide1(TestService1.class).methodWithObjectReturnValue());
+ assertNoMoreConversations();
+ }
+
@Test
public void testMethodWithPrimitiveReturnValue()
{
@@ -230,6 +250,19 @@ public class ServiceConversationTest
}
}
+ @Test
+ public void testMethodWithNullParameter()
+ {
+ context.checking(new Expectations()
+ {
+ {
+ one(serviceOnServerSideWrapper2.getService()).methodWithObjectParameter(null);
+ }
+ });
+ getServiceOnClientSide1(TestService2.class).methodWithObjectParameter(null);
+ assertNoMoreConversations();
+ }
+
@Test
public void testMethodWithPrimitiveParameter()
{
@@ -270,19 +303,35 @@ public class ServiceConversationTest
}
}
+ @Test
+ public void testMethodWithAutomaticProgressShouldNotTimeout()
+ {
+ context.checking(new Expectations()
+ {
+ {
+ one(serviceOnServerSideWrapper1.getService()).methodWithAutomaticProgress();
+ will(new DoAllAction(new WaitAction(2 * TIMEOUT), new ReturnValueAction(1)));
+
+ }
+ });
+ Assert.assertEquals(getServiceOnClientSide1(TestService1.class)
+ .methodWithAutomaticProgress(), 1);
+ assertNoMoreConversations();
+ }
+
@Test(expectedExceptions = TimeoutExceptionUnchecked.class)
- public void testTimeout()
+ public void testMethodWithManualProgressShouldTimeoutWhenProgressIsNotReported()
{
try
{
context.checking(new Expectations()
{
{
- one(serviceOnServerSideWrapper1.getService()).methodWithoutReturnValue();
+ one(serviceOnServerSideWrapper1.getService()).methodWithManualProgress();
will(new WaitAction(2 * TIMEOUT));
}
});
- getServiceOnClientSide1(TestService1.class).methodWithoutReturnValue();
+ getServiceOnClientSide1(TestService1.class).methodWithManualProgress();
} finally
{
// wait for the server to wake up and clean up the conversation
@@ -290,6 +339,41 @@ public class ServiceConversationTest
}
}
+ @Test
+ public void testMethodWithManualProgressShouldNotTimeoutWhenProgressIsReported()
+ {
+ context.checking(new Expectations()
+ {
+ {
+ one(serviceOnServerSideWrapper1.getService()).methodWithManualProgress();
+ will(new Action()
+ {
+
+ @Override
+ public Object invoke(Invocation invocation) throws Throwable
+ {
+ for (int i = 1; i <= 10; i++)
+ {
+ ServiceConversationsThreadContext.getProgressListener().update(
+ "manualProgress", 10, i);
+ ConcurrencyUtilities.sleep(TIMEOUT / 2);
+ }
+ return null;
+ }
+
+ @Override
+ public void describeTo(Description description)
+ {
+ description
+ .appendText("processing method with manual progress reporting");
+ }
+ });
+ }
+ });
+ getServiceOnClientSide1(TestService1.class).methodWithManualProgress();
+ assertNoMoreConversations(5 * TIMEOUT);
+ }
+
@Test(expectedExceptions = IllegalArgumentException.class)
public void testUnknownClient()
{
@@ -506,6 +590,12 @@ public class ServiceConversationTest
@Conversational(progress = Progress.MANUAL)
public Object methodWithObjectReturnValue();
+ @Conversational(progress = Progress.MANUAL)
+ public Object methodWithManualProgress();
+
+ @Conversational(progress = Progress.AUTOMATIC)
+ public Object methodWithAutomaticProgress();
+
@Conversational(progress = Progress.MANUAL)
public Object echo(Object parameter);
@@ -567,6 +657,18 @@ public class ServiceConversationTest
return service.methodWithObjectReturnValue();
}
+ @Override
+ public Object methodWithManualProgress()
+ {
+ return service.methodWithManualProgress();
+ }
+
+ @Override
+ public Object methodWithAutomaticProgress()
+ {
+ return service.methodWithAutomaticProgress();
+ }
+
@Override
public Object echo(Object parameter)
{
diff --git a/openbis-common/source/java/ch/systemsx/cisd/common/conversation/manager/BaseServiceConversationServerManager.java b/openbis-common/source/java/ch/systemsx/cisd/common/conversation/manager/BaseServiceConversationServerManager.java
index 28f4cad2a9b..324029db26a 100644
--- a/openbis-common/source/java/ch/systemsx/cisd/common/conversation/manager/BaseServiceConversationServerManager.java
+++ b/openbis-common/source/java/ch/systemsx/cisd/common/conversation/manager/BaseServiceConversationServerManager.java
@@ -66,8 +66,8 @@ public abstract class BaseServiceConversationServerManager implements
} else
{
// try to report progress 10 times within the timeout time,
- // but keep the interval in range(1, 60) seconds
- return Math.min(Math.max(clientDetails.getTimeout() / 10, 1000),
+ // but keep the interval in range(1ms, 60sec)
+ return Math.min(Math.max(clientDetails.getTimeout() / 10, 1),
60 * 1000);
}
}
--
GitLab