diff --git a/.cursor/rules/go-coredata-load-naming.mdc b/.cursor/rules/go-coredata-load-naming.mdc index a20241da0..456d85c0e 100644 --- a/.cursor/rules/go-coredata-load-naming.mdc +++ b/.cursor/rules/go-coredata-load-naming.mdc @@ -58,15 +58,20 @@ func (ds *Things) LoadByParentID( ) error { ``` -# No cross-entity JOINs +# Cross-entity table references -Each entity file in `pkg/coredata` queries only its own table. When data from multiple entities is needed, the caller orchestrates separate calls. +Each entity file in `pkg/coredata` queries its own table. Never JOIN two +entity tables to **return columns from both** — the caller orchestrates +separate calls instead. -- Never JOIN two entity tables inside an entity method. -- Never return a raw ID belonging to a different entity — return the full entity and let the caller read the foreign key field. +**Subqueries for filtering are OK.** When the only purpose of the other +table is to narrow a `WHERE` clause (e.g. `IN (SELECT id FROM ...)`), +keep it in the query rather than materializing IDs in Go and passing +them as an `ANY(@ids)` parameter. A subquery keeps the filtering in the +database and eliminates an extra round trip. ```go -// BAD — cross-entity JOIN inside DetectedTrackers +// BAD — cross-entity JOIN that returns columns from both tables q := ` SELECT ctpd.common_third_party_id FROM detected_trackers dt @@ -80,4 +85,14 @@ filter := coredata.NewCommonThirdPartyDomainFilter(domains) var matched coredata.CommonThirdPartyDomains err = matched.Load(ctx, conn, 1, filter) thirdPartyID := matched[0].CommonThirdPartyID + +// GOOD — subquery only used for filtering, no columns returned from it +q := ` +SELECT id, pattern, tracker_type, ... +FROM tracker_patterns +WHERE common_tracker_pattern_id IN ( + SELECT id FROM common_tracker_patterns + WHERE common_third_party_id = @filter_common_third_party_id +) +` ``` diff --git a/pkg/cookiebanner/service.go b/pkg/cookiebanner/service.go index 20a79428a..12067126c 100644 --- a/pkg/cookiebanner/service.go +++ b/pkg/cookiebanner/service.go @@ -2746,40 +2746,6 @@ func (s *Service) GetCommonTrackerPatternsByIDs( return patterns, nil } -// LoadCommonTrackerPatternIDsByCommonThirdPartyID returns the IDs of -// every common tracker pattern referencing the given common third party. -// Used by the trackers list filter to translate a CommonThirdParty GID -// into a `common_tracker_pattern_id = ANY(...)` constraint without -// JOINing across entity tables in coredata. -func (s *Service) LoadCommonTrackerPatternIDsByCommonThirdPartyID( - ctx context.Context, - commonThirdPartyID gid.GID, -) ([]gid.GID, error) { - var ids []gid.GID - - err := s.pg.WithConn( - ctx, - func(ctx context.Context, conn pg.Querier) error { - var ( - patterns coredata.CommonTrackerPatterns - err error - ) - - ids, err = patterns.LoadIDsByCommonThirdPartyID(ctx, conn, commonThirdPartyID) - if err != nil { - return fmt.Errorf("cannot load common tracker pattern ids: %w", err) - } - - return nil - }, - ) - if err != nil { - return nil, err - } - - return ids, nil -} - // LoadDistinctThirdPartyIDsByCookieBannerID returns the distinct // org-scoped third-party IDs referenced by tracker patterns of the // banner. The companion diff --git a/pkg/coredata/common_tracker_pattern.go b/pkg/coredata/common_tracker_pattern.go index 505124285..5b88baa60 100644 --- a/pkg/coredata/common_tracker_pattern.go +++ b/pkg/coredata/common_tracker_pattern.go @@ -483,36 +483,3 @@ WHERE return nil } - -// LoadIDsByCommonThirdPartyID returns just the IDs of the common tracker -// patterns linked to the given common third party. Callers use it to feed -// a `common_tracker_pattern_id = ANY(...)` filter on tracker_patterns -// without crossing the entity boundary. -func (ps *CommonTrackerPatterns) LoadIDsByCommonThirdPartyID( - ctx context.Context, - conn pg.Querier, - commonThirdPartyID gid.GID, -) ([]gid.GID, error) { - q := ` -SELECT - id -FROM - common_tracker_patterns -WHERE - common_third_party_id = @common_third_party_id -` - - args := pgx.StrictNamedArgs{"common_third_party_id": commonThirdPartyID} - - rows, err := conn.Query(ctx, q, args) - if err != nil { - return nil, fmt.Errorf("cannot query common tracker pattern ids: %w", err) - } - - ids, err := pgx.CollectRows(rows, pgx.RowTo[gid.GID]) - if err != nil { - return nil, fmt.Errorf("cannot collect common tracker pattern ids: %w", err) - } - - return ids, nil -} diff --git a/pkg/coredata/tracker_pattern_filter.go b/pkg/coredata/tracker_pattern_filter.go index 77f6ee1a4..7fc6052f1 100644 --- a/pkg/coredata/tracker_pattern_filter.go +++ b/pkg/coredata/tracker_pattern_filter.go @@ -20,14 +20,14 @@ import ( ) type TrackerPatternFilter struct { - matchType *TrackerPatternMatchType - cookieCategoryID *gid.GID - excluded *bool - query *string - source *CookieSource - trackerType *TrackerType - thirdPartyID *gid.GID - commonTrackerPatternIDs []gid.GID + matchType *TrackerPatternMatchType + cookieCategoryID *gid.GID + excluded *bool + query *string + source *CookieSource + trackerType *TrackerType + thirdPartyID *gid.GID + commonThirdPartyID *gid.GID } func NewTrackerPatternFilter( @@ -62,14 +62,8 @@ func (f *TrackerPatternFilter) WithThirdPartyID(thirdPartyID *gid.GID) *TrackerP return f } -// WithCommonTrackerPatternIDs constrains the result to tracker patterns -// whose `common_tracker_pattern_id` is in the given set. Callers -// pre-resolve this list (typically via -// CommonTrackerPatterns.LoadIDsByCommonThirdPartyID) so the filter stays -// inside the tracker_patterns table. Passing an empty (non-nil) slice -// yields no rows. -func (f *TrackerPatternFilter) WithCommonTrackerPatternIDs(ids []gid.GID) *TrackerPatternFilter { - f.commonTrackerPatternIDs = ids +func (f *TrackerPatternFilter) WithCommonThirdPartyID(id *gid.GID) *TrackerPatternFilter { + f.commonThirdPartyID = id return f } @@ -130,9 +124,12 @@ func (f *TrackerPatternFilter) SQLFragment() string { END AND CASE - WHEN @has_common_tracker_pattern_ids_filter::boolean = false THEN TRUE - WHEN @has_common_tracker_pattern_ids_filter::boolean = true THEN - common_tracker_pattern_id = ANY(@filter_common_tracker_pattern_ids::text[]) + WHEN @has_common_third_party_id_filter::boolean = false THEN TRUE + WHEN @has_common_third_party_id_filter::boolean = true THEN + common_tracker_pattern_id IN ( + SELECT id FROM common_tracker_patterns + WHERE common_third_party_id = @filter_common_third_party_id::text + ) ELSE TRUE END )` @@ -144,21 +141,21 @@ func (f *TrackerPatternFilter) SQLArguments() pgx.StrictNamedArgs { } args := pgx.StrictNamedArgs{ - "has_match_type_filter": false, - "filter_match_type": nil, - "has_cookie_category_id_filter": false, - "filter_cookie_category_id": nil, - "has_excluded_filter": false, - "filter_excluded": nil, - "filter_query": nil, - "has_source_filter": false, - "filter_source": nil, - "has_tracker_type_filter": false, - "filter_tracker_type": nil, - "has_third_party_id_filter": false, - "filter_third_party_id": nil, - "has_common_tracker_pattern_ids_filter": false, - "filter_common_tracker_pattern_ids": []gid.GID{}, + "has_match_type_filter": false, + "filter_match_type": nil, + "has_cookie_category_id_filter": false, + "filter_cookie_category_id": nil, + "has_excluded_filter": false, + "filter_excluded": nil, + "filter_query": nil, + "has_source_filter": false, + "filter_source": nil, + "has_tracker_type_filter": false, + "filter_tracker_type": nil, + "has_third_party_id_filter": false, + "filter_third_party_id": nil, + "has_common_third_party_id_filter": false, + "filter_common_third_party_id": nil, } if f.matchType != nil { @@ -195,9 +192,9 @@ func (f *TrackerPatternFilter) SQLArguments() pgx.StrictNamedArgs { args["filter_third_party_id"] = *f.thirdPartyID } - if f.commonTrackerPatternIDs != nil { - args["has_common_tracker_pattern_ids_filter"] = true - args["filter_common_tracker_pattern_ids"] = f.commonTrackerPatternIDs + if f.commonThirdPartyID != nil { + args["has_common_third_party_id_filter"] = true + args["filter_common_third_party_id"] = *f.commonThirdPartyID } return args diff --git a/pkg/server/api/console/v1/cookie_banner_resolvers.go b/pkg/server/api/console/v1/cookie_banner_resolvers.go index bf0d7f832..b7fdd2fba 100644 --- a/pkg/server/api/console/v1/cookie_banner_resolvers.go +++ b/pkg/server/api/console/v1/cookie_banner_resolvers.go @@ -217,17 +217,7 @@ func (r *cookieBannerResolver) TrackerPatterns(ctx context.Context, obj *types.C case coredata.ThirdPartyEntityType: coredataFilter = coredataFilter.WithThirdPartyID(filter.ThirdPartyID) case coredata.CommonThirdPartyEntityType: - ids, err := r.cookieBanner.LoadCommonTrackerPatternIDsByCommonThirdPartyID(ctx, *filter.ThirdPartyID) - if err != nil { - r.logger.ErrorCtx(ctx, "cannot resolve common third party tracker patterns", log.Error(err)) - return nil, gqlutils.Internal(ctx) - } - - if ids == nil { - ids = []gid.GID{} - } - - coredataFilter = coredataFilter.WithCommonTrackerPatternIDs(ids) + coredataFilter = coredataFilter.WithCommonThirdPartyID(filter.ThirdPartyID) default: return nil, gqlutils.Invalidf(ctx, "thirdPartyId must reference a ThirdParty or CommonThirdParty") }