From f028c398db3e2317847fe7e7bcbe6bbe96bb0b1c Mon Sep 17 00:00:00 2001 From: Max VelDink Date: Wed, 3 Apr 2024 20:58:09 -0400 Subject: [PATCH] refactor!: Separate `Client` and `Provider` metadata, add client creation tests (#116) Signed-off-by: Max VelDink --- Gemfile | 2 - Gemfile.lock | 2 - lib/open_feature/sdk/api.rb | 8 +-- lib/open_feature/sdk/client.rb | 4 +- lib/open_feature/sdk/client_metadata.rb | 5 ++ lib/open_feature/sdk/configuration.rb | 4 -- lib/open_feature/sdk/metadata.rb | 38 ------------ lib/open_feature/sdk/provider.rb | 1 + .../sdk/provider/in_memory_provider.rb | 2 +- .../sdk/provider/no_op_provider.rb | 4 +- .../sdk/provider/provider_metadata.rb | 7 +++ spec/open_feature/sdk/api_spec.rb | 4 +- spec/open_feature/sdk/client_spec.rb | 5 +- .../specification/flag_evaluation_api_spec.rb | 58 +++++++++++++++++++ 14 files changed, 82 insertions(+), 62 deletions(-) create mode 100644 lib/open_feature/sdk/client_metadata.rb delete mode 100644 lib/open_feature/sdk/metadata.rb create mode 100644 lib/open_feature/sdk/provider/provider_metadata.rb diff --git a/Gemfile b/Gemfile index 76a864b..3b2e7f2 100644 --- a/Gemfile +++ b/Gemfile @@ -4,5 +4,3 @@ source "https://rubygems.org" # Specify your gem's dependencies in openfeature-sdk.gemspec gemspec - -gem "concurrent-ruby", require: "concurrent" diff --git a/Gemfile.lock b/Gemfile.lock index 9b084ee..9cdc00a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,7 +7,6 @@ GEM remote: https://rubygems.org/ specs: ast (2.4.2) - concurrent-ruby (1.2.3) debug (1.9.2) irb (~> 1.10) reline (>= 0.3.8) @@ -102,7 +101,6 @@ PLATFORMS x86_64-linux DEPENDENCIES - concurrent-ruby debug markly openfeature-sdk! diff --git a/lib/open_feature/sdk/api.rb b/lib/open_feature/sdk/api.rb index 4f57321..10334e1 100644 --- a/lib/open_feature/sdk/api.rb +++ b/lib/open_feature/sdk/api.rb @@ -6,8 +6,8 @@ require_relative "configuration" require_relative "evaluation_context" require_relative "evaluation_details" +require_relative "client_metadata" require_relative "client" -require_relative "metadata" require_relative "provider" module OpenFeature @@ -44,11 +44,11 @@ def configure(&block) end def build_client(name: nil, version: nil, domain: nil) - client_options = Metadata.new(name: name, version: version, domain: domain).freeze - active_provider = provider(domain:).nil? ? Provider::NoOpProvider.new : provider(domain:) - Client.new(provider: active_provider, client_options:, context:) + Client.new(provider: active_provider, domain:, context:) + rescue + Client.new(provider: Provider::NoOpProvider.new) end end end diff --git a/lib/open_feature/sdk/client.rb b/lib/open_feature/sdk/client.rb index 418bc2e..ec39a86 100644 --- a/lib/open_feature/sdk/client.rb +++ b/lib/open_feature/sdk/client.rb @@ -12,9 +12,9 @@ class Client attr_accessor :hooks - def initialize(provider:, client_options: nil, context: nil) + def initialize(provider:, domain: nil, context: nil) @provider = provider - @metadata = client_options + @metadata = ClientMetadata.new(domain:) @context = context @hooks = [] end diff --git a/lib/open_feature/sdk/client_metadata.rb b/lib/open_feature/sdk/client_metadata.rb new file mode 100644 index 0000000..8bbbcb8 --- /dev/null +++ b/lib/open_feature/sdk/client_metadata.rb @@ -0,0 +1,5 @@ +module OpenFeature + module SDK + ClientMetadata = Struct.new(:domain, keyword_init: true) + end +end diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb index d7a62fa..6953b31 100644 --- a/lib/open_feature/sdk/configuration.rb +++ b/lib/open_feature/sdk/configuration.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "concurrent" - require_relative "api" module OpenFeature @@ -15,8 +13,6 @@ class Configuration attr_accessor :context, :hooks - def_delegator :provider, :metadata - def initialize @hooks = [] @providers = {} diff --git a/lib/open_feature/sdk/metadata.rb b/lib/open_feature/sdk/metadata.rb deleted file mode 100644 index f1af85a..0000000 --- a/lib/open_feature/sdk/metadata.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -module OpenFeature - module SDK - # Metadata structure that defines general metadata relating to a Provider or Client - # - # Within the Metadata structure, the following attribute readers are available: - # - # * name - Defines the name of the structure - # - # * version - Allows you to specify version of the Metadata structure - # - # * domain - Allows you to specify the domain of the Metadata structure - # - # Usage: - # - # metadata = Metadata.new(name: 'name-for-metadata', version: 'v1.1.3', domain: 'test') - # metadata.name # 'name-for-metadata' - # metadata.version # version - # metadata_two = Metadata.new(name: 'name-for-metadata') - # metadata_two == metadata # true - equality based on values - class Metadata - attr_reader :name, :version, :domain - - def initialize(name:, version: nil, domain: nil) - @name = name - @version = version - @domain = domain - end - - def ==(other) - raise ArgumentError("Expected comparison to be between Metadata object") unless other.is_a?(Metadata) - - @name == other.name && @version == other.version - end - end - end -end diff --git a/lib/open_feature/sdk/provider.rb b/lib/open_feature/sdk/provider.rb index 959d069..f6b27cd 100644 --- a/lib/open_feature/sdk/provider.rb +++ b/lib/open_feature/sdk/provider.rb @@ -1,6 +1,7 @@ require_relative "provider/error_code" require_relative "provider/reason" require_relative "provider/resolution_details" +require_relative "provider/provider_metadata" require_relative "provider/no_op_provider" require_relative "provider/in_memory_provider" diff --git a/lib/open_feature/sdk/provider/in_memory_provider.rb b/lib/open_feature/sdk/provider/in_memory_provider.rb index 4beb372..5a49fab 100644 --- a/lib/open_feature/sdk/provider/in_memory_provider.rb +++ b/lib/open_feature/sdk/provider/in_memory_provider.rb @@ -8,7 +8,7 @@ class InMemoryProvider attr_reader :metadata def initialize(flags = {}) - @metadata = Metadata.new(name: NAME).freeze + @metadata = ProviderMetadata.new(name: NAME).freeze @flags = flags end diff --git a/lib/open_feature/sdk/provider/no_op_provider.rb b/lib/open_feature/sdk/provider/no_op_provider.rb index 3939272..358bd22 100644 --- a/lib/open_feature/sdk/provider/no_op_provider.rb +++ b/lib/open_feature/sdk/provider/no_op_provider.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative "../metadata" - # rubocop:disable Lint/UnusedMethodArgument module OpenFeature module SDK @@ -31,7 +29,7 @@ class NoOpProvider attr_reader :metadata def initialize - @metadata = Metadata.new(name: NAME).freeze + @metadata = ProviderMetadata.new(name: NAME).freeze end def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) diff --git a/lib/open_feature/sdk/provider/provider_metadata.rb b/lib/open_feature/sdk/provider/provider_metadata.rb new file mode 100644 index 0000000..b4497c0 --- /dev/null +++ b/lib/open_feature/sdk/provider/provider_metadata.rb @@ -0,0 +1,7 @@ +module OpenFeature + module SDK + module Provider + ProviderMetadata = Struct.new(:name, keyword_init: true) + end + end +end diff --git a/spec/open_feature/sdk/api_spec.rb b/spec/open_feature/sdk/api_spec.rb index d6488ef..2418e12 100644 --- a/spec/open_feature/sdk/api_spec.rb +++ b/spec/open_feature/sdk/api_spec.rb @@ -41,11 +41,11 @@ end it do - expect(api.provider.metadata).is_a?(OpenFeature::SDK::Metadata) + expect(api.provider.metadata).is_a?(OpenFeature::SDK::Provider::ProviderMetadata) end it do - expect(api.provider.metadata).to eq(OpenFeature::SDK::Metadata.new(name: OpenFeature::SDK::Provider::NoOpProvider::NAME)) + expect(api.provider.metadata).to eq(OpenFeature::SDK::Provider::ProviderMetadata.new(name: OpenFeature::SDK::Provider::NoOpProvider::NAME)) end end diff --git a/spec/open_feature/sdk/client_spec.rb b/spec/open_feature/sdk/client_spec.rb index 6f08c74..828402f 100644 --- a/spec/open_feature/sdk/client_spec.rb +++ b/spec/open_feature/sdk/client_spec.rb @@ -5,9 +5,8 @@ # https://openfeature.dev/docs/specification/sections/flag-evaluation#12-client-usage RSpec.describe OpenFeature::SDK::Client do - subject(:client) { described_class.new(provider: provider, client_options: client_metadata) } + subject(:client) { described_class.new(provider: provider, domain:) } let(:provider) { OpenFeature::SDK::Provider::NoOpProvider.new } - let(:client_metadata) { OpenFeature::SDK::Metadata.new(name: name, domain: domain) } let(:domain) { "testing" } let(:name) { "my-openfeature-client" } @@ -27,8 +26,6 @@ context "Requirement 1.2.2" do it "MUST define a metadata member or accessor, containing an immutable name field or accessor of type string, which corresponds to the name value supplied during client creation." do expect(client).to respond_to(:metadata) - expect(client.metadata).to respond_to(:name) - expect(client.metadata.name).to eq(name) expect(client.metadata.domain).to eq(domain) end end diff --git a/spec/specification/flag_evaluation_api_spec.rb b/spec/specification/flag_evaluation_api_spec.rb index 50a8c83..2ba5a9b 100644 --- a/spec/specification/flag_evaluation_api_spec.rb +++ b/spec/specification/flag_evaluation_api_spec.rb @@ -93,5 +93,63 @@ expect(OpenFeature::SDK.provider(domain: "not_here").metadata.name).to eq("In-memory Provider") end end + + context "Requirement 1.1.6" do + before do + default_provider = OpenFeature::SDK::Provider::InMemoryProvider.new + OpenFeature::SDK.set_provider(default_provider) + + domain_1_provider = OpenFeature::SDK::Provider::NoOpProvider.new + OpenFeature::SDK.set_provider(domain_1_provider, domain: "domain_1") + end + + specify "The API MUST provide a function for creating a client" do + client = OpenFeature::SDK.build_client + + expect(client.instance_variable_get(:@provider).metadata.name).to eq("In-memory Provider") + end + + specify "which accepts domain as an optional parameter." do + client = OpenFeature::SDK.build_client(domain: "domain_1") + + expect(client.instance_variable_get(:@provider).metadata.name).to eq("No-op Provider") + end + end + + context "Requirement 1.1.7" do + specify "The client creation function MUST NOT throw, or otherwise abnormally terminate." do + expect_any_instance_of(OpenFeature::SDK::Configuration).to receive(:provider).and_raise(StandardError) + + expect do + client = OpenFeature::SDK.build_client + + expect(client.instance_variable_get(:@provider).metadata.name).to eq("No-op Provider") + end.not_to raise_error + end + end + end + + context "1.2 - Client Usage" do + context "Requirement 1.2.1" do + pending "The client MUST provide a method to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed." + end + + context "Requirement 1.2.2" do + before do + default_provider = OpenFeature::SDK::Provider::InMemoryProvider.new + OpenFeature::SDK.set_provider(default_provider) + + domain_1_provider = OpenFeature::SDK::Provider::NoOpProvider.new + OpenFeature::SDK.set_provider(domain_1_provider, domain: "domain_1") + end + + specify "The client interface MUST define a metadata member or accessor, containing an immutable domain field or accessor of type string, which corresponds to the domain value supplied during client creation." do + client = OpenFeature::SDK.build_client + expect(client.metadata.domain).to be_nil + + client = OpenFeature::SDK.build_client(domain: "domain_1") + expect(client.metadata.domain).to eq("domain_1") + end + end end end