Skip to content
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.

Sign up for GitHub

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

Fix passing ext.prebid.targeting.* fields #1105

Merged
merged 1 commit into from
Jan 20, 2021
Merged

Fix passing ext.prebid.targeting.* fields #1105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions src/main/java/org/prebid/server/auction/AuctionRequestFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ public class AuctionRequestFactory {
private static final ConditionalLogger EMPTY_ACCOUNT_LOGGER = new ConditionalLogger("empty_account", logger);
private static final ConditionalLogger UNKNOWN_ACCOUNT_LOGGER = new ConditionalLogger("unknown_account", logger);

public static final String WEB_CHANNEL = "web";
public static final String APP_CHANNEL = "app";
private static final String WEB_CHANNEL = "web";
private static final String APP_CHANNEL = "app";

private final long maxRequestSize;
private final boolean enforceValidAccount;
Expand All @@ -100,7 +100,7 @@ public class AuctionRequestFactory {
private final TimeoutResolver timeoutResolver;
private final TimeoutFactory timeoutFactory;
private final ApplicationSettings applicationSettings;
private final IdGenerator idGenerator;
private final IdGenerator sourceIdGenerator;
private final PrivacyEnforcementService privacyEnforcementService;
private final JacksonMapper mapper;
private final OrtbTypesResolver ortbTypesResolver;
Expand All @@ -122,7 +122,7 @@ public AuctionRequestFactory(long maxRequestSize,
TimeoutResolver timeoutResolver,
TimeoutFactory timeoutFactory,
ApplicationSettings applicationSettings,
IdGenerator idGenerator,
IdGenerator sourceIdGenerator,
PrivacyEnforcementService privacyEnforcementService,
JacksonMapper mapper) {

Expand All @@ -143,7 +143,7 @@ public AuctionRequestFactory(long maxRequestSize,
this.timeoutResolver = Objects.requireNonNull(timeoutResolver);
this.timeoutFactory = Objects.requireNonNull(timeoutFactory);
this.applicationSettings = Objects.requireNonNull(applicationSettings);
this.idGenerator = Objects.requireNonNull(idGenerator);
this.sourceIdGenerator = Objects.requireNonNull(sourceIdGenerator);
this.privacyEnforcementService = Objects.requireNonNull(privacyEnforcementService);
this.mapper = Objects.requireNonNull(mapper);
}
Expand Down Expand Up @@ -450,7 +450,7 @@ private Site populateSite(Site site, HttpServerRequest request) {
private Source populateSource(Source source) {
final String tid = source != null ? source.getTid() : null;
if (StringUtils.isEmpty(tid)) {
final String generatedId = idGenerator.generateId();
final String generatedId = sourceIdGenerator.generateId();
if (StringUtils.isNotEmpty(generatedId)) {
final Source.SourceBuilder builder = source != null ? source.toBuilder() : Source.builder();
return builder
Expand Down Expand Up @@ -558,22 +558,17 @@ private ExtRequestTargeting targetingOrNull(ExtRequestPrebid prebid, Set<BidType
final boolean isIncludeWinnersNull = isTargetingNotNull && targeting.getIncludewinners() == null;
final boolean isIncludeBidderKeysNull = isTargetingNotNull && targeting.getIncludebidderkeys() == null;

final ExtRequestTargeting result;
if (isPriceGranularityNull || isPriceGranularityTextual || isIncludeWinnersNull || isIncludeBidderKeysNull) {
result = ExtRequestTargeting.builder()
.pricegranularity(populatePriceGranularity(targeting, isPriceGranularityNull,
return targeting.toBuilder()
.pricegranularity(resolvePriceGranularity(targeting, isPriceGranularityNull,
isPriceGranularityTextual, impMediaTypes))
.mediatypepricegranularity(targeting.getMediatypepricegranularity())
.includewinners(isIncludeWinnersNull || targeting.getIncludewinners())
.includebidderkeys(isIncludeBidderKeysNull
? !isWinningOnly(prebid.getCache())
: targeting.getIncludebidderkeys())
.includeformat(targeting.getIncludeformat())
.build();
} else {
result = null;
}
return result;
return null;
}

/**
Expand All @@ -591,16 +586,17 @@ private boolean isWinningOnly(ExtRequestPrebidCache cache) {
* In case of valid string price granularity replaced it with appropriate custom view.
* In case of invalid string value throws {@link InvalidRequestException}.
*/
private JsonNode populatePriceGranularity(ExtRequestTargeting targeting, boolean isPriceGranularityNull,
boolean isPriceGranularityTextual, Set<BidType> impMediaTypes) {
final JsonNode priceGranularityNode = targeting.getPricegranularity();
private JsonNode resolvePriceGranularity(ExtRequestTargeting targeting, boolean isPriceGranularityNull,
boolean isPriceGranularityTextual, Set<BidType> impMediaTypes) {

final boolean hasAllMediaTypes = checkExistingMediaTypes(targeting.getMediatypepricegranularity())
.containsAll(impMediaTypes);

if (isPriceGranularityNull && !hasAllMediaTypes) {
return mapper.mapper().valueToTree(ExtPriceGranularity.from(PriceGranularity.DEFAULT));
}

final JsonNode priceGranularityNode = targeting.getPricegranularity();
if (isPriceGranularityTextual) {
final PriceGranularity priceGranularity;
try {
Expand All @@ -610,6 +606,7 @@ private JsonNode populatePriceGranularity(ExtRequestTargeting targeting, boolean
}
return mapper.mapper().valueToTree(ExtPriceGranularity.from(priceGranularity));
}

return priceGranularityNode;
}

Expand Down Expand Up @@ -776,8 +773,8 @@ private Future<Account> accountFrom(BidRequest bidRequest, Timeout timeout, Rout
return blankAccountId
? responseForEmptyAccount(routingContext)
: applicationSettings.getAccountById(accountId, timeout)
.compose(this::ensureAccountActive,
exception -> accountFallback(exception, accountId, routingContext));
.compose(this::ensureAccountActive,
exception -> accountFallback(exception, accountId, routingContext));
}

/**
Expand Down
41 changes: 21 additions & 20 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public class BidResponseCreator {
private final BidderCatalog bidderCatalog;
private final EventsService eventsService;
private final StoredRequestProcessor storedRequestProcessor;
private final IdGenerator idGenerator;
private final IdGenerator bidIdGenerator;
private final int truncateAttrChars;
private final Clock clock;
private final JacksonMapper mapper;
Expand All @@ -115,7 +115,7 @@ public BidResponseCreator(CacheService cacheService,
BidderCatalog bidderCatalog,
EventsService eventsService,
StoredRequestProcessor storedRequestProcessor,
IdGenerator idGenerator,
IdGenerator bidIdGenerator,
int truncateAttrChars,
Clock clock,
JacksonMapper mapper) {
Expand All @@ -124,7 +124,7 @@ public BidResponseCreator(CacheService cacheService,
this.bidderCatalog = Objects.requireNonNull(bidderCatalog);
this.eventsService = Objects.requireNonNull(eventsService);
this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor);
this.idGenerator = Objects.requireNonNull(idGenerator);
this.bidIdGenerator = Objects.requireNonNull(bidIdGenerator);
this.truncateAttrChars = validateTruncateAttrChars(truncateAttrChars);
this.clock = Objects.requireNonNull(clock);
this.mapper = Objects.requireNonNull(mapper);
Expand Down Expand Up @@ -175,7 +175,6 @@ private static int validateTruncateAttrChars(int truncateAttrChars) {
if (truncateAttrChars < 0 || truncateAttrChars > 255) {
throw new IllegalArgumentException("truncateAttrChars must be between 0 and 255");
}

return truncateAttrChars;
}

Expand Down Expand Up @@ -203,8 +202,8 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
final Set<Bid> winningBidsByBidder = newOrEmptySet(targeting);

final GeneratedBidIds generatedBidIds = GeneratedBidIds.of(bidderResponses,
(ignored, bid) -> idGenerator.getType() != IdGeneratorType.none
? idGenerator.generateId()
(ignored, bid) -> bidIdGenerator.getType() != IdGeneratorType.none
? bidIdGenerator.generateId()
: bid.getId());

// determine winning bids only if targeting is present
Expand Down Expand Up @@ -313,7 +312,7 @@ private static BidderBid mostValuableBid(List<BidderBid> bidderBids) {
.filter(bidderBid -> StringUtils.isNotBlank(bidderBid.getBid().getDealid()))
.collect(Collectors.toList());

List<BidderBid> processedBidderBids = dealBidderBids.isEmpty() ? bidderBids : dealBidderBids;
final List<BidderBid> processedBidderBids = dealBidderBids.isEmpty() ? bidderBids : dealBidderBids;

return processedBidderBids.stream()
.max(Comparator.comparing(bidderBid -> bidderBid.getBid().getPrice(), Comparator.naturalOrder()))
Expand Down Expand Up @@ -448,16 +447,16 @@ private static boolean isValidForCaching(Bid bid) {
return bid.getDealid() != null ? price.compareTo(BigDecimal.ZERO) >= 0 : price.compareTo(BigDecimal.ZERO) > 0;
}

private GeneratedBidIds getGeneratedVideoBidIds(
List<BidderResponse> bidderResponses,
GeneratedBidIds generatedBidIds,
List<Imp> imps) {
private GeneratedBidIds getGeneratedVideoBidIds(List<BidderResponse> bidderResponses,
GeneratedBidIds generatedBidIds,
List<Imp> imps) {

final List<BidderResponse> vastModifyAllowedResponses = bidderResponses.stream()
.filter(bidderResponse -> bidderCatalog.isModifyingVastXmlAllowed(bidderResponse.getBidder()))
.map(bidderResponse -> makeVideoBidsBidderResponse(bidderResponse, imps))
.filter(Objects::nonNull)
.collect(Collectors.toList());

return GeneratedBidIds.of(vastModifyAllowedResponses,
(bidder, bid) -> generatedBidIds.getGeneratedId(bidder, bid.getId(), bid.getImpid()));
}
Expand All @@ -466,7 +465,9 @@ private static BidderResponse makeVideoBidsBidderResponse(BidderResponse bidderR
final List<BidderBid> videoBidderBids = bidderResponse.getSeatBid().getBids().stream()
.filter(bidderBid -> isVideoBid(bidderBid, imps))
.collect(Collectors.toList());

final BidderSeatBid bidderSeatBid = bidderResponse.getSeatBid();

return CollectionUtils.isNotEmpty(videoBidderBids)
? BidderResponse.of(
bidderResponse.getBidder(),
Expand Down Expand Up @@ -540,11 +541,10 @@ private Map<String, List<ExtBidderError>> toExtBidderErrors(List<BidderResponse>
CacheServiceResult cacheResult,
VideoStoredDataResult videoStoredDataResult,
Map<String, List<ExtBidderError>> bidErrors) {
final BidRequest bidRequest = auctionContext.getBidRequest();
final Map<String, List<ExtBidderError>> errors = new HashMap<>();

errors.putAll(extractBidderErrors(bidderResponses));
errors.putAll(extractDeprecatedBiddersErrors(bidRequest));
errors.putAll(extractDeprecatedBiddersErrors(auctionContext.getBidRequest()));
errors.putAll(extractPrebidErrors(videoStoredDataResult, auctionContext));
errors.putAll(extractCacheErrors(cacheResult));
if (MapUtils.isNotEmpty(bidErrors)) {
Expand Down Expand Up @@ -880,7 +880,7 @@ private Bid toBid(BidderBid bidderBid,
final ExtBidPrebidVideo extBidPrebidVideo = getExtBidPrebidVideo(bid.getExt());

final ExtBidPrebid extBidPrebid = ExtBidPrebid.builder()
.bidid(idGenerator.getType() != IdGeneratorType.none ? generatedBidId : null)
.bidid(bidIdGenerator.getType() != IdGeneratorType.none ? generatedBidId : null)
.type(bidType)
.targeting(targetingKeywords)
.cache(cache)
Expand Down Expand Up @@ -1014,8 +1014,9 @@ private TargetingKeywordsCreator resolveKeywordsCreator(BidType bidType,
boolean isApp,
BidRequest bidRequest,
Account account) {
final Map<BidType, TargetingKeywordsCreator> keywordsCreatorByBidType =
keywordsCreatorByBidType(targeting, isApp, bidRequest, account);

final Map<BidType, TargetingKeywordsCreator> keywordsCreatorByBidType = keywordsCreatorByBidType(targeting,
isApp, bidRequest, account);

return keywordsCreatorByBidType.getOrDefault(bidType, keywordsCreator(targeting, isApp, bidRequest, account));
}
Expand All @@ -1024,8 +1025,10 @@ private TargetingKeywordsCreator resolveKeywordsCreator(BidType bidType,
* Extracts targeting keywords settings from the bid request and creates {@link TargetingKeywordsCreator}
* instance if it is present.
*/
private TargetingKeywordsCreator keywordsCreator(
ExtRequestTargeting targeting, boolean isApp, BidRequest bidRequest, Account account) {
private TargetingKeywordsCreator keywordsCreator(ExtRequestTargeting targeting,
boolean isApp,
BidRequest bidRequest,
Account account) {

final JsonNode priceGranularityNode = targeting.getPricegranularity();
return priceGranularityNode == null || priceGranularityNode.isNull()
Expand All @@ -1043,7 +1046,6 @@ private Map<BidType, TargetingKeywordsCreator> keywordsCreatorByBidType(ExtReque
Account account) {

final ExtMediaTypePriceGranularity mediaTypePriceGranularity = targeting.getMediatypepricegranularity();

if (mediaTypePriceGranularity == null) {
return Collections.emptyMap();
}
Expand Down Expand Up @@ -1127,7 +1129,6 @@ private CacheAsset toCacheAsset(String cacheId) {
private String integrationFrom(AuctionContext auctionContext) {
final ExtRequest extRequest = auctionContext.getBidRequest().getExt();
final ExtRequestPrebid prebid = extRequest == null ? null : extRequest.getPrebid();

return prebid != null ? prebid.getIntegration() : null;
}

Expand Down
26 changes: 26 additions & 0 deletions src/test/java/org/prebid/server/auction/AuctionRequestFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.prebid.server.privacy.gdpr.model.TcfContext;
import org.prebid.server.privacy.model.Privacy;
import org.prebid.server.privacy.model.PrivacyContext;
import org.prebid.server.proto.openrtb.ext.ExtIncludeBrandCategory;
import org.prebid.server.proto.openrtb.ext.request.ExtGranularityRange;
import org.prebid.server.proto.openrtb.ext.request.ExtMediaTypePriceGranularity;
import org.prebid.server.proto.openrtb.ext.request.ExtPriceGranularity;
Expand Down Expand Up @@ -1140,6 +1141,31 @@ public void shouldNotChangeAnyOtherExtRequestPrebidCacheFields() {
.containsOnly(tuple(cacheBids, cacheVastxml));
}

@Test
public void shouldNotChangeAnyOtherExtRequestPrebidTargetingFields() {
// given
givenBidRequest(BidRequest.builder()
.imp(singletonList(Imp.builder().ext(mapper.createObjectNode()).build()))
.ext(ExtRequest.of(ExtRequestPrebid.builder()
.targeting(ExtRequestTargeting.builder()
.includebrandcategory(ExtIncludeBrandCategory.of(1, "publisher", true))
.truncateattrchars(10)
.build())
.build()))
.build());

// when
final BidRequest request = factory.fromRequest(routingContext, 0L).result().getBidRequest();

// then
assertThat(singletonList(request))
.extracting(BidRequest::getExt)
.extracting(ExtRequest::getPrebid)
.extracting(ExtRequestPrebid::getTargeting)
.extracting(ExtRequestTargeting::getIncludebrandcategory, ExtRequestTargeting::getTruncateattrchars)
.containsOnly(tuple(ExtIncludeBrandCategory.of(1, "publisher", true), 10));
}

@Test
public void shouldSetCacheWinningonlyFromRequestWhenCacheWinningonlyIsPresent() {
// given
Expand Down
11 changes: 10 additions & 1 deletion ...test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,14 +297,23 @@
"precision": 2
},
"includewinners": true,
"includebidderkeys": true
"includebidderkeys": true,
"includebrandcategory": {
"primaryadserver": 1,
"publisher": "",
"with_category": true
}
},
"cache": {
"vastxml": {}
},
"channel": {
"name": "web"
}
},
"appnexus": {
"include_brand_category": true,
"brand_category_uniqueness": true
}
}
}
13 changes: 11 additions & 2 deletions ...test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,23 @@
"precision": 2
},
"includewinners": true,
"includebidderkeys": true
"includebidderkeys": true,
"includebrandcategory": {
"primaryadserver": 1,
"publisher": "",
"with_category": true
}
},
"cache": {
"vastxml": {}
},
"channel": {
"name": "web"
}
},
"appnexus": {
"include_brand_category": true,
"brand_category_uniqueness": true
}
}
}
}