Issues 1145 403 811 (#21)

* Add IS_SET and IS_NOT_SET condition operators and tests

* the inner match functions for these clauses shouldn't be needed since it's evaluated in another completely different way

* MODULO operator & tests

* get_identity_segments

* fix typespec

* Modulo operator to support fractional values

Add tests for IS_SET and IS_NOT_SET

Remove warnings

Remove priority evaluation when extracting identity segments since it shouldn't change at all the segments

* Test for when IS_SET isn't met

* remove extra unecessary filtering

rename get_segment_features to get_identity_applicable_segments

Author: mnussbaumer

lib/flagsmith_engine.ex

@@ -1,4 +1,6 @@
 defmodule Flagsmith.Engine do
+  require Logger
+
   alias Flagsmith.Schemas.{
     Environment,
     Traits,
@@ -69,7 +71,8 @@ defmodule Flagsmith.Engine do
         override_traits \\ []
       ) do
     with identity <- Identity.set_env_key(identity, env),
-         segment_features <- get_segment_features(segments, identity, override_traits),
+         segment_features <-
+           get_identity_applicable_segments(segments, identity, override_traits),
          prioritized <- clean_segments_by_priority(segment_features),
          replaced <- replace_segment_features(fs, prioritized),
          pre_features <- replace_identity_features(replaced, identity_features),
@@ -78,6 +81,30 @@ defmodule Flagsmith.Engine do
     end
   end
 
+  @doc """
+  Get list of segments for a given `t:Flagsmith.Schemas.Identity.t/0` in a 
+  given `t:Flagsmith.Schemas.Environment.t/0`.
+  """
+  @spec get_identity_segments(
+          Environment.t(),
+          Identity.t(),
+          override_traits :: list(Traits.Trait.t())
+        ) :: list(Segments.IdentitySegment.t())
+  def get_identity_segments(
+        %Environment{
+          project: %Environment.Project{segments: segments}
+        } = env,
+        identity,
+        override_traits \\ []
+      ) do
+    with identity <- Identity.set_env_key(identity, env),
+         segments <-
+           get_identity_applicable_segments(segments, identity, override_traits),
+         replaced <- Enum.map(segments, &Segments.IdentitySegment.from_segment/1) do
+      replaced
+    end
+  end
+
   defp clean_segments_by_priority(segments) do
     # keep an ordered table by index so we can retrieve the segments in order
     # at the end when manipulating them
@@ -244,12 +271,12 @@ defmodule Flagsmith.Engine do
   Filters a list of segments accordingly to if they match an identity and traits
   (optionally using a list of traits to override those in the identity)
   """
-  @spec get_segment_features(
+  @spec get_identity_applicable_segments(
           segments :: list(Segments.Segment.t()),
           Identity.t(),
           override_traits :: list(Traits.Trait.t())
         ) :: list(Segments.Segment.t())
-  def get_segment_features(segments, identity, override_traits) do
+  def get_identity_applicable_segments(segments, identity, override_traits) do
     Enum.filter(segments, fn segment ->
       evaluate_identity_in_segment(identity, segment, override_traits)
     end)
@@ -386,6 +413,24 @@ defmodule Flagsmith.Engine do
     end
   end
 
+  def traits_match_segment_condition(
+        traits,
+        %Segments.Segment.Condition{operator: :IS_SET, property_: prop},
+        _segment_id,
+        _identifier
+      ) do
+    Enum.any?(traits, fn %Traits.Trait{trait_key: t_key} -> t_key == prop end)
+  end
+
+  def traits_match_segment_condition(
+        traits,
+        %Segments.Segment.Condition{operator: :IS_NOT_SET, property_: prop},
+        _segment_id,
+        _identifier
+      ) do
+    Enum.all?(traits, fn %Traits.Trait{trait_key: t_key} -> t_key != prop end)
+  end
+
   def traits_match_segment_condition(
         traits,
         %Segments.Segment.Condition{operator: operator, value: value, property_: prop},
@@ -535,6 +580,27 @@ defmodule Flagsmith.Engine do
     end
   end
 
+  def trait_match(:MODULO, trait, %Trait.Value{value: value}) do
+    with true <- is_binary(trait),
+         %Decimal{} <- value,
+         [mod, result] <- String.split(trait, "|"),
+         %Decimal{} = mod_val <- Decimal.new(mod),
+         %Decimal{} = result_val <- Decimal.new(result),
+         %Decimal{} = remainder <- Decimal.rem(value, mod_val) do
+      Decimal.equal?(remainder, result_val)
+    else
+      _ ->
+        false
+    end
+  rescue
+    Decimal.Error ->
+      Logger.warn(
+        "invalid MODULO segment rule or trait value :: rule: #{inspect(trait)} :: value: #{inspect(value)}"
+      )
+
+      false
+  end
+
   def trait_match(condition, not_cast, %Trait.Value{} = t_value_struct)
       when condition in @condition_operators and not is_struct(not_cast) and not is_map(not_cast) do
     case Trait.Value.is_semver(not_cast) do

lib/schemas/identity_segment.ex

@@ -0,0 +1,19 @@
+defmodule Flagsmith.Schemas.Segments.IdentitySegment do
+  use TypedEctoSchema
+
+  alias Flagsmith.Schemas.Segments
+
+  @moduledoc """
+  Ecto schema representing a Flagsmith Identity Segment.
+  """
+
+  @primary_key {:id, :id, autogenerate: false}
+  typed_embedded_schema do
+    field(:name, :string)
+  end
+
+  @doc false
+  @spec from_segment(Segments.Segment.t()) :: __MODULE__.t()
+  def from_segment(%Segments.Segment{id: id, name: name}),
+    do: %__MODULE__{id: id, name: name}
+end

lib/schemas/types/operator.ex

@@ -15,6 +15,9 @@ defmodule Flagsmith.Schemas.Types.Operator do
       :LESS_THAN_INCLUSIVE,
       :NOT_EQUAL,
       :CONTAINS,
+      :IS_SET,
+      :IS_NOT_SET,
+      :MODULO,
       :PERCENTAGE_SPLIT
     ]
 end

test/flagsmith_engine_test.exs

@@ -1,7 +1,7 @@
 defmodule Flagsmith.EngineTest do
   use ExUnit.Case, async: true
 
-  alias Flagsmith.Schemas.{Environment, Features, Segments}
+  alias Flagsmith.Schemas.{Environment, Features, Segments, Traits}
   alias Flagsmith.Engine.Test
 
   # stub the mock so that it calls the normal module as it would under regular usage
@@ -253,6 +253,34 @@ defmodule Flagsmith.EngineTest do
              ] = Flagsmith.Engine.get_identity_feature_states(env, identity, [])
     end
 
+    test "get_identity_segments/3", %{env: env, identity: identity} do
+      # the identity we're using has `show_popup` trait as false by default so
+      # it should evaluate as this segment being for this identity when no traits
+      # are passed
+      assert [%Flagsmith.Schemas.Segments.IdentitySegment{id: 5241, name: "test_segment"}] =
+               Flagsmith.Engine.get_identity_segments(env, identity, [])
+
+      # passing the trait as `true` should make this segment no longer match since
+      # the condition is `show_popup` to be false
+      assert [] =
+               Flagsmith.Engine.get_identity_segments(env, identity, [
+                 %Traits.Trait{
+                   trait_key: "show_popup",
+                   trait_value: %Traits.Trait.Value{value: true, type: :boolean}
+                 }
+               ])
+
+      # and passing the trait as `false` (as is the default) should make it match just
+      # the same as initially
+      assert [%Flagsmith.Schemas.Segments.IdentitySegment{id: 5241, name: "test_segment"}] =
+               Flagsmith.Engine.get_identity_segments(env, identity, [
+                 %Traits.Trait{
+                   trait_key: "show_popup",
+                   trait_value: %Traits.Trait.Value{value: false, type: :boolean}
+                 }
+               ])
+    end
+
     test "get_identity_feature_state/4", %{env: env, identity: identity} do
       assert %Features.FeatureState{
                id: 72267,

test/unit/segment_conditions_test.exs

@@ -55,7 +55,17 @@ defmodule Flagsmith.Engine.SegmentConditionsTest do
     {:NOT_CONTAINS, "bar", "baz", true},
     {:REGEX, "foo", "[a-z]+", true},
     {:REGEX, "FOO", "[a-z]+", false},
-    {:REGEX, "1.2.3", "\\d", true}
+    {:REGEX, "1.2.3", "\\d", true},
+    {:MODULO, 1, "2|0", false},
+    {:MODULO, 2, "2|0", true},
+    {:MODULO, 3, "2|0", false},
+    {:MODULO, 34.2, "4|3", false},
+    {:MODULO, 35.0, "4|3", true},
+    {:MODULO, "dummy", "3|0", false},
+    {:MODULO, "1.0.0", "3|0", false},
+    {:MODULO, false, "1|3", false},
+    {:MODULO, 3.5, "1.5|0.5", true},
+    {:MODULO, 4, "1.5|0.5", false}
   ]
 
   test "all conditions" do
@@ -354,4 +364,108 @@ defmodule Flagsmith.Engine.SegmentConditionsTest do
 
     refute Flagsmith.Engine.traits_match_segment_rule(traits_3, @segment_rule_nested_any, 1, 1)
   end
+
+  @segment_rule_all_is_or_not_set %Segment.Rule{
+    conditions: [],
+    rules: [
+      %Segment.Rule{
+        conditions: [
+          %Segment.Condition{
+            operator: :IS_SET,
+            property_: "test_is_set"
+          },
+          %Segment.Condition{
+            operator: :IS_NOT_SET,
+            property_: "test_is_not_set"
+          }
+        ],
+        rules: [],
+        type: :ALL
+      }
+    ],
+    type: :ALL
+  }
+
+  test "Segment.Rule IS_SET and IS_NOT_SET" do
+    # test that the segment matches (both conditions, IS_SET is true, and
+    # IS_NOT_SET true)
+    traits_1 = [
+      %Trait{
+        trait_key: "test_is_set",
+        trait_value: %Trait.Value{value: true, type: :boolean}
+      }
+    ]
+
+    assert Flagsmith.Engine.traits_match_segment_rule(
+             traits_1,
+             @segment_rule_all_is_or_not_set,
+             1,
+             1
+           )
+
+    # test that the segment matches even is the test_is_set trait value is false
+    traits_2 = [
+      %Trait{
+        trait_key: "test_is_set",
+        trait_value: %Trait.Value{value: false, type: :boolean}
+      }
+    ]
+
+    assert Flagsmith.Engine.traits_match_segment_rule(
+             traits_2,
+             @segment_rule_all_is_or_not_set,
+             1,
+             1
+           )
+
+    # refute because `test_is_not_set` is passed as a trait and so the segment
+    # condition IS_NOT_SET should fail since the segment specifies :ALL for rules
+    # validations
+    traits_3 = [
+      %Trait{
+        trait_key: "test_is_set",
+        trait_value: %Trait.Value{value: true, type: :boolean}
+      },
+      %Trait{
+        trait_key: "test_is_not_set",
+        trait_value: %Trait.Value{value: true, type: :boolean}
+      }
+    ]
+
+    refute Flagsmith.Engine.traits_match_segment_rule(
+             traits_3,
+             @segment_rule_all_is_or_not_set,
+             1,
+             1
+           )
+
+    # :IS_NOT_SET should still evaluate to false even if the trait value is `false`
+    # since it's still set
+    traits_4 = [
+      %Trait{
+        trait_key: "test_is_set",
+        trait_value: %Trait.Value{value: true, type: :boolean}
+      },
+      %Trait{
+        trait_key: "test_is_not_set",
+        trait_value: %Trait.Value{value: false, type: :boolean}
+      }
+    ]
+
+    refute Flagsmith.Engine.traits_match_segment_rule(
+             traits_4,
+             @segment_rule_all_is_or_not_set,
+             1,
+             1
+           )
+
+    # :IS_SET should fail since no trait is being passed to match it
+
+    refute Flagsmith.Engine.traits_match_segment_rule(
+             [],
+             @segment_rule_all_is_or_not_set,
+             1,
+             1
+           )
+  end
 end