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

Add Alchemy.config and DSL for type-safe configuration #3178

Merged
merged 20 commits into from
Feb 10, 2025

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Jan 30, 2025

What is this pull request for?

This adds a Ruby configuration parent class with a DSL for configuration objects. The intention is for this to replace the current use of parsed YAML files.

The disadvantages of the current system are

  • Typos raise no errors
  • Wrong types raise no errors
  • We could put almost anything into the config file, and it would break nothing
  • Configuration management involves merging big hashes
  • Deprecations are hard, because Hash keys can't be deprecated

The configuration object in this PR offers the following DSL

class MyConfig < Alchemy::Configuration
  class AnotherConfig < Alchemy::Configuration
    option :level_1, :string, default: "ground_floor"
    option :level_2, :string, default: "open_floor"
  end
  option :foo, :integer, default: 1
  option :foos, :integer_list, default: [1, 2]
  option :bar, :string, default: "string"
  option :bars, :string_list, default: %w(hello world)
  configuration :levels, AnotherConfig
end

This is equivalent to the following hash, and can indeed be initialized with it:

conf_hash = {
  foo: 1,
  foos: [1, 2],
  bar: "string",
  bars: ["hello", "world"],
  levels: {
    level_1: "ground_floor",
    level_2: "open_floor",
  }
}
myconfig = MyConfig.new(conf_hash)

Parts of the configuration can be set later on, via Ruby or via a Hash:

myconfig.set(bar: "Actually a bar")
myconfig.bars = ["The", "bar", "district"]
myconfig.levels.level_1 = "garden"

Trying to set an option that has not been defined before will raise an NoMethodError.
Trying to set an option with data that does not have the right type will raise a TypeError.

With this change, default configuration options are now located in Alchemy's lib folder, so we don't have a default config.yml file anymore. I've removed it in this PR, because it's likely going to go stale soon (defaults change, and so on).

New users will get a generated alchemy.rb file, but the existing config.yml files will continue to work with no deprecation messages for now.

Alchemy.configure do |config|
  config.auto_logout_time = 45
end

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 99.67105% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.69%. Comparing base (d2cf3f4) to head (19f2f7c).
Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
lib/alchemy/configuration/base_option.rb 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3178      +/-   ##
==========================================
+ Coverage   96.59%   96.69%   +0.10%     
==========================================
  Files         237      256      +19     
  Lines        6380     6606     +226     
==========================================
+ Hits         6163     6388     +225     
- Misses        217      218       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mamhoff mamhoff force-pushed the config-object branch 7 times, most recently from 7cb31d1 to 4626d14 Compare January 31, 2025 16:57
@mamhoff mamhoff marked this pull request as ready for review January 31, 2025 17:28
@mamhoff mamhoff requested a review from a team as a code owner January 31, 2025 17:28
@mamhoff
Copy link
Contributor Author

mamhoff commented Jan 31, 2025

Codeclimate is not right. Abstracting that out would create an indirection that will be hard to read. The metaprogramming here is not so easy as-is.

@tvdeyen tvdeyen added this to the 8.0 milestone Feb 3, 2025
@mamhoff mamhoff force-pushed the config-object branch 4 times, most recently from 86ac849 to d936c0c Compare February 4, 2025 18:05
@mamhoff mamhoff force-pushed the config-object branch 7 times, most recently from cff6cf5 to bee8d87 Compare February 6, 2025 23:40
@mamhoff mamhoff changed the title Config object Add Alchemy.config and DSL for type-safe configuration Feb 10, 2025
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Thanks

This describes the very basic functioning of a new configuration class
that other class can inherit from. It provides a class method `.option`
that takes a type and a default value, and generates setter and getter
methods.
We want to make sure the type of things is right. This ensures that.
This can validate and store anything in a typed fashion.
We have a lot of Boolean config options.
This should allow us to simply ingest yml files and keep the current
`alchemy/config.yml` files.
This class contains all of the keys documented in the alchemy/config.yml
file.

When doing this I realized I needed to tweak the
`Alchemy::Configuration` superclass a little bit:

- Replace "/" with "_" when getting and setting keys, because methods
  with slashes in them are strange
- Handle nil when setting values. Nil should be allowed for now (we
  could do mandatory/optional values, but that's not in scope right now.
This allows us to have a programmatic configuration DSL in e.g. an
initializer:

```rb
Alchemy.configure do |config|
  config.auto_logout_time = Rails.env.test? ? 10 : 20
end
```
We don't do this in the config class anymore, so we have to do it
somewhere. Defaults are given in the configuration classes, so we don't
need to run the config.yml in Alchemy itself.
This deprecated option was also in the old config.rb file, so we
reimplement it here.
We don't need it in Alchemy's config directory anymore, because it's all
in the default config.

Also, moving the config into an initializer.
The `const_missing` needs to be defined both as a class method and as an
instance method in order to catch references to Alchemy::Config from the
class level and from the instance level. To avoid repetition, I've moved
the method into its own module that we now both `include` and `extend`
in the main `Alchemy` module.
While this is somewhat complicated code, I am sure it'll come in handy -
not just for testing the `config.yml` file.
@tvdeyen tvdeyen enabled auto-merge February 10, 2025 14:33
@tvdeyen tvdeyen merged commit 6ca28c5 into AlchemyCMS:main Feb 10, 2025
53 checks passed
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.

2 participants