Repository: pheature-flags/pheature-flags (php)

Issue:
[toggle-core] Add id and type to strategy and segment read models
**Epic:** https://github.com/pheature-flags/pheature-flags/issues/1 Toggle Core
**Package:** https://github.com/pheature-flags/toggle-core 

**Description:**

Update read model classes to allow reading strategy and segment id and types.

Pull request description:
[#217] add id and type to strategy and segment read models
Closes #217 

Patch:
diff --git a/packages/toggle-core/src/Read/Segment.php b/packages/toggle-core/src/Read/Segment.php
index 79b6e1d..da0be1a 100644
--- a/packages/toggle-core/src/Read/Segment.php
+++ b/packages/toggle-core/src/Read/Segment.php
@@ -9,6 +9,7 @@ use JsonSerializable;
 interface Segment extends JsonSerializable
 {
     public function id(): string;
+    public function type(): string;
     /**
      * @return array<string, mixed>
      */
diff --git a/packages/toggle-core/src/Read/ToggleStrategy.php b/packages/toggle-core/src/Read/ToggleStrategy.php
index cf4f57c..436d05f 100644
--- a/packages/toggle-core/src/Read/ToggleStrategy.php
+++ b/packages/toggle-core/src/Read/ToggleStrategy.php
@@ -8,6 +8,8 @@ use JsonSerializable;
 
 interface ToggleStrategy extends JsonSerializable
 {
+    public function id(): string;
+    public function type(): string;
     public function isSatisfiedBy(ConsumerIdentity $identity): bool;
     /**
      * @return array<string, string|array>
diff --git a/packages/toggle-model/src/EnableByMatchingIdentityId.php b/packages/toggle-model/src/EnableByMatchingIdentityId.php
index 47474a4..87d28c8 100644
--- a/packages/toggle-model/src/EnableByMatchingIdentityId.php
+++ b/packages/toggle-model/src/EnableByMatchingIdentityId.php
@@ -13,9 +13,10 @@ use Pheature\Core\Toggle\Read\ToggleStrategy;
 final class EnableByMatchingIdentityId implements ToggleStrategy
 {
     public const NAME = 'enable_by_matching_identity_id';
+    private string $id;
     private Segments $segments;
 
-    public function __construct(Segments $segments)
+    public function __construct(string $id, Segments $segments)
     {
         foreach ($segments->all() as $segment) {
             if (false === $segment instanceof IdentitySegment) {
@@ -25,9 +26,20 @@ final class EnableByMatchingIdentityId implements ToggleStrategy
                 ));
             }
         }
+        $this->id = $id;
         $this->segments = $segments;
     }
 
+    public function id(): string
+    {
+        return $this->id;
+    }
+
+    public function type(): string
+    {
+        return self::NAME;
+    }
+
     public function isSatisfiedBy(ConsumerIdentity $identity): bool
     {
         foreach ($this->segments->all() as $segment) {
@@ -42,6 +54,7 @@ final class EnableByMatchingIdentityId implements ToggleStrategy
     public function toArray(): array
     {
         return [
+            'id' => $this->id,
             'type' => self::NAME,
             'segments' => array_map(
                 static fn(ISegment $segment): array => $segment->toArray(),
diff --git a/packages/toggle-model/src/EnableByMatchingSegment.php b/packages/toggle-model/src/EnableByMatchingSegment.php
index 8343fdc..6d19400 100644
--- a/packages/toggle-model/src/EnableByMatchingSegment.php
+++ b/packages/toggle-model/src/EnableByMatchingSegment.php
@@ -12,13 +12,25 @@ use Pheature\Core\Toggle\Read\ToggleStrategy;
 final class EnableByMatchingSegment implements ToggleStrategy
 {
     public const NAME = 'enable_by_matching_segment';
+    private string $id;
     private Segments $segments;
 
-    public function __construct(Segments $segments)
+    public function __construct(string $id, Segments $segments)
     {
+        $this->id = $id;
         $this->segments = $segments;
     }
 
+    public function id(): string
+    {
+        return $this->id;
+    }
+
+    public function type(): string
+    {
+        return self::NAME;
+    }
+
     public function isSatisfiedBy(ConsumerIdentity $identity): bool
     {
         foreach ($this->segments->all() as $segment) {
@@ -36,6 +48,7 @@ final class EnableByMatchingSegment implements ToggleStrategy
     public function toArray(): array
     {
         return [
+            'id' => $this->id,
             'type' => self::NAME,
             'segments' => array_map(
                 static fn(ISegment $segment): array => $segment->toArray(),
diff --git a/packages/toggle-model/src/IdentitySegment.php b/packages/toggle-model/src/IdentitySegment.php
index 39f13f1..ad25163 100644
--- a/packages/toggle-model/src/IdentitySegment.php
+++ b/packages/toggle-model/src/IdentitySegment.php
@@ -32,6 +32,11 @@ final class IdentitySegment implements Segment
         return $this->id;
     }
 
+    public function type(): string
+    {
+        return self::NAME;
+    }
+
     /**
      * @return array<string, mixed>
      */
@@ -59,6 +64,7 @@ final class IdentitySegment implements Segment
     {
         return [
             'id' => $this->id,
+            'type' => self::NAME,
             'criteria' => $this->criteria,
         ];
     }
diff --git a/packages/toggle-model/src/InCollectionMatchingSegment.php b/packages/toggle-model/src/InCollectionMatchingSegment.php
index e4e8904..e1fdec9 100644
--- a/packages/toggle-model/src/InCollectionMatchingSegment.php
+++ b/packages/toggle-model/src/InCollectionMatchingSegment.php
@@ -30,6 +30,11 @@ class InCollectionMatchingSegment implements Segment
         return $this->id;
     }
 
+    public function type(): string
+    {
+        return self::NAME;
+    }
+
     public function criteria(): array
     {
         return $this->criteria;
@@ -73,6 +78,7 @@ class InCollectionMatchingSegment implements Segment
     {
         return [
             'id' => $this->id,
+            'type' => self::NAME,
             'criteria' => $this->criteria,
         ];
     }
diff --git a/packages/toggle-model/src/StrategyFactory.php b/packages/toggle-model/src/StrategyFactory.php
index 0bfcf5d..2841a1c 100644
--- a/packages/toggle-model/src/StrategyFactory.php
+++ b/packages/toggle-model/src/StrategyFactory.php
@@ -15,10 +15,10 @@ final class StrategyFactory implements ToggleStrategyFactory
     {
         $segments = $segments ?? new Segments();
         if (EnableByMatchingSegment::NAME === $strategyType) {
-            return new EnableByMatchingSegment($segments);
+            return new EnableByMatchingSegment($strategyId, $segments);
         }
         if (EnableByMatchingIdentityId::NAME === $strategyType) {
-            return new EnableByMatchingIdentityId($segments);
+            return new EnableByMatchingIdentityId($strategyId, $segments);
         }
 
         throw InvalidStrategyTypeGiven::withType($strategyType);
diff --git a/packages/toggle-model/src/StrictMatchingSegment.php b/packages/toggle-model/src/StrictMatchingSegment.php
index 565cecd..a1bab29 100644
--- a/packages/toggle-model/src/StrictMatchingSegment.php
+++ b/packages/toggle-model/src/StrictMatchingSegment.php
@@ -32,6 +32,11 @@ final class StrictMatchingSegment implements Segment
         return $this->id;
     }
 
+    public function type(): string
+    {
+        return self::NAME;
+    }
+
     /**
      * @return array<string, mixed>
      */
@@ -66,6 +71,7 @@ final class StrictMatchingSegment implements Segment
     {
         return [
             'id' => $this->id,
+            'type' => self::NAME,
             'criteria' => $this->criteria,
         ];
     }


Test patch:
diff --git a/packages/toggle-model/test/EnableByMatchingIdentityIdTest.php b/packages/toggle-model/test/EnableByMatchingIdentityIdTest.php
index df10052..178d154 100644
--- a/packages/toggle-model/test/EnableByMatchingIdentityIdTest.php
+++ b/packages/toggle-model/test/EnableByMatchingIdentityIdTest.php
@@ -14,12 +14,14 @@ use PHPUnit\Framework\TestCase;
 
 final class EnableByMatchingIdentityIdTest extends TestCase
 {
+    private const STRATEGY_ID = 'our_strategy';
+
     public function testItShouldOnlyAcceptIdentitySegments(): void
     {
         $this->expectException(InvalidArgumentException::class);
 
         $segments = new Segments(new StrictMatchingSegment('some_segments', []));
-        new EnableByMatchingIdentityId($segments);
+        new EnableByMatchingIdentityId(self::STRATEGY_ID, $segments);
     }
 
     public function testItShouldBeSatisfiedByMatchingIdentitySegment(): void
@@ -28,15 +30,17 @@ final class EnableByMatchingIdentityIdTest extends TestCase
             'some_id',
         ]));
 
-        $strategy = new EnableByMatchingIdentityId($segments);
+        $strategy = new EnableByMatchingIdentityId(self::STRATEGY_ID, $segments);
         self::assertTrue($strategy->isSatisfiedBy(new Identity('some_id')));
+        self::assertSame(self::STRATEGY_ID, $strategy->id());
+        self::assertSame('enable_by_matching_identity_id', $strategy->type());
     }
 
     public function testItShouldNotBeSatisfiedWithoutAnyIdentitySegment(): void
     {
         $segments = new Segments();
 
-        $strategy = new EnableByMatchingIdentityId($segments);
+        $strategy = new EnableByMatchingIdentityId(self::STRATEGY_ID, $segments);
         self::assertFalse($strategy->isSatisfiedBy(new Identity('some_id', [
             'some_id',
         ])));
@@ -48,7 +52,7 @@ final class EnableByMatchingIdentityIdTest extends TestCase
             'some_id',
         ]));
 
