Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kobler: New adapter ported from Go #3684

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
54 changes: 16 additions & 38 deletions src/main/java/org/prebid/server/bidder/kobler/KoblerBidder.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.prebid.server.currency.CurrencyConversionService;
import org.prebid.server.exception.PreBidException;
import org.prebid.server.json.DecodeException;
import org.prebid.server.json.EncodeException;
import org.prebid.server.json.JacksonMapper;
import org.prebid.server.proto.openrtb.ext.ExtPrebid;
import org.prebid.server.proto.openrtb.ext.request.kobler.ExtImpKobler;
Expand All @@ -34,6 +33,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;

public class KoblerBidder implements Bidder<BidRequest> {

Expand Down Expand Up @@ -71,40 +71,30 @@ public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest bidRequ
final List<Imp> imps = bidRequest.getImp();
if (!imps.isEmpty()) {
try {
testMode = parseImpExt(imps.get(0)).getTest();
testMode = parseImpExt(imps.getFirst()).getTest();
} catch (PreBidException e) {
errors.add(BidderError.badInput(e.getMessage()));
return Result.withErrors(errors);
}
}

for (Imp imp : imps) {
try {
final Imp processedImp = processImp(bidRequest, imp);
modifiedImps.add(processedImp);
modifiedImps.add(processImp(bidRequest, imp));
} catch (PreBidException e) {
errors.add(BidderError.badInput(e.getMessage()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I can see, in case of any imp processing error it's immediately should return the Result

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any fix on that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if processImp throws an exception, we immediately return Result.withErrors(errors), instead of continuing the loop.

}
}

if (modifiedImps.isEmpty()) {
errors.add(BidderError.badInput("No valid impressions"));
return Result.withErrors(errors);
}

final BidRequest modifiedRequest = bidRequest.toBuilder()
.cur(currencies)
.imp(modifiedImps)
.build();

final String endpoint = testMode ? DEV_ENDPOINT : endpointUrl;

try {
final HttpRequest<BidRequest> httpRequest = BidderUtil.defaultRequest(modifiedRequest, endpoint, mapper);
return Result.of(Collections.singletonList(httpRequest), errors);
} catch (EncodeException e) {
errors.add(BidderError.badInput("Failed to encode request: " + e.getMessage()));
return Result.withErrors(errors);
}
final HttpRequest<BidRequest> httpRequest = BidderUtil.defaultRequest(modifiedRequest, endpoint, mapper);
return Result.of(Collections.singletonList(httpRequest), errors);
}

private Imp processImp(BidRequest bidRequest, Imp imp) {
Expand Down Expand Up @@ -146,46 +136,34 @@ public Result<List<BidderBid>> makeBids(BidderCall<BidRequest> httpCall, BidRequ
try {
final List<BidderError> errors = new ArrayList<>();
final BidResponse bidResponse = mapper.decodeValue(httpCall.getResponse().getBody(), BidResponse.class);
return Result.of(extractBids(bidResponse, errors), errors);
return Result.of(extractBids(bidResponse), errors);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Result.withValues(...) + get rid of redundant errors

} catch (DecodeException e) {
return Result.withError(BidderError.badServerResponse(e.getMessage()));
}
}

private List<BidderBid> extractBids(BidResponse bidResponse, List<BidderError> errors) {
private List<BidderBid> extractBids(BidResponse bidResponse) {
if (bidResponse == null || CollectionUtils.isEmpty(bidResponse.getSeatbid())) {
return Collections.emptyList();
}
return bidsFromResponse(bidResponse, errors);
return bidsFromResponse(bidResponse);
}

private List<BidderBid> bidsFromResponse(BidResponse bidResponse, List<BidderError> errors) {
private List<BidderBid> bidsFromResponse(BidResponse bidResponse) {
return bidResponse.getSeatbid().stream()
.filter(Objects::nonNull)
.map(SeatBid::getBid)
.filter(Objects::nonNull)
.flatMap(Collection::stream)
.map(bid -> BidderBid.of(bid, getBidType(bid), bidResponse.getCur()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add .filter(Objects::nonNull) before .map(bid -> ...)

.filter(Objects::nonNull)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, return all the filters back except for the last one

.toList();
}

private BidType getBidType(Bid bid) {
if (bid.getExt() == null) {
return BidType.banner;
}

final ObjectNode prebidNode = (ObjectNode) bid.getExt().get(EXT_PREBID);
if (prebidNode == null) {
return BidType.banner;
}

final ExtBidPrebid extBidPrebid = parseExtBidPrebid(prebidNode);
if (extBidPrebid == null || extBidPrebid.getType() == null) {
return BidType.banner;
}

return extBidPrebid.getType(); // jeśli udało się sparsować
return Optional.ofNullable(bid.getExt())
.map(ext -> ext.get(EXT_PREBID))
.map(ObjectNode.class::cast)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add .filter(JsonNode::isObject)

.map(this::parseExtBidPrebid)
.map(ExtBidPrebid::getType)
.orElse(BidType.banner);
}

private ExtBidPrebid parseExtBidPrebid(ObjectNode prebid) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.List;
import java.util.function.UnaryOperator;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static java.util.function.UnaryOperator.identity;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -79,23 +78,38 @@ public void makeHttpRequestsShouldReturnErrorIfNoValidImps() {

// Then
assertThat(result.getErrors())
.hasSize(2)
.hasSize(1)
.extracting(BidderError::getMessage)
.containsExactlyInAnyOrder("Currency conversion failed", "No valid impressions");
.containsExactlyInAnyOrder("Currency conversion failed");
}

@Test
public void makeHttpRequestsShouldReturnErrorIfNoImps() {
public void makeBidsShouldReturnEmptyListWhenBidResponseIsNull() {
// Given
final BidRequest bidRequest = BidRequest.builder().cur(singletonList("USD")).imp(emptyList()).build();
final BidderCall<BidRequest> httpCall = givenHttpCallWithBody("null");

// When
final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest);
final Result<List<BidderBid>> result = target.makeBids(httpCall, BidRequest.builder().build());

// Then
assertThat(result.getErrors())
.hasSize(1)
.allSatisfy(error -> assertThat(error.getMessage()).contains("No valid impressions"));
assertThat(result.getValue()).isEmpty();
assertThat(result.getErrors()).isEmpty();
}

@Test
public void makeBidsShouldReturnEmptyListWhenSeatbidIsEmpty() throws JsonProcessingException {
// Given
final BidResponse bidResponse = BidResponse.builder()
.seatbid(Collections.emptyList())
.build();
final BidderCall<BidRequest> httpCall = givenHttpCall(bidResponse);

// When
final Result<List<BidderBid>> result = target.makeBids(httpCall, BidRequest.builder().build());

// Then
assertThat(result.getValue()).isEmpty();
assertThat(result.getErrors()).isEmpty();
}

@Test
Expand Down Expand Up @@ -225,4 +239,11 @@ private BidderCall<BidRequest> givenHttpCall() {
HttpRequest.<BidRequest>builder().payload(null).build(),
HttpResponse.of(200, null, "invalid"), null);
}

private BidderCall<BidRequest> givenHttpCallWithBody(String body) {
return BidderCall.succeededHttp(
HttpRequest.<BidRequest>builder().payload(null).build(),
HttpResponse.of(200, null, body),
null);
}
}
Loading