Skip to content

Commit b794ade

Browse files
AntoxaAntoxicAntoxaAntoxic
committed
Add a warning for when targeting attributes are truncated
1 parent fa13078 commit b794ade

File tree

3 files changed

+88
-34
lines changed

3 files changed

+88
-34
lines changed

‎src/main/java/org/prebid/server/auction/BidResponseCreator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,8 +1564,8 @@ private Bid toBid(BidInfo bidInfo,
15641564
final String seat = targetingInfo.getSeat();
15651565
final String categoryDuration = bidInfo.getCategory();
15661566
targetingKeywords = keywordsCreator != null
1567-
? keywordsCreator.makeFor(
1568-
bid, seat, isWinningBid, cacheId, bidType.getName(), videoCacheId, categoryDuration, account)
1567+
? keywordsCreator.makeFor(bid, seat, isWinningBid, cacheId, bidType.getName(),
1568+
videoCacheId, categoryDuration, account, bidWarnings)
15691569
: null;
15701570
} else {
15711571
targetingKeywords = null;

‎src/main/java/org/prebid/server/auction/TargetingKeywordsCreator.java‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import com.iab.openrtb.response.Bid;
44
import org.apache.commons.lang3.StringUtils;
5+
import org.prebid.server.bidder.model.BidderError;
56
import org.prebid.server.proto.openrtb.ext.request.ExtPriceGranularity;
7+
import org.prebid.server.proto.openrtb.ext.response.ExtBidderError;
68
import org.prebid.server.settings.model.Account;
79

810
import java.math.BigDecimal;
@@ -154,7 +156,8 @@ Map<String, String> makeFor(Bid bid,
154156
String format,
155157
String vastCacheId,
156158
String categoryDuration,
157-
Account account) {
159+
Account account,
160+
Map<String, List<ExtBidderError>> bidWarnings) {
158161

159162
final Map<String, String> keywords = makeFor(
160163
bidder,
@@ -170,13 +173,13 @@ Map<String, String> makeFor(Bid bid,
170173
account);
171174

172175
if (resolver == null) {
173-
return truncateKeys(keywords);
176+
return truncateKeys(keywords, bidWarnings);
174177
}
175178

176179
final Map<String, String> augmentedKeywords = new HashMap<>(keywords);
177180
augmentedKeywords.putAll(resolver.resolve(bid, bidder));
178181

179-
return truncateKeys(augmentedKeywords);
182+
return truncateKeys(augmentedKeywords, bidWarnings);
180183
}
181184

182185
/**
@@ -261,18 +264,36 @@ private static String sizeFrom(Integer width, Integer height) {
261264
: null;
262265
}
263266

264-
private Map<String, String> truncateKeys(Map<String, String> keyValues) {
265-
return truncateAttrChars > 0
266-
? keyValues.entrySet().stream()
267-
.collect(Collectors
268-
.toMap(keyValue -> truncateKey(keyValue.getKey()), Map.Entry::getValue, (key1, key2) -> key1))
269-
: keyValues;
267+
private Map<String, String> truncateKeys(Map<String, String> keyValues,
268+
Map<String, List<ExtBidderError>> bidWarnings) {
269+
270+
if (truncateAttrChars > 0) {
271+
final List<String> truncatedKeys = new ArrayList<>();
272+
final Map<String, String> keys = keyValues.entrySet().stream().collect(Collectors.toMap(
273+
keyValue -> truncateKey(keyValue.getKey(), truncatedKeys),
274+
Map.Entry::getValue,
275+
(key1, key2) -> key1));
276+
277+
if (!truncatedKeys.isEmpty()) {
278+
final String errorMessage = "The following keys have been truncated: %s"
279+
.formatted(String.join(", ", truncatedKeys));
280+
bidWarnings.computeIfAbsent("targeting", ignored -> new ArrayList<>())
281+
.add(ExtBidderError.of(BidderError.Type.bad_input.getCode(), errorMessage));
282+
}
283+
284+
return keys;
285+
}
286+
287+
return keyValues;
270288
}
271289

272-
private String truncateKey(String key) {
273-
return key.length() > truncateAttrChars
274-
? key.substring(0, truncateAttrChars)
275-
: key;
290+
private String truncateKey(String key, List<String> truncatedKeys) {
291+
if (key.length() > truncateAttrChars) {
292+
truncatedKeys.add(key);
293+
return key.substring(0, truncateAttrChars);
294+
}
295+
296+
return key;
276297
}
277298

278299
/**

‎src/test/java/org/prebid/server/auction/TargetingKeywordsCreatorTest.java‎

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
package org.prebid.server.auction;
22

33
import com.iab.openrtb.response.Bid;
4+
import org.assertj.core.api.InstanceOfAssertFactories;
45
import org.junit.jupiter.api.Test;
56
import org.junit.jupiter.api.extension.ExtendWith;
67
import org.mockito.junit.jupiter.MockitoExtension;
78
import org.prebid.server.proto.openrtb.ext.request.ExtGranularityRange;
89
import org.prebid.server.proto.openrtb.ext.request.ExtPriceGranularity;
10+
import org.prebid.server.proto.openrtb.ext.response.ExtBidderError;
911
import org.prebid.server.settings.model.Account;
1012

1113
import java.math.BigDecimal;
14+
import java.util.HashMap;
15+
import java.util.List;
1216
import java.util.Map;
1317

1418
import static java.util.Collections.singletonList;
@@ -46,7 +50,7 @@ public void shouldReturnTargetingKeywordsForOrdinaryBidOpenrtb() {
4650
null,
4751
null,
4852
defaultKeyPrefix)
49-
.makeFor(bid, "bidder1", false, null, null, null, null, Account.empty("accountId"));
53+
.makeFor(bid, "bidder1", false, null, null, null, null, Account.empty("accountId"), new HashMap<>());
5054

5155
// then
5256
assertThat(keywords).containsOnly(
@@ -77,7 +81,8 @@ public void shouldReturnTargetingKeywordsWithEntireKeysOpenrtb() {
7781
null,
7882
null,
7983
defaultKeyPrefix)
80-
.makeFor(bid, "veryververyverylongbidder1", false, null, null, null, null, Account.empty("accountId"));
84+
.makeFor(bid, "veryververyverylongbidder1", false, null, null,
85+
null, null, Account.empty("accountId"), new HashMap<>());
8186

8287
// then
8388
assertThat(keywords).containsOnly(
@@ -113,7 +118,7 @@ public void shouldReturnTargetingKeywordsForWinningBidOpenrtb() {
113118
null,
114119
defaultKeyPrefix)
115120
.makeFor(bid, "bidder1", true, "cacheId1", "banner",
116-
"videoCacheId1", "categoryDuration", Account.empty("accountId"));
121+
"videoCacheId1", "categoryDuration", Account.empty("accountId"), new HashMap<>());
117122

118123
// then
119124
assertThat(keywords).containsOnly(
@@ -156,7 +161,7 @@ public void shouldIncludeFormatOpenrtb() {
156161
null,
157162
null,
158163
defaultKeyPrefix)
159-
.makeFor(bid, "", true, null, "banner", null, null, Account.empty("accountId"));
164+
.makeFor(bid, "", true, null, "banner", null, null, Account.empty("accountId"), new HashMap<>());
160165

161166
// then
162167
assertThat(keywords).contains(entry("hb_format", "banner"));
@@ -182,7 +187,7 @@ public void shouldNotIncludeCacheIdAndDealIdAndSizeOpenrtb() {
182187
null,
183188
null,
184189
defaultKeyPrefix)
185-
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"));
190+
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"), new HashMap<>());
186191

187192
// then
188193
assertThat(keywords).doesNotContainKeys("hb_cache_id_bidder", "hb_deal_bidder", "hb_size_bidder",
@@ -209,7 +214,7 @@ public void shouldReturnEnvKeyForAppRequestOpenrtb() {
209214
null,
210215
null,
211216
defaultKeyPrefix)
212-
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"));
217+
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"), new HashMap<>());
213218

214219
// then
215220
assertThat(keywords).contains(
@@ -237,7 +242,7 @@ public void shouldNotIncludeWinningBidTargetingIfIncludeWinnersFlagIsFalse() {
237242
null,
238243
null,
239244
defaultKeyPrefix)
240-
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"));
245+
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"), new HashMap<>());
241246

242247
// then
243248
assertThat(keywords).doesNotContainKeys("hb_bidder", "hb_pb");
@@ -263,7 +268,7 @@ public void shouldIncludeWinningBidTargetingIfIncludeWinnersFlagIsTrue() {
263268
null,
264269
null,
265270
defaultKeyPrefix)
266-
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"));
271+
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"), new HashMap<>());
267272

268273
// then
269274
assertThat(keywords).containsKeys("hb_bidder", "hb_pb");
@@ -289,7 +294,7 @@ public void shouldNotIncludeBidderKeysTargetingIfIncludeBidderKeysFlagIsFalse()
289294
null,
290295
null,
291296
defaultKeyPrefix)
292-
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"));
297+
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"), new HashMap<>());
293298

294299
// then
295300
assertThat(keywords).doesNotContainKeys("hb_bidder_bidder1", "hb_pb_bidder1");
@@ -315,7 +320,7 @@ public void shouldIncludeBidderKeysTargetingIfIncludeBidderKeysFlagIsTrue() {
315320
null,
316321
null,
317322
defaultKeyPrefix)
318-
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"));
323+
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"), new HashMap<>());
319324

320325
// then
321326
assertThat(keywords).containsKeys("hb_bidder_bidder1", "hb_pb_bidder1");
@@ -325,6 +330,7 @@ public void shouldIncludeBidderKeysTargetingIfIncludeBidderKeysFlagIsTrue() {
325330
public void shouldTruncateTargetingBidderKeywordsIfTruncateAttrCharsIsDefined() {
326331
// given
327332
final Bid bid = Bid.builder().price(BigDecimal.ONE).build();
333+
final Map<String, List<ExtBidderError>> bidWarnings = new HashMap<>();
328334

329335
// when
330336
final Map<String, String> keywords = TargetingKeywordsCreator.create(
@@ -341,17 +347,24 @@ public void shouldTruncateTargetingBidderKeywordsIfTruncateAttrCharsIsDefined()
341347
null,
342348
null,
343349
defaultKeyPrefix)
344-
.makeFor(bid, "someVeryLongBidderName", true, null, null, null, null, Account.empty("accountId"));
350+
.makeFor(bid, "someVeryLongBidderName", true, null, null,
351+
null, null, Account.empty("accountId"), bidWarnings);
345352

346353
// then
347354
assertThat(keywords).hasSize(2)
348355
.containsKeys("hb_bidder_someVeryLo", "hb_pb_someVeryLongBi");
356+
assertThat(bidWarnings)
357+
.extracting(warnings -> warnings.get("targeting"))
358+
.asInstanceOf(InstanceOfAssertFactories.LIST)
359+
.containsOnly(ExtBidderError.of(2, "The following keys have been truncated: "
360+
+ "hb_pb_someVeryLongBidderName, hb_bidder_someVeryLongBidderName"));
349361
}
350362

351363
@Test
352364
public void shouldTruncateTargetingWithoutBidderSuffixKeywordsIfTruncateAttrCharsIsDefined() {
353365
// given
354366
final Bid bid = Bid.builder().price(BigDecimal.ONE).build();
367+
final Map<String, List<ExtBidderError>> bidWarnings = new HashMap<>();
355368

356369
// when
357370
final Map<String, String> keywords = TargetingKeywordsCreator.create(
@@ -368,17 +381,23 @@ public void shouldTruncateTargetingWithoutBidderSuffixKeywordsIfTruncateAttrChar
368381
null,
369382
null,
370383
defaultKeyPrefix)
371-
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"));
384+
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"), bidWarnings);
372385

373386
// then
374387
assertThat(keywords).hasSize(2)
375388
.containsKeys("hb_bidd", "hb_pb");
389+
390+
assertThat(bidWarnings)
391+
.extracting(warnings -> warnings.get("targeting"))
392+
.asInstanceOf(InstanceOfAssertFactories.LIST)
393+
.containsOnly(ExtBidderError.of(2, "The following keys have been truncated: hb_bidder"));
376394
}
377395

378396
@Test
379397
public void shouldTruncateTargetingAndDropDuplicatedWhenTruncateIsTooShort() {
380398
// given
381399
final Bid bid = Bid.builder().price(BigDecimal.ONE).build();
400+
final Map<String, List<ExtBidderError>> bidWarnings = new HashMap<>();
382401

383402
// when
384403
final Map<String, String> keywords = TargetingKeywordsCreator.create(
@@ -395,18 +414,24 @@ public void shouldTruncateTargetingAndDropDuplicatedWhenTruncateIsTooShort() {
395414
null,
396415
null,
397416
defaultKeyPrefix)
398-
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"));
417+
.makeFor(bid, "bidder", true, null, null, null, null, Account.empty("accountId"), bidWarnings);
399418

400419
// then
401-
// Without truncating: "hb_bidder", "hb_bidder_bidder", "hb_env", "hb_env_bidder", "hb_pb", "hb_pb_bidder"
402420
assertThat(keywords).hasSize(4)
403421
.containsKeys("hb_bid", "hb_env", "hb_pb", "hb_pb_");
422+
423+
assertThat(bidWarnings)
424+
.extracting(warnings -> warnings.get("targeting"))
425+
.asInstanceOf(InstanceOfAssertFactories.LIST)
426+
.containsOnly(ExtBidderError.of(2, "The following keys have been truncated: "
427+
+ "hb_pb_bidder, hb_bidder, hb_bidder_bidder, hb_env_bidder"));
404428
}
405429

406430
@Test
407431
public void shouldNotTruncateTargetingKeywordsIfTruncateAttrCharsIsNotDefined() {
408432
// given
409433
final Bid bid = Bid.builder().price(BigDecimal.ONE).build();
434+
final Map<String, List<ExtBidderError>> bidWarnings = new HashMap<>();
410435

411436
// when
412437
final Map<String, String> keywords = TargetingKeywordsCreator.create(
@@ -423,11 +448,13 @@ public void shouldNotTruncateTargetingKeywordsIfTruncateAttrCharsIsNotDefined()
423448
null,
424449
null,
425450
defaultKeyPrefix)
426-
.makeFor(bid, "someVeryLongBidderName", true, null, null, null, null, Account.empty("accountId"));
451+
.makeFor(bid, "someVeryLongBidderName", true, null, null,
452+
null, null, Account.empty("accountId"), bidWarnings);
427453

428454
// then
429455
assertThat(keywords).hasSize(2)
430456
.containsKeys("hb_bidder_someVeryLongBidderName", "hb_pb_someVeryLongBidderName");
457+
assertThat(bidWarnings).isEmpty();
431458
}
432459

433460
@Test
@@ -440,6 +467,7 @@ public void shouldTruncateKeysFromResolver() {
440467

441468
final TargetingKeywordsResolver resolver = mock(TargetingKeywordsResolver.class);
442469
given(resolver.resolve(any(), anyString())).willReturn(singletonMap("key_longer_than_twenty", "value1"));
470+
final Map<String, List<ExtBidderError>> bidWarnings = new HashMap<>();
443471

444472
// when
445473
final Map<String, String> keywords = TargetingKeywordsCreator.create(
@@ -456,10 +484,15 @@ public void shouldTruncateKeysFromResolver() {
456484
null,
457485
resolver,
458486
defaultKeyPrefix)
459-
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"));
487+
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"), bidWarnings);
460488

461489
// then
462490
assertThat(keywords).contains(entry("key_longer_than_twen", "value1"));
491+
492+
assertThat(bidWarnings)
493+
.extracting(warnings -> warnings.get("targeting"))
494+
.asInstanceOf(InstanceOfAssertFactories.LIST)
495+
.containsOnly(ExtBidderError.of(2, "The following keys have been truncated: key_longer_than_twenty"));
463496
}
464497

465498
@Test
@@ -488,7 +521,7 @@ public void shouldIncludeKeywordsFromResolver() {
488521
null,
489522
resolver,
490523
defaultKeyPrefix)
491-
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"));
524+
.makeFor(bid, "bidder1", true, null, null, null, null, Account.empty("accountId"), new HashMap<>());
492525

493526
// then
494527
assertThat(keywords).contains(entry("keyword1", "value1"));
@@ -514,7 +547,7 @@ public void shouldIncludeDealBidTargetingIfAlwaysIncludeDealsFlagIsTrue() {
514547
null,
515548
null,
516549
defaultKeyPrefix)
517-
.makeFor(bid, "bidder1", false, null, null, null, null, Account.empty("accountId"));
550+
.makeFor(bid, "bidder1", false, null, null, null, null, Account.empty("accountId"), new HashMap<>());
518551

519552
// then
520553
assertThat(keywords).containsOnlyKeys("hb_bidder_bidder1", "hb_deal_bidder1", "hb_pb_bidder1");
@@ -540,7 +573,7 @@ public void shouldNotIncludeDealBidTargetingIfAlwaysIncludeDealsFlagIsFalse() {
540573
null,
541574
null,
542575
defaultKeyPrefix)
543-
.makeFor(bid, "bidder1", false, null, null, null, null, Account.empty("accountId"));
576+
.makeFor(bid, "bidder1", false, null, null, null, null, Account.empty("accountId"), new HashMap<>());
544577

545578
// then
546579
assertThat(keywords).doesNotContainKeys("hb_bidder_bidder1", "hb_deal_bidder1", "hb_pb_bidder1");

0 commit comments

Comments
 (0)