Skip to content
This repository
Browse code

Don't publish subscribe events for subscriptions that already exist.

  • Loading branch information...
commit a3fd080d1dcbcca75a7fbc0b9a6938cdae18373d 1 parent 91e6ce1
James Coglan authored
4  javascript/engines/memory.js
@@ -50,13 +50,13 @@ Faye.Engine.Memory = Faye.Class(Faye.Engine.Base, {
50 50
     var clients = this._clients, channels = this._channels;
51 51
     
52 52
     clients[clientId] = clients[clientId] || new Faye.Set();
53  
-    clients[clientId].add(channel);
  53
+    var trigger = clients[clientId].add(channel);
54 54
     
55 55
     channels[channel] = channels[channel] || new Faye.Set();
56 56
     channels[channel].add(clientId);
57 57
     
58 58
     this.debug('Subscribed client ? to channel ?', clientId, channel);
59  
-    this.trigger('subscribe', clientId, channel);
  59
+    if (trigger) this.trigger('subscribe', clientId, channel);
60 60
     if (callback) callback.call(scope, true);
61 61
   },
62 62
   
5  javascript/engines/redis.js
@@ -100,10 +100,11 @@ Faye.Engine.Redis = Faye.Class(Faye.Engine.Base, {
100 100
   
101 101
   subscribe: function(clientId, channel, callback, scope) {
102 102
     var self = this;
103  
-    this._redis.sadd(this._ns + '/clients/' + clientId + '/channels', channel);
  103
+    this._redis.sadd(this._ns + '/clients/' + clientId + '/channels', channel, function(error, added) {
  104
+      if (added === 1) self.trigger('subscribe', clientId, channel);
  105
+    });
104 106
     this._redis.sadd(this._ns + '/channels' + channel, clientId, function() {
105 107
       self.debug('Subscribed client ? to channel ?', clientId, channel);
106  
-      self.trigger('subscribe', clientId, channel);
107 108
       if (callback) callback.call(scope);
108 109
     });
109 110
   },
6  lib/faye/engines/memory.rb
@@ -48,14 +48,14 @@ def ping(client_id)
48 48
       
49 49
       def subscribe(client_id, channel, &callback)
50 50
         @clients[client_id] ||= Set.new
51  
-        @clients[client_id].add(channel)
  51
+        should_trigger = @clients[client_id].add?(channel)
52 52
         
53 53
         @channels[channel] ||= Set.new
54 54
         @channels[channel].add(client_id)
55 55
         
56 56
         debug 'Subscribed client ? to channel ?', client_id, channel
  57
+        trigger(:subscribe, client_id, channel) if should_trigger
57 58
         callback.call(true) if callback
58  
-        trigger(:subscribe, client_id, channel)
59 59
       end
60 60
       
61 61
       def unsubscribe(client_id, channel, &callback)
@@ -70,8 +70,8 @@ def unsubscribe(client_id, channel, &callback)
70 70
         end
71 71
         
72 72
         debug 'Unsubscribed client ? from channel ?', client_id, channel
73  
-        callback.call(true) if callback
74 73
         trigger(:unsubscribe, client_id, channel) if should_trigger
  74
+        callback.call(true) if callback
75 75
       end
76 76
       
77 77
       def publish(message)
5  lib/faye/engines/redis.rb
@@ -101,11 +101,12 @@ def ping(client_id)
101 101
       
102 102
       def subscribe(client_id, channel, &callback)
103 103
         init
104  
-        @redis.sadd(@ns + "/clients/#{client_id}/channels", channel)
  104
+        @redis.sadd(@ns + "/clients/#{client_id}/channels", channel) do |added|
  105
+          trigger(:subscribe, client_id, channel) if added == 1
  106
+        end
105 107
         @redis.sadd(@ns + "/channels#{channel}", client_id) do
106 108
           debug 'Subscribed client ? to channel ?', client_id, channel
107 109
           callback.call if callback
108  
-          trigger(:subscribe, client_id, channel)
109 110
         end
110 111
       end
111 112
       
9  spec/javascript/engine_spec.js
@@ -217,6 +217,15 @@ JS.ENV.EngineSpec = JS.Test.describe("Pub/sub engines", function() { with(this)
217 217
         expect_event("alice", "subscribe", ["/messages/foo"])
218 218
         subscribe("alice", "/messages/foo")
219 219
       }})
  220
+      
  221
+      describe("when the client is subscribed to the channel", function() { with(this) {
  222
+        before(function() { this.subscribe("alice", "/messages/foo") })
  223
+        
  224
+        it("does not publish an event", function() { with(this) {
  225
+          expect_no_event("alice", "subscribe", ["/messages/foo"])
  226
+          subscribe("alice", "/messages/foo")
  227
+        }})
  228
+      }})
220 229
     }})
221 230
     
222 231
     
9  spec/ruby/engine_spec.rb
@@ -210,6 +210,15 @@ def create_engine
210 210
         expect_event :alice, :subscribe, ["/messages/foo"]
211 211
         subscribe :alice, "/messages/foo"
212 212
       end
  213
+      
  214
+      describe "when the client is subscribed to the channel" do
  215
+        before { subscribe :alice, "/messages/foo" }
  216
+        
  217
+        it "does not publish an event" do
  218
+          expect_no_event :alice, :subscribe, ["/messages/foo"]
  219
+          subscribe :alice, "/messages/foo"
  220
+        end
  221
+      end
213 222
     end
214 223
 
215 224
     describe :unsubscribe do

0 notes on commit a3fd080

Please sign in to comment.
Something went wrong with that request. Please try again.