-
Notifications
You must be signed in to change notification settings - Fork 224
Core: EID Permissions extension #4349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
src/main/java/org/prebid/server/auction/model/EidPermissionIndex.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/auction/model/EidPermissionIndex.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/auction/model/EidPermissionIndex.java
Outdated
Show resolved
Hide resolved
f174bd6 to
9a711eb
Compare
# Conflicts: # src/test/java/org/prebid/server/auction/ExchangeServiceTest.java
| return (eidPermission.getInserter() == null || eidPermission.getInserter().equals(eid.getInserter())) | ||
| && (eidPermission.getSource() == null || eidPermission.getSource().equals(eid.getSource())) | ||
| && (eidPermission.getMatcher() == null || eidPermission.getMatcher().equals(eid.getMatcher())) | ||
| && (eidPermission.getMm() == null || eidPermission.getMm().equals(eid.getMm())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add method nullOrEquals(T permission, T eid)
| @Deprecated | ||
| public static ExtRequestPrebidDataEidPermissions of(String source, List<String> bidders) { | ||
| return new ExtRequestPrebidDataEidPermissions(null, source, null, null, bidders); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the idea to keep it
| List<String> warnings) throws ValidationException { | ||
|
|
||
| if (eidPermissions == null) { | ||
| if (ObjectUtils.isEmpty(eidPermissions)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CollectionUtils
| validateEidPermissionCriteria(eidPermission.getInserter(), | ||
| eidPermission.getSource(), | ||
| eidPermission.getMatcher(), | ||
| eidPermission.getMm()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateEidPermissionCriteria(
eidPermission.getInserter(),
eidPermission.getSource(),
eidPermission.getMatcher(),
eidPermission.getMm());
| String source, | ||
| String matcher, | ||
| Integer mm) throws ValidationException { | ||
| if (StringUtils.isEmpty(inserter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add empty line after multi-line declaration
| if (StringUtils.isEmpty(inserter) | ||
| && StringUtils.isEmpty(source) | ||
| && StringUtils.isEmpty(matcher) | ||
| && mm == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about to use StringUtils.isAllEmpty?
| public void shouldFilterUserExtEidsWhenBidderIsNotAllowedForInserterIgnoringCase() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().inserter("inserter1").build(), Eid.builder().inserter("inserter2").build()), | ||
| singletonList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| singletonList(Eid.builder().inserter("inserter2").build())); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldFilterUserExtEidsWhenBidderIsNotAllowedForMatcherIgnoringCase() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().matcher("matcher1").build(), Eid.builder().matcher("matcher2").build()), | ||
| singletonList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .matcher("matcher1") | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| singletonList(Eid.builder().matcher("matcher2").build())); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldFilterUserExtEidsWhenBidderIsNotAllowedForMm() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().mm(1).build(), Eid.builder().mm(2).build()), | ||
| singletonList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .mm(1) | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| singletonList(Eid.builder().mm(2).build())); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldFilterUserExtEidsWhenBidderIsNotAllowedUsingMultipleCriteria() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build()), | ||
| singletonList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .source("source1") | ||
| .matcher("matcher1") | ||
| .mm(1) | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| singletonList(Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build())); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldFilterUserExtEidsWhenEveryCriteriaMatches() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build()), | ||
| singletonList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .source("source2") | ||
| .matcher("matcher3") | ||
| .mm(4) | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build())); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldFilterUserExtEidsWhenBidderIsNotAllowedUsingTheMostSpecificRule() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build()), | ||
| asList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .bidders(singletonList("someBidder")) | ||
| .build(), | ||
| ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .source("source1") | ||
| .matcher("matcher1") | ||
| .mm(1) | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| singletonList(Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build())); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldNotFilterUserExtEidsWhenBidderIsAllowedUsingTheMostSpecificRule() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build()), | ||
| asList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build(), | ||
| ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .source("source1") | ||
| .matcher("matcher1") | ||
| .mm(1) | ||
| .bidders(singletonList("someBidder")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build())); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldNotFilterUserExtEidsWhenBidderIsAllowedUsingMultipleSameSpecificityRules() { | ||
| testUserEidsPermissionFiltering( | ||
| // given | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build()), | ||
| asList(ExtRequestPrebidDataEidPermissions.builder() | ||
| .inserter("inserter1") | ||
| .source("source1") | ||
| .bidders(singletonList("OtHeRbIdDeR")) | ||
| .build(), | ||
| ExtRequestPrebidDataEidPermissions.builder() | ||
| .matcher("matcher1") | ||
| .mm(1) | ||
| .bidders(singletonList("someBidder")) | ||
| .build()), | ||
| emptyMap(), | ||
| // expected | ||
| asList(Eid.builder().inserter("inserter1").source("source1").matcher("matcher1").mm(1).build(), | ||
| Eid.builder().inserter("inserter2").source("source2").matcher("matcher2").mm(2).build())); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you have extracted logic from ExchangeService -> tests should be moved too. Also, please, unwrap tests. This is unreadable.
| private List<Eid> resolveAllowedEids(List<Eid> userEids, String bidder, EidPermissionHolder eidPermissionHolder) { | ||
| return CollectionUtils.emptyIfNull(userEids) | ||
| .stream() | ||
| .filter(userEid -> isUserEidAllowed(userEid.getSource(), eidPermissions, bidder)) | ||
| .filter(userEid -> eidPermissionHolder.isAllowed(userEid, bidder)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can also be moved to EidPermissionHolder.
🔧 Type of changes
✨ What's the context?
prebid/prebid-server#4623