-        $strategy = new EnableByMatchingIdentityId($segments);
+        $strategy = new EnableByMatchingIdentityId(self::STRATEGY_ID, $segments);
         self::assertFalse($strategy->isSatisfiedBy(new Identity('some_other_id')));
     }
 
@@ -58,12 +62,14 @@ final class EnableByMatchingIdentityIdTest extends TestCase
             'some_id',
         ]));
 
-        $strategy = new EnableByMatchingIdentityId($segments);
+        $strategy = new EnableByMatchingIdentityId(self::STRATEGY_ID, $segments);
         self::assertSame([
+            'id' => 'our_strategy',
             'type' => EnableByMatchingIdentityId::NAME,
             'segments' => [
                 [
                     'id'  => 'developers',
+                    'type'  => 'identity_segment',
                     'criteria' => ['some_id']
                 ]
             ],
diff --git a/packages/toggle-model/test/EnableByMatchingSegmentTest.php b/packages/toggle-model/test/EnableByMatchingSegmentTest.php
index f0c15e7..5aab23e 100644
--- a/packages/toggle-model/test/EnableByMatchingSegmentTest.php
+++ b/packages/toggle-model/test/EnableByMatchingSegmentTest.php
@@ -12,13 +12,17 @@ use PHPUnit\Framework\TestCase;
 
 final class EnableByMatchingSegmentTest extends TestCase
 {
+    private const STRATEGY_ID = 'an_strategy';
+
     /** @dataProvider getSegmentsCriteria */
     public function testItShouldBeSatisfiedByMatchingSegment(array $criteria): void
     {
         $segments = new Segments(new StrictMatchingSegment('users_from_barcelona', $criteria));
 
-        $strategy = new EnableByMatchingSegment($segments);
+        $strategy = new EnableByMatchingSegment(self::STRATEGY_ID, $segments);
         self::assertTrue($strategy->isSatisfiedBy(new Identity('some_id', $criteria)));
+        self::assertSame(self::STRATEGY_ID, $strategy->id());
+        self::assertSame('enable_by_matching_segment', $strategy->type());
     }
 
     public function testItShouldNotBeSatisfiedByUnMatchingSegment(): void
@@ -27,7 +31,7 @@ final class EnableByMatchingSegmentTest extends TestCase
             'location' => 'barcelona',
         ]));
 
