wisper icon indicating copy to clipboard operation
wisper copied to clipboard

Frozen objects can't publish

Open replaid opened this issue 8 years ago • 2 comments

I work on a project that uses the values gem for immutable (hence frozen) value objects in several places. We add behavior to the objects that doesn't mutate their data. Sometimes we generate events of interest that we'd like to publish with Wisper. But due to the lazy-loading of @local_registrations we can't unless we override initialize to set @local_registrations = Set.new and then call super (which is where #freeze gets called).

Just this one niggling line of code keeping Wisper from working smoothly for a frozen object.

I'd be happy to put together a pull request—can you tell me what you would or would not want to see in a fix for this issue?

replaid avatar Feb 10 '17 01:02 replaid

At the moment I can't see a problem with moving @local_registrations, something like:

def initialize(*args)
  @local_registrations = Set.new
  super
end

def local_registrations
  @local_registrations
end

The PR would need a test to make sure publishing works with frozen objects so it doesn't get changed in the future :) Probably in the integration_spec.

krisleech avatar Feb 16 '17 12:02 krisleech

That fixes the case where the publisher doesn't have a constructor.

Here are two specs I added to integration_spec.rb. Adding your suggested code, the first one passes, the second fails because the publisher overwrites #initialize after the module is included:

  it 'publishes events from frozen publishers' do
    listener = double('listener')
    expect(listener).to receive(:success).with('hello')

    command = MyCommand.new
    command.freeze

    command.subscribe(listener)

    command.execute(true)
  end

  it 'publishes events from frozen publishers with custom constructors' do
    listener = double('listener')
    expect(listener).to receive(:success).with('constructor argument')

    command = MyCommandWithConstructor.new('constructor argument')
    command.freeze

    command.subscribe(listener)

    command.execute
  end

So I propose to not support local registrations on frozen objects (silently "just work" at subscribe time; raise an error if someone tries to add one). I think anybody bothering with frozen objects will agree that's reasonable, at least as a first step.

replaid avatar Feb 16 '17 16:02 replaid