Skip to content

Commit 996d96f

Browse files
sangarbesangarbe
committed
fixes review comments
1 parent 6f8ce04 commit 996d96f

4 files changed

Lines changed: 44 additions & 151 deletions

File tree

‎src/main/java/org/prebid/server/bidder/seedtag/SeedtagBidder.java‎

Lines changed: 14 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@
66
import com.iab.openrtb.response.Bid;
77
import com.iab.openrtb.response.BidResponse;
88
import com.iab.openrtb.response.SeatBid;
9-
import io.vertx.core.http.HttpMethod;
109
import org.apache.commons.collections4.CollectionUtils;
11-
import org.apache.commons.lang3.StringUtils;
1210
import org.prebid.server.bidder.Bidder;
1311
import org.prebid.server.bidder.model.BidderBid;
1412
import org.prebid.server.bidder.model.BidderCall;
@@ -45,8 +43,8 @@ public class SeedtagBidder implements Bidder<BidRequest> {
4543
private final CurrencyConversionService currencyConversionService;
4644

4745
public SeedtagBidder(String endpointUrl,
48-
CurrencyConversionService currencyConversionService,
49-
JacksonMapper mapper) {
46+
CurrencyConversionService currencyConversionService,
47+
JacksonMapper mapper) {
5048

5149
this.endpointUrl = HttpUtil.validateUrl(Objects.requireNonNull(endpointUrl));
5250
this.currencyConversionService = Objects.requireNonNull(currencyConversionService);
@@ -55,13 +53,11 @@ public SeedtagBidder(String endpointUrl,
5553

5654
@Override
5755
public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request) {
58-
5956
final List<Imp> modifiedImps = new ArrayList<>();
6057
final List<BidderError> errors = new ArrayList<>();
6158

6259
for (Imp imp : request.getImp()) {
6360
try {
64-
parseImpExt(imp);
6561
final Price bidFloorPrice = resolveBidFloor(imp, request);
6662

6763
modifiedImps.add(modifyImp(imp, bidFloorPrice));
@@ -75,18 +71,11 @@ public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request
7571
}
7672

7773
final BidRequest modifiedBidRequest = request.toBuilder()
78-
.cur(Collections.singletonList(BIDDER_CURRENCY))
7974
.imp(modifiedImps)
8075
.build();
81-
82-
return Result.withValue(HttpRequest.<BidRequest>builder()
83-
.method(HttpMethod.POST)
84-
.uri(endpointUrl)
85-
.headers(HttpUtil.headers())
86-
.payload(modifiedBidRequest)
87-
.body(mapper.encodeToBytes(modifiedBidRequest))
88-
.impIds(BidderUtil.impIds(modifiedBidRequest))
89-
.build());
76+
return Result.of(
77+
Collections.singletonList(BidderUtil.defaultRequest(modifiedBidRequest, endpointUrl, mapper)),
78+
errors);
9079
}
9180

9281
private static Imp modifyImp(Imp imp, Price bidFloorPrice) {
@@ -98,32 +87,19 @@ private static Imp modifyImp(Imp imp, Price bidFloorPrice) {
9887

9988
private Price resolveBidFloor(Imp imp, BidRequest bidRequest) {
10089
final Price initialBidFloorPrice = Price.of(imp.getBidfloorcur(), imp.getBidfloor());
101-
return BidderUtil.isValidPrice(initialBidFloorPrice)
102-
&& !StringUtils.equalsIgnoreCase(initialBidFloorPrice.getCurrency(), BIDDER_CURRENCY)
90+
return BidderUtil.shouldConvertBidFloor(initialBidFloorPrice, BIDDER_CURRENCY)
10391
? convertBidFloor(initialBidFloorPrice, imp.getId(), bidRequest)
10492
: initialBidFloorPrice;
10593
}
10694

10795
private Price convertBidFloor(Price bidFloorPrice, String impId, BidRequest bidRequest) {
108-
final String bidFloorCur = bidFloorPrice.getCurrency();
109-
try {
110-
final BigDecimal convertedPrice = currencyConversionService
111-
.convertCurrency(bidFloorPrice.getValue(), bidRequest, bidFloorCur, BIDDER_CURRENCY);
112-
113-
return Price.of(BIDDER_CURRENCY, convertedPrice);
114-
} catch (PreBidException e) {
115-
throw new PreBidException(
116-
"Unable to convert provided bid floor currency from %s to %s for imp `%s`"
117-
.formatted(bidFloorCur, BIDDER_CURRENCY, impId));
118-
}
119-
}
96+
final BigDecimal convertedPrice = currencyConversionService.convertCurrency(
97+
bidFloorPrice.getValue(),
98+
bidRequest,
99+
bidFloorPrice.getCurrency(),
100+
BIDDER_CURRENCY);
120101

121-
private ExtImpSeedtag parseImpExt(Imp imp) {
122-
try {
123-
return mapper.mapper().convertValue(imp.getExt(), SEEDTAG_EXT_TYPE_REFERENCE).getBidder();
124-
} catch (IllegalArgumentException e) {
125-
throw new PreBidException(e.getMessage());
126-
}
102+
return Price.of(BIDDER_CURRENCY, convertedPrice);
127103
}
128104

129105
@Override
@@ -148,6 +124,7 @@ private static List<BidderBid> extractBids(BidResponse bidResponse, List<BidderE
148124
.map(SeatBid::getBid)
149125
.filter(Objects::nonNull)
150126
.flatMap(Collection::stream)
127+
.filter(Objects::nonNull)
151128
.map(bid -> makeBidderBid(bid, errors))
152129
.filter(Objects::nonNull)
153130
.toList();
@@ -169,9 +146,7 @@ private static BidType getBidType(Bid bid) {
169146
return switch (bid.getMtype()) {
170147
case 1 -> BidType.banner;
171148
case 2 -> BidType.video;
172-
default -> throw new PreBidException(
173-
"Invalid bid.mtype for bid.id: '%s'"
174-
.formatted(bid.getId()));
149+
default -> throw new PreBidException("Invalid bid.mtype for bid.id: '%s'".formatted(bid.getId()));
175150
};
176151
}
177152
}

‎src/main/java/org/prebid/server/spring/config/bidder/SeedtagConfiguration.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,15 @@ BidderConfigurationProperties configurationProperties() {
3030

3131
@Bean
3232
BidderDeps seedtagBidderDeps(BidderConfigurationProperties seedtagConfigurationProperties,
33-
@NotBlank @Value("${external-url}") String externalUrl,
34-
CurrencyConversionService currencyConversionService,
35-
JacksonMapper mapper) {
33+
@NotBlank @Value("${external-url}") String externalUrl,
34+
CurrencyConversionService currencyConversionService,
35+
JacksonMapper mapper) {
3636

3737
return BidderDepsAssembler.forBidder(BIDDER_NAME)
3838
.withConfig(seedtagConfigurationProperties)
3939
.usersyncerCreator(UsersyncerCreator.create(externalUrl))
40-
.bidderCreator(config -> new SeedtagBidder(config.getEndpoint(),
40+
.bidderCreator(config -> new SeedtagBidder(
41+
config.getEndpoint(),
4142
currencyConversionService,
4243
mapper))
4344
.assemble();

‎src/test/java/org/prebid/server/bidder/seedtag/SeedtagBidderTest.java‎

Lines changed: 25 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.prebid.server.bidder.seedtag;
22

33
import com.fasterxml.jackson.core.JsonProcessingException;
4-
import com.iab.openrtb.request.Banner;
54
import com.iab.openrtb.request.BidRequest;
65
import com.iab.openrtb.request.Imp;
76
import com.iab.openrtb.response.Bid;
@@ -21,17 +20,14 @@
2120
import org.prebid.server.bidder.model.Result;
2221
import org.prebid.server.currency.CurrencyConversionService;
2322
import org.prebid.server.exception.PreBidException;
24-
import org.prebid.server.proto.openrtb.ext.ExtPrebid;
25-
import org.prebid.server.proto.openrtb.ext.request.seedtag.ExtImpSeedtag;
2623

2724
import java.math.BigDecimal;
28-
import java.util.Arrays;
2925
import java.util.List;
30-
import java.util.function.Function;
26+
import java.util.function.UnaryOperator;
3127

3228
import static java.util.Arrays.asList;
3329
import static java.util.Collections.singletonList;
34-
import static java.util.function.Function.identity;
30+
import static java.util.function.UnaryOperator.identity;
3531
import static org.assertj.core.api.Assertions.assertThat;
3632
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
3733
import static org.assertj.core.api.Assertions.tuple;
@@ -58,17 +54,19 @@ public void setUp() {
5854

5955
@Test
6056
public void creationShouldFailOnInvalidEndpointUrl() {
61-
assertThatIllegalArgumentException().isThrownBy(() -> new SeedtagBidder("invalid_url",
62-
currencyConversionService,
63-
jacksonMapper));
57+
assertThatIllegalArgumentException()
58+
.isThrownBy(() -> new SeedtagBidder(
59+
"invalid_url",
60+
currencyConversionService,
61+
jacksonMapper));
6462
}
6563

6664
@Test
6765
public void makeHttpRequestsShouldMakeOneRequestWithAllImps() {
6866
// given
6967
final BidRequest bidRequest = givenBidRequest(
7068
identity(),
71-
requestBuilder -> requestBuilder.imp(Arrays.asList(
69+
requestBuilder -> requestBuilder.imp(asList(
7270
givenImp(identity()),
7371
givenImp(identity()))));
7472

@@ -90,7 +88,7 @@ public void makeHttpRequestsShouldConvertCurrency() {
9088
.willReturn(BigDecimal.TEN);
9189

9290
final BidRequest bidRequest = givenBidRequest(imp -> imp
93-
.bidfloor(BigDecimal.TEN)
91+
.bidfloor(BigDecimal.TWO)
9492
.bidfloorcur("EUR"));
9593

9694
// when
@@ -106,98 +104,35 @@ public void makeHttpRequestsShouldConvertCurrency() {
106104
}
107105

108106
@Test
109-
public void makeHttpRequestsShouldMakeOneRequesWithAllCurrencyConvertedImps() {
107+
public void makeHttpRequestsShouldSkipImpsWithCurrencyThatCanNotBeConverted() {
110108
// given
111109
given(currencyConversionService.convertCurrency(any(), any(), anyString(), anyString()))
112110
.willThrow(PreBidException.class);
113111

114112
final BidRequest bidRequest = givenBidRequest(
115113
identity(),
116-
requestBuilder -> requestBuilder.imp(Arrays.asList(
114+
requestBuilder -> requestBuilder.imp(asList(
117115
givenImp(identity()),
118116
givenImp(impBuilder -> impBuilder
119117
.bidfloor(BigDecimal.TEN)
120-
.bidfloorcur("EUR")
121-
))));
118+
.bidfloorcur("EUR"))
119+
)));
122120

123121
// when
124122
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
125123

126124
// then
127-
assertThat(result.getErrors()).isEmpty();
125+
assertThat(result.getErrors()).hasSize(1);
128126
assertThat(result.getValue()).hasSize(1)
129127
.extracting(HttpRequest::getPayload)
130128
.flatExtracting(BidRequest::getImp)
131129
.hasSize(1);
132130
}
133131

134-
@Test
135-
public void makeHttpRequestsShouldReturnErrorMessageOnFailedConvertCurrencyForAllImps() {
136-
// given
137-
given(currencyConversionService.convertCurrency(any(), any(), anyString(), anyString()))
138-
.willThrow(PreBidException.class);
139-
140-
final BidRequest bidRequest = givenBidRequest(impBuilder -> impBuilder
141-
.bidfloor(BigDecimal.TEN)
142-
.bidfloorcur("EUR"));
143-
144-
// when
145-
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
146-
147-
// then
148-
assertThat(result.getErrors()).allSatisfy(bidderError -> {
149-
assertThat(bidderError.getType())
150-
.isEqualTo(BidderError.Type.bad_input);
151-
assertThat(bidderError.getMessage())
152-
.isEqualTo("Unable to convert provided bid floor currency from EUR to USD for imp `123`");
153-
});
154-
}
155-
156-
@Test
157-
public void makeHttpRequestsShouldReturnErrorIfImpExtCanNotBeParsed() {
158-
// given
159-
final BidRequest bidRequest = BidRequest.builder()
160-
.imp(asList(Imp.builder()
161-
.ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode())))
162-
.build()))
163-
.build();
164-
165-
// when
166-
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
167-
168-
// then
169-
assertThat(result.getErrors()).hasSize(1)
170-
.allSatisfy(error -> {
171-
assertThat(error.getType()).isEqualTo(BidderError.Type.bad_input);
172-
assertThat(error.getMessage()).startsWith("Cannot deserialize value");
173-
});
174-
assertThat(result.getValue()).isEmpty();
175-
}
176-
177-
@Test
178-
public void makeHttpRequestsShouldReturnEveryOccurredErrorWithNoValue() {
179-
// given
180-
final BidRequest bidRequest = BidRequest.builder()
181-
.imp(asList(Imp.builder()
182-
.ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode())))
183-
.build(),
184-
Imp.builder()
185-
.ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode())))
186-
.build()))
187-
.build();
188-
189-
// when
190-
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
191-
192-
// then
193-
assertThat(result.getErrors()).hasSize(2);
194-
assertThat(result.getValue()).isEmpty();
195-
}
196-
197132
@Test
198133
public void makeBidsShouldReturnEmptyListIfBidResponseIsNull() throws JsonProcessingException {
199134
// given
200-
final BidderCall<BidRequest> httpCall = givenHttpCall(null, mapper.writeValueAsString(null));
135+
final BidderCall<BidRequest> httpCall = givenHttpCall(mapper.writeValueAsString(null));
201136

202137
// when
203138
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);
@@ -210,8 +145,7 @@ public void makeBidsShouldReturnEmptyListIfBidResponseIsNull() throws JsonProces
210145
@Test
211146
public void makeBidsShouldReturnEmptyListIfBidResponseSeatBidIsNull() throws JsonProcessingException {
212147
// given
213-
final BidderCall<BidRequest> httpCall = givenHttpCall(null,
214-
mapper.writeValueAsString(BidResponse.builder().build()));
148+
final BidderCall<BidRequest> httpCall = givenHttpCall(mapper.writeValueAsString(BidResponse.builder().build()));
215149

216150
// when
217151
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);
@@ -224,7 +158,7 @@ public void makeBidsShouldReturnEmptyListIfBidResponseSeatBidIsNull() throws Jso
224158
@Test
225159
public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() {
226160
// given
227-
final BidderCall<BidRequest> httpCall = givenHttpCall(null, "invalid");
161+
final BidderCall<BidRequest> httpCall = givenHttpCall("invalid");
228162

229163
// when
230164
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);
@@ -242,9 +176,6 @@ public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() {
242176
public void makeBidsShouldReturnBannerBidIfMediaTypeBanner() throws JsonProcessingException {
243177
// given
244178
final BidderCall<BidRequest> httpCall = givenHttpCall(
245-
BidRequest.builder()
246-
.imp(singletonList(Imp.builder().id("123").build()))
247-
.build(),
248179
mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123").mtype(1))));
249180

250181
// when
@@ -260,9 +191,6 @@ public void makeBidsShouldReturnBannerBidIfMediaTypeBanner() throws JsonProcessi
260191
public void makeBidsShouldReturnVideoBidIfMediaTypeVideo() throws JsonProcessingException {
261192
// given
262193
final BidderCall<BidRequest> httpCall = givenHttpCall(
263-
BidRequest.builder()
264-
.imp(singletonList(Imp.builder().id("123").build()))
265-
.build(),
266194
mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123").mtype(2))));
267195

268196
// when
@@ -278,9 +206,6 @@ public void makeBidsShouldReturnVideoBidIfMediaTypeVideo() throws JsonProcessing
278206
public void makeBidsShouldReturnErrorIfMediaTypeInvalid() throws JsonProcessingException {
279207
// given
280208
final BidderCall<BidRequest> httpCall = givenHttpCall(
281-
BidRequest.builder()
282-
.imp(singletonList(Imp.builder().id("123").build()))
283-
.build(),
284209
mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123").mtype(4).id("456"))));
285210

286211
// when
@@ -296,28 +221,22 @@ public void makeBidsShouldReturnErrorIfMediaTypeInvalid() throws JsonProcessingE
296221
}
297222

298223
private static BidRequest givenBidRequest(
299-
Function<Imp.ImpBuilder, Imp.ImpBuilder> impCustomizer,
300-
Function<BidRequest.BidRequestBuilder, BidRequest.BidRequestBuilder> requestCustomizer) {
224+
UnaryOperator<Imp.ImpBuilder> impCustomizer,
225+
UnaryOperator<BidRequest.BidRequestBuilder> requestCustomizer) {
301226
return requestCustomizer.apply(BidRequest.builder()
302227
.imp(singletonList(givenImp(impCustomizer))))
303228
.build();
304229
}
305230

306-
private static BidRequest givenBidRequest(
307-
Function<Imp.ImpBuilder, Imp.ImpBuilder> impCustomizer) {
231+
private static BidRequest givenBidRequest(UnaryOperator<Imp.ImpBuilder> impCustomizer) {
308232
return givenBidRequest(impCustomizer, identity());
309233
}
310234

311-
private static Imp givenImp(Function<Imp.ImpBuilder, Imp.ImpBuilder> impCustomizer) {
312-
return impCustomizer.apply(Imp.builder()
313-
.id("123"))
314-
.banner(Banner.builder().build())
315-
.ext(mapper.valueToTree(ExtPrebid.of(null,
316-
ExtImpSeedtag.of("adUnitId"))))
317-
.build();
235+
private static Imp givenImp(UnaryOperator<Imp.ImpBuilder> impCustomizer) {
236+
return impCustomizer.apply(Imp.builder().id("123")).build();
318237
}
319238

320-
private static BidResponse givenBidResponse(Function<Bid.BidBuilder, Bid.BidBuilder> bidCustomizer) {
239+
private static BidResponse givenBidResponse(UnaryOperator<Bid.BidBuilder> bidCustomizer) {
321240
return BidResponse.builder()
322241
.cur("USD")
323242
.seatbid(singletonList(SeatBid.builder()
@@ -326,8 +245,7 @@ private static BidResponse givenBidResponse(Function<Bid.BidBuilder, Bid.BidBuil
326245
.build();
327246
}
328247

329-
private static BidderCall<BidRequest> givenHttpCall(BidRequest bidRequest, String body) {
330-
return BidderCall.succeededHttp(HttpRequest.<BidRequest>builder().payload(bidRequest).build(),
331-
HttpResponse.of(200, null, body), null);
248+
private static BidderCall<BidRequest> givenHttpCall(String responseBody) {
249+
return BidderCall.succeededHttp(null, HttpResponse.of(200, null, responseBody), null);
332250
}
333251
}

0 commit comments

Comments
 (0)