-        $strategy = new EnableByMatchingSegment($segments);
+        $strategy = new EnableByMatchingSegment(self::STRATEGY_ID, $segments);
         self::assertFalse($strategy->isSatisfiedBy(new Identity('some_id', [
             'location' => 'bilbo',
         ])));
@@ -37,7 +41,7 @@ final class EnableByMatchingSegmentTest extends TestCase
     {
         $segments = new Segments();
 
-        $strategy = new EnableByMatchingSegment($segments);
+        $strategy = new EnableByMatchingSegment(self::STRATEGY_ID, $segments);
         self::assertFalse($strategy->isSatisfiedBy(new Identity('some_id', [
             'location' => 'bilbo',
         ])));
@@ -47,7 +51,7 @@ final class EnableByMatchingSegmentTest extends TestCase
     {
         $segments = new Segments(new StrictMatchingSegment('users_from_barcelona', []));
 
-        $strategy = new EnableByMatchingSegment($segments);
+        $strategy = new EnableByMatchingSegment(self::STRATEGY_ID, $segments);
         self::assertFalse($strategy->isSatisfiedBy(new Identity('some_id', [
             'location' => 'bilbo',
         ])));
@@ -59,12 +63,14 @@ final class EnableByMatchingSegmentTest extends TestCase
             'location' => 'barcelona',
         ]));
 
