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/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 {

broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java

@@ -45,16 +45,24 @@ 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, content.toString());
-        respond(response, o.toString());
     }
 
     static protected void respond(HttpServletResponse response, String content) throws IOException, ServletException {

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<Lock> 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);
+            // NOTE: only the owner of the lock should get the ID, 
-            o.put("project_id", lock.pid);
+            // otherwise others can just fake ownership of other people's locks
-            o.put("user_id", lock.uid);
+            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;

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;
+    Logger logger;
-
+    File data;
-    protected File data;
     
     @BeforeSuite
     public void suite_init() {
@@ -43,60 +51,113 @@ 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());
+        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);
     }
 
-    // System under test
+    @AfterTest
-    GridworksBroker SUT = null;
+    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);
+            JSONObject result = call(broker, request, response, EXPIRE);
-            logger.info(writer.toString());
+            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();
         }
@@ -104,19 +165,37 @@ public class GridworksBrokerTests {
     
     // ------------------------------------------------------------------------------------
     
-    private void call(GridworksBroker broker, HttpServletRequest request, HttpServletResponse response, String service, Map<String,String> 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<String,String> e : params.entrySet()) {
+            for (int i = 0; i < params.length; ) {
-                when(request.getParameter(e.getKey())).thenReturn(e.getValue());
+                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);
+                }
             }
         }
 
+        StringWriter writer = new StringWriter();
+        when(response.getWriter()).thenReturn(new PrintWriter(writer));
+        
         broker.process(service, request, response);
 
-        if (params != null) {
+        JSONObject result = new JSONObject(writer.toString());
-            for (Entry<String,String> e : params.entrySet()) {
+        
-                verify(request,times(1)).getParameter(e.getKey());
+        logger.info(result.toString());
-            }
+        
-        }
+        return result;
     }
 }

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