diff --git a/.changeset/steady-flags-align.md b/.changeset/steady-flags-align.md new file mode 100644 index 000000000..b011b33ba --- /dev/null +++ b/.changeset/steady-flags-align.md @@ -0,0 +1,5 @@ +--- +"posthog-server": patch +--- + +Align local `is_set` and `is_not_set` evaluation with partial property context. diff --git a/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt b/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt index 5abab6a92..87d519070 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt @@ -139,9 +139,8 @@ internal class FlagEvaluator( throw InconclusiveMatchException("Can't match properties without a given property value") } - // is_not_set operator can't be evaluated locally - if (propertyOperator == PropertyOperator.IS_NOT_SET) { - throw InconclusiveMatchException("Can't match properties with operator is_not_set") + if (propertyOperator == PropertyOperator.IS_SET || propertyOperator == PropertyOperator.IS_NOT_SET) { + return propertyOperator == PropertyOperator.IS_SET } val overrideValue = propertyValues[key] @@ -157,7 +156,6 @@ internal class FlagEvaluator( if (propertyOperator == PropertyOperator.EXACT) matches else !matches } - PropertyOperator.IS_SET -> propertyValues.containsKey(key) PropertyOperator.ICONTAINS -> stringContains( overrideValue.toString(), diff --git a/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt b/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt index 92fb2fc00..85500fedd 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt @@ -884,7 +884,7 @@ internal class PostHogFeatureFlags( } // Use group's key and properties for evaluation - Pair(groupKey, groupProperties) + Pair(groupKey, groupProperties?.get(groupTypeName)) } else { // Person-based flag - use person's ID and properties Pair(distinctId, personProperties) diff --git a/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt b/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt index 56af2ab69..7f5342b55 100644 --- a/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt @@ -159,9 +159,39 @@ internal class FlagEvaluatorTest { negation = false, dependencyChain = null, ) - val properties = mapOf("email" to "test@example.com") - assertTrue(evaluator.matchProperty(property, properties)) + val propertiesWithout = mapOf("name" to "Test") + try { + evaluator.matchProperty(property, propertiesWithout) + assertTrue("Should have thrown InconclusiveMatchException", false) + } catch (e: InconclusiveMatchException) { + // Expected + } + + val presentValues = + listOf( + null, + false, + 0, + "", + emptyList(), + emptyMap(), + ) + for (value in presentValues) { + assertTrue(evaluator.matchProperty(property, mapOf("email" to value))) + } + } + @Test + internal fun testMatchPropertyIsNotSet() { + val property = + FlagProperty( + key = "email", + propertyValue = null, + propertyOperator = PropertyOperator.IS_NOT_SET, + type = PropertyType.PERSON, + negation = false, + dependencyChain = null, + ) val propertiesWithout = mapOf("name" to "Test") try { evaluator.matchProperty(property, propertiesWithout) @@ -169,6 +199,19 @@ internal class FlagEvaluatorTest { } catch (e: InconclusiveMatchException) { // Expected } + + val presentValues = + listOf( + null, + false, + 0, + "", + emptyList(), + emptyMap(), + ) + for (value in presentValues) { + assertFalse(evaluator.matchProperty(property, mapOf("email" to value))) + } } @Test diff --git a/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt b/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt index a858c8ff3..35c7172d6 100644 --- a/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt @@ -1698,6 +1698,77 @@ internal class PostHogFeatureFlagsTest { mockServer.shutdown() } + @Test + fun `group presence operators use properties for the matching group type`() { + for ((operator, expected) in mapOf("is_set" to true, "is_not_set" to false)) { + val flagKey = "group-$operator" + val localEvalResponse = + """ + { + "flags": [ + { + "id": 1, + "name": "$flagKey", + "key": "$flagKey", + "active": true, + "filters": { + "aggregation_group_type_index": 2, + "groups": [ + { + "properties": [ + { + "key": "plan", + "value": "$operator", + "operator": "$operator", + "type": "person", + "negation": false + } + ], + "rollout_percentage": 100 + } + ] + }, + "version": 1 + } + ], + "group_type_mapping": { "2": "organization" }, + "cohorts": {} + } + """.trimIndent() + val logger = TestLogger() + val mockServer = + createMockHttp( + jsonResponse(localEvalResponse), + jsonResponse(createEmptyFlagsResponse()), + ) + val config = createTestConfig(logger, mockServer.url("/").toString()) + val featureFlags = + PostHogFeatureFlags( + config, + PostHogApi(config), + 60000, + 100, + localEvaluation = true, + personalApiKey = "test-personal-key", + ) + + val result = + featureFlags.getFeatureFlag( + key = flagKey, + defaultValue = null, + distinctId = "user-123", + groups = mapOf("organization" to "org-456"), + groupProperties = mapOf("organization" to mapOf("plan" to null)), + ) + + assertEquals(expected, result) + assertTrue(logger.containsLog("Local evaluation successful")) + + featureFlags.shutDown() + mockServer.shutdown() + } + } + @Test fun `group-based flag returns false when required group is missing`() { val logger = TestLogger()