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 4a7a5cc5f0e..c44d6c58788 100644 --- a/src/main/java/org/prebid/server/bidder/rubicon/RubiconBidder.java +++ b/src/main/java/org/prebid/server/bidder/rubicon/RubiconBidder.java @@ -29,7 +29,6 @@ import io.vertx.core.MultiMap; import io.vertx.core.http.HttpMethod; import org.apache.commons.collections4.CollectionUtils; -import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.math.NumberUtils; @@ -105,19 +104,16 @@ import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebidMeta; import org.prebid.server.util.BidderUtil; import org.prebid.server.util.HttpUtil; -import org.prebid.server.util.ListUtil; import org.prebid.server.util.ObjectUtil; import org.prebid.server.version.PrebidVersionProvider; import java.math.BigDecimal; import java.net.URISyntaxException; -import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; import java.util.Collection; import java.util.Collections; -import java.util.Deque; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -130,6 +126,7 @@ import java.util.UUID; import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import java.util.stream.StreamSupport; public class RubiconBidder implements Bidder { @@ -804,10 +801,6 @@ private static String getTextValueFromNodeByPath(JsonNode node, String path) { return nodeByPath != null && nodeByPath.isTextual() ? nodeByPath.textValue() : null; } - private static String getTextValueFromNode(JsonNode node) { - return node != null && node.isTextual() ? node.textValue() : null; - } - private void populateFirstPartyDataAttributes(ObjectNode sourceNode, ObjectNode targetNode) { if (sourceNode == null || sourceNode.isNull()) { return; @@ -1249,97 +1242,40 @@ private void mergeFirstPartyDataFromUser(ExtUser userExt, ObjectNode result) { } } - private static void enrichWithIabAndSegtaxAttribute(ObjectNode target, List data, Set segtaxValues) { - final Map> segments = CollectionUtils.emptyIfNull(data).stream() + private static void enrichWithIabAndSegtaxAttribute(ObjectNode target, List data, Set iabTaxes) { + CollectionUtils.emptyIfNull(data).stream() .filter(Objects::nonNull) - .map(RubiconBidder::getValidSegments) - .filter(Objects::nonNull) - .collect(Collectors.toMap( - Map.Entry::getKey, - Map.Entry::getValue, - (first, second) -> { - first.addAll(second); - return first; - })); - - if (MapUtils.isEmpty(segments)) { - return; - } - - final Map> relevantSegments = pickRelevantSegments(segments); - final Map> resultSegments = groupBySegtaxValues(relevantSegments, segtaxValues); - - resultSegments.forEach((segtaxValue, segmentIds) -> { - final ArrayNode array = target.putArray(segtaxValue); - segmentIds.forEach(array::add); - }); + .flatMap(dataEntry -> extractTaxToSegmentId(dataEntry, iabTaxes)) + .limit(MAX_NUMBER_OF_SEGMENTS) + .forEach(entry -> getArrayNodeOrCreate(target, entry.getKey()).add(entry.getValue())); } - private static Map> groupBySegtaxValues(Map> segments, - Set segtaxValues) { - - return segments.entrySet().stream() - .collect(Collectors.toMap( - entry -> resolveSegmentName(entry.getKey(), segtaxValues), - Map.Entry::getValue, - ListUtil::union)); - } - - private static String resolveSegmentName(Integer taxonomyId, Set segtaxValues) { - return segtaxValues.contains(taxonomyId) ? SEGTAX_IAB : SEGTAX_TAX + taxonomyId; - } - - private static Map.Entry> getValidSegments(Data data) { + private static Stream> extractTaxToSegmentId(Data data, Set iabTaxes) { final ObjectNode ext = data.getExt(); final JsonNode taxonomyId = ext != null ? ext.get(SEGTAX) : null; if (taxonomyId == null || !taxonomyId.isInt()) { - return null; + return Stream.empty(); } - final Deque segments = getValidOnlySegments(data.getSegment()); - return CollectionUtils.isNotEmpty(segments) ? Map.entry(taxonomyId.intValue(), segments) : null; + final String taxKey = resolveTaxName(taxonomyId.intValue(), iabTaxes); + return CollectionUtils.emptyIfNull(data.getSegment()).stream() + .filter(Objects::nonNull) + .map(Segment::getId) + .filter(StringUtils::isNotBlank) + .map(id -> Map.entry(taxKey, id)); } - private static Deque getValidOnlySegments(List segments) { - return CollectionUtils.isNotEmpty(segments) - ? segments.stream() - .filter(segment -> StringUtils.isNotBlank(segment.getId())) - .collect(Collectors.toCollection(ArrayDeque::new)) - : null; + private static String resolveTaxName(Integer taxonomyId, Set iabTaxes) { + return iabTaxes.contains(taxonomyId) ? SEGTAX_IAB : SEGTAX_TAX + taxonomyId; } - private static Map> pickRelevantSegments(final Map> segments) { - final Map> result = new HashMap<>(); - final List segmentsKeys = new ArrayList<>(segments.keySet()); - - int i = 0; - int consumedSegmentsCount = 0; - - while (consumedSegmentsCount < MAX_NUMBER_OF_SEGMENTS && !segmentsKeys.isEmpty()) { - final int segmentsIndex = i % segmentsKeys.size(); - final Integer segmentKey = segmentsKeys.get(segmentsIndex); - final Deque currentSegments = segments.get(segmentKey); - - final Segment lastSegment = currentSegments.pollLast(); - result.computeIfAbsent(segmentKey, key -> new ArrayList<>()).add(lastSegment.getId()); - consumedSegmentsCount++; - - if (currentSegments.isEmpty()) { - segmentsKeys.remove(segmentKey); - i--; - } - i++; + private static ArrayNode getArrayNodeOrCreate(ObjectNode parent, String field) { + final JsonNode node = parent.get(field); + if (node == null || !node.isArray()) { + return parent.putArray(field); } - return result; - } - - private static Segment getAndRemoveLastSegment(List list) { - final int lastElementIndex = list.size() - 1; - final Segment lastSegment = list.get(lastElementIndex); - list.remove(lastElementIndex); - - return lastSegment; + return (ArrayNode) node; } private void processWarnings(List errors, List priceFloorsWarnings) { diff --git a/src/test/java/org/prebid/server/bidder/rubicon/RubiconBidderTest.java b/src/test/java/org/prebid/server/bidder/rubicon/RubiconBidderTest.java index fce71c31997..4a01377ee4a 100644 --- a/src/test/java/org/prebid/server/bidder/rubicon/RubiconBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/rubicon/RubiconBidderTest.java @@ -1106,34 +1106,6 @@ public void makeHttpRequestsShouldFillUserExtRpWithIabAttributeIfSegtaxEqualsFou .build())); } - @Test - public void makeHttpRequestsShouldFillUserExtRpWithSegtaxValuesWithSegtaxesFromEachData() { - // given - final List dataWithSegments = asList(givenTestDataWithSegmentEntries(3), - givenDataWithSegmentEntry(3, "Included_SegmentId_1"), - givenDataWithSegmentEntry(3, "Included_SegmentId_2")); - - final BidRequest bidRequest = givenBidRequest( - builder -> builder.user(User.builder().data(dataWithSegments).build()), - builder -> builder.video(Video.builder().build()), - identity()); - - // when - final Result>> result = target.makeHttpRequests(bidRequest); - - // then - assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()).hasSize(1).doesNotContainNull() - .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) - .extracting(BidRequest::getUser).doesNotContainNull() - .extracting(User::getExt) - .extracting(userExt -> userExt.getProperty("rp")) - .extracting(rp -> rp.get("target")) - .extracting(target -> target.get("tax3")) - .flatExtracting(tax3 -> mapper.convertValue(tax3, List.class)) - .contains("Included_SegmentId_1", "Included_SegmentId_2"); - } - @Test public void makeHttpRequestsShouldRemoveUserDataObject() { // given @@ -1157,7 +1129,7 @@ public void makeHttpRequestsShouldRemoveUserDataObject() { } @Test - public void makeHttpRequestsShouldFillSiteExtRpWithSegtaxValuesWithNoMoreThanHundredEntriesWithEvenDistribution() + public void makeHttpRequestsShouldFillSiteExtRpWithSegtaxValuesWithNoMoreThanHundredEntriesFromDifferentSources() throws IOException { // given @@ -1186,16 +1158,16 @@ public void makeHttpRequestsShouldFillSiteExtRpWithSegtaxValuesWithNoMoreThanHun final BidRequest capturedBidRequest = mapper.readValue(result.getValue().get(0).getBody(), BidRequest.class); final JsonNode targetNode = capturedBidRequest.getSite().getExt().getProperty("rp").get("target"); - assertThat(targetNode.elements()).toIterable().hasSize(4); + assertThat(targetNode.elements()).toIterable().hasSize(3); final List expectedIabValues = Streams.concat( IntStream.range(1, 6).mapToObj(i -> "firstSegmentId_" + i), IntStream.range(1, 5).mapToObj(i -> "secondSegmentId_" + i), IntStream.range(1, 2).mapToObj(i -> "fifthSegmentId_" + i), - IntStream.range(59, 101).mapToObj(i -> "sixthSegmentId_" + i)) + IntStream.range(1, 86).mapToObj(i -> "sixthSegmentId_" + i)) .toList(); - assertThat(targetNode.get("iab").elements()).toIterable().hasSize(52) + assertThat(targetNode.get("iab").elements()).toIterable().hasSize(95) .extracting(JsonNode::asText) .containsExactlyInAnyOrderElementsOf(expectedIabValues); @@ -1209,11 +1181,7 @@ public void makeHttpRequestsShouldFillSiteExtRpWithSegtaxValuesWithNoMoreThanHun .extracting(JsonNode::asText) .containsExactlyInAnyOrderElementsOf(expectedTax4Values); - final List expectedTax7Values = IntStream.range(58, 101).mapToObj(i -> "seventhSegmentId_" + i) - .toList(); - assertThat(targetNode.get("tax7").elements()).toIterable().hasSize(43) - .extracting(JsonNode::asText) - .containsExactlyInAnyOrderElementsOf(expectedTax7Values); + assertThat(targetNode.get("tax7")).isNull(); } @Test @@ -1249,7 +1217,7 @@ public void makeHttpRequestsShouldFillSiteExtRpWithSegtaxValuesWithNoMoreThanHun } @Test - public void makeHttpRequestsShouldFillUserExtRpWithSegtaxValuesWithNoMoreThanHundredEntriesWithEvenDistribution() + public void makeHttpRequestsShouldFillUserExtRpWithSegtaxValuesWithNoMoreThanHundredEntriesFromDifferentSources() throws IOException { // given @@ -1276,7 +1244,7 @@ public void makeHttpRequestsShouldFillUserExtRpWithSegtaxValuesWithNoMoreThanHun final BidRequest capturedBidRequest = mapper.readValue(result.getValue().get(0).getBody(), BidRequest.class); final JsonNode targetNode = capturedBidRequest.getUser().getExt().getProperty("rp").get("target"); - assertThat(targetNode.elements()).toIterable().hasSize(7); + assertThat(targetNode.elements()).toIterable().hasSize(6); final List expectedIabValues = IntStream.range(1, 3).mapToObj(i -> "fourthSegmentId_" + i).toList(); assertThat(targetNode.get("iab").elements()).toIterable() @@ -1308,18 +1276,13 @@ public void makeHttpRequestsShouldFillUserExtRpWithSegtaxValuesWithNoMoreThanHun .extracting(JsonNode::asText) .containsExactlyInAnyOrderElementsOf(expectedTax5Values); - final List expectedTax6Values = IntStream.range(59, 101).mapToObj(i -> "sixthSegmentId_" + i).toList(); + final List expectedTax6Values = IntStream.range(1, 86).mapToObj(i -> "sixthSegmentId_" + i).toList(); assertThat(targetNode.get("tax6").elements()).toIterable() - .hasSize(42) + .hasSize(85) .extracting(JsonNode::asText) .containsExactlyInAnyOrderElementsOf(expectedTax6Values); - final List expectedTax7Values = IntStream.range(58, 101).mapToObj(i -> "seventhSegmentId_" + i) - .toList(); - assertThat(targetNode.get("tax7").elements()).toIterable() - .hasSize(43) - .extracting(JsonNode::asText) - .containsExactlyInAnyOrderElementsOf(expectedTax7Values); + assertThat(targetNode.get("tax7")).isNull(); } @@ -2105,9 +2068,9 @@ public void makeHttpRequestsShouldNotCreateUserExtTpIdWithAdServerEidSourceIfExt final BidRequest bidRequest = givenBidRequest(builder -> builder.user(User.builder() .ext(ExtUser.builder() .eids(singletonList(Eid.builder() - .source("adserver.org") - .uids(singletonList(Uid.builder().id("id").build())) - .build())) + .source("adserver.org") + .uids(singletonList(Uid.builder().id("id").build())) + .build())) .build()) .build()), builder -> builder.video(Video.builder().build()), identity()); @@ -2711,7 +2674,7 @@ public void makeHttpRequestsShouldMergeImpExtRubiconAndDataKeywordsToRubiconImpE } @Test - public void makeHttpRequestsShouldCopyDataSearchToRubiconImpExtRpTargetSearch() throws IOException { + public void makeHttpRequestsShouldCopyDataSearchToRubiconImpExtRpTargetSearch() { // given final BidRequest bidRequest = givenBidRequest( identity(), @@ -2957,8 +2920,7 @@ public void makeHttpRequestsShouldProcessMetricsAndFillViewabilityVendor() { } @Test - public void makeHttpRequestsShouldReturnOnlyLineItemRequestsWithExpectedFieldsWhenImpPmpDealsArePresent() - throws IOException { + public void makeHttpRequestsShouldReturnOnlyLineItemRequestsWithExpectedFieldsWhenImpPmpDealsArePresent() { // given final List dealsList = asList( Deal.builder().ext(mapper.valueToTree(ExtDeal.of(ExtDealLine.of(null, "123", @@ -4071,16 +4033,6 @@ private static RubiconBid givenRubiconBid(UnaryOperator segments = IntStream.range(0, 1000) - .mapToObj(index -> Segment.builder().id("segmentId_" + index).build()) - .toList(); - return Data.builder() - .segment(segments) - .ext(mapper.createObjectNode().put("segtax", segtax)) - .build(); - } - private static ObjectNode givenImpExtRpTarget() { return mapper.createObjectNode() .put("pbs_login", USERNAME) diff --git a/src/test/resources/org/prebid/server/it/openrtb2/magnite/test-magnite-bid-request.json b/src/test/resources/org/prebid/server/it/openrtb2/magnite/test-magnite-bid-request.json index 4b14180a370..20d09566628 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/magnite/test-magnite-bid-request.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/magnite/test-magnite-bid-request.json @@ -54,8 +54,8 @@ "site_id": 3001, "target": { "iab": [ - "segmentId2", "segmentId1", + "segmentId2", "segmentId3" ] } diff --git a/src/test/resources/org/prebid/server/it/openrtb2/rubicon/test-rubicon-bid-request.json b/src/test/resources/org/prebid/server/it/openrtb2/rubicon/test-rubicon-bid-request.json index 4b14180a370..20d09566628 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/rubicon/test-rubicon-bid-request.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/rubicon/test-rubicon-bid-request.json @@ -54,8 +54,8 @@ "site_id": 3001, "target": { "iab": [ - "segmentId2", "segmentId1", + "segmentId2", "segmentId3" ] }