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

Add bidscube bidder #1350

Merged
merged 8 commits into from
Jul 7, 2021
Merged

Add bidscube bidder #1350

merged 8 commits into from
Jul 7, 2021

Conversation

And1sS
Copy link
Member

@And1sS And1sS commented Jul 1, 2021

No description provided.

public class BidscubeTest extends IntegrationTest {

@Test
public void openrtb2AuctionShouldRespondWithBidsFromBetween() throws IOException, JSONException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bidscube

.method(HttpMethod.POST)
.uri(endpointUrl)
.headers(HttpUtil.headers())
.payload(request.toBuilder().imp(Collections.singletonList(imp)).build())
Copy link
Collaborator

Choose a reason for hiding this comment

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

In go they take "bidder" property and then set it as ext

bidderExt, bidderExtExists := impExt["bidder"]
...
impression.Ext = bidderExt

Comment on lines 73 to 80
extImpBidscube = mapper.mapper().convertValue(imp.getExt(), BIDSCUBE_IMP_EXT_TYPE_REFERENCE).getBidder();
} catch (IllegalArgumentException e) {
throw new PreBidException("Missing required bidder parameters");
}

if (StringUtils.isEmpty(extImpBidscube.getPlacementId())) {
throw new PreBidException("Missing required bidder parameters");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need converting, just Node replacement instead of full ext, just it bidder property

.seatbid(givenSeatBid(
givenBid("123", banner, bidBuilder -> bidBuilder.ext(null)),
givenBid("456", banner, bidBuilder -> bidBuilder.ext(mapper.valueToTree(
ExtPrebid.of(null, null)))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add this test case givenBid("123", null))

Comment on lines 123 to 145
private BidderBid constructBidderBid(Bid bid, BidResponse bidResponse, List<BidderError> errors) {
try {
final JsonNode bidExt = bid.getExt();
if (bidExt == null) {
throw new IllegalArgumentException();
}

final String bidTypeString = bidExt.path("prebid").path("type").asText("");
final BidType bidType = mapper.decodeValue(String.format("\"%s\"", bidTypeString), BidType.class);

return BidderBid.of(bid, resolveBidType(bidType), bidResponse.getCur());
} catch (IllegalArgumentException | DecodeException e) {
errors.add(BidderError.badInput("Unable to read bid.ext.prebid.type"));
return null;
}
}

private static BidType resolveBidType(BidType bidType) {
if (!POSSIBLE_BID_TYPES.contains(bidType)) {
return BidType.banner;
}
return bidType;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's update this code to this one

private static BidderBid constructBidderBid(Bid bid, BidResponse bidResponse, List<BidderError> errors) {
        final JsonNode bidExt = bid.getExt();
        final JsonNode prebidNode = isNotEmptyOrMissedNode(bidExt) ? bidExt.get("prebid") : null;
        final JsonNode typeNode = isNotEmptyOrMissedNode(prebidNode) ? prebidNode.get("type") : null;
        if (typeNode == null || !typeNode.isTextual()) {
            errors.add(BidderError.badInput("Unable to read bid.ext.prebid.type"));
            return null;
        }

        return BidderBid.of(bid, resolveBidType(typeNode.asText()), bidResponse.getCur());
    }

    private static boolean isNotEmptyOrMissedNode(JsonNode node) {
        return node != null && !node.isEmpty();
    }

    private static BidType resolveBidType(String bidType) {
        if (!POSSIBLE_BID_TYPES.contains(bidType)) {
            return BidType.banner;
        }
        return BidType.valueOf(bidType);
    }

And POSSIBLE_BID_TYPES to ImmutableSet.of("banner", "video", "native")
Now we can work only with String as in go and with unknown media types

final Result<List<HttpRequest<BidRequest>>> result = bidscubeBidder.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).containsExactly(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also check if value is missed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

assertThat(result.getValue()).isEmpty();

@And1sS And1sS requested a review from SerhiiNahornyi July 2, 2021 20:46
final Result<List<HttpRequest<BidRequest>>> result = bidscubeBidder.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).containsExactly(
Copy link
Collaborator

Choose a reason for hiding this comment

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

assertThat(result.getValue()).isEmpty();

@rpanchyk rpanchyk merged commit a16963a into master Jul 7, 2021
@rpanchyk rpanchyk deleted the add-bidscube-bidder branch July 7, 2021 11:21
nickluck9 pushed a commit that referenced this pull request Jul 8, 2021
nickluck9 pushed a commit that referenced this pull request Aug 9, 2021
nickluck9 pushed a commit that referenced this pull request Aug 10, 2021
nickluck9 pushed a commit that referenced this pull request Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants