From 6e773d2d82894d3b53cea72d340aee6dc1229763 Mon Sep 17 00:00:00 2001 From: DGarbar Date: Mon, 26 Apr 2021 14:46:20 +0300 Subject: [PATCH 1/2] Make bid immutable --- .../java/com/iab/openrtb/response/Bid.java | 60 +++++--- .../server/auction/BidResponseCreator.java | 140 ++++++++++++------ .../server/auction/ExchangeService.java | 7 +- .../prebid/server/auction/model/BidInfo.java | 3 + .../bidder/appnexus/AppnexusBidder.java | 9 +- .../bidder/beachfront/BeachfrontBidder.java | 32 ++-- .../bidder/facebook/FacebookBidder.java | 8 +- .../server/bidder/gamma/GammaBidder.java | 13 +- .../server/bidder/gamma/model/GammaBid.java | 24 +-- .../prebid/server/bidder/model/BidderBid.java | 4 + .../server/bidder/model/BidderSeatBid.java | 4 + .../server/bidder/rubicon/RubiconBidder.java | 26 ++-- .../server/bidder/sovrn/SovrnBidder.java | 5 +- .../server/auction/ExchangeServiceTest.java | 9 +- .../server/bidder/gamma/GammaBidderTest.java | 112 +++++++------- 15 files changed, 281 insertions(+), 175 deletions(-) diff --git a/src/main/java/com/iab/openrtb/response/Bid.java b/src/main/java/com/iab/openrtb/response/Bid.java index 3e7922afa72..54ea0bbe945 100644 --- a/src/main/java/com/iab/openrtb/response/Bid.java +++ b/src/main/java/com/iab/openrtb/response/Bid.java @@ -1,9 +1,8 @@ package com.iab.openrtb.response; import com.fasterxml.jackson.databind.node.ObjectNode; -import lombok.Data; -import lombok.NoArgsConstructor; -import lombok.experimental.SuperBuilder; +import lombok.Builder; +import lombok.Value; import java.math.BigDecimal; import java.util.List; @@ -13,22 +12,19 @@ * relates to a specific impression in the bid request via the {@code impid} * attribute and constitutes an offer to buy that impression for a given * {@code price}. - *

