jimsynz / faye-rails

Simple Rails glue for the Faye messaging protocol.
MIT License
435 stars 79 forks source link

Fix Channel#unsubscribe #59

Closed flash-gordon closed 9 years ago

flash-gordon commented 10 years ago

Hi there! Here is a simple fox for unsubscribbing from channel. I spent some time for figuring out why this code does not work:

module Herald
  class ChartController < FayeRails::Controller
    channel '/chart' do
      subscribe do
        # ...
      end
    end

    def self.before_remove_const
      @channels.each(&:unsubscribe)
    end
  end
end

Rails called before_remove_const method on every class reloading but it had no effect. On every class loading subscriptions count just increased. As it turned out Faye::Protocol::Channel#unsubscribe expects string but receives Faye::Subscription object =( And no exception =( I fix a couple of lines and now all work fine. But. I cannot write a test for it because there is one already. And it does not work. Here is the code:

it "should not receive messages after unsubscribe" do
  Fiber.new do
    this_fiber = Fiber.current
    controller.channel('/widgets/99') do
      subscribe do
        this_fiber.resume message
      end
      unsubscribe
    end
    EM.schedule do
      FayeRails.client.publish('/widgets/99', "Welcome to Spacely Sprockets.")
    end
    EM.add_timer 1 do
      this_fiber.resume 'timeout'
    end
    Fiber.yield.should == 'timeout'
  end.resume
end

First of all EM is supposed to run in a separate thread. And this is completely not compatible with Fiber#resume. You cannot call Fiber#resume from other thread. Second, spec helper:

Thread.new do
  run Dummy::Application
end

If you'll try to join this thread then you will see

faye-rails/spec/spec_helper.rb:16:in `block in <top (required)>': undefined method `run' for main:Object (NoMethodError)

So this is does not work either. Third.

it "should not receive messages after unsubscribe" do
  Fiber.new do
    # ...
    Fiber.yield.should == '...'
  end.resume
end

Fiber execution interrupts on Fiber.yield call. And #should method is never called. So controller_spec.rb must be massively rewritten because it does not test anything. I can do something about it if it will be welcome.

flash-gordon commented 10 years ago

Travis points that specs must be updated for Rspec 3

jimsynz commented 9 years ago

Hi there.

Feel free to have a hack on the specs if you want. It'd certainly make my live a lot easier :) :+1: