Skip to content

Commit

Permalink
AdkernelAdn bidder review (#1023)
Browse files Browse the repository at this point in the history
  • Loading branch information
@SerhiiNahornyi
SerhiiNahornyi authored Nov 19, 2020
1 parent d68d854 commit 4aa6ad4
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 104 deletions.
145 changes: 68 additions & 77 deletions src/main/java/org/prebid/server/bidder/adkerneladn/AdkernelAdnBidder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import io.vertx.core.MultiMap;
import io.vertx.core.http.HttpMethod;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.bidder.Bidder;
import org.prebid.server.bidder.model.BidderBid;
import org.prebid.server.bidder.model.BidderError;
Expand All @@ -26,8 +28,6 @@
import org.prebid.server.proto.openrtb.ext.response.BidType;
import org.prebid.server.util.HttpUtil;

import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -45,6 +45,9 @@ public class AdkernelAdnBidder implements Bidder<BidRequest> {
private static final TypeReference<ExtPrebid<?, ExtImpAdkernelAdn>> ADKERNELADN_EXT_TYPE_REFERENCE =
new TypeReference<ExtPrebid<?, ExtImpAdkernelAdn>>() {
};
private static final String DEFAULT_DOMAIN = "tag.adkernel.com";
private static final String URL_HOST_MACRO = "{{Host}}";
private static final String URL_PUBLISHER_ID_MACRO = "{{PublisherID}}";

private final String endpointUrl;
private final JacksonMapper mapper;
Expand All @@ -56,39 +59,36 @@ public AdkernelAdnBidder(String endpointUrl, JacksonMapper mapper) {

@Override
public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest bidRequest) {
final List<Imp> imps = bidRequest.getImp();

final List<Imp> validImps = bidRequest.getImp();
final List<BidderError> errors = new ArrayList<>();
final List<HttpRequest<BidRequest>> httpRequests = new ArrayList<>();
try {
final List<ExtImpAdkernelAdn> impExts = getAndValidateImpExt(imps);
final Map<ExtImpAdkernelAdn, List<Imp>> pubToImps = dispatchImpressions(imps, impExts);
httpRequests.addAll(buildAdapterRequests(bidRequest, pubToImps, endpointUrl));
} catch (PreBidException e) {
errors.add(BidderError.badInput(e.getMessage()));
}

return Result.of(httpRequests, errors);
}
final Map<Imp, ExtImpAdkernelAdn> impWithExts = getAndValidateImpExt(validImps, errors);
final Map<ExtImpAdkernelAdn, List<Imp>> pubToImps = dispatchImpressions(impWithExts, errors);
if (MapUtils.isEmpty(pubToImps)) {
return Result.withErrors(errors);
}

private static MultiMap headers() {
return HttpUtil.headers()
.add(HttpUtil.X_OPENRTB_VERSION_HEADER, "2.5");
return Result.of(buildAdapterRequests(bidRequest, pubToImps), errors);
}

private List<ExtImpAdkernelAdn> getAndValidateImpExt(List<Imp> imps) {
return imps.stream()
.map(AdkernelAdnBidder::validateImp)
.map(this::parseAndValidateAdkernelAdnExt)
.collect(Collectors.toList());
private Map<Imp, ExtImpAdkernelAdn> getAndValidateImpExt(List<Imp> imps, List<BidderError> errors) {
final Map<Imp, ExtImpAdkernelAdn> validImpsWithExts = new HashMap<>();
for (Imp imp : imps) {
try {
validateImp(imp);
validImpsWithExts.put(imp, parseAndValidateAdkernelAdnExt(imp));
} catch (PreBidException e) {
errors.add(BidderError.badInput(e.getMessage()));
}
}
return validImpsWithExts;
}

private static Imp validateImp(Imp imp) {
private static void validateImp(Imp imp) {
if (imp.getBanner() == null && imp.getVideo() == null) {
throw new PreBidException(String.format("Invalid imp with id=%s. Expected imp.banner or imp.video",
imp.getId()));
}
return imp;
}

private ExtImpAdkernelAdn parseAndValidateAdkernelAdnExt(Imp imp) {
Expand All @@ -105,41 +105,42 @@ private ExtImpAdkernelAdn parseAndValidateAdkernelAdnExt(Imp imp) {
return adkernelAdnExt;
}

/**
* Group impressions by AdKernel-specific parameters `pubId` & `host`.
*/
private static Map<ExtImpAdkernelAdn, List<Imp>> dispatchImpressions(List<Imp> imps,
List<ExtImpAdkernelAdn> impExts) {
private static Map<ExtImpAdkernelAdn, List<Imp>> dispatchImpressions(Map<Imp, ExtImpAdkernelAdn> impsWithExts,
List<BidderError> errors) {
final Map<ExtImpAdkernelAdn, List<Imp>> result = new HashMap<>();

for (int i = 0; i < imps.size(); i++) {
final Imp imp = compatImpression(imps.get(i));
final ExtImpAdkernelAdn impExt = impExts.get(i);
for (Imp key : impsWithExts.keySet()) {
final Imp imp;
try {
imp = compatImpression(key);
} catch (PreBidException e) {
errors.add(BidderError.badInput(e.getMessage()));
continue;
}
final ExtImpAdkernelAdn impExt = impsWithExts.get(key);
result.putIfAbsent(impExt, new ArrayList<>());
result.get(impExt).add(imp);
}

return result;
}

/**
* Alter impression info to comply with adkernel platform requirements.
*/
private static Imp compatImpression(Imp imp) {
final Imp.ImpBuilder impBuilder = imp.toBuilder();
impBuilder.ext(null); // do not forward ext to adkernel platform
impBuilder.ext(null);

final Banner banner = imp.getBanner();
if (banner != null) {
return compatBannerImpression(impBuilder, banner);
compatBannerImpression(impBuilder, banner);
}
return impBuilder.audio(null)
return impBuilder
.audio(null)
.xNative(null)
.build();
}

private static Imp compatBannerImpression(Imp.ImpBuilder impBuilder, Banner compatBanner) {
private static void compatBannerImpression(Imp.ImpBuilder impBuilder, Banner compatBanner) {
if (compatBanner.getW() == null && compatBanner.getH() == null) {
// As banner.w/h are required fields for adkernel adn platform - take the first format entry
final List<Format> compatBannerFormat = compatBanner.getFormat();

if (CollectionUtils.isEmpty(compatBannerFormat)) {
Expand All @@ -161,32 +162,32 @@ private static Imp compatBannerImpression(Imp.ImpBuilder impBuilder, Banner comp
impBuilder.banner(bannerBuilder.build());
}

return impBuilder.video(null)
.audio(null)
.xNative(null)
.build();
impBuilder.video(null);
}

private List<HttpRequest<BidRequest>> buildAdapterRequests(BidRequest preBidRequest,
Map<ExtImpAdkernelAdn, List<Imp>> pubToImps,
String endpointUrl) {
Map<ExtImpAdkernelAdn, List<Imp>> pubToImps) {
final List<HttpRequest<BidRequest>> result = new ArrayList<>();

for (Map.Entry<ExtImpAdkernelAdn, List<Imp>> entry : pubToImps.entrySet()) {
final BidRequest outgoingRequest = createBidRequest(preBidRequest, entry.getValue());
final String body = mapper.encode(outgoingRequest);
result.add(HttpRequest.<BidRequest>builder()
.method(HttpMethod.POST)
.uri(buildEndpoint(entry.getKey(), endpointUrl))
.body(body)
.headers(headers())
.payload(outgoingRequest)
.build());
result.add(createRequest(entry.getKey(), entry.getValue(), preBidRequest));
}

return result;
}

private HttpRequest<BidRequest> createRequest(ExtImpAdkernelAdn extImp, List<Imp> imps, BidRequest preBidRequest) {
final BidRequest outgoingRequest = createBidRequest(preBidRequest, imps);
final String body = mapper.encode(outgoingRequest);
return HttpRequest.<BidRequest>builder()
.method(HttpMethod.POST)
.uri(buildEndpoint(extImp))
.body(body)
.headers(headers())
.payload(outgoingRequest)
.build();
}

private static BidRequest createBidRequest(BidRequest preBidRequest, List<Imp> imps) {
final BidRequest.BidRequestBuilder bidRequestBuilder = preBidRequest.toBuilder()
.imp(imps);
Expand All @@ -203,27 +204,18 @@ private static BidRequest createBidRequest(BidRequest preBidRequest, List<Imp> i
return bidRequestBuilder.build();
}

/**
* Builds endpoint url based on adapter-specific pub settings from imp.ext.
*/
private static String buildEndpoint(ExtImpAdkernelAdn impExt, String endpointUrl) {
final String updatedEndpointUrl;
private String buildEndpoint(ExtImpAdkernelAdn impExt) {
final String impHost = impExt.getHost();
final String host = StringUtils.isNotBlank(impHost) ? impHost : DEFAULT_DOMAIN;

if (impExt.getHost() != null) {
final URL url;
try {
url = new URL(endpointUrl);
} catch (MalformedURLException e) {
throw new PreBidException(
String.format("Error occurred while parsing AdkernelAdn endpoint url: %s", endpointUrl), e);
}
final String currentHostAndPort = url.getHost() + (url.getPort() == -1 ? "" : ":" + url.getPort());
updatedEndpointUrl = endpointUrl.replace(currentHostAndPort, impExt.getHost());
} else {
updatedEndpointUrl = endpointUrl;
}
return endpointUrl
.replace(URL_HOST_MACRO, host)
.replace(URL_PUBLISHER_ID_MACRO, impExt.getPubId().toString());
}

return String.format("%s%s", updatedEndpointUrl, impExt.getPubId());
private static MultiMap headers() {
return HttpUtil.headers()
.add(HttpUtil.X_OPENRTB_VERSION_HEADER, "2.5");
}

@Override
Expand All @@ -249,14 +241,13 @@ private static List<BidderBid> extractBids(BidRequest bidRequest, BidResponse bi
private static List<BidderBid> bidsFromResponse(BidRequest bidRequest, BidResponse bidResponse) {
return bidResponse.getSeatbid().stream()
.map(SeatBid::getBid)
.filter(Objects::nonNull)
.flatMap(Collection::stream)
.filter(Objects::nonNull)
.map(bid -> BidderBid.of(bid, getType(bid.getImpid(), bidRequest.getImp()), bidResponse.getCur()))
.collect(Collectors.toList());
}

/**
* Figures out which media type this bid is for.
*/
private static BidType getType(String impId, List<Imp> imps) {
for (Imp imp : imps) {
if (imp.getId().equals(impId) && imp.getBanner() != null) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/bidder-config/adkerneladn.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
adapters:
adkerneladn:
enabled: false
endpoint: http://tag.adkernel.com/rtbpub?account=
endpoint: http://{{Host}}/rtbpub?account={{PublisherID}}
pbs-enforces-gdpr: true
pbs-enforces-ccpa: true
modifying-vast-xml-allowed: true
Expand Down
26 changes: 3 additions & 23 deletions src/test/java/org/prebid/server/bidder/adkerneladn/AdkernelAdnBidderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

public class AdkernelAdnBidderTest extends VertxTest {

private static final String ENDPOINT_URL = "http://test.domain.com/rtbpub?account=";
private static final String ENDPOINT_URL = "http://{{Host}}/test?account={{PublisherID}}";

private AdkernelAdnBidder adkernelAdnBidder;

Expand Down Expand Up @@ -149,7 +149,7 @@ public void makeHttpRequestsShouldFillMethodAndUrlAndExpectedHeaders() {
// then
assertThat(result.getValue()).hasSize(1).element(0).isNotNull()
.returns(HttpMethod.POST, HttpRequest::getMethod)
.returns("http://test.domain.com/rtbpub?account=50357", HttpRequest::getUri);
.returns("http://tag.adkernel.com/test?account=50357", HttpRequest::getUri);
assertThat(result.getValue().get(0).getHeaders()).isNotNull()
.extracting(Map.Entry::getKey, Map.Entry::getValue)
.containsOnly(
Expand All @@ -172,27 +172,7 @@ public void makeHttpRequestShouldChangeDomainIfHostIsSpecified() {
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1)
.extracting(HttpRequest::getUri)
.containsOnly("http://different.domanin.com/rtbpub?account=50357");
}

@Test
public void makeHttpRequestShouldRemovePortIfHostIsSpecified() {
// given
final String urlWithPort = "http://test:8080/rtbpub?account=";
adkernelAdnBidder = new AdkernelAdnBidder(urlWithPort, jacksonMapper);

final BidRequest bidRequest = givenBidRequest(
identity(),
extImpAdkernelAdnBuilder -> extImpAdkernelAdnBuilder.host("different.domanin.com"));

// when
final Result<List<HttpRequest<BidRequest>>> result = adkernelAdnBidder.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1)
.extracting(HttpRequest::getUri)
.containsOnly("http://different.domanin.com/rtbpub?account=50357");
.containsOnly("http://different.domanin.com/test?account=50357");
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/prebid/server/it/AdkernelAdnTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class AdkernelAdnTest extends IntegrationTest {
public void openrtb2AuctionShouldRespondWithBidsFromAdkerneladn() throws IOException, JSONException {
// given
// adkernelAdn bid response for imp 021
WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/adkernelAdn-exchange"))
WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/adkernelAdn.tag.adkernel.com"))
.withQueryParam("account", equalTo("101"))
.withHeader("Content-Type", equalToIgnoreCase("application/json;charset=UTF-8"))
.withHeader("Accept", equalTo("application/json"))
Expand All @@ -36,7 +36,7 @@ public void openrtb2AuctionShouldRespondWithBidsFromAdkerneladn() throws IOExcep
jsonFrom("openrtb2/adkerneladn/test-adkerneladn-bid-response-1.json"))));

// adkernelAdn bid response for imp 022
WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/adkernelAdn-exchange"))
WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/adkernelAdn.tag.adkernel.com"))
.withQueryParam("account", equalTo("102"))
.withHeader("Content-Type", equalToIgnoreCase("application/json;charset=UTF-8"))
.withHeader("Accept", equalTo("application/json"))
Expand Down
2 changes: 1 addition & 1 deletion src/test/resources/org/prebid/server/it/test-application.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ adapters.adhese.endpoint=http://localhost:8090/adhese-exchange
adapters.adhese.pbs-enforces-gdpr=true
adapters.adhese.usersync.url=//adhese-usersync
adapters.adkerneladn.enabled=true
adapters.adkerneladn.endpoint=http://localhost:8090/adkernelAdn-exchange?account=
adapters.adkerneladn.endpoint=http://localhost:8090/adkernelAdn.{{Host}}?account={{PublisherID}}
adapters.adkerneladn.pbs-enforces-gdpr=true
adapters.adkerneladn.usersync.url=//adkernelAdn-usersync
adapters.adkernel.enabled=true
Expand Down

0 comments on commit 4aa6ad4

Please sign in to comment.