Merge pull request #89 from TurkServer/server-treatments

This branch fixes #88 and completes the implementation of `Turkserver.treatment` on both the server and client with access to all relevant instance and assignment parameters.

package.js

@@ -74,6 +74,7 @@ Package.onUse(function (api) {
   api.addFiles([
     'server/config.js',
     'server/turkserver.coffee',
+    'server/server_api.js',
     'server/mturk.js',
     'server/lobby_server.coffee',
     'server/batches.coffee',

server/assignment.js

@@ -86,8 +86,19 @@ class Assignment {
    * @returns {TurkServer.Assignment} The assignment object.
    */
   static currentAssignment() {
-    let userId = Meteor.userId();
-    if (userId == null) return;
+    let userId = null;
+    try {
+      userId = Meteor.userId();
+    }
+    catch (e) {
+      // We aren't in a method, so Meteor throws this error:
+      // "Error: Meteor.userId can only be invoked in method calls. Use this.userId in publish functions."
+      // Note that isn't just publish functions, but any server code not
+      // triggered by a client.
+      return null;
+    }
+    if (userId == null) return null;
+
     return Assignment.getCurrentUserAssignment(userId);
   }
 

server/instance.js

@@ -4,16 +4,6 @@ const init_queue = [];
   XXX Note that the collection called "Experiments" now actually refers to instances
  */
 
-/**
- * @summary Get the treatments of the current instance
- * @locus Anywhere
- * @returns The treatments of the currently scoped instance
- */
-TurkServer.treatment = () => {
-  const inst = TurkServer.Instance.currentInstance();
-  return inst && inst.treatment();
-};
-
 // map of groupId to instance objects
 // XXX Can't use WeakMap here because we have primitive keys
 const _instances = new Map();

server/server_api.js

@@ -0,0 +1,19 @@
+/**
+ * @summary Access treatment data assigned to the current user (assignment)
+ * or the user's current world (instance).
+ * @locus Server
+ * @returns {Object} treatment key/value pairs
+ */
+TurkServer.treatment = function() {
+  const instance = TurkServer.Instance.currentInstance();
+  const asst = TurkServer.Assignment.currentAssignment();
+
+  const instTreatments = instance && instance.getTreatmentNames() || [];
+  const asstTreatments = asst && asst.getTreatmentNames() || [];
+
+  return TurkServer._mergeTreatments(Treatments.find({
+      name: {
+        $in: instTreatments.concat(asstTreatments)
+      }
+    }));
+};

tests/experiment_client_tests.coffee

@@ -58,10 +58,22 @@ if Meteor.isServer
       TurkServer.Instance.currentInstance().teardown(returnToLobby)
       return
 
+    getServerTreatment: ->
+      return TurkServer.treatment()
+
 if Meteor.isClient
   tol = 20 # range in ms that we can be off in adjacent cols
   big_tol = 500 # max range we tolerate in a round trip to the server (async method)
 
+  expectedTreatment = {
+    fooProperty: "bar"  # world
+    foo2: "baz"         # user
+  }
+
+  checkTreatments = (test, obj) ->
+    for k, v of expectedTreatment
+      test.equal obj[k], v, "for key #{k} actual value #{obj[k]} doesn't match expected value #{v}"
+
   Tinytest.addAsync "experiment - client - login and creation of assignment metadata", (test, next) ->
     InsecureLogin.ready ->
       test.isTrue Meteor.userId()
@@ -107,6 +119,8 @@ if Meteor.isClient
       test.isTrue userTreatment.name
       test.equal userTreatment.foo2, "baz"
 
+      checkTreatments(test, TurkServer.treatment())
+
       next()
 
     fail = ->
@@ -119,14 +133,23 @@ if Meteor.isClient
       return true if treatment.treatments.length
     ), verify, fail, 2000
 
+  Tinytest.addAsync "experiment - assignment - test treatments on server", (test, next) ->
+    # Even though this is a "client" test, it is testing a server function
+    # because assignment treatments are different on the client and server
+    Meteor.call "getServerTreatment", (err, res) ->
+      test.fail() if err?
+
+      checkTreatments(test, res)
+      next()
+
   Tinytest.addAsync "experiment - client - current payment variable", (test, next) ->
     amount = 0.42
 
     Meteor.call "setAssignmentPayment", amount, (err, res) ->
       test.equal TurkServer.currentPayment(), amount
       next()
 
-  Tinytest.addAsync "experiment - client - assignment metadata and local time vars", (test, next) ->
+  Tinytest.addAsync "experiment - assignment - assignment metadata and local time vars", (test, next) ->
     asstData = null
 
     verify = ->
@@ -153,7 +176,7 @@ if Meteor.isClient
       return true if asstData?
     ), verify, fail, 2000
 
-  Tinytest.addAsync "experiment - client - no time fields", (test, next) ->
+  Tinytest.addAsync "experiment - assignment - no time fields", (test, next) ->
     fields = [
       {
         id: TurkServer.group()
@@ -181,7 +204,7 @@ if Meteor.isClient
 
       next()
 
-  Tinytest.addAsync "experiment - client - joined time computation", (test, next) ->
+  Tinytest.addAsync "experiment - assignment - joined time computation", (test, next) ->
     fields = [
       {
         id: TurkServer.group()
@@ -211,7 +234,7 @@ if Meteor.isClient
 
       next()
 
-  Tinytest.addAsync "experiment - client - instance ended state", (test, next) ->
+  Tinytest.addAsync "experiment - instance - instance ended state", (test, next) ->
     # In experiment. not ended
     test.isTrue TurkServer.inExperiment()
     test.isFalse TurkServer.instanceEnded()
@@ -226,7 +249,8 @@ if Meteor.isClient
     Next test edits instance fields, so client APIs may break state
   ###
 
-  Tinytest.addAsync "experiment - client - selects correct instance of multiple", (test, next) ->
+  Tinytest.addAsync "experiment - instance - client selects correct instance of
+ multiple", (test, next) ->
     fields = [
       {
         id: Random.id()

tests/experiment_tests.coffee

@@ -79,6 +79,10 @@ Tinytest.add "experiment - batch - creation and retrieval", withCleanup (test) -
 
   test.equal batch2, batch
 
+Tinytest.add "experiment - assignment - currentAssignment in standalone
+ server code returns null", (test) ->
+  test.equal TurkServer.Assignment.currentAssignment(), null
+
 Tinytest.add "experiment - instance - throws error if doesn't exist", withCleanup (test) ->
   test.throws ->
     TurkServer.Instance.getInstance("yabbadabbadoober")
@@ -149,12 +153,18 @@ Tinytest.add "experiment - instance - teardown and log", withCleanup (test) ->
 Tinytest.add "experiment - instance - get treatment on server", withCleanup (test) ->
   instance = batch.createInstance(["fooTreatment"])
 
+  # Note this only tests world treatments. Assignment treatments have to be
+  # tested with the janky client setup.
+
+  # However, This also tests accessing server treatments outside of a client context.
   instance.bindOperation ->
     treatment = TurkServer.treatment()
     test.equal treatment.treatments[0], "fooTreatment"
+    test.equal treatment.fooProperty, "bar"
 
   # Undefined outside of an experiment instance
-  test.equal TurkServer.treatment(), undefined
+  treatment = TurkServer.treatment()
+  test.equal treatment.fooProperty, undefined
 
 Tinytest.add "experiment - instance - global group", withCleanup (test) ->
   instance = batch.createInstance([])
@@ -181,7 +191,7 @@ Tinytest.add "experiment - instance - global group", withCleanup (test) ->
   test.equal stuff[2].foo2, "bar"
   test.equal stuff[2]._groupId, instance.groupId
 
-Tinytest.add "experiment - instance - reject adding user to ended instance", withCleanup (test) ->
+Tinytest.add "experiment - assignment - reject adding user to ended instance", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -201,7 +211,7 @@ Tinytest.add "experiment - instance - reject adding user to ended instance", wit
 
   test.isFalse asstData.instances
 
-Tinytest.add "experiment - instance - addAssignment records start time and instance id", withCleanup (test) ->
+Tinytest.add "experiment - assignment - addAssignment records start time and instance id", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -225,7 +235,7 @@ Tinytest.add "experiment - instance - addAssignment records start time and insta
   test.equal asstData.instances[0].id, instance.groupId
   test.isTrue asstData.instances[0].joinTime
 
-Tinytest.add "experiment - instance - second addAssignment does not change date", withCleanup (test) ->
+Tinytest.add "experiment - assignment - second addAssignment does not change date", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -247,7 +257,7 @@ Tinytest.add "experiment - instance - second addAssignment does not change date"
   # Should be the same date as originally
   test.equal instanceData.startTime, startedDate
 
-Tinytest.add "experiment - instance - teardown with returned assignment", withCleanup (test) ->
+Tinytest.add "experiment - assignment - teardown with returned assignment", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -267,7 +277,7 @@ Tinytest.add "experiment - instance - teardown with returned assignment", withCl
   test.isTrue asstData.instances[0]
   test.equal asstData.status, "returned"
 
-Tinytest.add "experiment - instance - user disconnect and reconnect", withCleanup (test) ->
+Tinytest.add "experiment - assignment - user disconnect and reconnect", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -306,7 +316,7 @@ Tinytest.add "experiment - instance - user disconnect and reconnect", withCleanu
   test.isTrue asstData.instances[0].disconnectedTime > 0
   test.isTrue asstData.instances[0].disconnectedTime < Date.now() - discTime
 
-Tinytest.add "experiment - instance - user idle and re-activate", withCleanup (test) ->
+Tinytest.add "experiment - assignment - user idle and re-activate", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -362,7 +372,7 @@ Tinytest.add "experiment - instance - user idle and re-activate", withCleanup (t
   test.isFalse asstData.instances[0].lastIdle
   test.equal asstData.instances[0].idleTime, offset + offset
 
-Tinytest.add "experiment - instance - user disconnect while idle", withCleanup (test) ->
+Tinytest.add "experiment - assignment - user disconnect while idle", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -387,7 +397,7 @@ Tinytest.add "experiment - instance - user disconnect while idle", withCleanup (
   # Check that disconnect fields exist
   test.isTrue asstData.instances[0].lastDisconnect
 
-Tinytest.add "experiment - instance - idleness is cleared on reconnection", withCleanup (test) ->
+Tinytest.add "experiment - assignment - idleness is cleared on reconnection", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -419,7 +429,7 @@ Tinytest.add "experiment - instance - idleness is cleared on reconnection", with
   test.isFalse asstData.instances[0].lastDisconnect
   test.isTrue asstData.instances[0].disconnectedTime
 
-Tinytest.add "experiment - instance - teardown while disconnected", withCleanup (test) ->
+Tinytest.add "experiment - assignment - teardown while disconnected", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -446,7 +456,7 @@ Tinytest.add "experiment - instance - teardown while disconnected", withCleanup
   test.isTrue asstData.instances[0].disconnectedTime > 0
   test.isTrue asstData.instances[0].disconnectedTime < Date.now() - discTime
 
-Tinytest.add "experiment - instance - teardown while idle", withCleanup (test) ->
+Tinytest.add "experiment - assignment - teardown while idle", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -470,7 +480,7 @@ Tinytest.add "experiment - instance - teardown while idle", withCleanup (test) -
   test.isFalse asstData.instances[0].lastIdle
   test.isTrue asstData.instances[0].idleTime
 
-Tinytest.add "experiment - instance - leave instance after teardown", withCleanup (test) ->
+Tinytest.add "experiment - assignment - leave instance after teardown", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()
 
@@ -501,7 +511,7 @@ Tinytest.add "experiment - instance - leave instance after teardown", withCleanu
   test.isTrue asstData.instances[0].disconnectedTime > 0
   test.isTrue asstData.instances[0].disconnectedTime < 200
 
-Tinytest.add "experiment - instance - teardown and join second instance", withCleanup (test) ->
+Tinytest.add "experiment - assignment - teardown and join second instance", withCleanup (test) ->
   instance = batch.createInstance([])
   instance.setup()