Merge pull request #140 from Countly/fix_segm_same_keys

fix: segmentation same key

CHANGELOG.md

@@ -1,4 +1,5 @@
-## XX.XX.XX
+## 24.1.1
+* Fixed a bug where same, null, and empty keys were permitted in the Segmentation.
 * Fixed an issue where some requests are not url encoded.
 
 ## 24.1.0

countlyCommon/TestingRelated/EventTests.cs

@@ -75,6 +75,38 @@ public async void TestEventLimits()
             Countly.Instance.SessionEnd().Wait();
         }
 
+        [Fact]
+        /// <summary>
+        /// It validates that segmentation cannot have same keys
+        /// </summary>
+        public async void TestEventSameSegmentationKey()
+        {
+            CountlyConfig cc = TestHelper.CreateConfig();
+
+            Countly.Instance.Init(cc).Wait();
+            Countly.Instance.SessionBegin().Wait();
+
+            Countly.Instance.deferUpload = true;
+
+            Segmentation segm = new Segmentation();
+            segm.Add("key1", "value1");
+            segm.Add("key2", "value2");
+            segm.Add("key2", "value3");
+
+            Assert.Equal("key1", segm.segmentation[0].Key);
+            Assert.Equal("value1", segm.segmentation[0].Value);
+            Assert.Equal("key2", segm.segmentation[1].Key);
+            Assert.Equal("value3", segm.segmentation[1].Value);
+
+            Assert.True(segm.segmentation.Count == 2); // not 3 becasue key 2 overridden and segmentation does not permit same keys
+            bool res = await Countly.RecordEvent("test_event", 1, 23, 5.0, Segmentation: segm);
+            Assert.True(res);
+
+            CountlyEvent countlyEvent = Countly.Instance.Events[0];
+            validateEventData(countlyEvent, "test_event", 1, 23, 5, segm);
+            Countly.Instance.SessionEnd().Wait();
+        }
+
         /// <summary>
         /// It validates the cancellation of timed events on changing device id without merge.
         ///

countlyCommon/TestingRelated/SegmentationTests.cs

@@ -0,0 +1,92 @@
+using System;
+using System.Collections.Generic;
+using CountlySDK;
+using CountlySDK.Entities;
+using Xunit;
+using static CountlySDK.CountlyCommon.CountlyBase;
+
+namespace TestProject_common
+{
+    public class SegmentationTests : IDisposable
+    {
+        /// <summary>
+        /// Test setup
+        /// </summary>
+        public SegmentationTests()
+        {
+            CountlyImpl.SetPCLStorageIfNeeded();
+            Countly.Halt();
+            TestHelper.CleanDataFiles();
+            Countly.Instance.deferUpload = true;
+        }
+
+        /// <summary>
+        /// Test cleanup
+        /// </summary>
+        public void Dispose()
+        {
+            Countly.Instance.HaltInternal().Wait();
+        }
+
+        [Fact]
+        /// <summary>
+        /// It validates the invalid key values
+        /// All values are accepted
+        /// Only validated invalid keys
+        /// </summary>
+        public void InvalidKeyValues()
+        {
+            Segmentation segmentation = new Segmentation();
+
+            segmentation.Add(null, null);
+            Assert.Empty(segmentation.segmentation);
+
+            segmentation.Add("", null);
+            Assert.Empty(segmentation.segmentation);
+
+            segmentation.Add(" ", null);
+            Assert.Equal(" ", segmentation.segmentation[0].Key);
+            Assert.Null(segmentation.segmentation[0].Value);
+        }
+
+        [Fact]
+        /// <summary>
+        /// It validates valid values
+        /// </summary>
+        public void ValidKeyValues()
+        {
+            Segmentation segmentation = new Segmentation();
+
+            segmentation.Add("1", "2");
+            Assert.Equal("1", segmentation.segmentation[0].Key);
+            Assert.Equal("2", segmentation.segmentation[0].Value);
+
+            segmentation.Add("3", "4");
+            Assert.Equal("3", segmentation.segmentation[1].Key);
+            Assert.Equal("4", segmentation.segmentation[1].Value);
+            Assert.Equal(2, segmentation.segmentation.Count);
+
+        }
+
+        [Fact]
+        /// <summary>
+        /// It validates valid values with same keys,
+        /// this test shows no same keys accepted
+        /// </summary>
+        public void ValidKeyValues_SameKeys()
+        {
+            Segmentation segmentation = new Segmentation();
+
+            segmentation.Add("1", "2");
+            Assert.Equal("1", segmentation.segmentation[0].Key);
+            Assert.Equal("2", segmentation.segmentation[0].Value);
+
+            segmentation.Add("1", "4");
+            Assert.Equal("1", segmentation.segmentation[0].Key);
+            Assert.Equal("4", segmentation.segmentation[0].Value);
+            Assert.Single(segmentation.segmentation);
+
+        }
+
+    }
+}

countlyCommon/countlyCommon/Entities/Segmentation.cs

@@ -47,13 +47,25 @@ public Segmentation()
         }
 
         /// <summary>
-        /// Add new segmentation value
+        /// Add new segmentation value, omits the null keys
+        /// overrides the same keys
         /// </summary>
         /// <param name="Key">Segmenation key</param>
         /// <param name="Value">Segmenation value</param>
         public void Add(string Key, string Value)
         {
-            segmentation.Add(new SegmentationItem(Key, Value));
+            if (string.IsNullOrEmpty(Key)) {
+                return;
+            }
+            // Check if a segmentation item with the same key already exists
+            SegmentationItem existingItem = segmentation.Find(item => item.Key == Key);
+            if (existingItem != null) {
+                // Update the value if the key exists
+                existingItem.Value = Value;
+            } else {
+                // Add a new item if the key doesn't exist
+                segmentation.Add(new SegmentationItem(Key, Value));
+            }
         }
 
         public int CompareTo(Segmentation other)

net35/CountlyTest_35/CountlyTest_35.csproj

@@ -86,6 +86,9 @@
     <Compile Include="..\..\countlyCommon\TestingRelated\RequestTestCases.cs">
       <Link>RequestTestCases.cs</Link>
     </Compile>
+    <Compile Include="..\..\countlyCommon\TestingRelated\SegmentationTests.cs">
+      <Link>SegmentationTests.cs</Link>
+    </Compile>
     <Compile Include="..\..\countlyCommon\TestingRelated\SessionTests.cs">
       <Link>SessionTests.cs</Link>
     </Compile>

net45/CountlyTest_45/CountlyTest_45.csproj

@@ -85,6 +85,9 @@
     <Compile Include="..\..\countlyCommon\TestingRelated\RequestTestCases.cs">
       <Link>RequestTestCases.cs</Link>
     </Compile>
+    <Compile Include="..\..\countlyCommon\TestingRelated\SegmentationTests.cs">
+      <Link>SegmentationTests.cs</Link>
+    </Compile>
     <Compile Include="..\..\countlyCommon\TestingRelated\SessionTests.cs">
       <Link>SessionTests.cs</Link>
     </Compile>

netstd/CountlyTest_461/CountlyTest_461.csproj

@@ -96,6 +96,9 @@
     <Compile Include="..\..\countlyCommon\TestingRelated\RequestTestCases.cs">
       <Link>RequestTestCases.cs</Link>
     </Compile>
+    <Compile Include="..\..\countlyCommon\TestingRelated\SegmentationTests.cs">
+      <Link>SegmentationTests.cs</Link>
+    </Compile>
     <Compile Include="..\..\countlyCommon\TestingRelated\SessionTests.cs">
       <Link>SessionTests.cs</Link>
     </Compile>