-        $strategy = new EnableByMatchingSegment($segments);
+        $strategy = new EnableByMatchingSegment(self::STRATEGY_ID, $segments);
         self::assertSame([
+            'id' => 'an_strategy',
             'type' => EnableByMatchingSegment::NAME,
             'segments' => [
                 [
                     'id'  => 'users_from_barcelona',
+                    'type' => 'strict_matching_segment',
                     'criteria' => [
                         'location' => 'barcelona',
                     ],
diff --git a/packages/toggle-model/test/IdentitySegmentTest.php b/packages/toggle-model/test/IdentitySegmentTest.php
new file mode 100644
index 0000000..18a3733
--- /dev/null
+++ b/packages/toggle-model/test/IdentitySegmentTest.php
@@ -0,0 +1,86 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pheature\Test\Model\Toggle;
+
+use Generator;
+use Pheature\Model\Toggle\IdentitySegment;
+use PHPUnit\Framework\TestCase;
+
+final class IdentitySegmentTest extends TestCase
+{
+    private const SEGMENT_ID = 'a_segment_id';
+
+    /** @dataProvider nonMatchingPayloads */
+    public function testItShouldNotMatch(array $criteria, array $payload): void
+    {
+        $segment = new IdentitySegment(self::SEGMENT_ID, $criteria);
+
+        self::assertFalse($segment->match($payload));
+        self::assertSame(self::SEGMENT_ID, $segment->id());
+        self::assertSame('identity_segment', $segment->type());
+        self::assertSame($criteria, $segment->criteria());
+        self::assertSame([
+            'id' => self::SEGMENT_ID,
+            'type' => 'identity_segment',
+            'criteria' => $criteria,
+        ], $segment->jsonSerialize());
+    }
+
+    /** @dataProvider matchingPayloads */
+    public function testItShouldMatch(array $criteria, array $payload): void
+    {
+        $segment = new IdentitySegment(self::SEGMENT_ID, $criteria);
+
+        self::assertTrue($segment->match($payload));
+        self::assertSame(self::SEGMENT_ID, $segment->id());
+        self::assertSame('identity_segment', $segment->type());
+        self::assertSame($criteria, $segment->criteria());
+        self::assertSame([
+            'id' => self::SEGMENT_ID,
+            'type' => 'identity_segment',
+            'criteria' => $criteria,
+        ], $segment->jsonSerialize());
+    }
+
+    public function nonMatchingPayloads(): Generator
+    {
+        yield 'no criteria present' => [
+            'criteria' => [],
+            'payload' => [
+                'identity_id' => 'some_identity',
+            ]
+        ];
+
+        yield 'no identity present' => [
+            'criteria' => [
+                'identity_id' => 'some_identity',
+            ],
+            'payload' => [
+                'other' => 'some_identity',
+            ]
+        ];
+
+        yield 'no matching identity_id present' => [
+            'criteria' => [
+                'identity_id' => ['some_identity'],
+            ],
+            'payload' => [
+                'identity_id' => 'some_other_identity',
+            ]
+        ];
+    }
+
+    public function matchingPayloads(): Generator
+    {
+        yield 'matching identity_id present' => [
+            'criteria' => [
+                'identity_id' => 'some_identity',
+            ],
+            'payload' => [
+                'identity_id' => 'some_identity',
+            ]
+        ];
+    }
+}
diff --git a/packages/toggle-model/test/InCollectionMatchingSegmentTest.php b/packages/toggle-model/test/InCollectionMatchingSegmentTest.php
index d7317a1..fd69eed 100644
--- a/packages/toggle-model/test/InCollectionMatchingSegmentTest.php
+++ b/packages/toggle-model/test/InCollectionMatchingSegmentTest.php
@@ -16,6 +16,14 @@ class InCollectionMatchingSegmentTest extends TestCase
         $segment = new InCollectionMatchingSegment(self::SEGMENT_ID, $criteria);
 
         self::assertFalse($segment->match($payload));
+        self::assertSame(self::SEGMENT_ID, $segment->id());
+        self::assertSame('in_collection_matching_segment', $segment->type());
+        self::assertSame($criteria, $segment->criteria());
+        self::assertSame([
+            'id' => self::SEGMENT_ID,
+            'type' => 'in_collection_matching_segment',
+            'criteria' => $criteria,
+        ], $segment->jsonSerialize());
     }
 
     /** @dataProvider matchingPayloads */
@@ -24,6 +32,14 @@ class InCollectionMatchingSegmentTest extends TestCase
         $segment = new InCollectionMatchingSegment(self::SEGMENT_ID, $criteria);
 
         self::assertTrue($segment->match($payload));
+        self::assertSame(self::SEGMENT_ID, $segment->id());
+        self::assertSame('in_collection_matching_segment', $segment->type());
+        self::assertSame($criteria, $segment->criteria());
+        self::assertSame([
+            'id' => self::SEGMENT_ID,
+            'type' => 'in_collection_matching_segment',
+            'criteria' => $criteria,
+        ], $segment->jsonSerialize());
     }
 
     public function nonMatchingPayloads(): Generator
diff --git a/packages/toggle-model/test/StrictMatchingSegmentTest.php b/packages/toggle-model/test/StrictMatchingSegmentTest.php
new file mode 100644
index 0000000..2e84bec
--- /dev/null
+++ b/packages/toggle-model/test/StrictMatchingSegmentTest.php
@@ -0,0 +1,116 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pheature\Test\Model\Toggle;
+
+use Generator;
+use Pheature\Model\Toggle\StrictMatchingSegment;
+use PHPUnit\Framework\TestCase;
+
+final class StrictMatchingSegmentTest extends TestCase
+{
+    private const SEGMENT_ID = 'a_segment_id';
+
+    /** @dataProvider nonMatchingPayloads */
+    public function testItShouldNotMatch(array $criteria, array $payload): void
+    {
+        $segment = new StrictMatchingSegment(self::SEGMENT_ID, $criteria);
+
+        self::assertFalse($segment->match($payload));
+        self::assertSame(self::SEGMENT_ID, $segment->id());
+        self::assertSame('strict_matching_segment', $segment->type());
+        self::assertSame($criteria, $segment->criteria());
+        self::assertSame([
+            'id' => self::SEGMENT_ID,
+            'type' => 'strict_matching_segment',
+            'criteria' => $criteria,
+        ], $segment->jsonSerialize());
+    }
+
+    /** @dataProvider matchingPayloads */
+    public function testItShouldMatch(array $criteria, array $payload): void
+    {
+        $segment = new StrictMatchingSegment(self::SEGMENT_ID, $criteria);
+
+        self::assertTrue($segment->match($payload));
+        self::assertSame(self::SEGMENT_ID, $segment->id());
+        self::assertSame('strict_matching_segment', $segment->type());
+        self::assertSame($criteria, $segment->criteria());
+        self::assertSame([
+            'id' => self::SEGMENT_ID,
+            'type' => 'strict_matching_segment',
+            'criteria' => $criteria,
+        ], $segment->jsonSerialize());
+    }
+
+    public function nonMatchingPayloads(): Generator
+    {
+        yield 'no criteria present' => [
+            'criteria' => [],
+            'payload' => [
+                'location' => 'girona',
+            ]
+        ];
+
+        yield 'different value comparison' => [
+            'criteria' => [
+                'location' => 'barcelona',
+            ],
+            'payload' => [
+                'location' => 'girona',
+            ]
+        ];
+
+        yield 'different key comparison' => [
+            'criteria' => [
+                'location' => 'barcelona',
+            ],
+            'payload' => [
+                'city' => 'barcelona',
+            ]
+        ];
+        yield 'strict matching multiple criteria with multiple fields' => [
+            'criteria' => [
+                'user_type' => 'top',
+                'location' => 'barcelona',
+            ],
+            'payload' => [
+                'location' => 'barcelona',
+                'user_type' => 'default',
+            ],
+        ];
+
+    }
+
+    public function matchingPayloads(): Generator
+    {
+        yield 'strict matching criteria' => [
+            'criteria' => [
+                'location' => 'barcelona',
+            ],
+            'payload' => [
+                'location' => 'barcelona',
+            ],
+        ];
+        yield 'strict matching criteria with multiple fields' => [
+            'criteria' => [
+                'location' => 'barcelona',
+            ],
+            'payload' => [
+                'location' => 'barcelona',
+                'user_type' => 'top',
+            ],
+        ];
+        yield 'strict matching multiple criteria with multiple fields' => [
+            'criteria' => [
+                'user_type' => 'top',
+                'location' => 'barcelona',
+            ],
+            'payload' => [
+                'location' => 'barcelona',
+                'user_type' => 'top',
+            ],
+        ];
+    }
+}