From 32089e3b86275b3f5718f1a0d0914e1dbbf16091 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 21 Jul 2025 20:45:03 +0000 Subject: [PATCH 1/8] Improve user ID handling in DevCycleUser from EvaluationContext Co-authored-by: jonathan --- .phpunit.cache/test-results | 1 + lib/Model/DevCycleUser.php | 29 +++++++++---- test/Api/OpenFeatureTest.php | 79 +++++++++++++++++++++++++++++++++++- 3 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 .phpunit.cache/test-results diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results new file mode 100644 index 0000000..971196f --- /dev/null +++ b/.phpunit.cache/test-results @@ -0,0 +1 @@ +{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.301,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.109,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.102,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.111,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.153}} \ No newline at end of file diff --git a/lib/Model/DevCycleUser.php b/lib/Model/DevCycleUser.php index d63c195..b545872 100644 --- a/lib/Model/DevCycleUser.php +++ b/lib/Model/DevCycleUser.php @@ -713,18 +713,33 @@ public function toHeaderValue(): string public static function FromEvaluationContext(EvaluationContext $context): DevCycleUser { $user = new DevCycleUser(); - if ($context->getTargetingKey() === null && $context->getAttributes()->get("user_id") === null) { - throw new \InvalidArgumentException('targetingKey or user_id is missing from EvaluationContext'); - } - if ($context->getAttributes()->get("user_id") !== null) { - $userId = $context->getAttributes()->get("user_id"); - } else { + + // Priority order: targetingKey -> user_id -> userId + $userId = null; + $usedUserIdSource = null; + + if ($context->getTargetingKey() !== null) { $userId = $context->getTargetingKey(); + $usedUserIdSource = 'targetingKey'; + } elseif ($context->getAttributes()->get("user_id") !== null) { + $userId = $context->getAttributes()->get("user_id"); + $usedUserIdSource = 'user_id'; + } elseif ($context->getAttributes()->get("userId") !== null) { + $userId = $context->getAttributes()->get("userId"); + $usedUserIdSource = 'userId'; + } + + if ($userId === null) { + throw new \InvalidArgumentException('targetingKey, user_id, or userId is missing from EvaluationContext'); } + $user->setUserId($userId); foreach ($context->getAttributes()->toArray() as $key => $value) { - if ($key === 'user_id' || $key === 'targetingKey') { + // Skip the field that was used as the main user ID to avoid duplication in custom data + if ($key === 'targetingKey' || + ($key === 'user_id' && $usedUserIdSource === 'user_id') || + ($key === 'userId' && $usedUserIdSource === 'userId')) { continue; } switch ($key) { diff --git a/test/Api/OpenFeatureTest.php b/test/Api/OpenFeatureTest.php index 006272b..abac418 100644 --- a/test/Api/OpenFeatureTest.php +++ b/test/Api/OpenFeatureTest.php @@ -27,22 +27,97 @@ final class OpenFeatureTest extends TestCase public function testEvaluationContextTargetingKey() { - + // Test targetingKey only $evalContextTK = new EvaluationContext(targetingKey: "test"); $user = DevCycleUser::FromEvaluationContext($evalContextTK); self::assertEquals("test", $user->getUserId(), 'User ID not properly passed through/set'); + // Test user_id only $attributes = new Attributes(array("user_id" => "test")); $evaluationContext = new EvaluationContext(attributes: $attributes); $user = DevCycleUser::FromEvaluationContext($evaluationContext); self::assertEquals("test", $user->getUserId(), 'User ID not properly passed through/set'); + // Test userId only + $attributes = new Attributes(array("userId" => "test")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("test", $user->getUserId(), 'User ID not properly passed through/set'); + + // Test priority: targetingKey should take precedence over user_id $attributes = new Attributes(array("user_id" => "test")); $evaluationContext = new EvaluationContext(targetingKey: "test2", attributes: $attributes); $user = DevCycleUser::FromEvaluationContext($evaluationContext); - self::assertEquals("test", $user->getUserId(), 'User ID not properly passed through/set'); + self::assertEquals("test2", $user->getUserId(), 'targetingKey should take precedence over user_id'); + + // Test priority: targetingKey should take precedence over userId + $attributes = new Attributes(array("userId" => "test")); + $evaluationContext = new EvaluationContext(targetingKey: "test2", attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("test2", $user->getUserId(), 'targetingKey should take precedence over userId'); + + // Test priority: user_id should take precedence over userId + $attributes = new Attributes(array("user_id" => "test", "userId" => "test2")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("test", $user->getUserId(), 'user_id should take precedence over userId'); + // Test all three present: targetingKey should win + $attributes = new Attributes(array("user_id" => "test", "userId" => "test2")); + $evaluationContext = new EvaluationContext(targetingKey: "test3", attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("test3", $user->getUserId(), 'targetingKey should take precedence over all other user ID fields'); } + + public function testUserIdFieldsNotInCustomData() + { + // Test that when user_id is used as main user ID, it doesn't appear in custom data + $attributes = new Attributes(array("user_id" => "test-user", "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("test-user", $user->getUserId(), 'user_id should be used as main user ID'); + self::assertArrayNotHasKey("user_id", $user->getCustomData(), 'user_id should not appear in custom data when used as main user ID'); + self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); + + // Test that when userId is used as main user ID, it doesn't appear in custom data + $attributes = new Attributes(array("userId" => "test-user", "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("test-user", $user->getUserId(), 'userId should be used as main user ID'); + self::assertArrayNotHasKey("userId", $user->getCustomData(), 'userId should not appear in custom data when used as main user ID'); + self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); + + // Test that when targetingKey is used, both user_id and userId can appear in custom data + $attributes = new Attributes(array("user_id" => "test-user-id", "userId" => "test-userId", "other_field" => "value")); + $evaluationContext = new EvaluationContext(targetingKey: "targeting-key", attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("targeting-key", $user->getUserId(), 'targetingKey should be used as main user ID'); + self::assertArrayHasKey("user_id", $user->getCustomData(), 'user_id should appear in custom data when not used as main user ID'); + self::assertArrayHasKey("userId", $user->getCustomData(), 'userId should appear in custom data when not used as main user ID'); + self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); + + // Test that when user_id is used as main user ID, unused userId can appear in custom data + $attributes = new Attributes(array("user_id" => "test-user-id", "userId" => "test-userId", "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + self::assertEquals("test-user-id", $user->getUserId(), 'user_id should be used as main user ID'); + self::assertArrayNotHasKey("user_id", $user->getCustomData(), 'user_id should not appear in custom data when used as main user ID'); + self::assertArrayHasKey("userId", $user->getCustomData(), 'userId should appear in custom data when not used as main user ID'); + self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); + } + + public function testMissingUserIdThrowsException() + { + // Test that missing all user ID fields throws exception + $attributes = new Attributes(array("other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('targetingKey, user_id, or userId is missing from EvaluationContext'); + + DevCycleUser::FromEvaluationContext($evaluationContext); + } + public function testEvaluationContext() { $attributes = new Attributes(array( From f30973ada90645359170488bd4fa1c7636ab6eb2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Jul 2025 15:16:06 +0000 Subject: [PATCH 2/8] Remove user ID fields from custom data to prevent duplication Co-authored-by: jonathan --- .phpunit.cache/test-results | 2 +- lib/Model/DevCycleUser.php | 10 ++-------- test/Api/OpenFeatureTest.php | 12 ++++++------ 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results index 971196f..1030834 100644 --- a/.phpunit.cache/test-results +++ b/.phpunit.cache/test-results @@ -1 +1 @@ -{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.301,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.109,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.102,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.111,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.153}} \ No newline at end of file +{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172}} \ No newline at end of file diff --git a/lib/Model/DevCycleUser.php b/lib/Model/DevCycleUser.php index b545872..4187cc5 100644 --- a/lib/Model/DevCycleUser.php +++ b/lib/Model/DevCycleUser.php @@ -716,17 +716,13 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc // Priority order: targetingKey -> user_id -> userId $userId = null; - $usedUserIdSource = null; if ($context->getTargetingKey() !== null) { $userId = $context->getTargetingKey(); - $usedUserIdSource = 'targetingKey'; } elseif ($context->getAttributes()->get("user_id") !== null) { $userId = $context->getAttributes()->get("user_id"); - $usedUserIdSource = 'user_id'; } elseif ($context->getAttributes()->get("userId") !== null) { $userId = $context->getAttributes()->get("userId"); - $usedUserIdSource = 'userId'; } if ($userId === null) { @@ -736,10 +732,8 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc $user->setUserId($userId); foreach ($context->getAttributes()->toArray() as $key => $value) { - // Skip the field that was used as the main user ID to avoid duplication in custom data - if ($key === 'targetingKey' || - ($key === 'user_id' && $usedUserIdSource === 'user_id') || - ($key === 'userId' && $usedUserIdSource === 'userId')) { + // Always skip all user ID field types to avoid duplication in custom data + if ($key === 'targetingKey' || $key === 'user_id' || $key === 'userId') { continue; } switch ($key) { diff --git a/test/Api/OpenFeatureTest.php b/test/Api/OpenFeatureTest.php index abac418..6fba526 100644 --- a/test/Api/OpenFeatureTest.php +++ b/test/Api/OpenFeatureTest.php @@ -87,22 +87,22 @@ public function testUserIdFieldsNotInCustomData() self::assertArrayNotHasKey("userId", $user->getCustomData(), 'userId should not appear in custom data when used as main user ID'); self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); - // Test that when targetingKey is used, both user_id and userId can appear in custom data + // Test that all user ID fields are always excluded from custom data $attributes = new Attributes(array("user_id" => "test-user-id", "userId" => "test-userId", "other_field" => "value")); $evaluationContext = new EvaluationContext(targetingKey: "targeting-key", attributes: $attributes); $user = DevCycleUser::FromEvaluationContext($evaluationContext); self::assertEquals("targeting-key", $user->getUserId(), 'targetingKey should be used as main user ID'); - self::assertArrayHasKey("user_id", $user->getCustomData(), 'user_id should appear in custom data when not used as main user ID'); - self::assertArrayHasKey("userId", $user->getCustomData(), 'userId should appear in custom data when not used as main user ID'); + self::assertArrayNotHasKey("user_id", $user->getCustomData(), 'user_id should never appear in custom data'); + self::assertArrayNotHasKey("userId", $user->getCustomData(), 'userId should never appear in custom data'); self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); - // Test that when user_id is used as main user ID, unused userId can appear in custom data + // Test that all user ID fields are excluded even when not used as main user ID $attributes = new Attributes(array("user_id" => "test-user-id", "userId" => "test-userId", "other_field" => "value")); $evaluationContext = new EvaluationContext(attributes: $attributes); $user = DevCycleUser::FromEvaluationContext($evaluationContext); self::assertEquals("test-user-id", $user->getUserId(), 'user_id should be used as main user ID'); - self::assertArrayNotHasKey("user_id", $user->getCustomData(), 'user_id should not appear in custom data when used as main user ID'); - self::assertArrayHasKey("userId", $user->getCustomData(), 'userId should appear in custom data when not used as main user ID'); + self::assertArrayNotHasKey("user_id", $user->getCustomData(), 'user_id should never appear in custom data'); + self::assertArrayNotHasKey("userId", $user->getCustomData(), 'userId should never appear in custom data'); self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); } From 97f183a90e9ba63386512189a3c9316a82e63f8d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Jul 2025 15:20:05 +0000 Subject: [PATCH 3/8] Validate user ID types in OpenFeature context and improve error handling Co-authored-by: jonathan --- .phpunit.cache/test-results | 2 +- lib/Model/DevCycleUser.php | 8 ++++---- test/Api/OpenFeatureTest.php | 25 ++++++++++++++++++++++++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results index 1030834..d45bd49 100644 --- a/.phpunit.cache/test-results +++ b/.phpunit.cache/test-results @@ -1 +1 @@ -{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172}} \ No newline at end of file +{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesThrowException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesSkippedInPriority":0}} \ No newline at end of file diff --git a/lib/Model/DevCycleUser.php b/lib/Model/DevCycleUser.php index 4187cc5..71107aa 100644 --- a/lib/Model/DevCycleUser.php +++ b/lib/Model/DevCycleUser.php @@ -717,16 +717,16 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc // Priority order: targetingKey -> user_id -> userId $userId = null; - if ($context->getTargetingKey() !== null) { + if ($context->getTargetingKey() !== null && ValueTypeValidator::isString($context->getTargetingKey())) { $userId = $context->getTargetingKey(); - } elseif ($context->getAttributes()->get("user_id") !== null) { + } elseif ($context->getAttributes()->get("user_id") !== null && ValueTypeValidator::isString($context->getAttributes()->get("user_id"))) { $userId = $context->getAttributes()->get("user_id"); - } elseif ($context->getAttributes()->get("userId") !== null) { + } elseif ($context->getAttributes()->get("userId") !== null && ValueTypeValidator::isString($context->getAttributes()->get("userId"))) { $userId = $context->getAttributes()->get("userId"); } if ($userId === null) { - throw new \InvalidArgumentException('targetingKey, user_id, or userId is missing from EvaluationContext'); + throw new \InvalidArgumentException('targetingKey, user_id, or userId is missing from EvaluationContext or is not a valid string'); } $user->setUserId($userId); diff --git a/test/Api/OpenFeatureTest.php b/test/Api/OpenFeatureTest.php index 6fba526..55960e3 100644 --- a/test/Api/OpenFeatureTest.php +++ b/test/Api/OpenFeatureTest.php @@ -113,11 +113,34 @@ public function testMissingUserIdThrowsException() $evaluationContext = new EvaluationContext(attributes: $attributes); $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('targetingKey, user_id, or userId is missing from EvaluationContext'); + $this->expectExceptionMessage('targetingKey, user_id, or userId is missing from EvaluationContext or is not a valid string'); DevCycleUser::FromEvaluationContext($evaluationContext); } + public function testInvalidUserIdTypesThrowException() + { + // Test that non-string user_id throws exception + $attributes = new Attributes(array("user_id" => 123, "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('targetingKey, user_id, or userId is missing from EvaluationContext or is not a valid string'); + + DevCycleUser::FromEvaluationContext($evaluationContext); + } + + public function testInvalidUserIdTypesSkippedInPriority() + { + // Test that invalid types are skipped and next valid type is used + $attributes = new Attributes(array("user_id" => 123, "userId" => "valid-user-id", "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + $user = DevCycleUser::FromEvaluationContext($evaluationContext); + + self::assertEquals("valid-user-id", $user->getUserId(), 'userId should be used when user_id is invalid type'); + self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); + } + public function testEvaluationContext() { $attributes = new Attributes(array( From 88f7cac5bdd848495095a7ecf777b6c09709c88f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Jul 2025 15:30:05 +0000 Subject: [PATCH 4/8] Improve user ID validation and error handling in DevCycleUser Co-authored-by: jonathan --- .phpunit.cache/test-results | 2 +- lib/Model/DevCycleUser.php | 12 ++++++++---- test/Api/OpenFeatureTest.php | 21 +++++---------------- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results index d45bd49..78f613c 100644 --- a/.phpunit.cache/test-results +++ b/.phpunit.cache/test-results @@ -1 +1 @@ -{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesThrowException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesSkippedInPriority":0}} \ No newline at end of file +{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":7},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesThrowException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesSkippedInPriority":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":0.003,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypeThrowsException":0}} \ No newline at end of file diff --git a/lib/Model/DevCycleUser.php b/lib/Model/DevCycleUser.php index 71107aa..411e5e5 100644 --- a/lib/Model/DevCycleUser.php +++ b/lib/Model/DevCycleUser.php @@ -717,16 +717,20 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc // Priority order: targetingKey -> user_id -> userId $userId = null; - if ($context->getTargetingKey() !== null && ValueTypeValidator::isString($context->getTargetingKey())) { + if ($context->getTargetingKey() !== null) { $userId = $context->getTargetingKey(); - } elseif ($context->getAttributes()->get("user_id") !== null && ValueTypeValidator::isString($context->getAttributes()->get("user_id"))) { + } elseif ($context->getAttributes()->get("user_id") !== null) { $userId = $context->getAttributes()->get("user_id"); - } elseif ($context->getAttributes()->get("userId") !== null && ValueTypeValidator::isString($context->getAttributes()->get("userId"))) { + } elseif ($context->getAttributes()->get("userId") !== null) { $userId = $context->getAttributes()->get("userId"); } if ($userId === null) { - throw new \InvalidArgumentException('targetingKey, user_id, or userId is missing from EvaluationContext or is not a valid string'); + throw new \InvalidArgumentException('targetingKey, user_id, or userId is missing from EvaluationContext'); + } + + if (!ValueTypeValidator::isString($userId)) { + throw new \InvalidArgumentException('User ID must be a string value'); } $user->setUserId($userId); diff --git a/test/Api/OpenFeatureTest.php b/test/Api/OpenFeatureTest.php index 55960e3..e00edab 100644 --- a/test/Api/OpenFeatureTest.php +++ b/test/Api/OpenFeatureTest.php @@ -113,34 +113,23 @@ public function testMissingUserIdThrowsException() $evaluationContext = new EvaluationContext(attributes: $attributes); $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('targetingKey, user_id, or userId is missing from EvaluationContext or is not a valid string'); + $this->expectExceptionMessage('targetingKey, user_id, or userId is missing from EvaluationContext'); DevCycleUser::FromEvaluationContext($evaluationContext); } - public function testInvalidUserIdTypesThrowException() + public function testInvalidUserIdTypeThrowsException() { - // Test that non-string user_id throws exception - $attributes = new Attributes(array("user_id" => 123, "other_field" => "value")); + // Test that non-string user_id throws string validation exception + $attributes = new Attributes(array("user_id" => array("invalid", "array"), "other_field" => "value")); $evaluationContext = new EvaluationContext(attributes: $attributes); $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('targetingKey, user_id, or userId is missing from EvaluationContext or is not a valid string'); + $this->expectExceptionMessage('User ID must be a string value'); DevCycleUser::FromEvaluationContext($evaluationContext); } - public function testInvalidUserIdTypesSkippedInPriority() - { - // Test that invalid types are skipped and next valid type is used - $attributes = new Attributes(array("user_id" => 123, "userId" => "valid-user-id", "other_field" => "value")); - $evaluationContext = new EvaluationContext(attributes: $attributes); - $user = DevCycleUser::FromEvaluationContext($evaluationContext); - - self::assertEquals("valid-user-id", $user->getUserId(), 'userId should be used when user_id is invalid type'); - self::assertArrayHasKey("other_field", $user->getCustomData(), 'other fields should still appear in custom data'); - } - public function testEvaluationContext() { $attributes = new Attributes(array( From 3d963aab4774bc1d2bf725c6b350f5eacf73f5b5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Jul 2025 16:06:41 +0000 Subject: [PATCH 5/8] Improve user ID validation with more descriptive error messages Co-authored-by: jonathan --- .phpunit.cache/test-results | 2 +- lib/Model/DevCycleUser.php | 6 +++++- test/Api/OpenFeatureTest.php | 14 +++++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results index 78f613c..a4c14b5 100644 --- a/.phpunit.cache/test-results +++ b/.phpunit.cache/test-results @@ -1 +1 @@ -{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":7},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesThrowException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesSkippedInPriority":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":0.003,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypeThrowsException":0}} \ No newline at end of file +{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":7},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesThrowException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesSkippedInPriority":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":0.003,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypeThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdFieldTypeThrowsException":0}} \ No newline at end of file diff --git a/lib/Model/DevCycleUser.php b/lib/Model/DevCycleUser.php index 411e5e5..83cc789 100644 --- a/lib/Model/DevCycleUser.php +++ b/lib/Model/DevCycleUser.php @@ -716,13 +716,17 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc // Priority order: targetingKey -> user_id -> userId $userId = null; + $userIdSource = null; if ($context->getTargetingKey() !== null) { $userId = $context->getTargetingKey(); + $userIdSource = 'targetingKey'; } elseif ($context->getAttributes()->get("user_id") !== null) { $userId = $context->getAttributes()->get("user_id"); + $userIdSource = 'user_id'; } elseif ($context->getAttributes()->get("userId") !== null) { $userId = $context->getAttributes()->get("userId"); + $userIdSource = 'userId'; } if ($userId === null) { @@ -730,7 +734,7 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc } if (!ValueTypeValidator::isString($userId)) { - throw new \InvalidArgumentException('User ID must be a string value'); + throw new \InvalidArgumentException("User ID field '{$userIdSource}' must be a string value, got " . gettype($userId)); } $user->setUserId($userId); diff --git a/test/Api/OpenFeatureTest.php b/test/Api/OpenFeatureTest.php index e00edab..205d793 100644 --- a/test/Api/OpenFeatureTest.php +++ b/test/Api/OpenFeatureTest.php @@ -125,7 +125,19 @@ public function testInvalidUserIdTypeThrowsException() $evaluationContext = new EvaluationContext(attributes: $attributes); $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('User ID must be a string value'); + $this->expectExceptionMessage("User ID field 'user_id' must be a string value, got array"); + + DevCycleUser::FromEvaluationContext($evaluationContext); + } + + public function testInvalidUserIdFieldTypeThrowsException() + { + // Test that non-string userId throws string validation exception with correct field name + $attributes = new Attributes(array("userId" => true, "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage("User ID field 'userId' must be a string value, got boolean"); DevCycleUser::FromEvaluationContext($evaluationContext); } From 0d319497d30842c0674ba8ea92dd5e004d3359fd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Jul 2025 16:09:42 +0000 Subject: [PATCH 6/8] Ignore PHPUnit cache directory and remove test results cache Co-authored-by: jonathan --- .gitignore | 1 + .phpunit.cache/test-results | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 .phpunit.cache/test-results diff --git a/.gitignore b/.gitignore index 990331b..ad588fe 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ composer.phar # PHPUnit cache .phpunit.result.cache +.phpunit.cache/ .vscode/launch.json .idea/* diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results deleted file mode 100644 index a4c14b5..0000000 --- a/.phpunit.cache/test-results +++ /dev/null @@ -1 +0,0 @@ -{"version":1,"defects":{"DevCycle\\Test\\Api\\DevCycleClientTest::testGetFeatures":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariableByKey":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariable_invalidSDKKey_isDefaultedTrue":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testVariableDefaultedDoesNotThrow":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testGetVariables":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testPostEvents":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testTrackNoType":8,"DevCycle\\Test\\Api\\DevCycleClientTest::testOpenFeature":8,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":7},"times":{"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContextTargetingKey":0.002,"DevCycle\\Test\\Api\\OpenFeatureTest::testUserIdFieldsNotInCustomData":0.001,"DevCycle\\Test\\Api\\OpenFeatureTest::testMissingUserIdThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testEvaluationContext":0,"DevCycle\\Test\\Api\\HookTest::testHooksWorkCorrectly":2.12,"DevCycle\\Test\\Api\\HookTest::testBeforeHookErrorIsCaught":2.095,"DevCycle\\Test\\Api\\HookTest::testAfterHookErrorIsCaught":2.086,"DevCycle\\Test\\Api\\HookTest::testFunctionalityWithoutHooks":2.086,"DevCycle\\Test\\Api\\HookTest::testOnFinallyAlwaysCalled":2.172,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesThrowException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypesSkippedInPriority":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidTargetingKeyTypeThrowsException":0.003,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdTypeThrowsException":0,"DevCycle\\Test\\Api\\OpenFeatureTest::testInvalidUserIdFieldTypeThrowsException":0}} \ No newline at end of file From 13f23fc79eb326389b6e58805ce459e437e2f960 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Jul 2025 16:13:01 +0000 Subject: [PATCH 7/8] Remove user ID type validation in DevCycleUser and related tests Co-authored-by: jonathan --- lib/Model/DevCycleUser.php | 8 -------- test/Api/OpenFeatureTest.php | 22 ---------------------- 2 files changed, 30 deletions(-) diff --git a/lib/Model/DevCycleUser.php b/lib/Model/DevCycleUser.php index 83cc789..4187cc5 100644 --- a/lib/Model/DevCycleUser.php +++ b/lib/Model/DevCycleUser.php @@ -716,27 +716,19 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc // Priority order: targetingKey -> user_id -> userId $userId = null; - $userIdSource = null; if ($context->getTargetingKey() !== null) { $userId = $context->getTargetingKey(); - $userIdSource = 'targetingKey'; } elseif ($context->getAttributes()->get("user_id") !== null) { $userId = $context->getAttributes()->get("user_id"); - $userIdSource = 'user_id'; } elseif ($context->getAttributes()->get("userId") !== null) { $userId = $context->getAttributes()->get("userId"); - $userIdSource = 'userId'; } if ($userId === null) { throw new \InvalidArgumentException('targetingKey, user_id, or userId is missing from EvaluationContext'); } - if (!ValueTypeValidator::isString($userId)) { - throw new \InvalidArgumentException("User ID field '{$userIdSource}' must be a string value, got " . gettype($userId)); - } - $user->setUserId($userId); foreach ($context->getAttributes()->toArray() as $key => $value) { diff --git a/test/Api/OpenFeatureTest.php b/test/Api/OpenFeatureTest.php index 205d793..1e2c07d 100644 --- a/test/Api/OpenFeatureTest.php +++ b/test/Api/OpenFeatureTest.php @@ -118,29 +118,7 @@ public function testMissingUserIdThrowsException() DevCycleUser::FromEvaluationContext($evaluationContext); } - public function testInvalidUserIdTypeThrowsException() - { - // Test that non-string user_id throws string validation exception - $attributes = new Attributes(array("user_id" => array("invalid", "array"), "other_field" => "value")); - $evaluationContext = new EvaluationContext(attributes: $attributes); - - $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage("User ID field 'user_id' must be a string value, got array"); - - DevCycleUser::FromEvaluationContext($evaluationContext); - } - public function testInvalidUserIdFieldTypeThrowsException() - { - // Test that non-string userId throws string validation exception with correct field name - $attributes = new Attributes(array("userId" => true, "other_field" => "value")); - $evaluationContext = new EvaluationContext(attributes: $attributes); - - $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage("User ID field 'userId' must be a string value, got boolean"); - - DevCycleUser::FromEvaluationContext($evaluationContext); - } public function testEvaluationContext() { From a761a907bc825a8885291fc42c24f8d11982fd7f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Jul 2025 16:14:28 +0000 Subject: [PATCH 8/8] Add user ID type validation in DevCycleUser creation Co-authored-by: jonathan --- lib/Model/DevCycleUser.php | 8 ++++++++ test/Api/OpenFeatureTest.php | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lib/Model/DevCycleUser.php b/lib/Model/DevCycleUser.php index 4187cc5..83cc789 100644 --- a/lib/Model/DevCycleUser.php +++ b/lib/Model/DevCycleUser.php @@ -716,19 +716,27 @@ public static function FromEvaluationContext(EvaluationContext $context): DevCyc // Priority order: targetingKey -> user_id -> userId $userId = null; + $userIdSource = null; if ($context->getTargetingKey() !== null) { $userId = $context->getTargetingKey(); + $userIdSource = 'targetingKey'; } elseif ($context->getAttributes()->get("user_id") !== null) { $userId = $context->getAttributes()->get("user_id"); + $userIdSource = 'user_id'; } elseif ($context->getAttributes()->get("userId") !== null) { $userId = $context->getAttributes()->get("userId"); + $userIdSource = 'userId'; } if ($userId === null) { throw new \InvalidArgumentException('targetingKey, user_id, or userId is missing from EvaluationContext'); } + if (!ValueTypeValidator::isString($userId)) { + throw new \InvalidArgumentException("User ID field '{$userIdSource}' must be a string value, got " . gettype($userId)); + } + $user->setUserId($userId); foreach ($context->getAttributes()->toArray() as $key => $value) { diff --git a/test/Api/OpenFeatureTest.php b/test/Api/OpenFeatureTest.php index 1e2c07d..205d793 100644 --- a/test/Api/OpenFeatureTest.php +++ b/test/Api/OpenFeatureTest.php @@ -118,7 +118,29 @@ public function testMissingUserIdThrowsException() DevCycleUser::FromEvaluationContext($evaluationContext); } + public function testInvalidUserIdTypeThrowsException() + { + // Test that non-string user_id throws string validation exception + $attributes = new Attributes(array("user_id" => array("invalid", "array"), "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage("User ID field 'user_id' must be a string value, got array"); + + DevCycleUser::FromEvaluationContext($evaluationContext); + } + public function testInvalidUserIdFieldTypeThrowsException() + { + // Test that non-string userId throws string validation exception with correct field name + $attributes = new Attributes(array("userId" => true, "other_field" => "value")); + $evaluationContext = new EvaluationContext(attributes: $attributes); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage("User ID field 'userId' must be a string value, got boolean"); + + DevCycleUser::FromEvaluationContext($evaluationContext); + } public function testEvaluationContext() {