fix: scope flag deletion by flagSetId to support per-selector sync

[erenatas]

This PR

When an upstream provider sends per-selector stream messages (each with a flagSetId in metadata),

• store.Update now only deletes stale flags within the same flagSetId(s), preserving flags from other selector.
• Updated ToLogString function since the old ToLogString only handled single-key selectors and returned <none> for compound ones. With the scoped deletion, selectors now have two keys (flagSetId+source), so debug logs would show <none> instead of the actual query — making it impossible to troubleshoot store operations.

Related Issues

This change should fix using flagd with a provider that has multiple selectors

[netlify]

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name	Link
🔨 Latest commit	5bf055b
🔍 Latest deploy log	https://app.netlify.com/projects/polite-licorice-3db33c/deploys/69cd51f698caae0008e87578

[gemini-code-assist]

Code Review

This pull request introduces flagSetId-based scoping for flag updates, allowing for more granular control over flag deletions by targeting specific flag sets rather than the entire source. It also improves the selector's log string generation by supporting multiple keys and ensuring deterministic output through sorting. Feedback was provided to refine the scoping logic to correctly handle empty flagSetId strings and to optimize performance by utilizing the existing write transaction for queries instead of creating new read transactions.

[gemini-code-assist]

The current implementation has two areas for improvement:

1. Scoping for Default Selector: The check fsi != "" prevents scoping when the flagSetId is an empty string. Since flagd normalizes empty or missing flagSetIds to a default internal UUID (nilFlagSetId), skipping scoping for "" causes the "default" selector to fall back to source-scoped deletion. This may unintentionally delete flags from other selectors. Removing fsi != "" and relying on ok ensures consistent scoping for all selectors.
2. Transaction Efficiency: Calling s.GetAll inside Update is suboptimal because it initiates new read transactions while a write transaction (txn) is already active. It is more efficient to use the existing txn directly. This also allows for proper error handling during the query phase.

	if fsi, ok := metadata["flagSetId"].(string); ok {
		seenFlagSetIds := map[string]struct{}{fsi: {}}
		for id := range newFlags {
			seenFlagSetIds[id.flagSetId] = struct{}{}
		}
		for fsi := range seenFlagSetIds {
			sel := NewSelector(flagSetIdIndex + "=" + fsi).WithIndex(sourceIndex, source)
			indexId, constraints := sel.ToQuery()
			it, err := txn.Get(flagsTable, indexId, constraints...)
			if err != nil {
				s.logger.Error(fmt.Sprintf("unable to query flags for flagSetId %s: %v", fsi, err))
				continue
			}
			oldFlags = append(oldFlags, s.collect(it)...)
		}
	} else {
		sel := NewSelector(sourceIndex + "=" + source)
		indexId, constraints := sel.ToQuery()
		it, err := txn.Get(flagsTable, indexId, constraints...)
		if err != nil {
			s.logger.Error(fmt.Sprintf("unable to query flags for source %s: %v", source, err))
		} else {
			oldFlags = s.collect(it)
		}
	}

[erenatas]

Flags with an empty-string flagSetId are stored as "", but NewSelector silently rewrites "" to nilFlagSetId, so removing the guard would make the query look for the wrong value and never find those flags.

[erenatas]

Transaction Efficiency: Calling s.GetAll inside Update is suboptimal because it initiates new read transactions while a write transaction (txn) is already active. It is more efficient to use the existing txn directly. This also allows for proper error handling during the query phase.

Updated

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud