Graph updates do not filter

[cflipse]

Graph commands work great for creates, or where the only filtering needed is based off the root element; however nested commands do not get applied well. Update or Delete commands are applied to all elements of a given relation.

Of additional interest is that all command execute methods will receive 2 arguments, where the standard delete command expects 0 arguments. This probably points to the solution required -- making it possible to filter from within the execute command.

I suspect being able to do something like:

def execute(tuple, user)
  execute_on(relation.for_user(user).by_id(tuple[:id]))
end

would be a good solution, but that essentially requires updating every adapter command to work correctly.

Additionally, I'm not sure that there's a way to cleanly separate update values, such that the update only happens to certain elements; this probably comes down to requiring a super loop in the exectute method, but I suspect that this is too easy to mess up.

  def execute(tuples,  user)
    for_user = relation.for_user(user)
    tuples.each do |tuple|
      super(execute_on(for_user.by_id(tuple[:id]), tuple))
    end
  end

  # avoids a situation where
  # [{ name: "Task One", id: 33 }, { name: "Task Two", id: 34 }] 
  # ends up naming all tasks "Task Two"

[solnic]

So, if I understand this correctly, the problem is that we don't support restricting relations of update and delete commands when using a command graph.

The only API we have to restrict them is proxying to relation and returning a new command with restricted relation like delete_user.by_id(1).call) and there is no way to do delete_user.call(:by_id, 1). I'm wondering if having that would help here somehow.

Regarding update, there is no API for updating many tuples where each tuple would require restricting a relation, an update is applied to the whole relation, when an update command receives many tuples I'm actually not sure what's gonna happen now. In case of a graph with an update command that receives many tuples we simply cannot support that. Changing execute logic to make it loop through tuples and make assumptions about how a relation should be restricted would be a bit against the design.

I feel like what we need is a way to configure graph to restrict commands and in case of many tuples it would loop through them and call a command separately. Maybe something like:

rom.command([
  :users, [{ update: [:by_id, [:id]] }]
])

# then graph internally would do something like:

input = { users: [ { id: 1, name: "Jane" }, { id: 2, name: "John" } ] }
input[:users].each { |tuple| rom.command(:users).by_id(tuple[:id]).call(tuple) }

[solnic]

ps. actually my idea applies to both update and delete command types

[cflipse]

maybe something like that, yes. Though I can see cases where it gets really wonky, because you need to restrict by data from both the parent relation, and within the sub relation ... [:tasks, {by_user_id: :user_id, by_id: :id }] ... :/

At that point though ... meh. Is there a reason to have symbols for the command, rather than a curried command? I guess that fails because you may need to feed input into different points along the command chain. :/ command(:tasks).update.for_user.by_name doesn't actually work out.

[solnic]

Symbols are more concise and were sufficient until now :) We can come up with an API that can use curried commands instead, graph builder simply resolves commands from symbols so if it receives a command already it can just use that.

[solnic]

So how about:

rom.command(
  [{ users: :user }, [
    { update: [:by_id, ['user.id']] },
    [:tasks, [{ update: [:by_user_and_name, ['user.id', 'user.tasks.name']] }]
  ]
)

Where we have a structure [relation_name, { command_name: [relation_view, [paths_to_args_from_input]]. I believe this could work.

[solnic]

@cflipse I made your spec pass in #313 (with a couple of small corrections in the spec itself) :tada:

[cflipse]

I start worrying when I see method paths encoded as strings.

Is that really better than

rom.command(
  [{ users: :user }, [
    { update: ->(r, user) { r.by_id(user[:id]) } },
    [ :tasks, [{ update: ->(r, user, task) { r.by_user_and_name(user[:id], task[:name]) }] }]
  ]
)

I'm not sure that's great either. Mostly at this point, I'm kind of afraid that the syntax we're coming up with is just this side of impenetrable.

[solnic]

I wanted to stick to ast-like structure and using array/hash so that it's easy to build a DSL on top of it. I treat this interface as a lower-level thing. We could easily support procs too. I can experiment with this and see how it goes.

[solnic]

@cflipse ended up with proc-only approach, thanks for suggestion, could you test if it covers your use-cases? I'd like to refactor it and merge it in soon

[cflipse]

Sure, will take a look

[solnic]

@cflipse bump ;)

[cflipse]

Sorry, been at a conference all weekend. I'll check this first thing in the morning.

[cflipse]

Ugh; I'm having the damndest time even getting a simple case to work, at the moment. Going crosseyed on brackets and braces I think. Am looking though

[solnic]

This is now fixed as you can pass procs that receive command + input so you can properly restrict the relations. There will also be a new DSL for building up a graph via #323