From ce84258bfd7e317d39414162de62877f1bcdd250 Mon Sep 17 00:00:00 2001
From: Anubhav Jain <anubhav@meteor.com>
Date: Tue, 21 Jul 2015 16:50:48 -0700
Subject: [PATCH] Fixed getLastRateLimitEvent mixup and updated DDPRateLimiter
 tests to reflect that.

---
 .../ddp-rate-limiter-test-service.js          | 48 +++++--------------
 .../ddp-rate-limiter-tests.js                 |  4 +-
 packages/ddp-rate-limiter/package.js          |  3 +-
 3 files changed, 15 insertions(+), 40 deletions(-)

diff --git a/packages/ddp-rate-limiter/ddp-rate-limiter-test-service.js b/packages/ddp-rate-limiter/ddp-rate-limiter-test-service.js
index e71605eeb8..55a18d9daa 100644
--- a/packages/ddp-rate-limiter/ddp-rate-limiter-test-service.js
+++ b/packages/ddp-rate-limiter/ddp-rate-limiter-test-service.js
@@ -6,7 +6,18 @@ Meteor.methods({
     var connection = this.connection;
     connection.lastRateLimitEvent = connection.lastRateLimitEvent || {};
     connection.lastMethodName = connection.lastMethodName || '';
+    // XXX In Javascript v8 engine, we are currently guaranteed the ordering of
+    // the keys in objects as they are listed. This may change in future
+    // iterations of v8 for performance reasons and will potentially break this
+    // test.
     this.ruleId = DDPRateLimiter.addRule({
+      name: function (name) {
+        connection.lastMethodName = name;
+        if (name !== 'getLastRateLimitEvent') {
+          connection.lastRateLimitEvent.name = name;
+        }
+        return name !== "a-method-that-is-not-rate-limited";
+      },
       userId: function (userId) {
         connection.lastRateLimitEvent.userId = userId;
         return true;
@@ -19,13 +30,6 @@ Meteor.methods({
         }
         return true;
       },
-      name: function (name) {
-        if (name !== 'getLastRateLimitEvent') {
-          connection.lastRateLimitEvent.name = name;
-        }
-        connection.lastMethodName = name;
-        return name !== "a-method-that-is-not-rate-limited";
-      },
       clientAddress: function (clientAddress) {
         connection.lastRateLimitEvent.clientAddress = clientAddress
         return true;
@@ -54,36 +58,6 @@ Meteor.methods({
   },
   'a-method-that-is-not-rate-limited': function () {
     return "not-rate-limited";
-  },
-  addSubscriptionRuleToDDPRateLimiter: function () {
-    var connection = this.connection;
-    connection.lastRateLimitEvent = connection.lastRateLimitEvent || {};
-    connection.lastMethodName = connection.lastMethodName || '';
-    this.ruleId = DDPRateLimiter.addRule({
-      userId: function (userId) {
-        connection.lastRateLimitEvent.userId = userId;
-        return true;
-      },
-      name: function (name) {
-        connection.lastMethodName = name;
-        // Special check to return proper name since 'getLastRateLimitEvent'
-        // is another method call
-        if (name !== 'getLastRateLimitEvent')
-          connection.lastRateLimitEvent.name = name;
-        return true;
-      },
-      type: function (type) {
-        if (connection.lastMethodName !== 'getLastRateLimitEvent')
-          connection.lastRateLimitEvent.type = type;
-        return type === 'subscription';
-      },
-      clientAddress: function (clientAddress) {
-        connection.lastRateLimitEvent.clientAddress = clientAddress;
-        return true;
-      },
-      connectionId: this.connection.id
-    }, RATE_LIMIT_NUM_CALLS, RATE_LIMIT_INTERVAL_TIME_MS);
-    return this.ruleId;
   }
 });
 
diff --git a/packages/ddp-rate-limiter/ddp-rate-limiter-tests.js b/packages/ddp-rate-limiter/ddp-rate-limiter-tests.js
index 6ad1b9845e..236960d93e 100644
--- a/packages/ddp-rate-limiter/ddp-rate-limiter-tests.js
+++ b/packages/ddp-rate-limiter/ddp-rate-limiter-tests.js
@@ -74,7 +74,7 @@ testAsyncMulti("ddp rate limiter - matchers get passed correct arguments", [
 testAsyncMulti("ddp rate limiter - we can return with type 'subscription'", [
   function (test, expect) {
     var self = this;
-    Meteor.call("addSubscriptionRuleToDDPRateLimiter", expect(
+    Meteor.call("addRuleToDDPRateLimiter", expect(
       function(error, result) {
         self.ruleId = result;
       }));
@@ -101,7 +101,7 @@ testAsyncMulti("ddp rate limiter - we can return with type 'subscription'", [
 testAsyncMulti("ddp rate limiter - rate limits to subscriptions", [
   function (test, expect) {
     var self = this;
-    Meteor.call("addSubscriptionRuleToDDPRateLimiter", expect(
+    Meteor.call("addRuleToDDPRateLimiter", expect(
       function(error, result) {
         self.ruleId = result;
       })
diff --git a/packages/ddp-rate-limiter/package.js b/packages/ddp-rate-limiter/package.js
index 47337704ef..d67ce2ed30 100644
--- a/packages/ddp-rate-limiter/package.js
+++ b/packages/ddp-rate-limiter/package.js
@@ -20,10 +20,11 @@ Package.onUse(function(api) {
 
 Package.onTest(function(api) {
   api.use('underscore');
-  api.use('ddp-rate-limiter');
   api.use(['accounts-password', 'tinytest', 'test-helpers', 'tracker',
            'accounts-base', 'random', 'email', 'underscore', 'check',
            'ddp', 'ecmascript', 'es5-shim']);
+  api.use('ddp-rate-limiter');
+
   api.addFiles('ddp-rate-limiter-tests-common.js');
   api.addFiles('ddp-rate-limiter-test-service.js', 'server');
   api.addFiles('ddp-rate-limiter-tests.js', 'client');