From 07532fad61c7a98b0f3e8b262f025a06890362b5 Mon Sep 17 00:00:00 2001 From: Stefano Mazzocchi Date: Fri, 2 Jul 2010 09:31:02 +0000 Subject: [PATCH] more tests and more work on the broker git-svn-id: http://google-refine.googlecode.com/svn/trunk@1069 7d457c2a-affb-35e4-300a-418c747d4874 --- .../broker/AppEngineGridworksBrokerImpl.java | 11 +- .../gridworks/broker/GridworksBroker.java | 41 +++-- .../gridworks/broker/GridworksBrokerImpl.java | 32 ++-- .../broker/tests/GridworksBrokerTests.java | 157 +++++++++++++----- broker/core/tests/src/tests.log4j.properties | 2 +- 5 files changed, 171 insertions(+), 72 deletions(-) diff --git a/broker/appengine/src/com/metaweb/gridworks/broker/AppEngineGridworksBrokerImpl.java b/broker/appengine/src/com/metaweb/gridworks/broker/AppEngineGridworksBrokerImpl.java index 7d4880d1a..6998776e3 100644 --- a/broker/appengine/src/com/metaweb/gridworks/broker/AppEngineGridworksBrokerImpl.java +++ b/broker/appengine/src/com/metaweb/gridworks/broker/AppEngineGridworksBrokerImpl.java @@ -87,19 +87,22 @@ public class AppEngineGridworksBrokerImpl extends GridworksBroker { } } - protected void getLock(HttpServletResponse response, String pid) throws Exception { + protected void getState(HttpServletResponse response, String pid, String rev) throws Exception { PersistenceManager pm = pmfInstance.getPersistenceManager(); try { + // FIXME respond(response, lockToJSON(getLock(pm,pid))); } finally { pm.close(); } } - protected void obtainLock(HttpServletResponse response, String pid, String uid) throws Exception { + protected void obtainLock(HttpServletResponse response, String pid, String uid, String type) throws Exception { PersistenceManager pm = pmfInstance.getPersistenceManager(); + // FIXME (use type) + try { Lock lock = getLock(pm, pid); if (lock == null) { @@ -224,7 +227,7 @@ public class AppEngineGridworksBrokerImpl extends GridworksBroker { // --------------------------------------------------------------------------------- - protected void getProject(HttpServletResponse response, String pid) throws Exception { + protected void openProject(HttpServletResponse response, String pid) throws Exception { PersistenceManager pm = pmfInstance.getPersistenceManager(); try { @@ -271,7 +274,7 @@ public class AppEngineGridworksBrokerImpl extends GridworksBroker { pm.close(); } } - + // --------------------------------------------------------------------------------- Project getProject(PersistenceManager pm, String pid) { diff --git a/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java b/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java index 1b9562ac3..a15842622 100644 --- a/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java +++ b/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java @@ -44,17 +44,25 @@ import edu.mit.simile.butterfly.ButterflyModuleImpl; * */ public abstract class GridworksBroker extends ButterflyModuleImpl { - - protected static final Logger logger = LoggerFactory.getLogger("gridworks.broker"); + + static final public String GET_STATE = "get_state"; + static final public String EXPIRE = "expire"; + static final public String OBTAIN_LOCK = "obtain_lock"; + static final public String RELEASE_LOCK = "release_lock"; + static final public String TRANSFORM = "transform"; + static final public String START = "start"; + static final public String OPEN = "open"; + + static final public int ALL = 0; + static final public int COLUMN = 1; + static final public int CELL = 2; + + static final protected Logger logger = LoggerFactory.getLogger("gridworks.broker"); static final protected String USER_INFO_URL = "http://www.freebase.com/api/service/user_info"; static final protected String DELEGATED_OAUTH_HEADER = "X-Freebase-Credentials"; static final protected String OAUTH_HEADER = "Authorization"; - static final protected int ALL = 0; - static final protected int COLUMN = 1; - static final protected int CELL = 2; - static protected String OK; static { @@ -98,31 +106,31 @@ public abstract class GridworksBroker extends ButterflyModuleImpl { try { - if ("get_state".equals(path)) { + if (GET_STATE.equals(path)) { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); getState(response, getParameter(request, "pid"), getUserId(request), getInteger(request, "rev")); - } else if ("expire".equals(path)) { + } else if (EXPIRE.equals(path)) { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); expire(response); - } else if ("obtain_lock".equals(path)) { + } else if (OBTAIN_LOCK.equals(path)) { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); obtainLock(response, getParameter(request, "pid"), getUserId(request), getInteger(request, "locktype"), getParameter(request, "lockvalue")); - } else if ("release_lock".equals(path)) { + } else if (RELEASE_LOCK.equals(path)) { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); releaseLock(response, getParameter(request, "pid"), getUserId(request), getParameter(request, "lock")); - } else if ("transform".equals(path)) { + } else if (TRANSFORM.equals(path)) { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); addTransformations(response, getParameter(request, "pid"), getUserId(request), getParameter(request, "lock"), getList(request, "transformations")); - } else if ("start".equals(path)) { + } else if (START.equals(path)) { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); startProject(response, getParameter(request, "pid"), getUserId(request), getParameter(request, "lock"), getData(request), getParameter(request, "metadata"), getInteger(request, "rev")); - } else if ("open".equals(path)) { + } else if (OPEN.equals(path)) { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); openProject(response, getParameter(request, "pid")); @@ -243,7 +251,7 @@ public abstract class GridworksBroker extends ButterflyModuleImpl { try { JSONObject o = new JSONObject(); - o.put("code", "error"); + o.put("status", "error"); o.put("message", error); response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); respond(response, o.toString()); @@ -260,7 +268,7 @@ public abstract class GridworksBroker extends ButterflyModuleImpl { try { JSONObject o = new JSONObject(); - o.put("code", "error"); + o.put("status", "error"); o.put("message", e.getMessage()); StringWriter sw = new StringWriter(); @@ -283,8 +291,7 @@ public abstract class GridworksBroker extends ButterflyModuleImpl { throw new ServletException("Content object can't be null"); } - JSONObject o = new JSONObject(); - respond(response, o.toString()); + respond(response, content.toString()); } static protected void respond(HttpServletResponse response, String content) throws IOException, ServletException { diff --git a/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java b/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java index 311802429..524917128 100644 --- a/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java +++ b/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java @@ -57,7 +57,7 @@ public class GridworksBrokerImpl extends GridworksBroker { timer = new Timer(); expirer = new Expirer(); - timer.schedule(expirer, LOCK_EXPIRATION_CHECK_DELAY, LOCK_EXPIRATION_CHECK_DELAY); + timer.schedule(expirer, 0, LOCK_EXPIRATION_CHECK_DELAY); String dataDir = config.getInitParameter("gridworks.data"); if (dataDir == null) dataDir = "data"; @@ -159,7 +159,7 @@ public class GridworksBrokerImpl extends GridworksBroker { @Override protected void obtainLock(HttpServletResponse response, String pid, String uid, int locktype, String lockvalue) throws Exception { - + logger.trace("> obtain lock"); Lock lock = null; Transaction txn = env.beginTransaction(null, null); @@ -170,6 +170,8 @@ public class GridworksBrokerImpl extends GridworksBroker { try { for (Lock l : cursor) { + logger.info("found lock: {}", l.id); + if (locktype == ALL) { if (l.type == ALL) { lock = l; @@ -195,6 +197,7 @@ public class GridworksBrokerImpl extends GridworksBroker { } if (lock == null) { + logger.info("no lock found, creating new"); lock = new Lock(Long.toHexString(txn.getId()), pid, uid, locktype, lockvalue); lockById.put(txn, lock); txn.commit(); @@ -207,7 +210,9 @@ public class GridworksBrokerImpl extends GridworksBroker { } } - respond(response, lockToJSON(lock)); + respond(response, lockToJSON(lock, uid)); + + logger.trace("< obtain lock"); } @Override @@ -221,7 +226,7 @@ public class GridworksBrokerImpl extends GridworksBroker { if (!lock.uid.equals(uid)) { throw new RuntimeException("User id doesn't match the lock owner, can't release the lock"); } - lockById.delete(pid); + lockById.delete(lid); txn.commit(); } } finally { @@ -368,16 +373,14 @@ public class GridworksBrokerImpl extends GridworksBroker { writer.value(project.transformations.get(i)); } writer.endArray(); - writer.endObject(); EntityCursor cursor = locksByProject.subIndex(pid).entities(); try { - writer.object(); writer.key("locks"); writer.array(); for (Lock lock : cursor) { - writer.value(lockToJSON(lock)); + writer.value(lockToJSON(lock, uid)); } writer.endArray(); writer.endObject(); @@ -464,12 +467,19 @@ public class GridworksBrokerImpl extends GridworksBroker { return lock; } - JSONObject lockToJSON(Lock lock) throws JSONException { + JSONObject lockToJSON(Lock lock, String uid) throws JSONException { JSONObject o = new JSONObject(); if (lock != null) { - o.put("lock_id", lock.id); - o.put("project_id", lock.pid); - o.put("user_id", lock.uid); + // NOTE: only the owner of the lock should get the ID, + // otherwise others can just fake ownership of other people's locks + if (lock.uid.equals(uid)) { + o.put("lock", lock.id); + } + + o.put("pid", lock.pid); + o.put("uid", lock.uid); + o.put("type", lock.type); + o.put("value", lock.value); o.put("timestamp", lock.timestamp); } return o; diff --git a/broker/core/tests/src/com/metaweb/gridworks/broker/tests/GridworksBrokerTests.java b/broker/core/tests/src/com/metaweb/gridworks/broker/tests/GridworksBrokerTests.java index 37b26c40b..e46b0688a 100644 --- a/broker/core/tests/src/com/metaweb/gridworks/broker/tests/GridworksBrokerTests.java +++ b/broker/core/tests/src/com/metaweb/gridworks/broker/tests/GridworksBrokerTests.java @@ -1,25 +1,34 @@ package com.metaweb.gridworks.broker.tests; +import static com.metaweb.gridworks.broker.GridworksBroker.ALL; +import static com.metaweb.gridworks.broker.GridworksBroker.EXPIRE; +import static com.metaweb.gridworks.broker.GridworksBroker.GET_STATE; +import static com.metaweb.gridworks.broker.GridworksBroker.OBTAIN_LOCK; +import static com.metaweb.gridworks.broker.GridworksBroker.RELEASE_LOCK; +import static com.metaweb.gridworks.broker.GridworksBroker.START; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.io.ByteArrayInputStream; import java.io.File; +import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.Map; -import java.util.Map.Entry; import javax.servlet.ServletConfig; +import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.Assert; import org.testng.annotations.AfterMethod; import org.testng.annotations.AfterSuite; +import org.testng.annotations.AfterTest; import org.testng.annotations.BeforeMethod; import org.testng.annotations.BeforeSuite; import org.testng.annotations.BeforeTest; @@ -30,9 +39,8 @@ import com.metaweb.gridworks.broker.GridworksBrokerImpl; public class GridworksBrokerTests { - protected Logger logger; - - protected File data; + Logger logger; + File data; @BeforeSuite public void suite_init() { @@ -43,80 +51,151 @@ public class GridworksBrokerTests { @AfterSuite public void suite_destroy() { + for (File f : data.listFiles()) { + f.delete(); + } + data.delete(); } + // ------------------------------------------------------------------------------------ + + ServletConfig config = null; + GridworksBroker broker = null; + @BeforeTest - public void test_init() { + public void test_init() throws Exception { logger = LoggerFactory.getLogger(this.getClass()); - } - - // System under test - GridworksBroker SUT = null; + config = mock(ServletConfig.class); + when(config.getInitParameter("gridworks.data")).thenReturn(data.getAbsolutePath()); + when(config.getInitParameter("gridworks.development")).thenReturn("true"); + + broker = new GridworksBrokerImpl(); + broker.init(config); + } + + @AfterTest + public void test_destroy() throws Exception { + broker.destroy(); + broker = null; + config = null; + } + + // ------------------------------------------------------------------------------------ - // mocks HttpServletRequest request = null; HttpServletResponse response = null; - ServletConfig config = null; StringWriter writer = null; @BeforeMethod public void setup() throws Exception { - config = mock(ServletConfig.class); request = mock(HttpServletRequest.class); response = mock(HttpServletResponse.class); - writer = new StringWriter(); - - when(config.getInitParameter("gridworks.data")).thenReturn(data.getAbsolutePath()); - when(response.getWriter()).thenReturn(new PrintWriter(writer)); - - SUT = new GridworksBrokerImpl(); - SUT.init(config); } @AfterMethod public void teardown() throws Exception { - SUT.destroy(); - SUT = null; - - writer = null; response = null; request = null; - config = null; } // ------------------------------------------------------------------------------------ @Test public void testLifeCycle() { - logger.info("testing lifecycle"); Assert.assertTrue(true); } @Test public void testService() { try { - call(SUT, request, response, "expire", null); - logger.info(writer.toString()); + JSONObject result = call(broker, request, response, EXPIRE); + assertJSON(result, "status", "ok"); } catch (Exception e) { Assert.fail(); } } + @Test + public void testObtainLockFailure() { + try { + JSONObject result = call(broker, request, response, OBTAIN_LOCK); + assertJSON(result, "status", "error"); + } catch (Exception e) { + Assert.fail(); + } + } + + @Test + public void testReleaseLockFailure() { + try { + JSONObject result = call(broker, request, response, RELEASE_LOCK); + assertJSON(result, "status", "error"); + } catch (Exception e) { + Assert.fail(); + } + } + + @Test + public void testStartProject() { + try { + JSONObject result = call(broker, request, response, OBTAIN_LOCK, "pid", "1", "uid", "testuser", "locktype", Integer.toString(ALL), "lockvalue", ""); + assertJSON(result, "uid", "testuser"); + String lock = result.getString("lock"); + + result = call(broker, request, response, START, "pid", "1", "uid", "testuser", "lock", lock, "data", "blah", "metadata", "{}", "rev", "0"); + assertJSON(result, "status", "ok"); + + result = call(broker, request, response, GET_STATE, "pid", "1", "uid", "testuser", "rev", "0"); + JSONArray locks = result.getJSONArray("locks"); + Assert.assertEquals(locks.length(), 1); + JSONObject l = locks.getJSONObject(0); + assertJSON(l, "uid", "testuser"); + Assert.assertEquals(l.getInt("type"), ALL); + + result = call(broker, request, response, RELEASE_LOCK, "pid", "1", "uid", "testuser", "lock", lock); + assertJSON(result, "status", "ok"); + + result = call(broker, request, response, GET_STATE, "pid", "1", "uid", "testuser", "rev", "0"); + locks = result.getJSONArray("locks"); + Assert.assertEquals(locks.length(), 0); + } catch (Exception e) { + Assert.fail(); + } + } + // ------------------------------------------------------------------------------------ - private void call(GridworksBroker broker, HttpServletRequest request, HttpServletResponse response, String service, Map params) throws Exception { + private void assertJSON(JSONObject o, String name, String value) throws JSONException { + Assert.assertEquals(o.get(name), value); + } + + private JSONObject call(GridworksBroker broker, HttpServletRequest request, HttpServletResponse response, String service, String... params) throws Exception { if (params != null) { - for (Entry e : params.entrySet()) { - when(request.getParameter(e.getKey())).thenReturn(e.getValue()); + for (int i = 0; i < params.length; ) { + String name = params[i++]; + String value = params[i++]; + if ("data".equals(name)) { + final ByteArrayInputStream inputStream = new ByteArrayInputStream(value.getBytes("UTF-8")); + when(request.getInputStream()).thenReturn(new ServletInputStream() { + public int read() throws IOException { + return inputStream.read(); + } + }); + } else { + when(request.getParameter(name)).thenReturn(value); + } } } - broker.process(service, request, response); + StringWriter writer = new StringWriter(); + when(response.getWriter()).thenReturn(new PrintWriter(writer)); - if (params != null) { - for (Entry e : params.entrySet()) { - verify(request,times(1)).getParameter(e.getKey()); - } - } + broker.process(service, request, response); + + JSONObject result = new JSONObject(writer.toString()); + + logger.info(result.toString()); + + return result; } } diff --git a/broker/core/tests/src/tests.log4j.properties b/broker/core/tests/src/tests.log4j.properties index 63c3978ef..88430d9ad 100644 --- a/broker/core/tests/src/tests.log4j.properties +++ b/broker/core/tests/src/tests.log4j.properties @@ -1,4 +1,4 @@ -log4j.rootLogger=DEBUG, console +log4j.rootLogger=INFO, console log4j.appender.console=org.apache.log4j.ConsoleAppender