rom-rb / rom-http

Abstract HTTP adapter for ROM
https://rom-rb.org
MIT License
73 stars 18 forks source link

Flatten result of Create#execute #25

Closed maximderbin closed 7 years ago

maximderbin commented 7 years ago

Description: relation.insert always return array as we do Array([JSON.parse(response.body, symbolize_names: true)]).flatten in ResponseHandler#call

This is why Command::Create#execute results in something like: [[{id: 1, foo: 'bar'}]].

This behaviour brakes command composition and rom-sql associations

Example:

require 'bundler/inline'
require 'json'

gemfile(true) do
  source 'https://rubygems.org'
  gem 'rom', github: 'rom-rb/rom'
  gem 'rom-sql', github: 'rom-rb/rom-sql'
  gem 'rom-http', github: 'rom-rb/rom-http'
  gem 'sqlite3'
end

class RequestHandler
  def call(dataset)
    { id: '0032C000007NbpbQAC' }.to_json
  end
end

class ResponseHandler
  def call(response, dataset)
    Array([JSON.parse(response, symbolize_names: true)]).flatten
  end
end

config = ROM::Configuration.new(
  default: [:sql, 'sqlite::memory'],
  http: [:http,
    uri: 'https://foo.com',
    headers: {
      Accept: 'application/json'
    },
    request_handler: RequestHandler.new,
    response_handler: ResponseHandler.new
  ]
)

gateway = config.gateways[:default]
migrations = []

migrations << gateway.migration do
  change do
    create_table :users do
      primary_key :id
      column :contact_id, String, null: false, unique: true
    end
  end
end

migrations.each do |migration|
  migration.apply(gateway.connection, :up)
end

class Users < ROM::Relation[:sql]
  schema(infer: true) do
    associations do
      belongs_to :contact
    end
  end
end

class Contacts < ROM::Relation[:http]
  gateway :http

  schema do
    attribute :id, ROM::Types::Strict::String.meta(primary_key: true)
  end
end

module Commands
  class CreateUser < ROM::Commands::Create[:sql]
    relation :users
    register_as :create
    result :one

    associates :contact, key: [:contact_id, :id]
  end

  class CreateContact < ROM::Commands::Create[:http]
    relation :contacts
    register_as :create
    result :one
  end
end

config.register_relation(Users)
config.register_relation(Contacts)
config.register_command(Commands::CreateUser)
config.register_command(Commands::CreateContact)

env = ROM.container(config)

command = env.command([
  { contact: :contacts }, [:create, [{ user: :users }, [:create]]]
])
command.call(contact: { foo: 'bar', user: {} })
# /Users/simpleman/code/rom-http/vendor/ruby/2.3.0/bundler/gems/rom-604baf6c8633/lib/rom/commands/graph/input_evaluator.rb:45:in `call':
# undefined method `at' for {:foo=>"bar", :user=>{}}:Hash (NoMethodError)
AMHOL commented 7 years ago

@maximderbin thanks for the PR, I never actually got around to using rom-http beyond a simple PoC that only dealt with reads, sorry I missed that.

I'm not sure this should be handled implicitly in Command#execute TBH, it could mess with peoples data and cause unexpected errors, say if a command is expected to return a multi-dimensional array, this could be done in the response handler with something like:

class ResponseHandler
  def call(response, dataset)
    if %i(post put patch).include?(dataset.request_method)
      JSON.parse(response, symbolize_names: true)
    else
      Array([JSON.parse(response, symbolize_names: true)]).flatten
    end
  end
end

Let me know what you think.

cc: @solnic @cflipse

maximderbin commented 7 years ago

@AMHOL Yep I like it better. Do you want me to update usage examples then?

I'm also just paying around with it as POC. Just trying to break it haha 😆 Looking in potential replacing nasty her with rom-http

AMHOL commented 7 years ago

@maximderbin that would be great.

You're doing a good job so far :wink: lol

Interesting, first time I've heard of her

Gonna close this PR since we agree on the other solution

maximderbin commented 7 years ago

@AMHOL https://github.com/remiprev/her it's ORM over http. Like https://github.com/rails/activeresource but a liittle bit better. But too much buggy and too much metaprogramming in sources. And you can't combine AR and her. ROM looks much more appropriate for this