Skip to content

Commit 7157ed0

Browse files
Net-burstNet-burst
authored
Bugfix: Make OpenRTB battr logic more strict (#3538)
1 parent 33c828b commit 7157ed0

5 files changed

Lines changed: 305 additions & 49 deletions

File tree

‎extra/modules/ortb2-blocking/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/AccountConfigReader.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,13 @@ public Result<BlockedAttributes> blockedAttributesFor(BidRequest bidRequest) {
104104
final Result<List<String>> bapp =
105105
blockedAttribute(BAPP_FIELD, String.class, BLOCKED_APP_FIELD, requestMediaTypes);
106106
final Result<Map<String, List<Integer>>> btype =
107-
blockedAttributesForImps(BTYPE_FIELD, Integer.class, BLOCKED_BANNER_TYPE_FIELD, bidRequest);
107+
blockedAttributesForImps(BTYPE_FIELD, Integer.class, BLOCKED_BANNER_TYPE_FIELD, BANNER_MEDIA_TYPE, bidRequest);
108108
final Result<Map<String, List<Integer>>> bannerBattr =
109-
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_BANNER_ATTR_FIELD, bidRequest);
109+
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_BANNER_ATTR_FIELD, BANNER_MEDIA_TYPE, bidRequest);
110110
final Result<Map<String, List<Integer>>> videoBattr =
111-
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_VIDEO_ATTR_FIELD, bidRequest);
111+
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_VIDEO_ATTR_FIELD, VIDEO_MEDIA_TYPE, bidRequest);
112112
final Result<Map<String, List<Integer>>> audioBattr =
113-
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_AUDIO_ATTR_FIELD, bidRequest);
113+
blockedAttributesForImps(BATTR_FIELD, Integer.class, BLOCKED_AUDIO_ATTR_FIELD, AUDIO_MEDIA_TYPE, bidRequest);
114114
final Result<Map<MediaType, Map<String, List<Integer>>>> battr =
115115
mergeBlockedAttributes(bannerBattr, videoBattr, audioBattr);
116116

