From cf60f11b11616eb1f9d6622d8e8e5b0e928ec7e7 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Mon, 8 Apr 2024 19:25:03 -0700 Subject: [PATCH 1/8] used build for semver class --- .../engine/matchers/in_list_semver_matcher.rb | 10 +++++----- lib/splitclient-rb/engine/matchers/semver.rb | 17 +++++++++++++---- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb index 7a424483..9a3ae614 100644 --- a/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb @@ -10,20 +10,20 @@ def initialize(attribute, list_value, logger, validator) super(logger) @validator = validator @attribute = attribute - @semver_list = list_value.map { |item| SplitIoClient::Semver.new(item) } + @semver_list = list_value.map { |item| SplitIoClient::Semver.build(item, logger) } @logger = logger end def match?(args) @logger.log_if_debug('[InListSemverMatcher] evaluating value and attributes.') - return false unless @validator.valid_matcher_arguments(args) + return false unless @validator.valid_matcher_arguments(args) && args[:attributes][@attribute.to_sym].is_a?(String) - value_to_match = args[:attributes][@attribute.to_sym] - unless value_to_match.is_a?(String) + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + unless !value_to_match.nil? && @semver_list.all? { |n| !n.nil? } @logger.log_if_debug('whitelistMatcherData is required for IN_LIST_SEMVER matcher type') return false end - matches = (@semver_list.map { |item| item.compare(SplitIoClient::Semver.new(value_to_match)) }).any?(&:zero?) + matches = (@semver_list.map { |item| item.compare(value_to_match) }).any?(&:zero?) @logger.log_if_debug("[InListSemverMatcher] #{value_to_match} matches -> #{matches}") matches end diff --git a/lib/splitclient-rb/engine/matchers/semver.rb b/lib/splitclient-rb/engine/matchers/semver.rb index ac9b7a58..92d9a6a8 100644 --- a/lib/splitclient-rb/engine/matchers/semver.rb +++ b/lib/splitclient-rb/engine/matchers/semver.rb @@ -8,10 +8,6 @@ class Semver attr_reader :major, :minor, :patch, :pre_release, :is_stable, :old_version - # - # Class Initializer - # - # @param version [String] raw version as read from splitChanges response. def initialize(version) @major = 0 @minor = 0 @@ -22,6 +18,19 @@ def initialize(version) parse end + # + # Class builder + # + # @param version [String] raw version as read from splitChanges response. + # + # @return [type] Semver instance + def self.build(version, logger) + new(version) + rescue RuntimeError => e + logger.warn("Failed to parse Semver data: #{e}") + nil + end + # # Check if there is any metadata characters in self._old_version. # From 980c272fd5e3f764e02e69bf6c95010e152c32b9 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 10 Apr 2024 09:38:46 -0700 Subject: [PATCH 2/8] added version attribute and used it in compare --- lib/splitclient-rb/engine/matchers/semver.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/splitclient-rb/engine/matchers/semver.rb b/lib/splitclient-rb/engine/matchers/semver.rb index 92d9a6a8..2dec6a71 100644 --- a/lib/splitclient-rb/engine/matchers/semver.rb +++ b/lib/splitclient-rb/engine/matchers/semver.rb @@ -6,7 +6,7 @@ class Semver PRE_RELEASE_DELIMITER = '-' VALUE_DELIMITER = '.' - attr_reader :major, :minor, :patch, :pre_release, :is_stable, :old_version + attr_reader :major, :minor, :patch, :pre_release, :is_stable, :old_version, :version def initialize(version) @major = 0 @@ -15,6 +15,8 @@ def initialize(version) @pre_release = [] @is_stable = false @old_version = version + @version = "" + @metadata = "" parse end @@ -39,7 +41,7 @@ def self.build(version, logger) def remove_metadata_if_exists index = @old_version.index(METADATA_DELIMITER) return @old_version if index.nil? - + @metadata = @old_version[index+1,@old_version.length] @old_version[0, index] end @@ -52,7 +54,7 @@ def remove_metadata_if_exists # # @returns [Integer] based on comparison def compare(to_compare) - return 0 if @old_version == to_compare.old_version + return 0 if @version == to_compare.version # Compare major, minor, and patch versions numerically return compare_attributes(to_compare) if compare_attributes(to_compare) != 0 @@ -100,6 +102,9 @@ def assign_major_minor_and_patch(version) @major = parts[0].to_i @minor = parts[1].to_i @patch = parts[2].to_i + @version = "#{@major}#{VALUE_DELIMITER}#{@minor}#{VALUE_DELIMITER}#{@patch}" + @version += "#{PRE_RELEASE_DELIMITER}#{@pre_release.join('.')}" if !@pre_release.empty? + @version += "#{METADATA_DELIMITER}#{@metadata}" if !@metadata.empty? end # From b9e8fd8282d6ced943f1ded164dfd25b8ab37f85 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 10 Apr 2024 09:50:44 -0700 Subject: [PATCH 3/8] updated semver spec --- spec/engine/matchers/semver_spec.rb | 103 +++++++++------------------- 1 file changed, 31 insertions(+), 72 deletions(-) diff --git a/spec/engine/matchers/semver_spec.rb b/spec/engine/matchers/semver_spec.rb index a0f98429..fbb705d5 100644 --- a/spec/engine/matchers/semver_spec.rb +++ b/spec/engine/matchers/semver_spec.rb @@ -1,92 +1,51 @@ require 'spec_helper' +require 'csv' describe SplitIoClient::Semver do - let(:valid_versions) { ["1.1.2", "1.1.1", "1.0.0", "1.0.0-rc.1", "1.0.0-beta.11", "1.0.0-beta.2", - "1.0.0-beta", "1.0.0-alpha.beta", "1.0.0-alpha.1", "1.0.0-alpha", "2.2.2-rc.2+metadata-lalala", "2.2.2-rc.1.2", - "1.2.3", "0.0.4", "1.1.2+meta", "1.1.2-prerelease+meta", "1.0.0-beta", "1.0.0-alpha", "1.0.0-alpha0.valid", - "1.0.0-alpha.0valid", "1.0.0-rc.1+build.1", "1.0.0-alpha-a.b-c-somethinglong+build.1-aef.1-its-okay", - "10.2.3-DEV-SNAPSHOT", "1.2.3-SNAPSHOT-123", "1.1.1-rc2", "1.0.0-0A.is.legal", "1.2.3----RC-SNAPSHOT.12.9.1--.12+788", - "1.2.3----R-S.12.9.1--.12+meta", "1.2.3----RC-SNAPSHOT.12.9.1--.12.88", "1.2.3----RC-SNAPSHOT.12.9.1--.12", - "9223372036854775807.9223372036854775807.9223372036854775807", "9223372036854775807.9223372036854775807.9223372036854775806", - "1.1.1-alpha.beta.rc.build.java.pr.support.10", "1.1.1-alpha.beta.rc.build.java.pr.support"] } + let(:valid_versions) do + CSV.parse(File.read(File.expand_path(File.join(File.dirname(__FILE__), + '../../test_data/splits/semver/valid-semantic-versions.csv')))) + end + let(:invalid_versions) do + CSV.parse(File.read(File.expand_path(File.join(File.dirname(__FILE__), + '../../test_data/splits/semver/invalid-semantic-versions.csv')))) + end + let(:equal_to_versions) do + CSV.parse(File.read(File.expand_path(File.join(File.dirname(__FILE__), + '../../test_data/splits/semver/equal-to-semver.csv')))) + end + + let(:logger) { Logger.new('/dev/null') } context 'check versions' do it 'accept valid versions' do - major = [1, 1, 1, 1, 1, 1, - 1, 1, 1, 1, 2, 2, - 1, 0, 1, 1, 1, 1, 1, - 1, 1, 1, - 10, 1, 1, 1, 1, - 1, 1, 1, - 9223372036854775807, 9223372036854775807, - 1,1] - minor = [1, 1, 0, 0, 0, 0, - 0, 0, 0, 0, 2, 2, - 2, 0, 1, 1, 0, 0, 0, - 0, 0, 0, - 2, 2, 1, 0, 2, - 2, 2, 2, - 9223372036854775807, 9223372036854775807, - 1, 1] - patch = [2, 1, 0, 0, 0, 0, - 0, 0, 0, 0, 2, 2, - 3, 4, 2, 2, 0, 0, 0, - 0, 0, 0, - 3, 3, 1, 0, 3, - 3, 3, 3, - 9223372036854775807, 9223372036854775806, - 1, 1] - pre_release = [[], [], [], ["rc","1"], ["beta","11"],["beta","2"], - ["beta"], ["alpha","beta"], ["alpha","1"], ["alpha"], ["rc","2"], ["rc","1","2"], - [], [], [], ["prerelease"], ["beta"], ["alpha"], ["alpha0","valid"], - ["alpha","0valid"], ["rc","1"], ["alpha-a","b-c-somethinglong"], - ["DEV-SNAPSHOT"], ["SNAPSHOT-123"], ["rc2"], ["0A","is","legal"], ["---RC-SNAPSHOT","12","9","1--","12"], - ["---R-S","12","9","1--","12"], ["---RC-SNAPSHOT","12","9","1--","12","88"], ["---RC-SNAPSHOT","12","9","1--","12"], - [], [], - ["alpha","beta","rc","build","java","pr","support","10"], ["alpha","beta","rc","build","java","pr","support"]] - - for i in (0..major.length-1) - semver = described_class.new(valid_versions[i]) - expect(verify_version(semver, major[i], minor[i], patch[i], pre_release[i], pre_release[i]==[])).to eq(true) + for i in (0..valid_versions.length-1) + expect(described_class.build(valid_versions[i][0], logger)).should_not be_nil end end it 'reject invalid versions' do - invalid_versions = [ - "1", "1.2", "1.alpha.2", "+invalid", "-invalid", "-invalid+invalid", "+justmeta", - "-invalid.01", "alpha", "alpha.beta", "alpha.beta.1", "alpha.1", "alpha+beta", - "alpha_beta", "alpha.", "alpha..", "beta", "-alpha.", "1.2", "1.2.3.DEV", "-1.0.3-gamma+b7718", - "1.2-SNAPSHOT", "1.2.31.2.3----RC-SNAPSHOT.12.09.1--..12+788", "1.2-RC-SNAPSHOT"] - for version in invalid_versions - expect{ described_class.new(version) }.to raise_error(RuntimeError) + expect(described_class.build(version[0], logger)).to eq(nil) end end end context 'compare versions' do - it 'higher, lower and equal' do - cnt = 0 - for i in (0..(valid_versions.length/2).to_i-1) - expect(described_class.new(valid_versions[cnt]).compare(described_class.new(valid_versions[cnt+1]))).to eq(1) - expect(described_class.new(valid_versions[cnt+1]).compare(described_class.new(valid_versions[cnt]))).to eq(-1) - expect(described_class.new(valid_versions[cnt]).compare(described_class.new(valid_versions[cnt]))).to eq(0) - expect(described_class.new(valid_versions[cnt+1]).compare(described_class.new(valid_versions[cnt+1]))).to eq(0) - cnt = cnt + 2 + it 'equal and not equal' do + for i in (1..valid_versions.length-1) + expect(described_class.build(valid_versions[i][0], logger).compare(described_class.build(valid_versions[i][1], logger))).to eq(1) + expect(described_class.build(valid_versions[i][1], logger).compare(described_class.build(valid_versions[i][0], logger))).to eq(-1) + expect(described_class.build(valid_versions[i][0], logger).compare(described_class.build(valid_versions[i][0], logger))).to eq(0) + expect(described_class.build(valid_versions[i][1], logger).compare(described_class.build(valid_versions[i][1], logger))).to eq(0) + end + for i in (1..equal_to_versions.length-1) + if valid_versions[i][2] + expect(described_class.build(valid_versions[i][0], logger).compare(described_class.build(valid_versions[i][1], logger))).to eq(0) + else + expect(described_class.build(valid_versions[i][0], logger).compare(described_class.build(valid_versions[i][1], logger))).not_to eq(0) + end end - expect(described_class.new("1.1.1").compare(described_class.new("1.1.1"))).to eq(0) - expect(described_class.new("1.1.1").compare(described_class.new("1.1.1+metadata"))).to eq(0) - expect(described_class.new("1.1.1").compare(described_class.new("1.1.1-rc.1"))).to eq(1) - expect(described_class.new("88.88.88").compare(described_class.new("88.88.88"))).to eq(0) - expect(described_class.new("1.2.3----RC-SNAPSHOT.12.9.1--.12").compare(described_class.new("1.2.3----RC-SNAPSHOT.12.9.1--.12"))).to eq(0) - expect(described_class.new("10.2.3-DEV-SNAPSHOT").compare(described_class.new("10.2.3-SNAPSHOT-123"))).to eq(-1) - end - end - def verify_version(semver, major, minor, patch, pre_release="", is_stable=True) - if semver.major == major && semver.minor == minor &&d semver.patch == patch && - semver.pre_release == pre_release && semver.is_stable == is_stable - return true end - return false end end From b1529d2e00057798b920c483871da8225118f3dd Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 10 Apr 2024 11:06:08 -0700 Subject: [PATCH 4/8] Fixed compare logic in semver, equalto and inlist matchers. --- .../matchers/equal_to_semver_matcher.rb | 2 +- .../engine/matchers/in_list_semver_matcher.rb | 2 +- lib/splitclient-rb/engine/matchers/semver.rb | 32 +++++++++---------- .../matches_between_semver_matcher_spec.rb | 4 +-- .../matches_equal_to_semver_matcher_spec.rb | 4 +-- ...er_than_or_equal_to_semver_matcher_spec.rb | 2 +- .../matches_in_list_semver_matcher_spec.rb | 7 ++-- ...ss_than_or_equal_to_semver_matcher_spec.rb | 2 +- 8 files changed, 28 insertions(+), 27 deletions(-) diff --git a/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb index 01f62baf..877b0f82 100644 --- a/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb @@ -23,7 +23,7 @@ def match?(args) @logger.log_if_debug('stringMatcherData is required for EQUAL_TO_SEMVER matcher type') return false end - matches = @semver.compare(SplitIoClient::Semver.new(value_to_match)).zero? + matches = (@semver.version == SplitIoClient::Semver.new(value_to_match).version) @logger.log_if_debug("[EqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") matches end diff --git a/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb index 9a3ae614..57b13ddd 100644 --- a/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb @@ -23,7 +23,7 @@ def match?(args) @logger.log_if_debug('whitelistMatcherData is required for IN_LIST_SEMVER matcher type') return false end - matches = (@semver_list.map { |item| item.compare(value_to_match) }).any?(&:zero?) + matches = (@semver_list.map { |item| item.version == value_to_match.version }).any? { |item| item == true } @logger.log_if_debug("[InListSemverMatcher] #{value_to_match} matches -> #{matches}") matches end diff --git a/lib/splitclient-rb/engine/matchers/semver.rb b/lib/splitclient-rb/engine/matchers/semver.rb index 2dec6a71..6669b94b 100644 --- a/lib/splitclient-rb/engine/matchers/semver.rb +++ b/lib/splitclient-rb/engine/matchers/semver.rb @@ -6,7 +6,7 @@ class Semver PRE_RELEASE_DELIMITER = '-' VALUE_DELIMITER = '.' - attr_reader :major, :minor, :patch, :pre_release, :is_stable, :old_version, :version + attr_reader :major, :minor, :patch, :pre_release, :is_stable, :version def initialize(version) @major = 0 @@ -14,10 +14,9 @@ def initialize(version) @patch = 0 @pre_release = [] @is_stable = false - @old_version = version - @version = "" - @metadata = "" - parse + @version = '' + @metadata = '' + parse(version) end # @@ -34,15 +33,16 @@ def self.build(version, logger) end # - # Check if there is any metadata characters in self._old_version. + # Check if there is any metadata characters in version. # # @return [type] String semver without the metadata # - def remove_metadata_if_exists - index = @old_version.index(METADATA_DELIMITER) - return @old_version if index.nil? - @metadata = @old_version[index+1,@old_version.length] - @old_version[0, index] + def remove_metadata_if_exists(old_version) + index = old_version.index(METADATA_DELIMITER) + return old_version if index.nil? + + @metadata = old_version[index + 1, old_version.length] + old_version[0, index] end # Compare the current Semver object to a given Semver object, return: @@ -70,10 +70,10 @@ def integer?(value) end # - # Parse the string in self._old_version to update the other internal variables + # Parse the string in version to update the other internal variables # - def parse - without_metadata = remove_metadata_if_exists + def parse(old_version) + without_metadata = remove_metadata_if_exists(old_version) index = without_metadata.index(PRE_RELEASE_DELIMITER) if index.nil? @@ -103,8 +103,8 @@ def assign_major_minor_and_patch(version) @minor = parts[1].to_i @patch = parts[2].to_i @version = "#{@major}#{VALUE_DELIMITER}#{@minor}#{VALUE_DELIMITER}#{@patch}" - @version += "#{PRE_RELEASE_DELIMITER}#{@pre_release.join('.')}" if !@pre_release.empty? - @version += "#{METADATA_DELIMITER}#{@metadata}" if !@metadata.empty? + @version += "#{PRE_RELEASE_DELIMITER}#{@pre_release.join('.')}" unless @pre_release.empty? + @version += "#{METADATA_DELIMITER}#{@metadata}" unless @metadata.empty? end # diff --git a/spec/engine/matchers/matches_between_semver_matcher_spec.rb b/spec/engine/matchers/matches_between_semver_matcher_spec.rb index 1fa7d0f8..12474791 100644 --- a/spec/engine/matchers/matches_between_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_between_semver_matcher_spec.rb @@ -14,9 +14,9 @@ matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.split_logger, config.split_validator) expect(matcher.attribute).to eq("version") semver_start = matcher.instance_variable_get(:@semver_start) - expect(semver_start.instance_variable_get(:@old_version)).to eq("2.1.8") + expect(semver_start.instance_variable_get(:@version)).to eq("2.1.8") semver_end = matcher.instance_variable_get(:@semver_end) - expect(semver_end.instance_variable_get(:@old_version)).to eq("2.1.11") + expect(semver_end.instance_variable_get(:@version)).to eq("2.1.11") end it 'matches' do diff --git a/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb index f1280866..f28d2521 100644 --- a/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb @@ -14,17 +14,17 @@ matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) expect(matcher.attribute).to eq("version") semver = matcher.instance_variable_get(:@semver) - expect(semver.instance_variable_get(:@old_version)).to eq("2.1.8") + expect(semver.instance_variable_get(:@version)).to eq("2.1.8") end it 'matches' do matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) - expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.8"})).to eq(true) end it 'does not match' do matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.5"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.5-rc1"})).to eq(false) end diff --git a/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb index 7888cbe9..be31742c 100644 --- a/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb @@ -14,7 +14,7 @@ matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) expect(matcher.attribute).to eq("version") semver = matcher.instance_variable_get(:@semver) - expect(semver.instance_variable_get(:@old_version)).to eq("2.1.8") + expect(semver.instance_variable_get(:@version)).to eq("2.1.8") end it 'matches' do diff --git a/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb b/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb index dad00078..bffe3da1 100644 --- a/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb @@ -14,18 +14,19 @@ matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.split_logger, config.split_validator) expect(matcher.attribute).to eq("version") semver_list = matcher.instance_variable_get(:@semver_list) - expect(semver_list[0].instance_variable_get(:@old_version)).to eq("2.1.8") - expect(semver_list[1].instance_variable_get(:@old_version)).to eq("2.1.11") + expect(semver_list[0].instance_variable_get(:@version)).to eq("2.1.8") + expect(semver_list[1].instance_variable_get(:@version)).to eq("2.1.11") end it 'matches' do matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.split_logger, config.split_validator) - expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(true) + expect(matcher.match?(:attributes=>{"version": "2.1.8"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.11"})).to eq(true) end it 'does not match' do matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.split_logger, config.split_validator) + expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.7"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.11-rc12"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.8-rc1"})).to eq(false) diff --git a/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb index 0a8bc6d3..978ec18e 100644 --- a/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb @@ -14,7 +14,7 @@ matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) expect(matcher.attribute).to eq("version") semver = matcher.instance_variable_get(:@semver) - expect(semver.instance_variable_get(:@old_version)).to eq("2.1.8") + expect(semver.instance_variable_get(:@version)).to eq("2.1.8") end it 'matches' do From a8761d71faf76e3301897cada26e23958dc01220 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 10 Apr 2024 19:10:53 -0700 Subject: [PATCH 5/8] Added nil check for all matchers and updated specs --- .../engine/matchers/between_semver_matcher.rb | 18 +++++++++--------- .../engine/matchers/equal_to_semver_matcher.rb | 14 +++++++------- .../greater_than_or_equal_to_semver_matcher.rb | 14 +++++++------- .../engine/matchers/in_list_semver_matcher.rb | 6 +++--- .../less_than_or_equal_to_semver_matcher.rb | 14 +++++++------- lib/splitclient-rb/engine/matchers/semver.rb | 4 ++-- .../repositories/splits_repository_spec.rb | 2 +- .../matches_between_semver_matcher_spec.rb | 8 ++++---- .../matches_equal_to_semver_matcher_spec.rb | 9 ++++----- ...ter_than_or_equal_to_semver_matcher_spec.rb | 8 ++++---- .../matches_in_list_semver_matcher_spec.rb | 8 ++++---- ...ess_than_or_equal_to_semver_matcher_spec.rb | 10 +++++----- 12 files changed, 57 insertions(+), 58 deletions(-) diff --git a/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb index 282dcf5c..8159272e 100644 --- a/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb @@ -10,23 +10,23 @@ def initialize(attribute, start_value, end_value, logger, validator) super(logger) @validator = validator @attribute = attribute - @semver_start = SplitIoClient::Semver.new(start_value) - @semver_end = SplitIoClient::Semver.new(end_value) + @semver_start = SplitIoClient::Semver.build(start_value, logger) + @semver_end = SplitIoClient::Semver.build(end_value, logger) @logger = logger end def match?(args) - @logger.log_if_debug('[BetweenSemverMatcher] evaluating value and attributes.') + @logger.debug('[BetweenSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = args[:attributes][@attribute.to_sym] - unless value_to_match.is_a?(String) - @logger.log_if_debug('betweenStringMatcherData is required for BETWEEN_SEMVER matcher type') + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + unless !value_to_match.nil? && !@semver_start.nil? && !@semver_end.nil? + @logger.error('betweenStringMatcherData is required for BETWEEN_SEMVER matcher type') return false end - matches = ([0, -1].include?(@semver_start.compare(SplitIoClient::Semver.new(value_to_match))) && - [0, 1].include?(@semver_end.compare(SplitIoClient::Semver.new(value_to_match)))) - @logger.log_if_debug("[BetweenMatcher] #{value_to_match} matches -> #{matches}") + matches = ([0, -1].include?(@semver_start.compare(value_to_match)) && + [0, 1].include?(@semver_end.compare(value_to_match))) + @logger.debug("[BetweenMatcher] #{value_to_match} matches -> #{matches}") matches end end diff --git a/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb index 877b0f82..0ffa2b2f 100644 --- a/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb @@ -10,21 +10,21 @@ def initialize(attribute, string_value, logger, validator) super(logger) @validator = validator @attribute = attribute - @semver = SplitIoClient::Semver.new(string_value) + @semver = SplitIoClient::Semver.build(string_value, logger) @logger = logger end def match?(args) - @logger.log_if_debug('[EqualsToSemverMatcher] evaluating value and attributes.') + @logger.debug('[EqualsToSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = args[:attributes][@attribute.to_sym] - unless value_to_match.is_a?(String) - @logger.log_if_debug('stringMatcherData is required for EQUAL_TO_SEMVER matcher type') + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + unless !value_to_match.nil? && !@semver.nil? + @logger.error('stringMatcherData is required for EQUAL_TO_SEMVER matcher type') return false end - matches = (@semver.version == SplitIoClient::Semver.new(value_to_match).version) - @logger.log_if_debug("[EqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") + matches = (@semver.version == value_to_match.version) + @logger.debug("[EqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") matches end end diff --git a/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb index 818962eb..cb9331c8 100644 --- a/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb @@ -10,21 +10,21 @@ def initialize(attribute, string_value, logger, validator) super(logger) @validator = validator @attribute = attribute - @semver = SplitIoClient::Semver.new(string_value) + @semver = SplitIoClient::Semver.build(string_value, logger) @logger = logger end def match?(args) - @logger.log_if_debug('[GreaterThanOrEqualsToSemverMatcher] evaluating value and attributes.') + @logger.debug('[GreaterThanOrEqualsToSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = args[:attributes][@attribute.to_sym] - unless value_to_match.is_a?(String) - @logger.log_if_debug('stringMatcherData is required for GREATER_THAN_OR_EQUAL_TO_SEMVER matcher type') + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + unless !value_to_match.nil? && !@semver.nil? + @logger.error('stringMatcherData is required for GREATER_THAN_OR_EQUAL_TO_SEMVER matcher type') return false end - matches = [0, 1].include?(@semver.compare(SplitIoClient::Semver.new(value_to_match))) - @logger.log_if_debug("[GreaterThanOrEqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") + matches = [0, 1].include?(@semver.compare(value_to_match)) + @logger.debug("[GreaterThanOrEqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") matches end end diff --git a/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb index 57b13ddd..298bc0b0 100644 --- a/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/in_list_semver_matcher.rb @@ -15,16 +15,16 @@ def initialize(attribute, list_value, logger, validator) end def match?(args) - @logger.log_if_debug('[InListSemverMatcher] evaluating value and attributes.') + @logger.debug('[InListSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) && args[:attributes][@attribute.to_sym].is_a?(String) value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) unless !value_to_match.nil? && @semver_list.all? { |n| !n.nil? } - @logger.log_if_debug('whitelistMatcherData is required for IN_LIST_SEMVER matcher type') + @logger.error('whitelistMatcherData is required for IN_LIST_SEMVER matcher type') return false end matches = (@semver_list.map { |item| item.version == value_to_match.version }).any? { |item| item == true } - @logger.log_if_debug("[InListSemverMatcher] #{value_to_match} matches -> #{matches}") + @logger.debug("[InListSemverMatcher] #{value_to_match} matches -> #{matches}") matches end end diff --git a/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb index eb95702f..ec10cb62 100644 --- a/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb @@ -10,21 +10,21 @@ def initialize(attribute, string_value, logger, validator) super(logger) @validator = validator @attribute = attribute - @semver = SplitIoClient::Semver.new(string_value) + @semver = SplitIoClient::Semver.build(string_value, logger) @logger = logger end def match?(args) - @logger.log_if_debug('[LessThanOrEqualsToSemverMatcher] evaluating value and attributes.') + @logger.debug('[LessThanOrEqualsToSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = args[:attributes][@attribute.to_sym] - unless value_to_match.is_a?(String) - @logger.log_if_debug('stringMatcherData is required for LESS_THAN_OR_EQUAL_TO_SEMVER matcher type') + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + unless !value_to_match.nil? && !@semver.nil? + @logger.error('stringMatcherData is required for LESS_THAN_OR_EQUAL_TO_SEMVER matcher type') return false end - matches = [0, -1].include?(@semver.compare(SplitIoClient::Semver.new(value_to_match))) - @logger.log_if_debug("[LessThanOrEqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") + matches = [0, -1].include?(@semver.compare(value_to_match)) + @logger.debug("[LessThanOrEqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") matches end end diff --git a/lib/splitclient-rb/engine/matchers/semver.rb b/lib/splitclient-rb/engine/matchers/semver.rb index 6669b94b..3de8d5f7 100644 --- a/lib/splitclient-rb/engine/matchers/semver.rb +++ b/lib/splitclient-rb/engine/matchers/semver.rb @@ -27,8 +27,8 @@ def initialize(version) # @return [type] Semver instance def self.build(version, logger) new(version) - rescue RuntimeError => e - logger.warn("Failed to parse Semver data: #{e}") + rescue StandardError, NoMethodError => e + logger.error("Failed to parse Semver data: #{e}") nil end diff --git a/spec/cache/repositories/splits_repository_spec.rb b/spec/cache/repositories/splits_repository_spec.rb index 6d8ae3f7..0b50df8a 100644 --- a/spec/cache/repositories/splits_repository_spec.rb +++ b/spec/cache/repositories/splits_repository_spec.rb @@ -126,7 +126,7 @@ } }] } - repository.update([split], [], -1) + repository.update([split.dup], [], -1) expect(repository.get_split('corge')[:conditions]).to eq [SplitIoClient::Cache::Repositories::SplitsRepository::DEFAULT_CONDITIONS_TEMPLATE] # test with multiple conditions diff --git a/spec/engine/matchers/matches_between_semver_matcher_spec.rb b/spec/engine/matchers/matches_between_semver_matcher_spec.rb index 12474791..b4431fe4 100644 --- a/spec/engine/matchers/matches_between_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_between_semver_matcher_spec.rb @@ -11,7 +11,7 @@ let(:config) { SplitIoClient::SplitConfig.new } it 'initilized params' do - matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.logger, config.split_validator) expect(matcher.attribute).to eq("version") semver_start = matcher.instance_variable_get(:@semver_start) expect(semver_start.instance_variable_get(:@version)).to eq("2.1.8") @@ -20,20 +20,20 @@ end it 'matches' do - matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.9"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.11-rc12"})).to eq(true) end it 'does not match' do - matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.5"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.12-rc1"})).to eq(false) end it 'invalid attribute' do - matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:betweenStringMatcherData][:start], raw[:betweenStringMatcherData][:end], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": 2.1})).to eq(false) expect(matcher.match?(:attributes=>{"version": nil})).to eq(false) end diff --git a/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb index f28d2521..6d382bb3 100644 --- a/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_equal_to_semver_matcher_spec.rb @@ -11,28 +11,27 @@ let(:config) { SplitIoClient::SplitConfig.new } it 'initilized params' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.attribute).to eq("version") semver = matcher.instance_variable_get(:@semver) expect(semver.instance_variable_get(:@version)).to eq("2.1.8") end it 'matches' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.8"})).to eq(true) end it 'does not match' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.5"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.5-rc1"})).to eq(false) end it 'invalid attribute' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": 2.1})).to eq(false) expect(matcher.match?(:attributes=>{"version": nil})).to eq(false) end - end diff --git a/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb index be31742c..db5d56cd 100644 --- a/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb @@ -11,14 +11,14 @@ let(:config) { SplitIoClient::SplitConfig.new } it 'initilized params' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.attribute).to eq("version") semver = matcher.instance_variable_get(:@semver) expect(semver.instance_variable_get(:@version)).to eq("2.1.8") end it 'matches' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.8"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.5"})).to eq(true) @@ -26,13 +26,13 @@ end it 'does not match' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.11"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.2.0"})).to eq(false) end it 'invalid attribute' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": 2.1})).to eq(false) expect(matcher.match?(:attributes=>{"version": nil})).to eq(false) end diff --git a/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb b/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb index bffe3da1..76461a65 100644 --- a/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_in_list_semver_matcher_spec.rb @@ -11,7 +11,7 @@ let(:config) { SplitIoClient::SplitConfig.new } it 'initilized params' do - matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.logger, config.split_validator) expect(matcher.attribute).to eq("version") semver_list = matcher.instance_variable_get(:@semver_list) expect(semver_list[0].instance_variable_get(:@version)).to eq("2.1.8") @@ -19,13 +19,13 @@ end it 'matches' do - matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.8"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.11"})).to eq(true) end it 'does not match' do - matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.7"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.11-rc12"})).to eq(false) @@ -33,7 +33,7 @@ end it 'invalid attribute' do - matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:whitelistMatcherData][:whitelist], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": 2.1})).to eq(false) expect(matcher.match?(:attributes=>{"version": nil})).to eq(false) end diff --git a/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb index 978ec18e..8b20d8ab 100644 --- a/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb @@ -8,17 +8,17 @@ 'matcherType': 'LESS_THAN_OR_EQUAL_TO_SEMVER', 'stringMatcherData': "2.1.8" } } - let(:config) { SplitIoClient::SplitConfig.new } + let(:config) { SplitIoClient::SplitConfig.new({:logger => Logger.new('/dev/null')}) } it 'initilized params' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.attribute).to eq("version") semver = matcher.instance_variable_get(:@semver) expect(semver.instance_variable_get(:@version)).to eq("2.1.8") end it 'matches' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.8+rc"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.8"})).to eq(true) expect(matcher.match?(:attributes=>{"version": "2.1.11"})).to eq(true) @@ -26,13 +26,13 @@ end it 'does not match' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": "2.1.5"})).to eq(false) expect(matcher.match?(:attributes=>{"version": "2.1.5-rc1"})).to eq(false) end it 'invalid attribute' do - matcher = described_class.new("version", raw[:stringMatcherData], config.split_logger, config.split_validator) + matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) expect(matcher.match?(:attributes=>{"version": 2.1})).to eq(false) expect(matcher.match?(:attributes=>{"version": nil})).to eq(false) end From 5a084a8dd0b327c3b5ed6c3c3ddddb2fb04b07bd Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 10 Apr 2024 19:19:52 -0700 Subject: [PATCH 6/8] polish --- lib/splitclient-rb/engine/matchers/between_semver_matcher.rb | 2 +- .../engine/matchers/equal_to_semver_matcher.rb | 2 +- .../matchers/greater_than_or_equal_to_semver_matcher.rb | 2 +- .../engine/matchers/less_than_or_equal_to_semver_matcher.rb | 2 +- lib/splitclient-rb/engine/matchers/semver.rb | 5 ++++- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb index 8159272e..e175c541 100644 --- a/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/between_semver_matcher.rb @@ -19,7 +19,7 @@ def match?(args) @logger.debug('[BetweenSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) unless !value_to_match.nil? && !@semver_start.nil? && !@semver_end.nil? @logger.error('betweenStringMatcherData is required for BETWEEN_SEMVER matcher type') return false diff --git a/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb index 0ffa2b2f..c77f78a1 100644 --- a/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/equal_to_semver_matcher.rb @@ -18,7 +18,7 @@ def match?(args) @logger.debug('[EqualsToSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) unless !value_to_match.nil? && !@semver.nil? @logger.error('stringMatcherData is required for EQUAL_TO_SEMVER matcher type') return false diff --git a/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb index cb9331c8..93114829 100644 --- a/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb @@ -18,7 +18,7 @@ def match?(args) @logger.debug('[GreaterThanOrEqualsToSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) unless !value_to_match.nil? && !@semver.nil? @logger.error('stringMatcherData is required for GREATER_THAN_OR_EQUAL_TO_SEMVER matcher type') return false diff --git a/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb index ec10cb62..23532044 100644 --- a/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb @@ -18,7 +18,7 @@ def match?(args) @logger.debug('[LessThanOrEqualsToSemverMatcher] evaluating value and attributes.') return false unless @validator.valid_matcher_arguments(args) - value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) + value_to_match = SplitIoClient::Semver.build(args[:attributes][@attribute.to_sym], @logger) unless !value_to_match.nil? && !@semver.nil? @logger.error('stringMatcherData is required for LESS_THAN_OR_EQUAL_TO_SEMVER matcher type') return false diff --git a/lib/splitclient-rb/engine/matchers/semver.rb b/lib/splitclient-rb/engine/matchers/semver.rb index 3de8d5f7..bb09ec0f 100644 --- a/lib/splitclient-rb/engine/matchers/semver.rb +++ b/lib/splitclient-rb/engine/matchers/semver.rb @@ -27,7 +27,10 @@ def initialize(version) # @return [type] Semver instance def self.build(version, logger) new(version) - rescue StandardError, NoMethodError => e + rescue NoMethodError => e + logger.error("Failed to parse Semver data, incorrect data type: #{e}") + nil + rescue StandardError => e logger.error("Failed to parse Semver data: #{e}") nil end From 7ec5da7bfe46f6c4e02ef2c7baa9ea4fa4feccdb Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 10 Apr 2024 20:21:38 -0700 Subject: [PATCH 7/8] added version length check for compare --- lib/splitclient-rb/engine/matchers/semver.rb | 5 ++- spec/engine/matchers/semver_spec.rb | 33 +++++++++++++++----- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/lib/splitclient-rb/engine/matchers/semver.rb b/lib/splitclient-rb/engine/matchers/semver.rb index bb09ec0f..151375fc 100644 --- a/lib/splitclient-rb/engine/matchers/semver.rb +++ b/lib/splitclient-rb/engine/matchers/semver.rb @@ -63,7 +63,10 @@ def compare(to_compare) return compare_attributes(to_compare) if compare_attributes(to_compare) != 0 # Compare pre-release versions lexically - compare_pre_release(to_compare) + return compare_pre_release(to_compare) if compare_pre_release(to_compare) != 0 + + # compare length of version + compare_vars(@version.length, to_compare.version.length) end private diff --git a/spec/engine/matchers/semver_spec.rb b/spec/engine/matchers/semver_spec.rb index b65b3304..6b3d3bef 100644 --- a/spec/engine/matchers/semver_spec.rb +++ b/spec/engine/matchers/semver_spec.rb @@ -4,15 +4,20 @@ describe SplitIoClient::Semver do let(:valid_versions) do CSV.parse(File.read(File.expand_path(File.join(File.dirname(__FILE__), - '../../test_data/splits/semver/valid-semantic-versions.csv')))) + '../../test_data/splits/semver/valid-semantic-versions.csv'))), headers: true) end let(:invalid_versions) do CSV.parse(File.read(File.expand_path(File.join(File.dirname(__FILE__), - '../../test_data/splits/semver/invalid-semantic-versions.csv')))) + '../../test_data/splits/semver/invalid-semantic-versions.csv'))), headers: true) end let(:equal_to_versions) do CSV.parse(File.read(File.expand_path(File.join(File.dirname(__FILE__), - '../../test_data/splits/semver/equal-to-semver.csv')))) + '../../test_data/splits/semver/equal-to-semver.csv'))), headers: true) + end + + let(:between_versions) do + CSV.parse(File.read(File.expand_path(File.join(File.dirname(__FILE__), + '../../test_data/splits/semver/between-semver.csv'))), headers: true) end let(:logger) { Logger.new('/dev/null') } @@ -20,7 +25,7 @@ context 'check versions' do it 'accept valid versions' do for i in (0..valid_versions.length-1) - expect(described_class.build(valid_versions[i][0], logger)).should_not be_nil + expect(described_class.build(valid_versions[i][0], logger)).not_to be_nil end end it 'reject invalid versions' do @@ -39,12 +44,26 @@ expect(described_class.build(valid_versions[i][1], logger).compare(described_class.build(valid_versions[i][1], logger))).to eq(0) end for i in (1..equal_to_versions.length-1) - if valid_versions[i][2] - expect(described_class.build(valid_versions[i][0], logger).compare(described_class.build(valid_versions[i][1], logger))).to eq(0) + if equal_to_versions[i][2]=='true' + expect(described_class.build(equal_to_versions[i][0], logger).compare(described_class.build(equal_to_versions[i][1], logger))).to eq(0) else - expect(described_class.build(valid_versions[i][0], logger).compare(described_class.build(valid_versions[i][1], logger))).not_to eq(0) + expect(described_class.build(equal_to_versions[i][0], logger).compare(described_class.build(equal_to_versions[i][1], logger))).not_to eq(0) end end + for i in (1..between_versions.length-1) + sem1 = described_class.build(between_versions[i][0], logger) + sem2 = described_class.build(between_versions[i][2], logger) + to_check = described_class.build(between_versions[i][1], logger) + if between_versions[i][3]=='true' + expect(sem1.compare(to_check)).to eq(-1) + expect(sem2.compare(to_check)).to eq(1) + else + compare1 = sem1.compare(to_check) + compare2 = sem2.compare(to_check) + expect(compare1 == -1 && compare2 == 1).to eq(false) + end + end + end end end From 4ecad51cf83dfefb26e80005689930e18812f0c9 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Thu, 11 Apr 2024 09:58:46 -0700 Subject: [PATCH 8/8] correcting specs and comparisons --- .../matchers/greater_than_or_equal_to_semver_matcher.rb | 2 +- .../engine/matchers/less_than_or_equal_to_semver_matcher.rb | 2 +- lib/splitclient-rb/engine/matchers/semver.rb | 5 +---- .../matches_greater_than_or_equal_to_semver_matcher_spec.rb | 4 ++-- .../matches_less_than_or_equal_to_semver_matcher_spec.rb | 4 ++-- spec/engine/matchers/semver_spec.rb | 4 ++-- 6 files changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb index 93114829..ffe46aef 100644 --- a/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/greater_than_or_equal_to_semver_matcher.rb @@ -23,7 +23,7 @@ def match?(args) @logger.error('stringMatcherData is required for GREATER_THAN_OR_EQUAL_TO_SEMVER matcher type') return false end - matches = [0, 1].include?(@semver.compare(value_to_match)) + matches = [0, 1].include?(value_to_match.compare(@semver)) @logger.debug("[GreaterThanOrEqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") matches end diff --git a/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb b/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb index 23532044..081a7317 100644 --- a/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb +++ b/lib/splitclient-rb/engine/matchers/less_than_or_equal_to_semver_matcher.rb @@ -23,7 +23,7 @@ def match?(args) @logger.error('stringMatcherData is required for LESS_THAN_OR_EQUAL_TO_SEMVER matcher type') return false end - matches = [0, -1].include?(@semver.compare(value_to_match)) + matches = [0, -1].include?(value_to_match.compare(@semver)) @logger.debug("[LessThanOrEqualsToSemverMatcher] #{value_to_match} matches -> #{matches}") matches end diff --git a/lib/splitclient-rb/engine/matchers/semver.rb b/lib/splitclient-rb/engine/matchers/semver.rb index 151375fc..bb09ec0f 100644 --- a/lib/splitclient-rb/engine/matchers/semver.rb +++ b/lib/splitclient-rb/engine/matchers/semver.rb @@ -63,10 +63,7 @@ def compare(to_compare) return compare_attributes(to_compare) if compare_attributes(to_compare) != 0 # Compare pre-release versions lexically - return compare_pre_release(to_compare) if compare_pre_release(to_compare) != 0 - - # compare length of version - compare_vars(@version.length, to_compare.version.length) + compare_pre_release(to_compare) end private diff --git a/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb index 11facb21..91c81dec 100644 --- a/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_greater_than_or_equal_to_semver_matcher_spec.rb @@ -27,8 +27,8 @@ it 'does not match' do matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) - expect(matcher.match?(:attributes=>{"version": "2.1.11"})).to eq(false) - expect(matcher.match?(:attributes=>{"version": "2.2.0"})).to eq(false) + expect(matcher.match?(:attributes=>{"version": "2.1.7"})).to eq(false) + expect(matcher.match?(:attributes=>{"version": "2.0.22"})).to eq(false) end it 'invalid attribute' do diff --git a/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb b/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb index 64d37c53..a551e657 100644 --- a/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb +++ b/spec/engine/matchers/matches_less_than_or_equal_to_semver_matcher_spec.rb @@ -27,8 +27,8 @@ it 'does not match' do matcher = described_class.new("version", raw[:stringMatcherData], config.logger, config.split_validator) - expect(matcher.match?(:attributes=>{"version": "2.1.5"})).to eq(false) - expect(matcher.match?(:attributes=>{"version": "2.1.5-rc1"})).to eq(false) + expect(matcher.match?(:attributes=>{"version": "2.1.10"})).to eq(false) + expect(matcher.match?(:attributes=>{"version": "2.2.0-rc1"})).to eq(false) end it 'invalid attribute' do diff --git a/spec/engine/matchers/semver_spec.rb b/spec/engine/matchers/semver_spec.rb index 6b3d3bef..a10f64bf 100644 --- a/spec/engine/matchers/semver_spec.rb +++ b/spec/engine/matchers/semver_spec.rb @@ -45,9 +45,9 @@ end for i in (1..equal_to_versions.length-1) if equal_to_versions[i][2]=='true' - expect(described_class.build(equal_to_versions[i][0], logger).compare(described_class.build(equal_to_versions[i][1], logger))).to eq(0) + expect(described_class.build(equal_to_versions[i][0], logger).version == described_class.build(equal_to_versions[i][1], logger).version).to eq(true) else - expect(described_class.build(equal_to_versions[i][0], logger).compare(described_class.build(equal_to_versions[i][1], logger))).not_to eq(0) + expect(described_class.build(equal_to_versions[i][0], logger) == described_class.build(equal_to_versions[i][1], logger)).to eq(false) end end for i in (1..between_versions.length-1)