- * IMPORTANT: unlike other data classes this one is mutable (annotated with {@link Data} instead of - * {@link lombok.Value}). Motivation: during the course of processing bids could be altered several times (price - * adjustment, post-processing). Creating new instance of the bid in each of these cases seems to cause unnecessary - * memory pressure. In order to avoid unnecessary allocations this class is made mutable (as an exception) i.e. this - * decision could be seen as a performance optimisation. */ -@SuperBuilder(toBuilder = true) -@NoArgsConstructor -@Data +@Builder(toBuilder = true) +@Value public class Bid { - /** Bidder generated bid ID to assist with logging/tracking. (required) */ + /** + * Bidder generated bid ID to assist with logging/tracking. (required) + */ String id; - /** ID of the Imp object in the related bid request. (required) */ + /** + * ID of the Imp object in the related bid request. (required) + */ String impid; /** @@ -72,7 +68,9 @@ public class Bid { */ String adm; - /** ID of a preloaded ad to be served if the bid wins. */ + /** + * ID of a preloaded ad to be served if the bid wins. + */ String adid; /** @@ -110,19 +108,29 @@ public class Bid { */ String crid; - /** IAB content categories of the creative. Refer to List 5.1. */ + /** + * IAB content categories of the creative. Refer to List 5.1. + */ List cat; - /** Set of attributes describing the creative. Refer to List 5.3. */ + /** + * Set of attributes describing the creative. Refer to List 5.3. + */ List attr; - /** API required by the markup if applicable. Refer to List 5.6. */ + /** + * API required by the markup if applicable. Refer to List 5.6. + */ Integer api; - /** Video response protocol of the markup if applicable. Refer to List 5.8. */ + /** + * Video response protocol of the markup if applicable. Refer to List 5.8. + */ Integer protocol; - /** Creative media rating per IQG guidelines. Refer to List 5.19. */ + /** + * Creative media rating per IQG guidelines. Refer to List 5.19. + */ Integer qagmediarating; /** @@ -138,10 +146,14 @@ public class Bid { */ String dealid; - /** Width of the creative in device independent pixels (DIPS). */ + /** + * Width of the creative in device independent pixels (DIPS). + */ Integer w; - /** Height of the creative in device independent pixels (DIPS). */ + /** + * Height of the creative in device independent pixels (DIPS). + */ Integer h; /** @@ -162,6 +174,8 @@ public class Bid { */ Integer exp; - /** Placeholder for bidder-specific extensions to OpenRTB. */ + /** + * Placeholder for bidder-specific extensions to OpenRTB. + */ ObjectNode ext; } diff --git a/src/main/java/org/prebid/server/auction/BidResponseCreator.java b/src/main/java/org/prebid/server/auction/BidResponseCreator.java index 49baeef1ade..4551cbf986c 100644 --- a/src/main/java/org/prebid/server/auction/BidResponseCreator.java +++ b/src/main/java/org/prebid/server/auction/BidResponseCreator.java @@ -167,9 +167,10 @@ Future create(List bidderResponses, .build(); final Map bidIdToGeneratedBidId = new HashMap<>(); - updateBidAdmInBidderResponses(bidderResponses, account, bidIdToGeneratedBidId, eventsContext); + final List modifiedBidderResponses = updateBidAdmInBidderResponses(bidderResponses, account, + bidIdToGeneratedBidId, eventsContext); - if (isEmptyBidderResponses(bidderResponses)) { + if (isEmptyBidderResponses(modifiedBidderResponses)) { final BidRequest bidRequest = auctionContext.getBidRequest(); return Future.succeededFuture(BidResponse.builder() .id(bidRequest.getId()) @@ -177,7 +178,7 @@ Future create(List bidderResponses, .nbr(0) // signal "Unknown Error" .seatbid(Collections.emptyList()) .ext(mapper.mapper().valueToTree(toExtBidResponse( - bidderResponses, + modifiedBidderResponses, auctionContext, CacheServiceResult.empty(), VideoStoredDataResult.empty(), @@ -188,7 +189,7 @@ Future create(List bidderResponses, } return cacheBidsAndCreateResponse( - bidderResponses, + modifiedBidderResponses, auctionContext, cacheInfo, bidderToMultiBids, @@ -197,15 +198,18 @@ Future create(List bidderResponses, debugEnabled); } - private void updateBidAdmInBidderResponses(List bidderResponses, - Account account, - Map bidIdToGeneratedBidId, - EventsContext eventsContext) { + private List updateBidAdmInBidderResponses(List bidderResponses, + Account account, + Map bidIdToGeneratedBidId, + EventsContext eventsContext) { + final List result = new ArrayList<>(); for (BidderResponse bidderResponse : bidderResponses) { final String bidder = bidderResponse.getBidder(); - for (BidderBid bidderBid : bidderResponse.getSeatBid().getBids()) { - final Bid bid = bidderBid.getBid(); + final List modifiedBidderBid = new ArrayList<>(); + final BidderSeatBid seatBid = bidderResponse.getSeatBid(); + for (BidderBid bidderBid : seatBid.getBids()) { + Bid bid = bidderBid.getBid(); final String generatedBidId = bidIdGenerator.getType() != IdGeneratorType.none ? bidIdGenerator.generateId() : null; @@ -221,10 +225,17 @@ private void updateBidAdmInBidderResponses(List bidderResponses, account.getId(), eventsContext); - bid.setAdm(adm); + bid = bid.toBuilder().adm(adm).build(); } + + modifiedBidderBid.add(bidderBid.with(bid)); } + + final BidderSeatBid modifiedSeatBid = seatBid.with(modifiedBidderBid); + result.add(bidderResponse.with(modifiedSeatBid)); } + + return result; } private static int validateTruncateAttrChars(int truncateAttrChars) { @@ -267,10 +278,10 @@ private Future cacheBidsAndCreateResponse(List bidd final Set winningBidInfos = targeting == null ? null : bidderResponseToTargetingBidInfos.values().stream() - .flatMap(Collection::stream) - .filter(TargetingBidInfo::isWinningBid) - .map(TargetingBidInfo::getBidInfo) - .collect(Collectors.toSet()); + .flatMap(Collection::stream) + .filter(TargetingBidInfo::isWinningBid) + .map(TargetingBidInfo::getBidInfo) + .collect(Collectors.toSet()); final Set bidsToCache = cacheInfo.isShouldCacheWinningBidsOnly() ? winningBidInfos : bidInfos; @@ -847,16 +858,21 @@ private SeatBid toSeatBid(List targetingBidInfos, .orElseThrow(() -> new IllegalArgumentException("Bidder was not defined for bidInfo")); final List bids = targetingBidInfos.stream() + .map(targetingBidInfo -> injectAdmWithCacheInfo( + targetingBidInfo, + requestCacheInfo, + bidToCacheInfo, + bidRequest, + bidErrors + )) + .filter(Objects::nonNull) .map(targetingBidInfo -> toBid( targetingBidInfo, targeting, bidRequest, - requestCacheInfo, - bidToCacheInfo, videoStoredDataResult.getImpIdToStoredVideo(), account, - eventsContext, - bidErrors)) + eventsContext)) .filter(Objects::nonNull) .collect(Collectors.toList()); @@ -867,36 +883,32 @@ private SeatBid toSeatBid(List targetingBidInfos, .build(); } - /** - * Returns an OpenRTB {@link Bid} with "prebid" and "bidder" extension fields populated. - */ - private Bid toBid(TargetingBidInfo targetingBidInfo, - ExtRequestTargeting targeting, - BidRequest bidRequest, - BidRequestCacheInfo requestCacheInfo, - Map bidsWithCacheIds, - Map impIdToStoredVideo, - Account account, - EventsContext eventsContext, - Map> bidErrors) { + private TargetingBidInfo injectAdmWithCacheInfo(TargetingBidInfo targetingBidInfo, + BidRequestCacheInfo requestCacheInfo, + Map bidsWithCacheIds, + BidRequest bidRequest, + Map> bidErrors) { final BidInfo bidInfo = targetingBidInfo.getBidInfo(); final Bid bid = bidInfo.getBid(); final BidType bidType = bidInfo.getBidType(); final String bidder = bidInfo.getBidder(); + final Imp correspondingImp = bidInfo.getCorrespondingImp(); final CacheInfo cacheInfo = bidsWithCacheIds.get(bid); + final boolean isApp = bidRequest.getApp() != null; + final String cacheId = cacheInfo != null ? cacheInfo.getCacheId() : null; final String videoCacheId = cacheInfo != null ? cacheInfo.getVideoCacheId() : null; + String modifiedBidAdm = bid.getAdm(); if ((videoCacheId != null && !requestCacheInfo.isReturnCreativeVideoBids()) || (cacheId != null && !requestCacheInfo.isReturnCreativeBids())) { - bid.setAdm(null); + modifiedBidAdm = null; } - final boolean isApp = bidRequest.getApp() != null; - if (isApp && bidType.equals(BidType.xNative) && bid.getAdm() != null) { + if (isApp && bidType.equals(BidType.xNative) && modifiedBidAdm != null) { try { - addNativeMarkup(bid, bidRequest.getImp()); + modifiedBidAdm = crateNativeMarkup(modifiedBidAdm, correspondingImp); } catch (PreBidException e) { bidErrors.computeIfAbsent(bidder, ignored -> new ArrayList<>()) .add(ExtBidderError.of(BidderError.Type.bad_server_response.getCode(), e.getMessage())); @@ -904,6 +916,34 @@ private Bid toBid(TargetingBidInfo targetingBidInfo, } } + final Bid modifiedBid = bid.toBuilder().adm(modifiedBidAdm).build(); + final BidInfo modifiedBidInfo = bidInfo.toBuilder() + .bid(modifiedBid) + .cacheInfo(cacheInfo) + .build(); + return targetingBidInfo.toBuilder().bidInfo(modifiedBidInfo).build(); + } + + /** + * Returns an OpenRTB {@link Bid} with "prebid" and "bidder" extension fields populated. + */ + private Bid toBid(TargetingBidInfo targetingBidInfo, + ExtRequestTargeting targeting, + BidRequest bidRequest, + Map impIdToStoredVideo, + Account account, + EventsContext eventsContext) { + final BidInfo bidInfo = targetingBidInfo.getBidInfo(); + final BidType bidType = bidInfo.getBidType(); + final String bidder = bidInfo.getBidder(); + final Bid bid = bidInfo.getBid(); + + final CacheInfo cacheInfo = bidInfo.getCacheInfo(); + final String cacheId = cacheInfo != null ? cacheInfo.getCacheId() : null; + final String videoCacheId = cacheInfo != null ? cacheInfo.getVideoCacheId() : null; + + final boolean isApp = bidRequest.getApp() != null; + final Map targetingKeywords; final String bidderCode = targetingBidInfo.getBidderCode(); if (targeting != null && targetingBidInfo.isTargetingEnabled() && targetingBidInfo.isBidderWinningBid()) { @@ -911,8 +951,8 @@ private Bid toBid(TargetingBidInfo targetingBidInfo, bidRequest, account); final boolean isWinningBid = targetingBidInfo.isWinningBid(); - targetingKeywords = keywordsCreator.makeFor(bid, bidderCode, isWinningBid, cacheId, bidType.getName(), - videoCacheId); + targetingKeywords = keywordsCreator.makeFor(bid, bidderCode, isWinningBid, cacheId, + bidType.getName(), videoCacheId); } else { targetingKeywords = null; } @@ -936,29 +976,29 @@ private Bid toBid(TargetingBidInfo targetingBidInfo, .video(extBidPrebidVideo) .build(); - bid.setExt(createBidExt(bid.getExt(), extBidPrebid)); - + final ObjectNode bidExt = createBidExt(bid.getExt(), extBidPrebid); final Integer ttl = cacheInfo != null ? ObjectUtils.max(cacheInfo.getTtl(), cacheInfo.getVideoTtl()) : null; - bid.setExp(ttl); - return bid; + return bid.toBuilder() + .ext(bidExt) + .exp(ttl) + .build(); } - private void addNativeMarkup(Bid bid, List imps) { + private String crateNativeMarkup(String bidAdm, Imp correspondingImp) { final Response nativeMarkup; try { - nativeMarkup = mapper.decodeValue(bid.getAdm(), Response.class); + nativeMarkup = mapper.decodeValue(bidAdm, Response.class); } catch (DecodeException e) { throw new PreBidException(e.getMessage()); } final List responseAssets = nativeMarkup.getAssets(); if (CollectionUtils.isNotEmpty(responseAssets)) { - final Native nativeImp = imps.stream() - .filter(imp -> imp.getId().equals(bid.getImpid()) && imp.getXNative() != null) - .findFirst() - .map(Imp::getXNative) - .orElseThrow(() -> new PreBidException("Could not find native imp")); + final Native nativeImp = correspondingImp != null ? correspondingImp.getXNative() : null; + if (nativeImp == null) { + throw new PreBidException("Could not find native imp"); + } final Request nativeRequest; try { @@ -968,8 +1008,10 @@ private void addNativeMarkup(Bid bid, List imps) { } responseAssets.forEach(asset -> setAssetTypes(asset, nativeRequest.getAssets())); - bid.setAdm(mapper.encode(nativeMarkup)); + return mapper.encode(nativeMarkup); } + + return bidAdm; } private static void setAssetTypes(Asset responseAsset, List requestAssets) { diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index 10cdcb17486..85aadc445cd 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -1096,15 +1096,16 @@ private BidderBid updateBidderBidWithBidPriceChanges(BidderBid bidderBid, updateExtWithOrigPriceValues(updatedBidExt, price, bidCurrency); + final Bid.BidBuilder bidBuilder = bid.toBuilder(); if (adjustedPrice.compareTo(price) != 0) { - bid.setPrice(adjustedPrice); + bidBuilder.price(adjustedPrice); } if (!updatedBidExt.isEmpty()) { - bid.setExt(updatedBidExt); + bidBuilder.ext(updatedBidExt); } - return bidderBid; + return bidderBid.with(bidBuilder.build()); } private static BidAdjustmentMediaType resolveBidAdjustmentMediaType(String bidImpId, diff --git a/src/main/java/org/prebid/server/auction/model/BidInfo.java b/src/main/java/org/prebid/server/auction/model/BidInfo.java index 27a7dfd73ea..a2d9ba2ab8b 100644 --- a/src/main/java/org/prebid/server/auction/model/BidInfo.java +++ b/src/main/java/org/prebid/server/auction/model/BidInfo.java @@ -4,6 +4,7 @@ import com.iab.openrtb.response.Bid; import lombok.Builder; import lombok.Value; +import org.prebid.server.cache.model.CacheInfo; import org.prebid.server.proto.openrtb.ext.response.BidType; @Builder(toBuilder = true) @@ -20,6 +21,8 @@ public class BidInfo { BidType bidType; + CacheInfo cacheInfo; + public String getBidId() { return generatedBidId != null ? generatedBidId : bid.getId(); } diff --git a/src/main/java/org/prebid/server/bidder/appnexus/AppnexusBidder.java b/src/main/java/org/prebid/server/bidder/appnexus/AppnexusBidder.java index 52568155086..19b875b2ab7 100644 --- a/src/main/java/org/prebid/server/bidder/appnexus/AppnexusBidder.java +++ b/src/main/java/org/prebid/server/bidder/appnexus/AppnexusBidder.java @@ -444,14 +444,17 @@ private BidderBid bidderBid(Bid bid, String currency) { } final String iabCategory = iabCategory(appnexus.getBrandCategoryId()); + + List cat = bid.getCat(); if (iabCategory != null) { - bid.setCat(Collections.singletonList(iabCategory)); + cat = Collections.singletonList(iabCategory); } else if (CollectionUtils.isNotEmpty(bid.getCat())) { //create empty categories array to force bid to be rejected - bid.setCat(Collections.emptyList()); + cat = Collections.emptyList(); } - return BidderBid.of(bid, bidType(appnexus.getBidAdType()), currency); + final Bid modifiedBid = bid.toBuilder().cat(cat).build(); + return BidderBid.of(modifiedBid, bidType(appnexus.getBidAdType()), currency); } private static String iabCategory(Integer brandId) { diff --git a/src/main/java/org/prebid/server/bidder/beachfront/BeachfrontBidder.java b/src/main/java/org/prebid/server/bidder/beachfront/BeachfrontBidder.java index f37945066a6..500ab36b1ae 100644 --- a/src/main/java/org/prebid/server/bidder/beachfront/BeachfrontBidder.java +++ b/src/main/java/org/prebid/server/bidder/beachfront/BeachfrontBidder.java @@ -453,30 +453,40 @@ private Result> processVideoResponse(String responseBody, HttpRe final List bids = bidResponse.getSeatbid().get(0).getBid(); final List imps = videoRequest.getRequest().getImp(); if (httpRequest.getUri().contains(NURL_VIDEO_ENDPOINT_SUFFIX)) { - return Result.withValues(updateVideoBids(bids, imps).stream() + return Result.withValues(updateNurlVideoBids(bids, imps).stream() .map(bid -> BidderBid.of(bid, BidType.video, bidResponse.getCur())) .collect(Collectors.toList())); } else { - return Result.withValues(bids.stream() - .peek(bid -> bid.setId(bid.getImpid() + "AdmVideo")) + return Result.withValues(updateVideoBids(bids).stream() .map(bid -> BidderBid.of(bid, BidType.video, bidResponse.getCur())) .collect(Collectors.toList())); } } - private static List updateVideoBids(List bids, List imps) { + private static List updateNurlVideoBids(List bids, List imps) { + final List result = new ArrayList<>(); for (int i = 0; i < bids.size(); i++) { - final Bid bid = bids.get(i); + Bid bid = bids.get(i); final Imp imp = imps.get(i); final String impId = imp.getId(); - bid.setCrid(getCrId(bid.getNurl())); - bid.setImpid(impId); - bid.setH(imp.getVideo().getH()); - bid.setW(imp.getVideo().getW()); - bid.setId(impId + "NurlVideo"); + + bid = bid.toBuilder() + .crid(getCrId(bid.getNurl())) + .impid(impId) + .h(imp.getVideo().getH()) + .w(imp.getVideo().getW()) + .id(impId + "NurlVideo") + .build(); + result.add(bid); } - return bids; + return result; + } + + private static List updateVideoBids(List bids) { + return bids.stream() + .map(bid -> bid.toBuilder().id(bid.getImpid() + "AdmVideo").build()) + .collect(Collectors.toList()); } private static String getCrId(String nurl) { diff --git a/src/main/java/org/prebid/server/bidder/facebook/FacebookBidder.java b/src/main/java/org/prebid/server/bidder/facebook/FacebookBidder.java index 7c0ba8b4776..0738c26e496 100644 --- a/src/main/java/org/prebid/server/bidder/facebook/FacebookBidder.java +++ b/src/main/java/org/prebid/server/bidder/facebook/FacebookBidder.java @@ -317,10 +317,12 @@ private BidderBid toBidderBid(Bid bid, List imps, String currency, List> makeBids(HttpCall httpCall, BidRequest bidR try { final GammaBidResponse bidResponse = mapper.decodeValue(body, GammaBidResponse.class); final List errors = new ArrayList<>(); - return Result.of(extractBidsAndFillErorrs(bidResponse, bidRequest, errors), errors); + return Result.of(extractBidsAndFillErrors(bidResponse, bidRequest, errors), errors); } catch (DecodeException e) { return Result.withError(BidderError.badServerResponse( String.format("bad server response: %s", e.getMessage()))); } } - private static List extractBidsAndFillErorrs(GammaBidResponse bidResponse, + private static List extractBidsAndFillErrors(GammaBidResponse bidResponse, BidRequest bidRequest, List errors) { return bidResponse == null || CollectionUtils.isEmpty(bidResponse.getSeatbid()) @@ -248,28 +248,29 @@ private static BidType getMediaTypes(String impId, List imps) { private static Bid convertBid(GammaBid gammaBid, BidType bidType) { final boolean isVideo = BidType.video.equals(bidType); - if (!isVideo && StringUtils.isBlank(gammaBid.getAdm())) { + if (!isVideo && StringUtils.isBlank(gammaBid.getBid().getAdm())) { throw new PreBidException("Missing Ad Markup. Run with request.debug = 1 for more info"); } + Bid bid = gammaBid.getBid(); if (isVideo) { //Return inline VAST XML Document (Section 6.4.2) final String vastXml = gammaBid.getVastXml(); if (StringUtils.isNotBlank(vastXml)) { - final Bid.BidBuilder bidBuilder = gammaBid.toBuilder().adm(vastXml); + final Bid.BidBuilder bidBuilder = gammaBid.getBid().toBuilder().adm(vastXml); final String vastUrl = gammaBid.getVastUrl(); if (StringUtils.isNotBlank(vastUrl)) { bidBuilder.nurl(vastUrl); } - return bidBuilder.build(); + bid = bidBuilder.build(); } else { throw new PreBidException("Missing Ad Markup. Run with request.debug = 1 for more info"); } } - return gammaBid; + return bid; } } diff --git a/src/main/java/org/prebid/server/bidder/gamma/model/GammaBid.java b/src/main/java/org/prebid/server/bidder/gamma/model/GammaBid.java index 26f24d9c498..991c1982b96 100644 --- a/src/main/java/org/prebid/server/bidder/gamma/model/GammaBid.java +++ b/src/main/java/org/prebid/server/bidder/gamma/model/GammaBid.java @@ -1,21 +1,27 @@ package org.prebid.server.bidder.gamma.model; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonUnwrapped; import com.iab.openrtb.response.Bid; -import lombok.Data; -import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; -import lombok.experimental.SuperBuilder; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.RequiredArgsConstructor; -@EqualsAndHashCode(callSuper = true) -@SuperBuilder -@NoArgsConstructor -@Data -public class GammaBid extends Bid { +@Getter +@Builder +@AllArgsConstructor(onConstructor = @__(@JsonIgnore)) +@RequiredArgsConstructor +public class GammaBid { + + @JsonUnwrapped + Bid bid; @JsonProperty("vastXml") String vastXml; @JsonProperty("vastUrl") String vastUrl; + } diff --git a/src/main/java/org/prebid/server/bidder/model/BidderBid.java b/src/main/java/org/prebid/server/bidder/model/BidderBid.java index 11aba88396e..52bd90db136 100644 --- a/src/main/java/org/prebid/server/bidder/model/BidderBid.java +++ b/src/main/java/org/prebid/server/bidder/model/BidderBid.java @@ -27,4 +27,8 @@ public class BidderBid { * Will be used for converting to ad server currency */ String bidCurrency; + + public BidderBid with(Bid bid) { + return BidderBid.of(bid, this.type, this.bidCurrency); + } } diff --git a/src/main/java/org/prebid/server/bidder/model/BidderSeatBid.java b/src/main/java/org/prebid/server/bidder/model/BidderSeatBid.java index 93e9afb809a..18c396a62c7 100644 --- a/src/main/java/org/prebid/server/bidder/model/BidderSeatBid.java +++ b/src/main/java/org/prebid/server/bidder/model/BidderSeatBid.java @@ -41,4 +41,8 @@ public class BidderSeatBid { * Error messages should help publishers understand what might account for "bad" bids. */ List errors; + + public BidderSeatBid with(List bids) { + return BidderSeatBid.of(bids, this.httpCalls, this.errors); + } } diff --git a/src/main/java/org/prebid/server/bidder/rubicon/RubiconBidder.java b/src/main/java/org/prebid/server/bidder/rubicon/RubiconBidder.java index 1f978943d22..7e225119457 100644 --- a/src/main/java/org/prebid/server/bidder/rubicon/RubiconBidder.java +++ b/src/main/java/org/prebid/server/bidder/rubicon/RubiconBidder.java @@ -930,7 +930,7 @@ private static ExtUserEidUid cleanExtUserEidUidStype(ExtUserEidUid extUserEidUid return extUserEidUidExt == null || !STYPE_TO_REMOVE.contains(extUserEidUidExt.getStype()) ? extUserEidUid : ExtUserEidUid.of(extUserEidUid.getId(), extUserEidUid.getAtype(), - ExtUserEidUidExt.of(extUserEidUidExt.getRtiPartner(), null)); + ExtUserEidUidExt.of(extUserEidUidExt.getRtiPartner(), null)); } private static Map> specialExtUserEids(List eids) { @@ -1092,10 +1092,10 @@ private Site makeSite(Site site, String impLanguage, ExtImpRubicon rubiconImpExt .content(makeSiteContent(null, impLanguage)) .build() : site.toBuilder() - .publisher(makePublisher(rubiconImpExt)) - .content(makeSiteContent(site.getContent(), impLanguage)) - .ext(makeSiteExt(site, rubiconImpExt)) - .build(); + .publisher(makePublisher(rubiconImpExt)) + .content(makeSiteContent(site.getContent(), impLanguage)) + .ext(makeSiteExt(site, rubiconImpExt)) + .build(); } private static Content makeSiteContent(Content siteContent, String impLanguage) { @@ -1181,23 +1181,27 @@ private static boolean validatePrice(Bid bid) { } private Bid updateBid(Bid bid, Imp imp, Float cmpOverrideFromRequest, BidResponse bidResponse) { + String bidId = bid.getId(); if (generateBidId) { // Since Rubicon XAPI returns openrtb_response.seatbid.bid.id not unique enough // generate new value for it - bid.setId(UUID.randomUUID().toString()); + bidId = UUID.randomUUID().toString(); } else if (Objects.equals(bid.getId(), "0")) { // Since Rubicon XAPI returns only one bid per response // copy bidResponse.bidid to openrtb_response.seatbid.bid.id - bid.setId(bidResponse.getBidid()); + bidId = bidResponse.getBidid(); } // Unconditionally set price if coming from CPM override final Float cpmOverride = ObjectUtils.defaultIfNull(cpmOverrideFromImp(imp), cmpOverrideFromRequest); - if (cpmOverride != null) { - bid.setPrice(new BigDecimal(String.valueOf(cpmOverride))); - } + final BigDecimal bidPrice = cpmOverride != null + ? new BigDecimal(String.valueOf(cpmOverride)) + : bid.getPrice(); - return bid; + return bid.toBuilder() + .id(bidId) + .price(bidPrice) + .build(); } private Float cmpOverrideFromRequest(BidRequest bidRequest) { diff --git a/src/main/java/org/prebid/server/bidder/sovrn/SovrnBidder.java b/src/main/java/org/prebid/server/bidder/sovrn/SovrnBidder.java index 3ee974870c1..646d7f6dac6 100644 --- a/src/main/java/org/prebid/server/bidder/sovrn/SovrnBidder.java +++ b/src/main/java/org/prebid/server/bidder/sovrn/SovrnBidder.java @@ -157,7 +157,8 @@ private static List bidsFromResponse(BidResponse bidResponse) { } private static Bid updateBid(Bid bid) { - bid.setAdm(HttpUtil.decodeUrl(bid.getAdm())); - return bid; + return bid.toBuilder() + .adm(HttpUtil.decodeUrl(bid.getAdm())) + .build(); } } diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index fec38e9c305..6a05b10758b 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -780,9 +780,16 @@ public void shouldCallBidResponseCreatorWithExpectedParamsAndUpdateDebugErrors() verify(bidResponseCreator).create(captor.capture(), eq(expectedAuctionContext), eq(expectedCacheInfo), eq(expectedMultiBidMap), eq(false)); + final ObjectNode expectedBidExt = mapper.createObjectNode().put("origbidcpm", new BigDecimal("7.89")); + final Bid expectedThirdBid = Bid.builder() + .id("bidId3") + .impid("impId1") + .price(BigDecimal.valueOf(7.89)) + .ext(expectedBidExt) + .build(); assertThat(captor.getValue()).containsOnly( BidderResponse.of("bidder2", BidderSeatBid.of(singletonList( - BidderBid.of(thirdBid, banner, null)), emptyList(), emptyList()), 0), + BidderBid.of(expectedThirdBid, banner, null)), emptyList(), emptyList()), 0), BidderResponse.of("bidder1", BidderSeatBid.of(emptyList(), emptyList(), emptyList()), 0)); } diff --git a/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java b/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java index 62169af095c..c77413323f4 100644 --- a/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java @@ -16,6 +16,8 @@ import org.junit.Test; import org.prebid.server.VertxTest; import org.prebid.server.bidder.gamma.model.GammaBid; +import org.prebid.server.bidder.gamma.model.GammaBidResponse; +import org.prebid.server.bidder.gamma.model.GammaSeatBid; import org.prebid.server.bidder.model.BidderBid; import org.prebid.server.bidder.model.BidderError; import org.prebid.server.bidder.model.HttpCall; @@ -238,13 +240,14 @@ public void makeBidsShouldSetAdmFromVastXmlIsPresentAndVideoType() throws JsonPr final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); final String adm = "ADM"; - final GammaBid bid = GammaBid.builder().id("impId").vastXml(adm).build(); + final Bid bid = Bid.builder().id("impId").build(); + final GammaBid gammaBid = GammaBid.builder().bid(bid).vastXml(adm).build(); final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( - BidResponse.builder() + GammaBidResponse.builder() .id("impId") .cur("USD") - .seatbid(singletonList(SeatBid.builder() - .bid(singletonList(bid)) + .seatbid(singletonList(GammaSeatBid.builder() + .bid(singletonList(gammaBid)) .build())) .build())); @@ -258,55 +261,56 @@ public void makeBidsShouldSetAdmFromVastXmlIsPresentAndVideoType() throws JsonPr assertThat(result.getValue()).containsOnly(BidderBid.of(expectedBid, video, "USD")); } - @Test - public void makeBidsShouldSetAdmFromVastXmlAndNurlFromVastUrlAndVideoType() throws JsonProcessingException { - // given - final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); - final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); - - final String adm = "ADM"; - final String nurl = "NURL"; - final GammaBid bid = GammaBid.builder().id("impId").vastXml(adm).vastUrl(nurl).build(); - final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( - BidResponse.builder() - .id("impId") - .cur("USD") - .seatbid(singletonList(SeatBid.builder() - .bid(singletonList(bid)) - .build())) - .build())); - - // when - final Result> result = gammaBidder.makeBids(httpCall, bidRequest); - - // then - assertThat(result.getErrors()).isEmpty(); - - final Bid expectedBid = Bid.builder().id("impId").adm(adm).nurl(nurl).build(); - assertThat(result.getValue()).containsOnly(BidderBid.of(expectedBid, video, "USD")); - } - - @Test - public void makeBidsShouldReturnErrorWhenNoVastXmlAndVideoType() throws JsonProcessingException { - // given - final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); - final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); - - final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( - BidResponse.builder() - .id("impId") - .seatbid(singletonList(SeatBid.builder() - .bid(singletonList(Bid.builder().build())) - .build())) - .build())); - - // when - final Result> result = gammaBidder.makeBids(httpCall, bidRequest); - - // then - assertThat(result.getErrors()).containsOnly(BidderError.badServerResponse( - "Missing Ad Markup. Run with request.debug = 1 for more info")); - } + // @Test + // public void makeBidsShouldSetAdmFromVastXmlAndNurlFromVastUrlAndVideoType() throws JsonProcessingException { + // // given + // final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); + // final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); + // + // final String adm = "ADM"; + // final String nurl = "NURL"; + // final Bid bid = Bid.builder().id("impId").build(); + // final GammaBid gammaBid = GammaBid.of(bid, adm, nurl); + // final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( + // GammaBidResponse.builder() + // .id("impId") + // .cur("USD") + // .seatbid(singletonList(GammaSeatBid.builder() + // .bid(singletonList(gammaBid)) + // .build())) + // .build())); + // + // // when + // final Result> result = gammaBidder.makeBids(httpCall, bidRequest); + // + // // then + // assertThat(result.getErrors()).isEmpty(); + // + // final Bid expectedBid = Bid.builder().id("impId").adm(adm).nurl(nurl).build(); + // assertThat(result.getValue()).containsOnly(BidderBid.of(expectedBid, video, "USD")); + // } + + // @Test + // public void makeBidsShouldReturnErrorWhenNoVastXmlAndVideoType() throws JsonProcessingException { + // // given + // final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); + // final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); + // + // final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( + // BidResponse.builder() + // .id("impId") + // .seatbid(singletonList(SeatBid.builder() + // .bid(singletonList(Bid.builder().build())) + // .build())) + // .build())); + // + // // when + // final Result> result = gammaBidder.makeBids(httpCall, bidRequest); + // + // // then + // assertThat(result.getErrors()).containsOnly(BidderError.badServerResponse( + // "Missing Ad Markup. Run with request.debug = 1 for more info")); + // } @Test public void makeBidsShouldReturnErrorWhenNoAdmAndNotVideoType() throws JsonProcessingException { @@ -350,7 +354,7 @@ public void makeBidsShouldReturnBannerWhenNoProvided() throws JsonProcessingExce // then assertThat(result.getErrors()).isEmpty(); - final GammaBid expectedBid = GammaBid.builder().adm("ADM").build(); + final Bid expectedBid = Bid.builder().adm("ADM").build(); assertThat(result.getValue()) .containsOnly(BidderBid.of(expectedBid, banner, "USD")); } From ba3a16c955d494778b058c28bc304ae6db2756c4 Mon Sep 17 00:00:00 2001 From: DGarbar Date: Mon, 26 Apr 2021 15:28:43 +0300 Subject: [PATCH 2/2] Fix tests --- .../server/bidder/gamma/GammaBidderTest.java | 100 +++++++++--------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java b/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java index c77413323f4..d6069fcd6fe 100644 --- a/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/gamma/GammaBidderTest.java @@ -261,56 +261,56 @@ public void makeBidsShouldSetAdmFromVastXmlIsPresentAndVideoType() throws JsonPr assertThat(result.getValue()).containsOnly(BidderBid.of(expectedBid, video, "USD")); } - // @Test - // public void makeBidsShouldSetAdmFromVastXmlAndNurlFromVastUrlAndVideoType() throws JsonProcessingException { - // // given - // final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); - // final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); - // - // final String adm = "ADM"; - // final String nurl = "NURL"; - // final Bid bid = Bid.builder().id("impId").build(); - // final GammaBid gammaBid = GammaBid.of(bid, adm, nurl); - // final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( - // GammaBidResponse.builder() - // .id("impId") - // .cur("USD") - // .seatbid(singletonList(GammaSeatBid.builder() - // .bid(singletonList(gammaBid)) - // .build())) - // .build())); - // - // // when - // final Result> result = gammaBidder.makeBids(httpCall, bidRequest); - // - // // then - // assertThat(result.getErrors()).isEmpty(); - // - // final Bid expectedBid = Bid.builder().id("impId").adm(adm).nurl(nurl).build(); - // assertThat(result.getValue()).containsOnly(BidderBid.of(expectedBid, video, "USD")); - // } - - // @Test - // public void makeBidsShouldReturnErrorWhenNoVastXmlAndVideoType() throws JsonProcessingException { - // // given - // final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); - // final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); - // - // final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( - // BidResponse.builder() - // .id("impId") - // .seatbid(singletonList(SeatBid.builder() - // .bid(singletonList(Bid.builder().build())) - // .build())) - // .build())); - // - // // when - // final Result> result = gammaBidder.makeBids(httpCall, bidRequest); - // - // // then - // assertThat(result.getErrors()).containsOnly(BidderError.badServerResponse( - // "Missing Ad Markup. Run with request.debug = 1 for more info")); - // } + @Test + public void makeBidsShouldSetAdmFromVastXmlAndNurlFromVastUrlAndVideoType() throws JsonProcessingException { + // given + final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); + final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); + + final String adm = "ADM"; + final String nurl = "NURL"; + final Bid bid = Bid.builder().id("impId").build(); + final GammaBid gammaBid = GammaBid.builder().bid(bid).vastXml(adm).vastUrl(nurl).build(); + final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( + GammaBidResponse.builder() + .id("impId") + .cur("USD") + .seatbid(singletonList(GammaSeatBid.builder() + .bid(singletonList(gammaBid)) + .build())) + .build())); + + // when + final Result> result = gammaBidder.makeBids(httpCall, bidRequest); + + // then + assertThat(result.getErrors()).isEmpty(); + + final Bid expectedBid = Bid.builder().id("impId").adm(adm).nurl(nurl).build(); + assertThat(result.getValue()).containsOnly(BidderBid.of(expectedBid, video, "USD")); + } + + @Test + public void makeBidsShouldReturnErrorWhenNoVastXmlAndVideoType() throws JsonProcessingException { + // given + final Imp imp = Imp.builder().id("impId").video(Video.builder().build()).build(); + final BidRequest bidRequest = BidRequest.builder().imp(singletonList(imp)).build(); + + final HttpCall httpCall = givenHttpCall(mapper.writeValueAsString( + BidResponse.builder() + .id("impId") + .seatbid(singletonList(SeatBid.builder() + .bid(singletonList(Bid.builder().build())) + .build())) + .build())); + + // when + final Result> result = gammaBidder.makeBids(httpCall, bidRequest); + + // then + assertThat(result.getErrors()).containsOnly(BidderError.badServerResponse( + "Missing Ad Markup. Run with request.debug = 1 for more info")); + } @Test public void makeBidsShouldReturnErrorWhenNoAdmAndNotVideoType() throws JsonProcessingException {