From b01f853cd58dad638d590c92bbb95588ab3045f3 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 8 Dec 2017 15:01:45 -0800 Subject: [PATCH 01/16] add semantic version operators --- ldclient-rb.gemspec | 3 +- lib/ldclient-rb/evaluation.rb | 73 +++++++++++------- spec/evaluation_spec.rb | 135 ++++++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 27 deletions(-) create mode 100644 spec/evaluation_spec.rb diff --git a/ldclient-rb.gemspec b/ldclient-rb.gemspec index c470ca04..92135ccd 100644 --- a/ldclient-rb.gemspec +++ b/ldclient-rb.gemspec @@ -28,7 +28,8 @@ Gem::Specification.new do |spec| spec.add_development_dependency "redis", "~> 3.3.5" spec.add_development_dependency "connection_pool", ">= 2.1.2" spec.add_development_dependency "moneta", "~> 1.0.0" - + spec.add_development_dependency "sem_version", "~> 2.0.1" + spec.add_runtime_dependency "json", "~> 1.8" spec.add_runtime_dependency "faraday", "~> 0.9" spec.add_runtime_dependency "faraday-http-cache", "~> 1.3.0" diff --git a/lib/ldclient-rb/evaluation.rb b/lib/ldclient-rb/evaluation.rb index 4a6c942f..d774a8d2 100644 --- a/lib/ldclient-rb/evaluation.rb +++ b/lib/ldclient-rb/evaluation.rb @@ -1,9 +1,44 @@ require "date" +require "sem_version" module LaunchDarkly module Evaluation BUILTINS = [:key, :ip, :country, :email, :firstName, :lastName, :avatar, :name, :anonymous] + DATE_OPERAND = lambda do |v| + if v.is_a? String + begin + DateTime.rfc3339(v).strftime("%Q").to_i + rescue => e + nil + end + elsif v.is_a? Numeric + v + else + nil + end + end + + SEMVER_OPERAND = lambda do |v| + if v.is_a? String + begin + SemVersion.from_loose_version(v) + rescue ArgumentError + nil + end + else + nil + end + end + + def self.comparator(converter, condition) + lambda do |a, b| + av = converter.call(a) + bv = converter.call(b) + !av.nil? && !bv.nil? && condition.call(av <=> bv) + end + end + OPERATORS = { in: lambda do |a, b| @@ -42,33 +77,15 @@ module Evaluation (a.is_a? Numeric) && (a >= b) end, before: - lambda do |a, b| - begin - if a.is_a? String - a = DateTime.rfc3339(a).strftime('%Q').to_i - end - if b.is_a? String - b = DateTime.rfc3339(b).strftime('%Q').to_i - end - (a.is_a? Numeric) ? a < b : false - rescue => e - false - end - end, + comparator(DATE_OPERAND, -> n { n < 0 }), after: - lambda do |a, b| - begin - if a.is_a? String - a = DateTime.rfc3339(a).strftime("%Q").to_i - end - if b.is_a? String - b = DateTime.rfc3339(b).strftime("%Q").to_i - end - (a.is_a? Numeric) ? a > b : false - rescue => e - false - end - end + comparator(DATE_OPERAND, -> n { n > 0 }), + semVerEqual: + comparator(SEMVER_OPERAND, -> n { n == 0 }), + semVerLessThan: + comparator(SEMVER_OPERAND, -> n { n < 0 }), + semVerGreaterThan: + comparator(SEMVER_OPERAND, -> n { n > 0 }) } class EvaluationError < StandardError @@ -257,5 +274,9 @@ def match_any(op, value, values) end return false end + + def operator(op) + OPERATORS[op.to_sym] + end end end diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb new file mode 100644 index 00000000..6a918c9c --- /dev/null +++ b/spec/evaluation_spec.rb @@ -0,0 +1,135 @@ +require "spec_helper" + +describe LaunchDarkly::Evaluation do + subject { LaunchDarkly::Evaluation } + + include LaunchDarkly::Evaluation + + describe "clause_match_user" do + it "can match built-in attribute" do + user = { key: 'x', name: 'Bob' } + clause = { attribute: 'name', op: 'in', values: ['Bob'] } + expect(clause_match_user(clause, user)).to be true + end + + it "can match custom attribute" do + user = { key: 'x', name: 'Bob', custom: { legs: 4 } } + clause = { attribute: 'legs', op: 'in', values: [4] } + expect(clause_match_user(clause, user)).to be true + end + + it "returns false for missing attribute" do + user = { key: 'x', name: 'Bob' } + clause = { attribute: 'legs', op: 'in', values: [4] } + expect(clause_match_user(clause, user)).to be false + end + end + + describe "operators" do + n99 = 99 + n99_0001 = 99.0001 + sX = "x" + sY = "y" + sZ = "z" + sXyz = "xyz" + s99 = "99" + sHelloWorld = "hello world" + dateStr1 = "2017-12-06T00:00:00.000-07:00" + dateStr2 = "2017-12-06T00:01:01.000-07:00" + dateMs1 = 10000000 + dateMs2 = 10000001 + invalidDate = "hey what's this?" + v2_0 = "2.0" + v2_0_0 = "2.0.0" + v2_0_1 = "2.0.1" + vInvalid = "xbad%ver" + + operatorTests = [ + # numeric comparisons + [ :in, n99, n99, true ], + [ :in, n99_0001, n99_0001, true ], + [ :in, n99, n99_0001, false ], + [ :in, n99_0001, n99, false ], + [ :lessThan, n99, n99_0001, true ], + [ :lessThan, n99_0001, n99, false ], + [ :lessThan, n99, n99, false ], + [ :lessThanOrEqual, n99, n99_0001, true ], + [ :lessThanOrEqual, n99_0001, n99, false ], + [ :lessThanOrEqual, n99, n99, true ], + [ :greaterThan, n99_0001, n99, true ], + [ :greaterThan, n99, n99_0001, false ], + [ :greaterThan, n99, n99, false ], + [ :greaterThanOrEqual, n99_0001, n99, true ], + [ :greaterThanOrEqual, n99, n99_0001, false ], + [ :greaterThanOrEqual, n99, n99, true ], + + # string comparisons + [ :in, sX, sX, true ], + [ :in, sX, sXyz, false ], + [ :startsWith, sXyz, sX, true ], + [ :startsWith, sX, sXyz, false ], + [ :endsWith, sXyz, sZ, true ], + [ :endsWith, sZ, sXyz, false ], + [ :contains, sXyz, sY, true ], + [ :contains, sY, sXyz, false ], + + # mixed strings and numbers + [ :in, s99, n99, false ], + [ :in, n99, s99, false ], + #[ :contains, s99, n99, false ], # currently throws exception + #[ :startsWith, s99, n99, false ], # currently throws exception + #[ :endsWith, s99, n99, false ] # currently throws exception + [ :lessThanOrEqual, s99, n99, false ], + #[ :lessThanOrEqual, n99, s99, false ], # currently throws exception + [ :greaterThanOrEqual, s99, n99, false ], + #[ :greaterThanOrEqual, n99, s99, false ], # currently throws exception + + # regex + [ :matches, sHelloWorld, "hello.*rld", true ], + [ :matches, sHelloWorld, "hello.*orl", true ], + [ :matches, sHelloWorld, "l+", true ], + [ :matches, sHelloWorld, "(world|planet)", true ], + [ :matches, sHelloWorld, "aloha", false ], + #[ :matches, sHelloWorld, new JsonPrimitive("***not a regex"), false ] # currently throws exception + + # dates + [ :before, dateStr1, dateStr2, true ], + [ :before, dateMs1, dateMs2, true ], + [ :before, dateStr2, dateStr1, false ], + [ :before, dateMs2, dateMs1, false ], + [ :before, dateStr1, dateStr1, false ], + [ :before, dateMs1, dateMs1, false ], + [ :before, dateStr1, invalidDate, false ], + [ :after, dateStr1, dateStr2, false ], + [ :after, dateMs1, dateMs2, false ], + [ :after, dateStr2, dateStr1, true ], + [ :after, dateMs2, dateMs1, true ], + [ :after, dateStr1, dateStr1, false ], + [ :after, dateMs1, dateMs1, false ], + [ :after, dateStr1, invalidDate, false ], + + # semver + [ :semVerEqual, v2_0_1, v2_0_1, true ], + [ :semVerLessThan, v2_0_0, v2_0_1, true ], + [ :semVerLessThan, v2_0, v2_0_1, true ], + [ :semVerLessThan, v2_0_1, v2_0_0, false ], + [ :semVerLessThan, v2_0_1, v2_0, false ], + [ :semVerGreaterThan, v2_0_1, v2_0_0, true ], + [ :semVerGreaterThan, v2_0_1, v2_0, true ], + [ :semVerGreaterThan, v2_0_0, v2_0_1, false ], + [ :semVerGreaterThan, v2_0, v2_0_1, false ], + [ :semVerLessThan, v2_0_1, vInvalid, false ], + [ :semVerGreaterThan, v2_0_1, vInvalid, false ] + ] + + operatorTests.each do |params| + op = params[0] + value1 = params[1] + value2 = params[2] + shouldBe = params[3] + it "should return #{shouldBe} for #{value1} #{op} #{value2}" do + expect(operator(op).call(value1, value2)).to be shouldBe + end + end + end +end From 4f85d6ed90e09b9daab7be0d8ac5ec55b6a0c11c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 11 Dec 2017 13:09:33 -0800 Subject: [PATCH 02/16] 2.0 and 2.0.0 should be equal --- spec/evaluation_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index 6a918c9c..41c3e0c3 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -110,6 +110,9 @@ # semver [ :semVerEqual, v2_0_1, v2_0_1, true ], + [ :semVerEqual, v2_0, v2_0_0, true ], + # Note on the above: the sem_version library returns exactly the same object for "2.0" and "2.0.0", + # which happens to be the behavior we want. [ :semVerLessThan, v2_0_0, v2_0_1, true ], [ :semVerLessThan, v2_0, v2_0_1, true ], [ :semVerLessThan, v2_0_1, v2_0_0, false ], From 92fc954896bf6ec0ed5043e15e229fd92b16f03d Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 11 Dec 2017 13:11:27 -0800 Subject: [PATCH 03/16] don't expose internal map for testing --- lib/ldclient-rb/evaluation.rb | 4 ---- spec/evaluation_spec.rb | 4 +++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/ldclient-rb/evaluation.rb b/lib/ldclient-rb/evaluation.rb index d774a8d2..81c18699 100644 --- a/lib/ldclient-rb/evaluation.rb +++ b/lib/ldclient-rb/evaluation.rb @@ -274,9 +274,5 @@ def match_any(op, value, values) end return false end - - def operator(op) - OPERATORS[op.to_sym] - end end end diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index 41c3e0c3..7ba6544b 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -131,7 +131,9 @@ value2 = params[2] shouldBe = params[3] it "should return #{shouldBe} for #{value1} #{op} #{value2}" do - expect(operator(op).call(value1, value2)).to be shouldBe + user = { key: 'x', custom: { foo: value1 } } + clause = { attribute: 'foo', op: op, values: [value2] } + expect(clause_match_user(clause, user)).to be shouldBe end end end From b6f67e73ec2df7d5bad8333be40257bae0c9a98c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 11 Dec 2017 13:16:28 -0800 Subject: [PATCH 04/16] clarify comments --- spec/evaluation_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index 7ba6544b..cb9efeb8 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -76,13 +76,13 @@ # mixed strings and numbers [ :in, s99, n99, false ], [ :in, n99, s99, false ], - #[ :contains, s99, n99, false ], # currently throws exception - #[ :startsWith, s99, n99, false ], # currently throws exception - #[ :endsWith, s99, n99, false ] # currently throws exception + #[ :contains, s99, n99, false ], # currently throws exception - would return false in Java SDK + #[ :startsWith, s99, n99, false ], # currently throws exception - would return false in Java SDK + #[ :endsWith, s99, n99, false ] # currently throws exception - would return false in Java SDK [ :lessThanOrEqual, s99, n99, false ], - #[ :lessThanOrEqual, n99, s99, false ], # currently throws exception + #[ :lessThanOrEqual, n99, s99, false ], # currently throws exception - would return false in Java SDK [ :greaterThanOrEqual, s99, n99, false ], - #[ :greaterThanOrEqual, n99, s99, false ], # currently throws exception + #[ :greaterThanOrEqual, n99, s99, false ], # currently throws exception - would return false in Java SDK # regex [ :matches, sHelloWorld, "hello.*rld", true ], @@ -90,7 +90,7 @@ [ :matches, sHelloWorld, "l+", true ], [ :matches, sHelloWorld, "(world|planet)", true ], [ :matches, sHelloWorld, "aloha", false ], - #[ :matches, sHelloWorld, new JsonPrimitive("***not a regex"), false ] # currently throws exception + #[ :matches, sHelloWorld, new JsonPrimitive("***not a regex"), false ] # currently throws exception - same as Java SDK # dates [ :before, dateStr1, dateStr2, true ], From 87c025a31726774874fc5a53ef3f2d765e4c96e1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 11 Dec 2017 15:17:06 -0800 Subject: [PATCH 05/16] misc test cleanup --- spec/evaluation_spec.rb | 140 ++++++++++++++++++---------------------- 1 file changed, 64 insertions(+), 76 deletions(-) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index cb9efeb8..14c84798 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -26,103 +26,91 @@ end describe "operators" do - n99 = 99 - n99_0001 = 99.0001 - sX = "x" - sY = "y" - sZ = "z" - sXyz = "xyz" - s99 = "99" - sHelloWorld = "hello world" dateStr1 = "2017-12-06T00:00:00.000-07:00" dateStr2 = "2017-12-06T00:01:01.000-07:00" dateMs1 = 10000000 dateMs2 = 10000001 invalidDate = "hey what's this?" - v2_0 = "2.0" - v2_0_0 = "2.0.0" - v2_0_1 = "2.0.1" - vInvalid = "xbad%ver" operatorTests = [ # numeric comparisons - [ :in, n99, n99, true ], - [ :in, n99_0001, n99_0001, true ], - [ :in, n99, n99_0001, false ], - [ :in, n99_0001, n99, false ], - [ :lessThan, n99, n99_0001, true ], - [ :lessThan, n99_0001, n99, false ], - [ :lessThan, n99, n99, false ], - [ :lessThanOrEqual, n99, n99_0001, true ], - [ :lessThanOrEqual, n99_0001, n99, false ], - [ :lessThanOrEqual, n99, n99, true ], - [ :greaterThan, n99_0001, n99, true ], - [ :greaterThan, n99, n99_0001, false ], - [ :greaterThan, n99, n99, false ], - [ :greaterThanOrEqual, n99_0001, n99, true ], - [ :greaterThanOrEqual, n99, n99_0001, false ], - [ :greaterThanOrEqual, n99, n99, true ], + [ :in, 99, 99, true ], + [ :in, 99.0001, 99.0001, true ], + [ :in, 99, 99.0001, false ], + [ :in, 99.0001, 99, false ], + [ :lessThan, 99, 99.0001, true ], + [ :lessThan, 99.0001, 99, false ], + [ :lessThan, 99, 99, false ], + [ :lessThanOrEqual, 99, 99.0001, true ], + [ :lessThanOrEqual, 99.0001, 99, false ], + [ :lessThanOrEqual, 99, 99, true ], + [ :greaterThan, 99.0001, 99, true ], + [ :greaterThan, 99, 99.0001, false ], + [ :greaterThan, 99, 99, false ], + [ :greaterThanOrEqual, 99.0001, 99, true ], + [ :greaterThanOrEqual, 99, 99.0001, false ], + [ :greaterThanOrEqual, 99, 99, true ], # string comparisons - [ :in, sX, sX, true ], - [ :in, sX, sXyz, false ], - [ :startsWith, sXyz, sX, true ], - [ :startsWith, sX, sXyz, false ], - [ :endsWith, sXyz, sZ, true ], - [ :endsWith, sZ, sXyz, false ], - [ :contains, sXyz, sY, true ], - [ :contains, sY, sXyz, false ], + [ :in, "x", "x", true ], + [ :in, "x", "xyz", false ], + [ :startsWith, "xyz", "x", true ], + [ :startsWith, "x", "xyz", false ], + [ :endsWith, "xyz", "z", true ], + [ :endsWith, "z", "xyz", false ], + [ :contains, "xyz", "y", true ], + [ :contains, "y", "xyz", false ], # mixed strings and numbers - [ :in, s99, n99, false ], - [ :in, n99, s99, false ], - #[ :contains, s99, n99, false ], # currently throws exception - would return false in Java SDK - #[ :startsWith, s99, n99, false ], # currently throws exception - would return false in Java SDK - #[ :endsWith, s99, n99, false ] # currently throws exception - would return false in Java SDK - [ :lessThanOrEqual, s99, n99, false ], - #[ :lessThanOrEqual, n99, s99, false ], # currently throws exception - would return false in Java SDK - [ :greaterThanOrEqual, s99, n99, false ], - #[ :greaterThanOrEqual, n99, s99, false ], # currently throws exception - would return false in Java SDK + [ :in, "99", 99, false ], + [ :in, 99, "99", false ], + #[ :contains, "99", 99, false ], # currently throws exception - would return false in Java SDK + #[ :startsWith, "99", 99, false ], # currently throws exception - would return false in Java SDK + #[ :endsWith, "99", 99, false ] # currently throws exception - would return false in Java SDK + [ :lessThanOrEqual, "99", 99, false ], + #[ :lessThanOrEqual, 99, "99", false ], # currently throws exception - would return false in Java SDK + [ :greaterThanOrEqual, "99", 99, false ], + #[ :greaterThanOrEqual, 99, "99", false ], # currently throws exception - would return false in Java SDK # regex - [ :matches, sHelloWorld, "hello.*rld", true ], - [ :matches, sHelloWorld, "hello.*orl", true ], - [ :matches, sHelloWorld, "l+", true ], - [ :matches, sHelloWorld, "(world|planet)", true ], - [ :matches, sHelloWorld, "aloha", false ], - #[ :matches, sHelloWorld, new JsonPrimitive("***not a regex"), false ] # currently throws exception - same as Java SDK + [ :matches, "hello world", "hello.*rld", true ], + [ :matches, "hello world", "hello.*orl", true ], + [ :matches, "hello world", "l+", true ], + [ :matches, "hello world", "(world|planet)", true ], + [ :matches, "hello world", "aloha", false ], + #[ :matches, "hello world", "***not a regex", false ] # currently throws exception - same as Java SDK # dates - [ :before, dateStr1, dateStr2, true ], - [ :before, dateMs1, dateMs2, true ], - [ :before, dateStr2, dateStr1, false ], - [ :before, dateMs2, dateMs1, false ], - [ :before, dateStr1, dateStr1, false ], - [ :before, dateMs1, dateMs1, false ], + [ :before, dateStr1, dateStr2, true ], + [ :before, dateMs1, dateMs2, true ], + [ :before, dateStr2, dateStr1, false ], + [ :before, dateMs2, dateMs1, false ], + [ :before, dateStr1, dateStr1, false ], + [ :before, dateMs1, dateMs1, false ], [ :before, dateStr1, invalidDate, false ], - [ :after, dateStr1, dateStr2, false ], - [ :after, dateMs1, dateMs2, false ], - [ :after, dateStr2, dateStr1, true ], - [ :after, dateMs2, dateMs1, true ], - [ :after, dateStr1, dateStr1, false ], - [ :after, dateMs1, dateMs1, false ], - [ :after, dateStr1, invalidDate, false ], + [ :after, dateStr1, dateStr2, false ], + [ :after, dateMs1, dateMs2, false ], + [ :after, dateStr2, dateStr1, true ], + [ :after, dateMs2, dateMs1, true ], + [ :after, dateStr1, dateStr1, false ], + [ :after, dateMs1, dateMs1, false ], + [ :after, dateStr1, invalidDate, false ], # semver - [ :semVerEqual, v2_0_1, v2_0_1, true ], - [ :semVerEqual, v2_0, v2_0_0, true ], + [ :semVerEqual, "2.0.1", "2.0.1", true ], + [ :semVerEqual, "2.0", "2.0.0", true ], # Note on the above: the sem_version library returns exactly the same object for "2.0" and "2.0.0", # which happens to be the behavior we want. - [ :semVerLessThan, v2_0_0, v2_0_1, true ], - [ :semVerLessThan, v2_0, v2_0_1, true ], - [ :semVerLessThan, v2_0_1, v2_0_0, false ], - [ :semVerLessThan, v2_0_1, v2_0, false ], - [ :semVerGreaterThan, v2_0_1, v2_0_0, true ], - [ :semVerGreaterThan, v2_0_1, v2_0, true ], - [ :semVerGreaterThan, v2_0_0, v2_0_1, false ], - [ :semVerGreaterThan, v2_0, v2_0_1, false ], - [ :semVerLessThan, v2_0_1, vInvalid, false ], - [ :semVerGreaterThan, v2_0_1, vInvalid, false ] + [ :semVerLessThan, "2.0.0", "2.0.1", true ], + [ :semVerLessThan, "2.0", "2.0.1", true ], + [ :semVerLessThan, "2.0.1", "2.0.0", false ], + [ :semVerLessThan, "2.0.1", "2.0", false ], + [ :semVerGreaterThan, "2.0.1", "2.0.0", true ], + [ :semVerGreaterThan, "2.0.1", "2.0", true ], + [ :semVerGreaterThan, "2.0.0", "2.0.1", false ], + [ :semVerGreaterThan, "2.0", "2.0.1", false ], + [ :semVerLessThan, "2.0.1", "xbad%ver", false ], + [ :semVerGreaterThan, "2.0.1", "xbad%ver", false ] ] operatorTests.each do |params| From 9d2fe2293c09b8119abd5021185b871794816fe0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 19 Dec 2017 10:14:49 -0800 Subject: [PATCH 06/16] fix dependency --- ldclient-rb.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient-rb.gemspec b/ldclient-rb.gemspec index 92135ccd..9f82d79f 100644 --- a/ldclient-rb.gemspec +++ b/ldclient-rb.gemspec @@ -28,11 +28,11 @@ Gem::Specification.new do |spec| spec.add_development_dependency "redis", "~> 3.3.5" spec.add_development_dependency "connection_pool", ">= 2.1.2" spec.add_development_dependency "moneta", "~> 1.0.0" - spec.add_development_dependency "sem_version", "~> 2.0.1" spec.add_runtime_dependency "json", "~> 1.8" spec.add_runtime_dependency "faraday", "~> 0.9" spec.add_runtime_dependency "faraday-http-cache", "~> 1.3.0" + spec.add_runtime_dependency "sem_version", "~> 2.0.1" spec.add_runtime_dependency "thread_safe", "~> 0.3" spec.add_runtime_dependency "net-http-persistent", "~> 2.9" spec.add_runtime_dependency "concurrent-ruby", "~> 1.0.4" From 179eb3a5c24e452345b009d9dad24c40381f4f10 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 12 Jan 2018 13:49:02 -0800 Subject: [PATCH 07/16] switch to "semantic" library and implement logic for missing minor/patch versions --- ldclient-rb.gemspec | 2 +- lib/ldclient-rb/evaluation.rb | 31 ++++++++++++++++++++++++++----- spec/evaluation_spec.rb | 3 +-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/ldclient-rb.gemspec b/ldclient-rb.gemspec index 9f82d79f..9a7a2301 100644 --- a/ldclient-rb.gemspec +++ b/ldclient-rb.gemspec @@ -32,7 +32,7 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency "json", "~> 1.8" spec.add_runtime_dependency "faraday", "~> 0.9" spec.add_runtime_dependency "faraday-http-cache", "~> 1.3.0" - spec.add_runtime_dependency "sem_version", "~> 2.0.1" + spec.add_runtime_dependency "semantic", "~> 1.6.0" spec.add_runtime_dependency "thread_safe", "~> 0.3" spec.add_runtime_dependency "net-http-persistent", "~> 2.9" spec.add_runtime_dependency "concurrent-ruby", "~> 1.0.4" diff --git a/lib/ldclient-rb/evaluation.rb b/lib/ldclient-rb/evaluation.rb index 81c18699..ade9da1c 100644 --- a/lib/ldclient-rb/evaluation.rb +++ b/lib/ldclient-rb/evaluation.rb @@ -1,10 +1,12 @@ require "date" -require "sem_version" +require "semantic" module LaunchDarkly module Evaluation BUILTINS = [:key, :ip, :country, :email, :firstName, :lastName, :avatar, :name, :anonymous] + NUMERIC_VERSION_COMPONENTS_REGEX = Regexp.new("^[0-9.]*") + DATE_OPERAND = lambda do |v| if v.is_a? String begin @@ -21,16 +23,35 @@ module Evaluation SEMVER_OPERAND = lambda do |v| if v.is_a? String - begin - SemVersion.from_loose_version(v) - rescue ArgumentError - nil + ret = tryParseSemver(v) + if ret.nil? + v = addZeroVersionComponent(v) + ret = tryParseSemver(v) + if ret.nil? + v = addZeroVersionComponent(v) + ret = tryParseSemver(v) + end end + ret else nil end end + def self.tryParseSemver(v) + begin + Semantic::Version.new(v) + rescue ArgumentError + nil + end + end + + def self.addZeroVersionComponent(v) + NUMERIC_VERSION_COMPONENTS_REGEX.match(v) { |m| + m[0] + ".0" + v[m[0].length..-1] + } + end + def self.comparator(converter, condition) lambda do |a, b| av = converter.call(a) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index 14c84798..bb3b5b3f 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -99,8 +99,7 @@ # semver [ :semVerEqual, "2.0.1", "2.0.1", true ], [ :semVerEqual, "2.0", "2.0.0", true ], - # Note on the above: the sem_version library returns exactly the same object for "2.0" and "2.0.0", - # which happens to be the behavior we want. + [ :semVerEqual, "2-rc1", "2.0.0-rc1", true ], [ :semVerLessThan, "2.0.0", "2.0.1", true ], [ :semVerLessThan, "2.0", "2.0.1", true ], [ :semVerLessThan, "2.0.1", "2.0.0", false ], From eb43baf68d9b54fda89286f25e07b94de59fd350 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 12 Jan 2018 15:27:12 -0800 Subject: [PATCH 08/16] tests for comparison of prerelease/build specifiers --- spec/evaluation_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index bb3b5b3f..10caf539 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -100,14 +100,17 @@ [ :semVerEqual, "2.0.1", "2.0.1", true ], [ :semVerEqual, "2.0", "2.0.0", true ], [ :semVerEqual, "2-rc1", "2.0.0-rc1", true ], + [ :semVerEqual, "2+build2", "2.0.0+build2", true ], [ :semVerLessThan, "2.0.0", "2.0.1", true ], [ :semVerLessThan, "2.0", "2.0.1", true ], [ :semVerLessThan, "2.0.1", "2.0.0", false ], [ :semVerLessThan, "2.0.1", "2.0", false ], + [ :semVerLessThan, "2.0.0-rc", "2.0.0-rc.beta", true ], [ :semVerGreaterThan, "2.0.1", "2.0.0", true ], [ :semVerGreaterThan, "2.0.1", "2.0", true ], [ :semVerGreaterThan, "2.0.0", "2.0.1", false ], [ :semVerGreaterThan, "2.0", "2.0.1", false ], + [ :semVerGreaterThan, "2.0.0-rc.1", "2.0.0-rc.0", true ], [ :semVerLessThan, "2.0.1", "xbad%ver", false ], [ :semVerGreaterThan, "2.0.1", "xbad%ver", false ] ] From cf878a1d3d1b1896d75fa54c19fe9241388b86ee Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 18 Jan 2018 16:55:45 -0800 Subject: [PATCH 09/16] allow bucketing by int attribute (using its string representation) --- lib/ldclient-rb/evaluation.rb | 11 ++++++++- spec/evaluation_spec.rb | 45 +++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 spec/evaluation_spec.rb diff --git a/lib/ldclient-rb/evaluation.rb b/lib/ldclient-rb/evaluation.rb index 4a6c942f..ae77605d 100644 --- a/lib/ldclient-rb/evaluation.rb +++ b/lib/ldclient-rb/evaluation.rb @@ -223,7 +223,10 @@ def variation_for_user(rule, user, flag) def bucket_user(user, key, bucket_by, salt) return nil unless user[:key] - id_hash = user_value(user, bucket_by) + id_hash = bucketable_string_value(user_value(user, bucket_by)) + if id_hash.nil? + return 0.0 + end if user[:secondary] id_hash += "." + user[:secondary] @@ -235,6 +238,12 @@ def bucket_user(user, key, bucket_by, salt) hash_val.to_i(16) / Float(0xFFFFFFFFFFFFFFF) end + def bucketable_string_value(value) + return value if value.is_a? String + return value.to_s if value.is_a? Integer + nil + end + def user_value(user, attribute) attribute = attribute.to_sym diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb new file mode 100644 index 00000000..4af0cb48 --- /dev/null +++ b/spec/evaluation_spec.rb @@ -0,0 +1,45 @@ +require "spec_helper" + +describe LaunchDarkly::Evaluation do + subject { LaunchDarkly::Evaluation } + + include LaunchDarkly::Evaluation + + describe "bucket_user" do + it "can bucket by int value (equivalent to string)" do + user = { + key: "userkey", + custom: { + stringAttr: "33333", + intAttr: 33333 + } + } + stringResult = bucket_user(user, "key", "stringAttr", "salt") + intResult = bucket_user(user, "key", "intAttr", "salt") + expect(intResult).to eq(stringResult) + end + + it "cannot bucket by float value" do + user = { + key: "userkey", + custom: { + floatAttr: 33.5 + } + } + result = bucket_user(user, "key", "floatAttr", "salt") + expect(result).to eq(0.0) + end + + + it "cannot bucket by bool value" do + user = { + key: "userkey", + custom: { + boolAttr: true + } + } + result = bucket_user(user, "key", "boolAttr", "salt") + expect(result).to eq(0.0) + end + end +end From 9fbb580ef753b3de88c058657545aac84d5e2e38 Mon Sep 17 00:00:00 2001 From: Andrew Shannon Brown Date: Tue, 23 Jan 2018 17:35:24 -0800 Subject: [PATCH 10/16] Remove accidental duplicate dependencies --- ldclient-rb.gemspec | 3 --- 1 file changed, 3 deletions(-) diff --git a/ldclient-rb.gemspec b/ldclient-rb.gemspec index b1b58314..1a89c256 100644 --- a/ldclient-rb.gemspec +++ b/ldclient-rb.gemspec @@ -29,9 +29,6 @@ Gem::Specification.new do |spec| spec.add_development_dependency "connection_pool", ">= 2.1.2" spec.add_development_dependency "moneta", "~> 1.0.0" - spec.add_runtime_dependency "json", "~> 1.8" - spec.add_runtime_dependency "faraday", "~> 0.9" - spec.add_runtime_dependency "faraday-http-cache", "~> 1.3.0" spec.add_runtime_dependency "json", [">= 1.8", "< 3"] spec.add_runtime_dependency "faraday", [">= 0.9", "< 2"] spec.add_runtime_dependency "faraday-http-cache", [">= 1.3.0", "< 3"] From efbd4c24821d72891b23fb59ddba237603c60943 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 24 Jan 2018 08:57:30 -0800 Subject: [PATCH 11/16] replace a lambda with a yield --- lib/ldclient-rb/evaluation.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/ldclient-rb/evaluation.rb b/lib/ldclient-rb/evaluation.rb index ade9da1c..41f71ae8 100644 --- a/lib/ldclient-rb/evaluation.rb +++ b/lib/ldclient-rb/evaluation.rb @@ -52,11 +52,15 @@ def self.addZeroVersionComponent(v) } end - def self.comparator(converter, condition) + def self.comparator(converter) lambda do |a, b| av = converter.call(a) bv = converter.call(b) - !av.nil? && !bv.nil? && condition.call(av <=> bv) + if !av.nil? && !bv.nil? + yield av <=> bv + else + return false + end end end @@ -98,15 +102,15 @@ def self.comparator(converter, condition) (a.is_a? Numeric) && (a >= b) end, before: - comparator(DATE_OPERAND, -> n { n < 0 }), + comparator(DATE_OPERAND) { |n| n < 0 }, after: - comparator(DATE_OPERAND, -> n { n > 0 }), + comparator(DATE_OPERAND) { |n| n > 0 }, semVerEqual: - comparator(SEMVER_OPERAND, -> n { n == 0 }), + comparator(SEMVER_OPERAND) { |n| n == 0 }, semVerLessThan: - comparator(SEMVER_OPERAND, -> n { n < 0 }), + comparator(SEMVER_OPERAND) { |n| n < 0 }, semVerGreaterThan: - comparator(SEMVER_OPERAND, -> n { n > 0 }) + comparator(SEMVER_OPERAND) { |n| n > 0 } } class EvaluationError < StandardError From 495c5fe9a9f4e765cd0868c296634731b5e500be Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 24 Jan 2018 09:12:56 -0800 Subject: [PATCH 12/16] misc cleanup --- lib/ldclient-rb/evaluation.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/ldclient-rb/evaluation.rb b/lib/ldclient-rb/evaluation.rb index 41f71ae8..8b2e4779 100644 --- a/lib/ldclient-rb/evaluation.rb +++ b/lib/ldclient-rb/evaluation.rb @@ -23,19 +23,15 @@ module Evaluation SEMVER_OPERAND = lambda do |v| if v.is_a? String - ret = tryParseSemver(v) - if ret.nil? - v = addZeroVersionComponent(v) - ret = tryParseSemver(v) - if ret.nil? + for _ in 0..2 do + begin + return Semantic::Version.new(v) + rescue ArgumentError v = addZeroVersionComponent(v) - ret = tryParseSemver(v) end end - ret - else - nil end + nil end def self.tryParseSemver(v) From 71a8c7a4a49b836ab7636a188822cac529eccad5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 24 Jan 2018 09:13:09 -0800 Subject: [PATCH 13/16] misc cleanup --- lib/ldclient-rb/evaluation.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/ldclient-rb/evaluation.rb b/lib/ldclient-rb/evaluation.rb index 8b2e4779..917fb479 100644 --- a/lib/ldclient-rb/evaluation.rb +++ b/lib/ldclient-rb/evaluation.rb @@ -34,14 +34,6 @@ module Evaluation nil end - def self.tryParseSemver(v) - begin - Semantic::Version.new(v) - rescue ArgumentError - nil - end - end - def self.addZeroVersionComponent(v) NUMERIC_VERSION_COMPONENTS_REGEX.match(v) { |m| m[0] + ".0" + v[m[0].length..-1] From acdfa5246e75d321c124204837c65b23e2aab5b1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Feb 2018 14:47:12 -0800 Subject: [PATCH 14/16] minor test improvements --- spec/evaluation_spec.rb | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index 145c48f4..28fb02fe 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -129,6 +129,20 @@ end describe "bucket_user" do + it "gets expected bucket values for specific keys" do + user = { key: "userKeyA" } + bucket = bucket_user(user, "hashKey", "key", "saltyA") + expect(bucket).to be_within(0.0000001).of(0.42157587); + + user = { key: "userKeyB" } + bucket = bucket_user(user, "hashKey", "key", "saltyA") + expect(bucket).to be_within(0.0000001).of(0.6708485); + + user = { key: "userKeyC" } + bucket = bucket_user(user, "hashKey", "key", "saltyA") + expect(bucket).to be_within(0.0000001).of(0.10343106); + end + it "can bucket by int value (equivalent to string)" do user = { key: "userkey", @@ -137,8 +151,10 @@ intAttr: 33333 } } - stringResult = bucket_user(user, "key", "stringAttr", "salt") - intResult = bucket_user(user, "key", "intAttr", "salt") + stringResult = bucket_user(user, "hashKey", "stringAttr", "saltyA") + intResult = bucket_user(user, "hashKey", "intAttr", "saltyA") + + expect(intResult).to be_within(0.0000001).of(0.54771423) expect(intResult).to eq(stringResult) end @@ -149,7 +165,7 @@ floatAttr: 33.5 } } - result = bucket_user(user, "key", "floatAttr", "salt") + result = bucket_user(user, "hashKey", "floatAttr", "saltyA") expect(result).to eq(0.0) end @@ -161,7 +177,7 @@ boolAttr: true } } - result = bucket_user(user, "key", "boolAttr", "salt") + result = bucket_user(user, "hashKey", "boolAttr", "saltyA") expect(result).to eq(0.0) end end From e8e5941abdb2f0bdf49d726cd942c7e598ce974d Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 6 Feb 2018 14:25:02 -0800 Subject: [PATCH 15/16] more unit tests for flag evals --- spec/evaluation_spec.rb | 130 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index 28fb02fe..31346630 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -2,9 +2,133 @@ describe LaunchDarkly::Evaluation do subject { LaunchDarkly::Evaluation } + let(:features) { LaunchDarkly::InMemoryFeatureStore.new } include LaunchDarkly::Evaluation + describe "evaluate" do + it "returns off variation if flag is off" do + flag = { + key: 'feature', + on: false, + offVariation: 1, + fallthrough: { variation: 0 }, + variations: ['a', 'b', 'c'] + } + user = { key: 'x' } + expect(evaluate(flag, user, features)).to eq({value: 'b', events: []}) + end + + it "returns nil if flag is off and off variation is unspecified" do + flag = { + key: 'feature', + on: false, + fallthrough: { variation: 0 }, + variations: ['a', 'b', 'c'] + } + user = { key: 'x' } + expect(evaluate(flag, user, features)).to eq({value: nil, events: []}) + end + + it "returns off variation if prerequisite is not found" do + flag = { + key: 'feature0', + on: true, + prerequisites: [{key: 'badfeature', variation: 1}], + fallthrough: { variation: 0 }, + offVariation: 1, + variations: ['a', 'b', 'c'] + } + user = { key: 'x' } + expect(evaluate(flag, user, features)).to eq({value: 'b', events: []}) + end + + it "returns off variation and event if prerequisite is not met" do + flag = { + key: 'feature0', + on: true, + prerequisites: [{key: 'feature1', variation: 1}], + fallthrough: { variation: 0 }, + offVariation: 1, + variations: ['a', 'b', 'c'], + version: 1 + } + flag1 = { + key: 'feature1', + on: true, + fallthrough: { variation: 0 }, + variations: ['d', 'e'], + version: 2 + } + features.upsert('feature1', flag1) + user = { key: 'x' } + events_should_be = [{kind: 'feature', key: 'feature1', value: 'd', version: 2, prereqOf: 'feature0'}] + expect(evaluate(flag, user, features)).to eq({value: 'b', events: events_should_be}) + end + + it "returns fallthrough variation and event if prerequisite is met and there are no rules" do + flag = { + key: 'feature0', + on: true, + prerequisites: [{key: 'feature1', variation: 1}], + fallthrough: { variation: 0 }, + offVariation: 1, + variations: ['a', 'b', 'c'], + version: 1 + } + flag1 = { + key: 'feature1', + on: true, + fallthrough: { variation: 1 }, + variations: ['d', 'e'], + version: 2 + } + features.upsert('feature1', flag1) + user = { key: 'x' } + events_should_be = [{kind: 'feature', key: 'feature1', value: 'e', version: 2, prereqOf: 'feature0'}] + expect(evaluate(flag, user, features)).to eq({value: 'a', events: events_should_be}) + end + + it "matches user from targets" do + flag = { + key: 'feature0', + on: true, + targets: [ + { values: [ 'whoever', 'userkey' ], variation: 2 } + ], + fallthrough: { variation: 0 }, + offVariation: 1, + variations: ['a', 'b', 'c'] + } + user = { key: 'userkey' } + expect(evaluate(flag, user, features)).to eq({value: 'c', events: []}) + end + + it "matches user from rules" do + flag = { + key: 'feature0', + on: true, + rules: [ + { + clauses: [ + { + attribute: 'key', + op: 'in', + values: [ 'userkey' ] + } + ], + variation: 2 + } + ], + fallthrough: { variation: 0 }, + offVariation: 1, + variations: ['a', 'b', 'c'] + } + user = { key: 'userkey' } + expect(evaluate(flag, user, features)).to eq({value: 'c', events: []}) + end + end + describe "clause_match_user" do it "can match built-in attribute" do user = { key: 'x', name: 'Bob' } @@ -23,6 +147,12 @@ clause = { attribute: 'legs', op: 'in', values: [4] } expect(clause_match_user(clause, user)).to be false end + + it "can be negated" do + user = { key: 'x', name: 'Bob' } + clause = { attribute: 'name', op: 'in', values: ['Bob'], negate: true } + expect(clause_match_user(clause, user)).to be false + end end describe "operators" do From c308d9dd9e98cd4a9f8e653f5d6370bfb478b90a Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 7 Feb 2018 10:53:50 -0800 Subject: [PATCH 16/16] use change expectation --- spec/evaluation_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/evaluation_spec.rb b/spec/evaluation_spec.rb index 31346630..092136fa 100644 --- a/spec/evaluation_spec.rb +++ b/spec/evaluation_spec.rb @@ -150,8 +150,10 @@ it "can be negated" do user = { key: 'x', name: 'Bob' } - clause = { attribute: 'name', op: 'in', values: ['Bob'], negate: true } - expect(clause_match_user(clause, user)).to be false + clause = { attribute: 'name', op: 'in', values: ['Bob'] } + expect { + clause[:negate] = true + }.to change {clause_match_user(clause, user)}.from(true).to(false) end end