From 8715dbdd64af786875f239621055905aae4c458b Mon Sep 17 00:00:00 2001 From: Patricio Date: Tue, 28 Apr 2026 11:57:20 -0300 Subject: [PATCH 1/2] feat(flags): support mixed targeting in local evaluation --- feature_flags_mixed_targeting_test.go | 209 ++++++++++++++++++++++++++ featureflags.go | 69 ++++++++- 2 files changed, 270 insertions(+), 8 deletions(-) create mode 100644 feature_flags_mixed_targeting_test.go diff --git a/feature_flags_mixed_targeting_test.go b/feature_flags_mixed_targeting_test.go new file mode 100644 index 0000000..e8233fc --- /dev/null +++ b/feature_flags_mixed_targeting_test.go @@ -0,0 +1,209 @@ +package posthog + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" +) + +// Mixed-targeting flag: flag-level aggregation is null, the first condition targets +// group "company" (index 0), the second targets persons by email. +const mixedTargetingFlagJSON = `{ + "flags": [{ + "id": 1, + "key": "mixed-flag", + "active": true, + "filters": { + "aggregation_group_type_index": null, + "groups": [ + { + "aggregation_group_type_index": 0, + "properties": [ + {"key": "plan", "value": "enterprise", "operator": "exact", "type": "group", "group_type_index": 0} + ], + "rollout_percentage": 100 + }, + { + "aggregation_group_type_index": null, + "properties": [ + {"key": "email", "value": "test@example.com", "operator": "exact", "type": "person"} + ], + "rollout_percentage": 100 + } + ] + } + }], + "group_type_mapping": {"0": "company"}, + "cohorts": {} +}` + +const onlyGroupConditionFlagJSON = `{ + "flags": [{ + "id": 1, + "key": "only-group-flag", + "active": true, + "filters": { + "aggregation_group_type_index": null, + "groups": [ + { + "aggregation_group_type_index": 0, + "properties": [ + {"key": "plan", "value": "enterprise", "operator": "exact", "type": "group", "group_type_index": 0} + ], + "rollout_percentage": 100 + } + ] + } + }], + "group_type_mapping": {"0": "company"}, + "cohorts": {} +}` + +func newMixedTargetingServer(t *testing.T, localFlagsJSON string, decideCalls *atomic.Int32) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Order matters: /flags/definitions must be checked before /flags or /flags/ + // so the prefix doesn't accidentally match the decide endpoint. + if strings.HasPrefix(r.URL.Path, "/flags/definitions") { + w.Write([]byte(localFlagsJSON)) + return + } + if r.URL.Path == "/flags" || r.URL.Path == "/flags/" { + if decideCalls != nil { + decideCalls.Add(1) + } + // Return a value that would clearly differ from local-eval result if we ever fall back. + w.Write([]byte(`{"featureFlags": {"mixed-flag": "server-fallback", "only-group-flag": "server-fallback"}}`)) + return + } + })) +} + +func TestMixedTargetingLocalEvaluation(t *testing.T) { + type opts struct { + groups Groups + personProperties Properties + groupProperties map[string]Properties + } + + cases := []struct { + name string + flagJSON string + flagKey string + opts opts + expectVal interface{} + }{ + { + name: "person condition matches when no groups passed", + flagJSON: mixedTargetingFlagJSON, + flagKey: "mixed-flag", + opts: opts{personProperties: Properties{"email": "test@example.com"}}, + expectVal: true, + }, + { + name: "group condition matches when group props match", + flagJSON: mixedTargetingFlagJSON, + flagKey: "mixed-flag", + opts: opts{ + groups: Groups{"company": "acme"}, + groupProperties: map[string]Properties{"company": {"plan": "enterprise"}}, + personProperties: Properties{"email": "nope@example.com"}, + }, + expectVal: true, + }, + { + name: "no match when both person and group fail", + flagJSON: mixedTargetingFlagJSON, + flagKey: "mixed-flag", + opts: opts{ + groups: Groups{"company": "acme"}, + groupProperties: map[string]Properties{"company": {"plan": "free"}}, + personProperties: Properties{"email": "nope@example.com"}, + }, + expectVal: false, + }, + { + name: "only group conditions, no groups passed: returns false without server fallback", + flagJSON: onlyGroupConditionFlagJSON, + flagKey: "only-group-flag", + opts: opts{}, + expectVal: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var decideCalls atomic.Int32 + server := newMixedTargetingServer(t, tc.flagJSON, &decideCalls) + defer server.Close() + + client, err := NewWithConfig("test-api-key", Config{ + PersonalApiKey: "test-personal-key", + Endpoint: server.URL, + }) + require.NoError(t, err) + defer client.Close() + + result, err := client.GetFeatureFlag(FeatureFlagPayload{ + Key: tc.flagKey, + DistinctId: "test-distinct-id", + Groups: tc.opts.groups, + PersonProperties: tc.opts.personProperties, + GroupProperties: tc.opts.groupProperties, + OnlyEvaluateLocally: false, + }) + require.NoError(t, err) + require.Equal(t, tc.expectVal, result) + require.Equal(t, int32(0), decideCalls.Load(), "expected no fallback to /flags decide endpoint") + }) + } +} + +// Verifies the rollout for a group condition under a mixed flag hashes on the +// group key, not the distinct_id — calling with the group passed must resolve +// locally without falling back to the decide endpoint. +func TestMixedTargetingRolloutBucketing(t *testing.T) { + flagJSON := `{ + "flags": [{ + "id": 1, + "key": "rollout-flag", + "active": true, + "filters": { + "aggregation_group_type_index": null, + "groups": [ + { + "aggregation_group_type_index": 0, + "properties": [], + "rollout_percentage": 100 + } + ] + } + }], + "group_type_mapping": {"0": "company"}, + "cohorts": {} + }` + var decideCalls atomic.Int32 + server := newMixedTargetingServer(t, flagJSON, &decideCalls) + defer server.Close() + + client, err := NewWithConfig("test-api-key", Config{ + PersonalApiKey: "test-personal-key", + Endpoint: server.URL, + }) + require.NoError(t, err) + defer client.Close() + + result, err := client.GetFeatureFlag(FeatureFlagPayload{ + Key: "rollout-flag", + DistinctId: "any-distinct-id", + Groups: Groups{"company": "acme"}, + GroupProperties: map[string]Properties{"company": {}}, + }) + require.NoError(t, err) + require.Equal(t, true, result) + require.Equal(t, int32(0), decideCalls.Load(), "expected no fallback to /flags decide endpoint") +} diff --git a/featureflags.go b/featureflags.go index d4b6d92..d992c60 100644 --- a/featureflags.go +++ b/featureflags.go @@ -123,9 +123,10 @@ type FlagVariant struct { } type FeatureFlagCondition struct { - Properties []FlagProperty `json:"properties"` - RolloutPercentage *float64 `json:"rollout_percentage"` - Variant *string `json:"variant"` + Properties []FlagProperty `json:"properties"` + RolloutPercentage *float64 `json:"rollout_percentage"` + Variant *string `json:"variant"` + AggregationGroupTypeIndex *uint8 `json:"aggregation_group_type_index"` } type FlagProperty struct { @@ -319,7 +320,7 @@ func (poller *FeatureFlagsPoller) evaluateFlagDependency( evaluationCache[depFlagKey] = false } else { // Recursively evaluate the dependency - result, err := poller.matchFeatureFlagProperties(depFlag, distinctId, deviceId, properties, cohorts, flagsByKey, evaluationCache) + result, err := poller.matchFeatureFlagProperties(depFlag, distinctId, deviceId, properties, cohorts, flagsByKey, evaluationCache, nil, nil) if err != nil { // If we can't evaluate a dependency, store nil and propagate the error evaluationCache[depFlagKey] = nil @@ -800,7 +801,7 @@ func (poller *FeatureFlagsPoller) computeFlagLocally( if _, ok := focusedGroupProperties["$group_key"]; !ok { focusedGroupProperties = Properties{"$group_key": groupKey}.Merge(focusedGroupProperties) } - return poller.matchFeatureFlagProperties(flag, groups[groupType].(string), nil, focusedGroupProperties, cohorts, flagsByKey, evaluationCache) + return poller.matchFeatureFlagProperties(flag, groups[groupType].(string), nil, focusedGroupProperties, cohorts, flagsByKey, evaluationCache, groups, groupProperties) } else { localPersonProperties := personProperties // Only add distinct_id if the flag has conditions that check person properties. @@ -818,7 +819,7 @@ func (poller *FeatureFlagsPoller) computeFlagLocally( } } } - return poller.matchFeatureFlagProperties(flag, distinctId, deviceId, localPersonProperties, cohorts, flagsByKey, evaluationCache) + return poller.matchFeatureFlagProperties(flag, distinctId, deviceId, localPersonProperties, cohorts, flagsByKey, evaluationCache, groups, groupProperties) } } @@ -877,13 +878,58 @@ func (poller *FeatureFlagsPoller) matchFeatureFlagProperties( cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, + groups Groups, + groupProperties map[string]Properties, ) (interface{}, error) { conditions := flag.Filters.Groups bucketingId := getBucketingID(flag, distinctId, deviceId) + flagAggregation := flag.Filters.AggregationGroupTypeIndex + groupTypeMapping := poller.getGroups() isInconclusive := false for _, condition := range conditions { - isMatch, err := poller.isConditionMatch(flag, distinctId, bucketingId, deviceId, condition, properties, cohorts, flagsByKey, evaluationCache) + // Per-condition aggregation overrides only when the condition explicitly + // sets its own AggregationGroupTypeIndex that differs from the flag level + // (mixed targeting). When absent, fall back to the flag-level aggregation + // so existing pure person and pure group flags keep their original behavior. + conditionAggregation := condition.AggregationGroupTypeIndex + if conditionAggregation == nil { + conditionAggregation = flagAggregation + } + + effectiveProperties := properties + effectiveBucketingId := bucketingId + + // Mixed-override path: condition-level aggregation differs from flag-level. + // This assumes flag-level aggregation is nil for mixed flags. + if !uint8PtrEqual(conditionAggregation, flagAggregation) { + if conditionAggregation != nil { + groupType, exists := groupTypeMapping[fmt.Sprintf("%d", *conditionAggregation)] + if !exists { + continue + } + groupKey, hasGroup := groups[groupType] + if !hasGroup { + continue + } + focusedGroupProperties, hasProps := groupProperties[groupType] + if !hasProps { + isInconclusive = true + continue + } + groupKeyStr, ok := groupKey.(string) + if !ok { + continue + } + if _, exists := focusedGroupProperties["$group_key"]; !exists { + focusedGroupProperties = Properties{"$group_key": groupKey}.Merge(focusedGroupProperties) + } + effectiveProperties = focusedGroupProperties + effectiveBucketingId = groupKeyStr + } + } + + isMatch, err := poller.isConditionMatch(flag, distinctId, effectiveBucketingId, deviceId, condition, effectiveProperties, cohorts, flagsByKey, evaluationCache) if err != nil { // Use direct type switch instead of errors.As to avoid pointer escape allocations. // Our error types are returned directly (not wrapped), so type assertion suffices. @@ -907,7 +953,7 @@ func (poller *FeatureFlagsPoller) matchFeatureFlagProperties( if variantOverride != nil && multivariates != nil && multivariates.Variants != nil && containsVariant(multivariates.Variants, *variantOverride) { return *variantOverride, nil } else { - return getMatchingVariant(flag, bucketingId), nil + return getMatchingVariant(flag, effectiveBucketingId), nil } } } @@ -919,6 +965,13 @@ func (poller *FeatureFlagsPoller) matchFeatureFlagProperties( return false, nil } +func uint8PtrEqual(a, b *uint8) bool { + if a == nil || b == nil { + return a == b + } + return *a == *b +} + func (poller *FeatureFlagsPoller) isConditionMatch( flag FeatureFlag, distinctId string, From ccfacebd9b97ab3b30c1a57b71f05b2dd41d1862 Mon Sep 17 00:00:00 2001 From: Patricio Date: Tue, 28 Apr 2026 13:09:33 -0300 Subject: [PATCH 2/2] fix(flags): include distinct_id in person_properties for /flags request --- featureflags.go | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/featureflags.go b/featureflags.go index d992c60..2fa6d9a 100644 --- a/featureflags.go +++ b/featureflags.go @@ -536,6 +536,10 @@ func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) (interface{}, bool, error) { flag, err := poller.getFeatureFlag(flagConfig) + // Make sure person_properties contains distinct_id so /flags request payloads + // have it under person_properties (matches the batch path and what server expects). + personProps := mergeDistinctIDIntoProperties(flagConfig.PersonProperties, flagConfig.DistinctId) + var result interface{} locallyEvaluated := false @@ -545,7 +549,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) flagConfig.DistinctId, flagConfig.DeviceId, flagConfig.Groups, - flagConfig.PersonProperties, + personProps, flagConfig.GroupProperties, poller.getCohorts(), ) @@ -557,7 +561,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) } if (err != nil || result == nil) && !flagConfig.OnlyEvaluateLocally { - result, err = poller.getFeatureFlagVariant(flagConfig.Key, flagConfig.DistinctId, flagConfig.DeviceId, flagConfig.Groups, flagConfig.PersonProperties, flagConfig.GroupProperties) + result, err = poller.getFeatureFlagVariant(flagConfig.Key, flagConfig.DistinctId, flagConfig.DeviceId, flagConfig.Groups, personProps, flagConfig.GroupProperties) if err != nil { return nil, locallyEvaluated, err } @@ -566,6 +570,21 @@ func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) return result, locallyEvaluated, err } +// mergeDistinctIDIntoProperties returns a copy of properties with distinct_id added +// (if not already present). Used so /flags request payloads always carry distinct_id +// inside person_properties as well as at the top level. +func mergeDistinctIDIntoProperties(properties Properties, distinctID string) Properties { + if _, ok := properties["distinct_id"]; ok { + return properties + } + merged := make(Properties, len(properties)+1) + merged["distinct_id"] = distinctID + for k, v := range properties { + merged[k] = v + } + return merged +} + func (poller *FeatureFlagsPoller) GetFeatureFlagPayload(flagConfig FeatureFlagPayload) (string, error) { flag, err := poller.getFeatureFlag(flagConfig) @@ -702,15 +721,7 @@ func (poller *FeatureFlagsPoller) GetAllFlags(flagConfig FeatureFlagPayloadNoKey // Pre-merge distinct_id into person properties once for the entire batch, // instead of copying the map per-flag inside computeFlagLocally. - personProps := flagConfig.PersonProperties - if _, ok := personProps["distinct_id"]; !ok { - merged := make(Properties, len(personProps)+1) - merged["distinct_id"] = flagConfig.DistinctId - for k, v := range personProps { - merged[k] = v - } - personProps = merged - } + personProps := mergeDistinctIDIntoProperties(flagConfig.PersonProperties, flagConfig.DistinctId) if len(featureFlags) == 0 { fallbackToDecide = true