Skip to content

Commit c606fab

Browse files
t-sormontet-sormonte
committed
CTMBNara Review
1 parent e35e27f commit c606fab

2 files changed

Lines changed: 46 additions & 88 deletions

File tree

‎src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java‎

Lines changed: 45 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@
3232
import java.util.ArrayList;
3333
import java.util.Collection;
3434
import java.util.Collections;
35-
import java.util.Iterator;
3635
import java.util.List;
37-
import java.util.Map;
3836
import java.util.Objects;
3937
import java.util.Optional;
4038

@@ -58,21 +56,18 @@ public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request
5856
String siteNetworkId = null;
5957

6058
for (Imp imp : request.getImp()) {
61-
try {
62-
final ExtImpSparteo bidderParams = parseExtImp(imp);
63-
64-
if (siteNetworkId == null && bidderParams.getNetworkId() != null) {
65-
siteNetworkId = bidderParams.getNetworkId();
59+
if (siteNetworkId == null) {
60+
try {
61+
siteNetworkId = parseExtImp(imp).getNetworkId();
62+
} catch (PreBidException e) {
63+
errors.add(BidderError.badInput(
64+
"ignoring imp id=%s, error processing ext: %s".formatted(
65+
imp.getId(), e.getMessage())));
6666
}
67-
68-
final ObjectNode modifiedExt = modifyImpExt(imp);
69-
70-
modifiedImps.add(imp.toBuilder().ext(modifiedExt).build());
71-
} catch (PreBidException e) {
72-
errors.add(BidderError.badInput(
73-
"ignoring imp id=%s, error processing ext: %s".formatted(
74-
imp.getId(), e.getMessage())));
7567
}
68+
69+
final ObjectNode modifiedExt = modifyImpExt(imp);
70+
modifiedImps.add(imp.toBuilder().ext(modifiedExt).build());
7671
}
7772

7873
if (modifiedImps.isEmpty()) {
@@ -99,24 +94,10 @@ private ExtImpSparteo parseExtImp(Imp imp) {
9994

10095
private static ObjectNode modifyImpExt(Imp imp) {
10196
final ObjectNode modifiedImpExt = imp.getExt().deepCopy();
102-
final JsonNode sparteoJsonNode = modifiedImpExt.get("sparteo");
103-
final ObjectNode sparteoNode = sparteoJsonNode == null || !sparteoJsonNode.isObject()
104-
? modifiedImpExt.putObject("sparteo")
105-
: (ObjectNode) sparteoJsonNode;
106-
107-
final JsonNode paramsJsonNode = sparteoNode.get("params");
108-
final ObjectNode paramsNode = paramsJsonNode == null || !paramsJsonNode.isObject()
109-
? sparteoNode.putObject("params")
110-
: (ObjectNode) paramsJsonNode;
111-
97+
final ObjectNode sparteoNode = modifiedImpExt.putObject("sparteo");
11298
final JsonNode bidderJsonNode = modifiedImpExt.remove("bidder");
113-
if (bidderJsonNode != null && bidderJsonNode.isObject()) {
114-
final Iterator<Map.Entry<String, JsonNode>> fields = bidderJsonNode.fields();
115-
while (fields.hasNext()) {
116-
final Map.Entry<String, JsonNode> field = fields.next();
117-
paramsNode.set(field.getKey(), field.getValue());
118-
}
119-
}
99+
sparteoNode.set("params", bidderJsonNode);
100+
120101
return modifiedImpExt;
121102
}
122103

@@ -125,26 +106,31 @@ private Site modifySite(Site site, String siteNetworkId, JacksonMapper mapper) {
125106
return site;
126107
}
127108

128-
final Publisher publisher = site.getPublisher();
109+
final Publisher originalPublisher = site.getPublisher();
110+
final ExtPublisher originalExt = originalPublisher.getExt();
129111

130-
final ExtPublisher extPublisher = publisher.getExt() != null
131-
? publisher.getExt()
112+
final ExtPublisher modifiedExt = originalExt != null
113+
? ExtPublisher.of(originalExt.getPrebid())
132114
: ExtPublisher.empty();
133115

134-
final JsonNode paramsProperty = extPublisher.getProperty("params");
116+
if (originalExt != null) {
117+
mapper.fillExtension(modifiedExt, originalExt);
118+
}
119+
120+
final JsonNode paramsProperty = modifiedExt.getProperty("params");
135121
final ObjectNode paramsNode;
136122

137123
if (paramsProperty != null && paramsProperty.isObject()) {
138124
paramsNode = (ObjectNode) paramsProperty;
139125
} else {
140126
paramsNode = mapper.mapper().createObjectNode();
141-
extPublisher.addProperty("params", paramsNode);
127+
modifiedExt.addProperty("params", paramsNode);
142128
}
143129

144130
paramsNode.put("networkId", siteNetworkId);
145131

146-
final Publisher modifiedPublisher = publisher.toBuilder()
147-
.ext(extPublisher)
132+
final Publisher modifiedPublisher = originalPublisher.toBuilder()
133+
.ext(modifiedExt)
148134
.build();
149135

150136
return site.toBuilder()
@@ -179,6 +165,26 @@ private List<BidderBid> extractBids(BidResponse bidResponse, List<BidderError> e
179165
.toList();
180166
}
181167

168+
private BidderBid toBidderBid(Bid bid, String currency, List<BidderError> errors) {
169+
try {
170+
final BidType bidType = getBidType(bid);
171+
172+
final Integer mtype = switch (bidType) {
173+
case banner -> 1;
174+
case video -> 2;
175+
case xNative -> 4;
176+
default -> null;
177+
};
178+
179+
final Bid bidWithMtype = mtype != null ? bid.toBuilder().mtype(mtype).build() : bid;
180+
181+
return BidderBid.of(bidWithMtype, bidType, currency);
182+
} catch (PreBidException e) {
183+
errors.add(BidderError.badServerResponse(e.getMessage()));
184+
return null;
185+
}
186+
}
187+
182188
private BidType getBidType(Bid bid) throws PreBidException {
183189
final BidType bidType = Optional.ofNullable(bid.getExt())
184190
.map(ext -> ext.get("prebid"))
@@ -203,24 +209,4 @@ private ExtBidPrebid parseExtBidPrebid(JsonNode prebidNode) {
203209
return null;
204210
}
205211
}
206-
207-
private BidderBid toBidderBid(Bid bid, String currency, List<BidderError> errors) {
208-
try {
209-
final BidType bidType = getBidType(bid);
210-
211-
final Integer mtype = switch (bidType) {
212-
case banner -> 1;
213-
case video -> 2;
214-
case xNative -> 4;
215-
default -> null;
216-
};
217-
218-
final Bid bidWithMtype = mtype != null ? bid.toBuilder().mtype(mtype).build() : bid;
219-
220-
return BidderBid.of(bidWithMtype, bidType, currency);
221-
} catch (PreBidException e) {
222-
errors.add(BidderError.badServerResponse(e.getMessage()));
223-
return null;
224-
}
225-
}
226212
}

‎src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ public void makeHttpRequestsShouldReturnErrorWhenImpExtIsInvalid() {
6363
assertThat(result.getErrors()).hasSize(1)
6464
.allSatisfy(error -> assertThat(error.getMessage())
6565
.startsWith("ignoring imp id=impId, error processing ext: invalid imp.ext"));
66-
assertThat(result.getValue()).isEmpty();
6766
}
6867

6968
@Test
@@ -76,7 +75,6 @@ public void makeHttpRequestsShouldReturnErrorWhenAllImpsAreInvalid() {
7675
final Result<List<HttpRequest<BidRequest>>> result = sparteoBidder.makeHttpRequests(bidRequest);
7776

7877
// then
79-
assertThat(result.getValue()).isEmpty();
8078
assertThat(result.getErrors()).hasSize(1);
8179
}
8280

@@ -101,7 +99,7 @@ public void makeHttpRequestsShouldReturnPartialResultWhenSomeImpsAreInvalid() {
10199
.flatExtracting(BidRequest::getImp)
102100
.extracting(Imp::getExt)
103101
.extracting((JsonNode ext) -> ext.at("/sparteo/params/key").asText())
104-
.containsExactly("value");
102+
.containsExactly("", "value");
105103
}
106104

107105
@Test
@@ -171,32 +169,6 @@ public void makeHttpRequestsShouldUseFirstNetworkIdWhenMultipleImpsDefineIt() {
171169
.containsExactly("id1");
172170
}
173171

174-
@Test
175-
public void makeHttpRequestsShouldMergeBidderAndSparteoParams() {
176-
// given
177-
final ObjectNode impExt = mapper.createObjectNode();
178-
impExt.set("bidder", mapper.createObjectNode().put("paramFromBidder", "value1"));
179-
final ObjectNode sparteoNode = impExt.putObject("sparteo");
180-
sparteoNode.putObject("params").put("paramFromSparteo", "value2");
181-
182-
final BidRequest bidRequest = givenBidRequest(givenImp(imp -> imp.ext(impExt)));
183-
184-
// when
185-
final Result<List<HttpRequest<BidRequest>>> result = sparteoBidder.makeHttpRequests(bidRequest);
186-
187-
// then
188-
assertThat(result.getErrors()).isEmpty();
189-
assertThat(result.getValue())
190-
.extracting(HttpRequest::getPayload)
191-
.flatExtracting(BidRequest::getImp)
192-
.extracting(Imp::getExt)
193-
.extracting(ext -> ext.at("/sparteo/params"))
194-
.allSatisfy(params -> {
195-
assertThat(params.at("/paramFromBidder").asText()).isEqualTo("value1");
196-
assertThat(params.at("/paramFromSparteo").asText()).isEqualTo("value2");
197-
});
198-
}
199-
200172
@Test
201173
public void makeHttpRequestsShouldOverwriteSparteoParamsWithBidderParamsOnConflict() {
202174
// given

0 commit comments

Comments
 (0)