@@ -226,19 +226,23 @@ private Integer blockedCattaxComplementFromConfig() {
226226
private <T> Result<Map<String, List<T>>> blockedAttributesForImps(String attribute,
227227
Class<T> attributeType,
228228
String fieldName,
229+
String attributeMediaType,
229230
BidRequest bidRequest) {
230231

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

234235
for (final Imp imp : bidRequest.getImp()) {
235-
final Result<List<T>> attributeForImp = blockedAttribute(
236-
attribute, attributeType, fieldName, mediaTypesFrom(imp));
237-
238-
if (attributeForImp.hasValue()) {
239-
attributeValues.put(imp.getId(), attributeForImp.getValue());
236+
final Set<String> actualMediaTypes = mediaTypesFrom(imp);
237+
if (actualMediaTypes.contains(attributeMediaType)) {
238+
final Result<List<T>> attributeForImp = blockedAttribute(
239+
attribute, attributeType, fieldName, actualMediaTypes);
240+
241+
if (attributeForImp.hasValue()) {
242+
attributeValues.put(imp.getId(), attributeForImp.getValue());
243+
}
244+
results.add(attributeForImp);
240245
}
241-
results.add(attributeForImp);
242246
}
243247

244248
return Result.of(

‎extra/modules/ortb2-blocking/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/RequestUpdater.java‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import java.util.List;
1414
import java.util.Map;
1515
import java.util.Objects;
16-
import java.util.Optional;
1716

1817
public class RequestUpdater {
1918

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

101100
private static Banner updateBanner(Banner banner, List<Integer> btype, List<Integer> battr) {
102-
final List<Integer> existingBtype = banner != null ? banner.getBtype() : null;
103-
final List<Integer> existingBattr = banner != null ? banner.getBattr() : null;
101+
if (banner == null) {
102+
return null;
103+
}
104+
105+
final List<Integer> existingBtype = banner.getBtype();
106+
final List<Integer> existingBattr = banner.getBattr();
104107

105108
return CollectionUtils.isEmpty(existingBtype) || CollectionUtils.isEmpty(existingBattr)
106-
? Optional.ofNullable(banner)
107-
.map(Banner::toBuilder)
108-
.orElseGet(Banner::builder)
109+
? banner.toBuilder()
109110
.btype(CollectionUtils.isNotEmpty(existingBtype) ? existingBtype : btype)
110111
.battr(CollectionUtils.isNotEmpty(existingBattr) ? existingBattr : battr)
111112
.build()
112113
: banner;
113114
}
114115

115116
private static Video updateVideo(Video video, List<Integer> battr) {
116-
final List<Integer> existingBattr = video != null ? video.getBattr() : null;
117+
if (video == null) {
118+
return null;
119+
}
120+
121+
final List<Integer> existingBattr = video.getBattr();
117122
return CollectionUtils.isEmpty(existingBattr)
118-
? Optional.ofNullable(video)
119-
.map(Video::toBuilder)
120-
.orElseGet(Video::builder)
123+
? video.toBuilder()
121124
.battr(battr)
122125
.build()
123126
: video;
124127
}
125128

126129
private static Audio updateAudio(Audio audio, List<Integer> battr) {
127-
final List<Integer> existingBattr = audio != null ? audio.getBattr() : null;
130+
if (audio == null) {
131+
return null;
132+
}
133+
134+
final List<Integer> existingBattr = audio.getBattr();
128135
return CollectionUtils.isEmpty(existingBattr)
129-
? Optional.ofNullable(audio)
130-
.map(Audio::toBuilder)
131-
.orElseGet(Audio::builder)
136+
? audio.toBuilder()
132137
.battr(battr)
133138
.build()
134139
: audio;

‎extra/modules/ortb2-blocking/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/AccountConfigReaderTest.java‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ public void blockedAttributesForShouldReturnErrorWhenBlockedBannerTypeIsNotInteg
400400
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);
401401

402402
// when and then
403-
assertThatThrownBy(() -> reader.blockedAttributesFor(emptyRequest()))
403+
assertThatThrownBy(() -> reader.blockedAttributesFor(request(imp -> imp.banner(Banner.builder().build()))))
404404
.isInstanceOf(InvalidAccountConfigurationException.class)
405405
.hasMessage("blocked-banner-type field in account configuration has unexpected type. "
406406
+ "Expected class java.lang.Integer");
@@ -700,38 +700,40 @@ public void blockedAttributesForShouldReturnResultWithBtypeAndWarningsFromOverri
700700
.btype(Attribute.btypeBuilder()
701701
.actionOverrides(AttributeActionOverrides.blocked(asList(
702702
ArrayOverride.of(
703-
Conditions.of(singletonList("bidder1"), singletonList("video")),
703+
Conditions.of(singletonList("bidder1"), singletonList("banner")),
704704
singletonList(1)),
705705
ArrayOverride.of(
706-
Conditions.of(singletonList("bidder1"), singletonList("video")),
706+
Conditions.of(singletonList("bidder1"), singletonList("banner")),
707707
singletonList(2)),
708708
ArrayOverride.of(
709-
Conditions.of(singletonList("bidder1"), singletonList("banner")),
709+
Conditions.of(singletonList("bidder1"), singletonList("video")),
710710
singletonList(3)),
711711
ArrayOverride.of(
712-
Conditions.of(singletonList("bidder1"), singletonList("banner")),
713-
singletonList(4)))))
712+
Conditions.of(singletonList("bidder1"), singletonList("video")),
713+
singletonList(4)),
714+
ArrayOverride.of(
715+
Conditions.of(singletonList("bidder1"), singletonList("audio")),
716+
singletonList(5)),
717+
ArrayOverride.of(
718+
Conditions.of(singletonList("bidder1"), singletonList("audio")),
719+
singletonList(6)))))
714720
.build())
715721
.build()));
716722
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);
717723

