From 61ad9405254564a563b88e30682e67deb87ae10f Mon Sep 17 00:00:00 2001 From: Alex Maltsev Date: Thu, 5 Aug 2021 16:30:39 +0300 Subject: [PATCH] Appnexus: make AdPodId optional (#1397) --- .../bidder/appnexus/AppnexusBidder.java | 117 +++++++++--------- ...emberId.java => ImpWithExtProperties.java} | 4 +- .../ext/request/appnexus/ExtImpAppnexus.java | 2 + .../static/bidder-params/appnexus.json | 4 + .../bidder/appnexus/AppnexusBidderTest.java | 96 +++++++++++--- .../test-video-appnexus-bid-request-1.json | 92 ++++++++++++++ .../test-video-appnexus-bid-request-2.json | 92 -------------- 7 files changed, 242 insertions(+), 165 deletions(-) rename src/main/java/org/prebid/server/bidder/appnexus/model/{ImpWithMemberId.java => ImpWithExtProperties.java} (77%) 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 701c36a14e6..883cbc29468 100644 --- a/src/main/java/org/prebid/server/bidder/appnexus/AppnexusBidder.java +++ b/src/main/java/org/prebid/server/bidder/appnexus/AppnexusBidder.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.collect.Lists; import com.iab.openrtb.request.Banner; import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; @@ -11,10 +12,12 @@ import com.iab.openrtb.response.SeatBid; import io.vertx.core.http.HttpMethod; import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.prebid.server.auction.model.Endpoint; import org.prebid.server.bidder.Bidder; -import org.prebid.server.bidder.appnexus.model.ImpWithMemberId; +import org.prebid.server.bidder.appnexus.model.ImpWithExtProperties; import org.prebid.server.bidder.appnexus.proto.AppnexusBidExt; import org.prebid.server.bidder.appnexus.proto.AppnexusBidExtAppnexus; import org.prebid.server.bidder.appnexus.proto.AppnexusImpExt; @@ -182,47 +185,33 @@ public AppnexusBidder(String endpointUrl, JacksonMapper mapper) { public Result>> makeHttpRequests(BidRequest bidRequest) { final List errors = new ArrayList<>(); final String defaultDisplayManagerVer = makeDefaultDisplayManagerVer(bidRequest); - final List processedImps = new ArrayList<>(); - final Set memberIds = new HashSet<>(); + final Set uniqueIds = new HashSet<>(); + Boolean generateAdPodId = null; + for (final Imp imp : bidRequest.getImp()) { try { - final ImpWithMemberId impWithMemberId = makeImpWithMemberId(imp, defaultDisplayManagerVer); - processedImps.add(impWithMemberId.getImp()); - memberIds.add(impWithMemberId.getMemberId()); - } catch (PreBidException e) { - errors.add(BidderError.badInput(e.getMessage())); - } - } + final ImpWithExtProperties impWithExtProperties = processImp(imp, defaultDisplayManagerVer); + final Boolean impGenerateAdPodId = impWithExtProperties.getGenerateAdPodId(); - final Set uniqueIds = memberIds.stream() - .filter(Objects::nonNull) - .collect(Collectors.toSet()); + generateAdPodId = ObjectUtils.defaultIfNull(generateAdPodId, impGenerateAdPodId); + if (!Objects.equals(generateAdPodId, impGenerateAdPodId)) { + return Result.withError(BidderError.badInput( + "Generate ad pod option should be same for all pods in request")); + } - final String url; - if (CollectionUtils.isNotEmpty(uniqueIds)) { - url = String.format("%s?member_id=%s", endpointUrl, uniqueIds.iterator().next()); - try { - validateMemberId(uniqueIds); + processedImps.add(impWithExtProperties.getImp()); + final String memberId = impWithExtProperties.getMemberId(); + if (memberId != null) { + uniqueIds.add(memberId); + } } catch (PreBidException e) { errors.add(BidderError.badInput(e.getMessage())); } - } else { - url = endpointUrl; } - final List> httpRequests; - if (isVideoRequest(bidRequest)) { - httpRequests = groupImpsByPod(processedImps) - .values().stream() - .map(podImps -> splitHttpRequests(bidRequest, updateRequestExtForVideo(bidRequest), podImps, url)) - .flatMap(Collection::stream) - .collect(Collectors.toList()); - } else { - httpRequests = splitHttpRequests(bidRequest, updateRequestExt(bidRequest), processedImps, url); - } - - return Result.of(httpRequests, errors); + final String url = constructUrl(uniqueIds, errors); + return Result.of(constructRequests(bidRequest, processedImps, url, generateAdPodId), errors); } private String makeDefaultDisplayManagerVer(BidRequest bidRequest) { @@ -290,6 +279,34 @@ private static boolean isIncludeBrandCategory(ExtRequest extRequest) { return includebrandcategory != null; } + private String constructUrl(Set ids, List errors) { + if (CollectionUtils.isNotEmpty(ids)) { + final String url = String.format("%s?member_id=%s", endpointUrl, ids.iterator().next()); + try { + validateMemberId(ids); + } catch (PreBidException e) { + errors.add(BidderError.badInput(e.getMessage())); + } + return url; + } + return endpointUrl; + } + + private List> constructRequests(BidRequest bidRequest, + List imps, + String url, + Boolean generateAdPodId) { + if (isVideoRequest(bidRequest) && BooleanUtils.isTrue(generateAdPodId)) { + return groupImpsByPod(imps) + .values().stream() + .map(podImps -> splitHttpRequests(bidRequest, updateRequestExtForVideo(bidRequest), podImps, url)) + .flatMap(Collection::stream) + .collect(Collectors.toList()); + } else { + return splitHttpRequests(bidRequest, updateRequestExt(bidRequest), imps, url); + } + } + private Map> groupImpsByPod(List processedImps) { return processedImps.stream() .collect(Collectors.groupingBy(imp -> StringUtils.substringBefore(imp.getId(), POD_SEPARATOR))); @@ -297,37 +314,22 @@ private Map> groupImpsByPod(List processedImps) { private List> splitHttpRequests(BidRequest bidRequest, ExtRequest requestExt, - List processedImps, + List imps, String url) { + final List> result = Lists.partition(imps, MAX_IMP_PER_REQUEST) + .stream() + .map(impsChunk -> createHttpRequest(bidRequest, requestExt, impsChunk, url)) + .collect(Collectors.toList()); - // Let's say there are 35 impressions and limit impressions per request equals to 10. - // In this case we need to create 4 requests with 10, 10, 10 and 5 impressions. - // With this formula initial capacity=(35+10-1)/10 = 4 - final int impSize = processedImps.size(); - final int numberOfRequests = (impSize + MAX_IMP_PER_REQUEST - 1) / MAX_IMP_PER_REQUEST; - final List> spitedRequests = new ArrayList<>(numberOfRequests); - - int startIndex = 0; - boolean impsLeft = true; - while (impsLeft) { - int endIndex = startIndex + MAX_IMP_PER_REQUEST; - if (endIndex >= impSize) { - impsLeft = false; - endIndex = impSize; - } - spitedRequests.add( - createHttpRequest(bidRequest, requestExt, processedImps.subList(startIndex, endIndex), url)); - startIndex = endIndex; - } - - return spitedRequests; + return result.isEmpty() + ? Collections.singletonList(createHttpRequest(bidRequest, requestExt, imps, url)) + : result; } private HttpRequest createHttpRequest(BidRequest bidRequest, ExtRequest requestExt, List imps, String url) { - final BidRequest outgoingRequest = bidRequest.toBuilder() .imp(imps) .ext(requestExt) @@ -342,7 +344,7 @@ private HttpRequest createHttpRequest(BidRequest bidRequest, .build(); } - private ImpWithMemberId makeImpWithMemberId(Imp imp, String defaultDisplayManagerVer) { + private ImpWithExtProperties processImp(Imp imp, String defaultDisplayManagerVer) { if (imp.getAudio() != null) { throw new PreBidException( String.format("Appnexus doesn't support audio Imps. Ignoring Imp ID=%s", imp.getId())); @@ -369,7 +371,8 @@ private ImpWithMemberId makeImpWithMemberId(Imp imp, String defaultDisplayManage impBuilder.displaymanagerver(defaultDisplayManagerVer); } - return ImpWithMemberId.of(impBuilder.build(), appnexusExt.getMember()); + return ImpWithExtProperties.of(impBuilder.build(), appnexusExt.getMember(), + appnexusExt.getGenerateAdPodId()); } private static boolean bidFloorIsValid(BigDecimal bidFloor) { diff --git a/src/main/java/org/prebid/server/bidder/appnexus/model/ImpWithMemberId.java b/src/main/java/org/prebid/server/bidder/appnexus/model/ImpWithExtProperties.java similarity index 77% rename from src/main/java/org/prebid/server/bidder/appnexus/model/ImpWithMemberId.java rename to src/main/java/org/prebid/server/bidder/appnexus/model/ImpWithExtProperties.java index 51cd7517160..defe2de2b4a 100644 --- a/src/main/java/org/prebid/server/bidder/appnexus/model/ImpWithMemberId.java +++ b/src/main/java/org/prebid/server/bidder/appnexus/model/ImpWithExtProperties.java @@ -6,9 +6,11 @@ @AllArgsConstructor(staticName = "of") @Value -public class ImpWithMemberId { +public class ImpWithExtProperties { Imp imp; String memberId; + + Boolean generateAdPodId; } diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/request/appnexus/ExtImpAppnexus.java b/src/main/java/org/prebid/server/proto/openrtb/ext/request/appnexus/ExtImpAppnexus.java index 4f2e68796ae..fd6a8e26b3d 100644 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/request/appnexus/ExtImpAppnexus.java +++ b/src/main/java/org/prebid/server/proto/openrtb/ext/request/appnexus/ExtImpAppnexus.java @@ -41,5 +41,7 @@ public class ExtImpAppnexus { Boolean usePmtRule; + Boolean generateAdPodId; + ObjectNode privateSizes; // At this time we do no processing on the private sizes, so just leaving it as a JSON blob } diff --git a/src/main/resources/static/bidder-params/appnexus.json b/src/main/resources/static/bidder-params/appnexus.json index 78701b95b48..bfcb275b644 100644 --- a/src/main/resources/static/bidder-params/appnexus.json +++ b/src/main/resources/static/bidder-params/appnexus.json @@ -71,6 +71,10 @@ "type": "boolean", "description": "Boolean to signal AppNexus to apply the relevant payment rule" }, + "generate_ad_pod_id": { + "type": "boolean", + "description": "Boolean to signal AppNexus to add ad pod id to each request" + }, "private_sizes": { "type": "array", "items": { diff --git a/src/test/java/org/prebid/server/bidder/appnexus/AppnexusBidderTest.java b/src/test/java/org/prebid/server/bidder/appnexus/AppnexusBidderTest.java index a84cbd13274..53217e24c59 100644 --- a/src/test/java/org/prebid/server/bidder/appnexus/AppnexusBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/appnexus/AppnexusBidderTest.java @@ -55,6 +55,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.function.UnaryOperator; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -790,7 +791,7 @@ public void makeHttpRequestShouldReturnSingleRequestWhenOnePod() { imp -> imp .id(String.format("1_%d", impIdSuffix)) .banner(Banner.builder().build()), - ext -> ext.placementId(10))) + ext -> ext.placementId(10).generateAdPodId(true))) .collect(Collectors.toList()); final BidRequest bidRequest = BidRequest.builder() .imp(imps) @@ -820,7 +821,7 @@ public void makeHttpRequestShouldReturnMultipleRequestsWhenOnePodAndManyImps() { imp -> imp .id(String.format("1_%d", impIdSuffix)) .banner(Banner.builder().build()), - ext -> ext.placementId(10))) + ext -> ext.placementId(10).generateAdPodId(true))) .collect(Collectors.toList()); final BidRequest bidRequest = BidRequest.builder() .imp(imps) @@ -852,7 +853,7 @@ public void makeHttpRequestShouldReturnMultipleRequestsWhenTwoPods() { imp -> imp .id(String.format("%d_%d", impIdPrefix, impIdSuffix)) .banner(Banner.builder().build()), - ext -> ext.placementId(10)))) + ext -> ext.placementId(10).generateAdPodId(true)))) .collect(Collectors.toList()); final BidRequest bidRequest = BidRequest.builder() .imp(imps) @@ -884,7 +885,7 @@ public void makeHttpRequestShouldReturnMultipleRequestsWhenTwoPodsAndManyImps() imp -> imp .id(String.format("%d_%d", impIdPrefix, impIdSuffix)) .banner(Banner.builder().build()), - ext -> ext.placementId(10)))) + ext -> ext.placementId(10).generateAdPodId(true)))) .collect(Collectors.toList()); final BidRequest bidRequest = BidRequest.builder() .imp(imps) @@ -906,6 +907,70 @@ public void makeHttpRequestShouldReturnMultipleRequestsWhenTwoPodsAndManyImps() .matches(adPodIds -> new HashSet<>(adPodIds).size() == 2); // adPodIds should be different } + @Test + public void makeHttpRequestShouldReturnErrorIfRequestContainsMultipleGenerateAdPodIdsValues() { + // given + final List imps = IntStream.rangeClosed(1, 2) + .boxed() + .flatMap(impIdPrefix -> IntStream.rangeClosed(0, 15) + .mapToObj(impIdSuffix -> givenImp( + imp -> imp + .id(String.format("%d_%d", impIdPrefix, impIdSuffix)) + .banner(Banner.builder().build()), + ext -> ext.placementId(10).generateAdPodId(impIdSuffix % 2 == 0)))) + .collect(Collectors.toList()); + final BidRequest bidRequest = BidRequest.builder() + .imp(imps) + .ext(ExtRequest.of(ExtRequestPrebid.builder() + .pbs(ExtRequestPrebidPbs.of(Endpoint.openrtb2_video.value())) + .build())) + .build(); + + // when + final Result>> result = appnexusBidder.makeHttpRequests(bidRequest); + + // then + assertThat(result.getValue()).isEmpty(); + assertThat(result.getErrors()) + .containsExactly(BidderError.badInput("Generate ad pod option should be same for all pods in request")); + } + + @Test + public void makeHttpRequestShouldNotGenerateAdPodIdWhenFlagIsNotSetInRequestImpExt() { + // given + final List imps = IntStream.rangeClosed(1, 2) + .boxed() + .flatMap(impIdPrefix -> IntStream.rangeClosed(0, 15) + .mapToObj(impIdSuffix -> givenImp( + imp -> imp + .id(String.format("%d_%d", impIdPrefix, impIdSuffix)) + .banner(Banner.builder().build()), + ext -> ext.placementId(10).generateAdPodId(false)))) + .collect(Collectors.toList()); + final BidRequest bidRequest = BidRequest.builder() + .imp(imps) + .ext(ExtRequest.of(ExtRequestPrebid.builder() + .targeting(ExtRequestTargeting.builder() + .includebrandcategory(ExtIncludeBrandCategory.of(null, null, null)) + .build()) + .pbs(ExtRequestPrebidPbs.of(Endpoint.openrtb2_video.value())) + .build())) + .build(); + + // when + final Result>> result = appnexusBidder.makeHttpRequests(bidRequest); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()) + .extracting(HttpRequest::getPayload) + .extracting(BidRequest::getExt) + .extracting(ext -> mapper.convertValue(ext.getProperties(), AppnexusReqExt.class)) + .extracting(AppnexusReqExt::getAppnexus).doesNotContainNull() + .extracting(AppnexusReqExtAppnexus::getAdpodId) + .matches(adPodIds -> adPodIds.stream().allMatch(Objects::isNull)); + } + @Test public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() { // given @@ -1139,7 +1204,7 @@ private static BidRequest givenBidRequest(UnaryOperator bidRe UnaryOperator impCustomizer, UnaryOperator extCustomizer) { return bidRequestCustomizer.apply(BidRequest.builder() - .imp(singletonList(givenImp(impCustomizer, extCustomizer)))) + .imp(singletonList(givenImp(impCustomizer, extCustomizer)))) .build(); } @@ -1150,7 +1215,7 @@ private static BidRequest givenBidRequest(UnaryOperator impCustomize private static Imp givenImp(UnaryOperator impCustomizer, UnaryOperator extCustomizer) { return impCustomizer.apply(Imp.builder() - .ext(givenExt(extCustomizer))) + .ext(givenExt(extCustomizer))) .build(); } @@ -1184,15 +1249,16 @@ private static String givenBidResponse( UnaryOperator bidExtCustomizer) throws JsonProcessingException { - return mapper.writeValueAsString(bidResponseCustomizer.apply(BidResponse.builder() - .seatbid(singletonList(SeatBid.builder() - .bid(singletonList(bidCustomizer.apply(Bid.builder() - .impid("impId") - .ext(mapper.valueToTree(AppnexusBidExt.of( - bidExtCustomizer.apply(AppnexusBidExtAppnexus.builder().bidAdType(BANNER_TYPE)) - .build())))) - .build())) - .build()))) + return mapper.writeValueAsString(bidResponseCustomizer.apply( + BidResponse.builder() + .seatbid(singletonList(SeatBid.builder() + .bid(singletonList(bidCustomizer.apply(Bid.builder() + .impid("impId") + .ext(mapper.valueToTree(AppnexusBidExt.of(bidExtCustomizer.apply( + AppnexusBidExtAppnexus.builder() + .bidAdType(BANNER_TYPE)) + .build())))).build())) + .build()))) .build()); } diff --git a/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-1.json b/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-1.json index 55e1ff8c6be..8c9fec28d83 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-1.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-1.json @@ -138,6 +138,98 @@ "placement_id": 14997137 } } + }, + { + "id": "2_0", + "video": { + "mimes": [ + "video/mp4" + ], + "minduration": 30, + "maxduration": 30, + "protocols": [ + 2, + 3, + 5, + 6 + ], + "w": 640, + "h": 480 + }, + "ext": { + "appnexus": { + "placement_id": 15016213 + } + } + }, + { + "id": "2_1", + "video": { + "mimes": [ + "video/mp4" + ], + "minduration": 30, + "maxduration": 30, + "protocols": [ + 2, + 3, + 5, + 6 + ], + "w": 640, + "h": 480 + }, + "ext": { + "appnexus": { + "placement_id": 15016213 + } + } + }, + { + "id": "2_2", + "video": { + "mimes": [ + "video/mp4" + ], + "minduration": 30, + "maxduration": 30, + "protocols": [ + 2, + 3, + 5, + 6 + ], + "w": 640, + "h": 480 + }, + "ext": { + "appnexus": { + "placement_id": 15016213 + } + } + }, + { + "id": "2_3", + "video": { + "mimes": [ + "video/mp4" + ], + "minduration": 30, + "maxduration": 30, + "protocols": [ + 2, + 3, + 5, + 6 + ], + "w": 640, + "h": 480 + }, + "ext": { + "appnexus": { + "placement_id": 15016213 + } + } } ], "site": { diff --git a/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-2.json b/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-2.json index d403a3e5cc2..67eae45f9ee 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-2.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/video/test-video-appnexus-bid-request-2.json @@ -1,98 +1,6 @@ { "id": "bid_id", "imp": [ - { - "id": "2_0", - "video": { - "mimes": [ - "video/mp4" - ], - "minduration": 30, - "maxduration": 30, - "protocols": [ - 2, - 3, - 5, - 6 - ], - "w": 640, - "h": 480 - }, - "ext": { - "appnexus": { - "placement_id": 15016213 - } - } - }, - { - "id": "2_1", - "video": { - "mimes": [ - "video/mp4" - ], - "minduration": 30, - "maxduration": 30, - "protocols": [ - 2, - 3, - 5, - 6 - ], - "w": 640, - "h": 480 - }, - "ext": { - "appnexus": { - "placement_id": 15016213 - } - } - }, - { - "id": "2_2", - "video": { - "mimes": [ - "video/mp4" - ], - "minduration": 30, - "maxduration": 30, - "protocols": [ - 2, - 3, - 5, - 6 - ], - "w": 640, - "h": 480 - }, - "ext": { - "appnexus": { - "placement_id": 15016213 - } - } - }, - { - "id": "2_3", - "video": { - "mimes": [ - "video/mp4" - ], - "minduration": 30, - "maxduration": 30, - "protocols": [ - 2, - 3, - 5, - 6 - ], - "w": 640, - "h": 480 - }, - "ext": { - "appnexus": { - "placement_id": 15016213 - } - } - }, { "id": "2_4", "video": {