Skip to content

Commit

Permalink
Bugfix: Make OpenRTB battr logic more strict (#3538)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Net-burst
Net-burst authored Nov 8, 2024
1 parent 33c828b commit 7157ed0
Show file tree
Hide file tree
Showing 5 changed files with 305 additions and 49 deletions.
24 changes: 14 additions & 10 deletions ...rc/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/AccountConfigReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ public Result<BlockedAttributes> blockedAttributesFor(BidRequest bidRequest) {
final Result<List<String>> bapp =
blockedAttribute(BAPP_FIELD, String.class, BLOCKED_APP_FIELD, requestMediaTypes);
final Result<Map<String, List<Integer>>> btype =
blockedAttributesForImps(BTYPE_FIELD, Integer.class, BLOCKED_BANNER_TYPE_FIELD, bidRequest);
blockedAttributesForImps(BTYPE_FIELD, Integer.class, BLOCKED_BANNER_TYPE_FIELD, BANNER_MEDIA_TYPE, bidRequest);
final Result<Map<String, List<Integer>>> bannerBattr =
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_BANNER_ATTR_FIELD, bidRequest);
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_BANNER_ATTR_FIELD, BANNER_MEDIA_TYPE, bidRequest);
final Result<Map<String, List<Integer>>> videoBattr =
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_VIDEO_ATTR_FIELD, bidRequest);
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_VIDEO_ATTR_FIELD, VIDEO_MEDIA_TYPE, bidRequest);
final Result<Map<String, List<Integer>>> audioBattr =
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_AUDIO_ATTR_FIELD, bidRequest);
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_AUDIO_ATTR_FIELD, AUDIO_MEDIA_TYPE, bidRequest);
final Result<Map<MediaType, Map<String, List<Integer>>>> battr =
mergeBlockedAttributes(bannerBattr, videoBattr, audioBattr);

Expand Down Expand Up @@ -226,19 +226,23 @@ private Integer blockedCattaxComplementFromConfig() {
private <T> Result<Map<String, List<T>>> blockedAttributesForImps(String attribute,
Class<T> attributeType,
String fieldName,
String attributeMediaType,
BidRequest bidRequest) {

final Map<String, List<T>> attributeValues = new HashMap<>();
final List<Result<?>> results = new ArrayList<>();

for (final Imp imp : bidRequest.getImp()) {
final Result<List<T>> attributeForImp = blockedAttribute(
attribute, attributeType, fieldName, mediaTypesFrom(imp));

if (attributeForImp.hasValue()) {
attributeValues.put(imp.getId(), attributeForImp.getValue());
final Set<String> actualMediaTypes = mediaTypesFrom(imp);
if (actualMediaTypes.contains(attributeMediaType)) {
final Result<List<T>> attributeForImp = blockedAttribute(
attribute, attributeType, fieldName, actualMediaTypes);

if (attributeForImp.hasValue()) {
attributeValues.put(imp.getId(), attributeForImp.getValue());
}
results.add(attributeForImp);
}
results.add(attributeForImp);
}

return Result.of(
Expand Down
33 changes: 19 additions & 14 deletions ...ing/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/RequestUpdater.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

public class RequestUpdater {

Expand Down Expand Up @@ -99,36 +98,42 @@ private static List<Integer> extractBattr(Map<MediaType, Map<String, List<Intege
}

private static Banner updateBanner(Banner banner, List<Integer> btype, List<Integer> battr) {
final List<Integer> existingBtype = banner != null ? banner.getBtype() : null;
final List<Integer> existingBattr = banner != null ? banner.getBattr() : null;
if (banner == null) {
return null;
}

final List<Integer> existingBtype = banner.getBtype();
final List<Integer> existingBattr = banner.getBattr();

return CollectionUtils.isEmpty(existingBtype) || CollectionUtils.isEmpty(existingBattr)
? Optional.ofNullable(banner)
.map(Banner::toBuilder)
.orElseGet(Banner::builder)
? banner.toBuilder()
.btype(CollectionUtils.isNotEmpty(existingBtype) ? existingBtype : btype)
.battr(CollectionUtils.isNotEmpty(existingBattr) ? existingBattr : battr)
.build()
: banner;
}

private static Video updateVideo(Video video, List<Integer> battr) {
final List<Integer> existingBattr = video != null ? video.getBattr() : null;
if (video == null) {
return null;
}

final List<Integer> existingBattr = video.getBattr();
return CollectionUtils.isEmpty(existingBattr)
? Optional.ofNullable(video)
.map(Video::toBuilder)
.orElseGet(Video::builder)
? video.toBuilder()
.battr(battr)
.build()
: video;
}

private static Audio updateAudio(Audio audio, List<Integer> battr) {
final List<Integer> existingBattr = audio != null ? audio.getBattr() : null;
if (audio == null) {
return null;
}

final List<Integer> existingBattr = audio.getBattr();
return CollectionUtils.isEmpty(existingBattr)
? Optional.ofNullable(audio)
.map(Audio::toBuilder)
.orElseGet(Audio::builder)
? audio.toBuilder()
.battr(battr)
.build()
: audio;
Expand Down
44 changes: 22 additions & 22 deletions ...est/java/org/prebid/server/hooks/modules/ortb2/blocking/core/AccountConfigReaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ public void blockedAttributesForShouldReturnErrorWhenBlockedBannerTypeIsNotInteg
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);

// when and then
assertThatThrownBy(() -> reader.blockedAttributesFor(emptyRequest()))
assertThatThrownBy(() -> reader.blockedAttributesFor(request(imp -> imp.banner(Banner.builder().build()))))
.isInstanceOf(InvalidAccountConfigurationException.class)
.hasMessage("blocked-banner-type field in account configuration has unexpected type. "
+ "Expected class java.lang.Integer");
Expand Down Expand Up @@ -700,38 +700,40 @@ public void blockedAttributesForShouldReturnResultWithBtypeAndWarningsFromOverri
.btype(Attribute.btypeBuilder()
.actionOverrides(AttributeActionOverrides.blocked(asList(
ArrayOverride.of(
Conditions.of(singletonList("bidder1"), singletonList("video")),
Conditions.of(singletonList("bidder1"), singletonList("banner")),
singletonList(1)),
ArrayOverride.of(
Conditions.of(singletonList("bidder1"), singletonList("video")),
Conditions.of(singletonList("bidder1"), singletonList("banner")),
singletonList(2)),
ArrayOverride.of(
Conditions.of(singletonList("bidder1"), singletonList("banner")),
Conditions.of(singletonList("bidder1"), singletonList("video")),
singletonList(3)),
ArrayOverride.of(
Conditions.of(singletonList("bidder1"), singletonList("banner")),
singletonList(4)))))
Conditions.of(singletonList("bidder1"), singletonList("video")),
singletonList(4)),
ArrayOverride.of(
Conditions.of(singletonList("bidder1"), singletonList("audio")),
singletonList(5)),
ArrayOverride.of(
Conditions.of(singletonList("bidder1"), singletonList("audio")),
singletonList(6)))))
.build())
.build()));
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);

