From 8bc8ba33752de2a74a425bb64d521f4fa840e488 Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Fri, 25 Jan 2019 12:54:35 +0000 Subject: [PATCH 01/11] [GCOM-1309] Add methods for users in multiple experiments --- Gibe.AbTest.Tests/AbTestTests.cs | 83 ++++++++++++++++++++++++++------ Gibe.AbTest/AbTest.cs | 31 +++++++++--- Gibe.AbTest/IAbTest.cs | 18 +++++++ Gibe.AbTest/IAbTestingService.cs | 18 +++---- 4 files changed, 116 insertions(+), 34 deletions(-) diff --git a/Gibe.AbTest.Tests/AbTestTests.cs b/Gibe.AbTest.Tests/AbTestTests.cs index 83f54ff..6f32b3e 100644 --- a/Gibe.AbTest.Tests/AbTestTests.cs +++ b/Gibe.AbTest.Tests/AbTestTests.cs @@ -1,51 +1,102 @@ -using System; +using NUnit.Framework; +using System; +using System.Collections.Generic; using System.Linq; -using NUnit.Framework; namespace Gibe.AbTest.Tests { [TestFixture] public class AbTestTests { - [Test] - public void AssignVariation_assigns_first_experiment_variation_when_random_number_is_0() + private IAbTestingService _abTestingService; + + [SetUp] + public void Setup() { - var fakeAbTestingService = new FakeAbTestingService(); + _abTestingService = new FakeAbTestingService(new List + { + new Experiment("Ex1", "Exp1", "Experiement 1", 1, true, DateTime.Now, null, + new []{ + new Variation(1, 0, 1, true, "{Exp1:'Variant 1'}", "Exp1"), + new Variation(2, 1, 1, true, "{Exp1:'Variant 2'}", "Exp1") - var abTest = new AbTest(fakeAbTestingService, new FakeRandomNumber(new [] { 0, 0 })); + }), + new Experiment("Ex2", "Exp2", "Experiement 2", 1, true, DateTime.Now, null, + new []{ + new Variation(3, 0, 1, true, "{Exp2:'Variant 1'}", "Exp2"), + new Variation(4, 1, 1, true, "{Exp2:'Variant 2'}", "Exp2") + }), + new Experiment("Ex3", "Exp3", "Experiement 3", 1, false, DateTime.Now, null, + new []{ + new Variation(5, 0, 1, true, "{Exp3:'Variant 1'}", "Exp3"), + new Variation(6, 1, 1, true, "{Exp2:'Variant 2'}", "Exp3") + }) + }); + } + + [Test] + public void AssignVariation_Assigns_First_Experiment_Variation_When_Random_Number_Is_0() + { + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new [] { 0, 0 })); var variation = abTest.AssignVariation(); - Assert.AreEqual(fakeAbTestingService.GetExperiments().First().Variations.First().Id, variation.Id); + Assert.AreEqual(_abTestingService.GetExperiments().First().Variations.First().Id, variation.Id); } [Test] - public void AssignVariation_assigns_second_experiment_variation_when_random_number_is_1() + public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Number_Is_1() { - var fakeAbTestingService = new FakeAbTestingService(); + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 1, 1 })); - var abTest = new AbTest(fakeAbTestingService, new FakeRandomNumber(new[] { 1, 1 })); + var variation = abTest.AssignVariation(); + + Assert.AreEqual(_abTestingService.GetExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); + } + + [Test] + public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Number_Is_2() + { + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 2, 2 })); var variation = abTest.AssignVariation(); - Assert.AreEqual(fakeAbTestingService.GetExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); + Assert.AreEqual(_abTestingService.GetExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); + } + + [Test] + public void AssignVariation_Assigns_Given_Experiment_First_Variation() + { + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 0, 0 })); + + var variation = abTest.AssignVariation("Exp1"); + + Assert.That(_abTestingService.GetExperiments().First(x => x.Key == "Exp1").Variations.First().Id, Is.EqualTo(variation.Id)); + } + + [Test] + public void AssignVariations_Assigns_First_Experiment_Variation_From_Each_Enabled_Experiment() + { + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 0, 0 })); + + var variations = abTest.AssignVariations().ToList(); + + Assert.That(_abTestingService.GetExperiments().Where(e => e.Enabled).Select(e => e.Variations.First().Id), Is.EqualTo(variations.Select(v => v.Id))); + Assert.That(variations.Count, Is.EqualTo(2)); } [Test] public void GetAssignedVariation_returns_variation_from_AbTestingService() { - const string experimentId = "ABC"; + const string experimentId = "Exp4"; const int variationNo = 1; - var fakeAbTestingService = new FakeAbTestingService(); - - var abTest = new AbTest(fakeAbTestingService, new FakeRandomNumber(new int[] {})); + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new int[] {})); var variation = abTest.GetAssignedVariation(experimentId, variationNo); Assert.AreEqual(experimentId, variation.ExperimentId); Assert.AreEqual(variationNo, variation.VariationNumber); - } } } diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index 45889e6..6a01e93 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -1,5 +1,4 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; namespace Gibe.AbTest @@ -9,7 +8,8 @@ public class AbTest : IAbTest private readonly IAbTestingService _abTestingService; private readonly IRandomNumber _randomNumber; - public AbTest(IAbTestingService abTestingService, IRandomNumber randomNumber) + public AbTest(IAbTestingService abTestingService, + IRandomNumber randomNumber) { _abTestingService = abTestingService; _randomNumber = randomNumber; @@ -22,6 +22,23 @@ public Variation AssignVariation() return RandomlySelectOption(selectedExperiment.Variations); } + public Variation AssignVariation(string experiementKey) + { + var experiment = _abTestingService.GetExperiments() + .Where(x => x.Enabled) + .First(x => x.Key == experiementKey); + return RandomlySelectOption(experiment.Variations); + } + + public IEnumerable AssignVariations() + { + var experiments = _abTestingService.GetExperiments().Where(x => x.Enabled); + foreach (var experiment in experiments) + { + yield return RandomlySelectOption(experiment.Variations); + } + } + public Variation GetAssignedVariation(string experimentId, int variationNumber) { return _abTestingService.GetVariation(experimentId, variationNumber); @@ -34,15 +51,13 @@ private T RandomlySelectOption(IEnumerable options) where T : IWeighted var selectedNumber = _randomNumber.Number(totalWeights); var currrentWeight = 0; - foreach (var t in opts) + foreach (var o in opts) { - currrentWeight += t.Weight; + currrentWeight += o.Weight; if (currrentWeight > selectedNumber) - return t; + return o; } return opts.Last(); } - - } } diff --git a/Gibe.AbTest/IAbTest.cs b/Gibe.AbTest/IAbTest.cs index 1911578..e9dba6b 100644 --- a/Gibe.AbTest/IAbTest.cs +++ b/Gibe.AbTest/IAbTest.cs @@ -1,4 +1,5 @@ using System; +using System.CodeDom; using System.Collections.Generic; using System.Linq; using System.Text; @@ -9,6 +10,8 @@ namespace Gibe.AbTest public interface IAbTest { Variation AssignVariation(); + Variation AssignVariation(string experiementKey); + IEnumerable AssignVariations(); Variation GetAssignedVariation(string experimentId, int variationNumber); } @@ -19,6 +22,21 @@ public Variation AssignVariation() return new Variation(1, 0, 1,true,"{Test:'test'}", "ABC1"); } + public Variation AssignVariation(string experiementKey) + { + return new Variation(1, 0, 1,true,"{Test:'test'}", experiementKey); + } + + public IEnumerable AssignVariations() + { + return new List + { + new Variation(1, 0, 1,true,"{Test:'test1'}", "ABC1"), + new Variation(2, 1, 1,true,"{Test:'test2'}", "DEF2"), + new Variation(3, 0, 1,true,"{Test:'test3'}", "GHI3") + }; + } + public Variation GetAssignedVariation(string experimentId, int variationNumber) { return new Variation(1, variationNumber, 1, true, "{Test:'test'}", experimentId); diff --git a/Gibe.AbTest/IAbTestingService.cs b/Gibe.AbTest/IAbTestingService.cs index 7ed51d5..866cb5d 100644 --- a/Gibe.AbTest/IAbTestingService.cs +++ b/Gibe.AbTest/IAbTestingService.cs @@ -1,8 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +using System.Collections.Generic; namespace Gibe.AbTest { @@ -15,6 +11,12 @@ public interface IAbTestingService public class FakeAbTestingService : IAbTestingService { + public IEnumerable Experiements; + public FakeAbTestingService(IEnumerable experiements) + { + Experiements = experiements; + } + public Variation GetVariation(string experimentId, int variationNumber) { return new Variation(1, variationNumber, 1, true, "{Test:'test'}", experimentId); @@ -31,11 +33,7 @@ public IEnumerable GetVariations(string experimentId) public IEnumerable GetExperiments() { - return new List - { - new Experiment("A1234", "AB1", "Desc 1", 1, true, DateTime.Now, null, GetVariations("AB1").ToArray()), - new Experiment("A2345", "AB2", "Desc 2", 1, true, DateTime.Now, null, GetVariations("AB2").ToArray()) - }; + return Experiements; } } } From 86a886407f4680d967d124264c2d3d3df0697c4a Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Tue, 29 Jan 2019 11:52:57 +0000 Subject: [PATCH 02/11] Add constructor to FakeAbTest --- Gibe.AbTest.Tests/AbTestTests.cs | 8 ++++---- Gibe.AbTest/AbTest.cs | 5 +++-- Gibe.AbTest/IAbTest.cs | 35 ++++++++++++++++---------------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/Gibe.AbTest.Tests/AbTestTests.cs b/Gibe.AbTest.Tests/AbTestTests.cs index 6f32b3e..bf58c15 100644 --- a/Gibe.AbTest.Tests/AbTestTests.cs +++ b/Gibe.AbTest.Tests/AbTestTests.cs @@ -39,7 +39,7 @@ public void AssignVariation_Assigns_First_Experiment_Variation_When_Random_Numbe { var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new [] { 0, 0 })); - var variation = abTest.AssignVariation(); + var variation = abTest.AssignRandomVariation(); Assert.AreEqual(_abTestingService.GetExperiments().First().Variations.First().Id, variation.Id); } @@ -49,7 +49,7 @@ public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Numb { var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 1, 1 })); - var variation = abTest.AssignVariation(); + var variation = abTest.AssignRandomVariation(); Assert.AreEqual(_abTestingService.GetExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); } @@ -59,7 +59,7 @@ public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Numb { var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 2, 2 })); - var variation = abTest.AssignVariation(); + var variation = abTest.AssignRandomVariation(); Assert.AreEqual(_abTestingService.GetExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); } @@ -93,7 +93,7 @@ public void GetAssignedVariation_returns_variation_from_AbTestingService() var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new int[] {})); - var variation = abTest.GetAssignedVariation(experimentId, variationNo); + var variation = abTest.AssignedVariation(experimentId, variationNo); Assert.AreEqual(experimentId, variation.ExperimentId); Assert.AreEqual(variationNo, variation.VariationNumber); diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index 6a01e93..f6c7151 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -15,7 +15,7 @@ public AbTest(IAbTestingService abTestingService, _randomNumber = randomNumber; } - public Variation AssignVariation() + public Variation AssignRandomVariation() { var experiments = _abTestingService.GetExperiments().Where(x => x.Enabled); var selectedExperiment = RandomlySelectOption(experiments); @@ -39,11 +39,12 @@ public IEnumerable AssignVariations() } } - public Variation GetAssignedVariation(string experimentId, int variationNumber) + public Variation AssignedVariation(string experimentId, int variationNumber) { return _abTestingService.GetVariation(experimentId, variationNumber); } + private T RandomlySelectOption(IEnumerable options) where T : IWeighted { var opts = options.ToArray(); diff --git a/Gibe.AbTest/IAbTest.cs b/Gibe.AbTest/IAbTest.cs index e9dba6b..043ab10 100644 --- a/Gibe.AbTest/IAbTest.cs +++ b/Gibe.AbTest/IAbTest.cs @@ -1,45 +1,44 @@ -using System; -using System.CodeDom; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; -using System.Text; -using System.Threading.Tasks; +using System.Net.Http.Headers; namespace Gibe.AbTest { public interface IAbTest { - Variation AssignVariation(); + Variation AssignRandomVariation(); Variation AssignVariation(string experiementKey); IEnumerable AssignVariations(); - Variation GetAssignedVariation(string experimentId, int variationNumber); + Variation AssignedVariation(string experimentId, int variationNumber); } public class FakeAbTest : IAbTest { - public Variation AssignVariation() + private readonly IEnumerable _variations; + + public FakeAbTest(IEnumerable variations) + { + _variations = variations; + } + + public Variation AssignRandomVariation() { - return new Variation(1, 0, 1,true,"{Test:'test'}", "ABC1"); + return _variations.First(); } public Variation AssignVariation(string experiementKey) { - return new Variation(1, 0, 1,true,"{Test:'test'}", experiementKey); + return _variations.First(v => v.ExperimentId == experiementKey); } public IEnumerable AssignVariations() { - return new List - { - new Variation(1, 0, 1,true,"{Test:'test1'}", "ABC1"), - new Variation(2, 1, 1,true,"{Test:'test2'}", "DEF2"), - new Variation(3, 0, 1,true,"{Test:'test3'}", "GHI3") - }; + return _variations.GroupBy(v => v.ExperimentId).Select(group => group.First()); } - public Variation GetAssignedVariation(string experimentId, int variationNumber) + public Variation AssignedVariation(string experimentId, int variationNumber) { - return new Variation(1, variationNumber, 1, true, "{Test:'test'}", experimentId); + return _variations.First(v => v.ExperimentId == experimentId && v.VariationNumber == variationNumber); } } } From 6766b804903ba604bf7d526cfffaef2549a4350b Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Tue, 29 Jan 2019 16:53:40 +0000 Subject: [PATCH 03/11] version: 1.1.0 --- version.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 version.txt diff --git a/version.txt b/version.txt new file mode 100644 index 0000000..9084fa2 --- /dev/null +++ b/version.txt @@ -0,0 +1 @@ +1.1.0 From 2e7f9fdc92b0440bc926d2a82d5033fa1baf5afc Mon Sep 17 00:00:00 2001 From: Steve Temple Date: Wed, 30 Jan 2019 11:25:59 +0000 Subject: [PATCH 04/11] Add more tests for repo --- Gibe.AbTest.Tests/AbTestRepositoryTests.cs | 48 ++++++++++++++++++++-- Gibe.AbTest.Tests/App.config | 2 +- Gibe.AbTest/AbTestRepository.cs | 16 ++------ Gibe.AbTest/IAbTestRepository.cs | 6 +-- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/Gibe.AbTest.Tests/AbTestRepositoryTests.cs b/Gibe.AbTest.Tests/AbTestRepositoryTests.cs index a8bf2b2..9855de7 100644 --- a/Gibe.AbTest.Tests/AbTestRepositoryTests.cs +++ b/Gibe.AbTest.Tests/AbTestRepositoryTests.cs @@ -11,11 +11,53 @@ namespace Gibe.AbTest.Tests [TestFixture] public class AbTestRepositoryTests { + private const string ValidExperimentId = "vapBwUPvTEuGcEVEKThGCA"; + private const string InvalidExperimentId = "NotRealId"; + [Test] - public void Test() + public void GetExperiments_Returns_All_Experiments() { - var repo = new AbTestRepository(new DefaultDatabaseProvider("GibeCommerce")); - var experiments = repo.GetExperiments().ToArray(); + var experiments = Repo().GetExperiments().ToArray(); + + Assert.That(experiments.Length, Is.EqualTo(2)); + } + + [Test] + public void GetExperiment_Returns_Experiment_When_Id_Exists() + { + var experiment = Repo().GetExperiment(ValidExperimentId); + + Assert.That(experiment, Is.Not.Null); + Assert.That(experiment.Id, Is.EqualTo(ValidExperimentId)); + } + + [Test] + public void GetExperiment_Returns_Null_When_Id_Does_Not_Exist() + { + var experiment = Repo().GetExperiment(InvalidExperimentId); + + Assert.That(experiment, Is.Null); + } + + [Test] + public void GetVariations_Returns_All_Variations_For_Experiment_When_Id_Exists() + { + var variations = Repo().GetVariations(ValidExperimentId).ToArray(); + + Assert.That(variations.Count(), Is.EqualTo(2)); + Assert.That(variations.All(v => v.ExperimentId == ValidExperimentId), Is.True); + } + + [Test] + public void GetVariations_Returns_Empty_Enumerable_For_Experiment_When_Id_Does_Not_Exist() + { + var variations = Repo().GetVariations(InvalidExperimentId); + + Assert.That(variations.Count(), Is.EqualTo(0)); } + + private IAbTestRepository Repo() => new AbTestRepository(new DefaultDatabaseProvider("GibeCommerce")); + + } } diff --git a/Gibe.AbTest.Tests/App.config b/Gibe.AbTest.Tests/App.config index e24dfdc..960840b 100644 --- a/Gibe.AbTest.Tests/App.config +++ b/Gibe.AbTest.Tests/App.config @@ -1,6 +1,6 @@  - + \ No newline at end of file diff --git a/Gibe.AbTest/AbTestRepository.cs b/Gibe.AbTest/AbTestRepository.cs index ea85457..f28ab28 100644 --- a/Gibe.AbTest/AbTestRepository.cs +++ b/Gibe.AbTest/AbTestRepository.cs @@ -17,7 +17,7 @@ public ExperimentDto GetExperiment(string id) { using (var db = _databaseProvider.GetDatabase()) { - return db.Single("WHERE Id = @0", id); + return db.SingleOrDefault("WHERE Id = @0", id); } } @@ -25,23 +25,15 @@ public IEnumerable GetExperiments() { using (var db = _databaseProvider.GetDatabase()) { - return db.Query("FROM AbExperiment"); + return db.Fetch("FROM AbExperiment"); } } - - public VariationDto GetVariation(int id) - { - using (var db = _databaseProvider.GetDatabase()) - { - return db.Single("WHERE Id = @0", id); - } - } - + public IEnumerable GetVariations(string experimentId) { using (var db = _databaseProvider.GetDatabase()) { - return db.Query("WHERE ExperimentId = @0", experimentId); + return db.Fetch("WHERE ExperimentId = @0", experimentId); } } } diff --git a/Gibe.AbTest/IAbTestRepository.cs b/Gibe.AbTest/IAbTestRepository.cs index dcb8724..b28b09d 100644 --- a/Gibe.AbTest/IAbTestRepository.cs +++ b/Gibe.AbTest/IAbTestRepository.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Security.Cryptography.X509Certificates; using Gibe.AbTest.Dto; namespace Gibe.AbTest @@ -9,10 +8,7 @@ public interface IAbTestRepository { ExperimentDto GetExperiment(string id); IEnumerable GetExperiments(); - VariationDto GetVariation(int id); IEnumerable GetVariations(string experimentId); - - } public class FakeAbTestRepository : IAbTestRepository @@ -41,7 +37,7 @@ public IEnumerable GetExperiments() }; } - public VariationDto GetVariation(int id) + private VariationDto GetVariation(int id) { return new VariationDto { From e98c32112e5766959fd6d2fccc04eebf1cbc3684 Mon Sep 17 00:00:00 2001 From: Steve Temple Date: Wed, 30 Jan 2019 11:33:59 +0000 Subject: [PATCH 05/11] Fixing typos in variable names --- Gibe.AbTest/AbTest.cs | 10 +++++----- Gibe.AbTest/Attributes/NotCachedAttribute.cs | 4 ---- Gibe.AbTest/IAbTestingService.cs | 8 ++++---- Gibe.AbTest/Variation.cs | 15 +++++---------- 4 files changed, 14 insertions(+), 23 deletions(-) diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index f6c7151..3f8a62d 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -22,11 +22,11 @@ public Variation AssignRandomVariation() return RandomlySelectOption(selectedExperiment.Variations); } - public Variation AssignVariation(string experiementKey) + public Variation AssignVariation(string experimentKey) { var experiment = _abTestingService.GetExperiments() .Where(x => x.Enabled) - .First(x => x.Key == experiementKey); + .First(x => x.Key == experimentKey); return RandomlySelectOption(experiment.Variations); } @@ -51,11 +51,11 @@ private T RandomlySelectOption(IEnumerable options) where T : IWeighted var totalWeights = opts.Sum(o => o.Weight); var selectedNumber = _randomNumber.Number(totalWeights); - var currrentWeight = 0; + var currentWeight = 0; foreach (var o in opts) { - currrentWeight += o.Weight; - if (currrentWeight > selectedNumber) + currentWeight += o.Weight; + if (currentWeight > selectedNumber) return o; } return opts.Last(); diff --git a/Gibe.AbTest/Attributes/NotCachedAttribute.cs b/Gibe.AbTest/Attributes/NotCachedAttribute.cs index 78e3af2..d83492d 100644 --- a/Gibe.AbTest/Attributes/NotCachedAttribute.cs +++ b/Gibe.AbTest/Attributes/NotCachedAttribute.cs @@ -1,8 +1,4 @@ using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; namespace Gibe.AbTest.Attributes { diff --git a/Gibe.AbTest/IAbTestingService.cs b/Gibe.AbTest/IAbTestingService.cs index 866cb5d..83d66c7 100644 --- a/Gibe.AbTest/IAbTestingService.cs +++ b/Gibe.AbTest/IAbTestingService.cs @@ -11,10 +11,10 @@ public interface IAbTestingService public class FakeAbTestingService : IAbTestingService { - public IEnumerable Experiements; - public FakeAbTestingService(IEnumerable experiements) + public IEnumerable Experiments; + public FakeAbTestingService(IEnumerable experiments) { - Experiements = experiements; + Experiments = experiments; } public Variation GetVariation(string experimentId, int variationNumber) @@ -33,7 +33,7 @@ public IEnumerable GetVariations(string experimentId) public IEnumerable GetExperiments() { - return Experiements; + return Experiments; } } } diff --git a/Gibe.AbTest/Variation.cs b/Gibe.AbTest/Variation.cs index 5f1b015..ccefa33 100644 --- a/Gibe.AbTest/Variation.cs +++ b/Gibe.AbTest/Variation.cs @@ -1,9 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using Gibe.AbTest.Dto; +using Gibe.AbTest.Dto; using Newtonsoft.Json; namespace Gibe.AbTest @@ -15,7 +10,7 @@ public class Variation : IWeighted public string ExperimentId { get; set; } public int Weight { get; set; } public bool Enabled { get; set; } - public string Defintion { get; set; } + public string Definition { get; set; } public Variation(VariationDto dto) : this(dto.Id, dto.VariationNumber, dto.Weight, dto.Enabled, dto.Definition, dto.ExperimentId) { } @@ -26,12 +21,12 @@ public Variation(int id, int variationNumber, int weight, bool enabled, string d ExperimentId = experimentId; Weight = weight; Enabled = enabled; - Defintion = definition; + Definition = definition; } - + public T GetDefinition() { - return JsonConvert.DeserializeObject(Defintion); + return JsonConvert.DeserializeObject(Definition); } public override string ToString() From 32eebb0715251fb9c65b7fc95786fc7e248a4623 Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Wed, 30 Jan 2019 16:58:46 +0000 Subject: [PATCH 06/11] Move IExperiementService into project and unit tests for behaviour --- Gibe.AbTest.Tests/AbTestTests.cs | 10 +- Gibe.AbTest.Tests/ExperimentServiceTests.cs | 149 +++++++++++++++++++ Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj | 7 + Gibe.AbTest.Tests/packages.config | 2 + Gibe.AbTest/AbTest.cs | 15 +- Gibe.AbTest/AbTestingService.cs | 1 - Gibe.AbTest/DefaultExperimentService.cs | 112 ++++++++++++++ Gibe.AbTest/ExperimentCookieValue.cs | 37 +++++ Gibe.AbTest/ExperimentCookieValueFactory.cs | 24 +++ Gibe.AbTest/Gibe.AbTest.csproj | 8 + Gibe.AbTest/IAbTest.cs | 22 ++- Gibe.AbTest/IExperimentCookieValueFactory.cs | 10 ++ Gibe.AbTest/IExperimentService.cs | 13 ++ Gibe.AbTest/packages.config | 1 + 14 files changed, 393 insertions(+), 18 deletions(-) create mode 100644 Gibe.AbTest.Tests/ExperimentServiceTests.cs create mode 100644 Gibe.AbTest/DefaultExperimentService.cs create mode 100644 Gibe.AbTest/ExperimentCookieValue.cs create mode 100644 Gibe.AbTest/ExperimentCookieValueFactory.cs create mode 100644 Gibe.AbTest/IExperimentCookieValueFactory.cs create mode 100644 Gibe.AbTest/IExperimentService.cs diff --git a/Gibe.AbTest.Tests/AbTestTests.cs b/Gibe.AbTest.Tests/AbTestTests.cs index bf58c15..f0359a8 100644 --- a/Gibe.AbTest.Tests/AbTestTests.cs +++ b/Gibe.AbTest.Tests/AbTestTests.cs @@ -15,18 +15,18 @@ public void Setup() { _abTestingService = new FakeAbTestingService(new List { - new Experiment("Ex1", "Exp1", "Experiement 1", 1, true, DateTime.Now, null, + new Experiment("Ex1", "Exp1", "Experiment 1", 1, true, DateTime.Now, null, new []{ new Variation(1, 0, 1, true, "{Exp1:'Variant 1'}", "Exp1"), new Variation(2, 1, 1, true, "{Exp1:'Variant 2'}", "Exp1") }), - new Experiment("Ex2", "Exp2", "Experiement 2", 1, true, DateTime.Now, null, + new Experiment("Ex2", "Exp2", "Experiment 2", 1, true, DateTime.Now, null, new []{ new Variation(3, 0, 1, true, "{Exp2:'Variant 1'}", "Exp2"), new Variation(4, 1, 1, true, "{Exp2:'Variant 2'}", "Exp2") }), - new Experiment("Ex3", "Exp3", "Experiement 3", 1, false, DateTime.Now, null, + new Experiment("Ex3", "Exp3", "Experiment 3", 1, false, DateTime.Now, null, new []{ new Variation(5, 0, 1, true, "{Exp3:'Variant 1'}", "Exp3"), new Variation(6, 1, 1, true, "{Exp2:'Variant 2'}", "Exp3") @@ -79,7 +79,7 @@ public void AssignVariations_Assigns_First_Experiment_Variation_From_Each_Enable { var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 0, 0 })); - var variations = abTest.AssignVariations().ToList(); + var variations = abTest.AllCurrentVariations().ToList(); Assert.That(_abTestingService.GetExperiments().Where(e => e.Enabled).Select(e => e.Variations.First().Id), Is.EqualTo(variations.Select(v => v.Id))); Assert.That(variations.Count, Is.EqualTo(2)); @@ -93,7 +93,7 @@ public void GetAssignedVariation_returns_variation_from_AbTestingService() var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new int[] {})); - var variation = abTest.AssignedVariation(experimentId, variationNo); + var variation = abTest.Variation(experimentId, variationNo); Assert.AreEqual(experimentId, variation.ExperimentId); Assert.AreEqual(variationNo, variation.VariationNumber); diff --git a/Gibe.AbTest.Tests/ExperimentServiceTests.cs b/Gibe.AbTest.Tests/ExperimentServiceTests.cs new file mode 100644 index 0000000..07bd21d --- /dev/null +++ b/Gibe.AbTest.Tests/ExperimentServiceTests.cs @@ -0,0 +1,149 @@ +using System.Collections.Generic; +using System.Linq; +using Gibe.Cookies; +using Moq; +using NUnit.Framework; + +namespace Gibe.AbTest.Tests +{ + [TestFixture] + public class ExperimentServiceTests + { + private const string CookieKey = "GCEXP"; + + private Mock _cookieService; + private IAbTest _abTest; + + [SetUp] + public void Setup() + { + _cookieService = new Mock(); + _abTest = new FakeAbTest( + new List{ + new Variation(1, 0, 1,true,"{Test:'test1'}", "vapBwUPvTEuGcEVEKThGCA"), + new Variation(2, 1, 1,true,"{Test:'test1'}", "vapBwUPvTEuGcEVEKThGCA"), + new Variation(3, 0, 1,true,"{Test:'test2'}", "vapBwUPvTEuGcEVEKThGCB"), + new Variation(4, 1, 1,true,"{Test:'test2'}", "vapBwUPvTEuGcEVEKThGCB"), + new Variation(5, 0, 1,true,"{Test:'test3'}", "vapBwUPvTEuGcEVEKThGCC"), + new Variation(6, 1, 1,true,"{Test:'test3'}", "vapBwUPvTEuGcEVEKThGCC") + }); + } + + private IExperimentService ExperimentService() + { + return new DefaultExperimentService(_cookieService.Object, _abTest, new ExperimentCookieValueFactory(_abTest)); + } + + [Test] + public void IsCurrentUserInExperiment_Returns_True_When_Cookie_Contains_Experiment() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1"); + + var result = ExperimentService().IsCurrentUserInExperiment(); + + Assert.That(result, Is.True); + } + + [Test] + public void IsCurrentUserInExperiment_Returns_False_When_No_Experiment_Cookie_Found() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns(""); + + var result = ExperimentService().IsCurrentUserInExperiment(); + + Assert.That(result, Is.False); + } + + [Test] + public void CurrentUserVariations_Returns_Enumerable_Of_Variations_Based_On_The_Cookie_Value() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1"); + + var results = ExperimentService().CurrentUserVariations(); + + AssertVariations(results, "vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1"); + } + + [Test] + public void CurrentUserVariation_Returns_Variation_For_ExperiementId_Based_On_The_Cookie_Value() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1"); + + var results = ExperimentService().CurrentUserVariation("vapBwUPvTEuGcEVEKThGCA"); + + AssertVariations(new []{results}, "vapBwUPvTEuGcEVEKThGCA~0"); + } + + [Test] + public void AssignUserVariations_Assigns_User_To_Random_Variations_For_Each_Experiment() + { + + var results = ExperimentService().AssignUserVariations(); + + Assert.That(results.Count(), Is.EqualTo(3)); + } + + [Test] + public void AssignUserVariations_Updates_The_Cookie_Value_For_The_Requested_Variation_Only() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + + var results = ExperimentService().AssignUserVariations("vapBwUPvTEuGcEVEKThGCA~1"); + + AssertVariations(results, "vapBwUPvTEuGcEVEKThGCA~1-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + } + + [Test] + public void AssignUserVariations_Does_Not_Change_The_Cookie_Value_For_The_Requested_Variation_When_The_Same() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + + var results = ExperimentService().AssignUserVariations("vapBwUPvTEuGcEVEKThGCA~0"); + + AssertVariations(results, "vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + } + + [Test] + public void AssignUserVariations_Does_Not_Change_The_Cookie_Value_When_The_Requested_Experiment_Does_Not_Exist() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + + var results = ExperimentService().AssignUserVariations("vapBwUPvTEuGcEVEKThGCD~0"); + + AssertVariations(results, "vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + } + + [Test] + public void AssignCurrentUserToVariation_Does_Not_Change_The_Cookie_Value_When_The_Requested_Variation_Does_Not_Exist() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + + var results = ExperimentService().AssignUserVariations("vapBwUPvTEuGcEVEKThGCA~3"); + + AssertVariations(results, "vapBwUPvTEuGcEVEKThGCA~0-vapBwUPvTEuGcEVEKThGCB~1-vapBwUPvTEuGcEVEKThGCC~0"); + } + + [Test] + public void AssignUserVariations_Assigns_User_To_Experiments_Not_Currently_In_When_Updating_Variation() + { + _cookieService.Setup(s => s.GetValue(It.IsAny())).Returns("vapBwUPvTEuGcEVEKThGCA~0"); + + var results = ExperimentService().AssignUserVariations("vapBwUPvTEuGcEVEKThGCA~1"); + + AssertVariations(results, "vapBwUPvTEuGcEVEKThGCA~1-vapBwUPvTEuGcEVEKThGCB~0-vapBwUPvTEuGcEVEKThGCC~0"); + } + + private static void AssertVariations(IEnumerable results, string variationsString) + { + var variations = variationsString.Split('-') + .Select(e => new Variation(0, int.Parse(e.Split('~')[1]), 1, true, "", e.Split('~')[0])); + + foreach (var variation in variations) + { + Assert.That(results.Any(r => r.ExperimentId == variation.ExperimentId && r.VariationNumber == variation.VariationNumber)); + } + Assert.That(results.Count(), Is.EqualTo(variations.Count())); + } + } +} + diff --git a/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj b/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj index c4747b0..715af39 100644 --- a/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj +++ b/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj @@ -35,10 +35,16 @@ 4 + + ..\packages\Gibe.Cookies.1.0.356\lib\Gibe.Cookies.dll + ..\packages\Gibe.NPoco.1.0.159\lib\Gibe.NPoco.dll True + + ..\packages\Moq.4.2.1502.0911\lib\net40\Moq.dll + ..\packages\NPoco.2.5.77\lib\net40\NPoco.dll True @@ -61,6 +67,7 @@ + diff --git a/Gibe.AbTest.Tests/packages.config b/Gibe.AbTest.Tests/packages.config index bc3117e..d463cc1 100644 --- a/Gibe.AbTest.Tests/packages.config +++ b/Gibe.AbTest.Tests/packages.config @@ -1,6 +1,8 @@  + + \ No newline at end of file diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index 3f8a62d..f785421 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -1,4 +1,6 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Data; using System.Linq; namespace Gibe.AbTest @@ -15,6 +17,11 @@ public AbTest(IAbTestingService abTestingService, _randomNumber = randomNumber; } + public IEnumerable AllExperiments() + { + return _abTestingService.GetExperiments().Where(x => x.Enabled); + } + public Variation AssignRandomVariation() { var experiments = _abTestingService.GetExperiments().Where(x => x.Enabled); @@ -30,16 +37,16 @@ public Variation AssignVariation(string experimentKey) return RandomlySelectOption(experiment.Variations); } - public IEnumerable AssignVariations() + public IEnumerable AllCurrentVariations() { - var experiments = _abTestingService.GetExperiments().Where(x => x.Enabled); + var experiments = _abTestingService.GetExperiments().Where(x => x.Enabled && DateTime.Now > x.StartDate && DateTime.Now < x.EndDate); foreach (var experiment in experiments) { yield return RandomlySelectOption(experiment.Variations); } } - public Variation AssignedVariation(string experimentId, int variationNumber) + public Variation Variation(string experimentId, int variationNumber) { return _abTestingService.GetVariation(experimentId, variationNumber); } diff --git a/Gibe.AbTest/AbTestingService.cs b/Gibe.AbTest/AbTestingService.cs index 72dd448..a99d372 100644 --- a/Gibe.AbTest/AbTestingService.cs +++ b/Gibe.AbTest/AbTestingService.cs @@ -1,6 +1,5 @@ using System.Collections.Generic; using System.Linq; -using Newtonsoft.Json; namespace Gibe.AbTest { diff --git a/Gibe.AbTest/DefaultExperimentService.cs b/Gibe.AbTest/DefaultExperimentService.cs new file mode 100644 index 0000000..117ce14 --- /dev/null +++ b/Gibe.AbTest/DefaultExperimentService.cs @@ -0,0 +1,112 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Gibe.Cookies; + +namespace Gibe.AbTest +{ + public class DefaultExperimentService : IExperimentService + { + private const string CookieKey = "GCEXP"; + private readonly ICookieService _cookieService; + private readonly IAbTest _abTest; + private readonly IExperimentCookieValueFactory _experimentCookieValueFactory; + + public DefaultExperimentService(ICookieService cookieService, + IAbTest abTest, + IExperimentCookieValueFactory experimentCookieValueFactory) + { + _cookieService = cookieService; + _abTest = abTest; + _experimentCookieValueFactory = experimentCookieValueFactory; + } + + //TODO: Not sure we need this, being in no experiments isn't a special case + public bool IsCurrentUserInExperiment() + { + var allVariations = _abTest.AllCurrentVariations().ToList(); + var cookieVariations = CookieVariations(_cookieService.GetValue(CookieKey)).ToList(); + + return allVariations.Any(v => cookieVariations.Any(c => c.ExperimentId == v.ExperimentId)); + } + + public IEnumerable CurrentUserVariations() + { + return CookieVariations(_cookieService.GetValue(CookieKey)); + } + + public Variation CurrentUserVariation(string experimentId) + { + return CookieVariations(_cookieService.GetValue(CookieKey)) + .First(v => v.ExperimentId == experimentId); + } + + public IEnumerable AssignUserVariations() + { + var variationsFromCookie = UserVariationsFromCookie(); + + SaveVariationsCookie(variationsFromCookie); + return variationsFromCookie; + } + + ///experiments/change/?value=vapBwUPvTEuGcEVEKThGCA~0 + public IEnumerable AssignUserVariations(string value) + { + var variationToSet = CookieVariations(value).First(); + var variationsFromCookie = UserVariationsFromCookie(); + + if (variationToSet != null) + { + for (var i = 0; i < variationsFromCookie.Count; i++) + { + if (variationsFromCookie[i].ExperimentId == variationToSet.ExperimentId) + { + variationsFromCookie[i] = variationToSet; + } + } + } + + SaveVariationsCookie(variationsFromCookie); + return variationsFromCookie; + } + + + private List UserVariationsFromCookie() + { + var cookieVariations = CookieVariations(_cookieService.GetValue(CookieKey)).ToList(); + + var allVariations = _abTest.AllCurrentVariations().ToList(); + + foreach (var cookieVariation in cookieVariations) + { + for (var i = 0; i < allVariations.Count; i++) + { + if (allVariations[i].ExperimentId == cookieVariation.ExperimentId) + { + allVariations[i] = cookieVariation; + } + } + } + + return allVariations; + } + + private void SaveVariationsCookie(IEnumerable variations) + { + var variationsCookie = _experimentCookieValueFactory.ExperimentCookieValue(variations); + + _cookieService.Create(CookieKey, variationsCookie.RawValue, DateTime.UtcNow.AddDays(120)); + } + + private IEnumerable CookieVariations(string cookieValue) + { + if (string.IsNullOrEmpty(cookieValue)) + { + return new List(); + } + + return _experimentCookieValueFactory.ExperimentCookieValue(cookieValue) + .Variations(); + } + } +} diff --git a/Gibe.AbTest/ExperimentCookieValue.cs b/Gibe.AbTest/ExperimentCookieValue.cs new file mode 100644 index 0000000..09e7aab --- /dev/null +++ b/Gibe.AbTest/ExperimentCookieValue.cs @@ -0,0 +1,37 @@ +using System.Collections.Generic; +using System.Linq; + +namespace Gibe.AbTest +{ + public class ExperimentCookieValue + { + private readonly IAbTest _abTest; + + private readonly string _rawValue; + + private const string Seperator = "~"; + private const string ExperimentSeperator = "-"; + + public ExperimentCookieValue(IAbTest abTest, string rawValue) + { + _abTest = abTest; + _rawValue = rawValue; + } + + public ExperimentCookieValue(IAbTest abTest, IEnumerable experimentVariantPairs) + { + _abTest = abTest; + _rawValue = string.Join(ExperimentSeperator, experimentVariantPairs.Select(v => + string.Join(Seperator, v.ExperimentId, v.VariationNumber))); + } + + public string RawValue => _rawValue; + + public IEnumerable Variations() + { + return _rawValue.Split(ExperimentSeperator.ToCharArray()) + .Select(e => + _abTest.Variation(e.Split(Seperator.ToCharArray())[0], int.Parse(e.Split(Seperator.ToCharArray())[1]))); + } + } +} diff --git a/Gibe.AbTest/ExperimentCookieValueFactory.cs b/Gibe.AbTest/ExperimentCookieValueFactory.cs new file mode 100644 index 0000000..da1813c --- /dev/null +++ b/Gibe.AbTest/ExperimentCookieValueFactory.cs @@ -0,0 +1,24 @@ +using System.Collections.Generic; + +namespace Gibe.AbTest +{ + public class ExperimentCookieValueFactory : IExperimentCookieValueFactory + { + private readonly IAbTest _abTest; + + public ExperimentCookieValueFactory(IAbTest abTest) + { + _abTest = abTest; + } + + public ExperimentCookieValue ExperimentCookieValue(string rawValue) + { + return new ExperimentCookieValue(_abTest, rawValue); + } + + public ExperimentCookieValue ExperimentCookieValue(IEnumerable variations) + { + return new ExperimentCookieValue(_abTest, variations); + } + } +} diff --git a/Gibe.AbTest/Gibe.AbTest.csproj b/Gibe.AbTest/Gibe.AbTest.csproj index 0fec46d..4256f20 100644 --- a/Gibe.AbTest/Gibe.AbTest.csproj +++ b/Gibe.AbTest/Gibe.AbTest.csproj @@ -34,6 +34,9 @@ ..\packages\Gibe.Caching.1.0.159\lib\Gibe.Caching.dll True + + ..\packages\Gibe.Cookies.1.0.356\lib\Gibe.Cookies.dll + ..\packages\Gibe.NPoco.1.0.159\lib\Gibe.NPoco.dll True @@ -62,11 +65,16 @@ + + + + + diff --git a/Gibe.AbTest/IAbTest.cs b/Gibe.AbTest/IAbTest.cs index 043ab10..cde0e08 100644 --- a/Gibe.AbTest/IAbTest.cs +++ b/Gibe.AbTest/IAbTest.cs @@ -6,10 +6,11 @@ namespace Gibe.AbTest { public interface IAbTest { + IEnumerable AllExperiments(); Variation AssignRandomVariation(); - Variation AssignVariation(string experiementKey); - IEnumerable AssignVariations(); - Variation AssignedVariation(string experimentId, int variationNumber); + Variation AssignVariation(string experimentKey); + IEnumerable AllCurrentVariations(); + Variation Variation(string experimentId, int variationNumber); } public class FakeAbTest : IAbTest @@ -21,24 +22,29 @@ public FakeAbTest(IEnumerable variations) _variations = variations; } + public IEnumerable AllExperiments() + { + return new List();//TODO: chance constructor to take experiments instead of variations + } + public Variation AssignRandomVariation() { return _variations.First(); } - public Variation AssignVariation(string experiementKey) + public Variation AssignVariation(string experimentKey) { - return _variations.First(v => v.ExperimentId == experiementKey); + return _variations.First(v => v.ExperimentId == experimentKey); } - public IEnumerable AssignVariations() + public IEnumerable AllCurrentVariations() { return _variations.GroupBy(v => v.ExperimentId).Select(group => group.First()); } - public Variation AssignedVariation(string experimentId, int variationNumber) + public Variation Variation(string experimentId, int variationNumber) { - return _variations.First(v => v.ExperimentId == experimentId && v.VariationNumber == variationNumber); + return _variations.FirstOrDefault(v => v.ExperimentId == experimentId && v.VariationNumber == variationNumber); } } } diff --git a/Gibe.AbTest/IExperimentCookieValueFactory.cs b/Gibe.AbTest/IExperimentCookieValueFactory.cs new file mode 100644 index 0000000..cbbe159 --- /dev/null +++ b/Gibe.AbTest/IExperimentCookieValueFactory.cs @@ -0,0 +1,10 @@ +using System.Collections.Generic; + +namespace Gibe.AbTest +{ + public interface IExperimentCookieValueFactory + { + ExperimentCookieValue ExperimentCookieValue(IEnumerable variations); + ExperimentCookieValue ExperimentCookieValue(string rawValue); + } +} \ No newline at end of file diff --git a/Gibe.AbTest/IExperimentService.cs b/Gibe.AbTest/IExperimentService.cs new file mode 100644 index 0000000..cc4a52d --- /dev/null +++ b/Gibe.AbTest/IExperimentService.cs @@ -0,0 +1,13 @@ +using System.Collections.Generic; + +namespace Gibe.AbTest +{ + public interface IExperimentService + { + bool IsCurrentUserInExperiment(); + IEnumerable CurrentUserVariations(); + Variation CurrentUserVariation(string experimentId); + IEnumerable AssignUserVariations(); + IEnumerable AssignUserVariations(string value); + } +} diff --git a/Gibe.AbTest/packages.config b/Gibe.AbTest/packages.config index 6a51443..7eae9c9 100644 --- a/Gibe.AbTest/packages.config +++ b/Gibe.AbTest/packages.config @@ -1,6 +1,7 @@  + From efa1320b1660b5526b406d578eba97722eeeae02 Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Tue, 17 Sep 2019 09:41:08 +0100 Subject: [PATCH 07/11] Address PR comments --- Gibe.AbTest/AbTest.cs | 2 +- Gibe.AbTest/AbTestRepository.cs | 3 ++- Gibe.AbTest/DefaultExperimentService.cs | 4 ++-- Gibe.AbTest/ExperimentCookieValue.cs | 12 +++++------- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index f785421..3190115 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -24,7 +24,7 @@ public IEnumerable AllExperiments() public Variation AssignRandomVariation() { - var experiments = _abTestingService.GetExperiments().Where(x => x.Enabled); + var experiments = _abTestingService.GetExperiments(); var selectedExperiment = RandomlySelectOption(experiments); return RandomlySelectOption(selectedExperiment.Variations); } diff --git a/Gibe.AbTest/AbTestRepository.cs b/Gibe.AbTest/AbTestRepository.cs index f28ab28..80b2709 100644 --- a/Gibe.AbTest/AbTestRepository.cs +++ b/Gibe.AbTest/AbTestRepository.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using Gibe.AbTest.Dto; using Gibe.NPoco; @@ -25,7 +26,7 @@ public IEnumerable GetExperiments() { using (var db = _databaseProvider.GetDatabase()) { - return db.Fetch("FROM AbExperiment"); + return db.Fetch("FROM AbExperiment WHERE [Enabled] = 1"); } } diff --git a/Gibe.AbTest/DefaultExperimentService.cs b/Gibe.AbTest/DefaultExperimentService.cs index 117ce14..c3a3621 100644 --- a/Gibe.AbTest/DefaultExperimentService.cs +++ b/Gibe.AbTest/DefaultExperimentService.cs @@ -95,14 +95,14 @@ private void SaveVariationsCookie(IEnumerable variations) { var variationsCookie = _experimentCookieValueFactory.ExperimentCookieValue(variations); - _cookieService.Create(CookieKey, variationsCookie.RawValue, DateTime.UtcNow.AddDays(120)); + _cookieService.Create(CookieKey, variationsCookie.RawValue, DateTime.Now.AddDays(120)); } private IEnumerable CookieVariations(string cookieValue) { if (string.IsNullOrEmpty(cookieValue)) { - return new List(); + return Enumerable.Empty(); } return _experimentCookieValueFactory.ExperimentCookieValue(cookieValue) diff --git a/Gibe.AbTest/ExperimentCookieValue.cs b/Gibe.AbTest/ExperimentCookieValue.cs index 09e7aab..a557018 100644 --- a/Gibe.AbTest/ExperimentCookieValue.cs +++ b/Gibe.AbTest/ExperimentCookieValue.cs @@ -7,29 +7,27 @@ public class ExperimentCookieValue { private readonly IAbTest _abTest; - private readonly string _rawValue; - private const string Seperator = "~"; private const string ExperimentSeperator = "-"; + public string RawValue; + public ExperimentCookieValue(IAbTest abTest, string rawValue) { _abTest = abTest; - _rawValue = rawValue; + RawValue = rawValue; } public ExperimentCookieValue(IAbTest abTest, IEnumerable experimentVariantPairs) { _abTest = abTest; - _rawValue = string.Join(ExperimentSeperator, experimentVariantPairs.Select(v => + RawValue = string.Join(ExperimentSeperator, experimentVariantPairs.Select(v => string.Join(Seperator, v.ExperimentId, v.VariationNumber))); } - public string RawValue => _rawValue; - public IEnumerable Variations() { - return _rawValue.Split(ExperimentSeperator.ToCharArray()) + return RawValue.Split(ExperimentSeperator.ToCharArray()) .Select(e => _abTest.Variation(e.Split(Seperator.ToCharArray())[0], int.Parse(e.Split(Seperator.ToCharArray())[1]))); } From 9149eb9f61e4b50db85620884f7178d23a2f0717 Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Tue, 17 Sep 2019 10:05:36 +0100 Subject: [PATCH 08/11] Move enabled check into repository --- Gibe.AbTest.Tests/AbTestRepositoryTests.cs | 2 +- Gibe.AbTest.Tests/AbTestTests.cs | 10 +++++----- Gibe.AbTest/AbTest.cs | 9 ++++----- Gibe.AbTest/AbTestRepository.cs | 3 +-- Gibe.AbTest/AbTestingService.cs | 4 ++-- Gibe.AbTest/CachingAbTestingService.cs | 6 +++--- Gibe.AbTest/IAbTestRepository.cs | 4 ++-- Gibe.AbTest/IAbTestingService.cs | 4 ++-- 8 files changed, 20 insertions(+), 22 deletions(-) diff --git a/Gibe.AbTest.Tests/AbTestRepositoryTests.cs b/Gibe.AbTest.Tests/AbTestRepositoryTests.cs index 9855de7..d04835a 100644 --- a/Gibe.AbTest.Tests/AbTestRepositoryTests.cs +++ b/Gibe.AbTest.Tests/AbTestRepositoryTests.cs @@ -17,7 +17,7 @@ public class AbTestRepositoryTests [Test] public void GetExperiments_Returns_All_Experiments() { - var experiments = Repo().GetExperiments().ToArray(); + var experiments = Repo().GetEnabledExperiments().ToArray(); Assert.That(experiments.Length, Is.EqualTo(2)); } diff --git a/Gibe.AbTest.Tests/AbTestTests.cs b/Gibe.AbTest.Tests/AbTestTests.cs index f0359a8..6402cfd 100644 --- a/Gibe.AbTest.Tests/AbTestTests.cs +++ b/Gibe.AbTest.Tests/AbTestTests.cs @@ -41,7 +41,7 @@ public void AssignVariation_Assigns_First_Experiment_Variation_When_Random_Numbe var variation = abTest.AssignRandomVariation(); - Assert.AreEqual(_abTestingService.GetExperiments().First().Variations.First().Id, variation.Id); + Assert.AreEqual(_abTestingService.GetEnabledExperiments().First().Variations.First().Id, variation.Id); } [Test] @@ -51,7 +51,7 @@ public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Numb var variation = abTest.AssignRandomVariation(); - Assert.AreEqual(_abTestingService.GetExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); + Assert.AreEqual(_abTestingService.GetEnabledExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); } [Test] @@ -61,7 +61,7 @@ public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Numb var variation = abTest.AssignRandomVariation(); - Assert.AreEqual(_abTestingService.GetExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); + Assert.AreEqual(_abTestingService.GetEnabledExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); } [Test] @@ -71,7 +71,7 @@ public void AssignVariation_Assigns_Given_Experiment_First_Variation() var variation = abTest.AssignVariation("Exp1"); - Assert.That(_abTestingService.GetExperiments().First(x => x.Key == "Exp1").Variations.First().Id, Is.EqualTo(variation.Id)); + Assert.That(_abTestingService.GetEnabledExperiments().First(x => x.Key == "Exp1").Variations.First().Id, Is.EqualTo(variation.Id)); } [Test] @@ -81,7 +81,7 @@ public void AssignVariations_Assigns_First_Experiment_Variation_From_Each_Enable var variations = abTest.AllCurrentVariations().ToList(); - Assert.That(_abTestingService.GetExperiments().Where(e => e.Enabled).Select(e => e.Variations.First().Id), Is.EqualTo(variations.Select(v => v.Id))); + Assert.That(_abTestingService.GetEnabledExperiments().Where(e => e.Enabled).Select(e => e.Variations.First().Id), Is.EqualTo(variations.Select(v => v.Id))); Assert.That(variations.Count, Is.EqualTo(2)); } diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index 3190115..95147f2 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -19,27 +19,26 @@ public AbTest(IAbTestingService abTestingService, public IEnumerable AllExperiments() { - return _abTestingService.GetExperiments().Where(x => x.Enabled); + return _abTestingService.GetEnabledExperiments(); } public Variation AssignRandomVariation() { - var experiments = _abTestingService.GetExperiments(); + var experiments = _abTestingService.GetEnabledExperiments(); var selectedExperiment = RandomlySelectOption(experiments); return RandomlySelectOption(selectedExperiment.Variations); } public Variation AssignVariation(string experimentKey) { - var experiment = _abTestingService.GetExperiments() - .Where(x => x.Enabled) + var experiment = _abTestingService.GetEnabledExperiments() .First(x => x.Key == experimentKey); return RandomlySelectOption(experiment.Variations); } public IEnumerable AllCurrentVariations() { - var experiments = _abTestingService.GetExperiments().Where(x => x.Enabled && DateTime.Now > x.StartDate && DateTime.Now < x.EndDate); + var experiments = _abTestingService.GetEnabledExperiments().Where(x => DateTime.Now > x.StartDate && DateTime.Now < x.EndDate); foreach (var experiment in experiments) { yield return RandomlySelectOption(experiment.Variations); diff --git a/Gibe.AbTest/AbTestRepository.cs b/Gibe.AbTest/AbTestRepository.cs index 80b2709..4618209 100644 --- a/Gibe.AbTest/AbTestRepository.cs +++ b/Gibe.AbTest/AbTestRepository.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Linq; using Gibe.AbTest.Dto; using Gibe.NPoco; @@ -22,7 +21,7 @@ public ExperimentDto GetExperiment(string id) } } - public IEnumerable GetExperiments() + public IEnumerable GetEnabledExperiments() { using (var db = _databaseProvider.GetDatabase()) { diff --git a/Gibe.AbTest/AbTestingService.cs b/Gibe.AbTest/AbTestingService.cs index a99d372..f8ce6a3 100644 --- a/Gibe.AbTest/AbTestingService.cs +++ b/Gibe.AbTest/AbTestingService.cs @@ -12,9 +12,9 @@ public AbTestingService(IAbTestRepository abTestRepository) _abTestRepository = abTestRepository; } - public IEnumerable GetExperiments() + public IEnumerable GetEnabledExperiments() { - return _abTestRepository.GetExperiments() + return _abTestRepository.GetEnabledExperiments() .Select(x => new Experiment(x, GetVariations(x.Id).ToArray())); } diff --git a/Gibe.AbTest/CachingAbTestingService.cs b/Gibe.AbTest/CachingAbTestingService.cs index 797c72f..43b601f 100644 --- a/Gibe.AbTest/CachingAbTestingService.cs +++ b/Gibe.AbTest/CachingAbTestingService.cs @@ -26,16 +26,16 @@ public Variation GetVariation(string experimentId, int variationNumber) public IEnumerable GetVariations(string experimentId) { - return GetExperiments().First(x => x.Id == experimentId).Variations; + return GetEnabledExperiments().First(x => x.Id == experimentId).Variations; } - public IEnumerable GetExperiments() + public IEnumerable GetEnabledExperiments() { if (_cache.Exists(CacheKey)) { return _cache.Get(CacheKey); } - var experiments = _abTestingService.GetExperiments().ToArray(); + var experiments = _abTestingService.GetEnabledExperiments().ToArray(); _cache.Add(CacheKey, experiments, TimeSpan.FromMinutes(15)); return experiments; } diff --git a/Gibe.AbTest/IAbTestRepository.cs b/Gibe.AbTest/IAbTestRepository.cs index b28b09d..68ea65e 100644 --- a/Gibe.AbTest/IAbTestRepository.cs +++ b/Gibe.AbTest/IAbTestRepository.cs @@ -7,7 +7,7 @@ namespace Gibe.AbTest public interface IAbTestRepository { ExperimentDto GetExperiment(string id); - IEnumerable GetExperiments(); + IEnumerable GetEnabledExperiments(); IEnumerable GetVariations(string experimentId); } @@ -27,7 +27,7 @@ public ExperimentDto GetExperiment(string id) }; } - public IEnumerable GetExperiments() + public IEnumerable GetEnabledExperiments() { return new List { diff --git a/Gibe.AbTest/IAbTestingService.cs b/Gibe.AbTest/IAbTestingService.cs index 83d66c7..d36ca58 100644 --- a/Gibe.AbTest/IAbTestingService.cs +++ b/Gibe.AbTest/IAbTestingService.cs @@ -6,7 +6,7 @@ public interface IAbTestingService { Variation GetVariation(string experimentId, int variationNumber); IEnumerable GetVariations(string experimentId); - IEnumerable GetExperiments(); + IEnumerable GetEnabledExperiments(); } public class FakeAbTestingService : IAbTestingService @@ -31,7 +31,7 @@ public IEnumerable GetVariations(string experimentId) }; } - public IEnumerable GetExperiments() + public IEnumerable GetEnabledExperiments() { return Experiments; } From c69153fea74fabfc7b7a8d7448b190b4eea64c92 Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Tue, 17 Sep 2019 12:00:46 +0100 Subject: [PATCH 09/11] Tidy up tests after merge --- Gibe.AbTest.Tests/AbTestRepositoryTests.cs | 6 +-- Gibe.AbTest.Tests/AbTestTests.cs | 50 +++++++++++++--------- Gibe.AbTest/AbTest.cs | 23 +++++----- Gibe.AbTest/IAbTest.cs | 10 +---- Gibe.AbTest/IAbTestingService.cs | 3 +- 5 files changed, 50 insertions(+), 42 deletions(-) diff --git a/Gibe.AbTest.Tests/AbTestRepositoryTests.cs b/Gibe.AbTest.Tests/AbTestRepositoryTests.cs index d04835a..db46ba9 100644 --- a/Gibe.AbTest.Tests/AbTestRepositoryTests.cs +++ b/Gibe.AbTest.Tests/AbTestRepositoryTests.cs @@ -11,15 +11,15 @@ namespace Gibe.AbTest.Tests [TestFixture] public class AbTestRepositoryTests { - private const string ValidExperimentId = "vapBwUPvTEuGcEVEKThGCA"; + private const string ValidExperimentId = "sq0S2zdKQhWEmBc6sQ4sfQ"; private const string InvalidExperimentId = "NotRealId"; [Test] - public void GetExperiments_Returns_All_Experiments() + public void GetEnabledExperiments_Returns_All_Enabled_Experiments() { var experiments = Repo().GetEnabledExperiments().ToArray(); - Assert.That(experiments.Length, Is.EqualTo(2)); + Assert.That(experiments.Length, Is.EqualTo(1)); } [Test] diff --git a/Gibe.AbTest.Tests/AbTestTests.cs b/Gibe.AbTest.Tests/AbTestTests.cs index 0b6740c..dbb6e43 100644 --- a/Gibe.AbTest.Tests/AbTestTests.cs +++ b/Gibe.AbTest.Tests/AbTestTests.cs @@ -11,6 +11,7 @@ public class AbTestTests private const string MobileUserAgent = "Mozilla/5.0 (Android; Mobile; rv:13.0) Gecko/13.0 Firefox/13.0"; private const string DesktopUserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0"; private IAbTestingService _abTestingService; + private IAbTestingService _simpleAbTestingService; [SetUp] public void Setup() @@ -21,17 +22,25 @@ public void Setup() new []{ new Variation(1, 0, 1, true, "{Exp1:'Variant 1'}", "Exp1", false), new Variation(2, 1, 1, true, "{Exp1:'Variant 2'}", "Exp1", false) - }), new Experiment("Ex2", "Exp2", "Experiment 2", 1, true, DateTime.Now, null, new []{ new Variation(3, 0, 1, true, "{Exp2:'Variant 1'}", "Exp2", false), - new Variation(4, 1, 1, true, "{Exp2:'Variant 2'}", "Exp2", false) + new Variation(4, 1, 1, true, "{Exp2:'Variant 2'}", "Exp2", true) }), new Experiment("Ex3", "Exp3", "Experiment 3", 1, false, DateTime.Now, null, new []{ - new Variation(5, 0, 1, true, "{Exp3:'Variant 1'}", "Exp3", false), - new Variation(6, 1, 1, true, "{Exp2:'Variant 2'}", "Exp3", false) + new Variation(5, 0, 1, true, "{Exp3:'Variant 1'}", "Exp3", true), + new Variation(6, 1, 1, true, "{Exp3:'Variant 2'}", "Exp3", true) + }) + }); + + _simpleAbTestingService = new FakeAbTestingService(new List + { + new Experiment("Ex1", "Exp1", "Experiment 1", 1, true, DateTime.Now, null, + new []{ + new Variation(1, 0, 1, true, "{Exp1:'Variant 1'}", "Exp1", false), + new Variation(2, 1, 1, true, "{Exp1:'Variant 2'}", "Exp1", false) }) }); } @@ -39,37 +48,38 @@ public void Setup() [Test] public void AssignVariation_Assigns_First_Experiment_Variation_When_Random_Number_Is_0() { - var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new [] { 0, 0 })); + var abTest = new AbTest(_simpleAbTestingService, new FakeRandomNumber(new [] { 0, 0 })); - var variation = abTest.AssignVariation(MobileUserAgent); - Assert.AreEqual(_abTestingService.GetEnabledExperiments().First().Variations.First().Id, variation.Id); + var variation = abTest.AssignRandomVariation(DesktopUserAgent); + Assert.AreEqual(_simpleAbTestingService.GetEnabledExperiments().First().Variations.First().Id, variation.Id); } [Test] public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Number_Is_1() { - var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 1, 1 })); - var variation = abTest.AssignRandomVariation(); + var abTest = new AbTest(_simpleAbTestingService, new FakeRandomNumber(new[] { 1, 1 })); + + var variation = abTest.AssignRandomVariation(DesktopUserAgent); - Assert.AreEqual(_abTestingService.GetEnabledExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); + Assert.AreEqual(_simpleAbTestingService.GetEnabledExperiments().First().Variations.ElementAt(1).Id, variation.Id); } [Test] public void AssignVariation_Assigns_Second_Experiment_Variation_When_Random_Number_Is_2() { - var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 2, 2 })); + var abTest = new AbTest(_simpleAbTestingService, new FakeRandomNumber(new[] { 2, 2 })); - var variation = abTest.AssignVariation(MobileUserAgent); - Assert.AreEqual(_abTestingService.GetEnabledExperiments().ElementAt(1).Variations.ElementAt(1).Id, variation.Id); + var variation = abTest.AssignRandomVariation(DesktopUserAgent); + Assert.AreEqual(_simpleAbTestingService.GetEnabledExperiments().First().Variations.ElementAt(1).Id, variation.Id); } [Test] public void AssignVariation_Assigns_Given_Experiment_First_Variation() { - var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 0, 0 })); + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 0, 0, 0 })); - var variation = abTest.AssignVariation("Exp1"); + var variation = abTest.AssignVariationByExperimentKey("Exp1"); Assert.That(_abTestingService.GetEnabledExperiments().First(x => x.Key == "Exp1").Variations.First().Id, Is.EqualTo(variation.Id)); } @@ -77,9 +87,9 @@ public void AssignVariation_Assigns_Given_Experiment_First_Variation() [Test] public void AssignVariation_does_not_assign_mobile_user_to_desktop_variant() { - var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 1, 1 })); + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 1, 1, 1 })); - var variation = abTest.AssignVariation(MobileUserAgent); + var variation = abTest.AssignRandomVariation(MobileUserAgent); Assert.IsFalse(variation.DesktopOnly); } @@ -87,9 +97,9 @@ public void AssignVariation_does_not_assign_mobile_user_to_desktop_variant() [Test] public void AssignVariation_assigns_desktop_user_to_desktop_variant() { - var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 1, 1 })); + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 1, 1, 1 })); - var variation = abTest.AssignVariation(DesktopUserAgent); + var variation = abTest.AssignRandomVariation(DesktopUserAgent); Assert.IsTrue(variation.DesktopOnly); } @@ -97,7 +107,7 @@ public void AssignVariation_assigns_desktop_user_to_desktop_variant() [Test] public void AssignVariations_Assigns_First_Experiment_Variation_From_Each_Enabled_Experiment() { - var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 0, 0 })); + var abTest = new AbTest(_abTestingService, new FakeRandomNumber(new[] { 0, 0, 0 })); var variations = abTest.AllCurrentVariations().ToList(); diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index e4d4179..4dcb69e 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -22,17 +22,10 @@ public IEnumerable AllExperiments() return _abTestingService.GetEnabledExperiments(); } - public Variation AssignRandomVariation() + public Variation AssignRandomVariation(string userAgent) { var experiments = _abTestingService.GetEnabledExperiments(); - var selectedExperiment = RandomlySelectOption(experiments); - return RandomlySelectOption(selectedExperiment.Variations); - } - - public Variation AssignVariation(string userAgent) - { - var experiments = _abTestingService.GetEnabledExperiments(); - var selectedExperiment = RandomlySelectOption(experiments); + var selectedExperiment = RandomlySelectOption(FilterExperiments(experiments, userAgent)); return RandomlySelectOption(FilterVariations(selectedExperiment.Variations, userAgent)); } @@ -45,7 +38,7 @@ public Variation AssignVariationByExperimentKey(string experimentKey) public IEnumerable AllCurrentVariations() { - var experiments = _abTestingService.GetEnabledExperiments().Where(x => DateTime.Now > x.StartDate && DateTime.Now < x.EndDate); + var experiments = _abTestingService.GetEnabledExperiments().Where(x => (DateTime.Now >= x.StartDate || x.StartDate == null ) && (DateTime.Now < x.EndDate || x.EndDate == null)); foreach (var experiment in experiments) { yield return RandomlySelectOption(experiment.Variations); @@ -57,6 +50,16 @@ public Variation Variation(string experimentId, int variationNumber) return _abTestingService.GetVariation(experimentId, variationNumber); } + private IEnumerable FilterExperiments(IEnumerable experments, string userAgent) + { + var filtered = experments.Where(e => e.Variations.Any(v => v.DesktopOnly) && !userAgent.Contains("Mobi") || !e.Variations.All(v => v.DesktopOnly)); + if (!filtered.Any()) + { + return experments.Take(1); //TODO: We should not return anything if there are no correct matches, this requires a refactor to use IEnumerables everywhere + } + return filtered; + } + private IEnumerable FilterVariations(IEnumerable variations, string userAgent) { var filtered = variations.Where(v => v.DesktopOnly && !userAgent.Contains("Mobi") || !v.DesktopOnly); diff --git a/Gibe.AbTest/IAbTest.cs b/Gibe.AbTest/IAbTest.cs index ff6c5c0..2f55acf 100644 --- a/Gibe.AbTest/IAbTest.cs +++ b/Gibe.AbTest/IAbTest.cs @@ -6,8 +6,7 @@ namespace Gibe.AbTest public interface IAbTest { IEnumerable AllExperiments(); - Variation AssignRandomVariation(); - Variation AssignVariation(string userAgent); + Variation AssignRandomVariation(string userAgent); Variation AssignVariationByExperimentKey(string experimentKey); IEnumerable AllCurrentVariations(); Variation Variation(string experimentId, int variationNumber); @@ -27,12 +26,7 @@ public IEnumerable AllExperiments() return new List();//TODO: chance constructor to take experiments instead of variations } - public Variation AssignRandomVariation() - { - return _variations.First(); - } - - public Variation AssignVariation(string userAgent) + public Variation AssignRandomVariation(string userAgent) { return _variations.First(); } diff --git a/Gibe.AbTest/IAbTestingService.cs b/Gibe.AbTest/IAbTestingService.cs index f842f67..eca2753 100644 --- a/Gibe.AbTest/IAbTestingService.cs +++ b/Gibe.AbTest/IAbTestingService.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; namespace Gibe.AbTest { @@ -40,7 +41,7 @@ public IEnumerable GetVariations(string experimentId) public IEnumerable GetEnabledExperiments() { - return Experiments; + return Experiments.Where(e => e.Enabled); } } } From 9e616bf57f718aa1ee74060d98b8ac3635c80c7b Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Tue, 17 Sep 2019 12:25:30 +0100 Subject: [PATCH 10/11] Move date check into repository --- Gibe.AbTest/AbTest.cs | 3 ++- Gibe.AbTest/AbTestRepository.cs | 9 +++++++-- Gibe.AbTest/AbTestingService.cs | 3 ++- Gibe.AbTest/IAbTestingService.cs | 3 +-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Gibe.AbTest/AbTest.cs b/Gibe.AbTest/AbTest.cs index 4dcb69e..3edcdfc 100644 --- a/Gibe.AbTest/AbTest.cs +++ b/Gibe.AbTest/AbTest.cs @@ -38,7 +38,7 @@ public Variation AssignVariationByExperimentKey(string experimentKey) public IEnumerable AllCurrentVariations() { - var experiments = _abTestingService.GetEnabledExperiments().Where(x => (DateTime.Now >= x.StartDate || x.StartDate == null ) && (DateTime.Now < x.EndDate || x.EndDate == null)); + var experiments = _abTestingService.GetEnabledExperiments(); foreach (var experiment in experiments) { yield return RandomlySelectOption(experiment.Variations); @@ -70,6 +70,7 @@ private IEnumerable FilterVariations(IEnumerable variation return filtered; } + private T RandomlySelectOption(IEnumerable options) where T : IWeighted { var opts = options.ToArray(); diff --git a/Gibe.AbTest/AbTestRepository.cs b/Gibe.AbTest/AbTestRepository.cs index 4618209..677f708 100644 --- a/Gibe.AbTest/AbTestRepository.cs +++ b/Gibe.AbTest/AbTestRepository.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using Gibe.AbTest.Dto; using Gibe.NPoco; @@ -25,7 +26,11 @@ public IEnumerable GetEnabledExperiments() { using (var db = _databaseProvider.GetDatabase()) { - return db.Fetch("FROM AbExperiment WHERE [Enabled] = 1"); + var now = DateTime.Now.ToString("yyyy-MM-dd HH:mm:ss"); + return db.Fetch($"FROM AbExperiment " + + $"WHERE [Enabled] = 1 " + + $"AND '{now}' >= [StartDate] OR StartDate IS NULL " + + $"AND '{now}' < [EndDate] OR StartDate IS NULL "); } } diff --git a/Gibe.AbTest/AbTestingService.cs b/Gibe.AbTest/AbTestingService.cs index f8ce6a3..a738719 100644 --- a/Gibe.AbTest/AbTestingService.cs +++ b/Gibe.AbTest/AbTestingService.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; namespace Gibe.AbTest diff --git a/Gibe.AbTest/IAbTestingService.cs b/Gibe.AbTest/IAbTestingService.cs index eca2753..f842f67 100644 --- a/Gibe.AbTest/IAbTestingService.cs +++ b/Gibe.AbTest/IAbTestingService.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Linq; namespace Gibe.AbTest { @@ -41,7 +40,7 @@ public IEnumerable GetVariations(string experimentId) public IEnumerable GetEnabledExperiments() { - return Experiments.Where(e => e.Enabled); + return Experiments; } } } From 2f98352eaddfcaac442d30d1671dc117b05eeee1 Mon Sep 17 00:00:00 2001 From: Thomas Adams Date: Thu, 20 May 2021 14:45:30 +0100 Subject: [PATCH 11/11] Merge master --- Gibe.AbTest.Tests/AbTestingServiceTests.cs | 93 ++++++++++++++++++++++ Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj | 1 + Gibe.AbTest/AbTestingService.cs | 51 ++++++++++-- Gibe.AbTest/IAbTestRepository.cs | 50 +++++------- Gibe.AbTest/IAbTestingService.cs | 3 +- 5 files changed, 159 insertions(+), 39 deletions(-) create mode 100644 Gibe.AbTest.Tests/AbTestingServiceTests.cs diff --git a/Gibe.AbTest.Tests/AbTestingServiceTests.cs b/Gibe.AbTest.Tests/AbTestingServiceTests.cs new file mode 100644 index 0000000..03e3aa2 --- /dev/null +++ b/Gibe.AbTest.Tests/AbTestingServiceTests.cs @@ -0,0 +1,93 @@ +using System.Collections.Generic; +using System.Linq; +using Gibe.AbTest.Dto; +using NUnit.Framework; + +namespace Gibe.AbTest.Tests +{ + [TestFixture] + public class AbTestingServiceTests + { + private IAbTestingService Service(IAbTestRepository repository) + { + return new AbTestingService(repository); + } + + [Test] + public void GetExperiments_Returns_Empty_Experiment_When_No_Experiments() + { + var fakeAbTestingService = new FakeAbTestRepository(new List(), new List()); + + var experiments = Service(fakeAbTestingService).GetEnabledExperiments(); + + var expected = new Experiment(new ExperimentDto + { + Id = "", + Key = "", + Description = "", + StartDate = new System.DateTime(2019, 01, 01), + EndDate = null, + Weight = 1, + Enabled = true + }, new[] { new Variation(0, 1, 1, true, "", "", false) }); + + AssertExperiment(experiments.First(), expected); + } + + [Test] + public void GetVariation_Returns_Empty_Variation_When_No_Experiments() + { + var fakeAbTestingService = new FakeAbTestRepository(new List(), new List()); + + var variation = Service(fakeAbTestingService).GetVariation("exp1", 1); + + var expected = new Variation(0, 1, 1, true, "", "", false); + + + AssertVariation(variation, expected); + } + + [Test] + public void GetVariations_Returns_Empty_Variation_When_No_Experiments() + { + var fakeAbTestingService = new FakeAbTestRepository(new List(), new List()); + + var variations = Service(fakeAbTestingService).GetVariations("exp1"); + + var expected = new Variation(0, 1, 1, true, "", "", false); + + AssertVariation(variations.First(), expected); + Assert.That(variations.Count(), Is.EqualTo(1)); + } + + private void AssertExperiment(Experiment actual, Experiment expected) + { + Assert.That(actual.Id, Is.EqualTo(expected.Id)); + Assert.That(actual.Key, Is.EqualTo(expected.Key)); + Assert.That(actual.Description, Is.EqualTo(expected.Description)); + Assert.That(actual.StartDate, Is.EqualTo(expected.StartDate)); + Assert.That(actual.EndDate, Is.EqualTo(expected.EndDate)); + Assert.That(actual.Weight, Is.EqualTo(expected.Weight)); + Assert.That(actual.Enabled, Is.EqualTo(expected.Enabled)); + + for (var i = 0; i < actual.Variations.Count(); i++) + { + var actualVariation = actual.Variations.ElementAt(i); + var expectedVariation = expected.Variations.ElementAt(i); + AssertVariation(actualVariation, expectedVariation); + } + } + + private void AssertVariation(Variation actual, Variation expected) + { + Assert.That(actual.Id, Is.EqualTo(expected.Id)); + Assert.That(actual.VariationNumber, Is.EqualTo(expected.VariationNumber)); + Assert.That(actual.Weight, Is.EqualTo(expected.Weight)); + Assert.That(actual.Enabled, Is.EqualTo(expected.Enabled)); + Assert.That(actual.Definition, Is.EqualTo(expected.Definition)); + Assert.That(actual.ExperimentId, Is.EqualTo(expected.ExperimentId)); + Assert.That(actual.DesktopOnly, Is.EqualTo(expected.DesktopOnly)); + } + + } +} diff --git a/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj b/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj index 715af39..af42eaa 100644 --- a/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj +++ b/Gibe.AbTest.Tests/Gibe.AbTest.Tests.csproj @@ -65,6 +65,7 @@ + diff --git a/Gibe.AbTest/AbTestingService.cs b/Gibe.AbTest/AbTestingService.cs index a738719..1d657f8 100644 --- a/Gibe.AbTest/AbTestingService.cs +++ b/Gibe.AbTest/AbTestingService.cs @@ -1,4 +1,5 @@ using System; +using Gibe.AbTest.Dto; using System.Collections.Generic; using System.Linq; @@ -15,21 +16,59 @@ public AbTestingService(IAbTestRepository abTestRepository) public IEnumerable GetEnabledExperiments() { - return _abTestRepository.GetEnabledExperiments() + var experiments = _abTestRepository.GetEnabledExperiments() .Select(x => new Experiment(x, GetVariations(x.Id).ToArray())); + + if (experiments.Any()) + { + return experiments; + } + + return new[] { EmptyExperiment() }; } public IEnumerable GetVariations(string experimentId) { - return _abTestRepository.GetVariations(experimentId) - .Select(v => new Variation(v)); + var variations = _abTestRepository.GetVariations(experimentId); + + if (variations.Any()) + { + return variations.Select(v => new Variation(v)); + } + + return new[] { EmptyVariation() }; } public Variation GetVariation(string experimentId, int variationNumber) { - var dto = _abTestRepository.GetVariations(experimentId) - .First(v => v.VariationNumber == variationNumber); - return new Variation(dto); + var variation = _abTestRepository.GetVariations(experimentId) + .FirstOrDefault(v => v.VariationNumber == variationNumber); + + if (variation != null) + { + return new Variation(variation); + } + + return EmptyVariation(); + } + + private Experiment EmptyExperiment() + { + return new Experiment(new Dto.ExperimentDto + { + Id = "", + Key = "", + Description = "", + StartDate = new System.DateTime(2019, 01, 01), + EndDate = null, + Weight = 1, + Enabled = true + }, new[] { EmptyVariation() }); + } + + private Variation EmptyVariation() + { + return new Variation(0, 1, 1, true, "", "", false); } } } diff --git a/Gibe.AbTest/IAbTestRepository.cs b/Gibe.AbTest/IAbTestRepository.cs index 68ea65e..a54b32d 100644 --- a/Gibe.AbTest/IAbTestRepository.cs +++ b/Gibe.AbTest/IAbTestRepository.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Security.Cryptography.X509Certificates; using Gibe.AbTest.Dto; namespace Gibe.AbTest @@ -9,55 +11,39 @@ public interface IAbTestRepository ExperimentDto GetExperiment(string id); IEnumerable GetEnabledExperiments(); IEnumerable GetVariations(string experimentId); + + } public class FakeAbTestRepository : IAbTestRepository { - + private IEnumerable _experiments; + private IEnumerable _variations; + + public FakeAbTestRepository(IEnumerable experiments, IEnumerable variations) + { + _experiments = experiments; + _variations = variations; + } + public ExperimentDto GetExperiment(string id) { - return new ExperimentDto - { - Id = id, - Enabled = true, - StartDate = DateTime.Now, - EndDate = null, - Key = "ABC123", - Weight = 1 - }; + return _experiments.First(); } public IEnumerable GetEnabledExperiments() { - return new List - { - GetExperiment("ABC"), - GetExperiment("DEF"), - GetExperiment("GHI") - }; + return _experiments; } private VariationDto GetVariation(int id) { - return new VariationDto - { - Id = id, - VariationNumber = 0, - Definition = "{Test:'test'}", - Enabled = true, - ExperimentId = "ABC", - Weight = 1 - }; + return _variations.First(); } - + public IEnumerable GetVariations(string experimentId) { - return new List - { - GetVariation(1), - GetVariation(2), - GetVariation(3) - }; + return _variations; } } } diff --git a/Gibe.AbTest/IAbTestingService.cs b/Gibe.AbTest/IAbTestingService.cs index f842f67..eca2753 100644 --- a/Gibe.AbTest/IAbTestingService.cs +++ b/Gibe.AbTest/IAbTestingService.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; namespace Gibe.AbTest { @@ -40,7 +41,7 @@ public IEnumerable GetVariations(string experimentId) public IEnumerable GetEnabledExperiments() { - return Experiments; + return Experiments.Where(e => e.Enabled); } } }