718724
// when and then
719725
final Map<String, List<Integer>> expectedBtype = new HashMap<>();
720726
expectedBtype.put("impId1", singletonList(1));
721-
expectedBtype.put("impId2", singletonList(3));
722727
assertThat(reader
723728
.blockedAttributesFor(BidRequest.builder()
724729
.imp(asList(
725-
Imp.builder().id("impId1").video(Video.builder().build()).build(),
726-
Imp.builder().id("impId2").banner(Banner.builder().build()).build()))
730+
Imp.builder().id("impId1").banner(Banner.builder().build()).build(),
731+
Imp.builder().id("impId2").video(Video.builder().build()).build()))
727732
.build()))
728733
.isEqualTo(Result.of(
729734
BlockedAttributes.builder().btype(expectedBtype).build(),
730-
asList(
731-
"More than one conditions matches request. Bidder: bidder1, " +
732-
"request media types: [video]",
733-
"More than one conditions matches request. Bidder: bidder1, " +
734-
"request media types: [banner]")));
735+
List.of("More than one conditions matches request. Bidder: bidder1, " +
736+
"request media types: [banner]")));
735737
}
736738

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

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

834836
// when and then
835-
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1")))).isEqualTo(
836-
Result.withValue(BlockedAttributes.builder()
837+
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1").video(Video.builder().build()))))
838+
.isEqualTo(Result.withValue(BlockedAttributes.builder()
837839
.badv(singletonList("domain3.com"))
838840
.bcat(singletonList("cat3"))
839841
.bapp(singletonList("app3"))
840-
.btype(singletonMap("impId1", singletonList(3)))
841842
.battr(singletonMap(MediaType.VIDEO, singletonMap("impId1", singletonList(3))))
842843
.build()));
843844
}
@@ -886,12 +887,11 @@ public void blockedAttributesForShouldReturnResultWithAllAttributesForAudio() {
886887
final AccountConfigReader reader = AccountConfigReader.create(accountConfig, "bidder1", ORTB_VERSION, true);
887888

888889
// when and then
889-
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1")))).isEqualTo(
890-
Result.withValue(BlockedAttributes.builder()
890+
assertThat(reader.blockedAttributesFor(request(imp -> imp.id("impId1").audio(Audio.builder().build()))))
891+
.isEqualTo(Result.withValue(BlockedAttributes.builder()
891892
.badv(singletonList("domain3.com"))
892893
.bcat(singletonList("cat3"))
893894
.bapp(singletonList("app3"))
894-
.btype(singletonMap("impId1", singletonList(3)))
895895
.battr(singletonMap(MediaType.AUDIO, singletonMap("impId1", singletonList(3))))
896896
.build()));
897897
}

‎extra/modules/ortb2-blocking/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/RequestUpdaterTest.java‎

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,12 @@ MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
354354
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
355355
.build());
356356
final BidRequest request = BidRequest.builder()
357-
.imp(singletonList(Imp.builder().id("impId1").build()))
357+
.imp(singletonList(Imp.builder()
358+
.id("impId1")
359+
.banner(Banner.builder().build())
360+
.video(Video.builder().build())
361+
.audio(Audio.builder().build())
362+
.build()))
358363
.build();
359364