// when and then
final Map<String, List<Integer>> expectedBtype = new HashMap<>();
expectedBtype.put("impId1", singletonList(1));
expectedBtype.put("impId2", singletonList(3));
assertThat(reader
.blockedAttributesFor(BidRequest.builder()
.imp(asList(
Imp.builder().id("impId1").video(Video.builder().build()).build(),
Imp.builder().id("impId2").banner(Banner.builder().build()).build()))
Imp.builder().id("impId1").banner(Banner.builder().build()).build(),
Imp.builder().id("impId2").video(Video.builder().build()).build()))
.build()))
.isEqualTo(Result.of(
BlockedAttributes.builder().btype(expectedBtype).build(),
asList(
"More than one conditions matches request. Bidder: bidder1, " +
"request media types: [video]",
"More than one conditions matches request. Bidder: bidder1, " +
"request media types: [banner]")));
List.of("More than one conditions matches request. Bidder: bidder1, " +
"request media types: [banner]")));
}

@Test
Expand Down Expand Up @@ -778,8 +780,8 @@ public void blockedAttributesForShouldReturnResultWithAllAttributesForBanner() {
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);

// when and then
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1")))).isEqualTo(
Result.withValue(BlockedAttributes.builder()
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1").banner(Banner.builder().build()))))
.isEqualTo(Result.withValue(BlockedAttributes.builder()
.badv(singletonList("domain3.com"))
.bcat(singletonList("cat3"))
.bapp(singletonList("app3"))
Expand Down Expand Up @@ -832,12 +834,11 @@ public void blockedAttributesForShouldReturnResultWithAllAttributesForVideo() {
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);

// when and then
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1")))).isEqualTo(
Result.withValue(BlockedAttributes.builder()
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1").video(Video.builder().build()))))
.isEqualTo(Result.withValue(BlockedAttributes.builder()
.badv(singletonList("domain3.com"))
.bcat(singletonList("cat3"))
.bapp(singletonList("app3"))
.btype(singletonMap("impId1", singletonList(3)))
.battr(singletonMap(MediaType.VIDEO, singletonMap("impId1", singletonList(3))))
.build()));
}
Expand Down Expand Up @@ -886,12 +887,11 @@ public void blockedAttributesForShouldReturnResultWithAllAttributesForAudio() {
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);

// when and then
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1")))).isEqualTo(
Result.withValue(BlockedAttributes.builder()
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1").audio(Audio.builder().build()))))
.isEqualTo(Result.withValue(BlockedAttributes.builder()
.badv(singletonList("domain3.com"))
.bcat(singletonList("cat3"))
.bapp(singletonList("app3"))
.btype(singletonMap("impId1", singletonList(3)))
.battr(singletonMap(MediaType.AUDIO, singletonMap("impId1", singletonList(3))))
.build()));
}
Expand Down
118 changes: 117 additions & 1 deletion ...src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/RequestUpdaterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,12 @@ MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
.build());
final BidRequest request = BidRequest.builder()
.imp(singletonList(Imp.builder().id("impId1").build()))
.imp(singletonList(Imp.builder()
.id("impId1")
.banner(Banner.builder().build())
.video(Video.builder().build())
.audio(Audio.builder().build())
.build()))
.build();

// when and then
Expand All @@ -373,4 +378,115 @@ MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
.build()))
.build());
}

