A proposal for a non-magical, but stricter context

[brianhempel]

In #46 we decided to use an OpenStruct for the context. It removed the magic of creating methods when things were set on the context. It seemed wrong to magically add methods to the interactor based on the calling parameters.

However, now we have problem that every time you want the given email, you have to say context.email. This is a break in the syntax-shortening advantage of encapsulation.

What if:

class AuthenticateUser
  include Interactor

  def call
    if user = User.authenticate(context.email, context.password)
      context.user = user
      context.token = user.secret_token
    else
      context.fail!(message: "authenticate_user.failure")
    end
  end
end

Instead became:

class AuthenticateUser
  include Interactor

  context_attributes :email, :password
  context_attributes :user, :token
  context_attribute :message

  def call
    if user = User.authenticate(email, password)
      context.user = user
      context.token = user.secret_token
    else
      context.fail!(message: "authenticate_user.failure")
    end
  end
end

The context itself is no longer an OpenStruct. Accessing or setting an undeclared attribute on the context will raise NoMethodError.

There are several advantages to this approach:

1. Accessing the context in code is easy: user instead of context.user.
2. It documents what attributes may be used/created on the context.
3. Typos fail fast inside interactors and inside code that calls interactors. AuthenticateUser.perform(eamil: "alice@example.com") can explode with an ArgumentError.

An organizer would gather all the context_attribute of its constituents and pre-populate the context will all the appropriate methods so that the application can call the organizer with arguments needed by any of the interactors.

[ecielam]

+1

context.user feels excessively verbose, and I didn't like it at all.

[laserlemon]

I love the idea and I think it may kill two birds with one stone.

I like the explicit context.user access because it's clear where the user object is coming from. However, a feature like you're suggesting could sit on top of that to provide the convenience methods that we used to have, only this time without the "magic" because it's clear where these values are coming from.

There's also a persistent (and justified) gripe that it's hard to glean what an interactor receives in its context. This helps to address that concern as well. The syntax that comes to mind for me is:

class AuthenticateUser
  include Interactor

  receives :email, :password
  provides :user, :message

  def call
    if user = User.authenticate(email, password)
      context.user = user
      context.token = user.secret_token
    else
      context.fail!(message: "authenticate_user.failure")
    end
  end
end

The receives and provides methods could actually be aliases of each other. The only purpose is documentation and to provide shortcut access to the values at the specified context keys.

In the future, this could be extended with options such as :required.

I wouldn't take any measures to ensure that a given context doesn't contain extra keys because that's perfectly legitimate. In the short term, I think these methods would simply provide convenience access to the context.

Thoughts?

[brianhempel]

Most (all?) of the interactors I've written receive and provide different things, so I'm in favor of that syntax. In the future it could be used to only add the provides from the returned context.

Every time I've used OpenStruct I've either not liked it, or regretted it, so I'm in favor of being more strict about what's in the context. But until we've got a good example that argues either way, we'll just have to disagree philosophically. :smile:

I do agree that shortcut access to the context with receives and provides is a win, regardless of whether or not we use them to make interactors stricter.

[laserlemon]

In the future it could be used to only add the provides from the returned context.

Could you elaborate on this? I don't understand.

[brianhempel]

We could make it so arguments aren't added to the context unless they're part of provides:

returned_context = AuthenticateUser.call(email: "alice@example.com", password: "password")
returned_context.attribute_names # => [:user, :token, :message]

Arguments not added to the context can still be accessed with methods inside the interactor, they're just stored on some sort of local_arguments object instead of on the context.

[laserlemon]

I don't think there's any need for something like local_arguments since interactors are just plain ol' Ruby objects. You can use instance variables or attr_accessors.

[ersatzryan]

I actually like this idea more than I initially thought I would. It prevents a nil smell that we get from using and OpenStruct.

[ersatzryan]

would this need to be a major version bump?

[laserlemon]

@ersatzryan Could you explain the nil smell it prevents? Would you expect that calling the email method directly would raise an error if not set?

[laserlemon]

My thought is that this would start as a backwards-compatible change. The receives (or similar) method would simply create the shortcut method(s) to the context and serve as documentation of what the interactor expects to receive. In my mind, it doesn't say that that key is necessarily required. Although, that could be added as an option:

receives :email, :password, required: true

I'm still not clear on what the default behavior should be for missing context keys/values. Should it fail? Should it raise an error?

[ersatzryan]

since we return a context from call and that context is an OpenStruct anytime you call a method that has not been set it doesn't result in an error only in nil this makes it so when you have work with a context object you have to be paranoid with lots of nil tests context.user && context.user.name etc.

