Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

schema(infer: true) overrides command's input #360

Closed
solnic opened this issue Nov 8, 2016 · 4 comments
Closed

schema(infer: true) overrides command's input #360

solnic opened this issue Nov 8, 2016 · 4 comments

Comments

@solnic
Copy link
Member

solnic commented Nov 8, 2016

From @Kukunin on November 4, 2016 17:56

In my application, I have different attributes format between schema (legacy tables) and my domain models. So I need to use mappers to both save and restore an entity.

While it's easy to use mappers for Relation, I haven't found a way to use for Command. So, I use command's input for this purpose.

It works great, until I want to set association for my relation. schema(infer: true) breaks everything.

After some investigation, I've found that inferred schema overrides my custom input for any commands.

So, without schema(infer: true), I get custom mapper as input

From: /Users/kukunin/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rom-sql-0.8.0/lib/rom/sql/commands/create.rb @ line 24 ROM::SQL::Commands::Create#execute:

    22: def execute(tuples)
    23:   insert_tuples = with_input_tuples(tuples) do |tuple|
 => 24:     attributes = input[tuple]
    25:     validator.call(attributes)
    26:     attributes.to_h
    27:   end
    28:
    29:   if insert_tuples.length > 1
    30:     multi_insert(insert_tuples)
    31:   else
    32:     insert(insert_tuples)
    33:   end
    34: end

[6] pry(#<Persistence::Commands::Hoi::CreateCaseFromModel>)> input
=> #<Method: Persistence::Mappers::Hoi::CasesTuple#to_tuple>

But with schema(infer: true), it is:

[5] pry(#<Persistence::Commands::Hoi::CreateCaseFromModel>)> input
=> #<Dry::Types::Constructor type=#<Dry::Types::Hash::Schema primitive=Hash options={:member_types=>{:id=>#<Dry::Types::Constrained type=#<Dry::Types::Definition primitive=Integer options={}> options={:rule=>#<Dry::Logic::Operations::And rules=[#<Dry::Logic::Rule::Predicate predicate=#<Method: Module(Dry::Logic::Predicates::Methods)#type?> options={:args=>[Integer]}>, #<Dry::Logic::Rule::Predicate predicate=#<Method: Module(Dry::Logic::Predicates::Methods)#gt?> options={:args=>[0]}>] options={}>, :meta=>{:primary_key=>true, :name=>:id}}.....A LOT OF TEXT

So, because It doesn't map my domain entity, it crashes.

My relation:
      class Relations::Cases < ROM::Relation[:sql]
        register_as :hoi_cases
        dataset :cases
      end

and command with custom mapper:

      class Commands::CreateCaseFromModel < ROM::Command::Create[:sql]
        relation :hoi_cases
        register_as :create_from_model

        input Mappers::CasesTuple.build.method(:to_tuple)
      end

and just for clarity, my mapper code:

      class CasesTuple < ROM::Mapper
        relation :hoi_quotes
        register_as :tuple

        # mapping rules here

        def to_tuple(params)
          call([params]).first
        end
      end

P.S. If you know a better way to map entity to underlying schema in 2-directions more easily - tell me, please.

Thanks!

@solnic
Copy link
Member Author

solnic commented Nov 8, 2016

We can tweak commands in a way that if there's an input handler configured which is not the default proc, then we'll compose it with the schema handler. Would that work?

@solnic
Copy link
Member Author

solnic commented Nov 8, 2016

From @flash-gordon on November 4, 2016 18:57

This is possible to override Command.build method in this way

      class Commands::CreateCaseFromModel < ROM::Command::Create[:sql]
        relation :hoi_cases
        register_as :create_from_model

        def self.build(relation, options = {})
          # pass any input you want, you can access schema's input via relation.schema_hash
          super(relation, options.merge(input: ...)) 
        end
      end

This will do the trick for now ^ But I agree we need to make it more user-friendly :)
See sources: https://github.com/rom-rb/rom/blob/master/lib/rom/plugins/command/schema.rb#L16

@solnic
Copy link
Member Author

solnic commented Nov 11, 2016

@Kukunin this has been fixed in rom 2.0.2, this illustrates how it works now:

require 'rom'
require 'securerandom'

config = ROM::Configuration.new(:sql, 'sqlite::memory')

config.default.create_table :posts do
  primary_key :id
  column :title, String, null: false
  column :uuid, String
end

config.relation(:posts) do
  schema(infer: true)
end

class CreatePost < ROM::Command::Create[:sql]
  relation :posts
  register_as :create
  input -> tuple { tuple.key?(:uuid) ? tuple : tuple.merge(uuid: SecureRandom.uuid) }
end

config.register_command(CreatePost)

rom = ROM.container(config)

puts rom.commands[:posts][:create].call(title: "Hello World").inspect
# [{:id=>1, :title=>"Hello World", :uuid=>"fd2eafb6-30c1-46aa-9a9d-770a90a30a2a"}]

Now, schema hash is used as the second processing step, this means that your input handler is used first, and its result is passed to schema hash.

In example here we configure our schema with a constrained type for :uuid attribute:

require 'rom'
require 'securerandom'

config = ROM::Configuration.new(:sql, 'sqlite::memory')

config.default.create_table :posts do
  primary_key :id
  column :title, String, null: false
  column :uuid, String
end

config.relation(:posts) do
  schema do
    attribute :id, ROM::SQL::Types::Serial
    attribute :title, ROM::SQL::Types::Strict::String
    attribute :uuid, ROM::SQL::Types::Strict::String.constrained(format: /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i)
  end
end

class CreatePost < ROM::Command::Create[:sql]
  relation :posts
  register_as :create
  input -> tuple { tuple.key?(:uuid) ? tuple : tuple.merge(uuid: SecureRandom.uuid) }
end

config.register_command(CreatePost)

rom = ROM.container(config)

rom.commands[:posts][:create].call(title: "Hello World", uuid: 'not-uuid').inspect
# "not-uuid" (String) has invalid type for :uuid (Dry::Types::SchemaError)

@Kukunin
Copy link
Contributor

Kukunin commented Nov 13, 2016

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants