diff --git a/.changeset/versioned-property-matching.md b/.changeset/versioned-property-matching.md new file mode 100644 index 0000000..af41f12 --- /dev/null +++ b/.changeset/versioned-property-matching.md @@ -0,0 +1,5 @@ +--- +"posthog-go": patch +--- + +Honor the definitions snapshot's `property_matching_version` during local flag evaluation: version 2 uses explicit boolean equality, while missing/1 and unknown versions retain legacy matching. Keep definitions and matching semantics together across reloads, cached 304 responses, group/cohort rules, and flag dependencies. Also preserve JSON-decoded dependency chains in cohort leaves, while requiring server evaluation for dependencies that need a different aggregation context. Same-group dependencies reuse the caller’s group key and properties for local evaluation. diff --git a/feature_flag_property_group.go b/feature_flag_property_group.go index 9ca8156..56199d8 100644 --- a/feature_flag_property_group.go +++ b/feature_flag_property_group.go @@ -1,21 +1,23 @@ package posthog -func (poller *FeatureFlagsPoller) matchCohort(property FlagProperty, properties Properties, cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, distinctId string, deviceId *string) (bool, error) { +func (poller *FeatureFlagsPoller) matchCohort(property FlagProperty, properties Properties, cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, distinctId string, deviceId *string, aggregationGroupTypeIndex *uint8, snapshots ...*flagsState) (bool, error) { + state := poller.evaluationState(snapshots) cohortId := valueToString(property.Value) propertyGroup, ok := cohorts[cohortId] if !ok { return false, errCohortRequiresServerEval } - return poller.matchPropertyGroup(propertyGroup, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId) + return poller.matchPropertyGroup(propertyGroup, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId, aggregationGroupTypeIndex, state) } -func (poller *FeatureFlagsPoller) matchPropertyGroup(propertyGroup PropertyGroup, properties Properties, cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, distinctId string, deviceId *string) (bool, error) { +func (poller *FeatureFlagsPoller) matchPropertyGroup(propertyGroup PropertyGroup, properties Properties, cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, distinctId string, deviceId *string, aggregationGroupTypeIndex *uint8, snapshots ...*flagsState) (bool, error) { + state := poller.evaluationState(snapshots) groupType := propertyGroup.Type // Use pre-parsed values if available (built at load time), otherwise fall back to raw values if len(propertyGroup.ParsedValues) > 0 { - return poller.matchParsedPropertyGroup(groupType, propertyGroup.ParsedValues, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId) + return poller.matchParsedPropertyGroup(groupType, propertyGroup.ParsedValues, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId, aggregationGroupTypeIndex, state) } if len(propertyGroup.Values) == 0 { @@ -26,18 +28,19 @@ func (poller *FeatureFlagsPoller) matchPropertyGroup(propertyGroup PropertyGroup // Raw values are a compatibility fallback. Convert them to the same typed // representation used by production cohorts so evaluation has one code path. parsedGroup := preParsePG(propertyGroup) - return poller.matchParsedPropertyGroup(groupType, parsedGroup.ParsedValues, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId) + return poller.matchParsedPropertyGroup(groupType, parsedGroup.ParsedValues, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId, aggregationGroupTypeIndex, state) } // matchParsedPropertyGroup evaluates pre-parsed property values without per-evaluation // reconstruction from map[string]any. This is the fast path for cohort matching. -func (poller *FeatureFlagsPoller) matchParsedPropertyGroup(groupType string, parsedValues []parsedPropertyValue, properties Properties, cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, distinctId string, deviceId *string) (bool, error) { +func (poller *FeatureFlagsPoller) matchParsedPropertyGroup(groupType string, parsedValues []parsedPropertyValue, properties Properties, cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, distinctId string, deviceId *string, aggregationGroupTypeIndex *uint8, snapshots ...*flagsState) (bool, error) { + state := poller.evaluationState(snapshots) errorMatchingLocally := false for i := range parsedValues { pv := &parsedValues[i] if pv.IsGroup { - matches, err := poller.matchPropertyGroup(pv.Group, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId) + matches, err := poller.matchPropertyGroup(pv.Group, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId, aggregationGroupTypeIndex, state) if err != nil { if isServerEvalError(err) { return false, err @@ -62,11 +65,11 @@ func (poller *FeatureFlagsPoller) matchParsedPropertyGroup(groupType string, par var err error fp := &pv.Property if fp.Type == "cohort" { - matches, err = poller.matchCohort(*fp, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId) + matches, err = poller.matchCohort(*fp, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId, aggregationGroupTypeIndex, state) } else if fp.Type == "flag" { - matches, err = poller.evaluateFlagDependency(*fp, flagsByKey, evaluationCache, distinctId, deviceId, properties, cohorts) + matches, err = poller.evaluateFlagDependency(*fp, flagsByKey, evaluationCache, distinctId, deviceId, properties, cohorts, aggregationGroupTypeIndex, state) } else { - matches, err = matchProperty(*fp, properties) + matches, err = matchProperty(*fp, properties, state.propertyMatchingVersion) } if err != nil { diff --git a/feature_flags_property_group_test.go b/feature_flags_property_group_test.go index 19a3c70..d293dc2 100644 --- a/feature_flags_property_group_test.go +++ b/feature_flags_property_group_test.go @@ -72,13 +72,13 @@ func TestMatchPropertyGroupRawAndParsedParity(t *testing.T) { poller := &FeatureFlagsPoller{} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rawResult, rawErr := poller.matchPropertyGroup(tt.group, tt.properties, nil, nil, nil, "distinct-id", nil) + rawResult, rawErr := poller.matchPropertyGroup(tt.group, tt.properties, nil, nil, nil, "distinct-id", nil, nil) require.Equal(t, tt.want, rawResult) require.Equal(t, tt.wantErr, rawErr) parsedGroup := preParsePG(tt.group) require.NotEmpty(t, parsedGroup.ParsedValues) - parsedResult, parsedErr := poller.matchPropertyGroup(parsedGroup, tt.properties, nil, nil, nil, "distinct-id", nil) + parsedResult, parsedErr := poller.matchPropertyGroup(parsedGroup, tt.properties, nil, nil, nil, "distinct-id", nil, nil) require.Equal(t, rawResult, parsedResult) require.Equal(t, rawErr, parsedErr) }) diff --git a/featureflags.go b/featureflags.go index 3eadfeb..0938235 100644 --- a/featureflags.go +++ b/featureflags.go @@ -67,11 +67,12 @@ func getOrCompileRegex(pattern string) (*regexp.Regexp, error) { // flagsState holds the feature flag data that is atomically swapped during updates. // This provides lock-free reads for the common path (flag evaluation). type flagsState struct { - featureFlags []FeatureFlag - flagsByKey map[string]FeatureFlag // pre-built index for O(1) lookup, avoids rebuilding per evaluation - cohorts map[string]PropertyGroup - groups map[string]string - flagsEtag string + featureFlags []FeatureFlag + flagsByKey map[string]FeatureFlag // pre-built index for O(1) lookup, avoids rebuilding per evaluation + cohorts map[string]PropertyGroup + groups map[string]string + flagsEtag string + propertyMatchingVersion int // minimalFlagCalledEvents is the server-controlled gate for minimal // $feature_flag_called events, cached from the local-evaluation payload. minimalFlagCalledEvents bool @@ -335,7 +336,10 @@ func (poller *FeatureFlagsPoller) evaluateFlagDependency( deviceId *string, properties Properties, cohorts map[string]PropertyGroup, + aggregationGroupTypeIndex *uint8, + snapshots ...*flagsState, ) (bool, error) { + state := poller.evaluationState(snapshots) // Some of these conditions should never happen, but we'll check them to be defensive. if property.Value == nil { return false, &InconclusiveMatchError{ @@ -378,6 +382,20 @@ func (poller *FeatureFlagsPoller) evaluateFlagDependency( // Evaluate all dependencies in the chain order for _, depFlagKey := range dependencyChain { + // Dependencies can reuse the caller's key and properties only for the + // same aggregation type. Check before the cache so repeated cohort + // references cannot bypass the cross-type server boundary. + depFlag := flagsByKey[depFlagKey] + if depFlag.Active { + if !uint8PtrEqual(depFlag.Filters.AggregationGroupTypeIndex, aggregationGroupTypeIndex) { + return false, &RequiresServerEvaluationError{"Flag dependency requires different aggregation context"} + } + for _, condition := range depFlag.Filters.Groups { + if condition.AggregationGroupTypeIndex != nil && !uint8PtrEqual(condition.AggregationGroupTypeIndex, aggregationGroupTypeIndex) { + return false, &RequiresServerEvaluationError{"Flag dependency requires different aggregation context"} + } + } + } if _, exists := evaluationCache[depFlagKey]; exists { continue } @@ -396,10 +414,19 @@ func (poller *FeatureFlagsPoller) evaluateFlagDependency( if !depFlag.Active { evaluationCache[depFlagKey] = false } else { + // Dependencies must preserve the same server boundary as direct evaluation. + if depFlag.EnsureExperienceContinuity != nil && *depFlag.EnsureExperienceContinuity { + evaluationCache[depFlagKey] = nil + return false, &InconclusiveMatchError{"Flag dependency has experience continuity enabled"} + } // Recursively evaluate the dependency - result, err := poller.matchFeatureFlagProperties(depFlag, distinctId, deviceId, properties, cohorts, flagsByKey, evaluationCache, nil, nil) + result, err := poller.matchFeatureFlagProperties(depFlag, distinctId, deviceId, properties, cohorts, flagsByKey, evaluationCache, nil, nil, state) if err != nil { - // If we can't evaluate a dependency, store nil and propagate the error + // Preserve server-required errors on repeated references instead of + // reducing them to a cached inconclusive value that cohorts can negate. + if isServerEvalError(err) { + return false, err + } evaluationCache[depFlagKey] = nil return false, &InconclusiveMatchError{ msg: fmt.Sprintf("Cannot evaluate flag dependency '%s': %s", depFlagKey, err.Error()), @@ -547,6 +574,7 @@ func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { groups: currentState.groups, flagsEtag: newEtag, minimalFlagCalledEvents: currentState.minimalFlagCalledEvents, + propertyMatchingVersion: currentState.propertyMatchingVersion, } poller.state.Store(newState) } @@ -576,7 +604,13 @@ func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { poller.Logger.Errorf("Unable to fetch feature flags: %s", err) return } - var featureFlagsResponse FeatureFlagsResponse + // Decode matching metadata privately to preserve FeatureFlagsResponse's public + // field layout, including compatibility with consumers' unkeyed literals. + var featureFlagsResponse struct { + FeatureFlagsResponse + // Only 2 enables explicit matching; missing and unknown versions stay legacy. + PropertyMatchingVersion int `json:"property_matching_version"` + } if err = json.Unmarshal(resBody, &featureFlagsResponse); err != nil { poller.Logger.Errorf("Unable to unmarshal response from api/feature_flag/local_evaluation: %s", err) return @@ -609,6 +643,7 @@ func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { groups: groups, flagsEtag: newEtag, minimalFlagCalledEvents: featureFlagsResponse.MinimalFlagCalledEvents, + propertyMatchingVersion: featureFlagsResponse.PropertyMatchingVersion, }) } @@ -624,7 +659,7 @@ func (poller *FeatureFlagsPoller) getMinimalFlagCalledEvents() bool { // It returns the flag value, whether that value was locally evaluated, and an error. // If local evaluation is inconclusive and OnlyEvaluateLocally is false, it falls back to /flags. func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) (interface{}, bool, error) { - flag, err := poller.getFeatureFlag(flagConfig) + flag, state, err := poller.getFeatureFlagWithState(flagConfig) var result interface{} locallyEvaluated := false @@ -637,7 +672,8 @@ func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) flagConfig.Groups, flagConfig.PersonProperties, flagConfig.GroupProperties, - poller.getCohorts(), + state.cohorts, + state, ) locallyEvaluated = err == nil && result != nil } @@ -659,7 +695,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) // GetFeatureFlagPayload returns the payload for the evaluated flag value. // It tries local payloads first and falls back to the remote API unless OnlyEvaluateLocally is true. func (poller *FeatureFlagsPoller) GetFeatureFlagPayload(flagConfig FeatureFlagPayload) (string, error) { - flag, err := poller.getFeatureFlag(flagConfig) + flag, state, err := poller.getFeatureFlagWithState(flagConfig) var variant interface{} @@ -671,7 +707,8 @@ func (poller *FeatureFlagsPoller) GetFeatureFlagPayload(flagConfig FeatureFlagPa flagConfig.Groups, flagConfig.PersonProperties, flagConfig.GroupProperties, - poller.getCohorts(), + state.cohorts, + state, ) } if err != nil { @@ -712,7 +749,7 @@ type flagValueAndPayload struct { // and payload. This avoids the double evaluation that would happen when calling // GetFeatureFlag and GetFeatureFlagPayload separately. func (poller *FeatureFlagsPoller) GetFeatureFlagWithPayload(flagConfig FeatureFlagPayload) flagValueAndPayload { - flag, err := poller.getFeatureFlag(flagConfig) + flag, state, err := poller.getFeatureFlagWithState(flagConfig) var result interface{} @@ -724,7 +761,8 @@ func (poller *FeatureFlagsPoller) GetFeatureFlagWithPayload(flagConfig FeatureFl flagConfig.Groups, flagConfig.PersonProperties, flagConfig.GroupProperties, - poller.getCohorts(), + state.cohorts, + state, ) } @@ -743,7 +781,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlagWithPayload(flagConfig FeatureFl locallyEvaluated := err == nil && result != nil hasExperiment := flag.HasExperiment - minimalFlagCalledEvents := poller.getMinimalFlagCalledEvents() + minimalFlagCalledEvents := state != nil && state.minimalFlagCalledEvents // Fall back to remote evaluation if local didn't produce a result if (err != nil || result == nil) && !flagConfig.OnlyEvaluateLocally { @@ -782,30 +820,30 @@ func (poller *FeatureFlagsPoller) GetFeatureFlagWithPayload(flagConfig FeatureFl } func (poller *FeatureFlagsPoller) getFeatureFlag(flagConfig FeatureFlagPayload) (FeatureFlag, error) { - // Wait for initial flag fetch to complete - <-poller.firstFeatureFlagRequestFinished - - // Use pre-built index for O(1) lookup instead of linear scan - flagsByKey := poller.getFlagsByKey() - if flagsByKey == nil { - return FeatureFlag{}, errors.New("flags were not successfully fetched yet") - } + flag, _, err := poller.getFeatureFlagWithState(flagConfig) + return flag, err +} - if f, ok := flagsByKey[flagConfig.Key]; ok { - return f, nil +// Select the flag and its matching context from one immutable definitions snapshot. +func (poller *FeatureFlagsPoller) getFeatureFlagWithState(flagConfig FeatureFlagPayload) (FeatureFlag, *flagsState, error) { + <-poller.firstFeatureFlagRequestFinished + state := poller.state.Load() + if state == nil || state.flagsByKey == nil { + return FeatureFlag{}, state, errors.New("flags were not successfully fetched yet") } - return FeatureFlag{}, nil + return state.flagsByKey[flagConfig.Key], state, nil } // GetAllFlags evaluates every available flag for the configured user. // Values are bools for boolean flags or strings for multivariate variants. func (poller *FeatureFlagsPoller) GetAllFlags(flagConfig FeatureFlagPayloadNoKey) (map[string]interface{}, error) { - featureFlags, err := poller.GetFeatureFlags() + state, err := poller.getLoadedState() if err != nil { return nil, err } + featureFlags := state.featureFlags fallbackToDecide := false - cohorts := poller.getCohorts() + cohorts := state.cohorts // Pre-size response map to avoid rehashing as flags are added response := make(map[string]interface{}, len(featureFlags)) @@ -822,6 +860,7 @@ func (poller *FeatureFlagsPoller) GetAllFlags(flagConfig FeatureFlagPayloadNoKey flagConfig.PersonProperties, flagConfig.GroupProperties, cohorts, + state, ) if err != nil { poller.Logger.Warnf("Unable to compute flag locally (%s) - %s", storedFlag.Key, err) @@ -861,7 +900,9 @@ func (poller *FeatureFlagsPoller) computeFlagLocally( personProperties Properties, groupProperties map[string]Properties, cohorts map[string]PropertyGroup, + snapshots ...*flagsState, ) (interface{}, error) { + state := poller.evaluationState(snapshots) if flag.EnsureExperienceContinuity != nil && *flag.EnsureExperienceContinuity { return nil, &InconclusiveMatchError{"Flag has experience continuity enabled"} } @@ -871,7 +912,7 @@ func (poller *FeatureFlagsPoller) computeFlagLocally( } // Use pre-built flagsByKey index (built once when flags are fetched, not per evaluation) - flagsByKey := poller.getFlagsByKey() + flagsByKey := state.flagsByKey // evaluationCache is created lazily — only allocated when flag has dependencies. // For simple flags (no dependencies), this avoids a map allocation per evaluation. @@ -881,7 +922,7 @@ func (poller *FeatureFlagsPoller) computeFlagLocally( } if flag.Filters.AggregationGroupTypeIndex != nil { - groupType, exists := poller.getGroups()[fmt.Sprintf("%d", *flag.Filters.AggregationGroupTypeIndex)] + groupType, exists := state.groups[fmt.Sprintf("%d", *flag.Filters.AggregationGroupTypeIndex)] if !exists { errMessage := "flag has unknown group type index" @@ -899,7 +940,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, groups, groupProperties) + return poller.matchFeatureFlagProperties(flag, groups[groupType].(string), nil, focusedGroupProperties, cohorts, flagsByKey, evaluationCache, groups, groupProperties, state) } else { localPersonProperties := personProperties // Only add distinct_id if the flag has conditions that check person properties. @@ -917,7 +958,7 @@ func (poller *FeatureFlagsPoller) computeFlagLocally( } } } - return poller.matchFeatureFlagProperties(flag, distinctId, deviceId, localPersonProperties, cohorts, flagsByKey, evaluationCache, groups, groupProperties) + return poller.matchFeatureFlagProperties(flag, distinctId, deviceId, localPersonProperties, cohorts, flagsByKey, evaluationCache, groups, groupProperties, state) } } @@ -978,11 +1019,13 @@ func (poller *FeatureFlagsPoller) matchFeatureFlagProperties( evaluationCache map[string]interface{}, groups Groups, groupProperties map[string]Properties, + snapshots ...*flagsState, ) (interface{}, error) { + state := poller.evaluationState(snapshots) conditions := flag.Filters.Groups bucketingId := getBucketingID(flag, distinctId, deviceId) flagAggregation := flag.Filters.AggregationGroupTypeIndex - groupTypeMapping := poller.getGroups() + groupTypeMapping := state.groups isInconclusive := false for _, condition := range conditions { @@ -1027,7 +1070,7 @@ func (poller *FeatureFlagsPoller) matchFeatureFlagProperties( } } - matchResult, err := poller.isConditionMatch(flag, distinctId, effectiveBucketingId, deviceId, condition, effectiveProperties, cohorts, flagsByKey, evaluationCache) + matchResult, err := poller.isConditionMatch(flag, distinctId, effectiveBucketingId, deviceId, condition, effectiveProperties, cohorts, flagsByKey, evaluationCache, state) 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. @@ -1114,19 +1157,31 @@ func (poller *FeatureFlagsPoller) isConditionMatch( cohorts map[string]PropertyGroup, flagsByKey map[string]FeatureFlag, evaluationCache map[string]interface{}, + snapshots ...*flagsState, ) (conditionMatchResult, error) { + state := poller.evaluationState(snapshots) if len(condition.Properties) > 0 { var ( isMatch bool err error ) + aggregationGroupTypeIndex := condition.AggregationGroupTypeIndex + if aggregationGroupTypeIndex == nil { + aggregationGroupTypeIndex = flag.Filters.AggregationGroupTypeIndex + } + dependencyDistinctId, dependencyDeviceId := distinctId, deviceId + if aggregationGroupTypeIndex != nil { + // A condition-level group override uses the group's key, not the + // original person's ID or device, for dependent flag bucketing. + dependencyDistinctId, dependencyDeviceId = bucketingId, nil + } for _, prop := range condition.Properties { if prop.Type == "cohort" { - isMatch, err = poller.matchCohort(prop, properties, cohorts, flagsByKey, evaluationCache, distinctId, deviceId) + isMatch, err = poller.matchCohort(prop, properties, cohorts, flagsByKey, evaluationCache, dependencyDistinctId, dependencyDeviceId, aggregationGroupTypeIndex, state) } else if prop.Type == "flag" { - isMatch, err = poller.evaluateFlagDependency(prop, flagsByKey, evaluationCache, distinctId, deviceId, properties, cohorts) + isMatch, err = poller.evaluateFlagDependency(prop, flagsByKey, evaluationCache, dependencyDistinctId, dependencyDeviceId, properties, cohorts, aggregationGroupTypeIndex, state) } else { - isMatch, err = matchProperty(prop, properties) + isMatch, err = matchProperty(prop, properties, state.propertyMatchingVersion) } if err != nil { @@ -1150,7 +1205,7 @@ func (poller *FeatureFlagsPoller) isConditionMatch( return conditionMatch, nil } -func matchProperty(property FlagProperty, properties Properties) (bool, error) { +func matchProperty(property FlagProperty, properties Properties, matchingVersions ...int) (bool, error) { key := property.Key operator := property.Operator value := property.Value @@ -1165,7 +1220,7 @@ func matchProperty(property FlagProperty, properties Properties) (bool, error) { override_value := properties[key] if operator == "exact" || operator == "is_not" { - matched, err := exactMatch(value, override_value) + matched, err := exactMatch(value, override_value, matchingVersions...) if err != nil { return false, err } @@ -1762,15 +1817,20 @@ func interfaceToFloat(val interface{}) (float64, error) { return i, nil } -// exactMatch mirrors the flags service's exact operator, including its -// boolean-array precedence. Integral float64 filter values are inconclusive +// exactMatch defaults to the flags service's legacy boolean-array precedence. +// Only version 2 uses explicit equality, retaining empty-array truthiness. +// Integral float64 filter values are inconclusive // because JSON decoding collapses integer and floating-point number kinds. -func exactMatch(value interface{}, overrideValue interface{}) (bool, error) { - if isTruthyOrFalsyPropertyValue(value) { +func exactMatch(value interface{}, overrideValue interface{}, matchingVersions ...int) (bool, error) { + explicitMatching := len(matchingVersions) > 0 && matchingVersions[0] == 2 + if !explicitMatching && isTruthyOrFalsyPropertyValue(value) { return isTruthyPropertyValue(value) == isTruthyPropertyValue(overrideValue), nil } if list, ok := value.([]interface{}); ok { + if len(list) == 0 { + return isTruthyPropertyValue(overrideValue), nil + } hadAmbiguousNumber := false overrideStr := unicodeLower(exactValueToString(overrideValue)) for _, item := range list { @@ -2158,6 +2218,14 @@ func calculateHash(key, distinctId, salt string) float64 { // GetFeatureFlags returns the locally loaded feature flag definitions. // It waits for the initial poll to complete and returns an error if flags were not loaded. func (poller *FeatureFlagsPoller) GetFeatureFlags() ([]FeatureFlag, error) { + state, err := poller.getLoadedState() + if err != nil { + return nil, err + } + return state.featureFlags, nil +} + +func (poller *FeatureFlagsPoller) getLoadedState() (*flagsState, error) { // When channel is open this will block. When channel is closed it will immediately exit. <-poller.firstFeatureFlagRequestFinished @@ -2168,7 +2236,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlags() ([]FeatureFlag, error) { return nil, errors.New("flags were not successfully fetched yet") } - return state.featureFlags, nil + return state, nil } // getState returns the current flags state or nil if not initialized. @@ -2225,7 +2293,7 @@ func preParsePG(pg PropertyGroup) PropertyGroup { Value: getSafeProp[any](prop, "value"), Type: getSafeProp[string](prop, "type"), Negation: getSafeProp[bool](prop, "negation"), - DependencyChain: getSafeProp[[]string](prop, "dependency_chain"), + DependencyChain: parseDependencyChain(prop["dependency_chain"]), }, }) } @@ -2234,6 +2302,27 @@ func preParsePG(pg PropertyGroup) PropertyGroup { return pg } +// parseDependencyChain handles both typed helper inputs and JSON-decoded cohort leaves. +// Malformed chains remain nil so dependency evaluation treats them as inconclusive. +func parseDependencyChain(value any) []string { + if chain, ok := value.([]string); ok { + return chain + } + values, ok := value.([]any) + if !ok { + return nil + } + chain := make([]string, len(values)) + for i, value := range values { + key, ok := value.(string) + if !ok { + return nil + } + chain[i] = key + } + return chain +} + // preDecodePayloads converts json.RawMessage payloads to strings once at load time, // so evaluations can use the decoded strings directly without per-call allocation. func preDecodePayloads(flags []FeatureFlag) { @@ -2357,24 +2446,22 @@ func (poller *FeatureFlagsPoller) getFeatureFlagVariants(distinctId string, devi // $feature/ property for each entry and excludes false-valued flags from // $active_feature_flags, matching the other SDKs. func (poller *FeatureFlagsPoller) getFeatureFlagVariantsWithFallback(distinctId string, deviceId *string, groups Groups, personProperties Properties, groupProperties map[string]Properties, onlyEvaluateLocally bool) (map[string]interface{}, error) { - var flags []FeatureFlag + var state *flagsState if onlyEvaluateLocally { - // Strictly local: wait for the initial fetch and surface its error, since there - // is no remote path to fall back to. - loaded, err := poller.GetFeatureFlags() + var err error + state, err = poller.getLoadedState() if err != nil { return nil, err } - flags = loaded - } else if state := poller.getState(); state != nil { - // Default path: non-blocking read of already-loaded definitions. A nil state - // (initial fetch not finished, or it failed) is treated as "no definitions" and - // falls through to the remote /flags request below, preserving prior default - // behavior without stalling the first capture on the local-eval fetch. - flags = state.featureFlags + } else { + // Capture's default path must not block on the initial definitions request. + state = poller.getState() + } + var flags []FeatureFlag + var cohorts map[string]PropertyGroup + if state != nil { + flags, cohorts = state.featureFlags, state.cohorts } - - cohorts := poller.getCohorts() result := make(map[string]interface{}, len(flags)) // Fall back to the remote request when no definitions are loaded or any flag @@ -2389,6 +2476,7 @@ func (poller *FeatureFlagsPoller) getFeatureFlagVariantsWithFallback(distinctId personProperties, groupProperties, cohorts, + state, ) if computeErr != nil { // Any flag we can't evaluate locally — experience continuity, missing @@ -2457,3 +2545,12 @@ func getSafeProp[T any](properties map[string]any, key string) T { return defaultValue } } + +// evaluationState preserves legacy defaults for context-free helper callers. +// Production evaluation passes the snapshot selected with the flag and never reloads it. +func (poller *FeatureFlagsPoller) evaluationState(snapshots []*flagsState) *flagsState { + if len(snapshots) > 0 && snapshots[0] != nil { + return snapshots[0] + } + return &flagsState{flagsByKey: poller.getFlagsByKey(), groups: poller.getGroups()} +} diff --git a/featureflags_cohort_continuity_test.go b/featureflags_cohort_continuity_test.go new file mode 100644 index 0000000..7945e55 --- /dev/null +++ b/featureflags_cohort_continuity_test.go @@ -0,0 +1,69 @@ +package posthog + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + json "github.com/goccy/go-json" +) + +func TestCohortFlagDependencyExperienceContinuity(t *testing.T) { + for _, version := range []int{0, 1, 2, 3} { + for _, continuity := range []bool{false, true} { + t.Run(fmt.Sprintf("version=%d/continuity=%t", version, continuity), func(t *testing.T) { + body := fmt.Sprintf(`{"property_matching_version":%d,"flags":[ + {"key":"sticky","active":true,"ensure_experience_continuity":%t,"filters":{"groups":[{"properties":[]}]}}, + {"key":"target","active":true,"filters":{"groups":[{"properties":[{"type":"cohort","key":"id","value":"c"}]}]}} + ],"cohorts":{"c":{"type":"OR","values":[{"type":"flag","key":"sticky","operator":"flag_evaluates_to","value":true,"dependency_chain":["sticky"]}]}}}`, version, continuity) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/flags/definitions" { + t.Errorf("unexpected remote request: %s", r.URL.Path) + w.WriteHeader(500) + return + } + fmt.Fprint(w, body) + })) + defer server.Close() + poller := newTestPoller(t, server.URL) + poller.firstFeatureFlagRequestFinished = make(chan bool) + poller.fetchNewFeatureFlags() + close(poller.firstFeatureFlagRequestFinished) + // The cohort dependency must remain inconclusive just like the direct flag. + for _, key := range []string{"sticky", "target"} { + config := FeatureFlagPayload{Key: key, DistinctId: "person", OnlyEvaluateLocally: true} + got, local, err := poller.GetFeatureFlag(config) + if continuity { + if err == nil || local { + t.Errorf("%s=%v local=%v err=%v; must remain inconclusive", key, got, local, err) + } + full := poller.GetFeatureFlagWithPayload(config) + if full.err == nil || full.locallyEvaluated { + t.Errorf("%s full result must remain inconclusive: %+v", key, full) + } + } else if err != nil || !local || got != true { + t.Errorf("%s=%v local=%v err=%v; want local true", key, got, local, err) + } + } + // Exercise raw hydration as well as the pre-parsed production path above. + var raw FeatureFlagsResponse + if err := json.Unmarshal([]byte(body), &raw); err != nil { + t.Fatal(err) + } + state := poller.state.Load() + cache := map[string]interface{}{} + for i := 0; i < 2; i++ { + got, err := poller.matchCohort(FlagProperty{Value: "c"}, nil, raw.Cohorts, state.flagsByKey, cache, "person", nil, nil, state) + if continuity { + if err == nil { + t.Errorf("raw cohort=%v; must remain inconclusive, including cached dependency", got) + } + } else if err != nil || !got { + t.Errorf("raw cohort=%v err=%v; want true", got, err) + } + } + }) + } + } +} diff --git a/featureflags_cohort_group_context_test.go b/featureflags_cohort_group_context_test.go new file mode 100644 index 0000000..b53b545 --- /dev/null +++ b/featureflags_cohort_group_context_test.go @@ -0,0 +1,194 @@ +package posthog + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + json "github.com/goccy/go-json" +) + +func TestCohortFlagDependencyGroupContext(t *testing.T) { + for _, version := range []int{0, 1, 2, 3} { + for _, aggregation := range []string{"person", "flag", "condition"} { + for _, shape := range []string{"OR", "AND", "negated", "nested", "indirect"} { + t.Run(fmt.Sprintf("v%d/%s/%s", version, aggregation, shape), func(t *testing.T) { + flagAggregation, conditionAggregation, propertyType := "", "", "person" + if aggregation == "flag" { + flagAggregation, propertyType = `"aggregation_group_type_index":0,`, "group" + } else if aggregation == "condition" { + conditionAggregation, propertyType = `"aggregation_group_type_index":0,`, "group" + } + leaf := `{"type":"flag","key":"dep","operator":"flag_evaluates_to","value":true,"dependency_chain":["dep"]}` + cohort := fmt.Sprintf(`{"type":"OR","values":[%s]}`, leaf) + switch shape { + case "AND": + cohort = fmt.Sprintf(`{"type":"AND","values":[%s]}`, leaf) + case "negated": + cohort = `{"type":"OR","values":[{"type":"flag","key":"dep","operator":"flag_evaluates_to","value":true,"negation":true,"dependency_chain":["dep"]}]}` + case "nested": + cohort = fmt.Sprintf(`{"type":"AND","values":[{"type":"OR","values":[%s]}]}`, leaf) + case "indirect": + cohort = `{"type":"AND","values":[{"type":"flag","key":"middle","operator":"flag_evaluates_to","value":true,"dependency_chain":["middle"]}]}` + } + body := fmt.Sprintf(`{"property_matching_version":%d,"group_type_mapping":{"0":"company"},"flags":[ + {"key":"dep","active":true,"filters":{%s"groups":[{%s"properties":[{"key":"plan","type":%q,"operator":"exact","value":"pro"}]}]}}, + {"key":"middle","active":true,"filters":{"groups":[{"properties":[%s]}]}}, + {"key":"target","active":true,"filters":{"groups":[{"properties":[{"type":"cohort","key":"id","value":"c"}]}],"payloads":{"true":"on","false":"off"}}} + ],"cohorts":{"c":%s}}`, version, flagAggregation, conditionAggregation, propertyType, leaf, cohort) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/flags/definitions" { + t.Errorf("local-only evaluation made a remote request: %s", r.URL.Path) + w.WriteHeader(500) + return + } + fmt.Fprint(w, body) + })) + defer server.Close() + poller := newTestPoller(t, server.URL) + poller.firstFeatureFlagRequestFinished = make(chan bool) + poller.fetchNewFeatureFlags() + close(poller.firstFeatureFlagRequestFinished) + properties := Properties{"plan": "free"} + if aggregation == "person" { + properties["plan"] = "pro" + } + config := FeatureFlagPayload{Key: "dep", DistinctId: "person", PersonProperties: properties, Groups: Groups{"company": "acme"}, GroupProperties: map[string]Properties{"company": {"plan": "pro"}}, OnlyEvaluateLocally: true} + direct, local, err := poller.GetFeatureFlag(config) + if err != nil || !local || direct != true { + t.Fatalf("control dep=%v local=%v err=%v", direct, local, err) + } + config.Key = "target" + want := shape != "negated" + got, local, err := poller.GetFeatureFlag(config) + full := poller.GetFeatureFlagWithPayload(config) + if aggregation == "person" { + if err != nil || !local || got != want || full.err != nil || !full.locallyEvaluated || full.value != want { + t.Errorf("person dependency=%v local=%v err=%v full=%+v; want %v", got, local, err, full, want) + } + } else { + if !isServerEvalError(err) || local || !isServerEvalError(full.err) || full.locallyEvaluated { + t.Errorf("dependency=%v local=%v err=%v full=%+v; must require server evaluation", got, local, err, full) + } + if payload, err := poller.GetFeatureFlagPayload(config); err == nil || payload != "" { + t.Errorf("payload=%q error=%v; must remain inconclusive", payload, err) + } + all, err := poller.GetAllFlags(FeatureFlagPayloadNoKey{DistinctId: config.DistinctId, PersonProperties: properties, Groups: config.Groups, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}) + if _, exists := all["target"]; err != nil || exists { + t.Errorf("bulk flags=%v err=%v; must omit target", all, err) + } + c := &client{Config: Config{Logger: newDefaultLogger(false)}, featureFlagsPoller: poller} + evaluations, err := c.EvaluateFlags(EvaluateFlagsPayload{DistinctId: config.DistinctId, PersonProperties: properties, Groups: config.Groups, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}) + if err != nil { + t.Fatal(err) + } + if _, exists := evaluations.flags["target"]; exists { + t.Error("EvaluateFlags must omit target") + } + captured, err := poller.getFeatureFlagVariantsWithFallback(config.DistinctId, nil, config.Groups, properties, config.GroupProperties, true) + if _, exists := captured["target"]; err != nil || exists { + t.Errorf("capture flags=%v err=%v; must omit target", captured, err) + } + } + var raw FeatureFlagsResponse + if err := json.Unmarshal([]byte(body), &raw); err != nil { + t.Fatal(err) + } + state := poller.state.Load() + for _, cohorts := range []map[string]PropertyGroup{raw.Cohorts, state.cohorts} { + // Repeated references must preserve server-required errors, even + // if a caller already cached a result without group context. + cache := map[string]interface{}{} + if aggregation != "person" && shape != "indirect" { + cache["dep"] = false + } + for i := 0; i < 2; i++ { + got, err := poller.matchCohort(FlagProperty{Value: "c"}, properties, cohorts, state.flagsByKey, cache, "person", nil, nil, state) + if aggregation == "person" { + if err != nil || got != want { + t.Errorf("cohort=%v err=%v; want %v", got, err, want) + } + } else if !isServerEvalError(err) { + t.Errorf("cohort=%v err=%v; must require server evaluation on call %d", got, err, i) + } + } + } + }) + } + } + } +} + +func TestGroupContextPersonFlagDependency(t *testing.T) { + for _, version := range []int{1, 2} { + for _, aggregation := range []string{"flag", "condition"} { + for _, shape := range []string{"direct", "cohort", "nested", "negated"} { + t.Run(fmt.Sprintf("v%d/%s/%s", version, aggregation, shape), func(t *testing.T) { + flagAggregation, conditionAggregation := `"aggregation_group_type_index":0,`, "" + if aggregation == "condition" { + flagAggregation, conditionAggregation = "", `"aggregation_group_type_index":0,` + } + leaf := `{"type":"flag","key":"dep","operator":"flag_evaluates_to","value":true,"dependency_chain":["dep"]}` + cohort := fmt.Sprintf(`{"type":"OR","values":[%s]}`, leaf) + if shape == "nested" { + cohort = `{"type":"AND","values":[{"type":"OR","values":[{"type":"cohort","value":"inner"}]}]}` + } else if shape == "negated" { + cohort = `{"type":"OR","values":[{"type":"flag","key":"dep","operator":"flag_evaluates_to","value":true,"negation":true,"dependency_chain":["dep"]}]}` + } + targetProperty := `{"type":"cohort","value":"c"}` + if shape == "direct" { + targetProperty = leaf + } + body := fmt.Sprintf(`{"property_matching_version":%d,"group_type_mapping":{"0":"company"},"flags":[ + {"key":"dep","active":true,"filters":{"groups":[{"properties":[{"type":"person","key":"plan","operator":"exact","value":"free"}]}]}}, + {"key":"target","active":true,"filters":{%s"groups":[{%s"properties":[%s]}]}}, + {"key":"property-only","active":true,"filters":{%s"groups":[{%s"properties":[{"type":"cohort","value":"properties"}]}]}} + ],"cohorts":{"c":%s,"inner":{"type":"OR","values":[%s]},"properties":{"type":"AND","values":[{"type":"group","key":"plan","operator":"exact","value":"pro"}]}}}`, version, flagAggregation, conditionAggregation, targetProperty, flagAggregation, conditionAggregation, cohort, leaf) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/flags/definitions" { + t.Errorf("local-only evaluation made remote request: %s", r.URL.Path) + w.WriteHeader(500) + return + } + fmt.Fprint(w, body) + })) + defer server.Close() + poller := newTestPoller(t, server.URL) + poller.firstFeatureFlagRequestFinished = make(chan bool) + poller.fetchNewFeatureFlags() + close(poller.firstFeatureFlagRequestFinished) + config := FeatureFlagPayload{DistinctId: "person", PersonProperties: Properties{"plan": "free"}, Groups: Groups{"company": "acme"}, GroupProperties: map[string]Properties{"company": {"plan": "pro"}}, OnlyEvaluateLocally: true} + for _, key := range []string{"dep", "property-only", "target"} { + config.Key = key + got, local, err := poller.GetFeatureFlag(config) + if key == "target" { + if !isServerEvalError(err) || local { + t.Errorf("target=%v local=%v err=%v; must require server evaluation", got, local, err) + } + } else if err != nil || !local || got != true { + t.Errorf("control %s=%v local=%v err=%v; want local true", key, got, local, err) + } + } + var raw FeatureFlagsResponse + if err := json.Unmarshal([]byte(body), &raw); err != nil { + t.Fatal(err) + } + state := poller.state.Load() + for _, cohorts := range []map[string]PropertyGroup{raw.Cohorts, state.cohorts} { + groupIndex := uint8(0) + for _, cached := range []interface{}{nil, false, true} { + cache := map[string]interface{}{"dep": cached} + for i := 0; i < 2; i++ { + got, err := poller.matchCohort(FlagProperty{Value: "c"}, config.GroupProperties["company"], cohorts, state.flagsByKey, cache, "acme", nil, &groupIndex, state) + if !isServerEvalError(err) { + t.Errorf("cohort=%v err=%v cached=%v; must require server evaluation", got, err, cached) + } + } + } + } + }) + } + } + } +} diff --git a/featureflags_matching_version_test.go b/featureflags_matching_version_test.go new file mode 100644 index 0000000..de1ca70 --- /dev/null +++ b/featureflags_matching_version_test.go @@ -0,0 +1,360 @@ +package posthog + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + json "github.com/goccy/go-json" +) + +func TestFeatureFlagsResponseUnkeyedLiteralCompatibility(t *testing.T) { + // Keep the released field count and order: consumers may use unkeyed literals. + response := FeatureFlagsResponse{nil, nil, nil, false} + body := `{"flags":[{"key":"example"}],"group_type_mapping":{"0":"company"},"cohorts":{"cohort":{"type":"AND","values":[]}},"minimal_flag_called_events":true,"property_matching_version":2}` + if err := json.Unmarshal([]byte(body), &response); err != nil { + t.Fatal(err) + } + if len(response.Flags) != 1 || response.Flags[0].Key != "example" { + t.Fatalf("flags = %v", response.Flags) + } + if response.GroupTypeMapping == nil || (*response.GroupTypeMapping)["0"] != "company" { + t.Fatalf("group mapping = %v", response.GroupTypeMapping) + } + if len(response.Cohorts) != 1 || response.Cohorts["cohort"].Type != "AND" { + t.Fatalf("cohorts = %v", response.Cohorts) + } + if !response.MinimalFlagCalledEvents { + t.Fatal("minimal flag called events was not decoded") + } +} + +// Exercise the wire envelope and public local-only APIs, not just the matcher. +func TestPropertyMatchingVersionDefinitions(t *testing.T) { + rows := []struct { + name, filter, property string + legacy, v2 bool + }{ + {"false banana", `false`, `"banana"`, true, false}, + {"false zero", `false`, `0`, true, false}, + {"boolean list true", `["true","false"]`, `"true"`, false, true}, + {"boolean list pro", `["true","false"]`, `"pro"`, true, false}, + {"empty true", `[]`, `true`, true, true}, + {"empty array", `[]`, `[]`, true, true}, + {"true array", `true`, `[true]`, true, false}, + {"false uppercase", `false`, `"FALSE"`, true, true}, + {"false null", `false`, `null`, true, false}, + {"false empty string", `false`, `""`, true, false}, + {"true empty property array", `true`, `[]`, true, false}, + {"nested filter member", `[[true],"pro"]`, `[true]`, true, true}, + {"empty nested truthy", `[]`, `[true,"TRUE",[]]`, true, true}, + {"empty false", `[]`, `false`, false, false}, + {"empty number", `[]`, `1`, false, false}, + {"empty arbitrary string", `[]`, `"banana"`, false, false}, + {"mixed members", `[true,"pro"]`, `"TRUE"`, true, true}, + {"normalized strings", `["FREE","PRO"]`, `"pro"`, true, true}, + {"unicode lowercase", `"İ"`, `"i̇"`, true, true}, + {"null equality", `null`, `null`, true, true}, + } + for _, version := range []string{"", `,"property_matching_version":1`, `,"property_matching_version":2`, `,"property_matching_version":3`, `,"property_matching_version":0`} { + for _, row := range rows { + for _, operator := range []string{"exact", "is_not"} { + t.Run(version+"/"+row.name+"/"+operator, func(t *testing.T) { + body := fmt.Sprintf(`{"flags":[{"key":"test","active":true,"filters":{"groups":[{"properties":[{"key":"value","type":"person","operator":%q,"value":%s}]}],"payloads":{"true":"on","false":"off"}}}]%s}`, operator, row.filter, version) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/flags/definitions" { + t.Errorf("unexpected remote request: %s", r.URL.Path) + w.WriteHeader(500) + return + } + fmt.Fprint(w, body) + })) + defer server.Close() + poller := newTestPoller(t, server.URL) + poller.firstFeatureFlagRequestFinished = make(chan bool) + poller.fetchNewFeatureFlags() + close(poller.firstFeatureFlagRequestFinished) + var property interface{} + if err := json.Unmarshal([]byte(row.property), &property); err != nil { + t.Fatal(err) + } + config := FeatureFlagPayload{Key: "test", DistinctId: "person", PersonProperties: Properties{"value": property}, OnlyEvaluateLocally: true} + want := row.legacy + if version == `,"property_matching_version":2` { + want = row.v2 + } + if operator == "is_not" { + want = !want + } + got, local, err := poller.GetFeatureFlag(config) + if err != nil || !local || got != want { + t.Fatalf("got %v local=%v err=%v; want %v", got, local, err, want) + } + full := poller.GetFeatureFlagWithPayload(config) + payload := "off" + if want { + payload = "on" + } + if full.err != nil || !full.locallyEvaluated || full.value != want || full.payload != payload { + t.Errorf("full result: %+v; want %v/%s", full, want, payload) + } + gotPayload, err := poller.GetFeatureFlagPayload(config) + if err != nil || gotPayload != payload { + t.Errorf("payload=%s err=%v", gotPayload, err) + } + }) + } + } + } +} + +const matchingVersionDefinitions = `{ + "flags": [ + {"key":"person","active":true,"filters":{"groups":[{"properties":[{"key":"value","type":"person","operator":"exact","value":false}]}]}}, + {"key":"group","active":true,"filters":{"aggregation_group_type_index":0,"groups":[{"properties":[{"key":"value","type":"group","operator":"exact","value":false}]}]}}, + {"key":"mixed","active":true,"filters":{"groups":[{"aggregation_group_type_index":0,"properties":[{"key":"value","type":"group","operator":"exact","value":false}]}]}}, + {"key":"cohort","active":true,"filters":{"groups":[{"properties":[{"key":"id","type":"cohort","value":"outer"}]}]}}, + {"key":"dependency","active":true,"filters":{"groups":[{"properties":[{"key":"person","type":"flag","operator":"flag_evaluates_to","value":true,"dependency_chain":["person"]}]}]}}, + {"key":"cohort-dependency","active":true,"filters":{"groups":[{"properties":[{"key":"id","type":"cohort","value":"dependency"}]}]}} + ], + "group_type_mapping":{"0":"company"}, + "cohorts":{ + "outer":{"type":"AND","values":[{"type":"OR","values":[{"key":"id","type":"cohort","value":"inner"}]}]}, + "inner":{"type":"AND","values":[{"key":"value","type":"person","operator":"exact","value":false}]}, + "dependency":{"type":"AND","values":[{"key":"person","type":"flag","operator":"flag_evaluates_to","value":true,"dependency_chain":["person"]}]} + } + %s +}` + +func TestPropertyMatchingVersionReloadAndPropagation(t *testing.T) { + body := "" + status := http.StatusOK + etag := "initial" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/flags/definitions" { + t.Errorf("unexpected remote request: %s", r.URL.Path) + w.WriteHeader(500) + return + } + w.Header().Set("ETag", etag) + w.WriteHeader(status) + fmt.Fprint(w, body) + })) + defer server.Close() + poller := newTestPoller(t, server.URL) + poller.firstFeatureFlagRequestFinished = make(chan bool) + close(poller.firstFeatureFlagRequestFinished) + config := FeatureFlagPayloadNoKey{DistinctId: "person", PersonProperties: Properties{"value": "banana"}, Groups: Groups{"company": "acme"}, GroupProperties: map[string]Properties{"company": {"value": "banana"}}, OnlyEvaluateLocally: true} + // Identical definitions, with only the selector changing. Dependency result + // caches must not survive an evaluation, including a version-only reload. + for _, step := range []struct { + name, version string + status int + malformed bool + want bool + storedVersion int + }{ + {"missing", "", 200, false, true, 0}, + {"v1", `,"property_matching_version":1`, 200, false, true, 1}, + {"v2", `,"property_matching_version":2`, 200, false, false, 2}, + {"back to v1", `,"property_matching_version":1`, 200, false, true, 1}, + {"v2 again", `,"property_matching_version":2`, 200, false, false, 2}, + {"304 new etag", "", 304, false, false, 2}, + {"304 no etag", "", 304, false, false, 2}, + {"server failure", "", 500, false, false, 2}, + {"decode failure", "", 200, true, false, 2}, + {"omitted resets", "", 200, false, true, 0}, + {"unknown legacy", `,"property_matching_version":3`, 200, false, true, 3}, + } { + t.Run(step.name, func(t *testing.T) { + status = step.status + body = fmt.Sprintf(matchingVersionDefinitions, step.version) + if step.malformed { + body = "{" + } + etag = step.name + if step.name == "304 no etag" { + etag = "" + } + oldState := poller.state.Load() + poller.fetchNewFeatureFlags() + state := poller.state.Load() + if state == nil || state.propertyMatchingVersion != step.storedVersion { + t.Fatalf("unexpected snapshot: %+v", state) + } + if (step.status == 500 || step.malformed || step.name == "304 no etag") && state != oldState { + t.Error("unchanged response replaced snapshot") + } + if step.name == "304 new etag" && state.flagsEtag != etag { + t.Error("304 did not update etag") + } + for _, flag := range state.featureFlags { + got, local, err := poller.GetFeatureFlag(FeatureFlagPayload{Key: flag.Key, DistinctId: config.DistinctId, PersonProperties: config.PersonProperties, Groups: config.Groups, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}) + if err != nil || !local || got != step.want { + t.Errorf("%s: got %v local=%v err=%v want=%v", flag.Key, got, local, err, step.want) + } + } + all, err := poller.GetAllFlags(config) + assertMatchingVersionFlags(t, all, err, step.want) + assertMatchingVersionEvaluations(t, poller, config, step.want) + for _, onlyLocal := range []bool{false, true} { + captured, err := poller.getFeatureFlagVariantsWithFallback(config.DistinctId, nil, config.Groups, config.PersonProperties, config.GroupProperties, onlyLocal) + assertMatchingVersionFlags(t, captured, err, step.want) + } + // Raw cohort hydration compatibility path must agree with pre-parsed cache. + var raw FeatureFlagsResponse + if err := json.Unmarshal([]byte(fmt.Sprintf(matchingVersionDefinitions, "")), &raw); err != nil { + t.Fatal(err) + } + got, err := poller.matchCohort(FlagProperty{Value: "outer"}, config.PersonProperties, raw.Cohorts, state.flagsByKey, map[string]interface{}{}, "person", nil, nil, state) + if err != nil || got != step.want { + t.Errorf("raw cohort got %v err=%v", got, err) + } + }) + } +} + +func assertMatchingVersionFlags(t *testing.T, flags map[string]interface{}, err error, want bool) { + t.Helper() + if err != nil || len(flags) != 6 { + t.Fatalf("flags=%v err=%v", flags, err) + } + for key, value := range flags { + if value != want { + t.Errorf("%s=%v want=%v", key, value, want) + } + } +} + +func TestPropertyMatchingVersionSafeguards(t *testing.T) { + for _, version := range []int{0, 1, 2, 3} { + for _, operator := range []string{"exact", "is_not"} { + prop := FlagProperty{Key: "value", Value: false, Operator: operator} + _, err := matchProperty(prop, Properties{}, version) + if err != errMissingPropertyValue { + t.Errorf("version %d %s missing error=%v", version, operator, err) + } + prop.Value = []interface{}{float64(1), "pro"} + _, err = matchProperty(prop, Properties{"value": "1"}, version) + if err != errAmbiguousExactNumber { + t.Errorf("version %d %s numeric error=%v", version, operator, err) + } + got, err := matchProperty(prop, Properties{"value": "PRO"}, version) + if err != nil || got != (operator == "exact") { + t.Errorf("unambiguous member: got %v err=%v", got, err) + } + prop.Value = `{"a":[true,"İ"],"b":null}` + got, err = matchProperty(prop, Properties{"value": map[string]interface{}{"b": nil, "a": []interface{}{true, "i̇"}}}, version) + if err != nil || got != (operator == "exact") { + t.Errorf("canonical composite: got %v err=%v", got, err) + } + } + } + got, err := matchProperty(FlagProperty{Key: "value", Value: false, Operator: "exact"}, Properties{"value": "banana"}) + if err != nil || !got { + t.Fatalf("context-free helper lost legacy default: %v %v", got, err) + } +} + +func TestPreParsePGDependencyChain(t *testing.T) { + for _, chain := range []interface{}{[]string{"person"}, []interface{}{"person"}} { + pg := preParsePG(PropertyGroup{Type: "AND", Values: []any{map[string]any{"type": "flag", "key": "person", "operator": "flag_evaluates_to", "value": true, "dependency_chain": chain}}}) + got := pg.ParsedValues[0].Property.DependencyChain + if len(got) != 1 || got[0] != "person" { + t.Errorf("chain %#v parsed as %#v", chain, got) + } + } + for _, chain := range []interface{}{nil, "person", []interface{}{1}, []interface{}{"person", nil}} { + pg := preParsePG(PropertyGroup{Type: "AND", Values: []any{map[string]any{"dependency_chain": chain}}}) + if pg.ParsedValues[0].Property.DependencyChain != nil { + t.Errorf("malformed chain %#v accepted", chain) + } + } +} + +// A JSON property can run user code during normalization. Swap definitions there +// to deterministically simulate a poll completing in the middle of evaluation. +type matchingVersionSwapProperty struct{ swap func() } + +func (p matchingVersionSwapProperty) MarshalJSON() ([]byte, error) { + p.swap() + return []byte(`"go"`), nil +} + +func TestPropertyMatchingVersionSnapshotPinned(t *testing.T) { + poller := &FeatureFlagsPoller{Logger: newDefaultLogger(false), firstFeatureFlagRequestFinished: make(chan bool)} + close(poller.firstFeatureFlagRequestFinished) + var definitions FeatureFlagsResponse + if err := json.Unmarshal([]byte(fmt.Sprintf(matchingVersionDefinitions, `,"property_matching_version":1`)), &definitions); err != nil { + t.Fatal(err) + } + for i := range definitions.Flags { + flag := &definitions.Flags[i] + for j := range flag.Filters.Groups { + condition := &flag.Filters.Groups[j] + condition.Properties = append([]FlagProperty{{Key: "gate", Value: "go", Operator: "exact"}}, condition.Properties...) + } + flag.Filters.Payloads = map[string]json.RawMessage{"true": json.RawMessage(`"on"`), "false": json.RawMessage(`"off"`)} + } + preDecodePayloads(definitions.Flags) + legacy := &flagsState{featureFlags: definitions.Flags, flagsByKey: buildFlagsByKey(definitions.Flags), groups: *definitions.GroupTypeMapping, cohorts: preParseCohortValues(definitions.Cohorts), propertyMatchingVersion: 1} + // A changed group mapping and missing cohorts/dependency index make accidental + // re-reads of any component of the snapshot observable, not just the version. + replacement := &flagsState{featureFlags: definitions.Flags, flagsByKey: map[string]FeatureFlag{}, groups: map[string]string{"0": "other"}, propertyMatchingVersion: 2} + swaps := 0 + gate := matchingVersionSwapProperty{swap: func() { swaps++; poller.state.Store(replacement) }} + properties := Properties{"gate": gate, "value": "banana"} + config := FeatureFlagPayload{Key: "cohort-dependency", DistinctId: "person", PersonProperties: properties, Groups: Groups{"company": "acme"}, GroupProperties: map[string]Properties{"company": properties}, OnlyEvaluateLocally: true} + for _, api := range []string{"value", "payload", "full", "all", "capture local", "capture default", "evaluations"} { + t.Run(api, func(t *testing.T) { + poller.state.Store(legacy) + swaps = 0 + switch api { + case "value": + got, local, err := poller.GetFeatureFlag(config) + if err != nil || !local || got != true { + t.Errorf("got %v local=%v err=%v", got, local, err) + } + case "payload": + got, err := poller.GetFeatureFlagPayload(config) + if err != nil || got != "on" { + t.Errorf("got %v err=%v", got, err) + } + case "full": + got := poller.GetFeatureFlagWithPayload(config) + if got.err != nil || !got.locallyEvaluated || got.value != true || got.payload != "on" { + t.Errorf("got %+v", got) + } + case "evaluations": + assertMatchingVersionEvaluations(t, poller, FeatureFlagPayloadNoKey{DistinctId: config.DistinctId, PersonProperties: properties, Groups: config.Groups, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}, true) + case "all": + got, err := poller.GetAllFlags(FeatureFlagPayloadNoKey{DistinctId: config.DistinctId, PersonProperties: properties, Groups: config.Groups, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}) + assertMatchingVersionFlags(t, got, err, true) + default: + got, err := poller.getFeatureFlagVariantsWithFallback(config.DistinctId, nil, config.Groups, properties, config.GroupProperties, api == "capture local") + assertMatchingVersionFlags(t, got, err, true) + } + if swaps == 0 || poller.state.Load() != replacement { + t.Fatal("test did not swap definitions during matching") + } + }) + } +} + +func assertMatchingVersionEvaluations(t *testing.T, poller *FeatureFlagsPoller, config FeatureFlagPayloadNoKey, want bool) { + t.Helper() + c := &client{featureFlagsPoller: poller} + evaluations, err := c.EvaluateFlags(EvaluateFlagsPayload{DistinctId: config.DistinctId, PersonProperties: config.PersonProperties, Groups: config.Groups, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}) + if err != nil { + t.Fatal(err) + } + flags := map[string]interface{}{} + for key, record := range evaluations.flags { + if !record.LocallyEvaluated { + t.Errorf("%s not locally evaluated", key) + } + flags[key] = record.Enabled + } + assertMatchingVersionFlags(t, flags, nil, want) +} diff --git a/featureflags_same_group_dependency_test.go b/featureflags_same_group_dependency_test.go new file mode 100644 index 0000000..28584b0 --- /dev/null +++ b/featureflags_same_group_dependency_test.go @@ -0,0 +1,171 @@ +package posthog + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + json "github.com/goccy/go-json" +) + +func TestSameGroupFlagDependency(t *testing.T) { + for _, version := range []int{0, 1, 2, 3} { + for _, aggregation := range []string{"flag", "condition"} { + for _, shape := range []string{"direct", "cohort", "nested", "indirect"} { + t.Run(fmt.Sprintf("v%d/%s/%s", version, aggregation, shape), func(t *testing.T) { + flagAggregation, conditionAggregation := `"aggregation_group_type_index":0,`, "" + if aggregation == "condition" { + flagAggregation, conditionAggregation = "", flagAggregation + } + leaf := `{"type":"flag","key":"dep","operator":"flag_evaluates_to","value":true,"dependency_chain":["dep"]}` + targetProperty := leaf + if shape == "cohort" || shape == "nested" { + targetProperty = `{"type":"cohort","value":"outer"}` + } else if shape == "indirect" { + targetProperty = `{"type":"flag","key":"middle","operator":"flag_evaluates_to","value":true,"dependency_chain":["dep","middle"]}` + } + cohort := fmt.Sprintf(`{"type":"OR","values":[%s]}`, leaf) + if shape == "nested" { + cohort = `{"type":"AND","values":[{"type":"OR","values":[{"type":"cohort","value":"inner"}]}]}` + } + body := fmt.Sprintf(`{"property_matching_version":%d,"group_type_mapping":{"0":"company"},"flags":[ + {"key":"dep","active":true,"filters":{"aggregation_group_type_index":0,"groups":[{"rollout_percentage":50,"properties":[{"key":"plan","type":"group","operator":"exact","value":"pro"},{"key":"$group_key","type":"group","operator":"exact","value":"acme"}]}]}}, + {"key":"middle","active":true,"filters":{"aggregation_group_type_index":0,"groups":[{"properties":[%s]}]}}, + {"key":"target","active":true,"filters":{%s"groups":[{%s"properties":[%s]}],"payloads":{"true":"on","false":"off"}}} + ],"cohorts":{"outer":%s,"inner":{"type":"OR","values":[%s]}}}`, version, leaf, flagAggregation, conditionAggregation, targetProperty, cohort, leaf) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/flags/definitions" { + t.Errorf("same-group dependency made remote request: %s", r.URL.Path) + w.WriteHeader(500) + return + } + fmt.Fprint(w, body) + })) + defer server.Close() + poller := newTestPoller(t, server.URL) + noRetries := 0 + var err error + poller.decider, err = newFlagsClient("test-key", server.URL, http.Client{}, time.Second, poller.Logger, &noRetries) + if err != nil { + t.Fatal(err) + } + poller.firstFeatureFlagRequestFinished = make(chan bool) + poller.fetchNewFeatureFlags() + close(poller.firstFeatureFlagRequestFinished) + // Choose a person on the opposite side of the rollout from the group. + wantRollout := checkIfSimpleFlagEnabled("dep", "acme", 50) + person := "" + for i := 0; i < 100; i++ { + candidate := fmt.Sprintf("person-%d", i) + if checkIfSimpleFlagEnabled("dep", candidate, 50) != wantRollout { + person = candidate + break + } + } + if person == "" { + t.Fatal("no contrasting person rollout fixture") + } + for _, plan := range []string{"pro", "free"} { + want := plan == "pro" && wantRollout + config := FeatureFlagPayload{Key: "dep", DistinctId: person, PersonProperties: Properties{"plan": "free"}, Groups: Groups{"company": "acme"}, GroupProperties: map[string]Properties{"company": {"plan": plan}}, OnlyEvaluateLocally: true} + direct, local, err := poller.GetFeatureFlag(config) + if err != nil || !local || direct != want { + t.Fatalf("control dep=%v local=%v err=%v; want %v", direct, local, err, want) + } + config.Key = "target" + for _, onlyLocal := range []bool{true, false} { + config.OnlyEvaluateLocally = onlyLocal + got, local, err := poller.GetFeatureFlag(config) + if err != nil || !local || got != want { + t.Errorf("plan=%s onlyLocal=%v target=%v local=%v err=%v; want %v", plan, onlyLocal, got, local, err, want) + } + } + config.OnlyEvaluateLocally = true + full := poller.GetFeatureFlagWithPayload(config) + if full.err != nil || !full.locallyEvaluated || full.value != want { + t.Errorf("full=%+v; want %v", full, want) + } + all, err := poller.GetAllFlags(FeatureFlagPayloadNoKey{DistinctId: person, Groups: config.Groups, PersonProperties: config.PersonProperties, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}) + if err != nil || all["target"] != want { + t.Errorf("bulk=%v err=%v; want target=%v", all, err, want) + } + c := &client{Config: Config{Logger: newDefaultLogger(false)}, featureFlagsPoller: poller} + evaluations, err := c.EvaluateFlags(EvaluateFlagsPayload{DistinctId: person, Groups: config.Groups, PersonProperties: config.PersonProperties, GroupProperties: config.GroupProperties, OnlyEvaluateLocally: true}) + if err != nil { + t.Fatal(err) + } + if _, exists := evaluations.flags["target"]; !exists { + t.Error("EvaluateFlags omitted same-group target") + } + captured, err := poller.getFeatureFlagVariantsWithFallback(person, nil, config.Groups, config.PersonProperties, config.GroupProperties, true) + if err != nil || captured["target"] != want { + t.Errorf("capture=%v err=%v; want target=%v", captured, err, want) + } + } + }) + } + } + } +} + +func TestGroupFlagDependencyRejectsDifferentAggregationBeforeCache(t *testing.T) { + for _, version := range []int{1, 2} { + for _, aggregation := range []string{"flag", "condition"} { + t.Run(fmt.Sprintf("v%d/%s", version, aggregation), func(t *testing.T) { + flagAggregation, conditionAggregation := `"aggregation_group_type_index":1,`, "" + if aggregation == "condition" { + // Same flag-level aggregation, but a condition needs another group. + flagAggregation, conditionAggregation = `"aggregation_group_type_index":0,`, `"aggregation_group_type_index":1,` + } + body := fmt.Sprintf(`{"property_matching_version":%d,"group_type_mapping":{"0":"company","1":"team"},"flags":[ + {"key":"dep","active":true,"filters":{%s"groups":[{%s"properties":[{"type":"group","key":"plan","operator":"exact","value":"pro"}]}]}}, + {"key":"target","active":true,"filters":{"aggregation_group_type_index":0,"groups":[{"properties":[{"type":"flag","key":"dep","operator":"flag_evaluates_to","value":true,"dependency_chain":["dep"]}]}]}} + ],"cohorts":{"c":{"type":"OR","values":[{"type":"flag","key":"dep","operator":"flag_evaluates_to","value":true,"negation":true,"dependency_chain":["dep"]}]}}}`, version, flagAggregation, conditionAggregation) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/flags/definitions" { + t.Errorf("local-only made remote request: %s", r.URL.Path) + w.WriteHeader(500) + return + } + fmt.Fprint(w, body) + })) + defer server.Close() + poller := newTestPoller(t, server.URL) + poller.firstFeatureFlagRequestFinished = make(chan bool) + poller.fetchNewFeatureFlags() + close(poller.firstFeatureFlagRequestFinished) + config := FeatureFlagPayload{DistinctId: "person", Groups: Groups{"company": "acme", "team": "engineering"}, GroupProperties: map[string]Properties{"company": {"plan": "free"}, "team": {"plan": "pro"}}, OnlyEvaluateLocally: true} + for _, key := range []string{"dep", "target"} { + config.Key = key + got, local, err := poller.GetFeatureFlag(config) + if key == "dep" { + if err != nil || !local || got != true { + t.Fatalf("control=%v local=%v err=%v", got, local, err) + } + } else if !isServerEvalError(err) || local { + t.Errorf("target=%v local=%v err=%v; must require server", got, local, err) + } + } + var raw FeatureFlagsResponse + if err := json.Unmarshal([]byte(body), &raw); err != nil { + t.Fatal(err) + } + state := poller.state.Load() + company := uint8(0) + for _, cohorts := range []map[string]PropertyGroup{raw.Cohorts, state.cohorts} { + for _, cached := range []interface{}{nil, false, true} { + cache := map[string]interface{}{"dep": cached} + for i := 0; i < 2; i++ { + got, err := poller.matchCohort(FlagProperty{Value: "c"}, config.GroupProperties["company"], cohorts, state.flagsByKey, cache, "acme", nil, &company, state) + if !isServerEvalError(err) { + t.Errorf("negated cohort=%v err=%v cached=%v; must require server", got, err, cached) + } + } + } + } + }) + } + } +} diff --git a/posthog.go b/posthog.go index 86aa231..a395f71 100644 --- a/posthog.go +++ b/posthog.go @@ -1286,10 +1286,11 @@ func (c *client) evaluateFlagsWithContext(ctx context.Context, payload EvaluateF // $feature_flag_called events with locally_evaluated=true. func (c *client) populateLocalEvaluations(records map[string]evaluatedFlagRecord, locallyEvaluated map[string]struct{}, payload EvaluateFlagsPayload) bool { poller := c.featureFlagsPoller - featureFlags, err := poller.GetFeatureFlags() + state, err := poller.getLoadedState() if err != nil { return true } + featureFlags := state.featureFlags if len(featureFlags) == 0 { return true } @@ -1301,9 +1302,9 @@ func (c *client) populateLocalEvaluations(records map[string]evaluatedFlagRecord missingFlagKeys[k] = struct{}{} } - cohorts := poller.getCohorts() + cohorts := state.cohorts fallbackToRemote := false - minimalFlagCalledEvents := poller.getMinimalFlagCalledEvents() + minimalFlagCalledEvents := state.minimalFlagCalledEvents const localReason = "Evaluated locally" for _, storedFlag := range featureFlags { @@ -1321,6 +1322,7 @@ func (c *client) populateLocalEvaluations(records map[string]evaluatedFlagRecord payload.PersonProperties, payload.GroupProperties, cohorts, + state, ) if err != nil { c.debugf("Unable to compute flag '%s' locally - %s", storedFlag.Key, err)