From 4143c948c7683b04823d14bda7b9c0397938e3c5 Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Sat, 6 Apr 2024 14:28:57 +0200 Subject: [PATCH] Refactor OpenFeature::FlagD::Provider::Configuration Signed-off-by: Alexandre Chakroun --- .../lib/openfeature/flagd/provider.rb | 4 +- .../flagd/provider/configuration.rb | 64 +++++-------------- .../spec/openfeature/flagd/provider_spec.rb | 21 +++--- 3 files changed, 27 insertions(+), 62 deletions(-) diff --git a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb index 4b0daae..3d9b633 100644 --- a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb +++ b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb @@ -50,9 +50,9 @@ def respond_to_missing?(method_name, include_private = false) end def configuration - @configuration ||= explicit_configuration + @configuration ||= Configuration.default_config .merge(Configuration.environment_variables_config) - .merge(Configuration.default_config) + .merge(explicit_configuration) end def configure(&block) diff --git a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/configuration.rb b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/configuration.rb index 8e44db2..bb58f63 100644 --- a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/configuration.rb +++ b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/configuration.rb @@ -6,9 +6,7 @@ module Provider # Represents the configuration object for the FlagD provider, # This class is not meant to be interacted with directly but instead through the # OpenFeature::FlagD::Provider.configure method - class Configuration - attr_accessor :host, :port, :tls, :unix_socket_path, :root_cert_path - + class Configuration < Struct.new(:host, :port, :tls, :unix_socket_path, :root_cert_path, keyword_init: true) ENVIRONMENT_CONFIG_NAME = { host: "FLAGD_HOST", port: "FLAGD_PORT", @@ -17,56 +15,26 @@ class Configuration root_cert_path: "FLAGD_SERVER_CERT_PATH" }.freeze - def merge(other_configuration) - return self if other_configuration.nil? - - @host = other_configuration.host if !other_configuration.host.nil? && @host.nil? - @port = other_configuration.port if !other_configuration.port.nil? && @port.nil? - @tls = other_configuration.tls if !other_configuration.tls.nil? && @tls.nil? - if !other_configuration.unix_socket_path.nil? && @unix_socket_path.nil? - @unix_socket_path = other_configuration.unix_socket_path + class << self + def default_config + new(host: "localhost", port: 8013, tls: false, unix_socket_path: nil, root_cert_path: nil) end - if !other_configuration.root_cert_path.nil? && @root_cert.nil? - @root_cert = File.read(other_configuration.root_cert_path) - end - - self - end - def self.environment_variables_config - configuration = Configuration.new - unless ENV[ENVIRONMENT_CONFIG_NAME[:host]].nil? - configuration.host = ENV.fetch(ENVIRONMENT_CONFIG_NAME[:host], - nil) - end - unless ENV[ENVIRONMENT_CONFIG_NAME[:port]].nil? - configuration.port = ENV.fetch(ENVIRONMENT_CONFIG_NAME[:port], - nil) + def environment_variables_config + new( + host: ENV.fetch(ENVIRONMENT_CONFIG_NAME[:host], nil), + port: ENV[ENVIRONMENT_CONFIG_NAME[:port]].nil? ? nil : Integer(ENV[ENVIRONMENT_CONFIG_NAME[:port]]), + tls: ENV[ENVIRONMENT_CONFIG_NAME[:tls]].nil? ? nil : ENV.fetch(ENVIRONMENT_CONFIG_NAME[:tls], nil) == "true", + unix_socket_path: ENV.fetch(ENVIRONMENT_CONFIG_NAME[:unix_socket_path], nil), + root_cert_path: ENV.fetch(ENVIRONMENT_CONFIG_NAME[:root_cert_path], nil) + ) end - unless ENV[ENVIRONMENT_CONFIG_NAME[:tls]].nil? - configuration.tls = ENV.fetch(ENVIRONMENT_CONFIG_NAME[:tls], - nil) == "true" - end - unless ENV[ENVIRONMENT_CONFIG_NAME[:unix_socket_path]].nil? - configuration.unix_socket_path = ENV.fetch(ENVIRONMENT_CONFIG_NAME[:unix_socket_path], - nil) - end - unless ENV[ENVIRONMENT_CONFIG_NAME[:root_cert_path]].nil? - root_cert_path = ENV.fetch(ENVIRONMENT_CONFIG_NAME[:root_cert_path], nil) - configuration.root_cert = File.read(root_cert_path) - end - - configuration end - def self.default_config - configuration = Configuration.new - configuration.host = "localhost" - configuration.port = 8013 - configuration.tls = false - configuration.unix_socket_path = nil - configuration.root_cert_path = nil - configuration + def merge(other_configuration) + return self if other_configuration.nil? + + self.class.new(**self.to_h.compact.merge(other_configuration.to_h.compact)) end end end diff --git a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb index 93c3352..221f993 100644 --- a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb +++ b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb @@ -44,29 +44,26 @@ it "uses the explicit configuration" do explicit_configuration - expect(described_class.configuration.host).to eq(explicit_host) - expect(described_class.configuration.port).to eq(explicit_port) - expect(described_class.configuration.tls).to eq(explicit_tls) + expect(described_class.configuration.host).to eq("explicit_host") + expect(described_class.configuration.port).to eq(8013) + expect(described_class.configuration.tls).to be_falsy end end end context "when defining environment variables" do - let(:env_host) { "172.16.1.2" } - let(:env_port) { "8014" } - let(:env_tls) { "true" } subject(:env_configuration) do - ENV["FLAGD_HOST"] = env_host - ENV["FLAGD_PORT"] = env_port - ENV["FLAGD_TLS"] = env_tls + ENV["FLAGD_HOST"] = "172.16.1.2" + ENV["FLAGD_PORT"] = "8014" + ENV["FLAGD_TLS"] = "true" described_class.configuration end it "uses environment variables when no explicit configuration" do env_configuration - expect(env_configuration.host).to eq(env_host) - expect(env_configuration.port).to eq(env_port) - expect(env_configuration.tls).to eq(env_tls == "true") + expect(env_configuration.host).to eq("172.16.1.2") + expect(env_configuration.port).to eq(8014) + expect(env_configuration.tls).to be_truthy end end end