Backend

[marten]

Notes

• I've kept the pavlov_options name instead of context so that existing operations don't need to be modified.
• Helpers detect if a backend is present, and if so, use the backend instead of going through Pavlov.
• RSpec expectations like Pavlov.should_receive(:query) will break when backend is introduced (to that controller or class). I have several ideas for workarounds, let me know if you need them.

  Fun Fact

If you add something like:

# spec/support/fake_controller_backend.rb
module FakeControllerBackend
  def self.included(base)
    base.let(:backend) { double("FakeBackend") }
    base.before(:each) do
      controller.stub(backend: backend)
    end
  end
end

# spec/spec_helper.rb
RSpec.configure {|c| c.include FakeControllerBackend, type: :controller }

You'll never ever be able to accidentally execute operations from controllers!

[marten]

I am reworking this branch so that it can be quickly merged without any backwards incompatibilities. That is to say, I want to keep the helpers and pavlov_options for now, and just inject a backend and operation_finder below the helpers.

Expect rebases and forced pushes on this branch. Tread carefully.

[marten]

@markijbema request for comments, after which I'll add changes to README, CHANGELOG and UPGRADING.

[markijbema]

I'm also missing a spec where we test an interactor, and set expectations on when backend is called. That would be useful for me to compare to our current implementation.

[markijbema]

Really liking this change overall. Seems like a great way out of using a classmethods on the Pavlov module, and really a step forwards. Awesome :)

[marten]

@markijbema re: spec for comparison: see the updated spec/integration/backend_spec.rb

[markijbema]

I'm still thinking about the upgrade path though, I don't fully see it yet. All in all the changes seem sane, but I wonder whether there's a way to upgrade the commands/queries/interactors one by one this way?

[marten]

Without the changes to the way pavlov_options are merged, the upgrade path was simply:

• Update Pavlov
• Tests should still pass
• Add a Backend class (rails g pavlov:install)
• Repeat: Change a single operation test from:

describe Interactors::Foo do
  include Pavlov::Helpers

  it 'runs a query' do
    Pavlov.should_receive(:query).with(:a_query_name)
    interactor :foo
  end
end

to:

describe Interactors::Foo do
  let(:backend) { Backend.new }

  it 'runs a query' do
    backend.should_receive(:query).with(:a_query_name)
    backend.interactor :foo
  end
end

• Repeat: Change a single controller to use the backend, and fix mock expectations set on Pavlov to expect on backend instead.

class FooController
  def index
    interactor :foo
  end
end

to

class FooController
  def index
    backend.interactor :foo
  end

  def backend
    @backend ||= Backend.new(pavlov_options: pavlov_options)
  end
end

You can do this step either locally or globally, but if you add def backend to ApplicationController, the helpers will switch to using backend globally, and therefore your mock expectations will fail globally. But fixing them should only be a matter of replacing Pavlov.should_receive by backend.should_receive. To expose the controller's backend, we used the snippet in the pull request description (under fun fact). We did this step globally, since it is only a gsub within spec/controllers.

• Remove all include Pavlov::Helpers. You should only be using the backend by now, and Pavlov::Helpers is now only from inside operations, where it's included for you.
• Done.

[jjoos]

Looks good!

The only comment I have is that initializing the backend with the context or pavlov_options does not feel natural to me. Since the life of the context is a lot shorter then that of a backend. But this will only be a problem when initializing a backend get's more expensive.

Have you got any thoughts about having multiple backends available in one app? (One for logging requests, etc?)

[marten]

What do you mean with a logging backend? Would that only log but not do anything or something? But my thoughts on multiple backends are: interesting, but I think it would be good if it could be extracted from an application. To do that I think you can extend and override aspects of backend. RoQua doesn't have a use case for this, so it's probably unlikely to come from our end. Basically, PDI. :)

[marten]

@jjoos I don't know that the backend ever has to be slow to initialize. You could always move the slow-to-create stuff out into seperate objects and keep those around, passing them into fresh backends.

@markijbema @jjoos What's blocking this?

[markijbema]

I maintain the backend shouldn't get a context in its constructor. And I think we shouldn't create that api if we are going to change it later.

[marten]

So what kind of architecture/names would you prefer?

[markijbema]

I would expect we need an object which contains both the backend and the context, say 'session'. On session, you would be able to call the same commands and queries as on backend, but not pass in the context. Session will then proxy those commands to backend, but inject the context.

I would expect it to work something like this, in the interactor (call chain):

command :foo, bar: 'baz'
session.command :foo, bar: 'baz'
backend.command :foo, bar: baz, context: session.context

Where the session is initiated like this:

Session.new(backend: backend, context: context)

Does this make sense?

[jjoos]

I think that the name session in aweful. But I agree with the concept.

[marten]

I think the name session is great, but unfortunately completely off-limits.

[jjoos]

Luckily we agree on the outcome ;)!

[marten]

Since I don't really believe we're going to come up with a good name soon, shall we proceed with either:

• something very lame But unique and search/replace friendly, eg: PavlovInteractionSession
• something whimsical and unique, eg Ivan

[jjoos]

I vote for: Cortex.

Since this term isn't used (much) in CS and means the shell, or 'outer portion' of something.

[marten]

Acknowledgement and acceptance of terms.

[marten]

We've stopped using this branch, so I'm voting to close this PR.