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.

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

Add adUnitCode to the imp.ext.prebid exception list #3610

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
44 changes: 15 additions & 29 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.prebid.server.auction;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.iab.openrtb.request.App;
Expand Down Expand Up @@ -56,7 +55,6 @@
import org.prebid.server.bidder.model.Price;
import org.prebid.server.cookie.UidsCookie;
import org.prebid.server.exception.InvalidRequestException;
import org.prebid.server.exception.PreBidException;
import org.prebid.server.execution.timeout.Timeout;
import org.prebid.server.execution.timeout.TimeoutFactory;
import org.prebid.server.floors.PriceFloorAdjuster;
Expand All @@ -79,8 +77,6 @@
import org.prebid.server.proto.openrtb.ext.request.ExtApp;
import org.prebid.server.proto.openrtb.ext.request.ExtBidderConfigOrtb;
import org.prebid.server.proto.openrtb.ext.request.ExtDooh;
import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebid;
import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebidFloors;
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.ExtRequestPrebidBidderConfig;
Expand Down Expand Up @@ -126,6 +122,8 @@ public class ExchangeService {
private static final Integer DEFAULT_MULTIBID_LIMIT_MAX = 9;
private static final String EID_ALLOWED_FOR_ALL_BIDDERS = "*";
private static final BigDecimal THOUSAND = BigDecimal.valueOf(1000);
private static final Set<String> BIDDER_FIELDS_EXCEPTION_LIST = Set.of(
"adunitcode", "storedrequest", "options", "is_rewarded_inventory");

private final double logSamplingRate;
private final BidderCatalog bidderCatalog;
Expand Down Expand Up @@ -870,7 +868,7 @@ private Imp prepareImp(Imp imp,
return imp.toBuilder()
.bidfloor(adjustedPrice.getValue())
.bidfloorcur(adjustedPrice.getCurrency())
.ext(prepareImpExt(bidder, imp.getExt(), adjustedPrice.getValue(), transmitTid, useFirstPartyData))
.ext(prepareImpExt(bidder, imp.getExt(), transmitTid, useFirstPartyData))
.build();
}

Expand All @@ -885,11 +883,10 @@ private Price resolveBidPrice(Imp imp,

private ObjectNode prepareImpExt(String bidder,
ObjectNode impExt,
BigDecimal adjustedFloor,
boolean transmitTid,
boolean useFirstPartyData) {
final JsonNode bidderNode = bidderParamsFromImpExt(impExt).get(bidder);
final JsonNode impExtPrebid = prepareImpExt(impExt.get(PREBID_EXT), adjustedFloor);
final JsonNode impExtPrebid = cleanUpImpExtPrebid(impExt.get(PREBID_EXT));
Optional.ofNullable(impExtPrebid).ifPresentOrElse(
ext -> impExt.set(PREBID_EXT, ext),
() -> impExt.remove(PREBID_EXT));
Expand All @@ -901,33 +898,22 @@ private ObjectNode prepareImpExt(String bidder,
return fpdResolver.resolveImpExt(impExt, useFirstPartyData);
}

private JsonNode prepareImpExt(JsonNode extImpPrebidNode, BigDecimal adjustedFloor) {
if (extImpPrebidNode.size() <= 1) {
private JsonNode cleanUpImpExtPrebid(JsonNode extImpPrebid) {
if (extImpPrebid.size() <= 1) {
return null;
}

final ExtImpPrebid extImpPrebid = extImpPrebid(extImpPrebidNode);
final ExtImpPrebidFloors floors = extImpPrebid.getFloors();
final ExtImpPrebidFloors updatedFloors = floors != null
? ExtImpPrebidFloors.of(floors.getFloorRule(),
floors.getFloorRuleValue(),
adjustedFloor,
floors.getFloorMin(),
floors.getFloorMinCur())
: null;

return mapper.mapper().valueToTree(extImpPrebid(extImpPrebidNode).toBuilder()
.floors(updatedFloors)
.bidder(null)
.build());
}
final Iterator<String> fieldsIterator = extImpPrebid.fieldNames();
final ObjectNode modifiedExtImpPrebid = extImpPrebid.deepCopy();

private ExtImpPrebid extImpPrebid(JsonNode extImpPrebid) {
try {
return mapper.mapper().treeToValue(extImpPrebid, ExtImpPrebid.class);
} catch (JsonProcessingException e) {
throw new PreBidException("Error decoding imp.ext.prebid: " + e.getMessage(), e);
while (fieldsIterator.hasNext()) {
final String fieldName = fieldsIterator.next();
if (!BIDDER_FIELDS_EXCEPTION_LIST.contains(fieldName)) {
modifiedExtImpPrebid.remove(fieldName);
}
}

return modifiedExtImpPrebid;
}

private App prepareApp(App app, ObjectNode fpdApp, boolean useFirstPartyData) {
Expand Down
48 changes: 2 additions & 46 deletions src/main/java/org/prebid/server/bidder/rubicon/RubiconBidder.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.prebid.server.bidder.rubicon;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
Expand Down Expand Up @@ -52,7 +51,6 @@
import org.prebid.server.bidder.rubicon.proto.request.RubiconExtPrebidBiddersBidder;
import org.prebid.server.bidder.rubicon.proto.request.RubiconExtPrebidBiddersBidderDebug;
import org.prebid.server.bidder.rubicon.proto.request.RubiconImpExt;
import org.prebid.server.bidder.rubicon.proto.request.RubiconImpExtPrebid;
import org.prebid.server.bidder.rubicon.proto.request.RubiconImpExtRp;
import org.prebid.server.bidder.rubicon.proto.request.RubiconImpExtRpRtb;
import org.prebid.server.bidder.rubicon.proto.request.RubiconImpExtRpTrack;
Expand Down Expand Up @@ -89,7 +87,6 @@
import org.prebid.server.proto.openrtb.ext.request.ExtDevice;
import org.prebid.server.proto.openrtb.ext.request.ExtImpContextDataAdserver;
import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebid;
import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebidFloors;
import org.prebid.server.proto.openrtb.ext.request.ExtPublisher;
import org.prebid.server.proto.openrtb.ext.request.ExtRegs;
import org.prebid.server.proto.openrtb.ext.request.ExtRegsDsa;
Expand Down Expand Up @@ -507,16 +504,7 @@ private Imp makeImp(Imp imp,
final Imp.ImpBuilder builder = imp.toBuilder()
.metric(makeMetrics(imp))
.ext(mapper.mapper().valueToTree(
makeImpExt(
imp,
bidRequest,
extImpRubicon,
resolvedFormats,
site,
app,
extRequest,
ipfCurrency,
priceFloorResult)));
makeImpExt(imp, extImpRubicon, resolvedFormats, site, app, extRequest)));

final BigDecimal resolvedBidFloor = ipfFloor != null
? convertToXAPICurrency(ipfFloor, ipfCurrency, imp, bidRequest)
Expand Down Expand Up @@ -672,18 +660,11 @@ private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor,
}

private RubiconImpExt makeImpExt(Imp imp,
BidRequest bidRequest,
ExtImpRubicon rubiconImpExt,
Set<ImpMediaType> formats,
Site site,
App app,
ExtRequest extRequest,
String ipfResolvedCurrency,
PriceFloorResult priceFloorResult) {

final RubiconImpExtPrebid rubiconImpExtPrebid = priceFloorResult != null
? makeRubiconExtPrebid(priceFloorResult, ipfResolvedCurrency, imp, bidRequest)
: null;
ExtRequest extRequest) {

final RubiconImpExtRpRtb rubiconImpExtRpRtb = CollectionUtils.isNotEmpty(formats)
? RubiconImpExtRpRtb.of(formats)
Expand All @@ -702,7 +683,6 @@ private RubiconImpExt makeImpExt(Imp imp,
.gpid(getGpid(imp.getExt()))
.skadn(getSkadn(imp.getExt()))
.tid(getTid(imp.getExt()))
.prebid(rubiconImpExtPrebid)
.build();
}

Expand All @@ -722,30 +702,6 @@ private JsonNode makeTarget(Imp imp, ExtImpRubicon rubiconImpExt, Site site, App
return result;
}

private RubiconImpExtPrebid makeRubiconExtPrebid(PriceFloorResult priceFloorResult,
String currency,
Imp imp,
BidRequest bidRequest) {
final ObjectNode impExt = imp.getExt();
final ExtImpPrebid extImpPrebid = extImpPrebid(impExt.get(PREBID_EXT));
final ExtImpPrebidFloors floors = extImpPrebid != null ? extImpPrebid.getFloors() : null;

return RubiconImpExtPrebid.of(ExtImpPrebidFloors.of(
priceFloorResult.getFloorRule(),
convertToXAPICurrency(priceFloorResult.getFloorRuleValue(), currency, imp, bidRequest),
convertToXAPICurrency(priceFloorResult.getFloorValue(), currency, imp, bidRequest),
floors != null ? floors.getFloorMin() : null,
floors != null ? floors.getFloorMinCur() : null));
}

private ExtImpPrebid extImpPrebid(JsonNode extImpPrebid) {
try {
return mapper.mapper().treeToValue(extImpPrebid, ExtImpPrebid.class);
} catch (JsonProcessingException e) {
throw new PreBidException("Error decoding imp.ext.prebid: " + e.getMessage(), e);
}
}

private void mergeFirstPartyDataFromSite(Site site, ObjectNode result) {
// merge OPENRTB.site.ext.data.* to every impression – XAPI.imp[].ext.rp.target.*
final ExtSite siteExt = site != null ? site.getExt() : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,10 @@ public class ExtImpPrebid {
* Defines the contract for bidrequest.imp[i].ext.prebid.imp
*/
ObjectNode imp;

/**
* Defines the contract for bidrequest.imp[i].ext.prebid.adunitcode
*/
@JsonProperty("adunitcode")
String adUnitCode;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class ImpExtPrebid {
ImpExtPrebidFloors floors
Map passThrough
Map<BidderName, Imp> imp
@JsonProperty("adunitcode")
String adUnitCode

static ImpExtPrebid getDefaultImpExtPrebid() {
new ImpExtPrebid().tap {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Prebid {
List<MultiBid> multibid
Pbs pbs
Server server
Map<BidderName, Map<String, Integer>> bidderParams
Map bidderParams
ExtPrebidFloors floors
Map passThrough
Events events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -776,10 +776,32 @@ class BidderParamsSpec extends BaseSpec {
"request.imp[0].ext.prebid.bidder contains unknown bidder: anyUnsupportedBidder"]
}

def "PBS should emit warning and proceed auction when ext.prebid fields include adunitcode"() {
given: "Default bid request with populated ext.prebid.bidderParams"
def genericBidderParams = PBSUtils.randomString
def bidRequest = BidRequest.defaultBidRequest.tap {
ext.prebid.bidderParams = [adUnitCode : PBSUtils.randomString,
(GENERIC.value): genericBidderParams]
}

when: "PBS processes auction request"
def response = defaultPbsService.sendAuctionRequest(bidRequest)

then: "Response shouldn't contain error"
assert !response.ext?.errors

and: "PBS should emit an warning"
assert response?.ext?.warnings[PREBID]*.code == [999]
assert response?.ext?.warnings[PREBID]*.message ==
["WARNING: request.imp[0].ext.prebid.bidder.adUnitCode was dropped with a reason: " +
"request.imp[0].ext.prebid.bidder contains unknown bidder: adUnitCode"]
}

def "PBS shouldn't emit warning and proceed auction when all imp.ext fields known for PBS"() {
given: "Default bid request with populated imp.ext"
def impExt = ImpExt.getDefaultImpExt().tap {
prebid.bidder.generic = null
prebid.adUnitCode = PBSUtils.randomString
generic = new Generic()
ae = PBSUtils.randomNumber
all = PBSUtils.randomNumber
Expand All @@ -795,9 +817,15 @@ class BidderParamsSpec extends BaseSpec {
}

when: "PBS processes auction request"
defaultPbsService.sendAuctionRequest(bidRequest)
def response = defaultPbsService.sendAuctionRequest(bidRequest)

then: "Response shouldn't contain error"
assert !response.ext?.errors

then: "Bidder request should contain same field as requested"
and: "Response shouldn't contain warning"
assert !response.ext?.warnings

and: "Bidder request should contain same field as requested"
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
verifyAll(bidderRequest.imp[0].ext) {
bidder == impExt.generic
Expand All @@ -809,9 +837,40 @@ class BidderParamsSpec extends BaseSpec {
gpid == impExt.gpid
skadn == impExt.skadn
tid == impExt.tid
prebid.adUnitCode == impExt.prebid.adUnitCode
}
}

def "PBS should proceed auction without warning when all ext.prebid.bidderParams fields are known"() {
given: "Default bid request with populated ext.prebid.bidderParams"
def genericBidderParams = PBSUtils.randomString
def bidRequest = BidRequest.defaultBidRequest.tap {
ext.prebid.bidderParams = [ae : PBSUtils.randomString,
all : PBSUtils.randomString,
context : PBSUtils.randomString,
data : PBSUtils.randomString,
general : PBSUtils.randomString,
gpid : PBSUtils.randomString,
skadn : PBSUtils.randomString,
tid : PBSUtils.randomString,
(GENERIC.value): genericBidderParams
]
}

when: "PBS processes auction request"
def response = defaultPbsService.sendAuctionRequest(bidRequest)

then: "Response shouldn't contain error"
assert !response.ext?.errors

and: "Response shouldn't contain warning"
assert !response.ext?.warnings

and: "Bidder request should bidderParams only for bidder"
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert bidderRequest.ext.prebid.bidderParams == [(GENERIC.value): genericBidderParams]
}

def "PBS should send request to bidder when adapters.bidder.meta-info.currency-accepted not specified"() {
given: "PBS with adapter configuration"
def pbsService = pbsServiceFactory.getService("adapters.generic.meta-info.currency-accepted": "")
Expand Down Expand Up @@ -847,7 +906,7 @@ class BidderParamsSpec extends BaseSpec {
def "PBS should send request to bidder when adapters.bidder.aliases.bidder.meta-info.currency-accepted not specified"() {
given: "PBS with adapter configuration"
def pbsService = pbsServiceFactory.getService(
"adapters.generic.aliases.alias.enabled" : "true",
"adapters.generic.aliases.alias.enabled": "true",
"adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/auction".toString(),
"adapters.generic.aliases.alias.meta-info.currency-accepted": "")

Expand Down Expand Up @@ -955,7 +1014,7 @@ class BidderParamsSpec extends BaseSpec {
def "PBS should send request to bidder when adapters.bidder.aliases.bidder.meta-info.currency-accepted intersect with requested currency"() {
given: "PBS with adapter configuration"
def pbsService = pbsServiceFactory.getService(
"adapters.generic.aliases.alias.enabled" : "true",
"adapters.generic.aliases.alias.enabled": "true",
"adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/auction".toString(),
"adapters.generic.aliases.alias.meta-info.currency-accepted": "${USD},${EUR}".toString())

Expand Down Expand Up @@ -996,7 +1055,7 @@ class BidderParamsSpec extends BaseSpec {
def "PBS shouldn't send request to bidder and emit warning when adapters.bidder.aliases.bidder.meta-info.currency-accepted not intersect with requested currency"() {
given: "PBS with adapter configuration"
def pbsService = pbsServiceFactory.getService(
"adapters.generic.aliases.alias.enabled" : "true",
"adapters.generic.aliases.alias.enabled": "true",
"adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/auction".toString(),
"adapters.generic.aliases.alias.meta-info.currency-accepted": "${JPY},${CHF}".toString())

Expand Down
Loading
Loading