As opposed to having an object that throws errors when you call methods that do not exist.

Tracking down an "Undefined Method for Nil::Class" 5 levels past where that nil was introduced is much harder than a custom error from the context or even a NoMethod just on the context.

[ersatzryan]

Since we return the context from call wouldn't needing to define what methods it has on it be not backwards compatible?

If I currently have something like

class FooController < ApplicationController
  def create
    result = FooCreator.call(foo_params: params)

    if result.foo
       redirect_to result.foo
    else
      render :new
    end
  end

class FooCreator
  include Interactor

  def call
     foo = Foo.new(context.foo_params)

     if foo.save
       context.foo = foo
     else
       fail!
     end
end

would it not bomb in the controller if it were upgraded to version that has new context not based on OpenStruct

[laserlemon]

I've been thinking more about the internal interface (inside the interactor itself) but it's a good point that these changes would also apply to how the interactor is accessed from the controller as well.

I'm not very concerned currently with unexpected nil values in the controller because the interactor should have clear expectation of what context keys will be set in the case of interactor success or failure.

We'd probably want to make sure that the ||= context assignment still works:

before do
  context.user ||= User.find(context.user_id)
end

[ryansch]

I'd be a fan of documenting the inputs and outputs of an interactor.

As far as required parameters go, we're using this syntax now:

required_parameters or: [:order_id, :order]
required_parameters :something_else

I use a before hook to ensure that both order_id and order are available in the context by filling one from the other.

Edit: I haven't yet gone so far as to write methods on to the interactor for those parameters but we've been talking about it as we upgrade from the old 'v3' branch to the released version 3.

[laserlemon]

LightService has a nice syntax for a similar feature documented here. The functionality differs from what we're talking about here but the macro methods are similar and might get the creative juices flowing for naming.

[ersatzryan]

I like the macro methods of expects and promises

[ersatzryan]

or perhaps expect and provides

[laserlemon]

I've used the "provide" terminology in the specs before.

[wyattjoh]

Any suggestions for the issue of ensuring required attributes are passed in while this feature is still being developed?

[zavan]

@wyattjoh What about being able to set default values to context attributes? Or should this be handled exclusively in the models that the Interactor handle?

[zavan]

Maybe in the future, the context object can evolve into something more complex and complete, like trailblazer's contracts (https://github.com/apotonick/trailblazer), thus allowing the validations to be removed from the model too...

[bradstewart]

I just started using this gem (thanks, it's great), and I really like where this is headed. As a quick and dirty "solution" to some of these issues, I've been using delegate--no more typing context every time you want to touch something, and it documents what I expect to be on the context.

E.g. delegate :user, :email, to: :context.

[quidproquo]

Love the concept behind this gem, though, it would be nice to have context be any type of object. Simply delegate method missing on the context to the passed-in object itself. I feel like forcing the developer to break up parameters into key/value pairs reduces flexibility.

[mjbellantoni]

:+1: to this proposal. I've been using this library (thank you! btw) for about a month and I almost immediately added an equivalent to context_attributes. I'm beginning to use organizers and pretty quickly wished something like requires/provides existed.

[jonstokes]

What about something that looks like this:

class CreateContact
  include Interactor

  expects :email, :first_name       # these are required, otherwise error
  provides :contact_id              # this may or may not be populated by the interactor
  allows :last_name, :phone_number  # these can be supplied, but if not default is nil

  allows :category do                # this can be supplied, or the block provides a default
    ContactCategories.default
  end

  allows :country, default: 'USA'     # this can be supplied, default declared inline
end

These could just start out as convenience methods, per Steve's suggestion, so that they don't break backwards compatibility.

[jonstokes]

FYI, all, if you're following this issue, I'm working on a PR that attempts to address it. You can find it here: https://github.com/collectiveidea/interactor/pull/82

[greyblake]

+1 for this proposal!

[BattleBrisket]

:+1: Is this still in the cards?

[andydust]

👍

[berfarah]

Huh... Great minds think alike, I guess!

I worked on something like this. Would welcome feedback on the features: https://github.com/berfarah/interactor-schema

[jonstokes]

@berfarah I made a gem for this, as well. Check it out, here: https://github.com/jonstokes/troupe

[laserlemon]

Closing for a few reasons:

1. 123 will address explicit inputs.

2. @jonstokes graciously wrote troupe.
3. There's no universally agreed-upon behavior for what should happen if what should be provided is not provided.