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

New settings api #651

Merged
merged 27 commits into from
Jul 9, 2021
Merged

New settings api #651

merged 27 commits into from
Jul 9, 2021

Conversation

solnic
Copy link
Member

@solnic solnic commented Jul 8, 2021

This replaces class-level config values with a new settings API powered by dry-configurable. All components have explicit settings defined now and there's a backward-compatibility shim added to rom/compat which makes sure that old-way of configuring classes through class attributes still works.

Here's how the default Relation settings look like:

    setting :auto_map, default: true
    setting :auto_struct, default: false
    setting :struct_namespace, default: ROM::Struct
    setting :wrap_class, default: Relation::Wrap

    setting :component do
      setting :id, default: :relation
      setting :dataset
      setting :adapter
      setting :gateway, default: :default
      setting :inflector, default: Inflector
    end

    setting :schema do
      setting :constant, default: Schema
      setting :dsl_class, default: Schema::DSL
      setting :attr_class, default: Attribute
      setting :inferrer, default: Schema::DEFAULT_INFERRER
    end

There's actually a very strict convention behind this grouping:

  • top-level settings are used by a component instance, ie in this case relation objects rely on things like auto_struct at runtime to know how mappers should be inferred
  • settings nested under component root key are used by configuration when establishing connections to gateways, figuring out which plugins should be enabled but first-and-foremost - IF a given component should become registered and what its :id should be. ie with component.id set to ":users" a relation will be available as rom.relations[:users] but also rom["relations.users"]
  • settings nested under a key that corresponds to a component type, schema in this example, are defaults that will be used when a given component defines its subcomponents. ie a relation can define its schema, so whatever you set here, will end up as defaults for that schema

On top of this, both class definitions and DSL usage is expected to work consistently when it comes to handling configuration settings and DSL options are now automatically translated to component configs.

For example, defining a component class vs defining a component via DSL ends up with the exact same config:

irb(main):017:1* class Users < ROM::Relation[:memory]
irb(main):018:0> end
irb(main):019:0> config = ROM::Configuration.new(:memory)
irb(main):020:0> config.relation(:users)
irb(main):021:0> config.components.get(:relations, id: :users).constant
=> ROM::Relations::Users
irb(main):022:0> config.components.get(:relations, id: :users).constant.config.component
=> #<Dry::Configurable::Config values={:id=>:users, :dataset=>:users, :adapter=>:memory, :gateway=>:default, :inflector=>#<Dry::Inflector>}>
irb(main):023:0> Users.config.component
=> #<Dry::Configurable::Config values={:id=>:users, :dataset=>:users, :adapter=>:memory, :gateway=>:default, :inflector=>#<Dry::Inflector>}>

@solnic solnic marked this pull request as ready for review July 9, 2021 17:10
solnic added 27 commits July 9, 2021 17:34
I finally realized that using relation's name object as a setting
was a mistake because it's a rich object used at runtime, it's not
part of the configuration.

Breaking it down into explicit :id and :dataset settings makes it
very clear what is what and we can easily infer them too. Still there's
additional complexity caused by backward compatibility but it will be
moved to rom/compat.

Specs are now running EVEN faster after this clean up, that's cool too.
@solnic solnic force-pushed the new-settings-api branch from ae822a4 to 144181b Compare July 9, 2021 17:35
@solnic solnic merged commit e08337d into master Jul 9, 2021
@solnic solnic deleted the new-settings-api branch July 9, 2021 17:40
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

Successfully merging this pull request may close these issues.

1 participant