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

Jump to bottom

Add hb_env=amp for Amp Requests #3433

Merged
merged 3 commits into from
Sep 19, 2024
Merged

Add hb_env=amp for Amp Requests #3433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e489af
Add hb_env=amp for Amp Requests
AntoxaAntoxic Sep 10, 2024
5473983
Refactor.
CTMBNara Sep 16, 2024
958d1f4
Test: `"hb_env"` (#3448)
marki1an Sep 18, 2024
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
31 changes: 17 additions & 14 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 @@ -122,6 +122,8 @@ public class BidResponseCreator {
private static final Integer MAX_TARGETING_KEY_LENGTH = 11;
private static final String DEFAULT_TARGETING_KEY_PREFIX = "hb";
public static final String DEFAULT_DEBUG_KEY = "prebid";
private static final String TARGETING_ENV_APP_VALUE = "mobile-app";
private static final String TARGETING_ENV_AMP_VALUE = "amp";

private final CoreCacheService coreCacheService;
private final BidderCatalog bidderCatalog;
Expand Down Expand Up @@ -1325,13 +1327,11 @@ private Bid toBid(BidInfo bidInfo,
final String cacheId = cacheInfo != null ? cacheInfo.getCacheId() : null;
final String videoCacheId = cacheInfo != null ? cacheInfo.getVideoCacheId() : null;

final boolean isApp = bidRequest.getApp() != null;

final Map<String, String> targetingKeywords;
final String bidderCode = targetingInfo.getBidderCode();
if (shouldIncludeTargetingInResponse(targeting, bidInfo.getTargetingInfo())) {
final TargetingKeywordsCreator keywordsCreator = resolveKeywordsCreator(
bidType, targeting, isApp, bidRequest, account, bidWarnings);
bidType, targeting, bidRequest, account, bidWarnings);

final boolean isWinningBid = targetingInfo.isWinningBid();
final String categoryDuration = bidInfo.getCategory();
Expand Down Expand Up @@ -1552,32 +1552,30 @@ private Events createEvents(String bidder,

private TargetingKeywordsCreator resolveKeywordsCreator(BidType bidType,
ExtRequestTargeting targeting,
boolean isApp,
BidRequest bidRequest,
Account account,
Map<String, List<ExtBidderError>> bidWarnings) {

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

return keywordsCreatorByBidType.getOrDefault(
bidType, keywordsCreator(targeting, isApp, bidRequest, account, bidWarnings));
bidType, keywordsCreator(targeting, bidRequest, account, bidWarnings));
}

/**
* 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,
Map<String, List<ExtBidderError>> bidWarnings) {

final JsonNode priceGranularityNode = targeting.getPricegranularity();
return priceGranularityNode == null || priceGranularityNode.isNull()
? null
: createKeywordsCreator(targeting, isApp, priceGranularityNode, bidRequest, account, bidWarnings);
: createKeywordsCreator(targeting, priceGranularityNode, bidRequest, account, bidWarnings);
}

/**
Expand All @@ -1586,7 +1584,6 @@ private TargetingKeywordsCreator keywordsCreator(ExtRequestTargeting targeting,
*/
private Map<BidType, TargetingKeywordsCreator> keywordsCreatorByBidType(
ExtRequestTargeting targeting,
boolean isApp,
BidRequest bidRequest,
Account account,
Map<String, List<ExtBidderError>> bidWarnings) {
Expand All @@ -1602,43 +1599,49 @@ private Map<BidType, TargetingKeywordsCreator> keywordsCreatorByBidType(
final boolean isBannerNull = banner == null || banner.isNull();
if (!isBannerNull) {
result.put(
BidType.banner, createKeywordsCreator(targeting, isApp, banner, bidRequest, account, bidWarnings));
BidType.banner, createKeywordsCreator(targeting, banner, bidRequest, account, bidWarnings));
}

final ObjectNode video = mediaTypePriceGranularity.getVideo();
final boolean isVideoNull = video == null || video.isNull();
if (!isVideoNull) {
result.put(
BidType.video, createKeywordsCreator(targeting, isApp, video, bidRequest, account, bidWarnings));
BidType.video, createKeywordsCreator(targeting, video, bidRequest, account, bidWarnings));
}

final ObjectNode xNative = mediaTypePriceGranularity.getXNative();
final boolean isNativeNull = xNative == null || xNative.isNull();
if (!isNativeNull) {
result.put(
BidType.xNative, createKeywordsCreator(targeting, isApp, xNative, bidRequest, account, bidWarnings)
BidType.xNative, createKeywordsCreator(targeting, xNative, bidRequest, account, bidWarnings)
);
}

return result;
}

private TargetingKeywordsCreator createKeywordsCreator(ExtRequestTargeting targeting,
boolean isApp,
JsonNode priceGranularity,
BidRequest bidRequest,
Account account,
Map<String, List<ExtBidderError>> bidWarnings) {
final int resolvedTruncateAttrChars = resolveTruncateAttrChars(targeting, account);
final String resolveKeyPrefix = resolveAndValidateKeyPrefix(
bidRequest, account, resolvedTruncateAttrChars, bidWarnings);

final String env = Optional.ofNullable(bidRequest.getExt())
.map(ExtRequest::getPrebid)
.map(ExtRequestPrebid::getAmp)
.map(ignored -> TARGETING_ENV_AMP_VALUE)
.orElse(bidRequest.getApp() == null ? null : TARGETING_ENV_APP_VALUE);

return TargetingKeywordsCreator.create(
parsePriceGranularity(priceGranularity),
BooleanUtils.toBoolean(targeting.getIncludewinners()),
BooleanUtils.toBoolean(targeting.getIncludebidderkeys()),
BooleanUtils.toBoolean(targeting.getAlwaysincludedeals()),
BooleanUtils.isTrue(targeting.getIncludeformat()),
isApp,
env,
resolvedTruncateAttrChars,
cacheHost,
cachePath,
Expand Down
18 changes: 7 additions & 11 deletions src/main/java/org/prebid/server/auction/TargetingKeywordsCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ public class TargetingKeywordsCreator {
* It will exist only if the incoming bidRequest defiend request.app instead of request.site.
*/
private static final String ENV_KEY = "_env";
/**
* Used as a value for ENV_KEY.
*/
private static final String ENV_APP_VALUE = "mobile-app";
/**
* Name of the Bidder. For example, "appnexus" or "rubicon".
*/
Expand Down Expand Up @@ -87,7 +83,7 @@ public class TargetingKeywordsCreator {
private final boolean includeBidderKeys;
private final boolean alwaysIncludeDeals;
private final boolean includeFormat;
private final boolean isApp;
private final String env;
private final int truncateAttrChars;
private final String cacheHost;
private final String cachePath;
Expand All @@ -99,7 +95,7 @@ private TargetingKeywordsCreator(PriceGranularity priceGranularity,
boolean includeBidderKeys,
boolean alwaysIncludeDeals,
boolean includeFormat,
boolean isApp,
String env,
int truncateAttrChars,
String cacheHost,
String cachePath,
Expand All @@ -111,7 +107,7 @@ private TargetingKeywordsCreator(PriceGranularity priceGranularity,
this.includeBidderKeys = includeBidderKeys;
this.alwaysIncludeDeals = alwaysIncludeDeals;
this.includeFormat = includeFormat;
this.isApp = isApp;
this.env = env;
this.truncateAttrChars = truncateAttrChars;
this.cacheHost = cacheHost;
this.cachePath = cachePath;
Expand All @@ -127,7 +123,7 @@ public static TargetingKeywordsCreator create(ExtPriceGranularity extPriceGranul
boolean includeBidderKeys,
boolean alwaysIncludeDeals,
boolean includeFormat,
boolean isApp,
String env,
int truncateAttrChars,
String cacheHost,
String cachePath,
Expand All @@ -139,7 +135,7 @@ public static TargetingKeywordsCreator create(ExtPriceGranularity extPriceGranul
includeBidderKeys,
alwaysIncludeDeals,
includeFormat,
isApp,
env,
truncateAttrChars,
cacheHost,
cachePath,
Expand Down Expand Up @@ -230,8 +226,8 @@ private Map<String, String> makeFor(String bidder,
if (StringUtils.isNotBlank(dealId)) {
keywordMap.put(this.keyPrefix + DEAL_KEY, dealId);
}
if (isApp) {
keywordMap.put(this.keyPrefix + ENV_KEY, ENV_APP_VALUE);
if (env != null) {
keywordMap.put(this.keyPrefix + ENV_KEY, env);
}
if (StringUtils.isNotBlank(categoryDuration)) {
keywordMap.put(this.keyPrefix + CATEGORY_DURATION_KEY, categoryDuration);
Expand Down
25 changes: 23 additions & 2 deletions src/test/groovy/org/prebid/server/functional/tests/TargetingSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ import static org.prebid.server.functional.testcontainers.Dependencies.getNetwor
class TargetingSpec extends BaseSpec {

private static final Integer TARGETING_PARAM_NAME_MAX_LENGTH = 20
private static final Integer TARGETING_KEYS_SIZE = 14
private static final Integer MAX_AMP_TARGETING_TRUNCATION_LENGTH = 11
private static final String DEFAULT_TARGETING_PREFIX = "hb"
private static final Integer TARGETING_PREFIX_LENGTH = 11
private static final Integer MAX_TRUNCATE_ATTR_CHARS = 255
private static final String HB_ENV_AMP = "amp"

def "PBS should include targeting bidder specific keys when alwaysIncludeDeals is true and deal bid wins"() {
given: "Bid request with alwaysIncludeDeals = true"
Expand Down Expand Up @@ -668,7 +670,7 @@ class TargetingSpec extends BaseSpec {

then: "Amp response should contain default targeting prefix"
def targeting = ampResponse.targeting
assert targeting.size() == 12
assert targeting.size() == TARGETING_KEYS_SIZE
assert targeting.keySet().every { it -> it.startsWith(DEFAULT_TARGETING_PREFIX) }
}

Expand All @@ -694,7 +696,7 @@ class TargetingSpec extends BaseSpec {

then: "Amp response should contain targeting response with custom prefix"
def targeting = ampResponse.targeting
assert targeting.size() == 12
assert targeting.size() == TARGETING_KEYS_SIZE
assert targeting.keySet().every { it -> it.startsWith(DEFAULT_TARGETING_PREFIX) }
}

Expand Down Expand Up @@ -1100,6 +1102,25 @@ class TargetingSpec extends BaseSpec {
assert ampData.secondUnknownField == secondUnknownValue
}

def "PBS amp should always send hb_env=amp when stored request does not contain app"() {
given: "Default AmpRequest"
def ampRequest = AmpRequest.defaultAmpRequest

and: "Default bid request"
def ampStoredRequest = BidRequest.defaultBidRequest

and: "Create and save stored request into DB"
def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest)
storedRequestDao.save(storedRequest)

when: "PBS processes amp request"
def ampResponse = defaultPbsService.sendAmpRequest(ampRequest)

then: "Amp response should contain amp hb_env"
def targeting = ampResponse.targeting
assert targeting["hb_env"] == HB_ENV_AMP
}

private static PrebidServerService getEnabledWinBidsPbsService() {
pbsServiceFactory.getService(["auction.cache.only-winning-bids": "true"])
}
Expand Down
43 changes: 41 additions & 2 deletions src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import org.prebid.server.proto.openrtb.ext.request.ExtRequest;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidAdservertargetingRule;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidAmp;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidChannel;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestTargeting;
import org.prebid.server.proto.openrtb.ext.request.ExtStoredRequest;
Expand Down Expand Up @@ -1551,7 +1552,7 @@ public void shouldPopulateTargetingKeywords() {

final AuctionContext auctionContext = givenAuctionContext(
givenBidRequest(
identity(),
request -> request.app(App.builder().build()),
extBuilder -> extBuilder.targeting(givenTargeting()),
givenImp()),
contextBuilder -> contextBuilder.auctionParticipations(toAuctionParticipant(bidderResponses)));
Expand All @@ -1569,7 +1570,45 @@ public void shouldPopulateTargetingKeywords() {
tuple("hb_pb", "5.00"),
tuple("hb_pb_bidder1", "5.00"),
tuple("hb_bidder", "bidder1"),
tuple("hb_bidder_bidder1", "bidder1"));
tuple("hb_bidder_bidder1", "bidder1"),
tuple("hb_env", "mobile-app"),
tuple("hb_env_bidder1", "mobile-app"));

verify(coreCacheService, never()).cacheBidsOpenrtb(anyList(), any(), any(), any());
}

@Test
public void shouldPopulateTargetingKeywordsForAmpRequest() {
// given
final Bid bid = Bid.builder().id("bidId1").price(BigDecimal.valueOf(5.67)).impid(IMP_ID).build();
final List<BidderResponse> bidderResponses = singletonList(BidderResponse.of("bidder1",
givenSeatBid(BidderBid.of(bid, banner, "USD")), 100));

final AuctionContext auctionContext = givenAuctionContext(
givenBidRequest(
request -> request.app(App.builder().build()),
extBuilder -> extBuilder
.targeting(givenTargeting())
.amp(ExtRequestPrebidAmp.of(Map.of("key", "value"))),
givenImp()),
contextBuilder -> contextBuilder.auctionParticipations(toAuctionParticipant(bidderResponses)));

// when
final BidResponse bidResponse = target.create(auctionContext, CACHE_INFO, MULTI_BIDS).result();

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid).hasSize(1)
.extracting(extractedBid -> toExtBidPrebid(extractedBid.getExt()).getTargeting())
.flatExtracting(Map::entrySet)
.extracting(Map.Entry::getKey, Map.Entry::getValue)
.containsOnly(
tuple("hb_pb", "5.00"),
tuple("hb_pb_bidder1", "5.00"),
tuple("hb_bidder", "bidder1"),
tuple("hb_bidder_bidder1", "bidder1"),
tuple("hb_env", "amp"),
tuple("hb_env_bidder1", "amp"));

verify(coreCacheService, never()).cacheBidsOpenrtb(anyList(), any(), any(), any());
}
Expand Down
Loading
Loading