Method based operations

[marten]

I'm not happy about the API as-is, but thought I'd throw this out as a first draft so that you can help think about the API. Main issue is how to define attributes.

For now, consider this a more of a spike, the implementation is probably messy and can be mostly ignored until we get an idea for the end-user API. That's also why I've only added an integration test. Once we know what we want I'll redo the thing with TDD.

[markijbema]

Since this is a spike, I think you don't really want code specific feedback, so generic feedback here:

I would think that we want to have the same objects for queries and commands if you do this. A lot of the code will be common, for instance, Lets say you have a command to write a hash to redis, and a query to retrieve it. The logic to calculate the redis-key will be the same.

But doing so is tricky. In our current approach it's easy to see that queries don't depend on commands, and it's easier to do that wrong if you combine them. In general I like this direction. I think our current queries have a lot of overhead while they don't need it (our queries do (/should do) authorization nor validation)

[markijbema]

Just noticed your comment about the attributes. would that be needed here? If we don't want validation, what is the advantage of defining them explicitly? Since correct attributes aren't forced on initialization of the query, but only when executing it, wouldn't a small helper function checking presence of required attributes be enough? What if we would do it like this:

class Queries::User
    USERS = {
      1 => 'Piet Snot',
      2 => 'Henk Hengelaar'
    }

    attribute :id

    additional_attributes
    def by_id(attributes = {})
      USERS[attributes[:id]]
    end

    additional_required_attributes [:name, :email]
    def update(attributes = {})
      USERS[attributes.fetch(:id)].set {
        name: attributes[:name],
        email: attributes[:email]
      } 
    end

    # or even
    command :update, requires: [:name, :email], accepts: [:twitter] do
      USERS[attributes.fetch(:id)].set {
        name: attributes[:name],
        email: attributes[:email],
        twitter: attributes.fetch(:twitter) { 'no-twitter-account' }
      } 

    end
end

[marten]

For defined attributes Virtus also performs coercion, so it might be confusing if you don't get that for method operations.

We could also wrap Virtus' attribute method, and start without the global attributes, clearing the set of defined attributes after every method definition.

class Queries::User
  include Pavlov::OperationCollection

  attribute :id, Integer
  def by_id
    USERS[id]
  end

  attribute :id, Integer
  attribute :name, String
  attribute :email, String
  attribute :twitter, String, default: 'no-twitter-account'
  def update
    USERS[attributes.fetch(:id)].set {
      name: name,
      email: email,
      twitter: twitter
    } 
  end
end

[marten]

Another option is to do this (which would be basically the entire needed patch to the existing codebase:

module Pavlov::Operation
  def self.call(*args, &block)
    new(*args).call(&block)
  end
end

module Pavlov
  def self.query(*args, &block)
    klass = class_for_query(query_name)
    klass.call(*args) # instead of klass.new(*args).call
  end
end

So that you can do:

class Queries::User
  ById = ->(attributes) { USERS[attributes.fetch[:id]) }
end

[markijbema]

Hmm, I like that as well. Would also allow you to use other types of callables (I think you can implement by id for ohm models by assigning the class ;) )