Privacy Activities: Pass Request #3064
Conversation
| GDPR gdpr.Policy | ||
| LMT lmt.Policy | ||
| GPP gpp.Policy | ||
| GPPSID []int8 | ||
| } |
There was a problem hiding this comment.
Cleaning up the data model.
Policies was a container of per-package Policy objects. I had a vision of this being a much more useful than it turned out. This new Policies object is a collection of privacy signals needed for Activity Control, in cases when there is no OpenRTB Bid Request... for the cookie and setuid endpoints so far. Will expand over time as needed.
it's my intention for this to have fully parsed data. Hence the GPP SID array instead of a string. When we add GPP, I would like this to contain the parsed GPP object.
privacy/rules.go
Outdated
| @@ -5,16 +5,20 @@ import ( | |||
| ) | |||
|
|
|||
| type Rule interface { | |||
| Evaluate(target Component) ActivityResult | |||
| Evaluate(target Component, request ActivityRequest) ActivityResult | |||
There was a problem hiding this comment.
We need to provide request-level information for Phase 3.5+ rule conditions. I debated on how to best represent this in Go, and landed on this proposal. The request can either be privacy policies from a non-OpenRTB source (like the user sync endpoints) or can be the full bid request. Rules should have as much freedom as possible in how to select the data (so TCF can define hunt paths for the 3 different locations).
privacy/rules.go
Outdated
| } | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
I know it's this is not an efficient algorithm. I didn't want to get fancy with a small data set. The sid array might be 2-10 items and the sid array from the request is 0-2 items. I purposefully loop over the request sids first as they're most likely to be shorter.
There was a problem hiding this comment.
Just a suggestion: we can turn sid to map in activityRulesToEnforcementRules and then access it like a map? Not in this PR, maybe in a later one.
There was a problem hiding this comment.
Sure. I think that would be good to add with Fetchers 2.0 so the map can be cached.
privacy/rules.go
Outdated
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Doesn't look too bad, eh? I experimented with request any and using a type swtich... but then type discovery seemed like an issue.
There was a problem hiding this comment.
Yes! Looks good and simple to understand!
privacy/rules_test.go
Outdated
| @@ -14,7 +16,7 @@ func TestComponentEnforcementRuleEvaluate(t *testing.T) { | |||
| activityResult ActivityResult | |||
| }{ | |||
| { | |||
| name: "activity_is_allowed", | |||
| name: "activity-allowed", | |||
There was a problem hiding this comment.
The is is redudant.
privacy/scrubber.go
Outdated
| var deviceCopy *openrtb2.Device | ||
| deviceCopy = ptrutil.Clone(bidRequest.Device) | ||
| userCopy := ptrutil.Clone(bidRequest.User) | ||
| deviceCopy := ptrutil.Clone(bidRequest.Device) |
There was a problem hiding this comment.
VS Code "yelled" at me to make this nitpick optimization.
There was a problem hiding this comment.
Oh so does it actually work? IntelliJIdea also had an error that looked like a compilation error, so I added those types explicitly. With this change all User and Device fields are red now. And userCopy.{field} helper is not available. Maybe we should put it back?
There was a problem hiding this comment.
Maybe we should put it back?
Auto-completion works fine for me on VS Code with Go 1.20.5. Is there an update available for IntelliJ?
There was a problem hiding this comment.
Just updated. It didn't help.
This is just one line of code that doesn't take memory and CPU, but makes IDEs happy.
Maybe we should put it back?
There was a problem hiding this comment.
Let's make a deal. I'll revert and could you please open a bug report for IntelliJ?
| ActivityTransmitUniqueRequestIds | ||
| ActivityTransmitTids | ||
| ActivityTransmitUniqueRequestIDs | ||
| ActivityTransmitTIDs |
There was a problem hiding this comment.
Oh sorry about this...
There was a problem hiding this comment.
Not a problem. Just being picky about honoring Go acronym conventions (my IDE "yelled" at me here too).
privacy/rules.go
Outdated
| @@ -23,16 +27,18 @@ func (r ComponentEnforcementRule) Evaluate(target Component) ActivityResult { | |||
| return ActivityAbstain | |||
| } | |||
|
|
|||
| if matched := evaluateGPPSID(r.gppSID, request); !matched { | |||
| return ActivityAbstain | |||
There was a problem hiding this comment.
I noticed this return value isn't covered by tests. Just want to point it out.
Extends the Activities Control infrastructure to pass request information. Implemented Phase 3.5 GPP SID signal as an initial proof of concept for the new request object.