@Test
public void shouldNotUpdateMissingBanner() {
// given
final RequestUpdater updater = RequestUpdater.create(
BlockedAttributes.builder()
.badv(asList("domain1.com", "domain2.com"))
.bcat(asList("cat1", "cat2"))
.bapp(asList("app1", "app2"))
.btype(singletonMap("impId1", asList(1, 2)))
.battr(Map.of(
MediaType.BANNER, singletonMap("impId1", asList(1, 2)),
MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
.build());
final BidRequest request = BidRequest.builder()
.imp(singletonList(Imp.builder()
.id("impId1")
.video(Video.builder().build())
.audio(Audio.builder().build())
.build()))
.build();

// when and then
assertThat(updater.update(request)).isEqualTo(BidRequest.builder()
.badv(asList("domain1.com", "domain2.com"))
.bcat(asList("cat1", "cat2"))
.bapp(asList("app1", "app2"))
.imp(singletonList(Imp.builder()
.id("impId1")
.video(Video.builder().battr(asList(3, 4)).build())
.audio(Audio.builder().battr(asList(5, 6)).build())
.build()))
.build());
}

@Test
public void shouldNotUpdateMissingVideo() {
// given
final RequestUpdater updater = RequestUpdater.create(
BlockedAttributes.builder()
.badv(asList("domain1.com", "domain2.com"))
.bcat(asList("cat1", "cat2"))
.bapp(asList("app1", "app2"))
.btype(singletonMap("impId1", asList(1, 2)))
.battr(Map.of(
MediaType.BANNER, singletonMap("impId1", asList(1, 2)),
MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
.build());
final BidRequest request = BidRequest.builder()
.imp(singletonList(Imp.builder()
.id("impId1")
.banner(Banner.builder().build())
.audio(Audio.builder().build())
.build()))
.build();

// when and then
assertThat(updater.update(request)).isEqualTo(BidRequest.builder()
.badv(asList("domain1.com", "domain2.com"))
.bcat(asList("cat1", "cat2"))
.bapp(asList("app1", "app2"))
.imp(singletonList(Imp.builder()
.id("impId1")
.banner(Banner.builder()
.btype(asList(1, 2))
.battr(asList(1, 2))
.build())
.audio(Audio.builder().battr(asList(5, 6)).build())
.build()))
.build());
}

@Test
public void shouldNotUpdateMissingAudio() {
// given
final RequestUpdater updater = RequestUpdater.create(
BlockedAttributes.builder()
.badv(asList("domain1.com", "domain2.com"))
.bcat(asList("cat1", "cat2"))
.bapp(asList("app1", "app2"))
.btype(singletonMap("impId1", asList(1, 2)))
.battr(Map.of(
MediaType.BANNER, singletonMap("impId1", asList(1, 2)),
MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
.build());
final BidRequest request = BidRequest.builder()
.imp(singletonList(Imp.builder()
.id("impId1")
.banner(Banner.builder().build())
.video(Video.builder().build())
.build()))
.build();

// when and then
assertThat(updater.update(request)).isEqualTo(BidRequest.builder()
.badv(asList("domain1.com", "domain2.com"))
.bcat(asList("cat1", "cat2"))
.bapp(asList("app1", "app2"))
.imp(singletonList(Imp.builder()
.id("impId1")
.banner(Banner.builder()
.btype(asList(1, 2))
.battr(asList(1, 2))
.build())
.video(Video.builder().battr(asList(3, 4)).build())
.build()))
.build());
}
}
Loading

0 comments on commit 7157ed0

Please sign in to comment.