Promote glob source and trigger draft on adoption
The pattern-analysis worker dropped two signals on every run. When InsertIfNotExists hit a pre-existing glob, the computed bestSource was discarded by the LoadByBannerIDTypeAndPattern fallback, so the SCRIPT > EXTENSION > PRE_EXISTING precedence advertised on bestSource was only ever enforced at first insert. Subsequent batches with stronger sources could not promote the glob, even though the page-script-wins rule already lives in detected_trackers at the row level. Separately, adoptUncategorisedPatterns returned an adopted bool that the worker discarded; the function moves detected trackers from uncategorised exact patterns into categorised globs, which is a real consent transition, but no draft banner version was created on adoption-only runs. Add a focused TrackerPattern.PromoteSource that only updates the source and updated_at columns. Express the precedence as a pure-Go shouldPromoteSource helper alongside bestSource so the rule is unit-testable without a database. The worker now calls InsertIfNotExists, then on conflict loads, skips when the slot is held by an exact pattern or a user-recategorised glob, and only calls PromoteSource when the candidate source ranks above the existing one. The skip branch is now documented: adoptUncategorisedPatterns is the safety net that re-homes uncategorised exacts into the existing glob via globMatch. Capture its adopted return value and use it (instead of the previous over-eager consentChanged flag) to gate ensureDraftVersionForBanner. Merging exacts into a glob in their own category never changes visitor consent, so the prior flag produced redundant draft versions on every non-uncategorised merge. Cover the new pieces with three test layers: pure-unit cases for shouldPromoteSource (precedence matrix including HTTP/nil collapse and equal-rank no-write), DB-backed tests for PromoteSource (touch only source + updated_at, ErrResourceNotFound for missing rows), and end-to-end worker tests for source promotion on an existing glob, draft-on-adoption, and the merge-only no-draft case. Signed-off-by: Émile Ré <emile@probo.com>
This commit is contained in:
@@ -137,18 +137,6 @@ func (h *patternAnalysisHandler) Process(ctx context.Context, banner coredata.Co
|
||||
func(ctx context.Context, tx pg.Tx) error {
|
||||
scope := coredata.NewScopeFromObjectID(banner.ID)
|
||||
|
||||
var uncategorised coredata.CookieCategory
|
||||
|
||||
hasUncategorised := true
|
||||
|
||||
if err := uncategorised.LoadUncategorisedByCookieBannerID(ctx, tx, scope, banner.ID); err != nil {
|
||||
if !errors.Is(err, coredata.ErrResourceNotFound) {
|
||||
return fmt.Errorf("cannot load uncategorised category: %w", err)
|
||||
}
|
||||
|
||||
hasUncategorised = false
|
||||
}
|
||||
|
||||
var exactPatterns coredata.TrackerPatterns
|
||||
if err := exactPatterns.LoadAllByCookieBannerID(
|
||||
ctx,
|
||||
@@ -163,8 +151,6 @@ func (h *patternAnalysisHandler) Process(ctx context.Context, banner coredata.Co
|
||||
|
||||
mergeGroups := findMergeGroups(exactPatterns, patternMergeThreshold)
|
||||
|
||||
consentChanged := false
|
||||
|
||||
for key, group := range mergeGroups {
|
||||
var maxAge *int
|
||||
|
||||
@@ -203,9 +189,23 @@ func (h *patternAnalysisHandler) Process(ctx context.Context, banner coredata.Co
|
||||
return fmt.Errorf("cannot load existing glob pattern %q: %w", key.template, err)
|
||||
}
|
||||
|
||||
if globPattern.CookieCategoryID != key.categoryID || globPattern.MatchType != coredata.TrackerPatternMatchTypeGlob {
|
||||
if globPattern.MatchType != coredata.TrackerPatternMatchTypeGlob || globPattern.CookieCategoryID != key.categoryID {
|
||||
// The slot is occupied by an exact pattern or
|
||||
// a user-recategorised glob. Skip the relink
|
||||
// here so we don't overwrite the user's
|
||||
// categorisation; the exact patterns in
|
||||
// `group` will be picked up below by
|
||||
// adoptUncategorisedPatterns if they live in
|
||||
// the uncategorised category and globMatch
|
||||
// the existing glob.
|
||||
continue
|
||||
}
|
||||
|
||||
if shouldPromoteSource(globPattern.Source, source) {
|
||||
if err := globPattern.PromoteSource(ctx, tx, scope, *source, now); err != nil {
|
||||
return fmt.Errorf("cannot promote source on glob pattern %q: %w", key.template, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for _, exactPattern := range group {
|
||||
@@ -219,20 +219,18 @@ func (h *patternAnalysisHandler) Process(ctx context.Context, banner coredata.Co
|
||||
}
|
||||
}
|
||||
|
||||
if !hasUncategorised || key.categoryID != uncategorised.ID {
|
||||
consentChanged = true
|
||||
}
|
||||
|
||||
h.logger.InfoCtx(
|
||||
ctx,
|
||||
"merged exact patterns into glob pattern",
|
||||
log.String("template", key.template),
|
||||
log.Int("count", len(group)),
|
||||
log.Bool("inserted", inserted),
|
||||
log.String("banner_id", banner.ID.String()),
|
||||
)
|
||||
}
|
||||
|
||||
if _, err := h.adoptUncategorisedPatterns(ctx, tx, scope, banner); err != nil {
|
||||
adopted, err := h.adoptUncategorisedPatterns(ctx, tx, scope, banner)
|
||||
if err != nil {
|
||||
return fmt.Errorf("cannot adopt uncategorised patterns: %w", err)
|
||||
}
|
||||
|
||||
@@ -241,7 +239,13 @@ func (h *patternAnalysisHandler) Process(ctx context.Context, banner coredata.Co
|
||||
return fmt.Errorf("cannot refresh last_matched_at: %w", err)
|
||||
}
|
||||
|
||||
if consentChanged {
|
||||
// Merging exact patterns into a glob in the same category
|
||||
// does not change visitor consent for those identifiers
|
||||
// (findMergeGroups keys on category, so every member of a
|
||||
// group is already under key.categoryID). Adoption is the
|
||||
// only operation in this worker that moves trackers
|
||||
// between categories and therefore changes consent.
|
||||
if adopted {
|
||||
if _, err := h.svc.ensureDraftVersionForBanner(ctx, tx, scope, banner.ID); err != nil {
|
||||
return fmt.Errorf("cannot ensure draft version: %w", err)
|
||||
}
|
||||
@@ -633,15 +637,45 @@ func globMatch(pattern, name string) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// sourceRank converts a CookieSource into a comparable rank that
|
||||
// reflects signal strength: SCRIPT > EXTENSION > PRE_EXISTING. HTTP
|
||||
// and nil collapse into the PRE_EXISTING rank because bestSource
|
||||
// already normalises them; if a future caller hands us either, the
|
||||
// ranking still produces a sane "no promotion" outcome against
|
||||
// PRE_EXISTING/EXTENSION/SCRIPT existing values.
|
||||
func sourceRank(s *coredata.CookieSource) int {
|
||||
if s == nil {
|
||||
return 0
|
||||
}
|
||||
|
||||
switch *s {
|
||||
case coredata.CookieSourceScript:
|
||||
return 2
|
||||
case coredata.CookieSourceExtension:
|
||||
return 1
|
||||
default:
|
||||
return 0
|
||||
}
|
||||
}
|
||||
|
||||
// shouldPromoteSource reports whether candidate represents a stronger
|
||||
// signal than existing under the SCRIPT > EXTENSION > PRE_EXISTING
|
||||
// precedence used across the cookie-banner pipeline. Equal ranks do
|
||||
// not promote so we avoid pointless writes.
|
||||
func shouldPromoteSource(existing, candidate *coredata.CookieSource) bool {
|
||||
return sourceRank(candidate) > sourceRank(existing)
|
||||
}
|
||||
|
||||
// bestSource rolls up the source values of a group of exact patterns
|
||||
// being merged into a single glob. Precedence is SCRIPT > EXTENSION
|
||||
// > PRE_EXISTING, mirroring both the upsert SQL's "page-script wins"
|
||||
// rule and the asymmetric signal strength of each bucket: SCRIPT is
|
||||
// high-confidence page evidence (a real page tracker), EXTENSION is
|
||||
// high-confidence extension evidence, and PRE_EXISTING is the
|
||||
// catch-all that may include extension state injected before SDK
|
||||
// load. HTTP and nil collapse into PRE_EXISTING here, preserving
|
||||
// the original two-value rollup behaviour for non-script values.
|
||||
// > PRE_EXISTING, mirroring both the page-script-wins rule in
|
||||
// detected_trackers and the asymmetric signal strength of each
|
||||
// bucket: SCRIPT is high-confidence page evidence (a real page
|
||||
// tracker), EXTENSION is high-confidence extension evidence, and
|
||||
// PRE_EXISTING is the catch-all that may include extension state
|
||||
// injected before SDK load. HTTP and nil collapse into PRE_EXISTING
|
||||
// here, preserving the original two-value rollup behaviour for
|
||||
// non-script values.
|
||||
func bestSource(patterns []*coredata.TrackerPattern) *coredata.CookieSource {
|
||||
var hasExtension bool
|
||||
|
||||
|
||||
Reference in New Issue
Block a user