diff --git a/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java b/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java index 45556ef00..4bc6a12f6 100644 --- a/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java +++ b/broker/core/src/com/metaweb/gridworks/broker/GridworksBroker.java @@ -54,7 +54,7 @@ public abstract class GridworksBroker extends ButterflyModuleImpl { static final public String OPEN = "open"; static final public int ALL = 0; - static final public int COLUMN = 1; + static final public int COL = 1; static final public int CELL = 2; static final protected Logger logger = LoggerFactory.getLogger("gridworks.broker"); diff --git a/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java b/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java index a0a9d24f1..312496e68 100644 --- a/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java +++ b/broker/core/src/com/metaweb/gridworks/broker/GridworksBrokerImpl.java @@ -161,6 +161,7 @@ public class GridworksBrokerImpl extends GridworksBroker { protected void obtainLock(HttpServletResponse response, String pid, String uid, int locktype, String lockvalue) throws Exception { logger.trace("> obtain lock"); Lock lock = null; + Lock blocker = null; Transaction txn = env.beginTransaction(null, null); @@ -168,27 +169,68 @@ public class GridworksBrokerImpl extends GridworksBroker { EntityCursor cursor = locksByProject.subIndex(pid).entities(); + /* + * ALL + * blocked -> somebody else's lock + * reuse -> you already have an ALL lock + * new -> else + * + * COL + * blocked -> somebody else's all lock || a lock on the same col + * reuse -> you have an ALL lock || a lock on the same col + * new -> else + * + * CELL + * blocked -> somebody else's all lock || a lock on the same col || a lock on the same cell + * reuse -> you have a lock on the same cell + * yes -> (you have a lock on the same cell) && (nobody else has a lock on the same cell || the same col || all) + * new -> else + * + */ + try { - for (Lock l : cursor) { - logger.info("found lock: {}", l.id); - - if (locktype == ALL) { - if (l.type == ALL) { - lock = l; + if (locktype == ALL) { + for (Lock l : cursor) { + if (!l.uid.equals(uid)) { + blocker = l; break; + } else { + if (l.type == ALL) { + lock = l; + break; + } } - } else if (locktype == COLUMN) { - if (l.type == ALL || - (l.type == COLUMN && l.value.equals(lockvalue))) { - lock = l; - break; + } + } else if (locktype == COL) { + for (Lock l : cursor) { + if (!l.uid.equals(uid)) { + if (l.type == ALL || + (l.type == COL && l.value.equals(lockvalue))) { + blocker = l; + break; + } + } else { + if (l.type == ALL || + (l.type == COL && l.value.equals(lockvalue))) { + lock = l; + break; + } } - } else if (locktype == CELL) { - if (l.type == ALL || - (l.type == COLUMN && l.value.equals(lockvalue.split(",")[0])) || - (l.type == CELL && l.value.equals(lockvalue))) { - lock = l; - break; + } + } else if (locktype == CELL) { + for (Lock l : cursor) { + if (!l.uid.equals(uid)) { + if (l.type == ALL || + (l.type == COL && l.value.equals(lockvalue.split(",")[0])) || + (l.type == CELL && l.value.equals(lockvalue))) { + blocker = l; + break; + } + } else { + if (l.type == CELL && l.value.equals(lockvalue)) { + lock = l; + break; + } } } } @@ -196,8 +238,13 @@ public class GridworksBrokerImpl extends GridworksBroker { cursor.close(); } + if (blocker != null) { + logger.info("found a blocking lock {}", lockToString(blocker)); + throw new RuntimeException("Can't obtain lock, it is blocked by a type '" + blocker.type + "' lock owned by '" + blocker.uid + "'"); + } + if (lock == null) { - logger.info("no lock found, creating new"); + logger.info("no comparable lock already exists, creating a new one"); lock = new Lock(Long.toHexString(txn.getId()), pid, uid, locktype, lockvalue); lockById.put(txn, lock); txn.commit(); @@ -287,6 +334,8 @@ public class GridworksBrokerImpl extends GridworksBroker { Lock lock = getLock(lid, pid, uid); + logger.info("obtained lock: {}", lockToString(lock)); + if (lock.type == ALL) { project.transformations.addAll(transformations); } else { @@ -295,8 +344,8 @@ public class GridworksBrokerImpl extends GridworksBroker { int type = o.getInt("op_type"); String value = o.getString("op_value"); - if (lock.type == COLUMN) { - if (type == COLUMN) { + if (lock.type == COL) { + if (type == COL) { if (value != null && value.equals(lock.value)) { project.transformations.add(s); } else { @@ -311,7 +360,7 @@ public class GridworksBrokerImpl extends GridworksBroker { } } } else if (lock.type == CELL) { - if (type == COLUMN) { + if (type == COL) { throw new RuntimeException("Can't apply '" + s + "': you offered a lock for a single cell and you're attempting an operation for the entire column."); } else if (type == CELL) { if (value != null && value.equals(lock.value)) { @@ -324,6 +373,8 @@ public class GridworksBrokerImpl extends GridworksBroker { } } + projectById.put(txn, project); + txn.commit(); } finally { if (txn != null) { @@ -374,7 +425,7 @@ public class GridworksBrokerImpl extends GridworksBroker { writer.array(); int size = project.transformations.size(); for (int i = rev; i < size; i++) { - writer.value(project.transformations.get(i)); + writer.value(new JSONObject(project.transformations.get(i))); } writer.endArray(); @@ -489,6 +540,10 @@ public class GridworksBrokerImpl extends GridworksBroker { return o; } + String lockToString(Lock lock) { + return lock.id + "," + lock.pid + "," + lock.uid + "," + lock.type + "," + lock.value; + } + @Entity static class Lock { 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 ed0712439..11c4f4334 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,6 +1,6 @@ package com.metaweb.gridworks.broker.tests; -import static com.metaweb.gridworks.broker.GridworksBroker.ALL; +import static com.metaweb.gridworks.broker.GridworksBroker.*; 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; @@ -109,8 +109,7 @@ public class GridworksBrokerTests { @Test public void testService() { try { - JSONObject result = call(broker, request, response, EXPIRE); - assertJSON(result, "status", "ok"); + success(broker, request, response, EXPIRE); } catch (Exception e) { Assert.fail(); } @@ -119,8 +118,7 @@ public class GridworksBrokerTests { @Test public void testObtainLockFailure() { try { - JSONObject result = call(broker, request, response, OBTAIN_LOCK); - assertJSON(result, "status", "error"); + failure(broker, request, response, OBTAIN_LOCK); } catch (Exception e) { Assert.fail(); } @@ -129,8 +127,7 @@ public class GridworksBrokerTests { @Test public void testReleaseLockFailure() { try { - JSONObject result = call(broker, request, response, RELEASE_LOCK); - assertJSON(result, "status", "error"); + failure(broker, request, response, RELEASE_LOCK); } catch (Exception e) { Assert.fail(); } @@ -139,37 +136,120 @@ public class GridworksBrokerTests { @Test public void testStartProject() { try { - String project = "1"; + String project = "proj1"; String user = "testuser"; + String user2 = "testuser2"; String data = "blah"; String metadata = "{}"; String rev = "0"; - JSONObject result = call(broker, request, response, OBTAIN_LOCK, "pid", project, "uid", user, "locktype", Integer.toString(ALL), "lockvalue", ""); + logger.info("--- obtain ALL lock on project ---"); + JSONObject result = success(broker, request, response, OBTAIN_LOCK, "pid", project, "uid", user, "locktype", Integer.toString(ALL), "lockvalue", ""); assertJSON(result, "uid", "testuser"); String lock = result.getString("lock"); - result = call(broker, request, response, START, "pid", project, "uid", user, "lock", lock, "data", data, "metadata", metadata, "rev", rev); - assertJSON(result, "status", "ok"); + logger.info("--- start project ---"); + success(broker, request, response, START, "pid", project, "uid", user, "lock", lock, "data", data, "metadata", metadata, "rev", rev); - result = call(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", rev); + logger.info("--- verify project state contains lock ---"); + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", rev); 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", project, "uid", user, "lock", lock); - assertJSON(result, "status", "ok"); + logger.info("--- release ALL lock on project ---"); + success(broker, request, response, RELEASE_LOCK, "pid", project, "uid", user, "lock", lock); - result = call(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", rev); + logger.info("--- verify no locks are present ---"); + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", rev); locks = result.getJSONArray("locks"); Assert.assertEquals(locks.length(), 0); - result = call(broker, request, response, OPEN, "pid", project, "uid", user, "rev", rev); - assertJSON(result, "status", "ok"); + logger.info("--- open project and verify data was loaded correctly ---"); + result = success(broker, request, response, OPEN, "pid", project, "uid", user, "rev", rev); JSONArray result_data = result.getJSONArray("data"); Assert.assertEquals(result_data.length(),data.getBytes("UTF-8").length); + + JSONArray tt; + JSONObject t; + + logger.info("--- obtain column lock ---"); + String column = "1"; + result = success(broker, request, response, OBTAIN_LOCK, "pid", project, "uid", user, "locktype", Integer.toString(COL), "lockvalue", column); + String col_lock = result.getString("lock"); + + logger.info("--- perform column transformation ---"); + t = new JSONObject(); + t.put("op_type", COL); + t.put("op_value", column); // operate on col 1 + t.put("value", new JSONObject()); + tt = new JSONArray(); + tt.put(t); + result = success(broker, request, response, TRANSFORM, "pid", project, "uid", user, "lock", col_lock, "transformations", tt.toString()); + + logger.info("--- make sure transformation was recorded properly ---"); + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", "0"); + tt = result.getJSONArray("transformations"); + Assert.assertEquals(tt.length(), 1); + t = tt.getJSONObject(0); + assertJSON(t, "op_value", column); + + logger.info("--- make sure revision numbers in state management work as expected ---"); + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", "1"); + tt = result.getJSONArray("transformations"); + Assert.assertEquals(tt.length(), 0); + + logger.info("--- perform cell transformation ---"); + String cell = "1"; + t = new JSONObject(); + t.put("op_type", CELL); + t.put("op_value", column + "," + cell); // operate on cell at row 1 column 1 + t.put("value", new JSONObject()); + tt = new JSONArray(); + tt.put(t); + result = success(broker, request, response, TRANSFORM, "pid", project, "uid", user, "lock", col_lock, "transformations", tt.toString()); + + logger.info("--- make sure transformation was recorded properly ---"); + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", "0"); + tt = result.getJSONArray("transformations"); + Assert.assertEquals(tt.length(), 2); + + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", "1"); + tt = result.getJSONArray("transformations"); + Assert.assertEquals(tt.length(), 1); + t = tt.getJSONObject(0); + assertJSON(t, "op_value", column + "," + cell); + + logger.info("--- make sure another user fails to acquire ALL lock ---"); + failure(broker, request, response, OBTAIN_LOCK, "pid", project, "uid", user2, "locktype", Integer.toString(ALL), "lockvalue", column); + + logger.info("--- make sure another user fails to acquire COL lock on the same column ---"); + failure(broker, request, response, OBTAIN_LOCK, "pid", project, "uid", user2, "locktype", Integer.toString(COL), "lockvalue", column); + + logger.info("--- make sure another user manages to acquire COL lock on another column ---"); + String column2 = "2"; + result = success(broker, request, response, OBTAIN_LOCK, "pid", project, "uid", user2, "locktype", Integer.toString(COL), "lockvalue", column2); + String col_lock2 = result.getString("lock"); + + logger.info("--- make sure that both locks are present ---"); + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", "2"); + locks = result.getJSONArray("locks"); + Assert.assertEquals(locks.length(), 2); + + logger.info("--- make sure we can't escalate our current COL lock to an ALL lock ---"); + failure(broker, request, response, OBTAIN_LOCK, "pid", project, "uid", user, "locktype", Integer.toString(ALL), "lockvalue", ""); + + logger.info("--- release column locks ---"); + success(broker, request, response, RELEASE_LOCK, "pid", project, "uid", user, "lock", col_lock); + success(broker, request, response, RELEASE_LOCK, "pid", project, "uid", user2, "lock", col_lock2); + + logger.info("--- make sure the project has no locks ---"); + result = success(broker, request, response, GET_STATE, "pid", project, "uid", user, "rev", "2"); + locks = result.getJSONArray("locks"); + Assert.assertEquals(locks.length(), 0); + } catch (Exception e) { Assert.fail(); } @@ -181,7 +261,15 @@ public class GridworksBrokerTests { Assert.assertEquals(o.get(name), value); } - private JSONObject call(GridworksBroker broker, HttpServletRequest request, HttpServletResponse response, String service, String... params) throws Exception { + private JSONObject success(GridworksBroker broker, HttpServletRequest request, HttpServletResponse response, String service, String... params) throws Exception { + return call(true, broker, request, response, service, params); + } + + private JSONObject failure(GridworksBroker broker, HttpServletRequest request, HttpServletResponse response, String service, String... params) throws Exception { + return call(false, broker, request, response, service, params); + } + + private JSONObject call(boolean successful, GridworksBroker broker, HttpServletRequest request, HttpServletResponse response, String service, String... params) throws Exception { if (params != null) { for (int i = 0; i < params.length; ) { String name = params[i++]; @@ -206,8 +294,14 @@ public class GridworksBrokerTests { JSONObject result = new JSONObject(writer.toString()); - logger.info(result.toString()); + if (successful) { + assertJSON(result, "status", "ok"); + } else { + assertJSON(result, "status", "error"); + } + logger.info(result.toString()); + return result; } }