RTBHouse: refactor tag id extraction logic for simplicity and improving readability; changed order of methods

Author: pjaworski-rtbh

src/main/java/org/prebid/server/bidder/rtbhouse/RtbhouseBidder.java

@@ -94,6 +94,110 @@ public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest bidRequ
         return Result.withValue(BidderUtil.defaultRequest(outgoingRequest, endpointUrl, mapper));
     }
 
+    private ExtImpRtbhouse parseImpExt(Imp imp) {
+        try {
+            return mapper.mapper().convertValue(imp.getExt(), RTBHOUSE_EXT_TYPE_REFERENCE).getBidder();
+        } catch (IllegalArgumentException e) {
+            throw new PreBidException(e.getMessage());
+        }
+    }
+
+    private Price resolveBidFloor(Imp imp, ExtImpRtbhouse impExt, BidRequest bidRequest) {
+        final List<String> brCur = bidRequest.getCur();
+        final Price initialBidFloorPrice = Price.of(imp.getBidfloorcur(), imp.getBidfloor());
+
+        final BigDecimal impExtBidFloor = impExt.getBidFloor();
+        final String impExtCurrency = impExtBidFloor != null && brCur != null && !brCur.isEmpty()
+                ? brCur.getFirst() : null;
+        final Price impExtBidFloorPrice = Price.of(impExtCurrency, impExtBidFloor);
+        final Price resolvedPrice = initialBidFloorPrice.getValue() == null
+                ? impExtBidFloorPrice : initialBidFloorPrice;
+
+        return BidderUtil.isValidPrice(resolvedPrice)
+                && !StringUtils.equalsIgnoreCase(resolvedPrice.getCurrency(), BIDDER_CURRENCY)
+                ? convertBidFloor(resolvedPrice, imp.getId(), bidRequest)
+                : resolvedPrice;
+    }
+
+    private Price convertBidFloor(Price bidFloorPrice, String impId, BidRequest bidRequest) {
+        final String bidFloorCur = bidFloorPrice.getCurrency();
+        try {
+            final BigDecimal convertedPrice = currencyConversionService
+                    .convertCurrency(bidFloorPrice.getValue(), bidRequest, bidFloorCur, BIDDER_CURRENCY);
+
+            return Price.of(BIDDER_CURRENCY, convertedPrice);
+        } catch (PreBidException e) {
+            throw new PreBidException(String.format(
+                    "Unable to convert provided bid floor currency from %s to %s for imp `%s`",
+                    bidFloorCur, BIDDER_CURRENCY, impId));
+        }
+    }
+
+    private static Imp modifyImp(Imp imp, Price bidFloorPrice) {
+        return imp.toBuilder()
+                .tagid(extractTagId(imp))
+                .bidfloorcur(ObjectUtil.getIfNotNull(bidFloorPrice, Price::getCurrency))
+                .bidfloor(ObjectUtil.getIfNotNull(bidFloorPrice, Price::getValue))
+                .pmp(null)
+                .build();
+    }
+
+    private static String extractTagId(Imp imp) {
+        return Optional.ofNullable(imp.getTagid())
+                .filter(StringUtils::isNotBlank)
+                .or(() -> extractGpid(imp))
+                .or(() -> extractAdslot(imp))
+                .or(() -> extractPbadslot(imp))
+                .or(() -> Optional.ofNullable(imp.getId())
+                        .filter(StringUtils::isNotBlank))
+                .orElse(null);
+    }
+
+    private static Optional<String> extractGpid(Imp imp) {
+        return Optional.ofNullable(imp.getExt())
+                .map(ext -> ext.get("gpid"))
+                .map(JsonNode::textValue)
+                .filter(StringUtils::isNotBlank);
+    }
+
+    private static Optional<String> extractAdslot(Imp imp) {
+        return Optional.ofNullable(imp.getExt())
+                .map(ext -> ext.get("data"))
+                .map(data -> data.get("adserver"))
+                .map(adserver -> adserver.get("adslot"))
+                .map(JsonNode::textValue)
+                .filter(StringUtils::isNotBlank);
+    }
+
+    private static Optional<String> extractPbadslot(Imp imp) {
+        return Optional.ofNullable(imp.getExt())
+                .map(ext -> ext.get("data"))
+                .map(data -> data.get("pbadslot"))
+                .map(JsonNode::textValue)
+                .filter(StringUtils::isNotBlank);
+    }
+
+    private Site modifySite(Site site, String publisherId) {
+        final ObjectNode prebidNode = mapper.mapper().createObjectNode();
+        prebidNode.put("publisherId", publisherId);
+
+        final ExtPublisher extPublisher = ExtPublisher.empty();
+        extPublisher.addProperty("prebid", prebidNode);
+
+        final Publisher publisher = Optional.ofNullable(site)
+                .map(Site::getPublisher)
+                .map(Publisher::toBuilder)
+                .orElseGet(Publisher::builder)
+                .ext(extPublisher)
+                .build();
+
+        return Optional.ofNullable(site)
+                .map(Site::toBuilder)
+                .orElseGet(Site::builder)
+                .publisher(publisher)
+                .build();
+    }
+
     @Override
     public Result<List<BidderBid>> makeBids(BidderCall<BidRequest> httpCall, BidRequest bidRequest) {
         try {
@@ -144,23 +248,6 @@ private BidderBid resolveBidderBid(Bid bid,
                 .build();
     }
 
-    private String resolveNativeAdm(String adm, List<BidderError> bidderErrors) {
-        final JsonNode admNode;
-        try {
-            admNode = mapper.mapper().readTree(adm);
-        } catch (JsonProcessingException e) {
-            bidderErrors.add(BidderError.badServerResponse("Unable to parse native adm: %s".formatted(adm)));
-            return null;
-        }
-
-        final JsonNode nativeNode = admNode.get("native");
-        if (nativeNode != null) {
-            return nativeNode.toString();
-        }
-
-        return adm;
-    }
-
     private static BidType getBidType(String impId, List<Imp> imps) {
         for (Imp imp : imps) {
             if (imp.getId().equals(impId)) {
@@ -176,95 +263,21 @@ private static BidType getBidType(String impId, List<Imp> imps) {
         return BidType.banner;
     }
 
-    private ExtImpRtbhouse parseImpExt(Imp imp) {
+    private String resolveNativeAdm(String adm, List<BidderError> bidderErrors) {
+        final JsonNode admNode;
         try {
-            return mapper.mapper().convertValue(imp.getExt(), RTBHOUSE_EXT_TYPE_REFERENCE).getBidder();
-        } catch (IllegalArgumentException e) {
-            throw new PreBidException(e.getMessage());
+            admNode = mapper.mapper().readTree(adm);
+        } catch (JsonProcessingException e) {
+            bidderErrors.add(BidderError.badServerResponse("Unable to parse native adm: %s".formatted(adm)));
+            return null;
         }
-    }
-
-    private static Imp modifyImp(Imp imp, Price bidFloorPrice) {
-        return imp.toBuilder()
-                .tagid(extractTagId(imp))
-                .bidfloorcur(ObjectUtil.getIfNotNull(bidFloorPrice, Price::getCurrency))
-                .bidfloor(ObjectUtil.getIfNotNull(bidFloorPrice, Price::getValue))
-                .pmp(null)
-                .build();
-    }
-
-    private static String extractTagId(Imp imp) {
-        return Optional.ofNullable(imp.getTagid())
-                .filter(StringUtils::isNotBlank)
-                .orElseGet(() -> extractTagIdFromExt(imp));
-    }
-
-    private static String extractTagIdFromExt(Imp imp) {
-        return Optional.ofNullable(imp.getExt())
-                .flatMap(ext -> extractGpid(ext))
-                .orElseGet(() -> extractFromData(imp));
-    }
-
-    private static Optional<String> extractGpid(JsonNode ext) {
-        return Optional.ofNullable(ext.get("gpid"))
-                .filter(JsonNode::isTextual)
-                .map(JsonNode::asText)
-                .filter(StringUtils::isNotBlank);
-    }
-
-    private static String extractFromData(Imp imp) {
-        return Optional.ofNullable(imp.getExt())
-                .map(ext -> ext.get("data"))
-                .filter(JsonNode::isObject)
-                .flatMap(data -> extractAdslot(data).or(() -> extractPbadslot(data)))
-                .orElseGet(() -> Optional.ofNullable(imp.getId()).filter(StringUtils::isNotBlank).orElse(null));
-    }
-
-    private static Optional<String> extractAdslot(JsonNode data) {
-        return Optional.ofNullable(data.get("adserver"))
-                .filter(JsonNode::isObject)
-                .map(adserver -> adserver.get("adslot"))
-                .filter(JsonNode::isTextual)
-                .map(JsonNode::asText)
-                .filter(StringUtils::isNotBlank);
-    }
-
-    private static Optional<String> extractPbadslot(JsonNode data) {
-        return Optional.ofNullable(data.get("pbadslot"))
-                .filter(JsonNode::isTextual)
-                .map(JsonNode::asText)
-                .filter(StringUtils::isNotBlank);
-    }
-
-    private Price resolveBidFloor(Imp imp, ExtImpRtbhouse impExt, BidRequest bidRequest) {
-        final List<String> brCur = bidRequest.getCur();
-        final Price initialBidFloorPrice = Price.of(imp.getBidfloorcur(), imp.getBidfloor());
 
-        final BigDecimal impExtBidFloor = impExt.getBidFloor();
-        final String impExtCurrency = impExtBidFloor != null && brCur != null && !brCur.isEmpty()
-                ? brCur.getFirst() : null;
-        final Price impExtBidFloorPrice = Price.of(impExtCurrency, impExtBidFloor);
-        final Price resolvedPrice = initialBidFloorPrice.getValue() == null
-                ? impExtBidFloorPrice : initialBidFloorPrice;
-
-        return BidderUtil.isValidPrice(resolvedPrice)
-                && !StringUtils.equalsIgnoreCase(resolvedPrice.getCurrency(), BIDDER_CURRENCY)
-                ? convertBidFloor(resolvedPrice, imp.getId(), bidRequest)
-                : resolvedPrice;
-    }
-
-    private Price convertBidFloor(Price bidFloorPrice, String impId, BidRequest bidRequest) {
-        final String bidFloorCur = bidFloorPrice.getCurrency();
-        try {
-            final BigDecimal convertedPrice = currencyConversionService
-                    .convertCurrency(bidFloorPrice.getValue(), bidRequest, bidFloorCur, BIDDER_CURRENCY);
-
-            return Price.of(BIDDER_CURRENCY, convertedPrice);
-        } catch (PreBidException e) {
-            throw new PreBidException(String.format(
-                    "Unable to convert provided bid floor currency from %s to %s for imp `%s`",
-                    bidFloorCur, BIDDER_CURRENCY, impId));
+        final JsonNode nativeNode = admNode.get("native");
+        if (nativeNode != null) {
+            return nativeNode.toString();
         }
+
+        return adm;
     }
 
     private static Bid resolveMacros(Bid bid) {
@@ -276,25 +289,4 @@ private static Bid resolveMacros(Bid bid) {
                 .adm(StringUtils.replace(bid.getAdm(), PRICE_MACRO, priceAsString))
                 .build();
     }
-
-    private Site modifySite(Site site, String publisherId) {
-        final ObjectNode prebidNode = mapper.mapper().createObjectNode();
-        prebidNode.put("publisherId", publisherId);
-
-        final ExtPublisher extPublisher = ExtPublisher.empty();
-        extPublisher.addProperty("prebid", prebidNode);
-
-        final Publisher publisher = Optional.ofNullable(site)
-                .map(Site::getPublisher)
-                .map(Publisher::toBuilder)
-                .orElseGet(Publisher::builder)
-                .ext(extPublisher)
-                .build();
-
-        return Optional.ofNullable(site)
-                .map(Site::toBuilder)
-                .orElseGet(Site::builder)
-                .publisher(publisher)
-                .build();
-    }
 }

src/test/java/org/prebid/server/bidder/rtbhouse/RtbhouseBidderTest.java

@@ -34,7 +34,9 @@
 
 import java.math.BigDecimal;
 import java.util.List;
+import java.util.Map;
 import java.util.function.Function;
+import java.util.function.UnaryOperator;
 
 import static java.util.Collections.singletonList;
 import static java.util.function.Function.identity;
@@ -521,13 +523,10 @@ public void makeHttpRequestsShouldAlwaysRemovePmpField() {
     @Test
     public void makeHttpRequestsShouldSetTagidFromGpid() {
         // given
-        final BidRequest bidRequest = BidRequest.builder()
-                .imp(singletonList(Imp.builder()
-                        .id("imp123")
-                        .ext(givenRtbhouseExt(node -> node.put("gpid", "gpid_value")))
-                        .build()))
-                .id("request_id")
-                .build();
+        final BidRequest bidRequest = givenBidRequest(
+                bidReq -> bidReq.id("request_id"),
+                imp -> imp.id("imp123").ext(givenRtbhouseExt(node -> node.put("gpid", "gpid_value"))),
+                identity());
 
         // when
         final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
@@ -544,14 +543,13 @@ public void makeHttpRequestsShouldSetTagidFromGpid() {
     @Test
     public void makeHttpRequestsShouldSetTagidFromAdserverAdslot() {
         // given
-        final BidRequest bidRequest = BidRequest.builder()
-                .imp(singletonList(Imp.builder()
-                        .id("imp123")
-                        .ext(givenRtbhouseExt(node -> node.set("data", mapper.createObjectNode()
-                                .set("adserver", mapper.createObjectNode().put("adslot", "adslot_value")))))
-                        .build()))
-                .id("request_id")
-                .build();
+        final BidRequest bidRequest = givenBidRequest(
+                bidReq -> bidReq.id("request_id"),
+                imp -> imp.id("imp123")
+                        .ext(givenRtbhouseExt(node ->
+                                node.set("data", mapper.valueToTree(
+                                        Map.of("adserver", Map.of("adslot", "adslot_value")))))),
+                identity());
 
         // when
         final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
@@ -568,14 +566,12 @@ public void makeHttpRequestsShouldSetTagidFromAdserverAdslot() {
     @Test
     public void makeHttpRequestsShouldSetTagidFromPbadslot() {
         // given
-        final BidRequest bidRequest = BidRequest.builder()
-                .imp(singletonList(Imp.builder()
-                        .id("imp123")
-                        .ext(givenRtbhouseExt(node -> node.set("data", mapper.createObjectNode()
-                                .put("pbadslot", "pbadslot_value"))))
-                        .build()))
-                .id("request_id")
-                .build();
+        final BidRequest bidRequest = givenBidRequest(
+                bidReq -> bidReq.id("request_id"),
+                imp -> imp.id("imp123")
+                        .ext(givenRtbhouseExt(node ->
+                                node.set("data", mapper.valueToTree(Map.of("pbadslot", "pbadslot_value"))))),
+                identity());
 
         // when
         final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
@@ -612,16 +608,10 @@ public void makeHttpRequestsShouldSetTagidFromImpIdWhenNoOtherFields() {
     @Test
     public void makeHttpRequestsShouldSetTagidToNullWhenNoFieldsAvailable() {
         // given
-        final BidRequest bidRequest = BidRequest.builder()
-                .imp(singletonList(Imp.builder()
-                        .id(null)
-                        .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpRtbhouse.builder()
-                                .publisherId("publisherId")
-                                .region("region")
-                                .build())))
-                        .build()))
-                .id("request_id")
-                .build();
+        final BidRequest bidRequest = givenBidRequest(
+                bidReq -> bidReq.id("request_id"),
+                imp -> imp.id(null),
+                identity());
 
         // when
         final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
@@ -638,14 +628,12 @@ public void makeHttpRequestsShouldSetTagidToNullWhenNoFieldsAvailable() {
     @Test
     public void makeHttpRequestsShouldPreserveExistingTagid() {
         // given
-        final BidRequest bidRequest = BidRequest.builder()
-                .imp(singletonList(Imp.builder()
-                        .id("imp123")
+        final BidRequest bidRequest = givenBidRequest(
+                bidReq -> bidReq.id("request_id"),
+                imp -> imp.id("imp123")
                         .tagid("existing_tagid")
-                        .ext(givenRtbhouseExt(node -> node.put("gpid", "gpid_value")))
-                        .build()))
-                .id("request_id")
-                .build();
+                        .ext(givenRtbhouseExt(node -> node.put("gpid", "gpid_value"))),
+                identity());
 
         // when
         final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
@@ -704,8 +692,8 @@ private static Imp givenImp(Function<Imp.ImpBuilder, Imp.ImpBuilder> impCustomiz
                 .build();
     }
 
-    private static ObjectNode givenRtbhouseExt(Function<ObjectNode, ObjectNode> extCustomizer) {
-        final ObjectNode extNode = (ObjectNode) mapper.valueToTree(ExtPrebid.of(null, ExtImpRtbhouse.builder()
+    private static ObjectNode givenRtbhouseExt(UnaryOperator<ObjectNode> extCustomizer) {
+        final ObjectNode extNode = mapper.valueToTree(ExtPrebid.of(null, ExtImpRtbhouse.builder()
                 .publisherId("publisherId")
                 .region("region")
                 .build()));