360365
// when and then
@@ -373,4 +378,115 @@ MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
373378
.build()))
374379
.build());
375380
}
381+
382+
@Test
383+
public void shouldNotUpdateMissingBanner() {
384+
// given
385+
final RequestUpdater updater = RequestUpdater.create(
386+
BlockedAttributes.builder()
387+
.badv(asList("domain1.com", "domain2.com"))
388+
.bcat(asList("cat1", "cat2"))
389+
.bapp(asList("app1", "app2"))
390+
.btype(singletonMap("impId1", asList(1, 2)))
391+
.battr(Map.of(
392+
MediaType.BANNER, singletonMap("impId1", asList(1, 2)),
393+
MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
394+
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
395+
.build());
396+
final BidRequest request = BidRequest.builder()
397+
.imp(singletonList(Imp.builder()
398+
.id("impId1")
399+
.video(Video.builder().build())
400+
.audio(Audio.builder().build())
401+
.build()))
402+
.build();
403+
404+
// when and then
405+
assertThat(updater.update(request)).isEqualTo(BidRequest.builder()
406+
.badv(asList("domain1.com", "domain2.com"))
407+
.bcat(asList("cat1", "cat2"))
408+
.bapp(asList("app1", "app2"))
409+
.imp(singletonList(Imp.builder()
410+
.id("impId1")
411+
.video(Video.builder().battr(asList(3, 4)).build())
412+
.audio(Audio.builder().battr(asList(5, 6)).build())
413+
.build()))
414+
.build());
415+
}
416+
417+
@Test
418+
public void shouldNotUpdateMissingVideo() {
419+
// given
420+
final RequestUpdater updater = RequestUpdater.create(
421+
BlockedAttributes.builder()
422+
.badv(asList("domain1.com", "domain2.com"))
423+
.bcat(asList("cat1", "cat2"))
424+
.bapp(asList("app1", "app2"))
425+
.btype(singletonMap("impId1", asList(1, 2)))
426+
.battr(Map.of(
427+
MediaType.BANNER, singletonMap("impId1", asList(1, 2)),
428+
MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
429+
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
430+
.build());
431+
final BidRequest request = BidRequest.builder()
432+
.imp(singletonList(Imp.builder()
433+
.id("impId1")
434+
.banner(Banner.builder().build())
435+
.audio(Audio.builder().build())
436+
.build()))
437+
.build();
438+
439+
// when and then
440+
assertThat(updater.update(request)).isEqualTo(BidRequest.builder()
441+
.badv(asList("domain1.com", "domain2.com"))
442+
.bcat(asList("cat1", "cat2"))
443+
.bapp(asList("app1", "app2"))
444+
.imp(singletonList(Imp.builder()
445+
.id("impId1")
446+
.banner(Banner.builder()
447+
.btype(asList(1, 2))
448+
.battr(asList(1, 2))
449+
.build())
450+
.audio(Audio.builder().battr(asList(5, 6)).build())
451+
.build()))
452+
.build());
453+
}
454+
455+
@Test
456+
public void shouldNotUpdateMissingAudio() {
457+
// given
458+
final RequestUpdater updater = RequestUpdater.create(
459+
BlockedAttributes.builder()
460+
.badv(asList("domain1.com", "domain2.com"))
461+
.bcat(asList("cat1", "cat2"))
462+
.bapp(asList("app1", "app2"))
463+
.btype(singletonMap("impId1", asList(1, 2)))
464+
.battr(Map.of(
465+
MediaType.BANNER, singletonMap("impId1", asList(1, 2)),
466+
MediaType.VIDEO, singletonMap("impId1", asList(3, 4)),
467+
MediaType.AUDIO, singletonMap("impId1", asList(5, 6))))
468+
.build());
469+
final BidRequest request = BidRequest.builder()
470+
.imp(singletonList(Imp.builder()
471+
.id("impId1")
472+
.banner(Banner.builder().build())
473+
.video(Video.builder().build())
474+
.build()))
475+
.build();
476+
477+
// when and then
478+
assertThat(updater.update(request)).isEqualTo(BidRequest.builder()
479+
.badv(asList("domain1.com", "domain2.com"))
480+
.bcat(asList("cat1", "cat2"))
481+
.bapp(asList("app1", "app2"))
482+
.imp(singletonList(Imp.builder()
483+
.id("impId1")
484+
.banner(Banner.builder()
485+
.btype(asList(1, 2))
486+
.battr(asList(1, 2))
487+
.build())
488+
.video(Video.builder().battr(asList(3, 4)).build())
489+
.build()))
490+
.build());
491+
}
376492
}

0 commit comments

Comments
 (0)