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

Configurable StructCompiler #528

Open
solnic opened this issue Apr 27, 2019 · 2 comments

Comments

@solnic
Copy link
Member

commented Apr 27, 2019

Currently, it's not possible to configure a custom StructCompiler because MapperCompiler uses a hard-coded constant. This can be easily changed to use an option with the default value set to StructCompiler.

Examples

This could be configurable on a per-relation basis, with the ability to pre-configure it for all relations from a specific gateway instance.

Setting for all relations in a given gateway

# set it for all relations, this can be handled in `finalize_relations` because we have
# access to gateway instance there
config = ROM::Configuration.new(:sql, 'sqlite::memory', struct_compiler: MyStructCompiler)
rom = ROM.container(config)

Setting per relation

class Users < ROM::Relation[:sql]
  # the same method should be used in `finalize_relation` when we set it
  # via gateway config
  struct_compiler MyStructCompiler

  schema(:users, infer: true)
end

TODO

  • Tweak MapperCompiler so that it defines option :struct_compiler, reader: false, default: -> { StructCompiler } and use it in the initialize method options[:struct_compiler].new
  • Tweak FinalizeRelations so that #build_relation uses gateway.options[:struct_compiler] to configure Relation.struct_compiler
  • Tweak Relation.mapper_registry so that it passes configured struct_compiler to the MapperCompiler constructor

Resources

Refs #519

@solnic solnic added the feature label Apr 27, 2019

@solnic solnic added this to the 5.1.0 milestone Apr 27, 2019

@parndt

This comment has been minimized.

Copy link

commented Apr 27, 2019

@flash-gordon

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

For the record, dry-struct 1.0 has support for "direct loading" i.e. without type checks. It's done internally via allocate + send(:initialize, attributes). It's ok to instantiate structs this way when the storage has internal type checks (e.g. in case of a SQL database). We probably want to use it for loading ROM::Structs because it's faster. Since it doesn't work for all cases, it should be an adapter-specific setting with possible manual override. We should also consider case where base class has constructor types in its definition (no idea if we should detect it automatically).

@solnic solnic modified the milestones: 5.1.0, 5.2.0 Jul 26, 2019

@solnic solnic added the core label Jul 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.