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

Insert processed bidder response hooks invocation logic #1234

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 46 additions & 12 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.iab.openrtb.response.BidResponse;
import com.iab.openrtb.response.Response;
import com.iab.openrtb.response.SeatBid;
import io.vertx.core.CompositeFuture;
import io.vertx.core.Future;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.collections4.ListUtils;
Expand Down Expand Up @@ -43,6 +44,9 @@
import org.prebid.server.exception.InvalidRequestException;
import org.prebid.server.exception.PreBidException;
import org.prebid.server.execution.Timeout;
import org.prebid.server.hooks.execution.HookStageExecutor;
import org.prebid.server.hooks.execution.model.HookStageExecutionResult;
import org.prebid.server.hooks.v1.bidder.BidderResponsePayload;
import org.prebid.server.identity.IdGenerator;
import org.prebid.server.identity.IdGeneratorType;
import org.prebid.server.json.DecodeException;
Expand Down Expand Up @@ -108,6 +112,7 @@ public class BidResponseCreator {
private final EventsService eventsService;
private final StoredRequestProcessor storedRequestProcessor;
private final IdGenerator bidIdGenerator;
private final HookStageExecutor hookStageExecutor;
private final int truncateAttrChars;
private final Clock clock;
private final JacksonMapper mapper;
Expand All @@ -124,6 +129,7 @@ public BidResponseCreator(CacheService cacheService,
StoredRequestProcessor storedRequestProcessor,
WinningBidComparator winningBidComparator,
IdGenerator bidIdGenerator,
HookStageExecutor hookStageExecutor,
int truncateAttrChars,
Clock clock,
JacksonMapper mapper) {
Expand All @@ -135,6 +141,7 @@ public BidResponseCreator(CacheService cacheService,
this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor);
this.winningBidComparator = Objects.requireNonNull(winningBidComparator);
this.bidIdGenerator = Objects.requireNonNull(bidIdGenerator);
this.hookStageExecutor = Objects.requireNonNull(hookStageExecutor);
this.truncateAttrChars = validateTruncateAttrChars(truncateAttrChars);
this.clock = Objects.requireNonNull(clock);
this.mapper = Objects.requireNonNull(mapper);
Expand Down Expand Up @@ -185,14 +192,15 @@ Future<BidResponse> create(List<BidderResponse> bidderResponses,
.build());
}

return cacheBidsAndCreateResponse(
bidderResponses,
auctionContext,
cacheInfo,
bidderToMultiBids,
bidIdToGeneratedBidId,
eventsContext,
debugEnabled);
return invokeProcessedBidderResponseHooks(bidderResponses, auctionContext)
.compose(updatedBidderResponses -> cacheBidsAndCreateResponse(
updatedBidderResponses,
auctionContext,
cacheInfo,
bidderToMultiBids,
bidIdToGeneratedBidId,
eventsContext,
debugEnabled));
}

private void updateBidAdmInBidderResponses(List<BidderResponse> bidderResponses,
Expand Down Expand Up @@ -241,6 +249,32 @@ private static boolean isEmptyBidderResponses(List<BidderResponse> bidderRespons
.allMatch(CollectionUtils::isEmpty);
}

private Future<List<BidderResponse>> invokeProcessedBidderResponseHooks(List<BidderResponse> bidderResponses,
AuctionContext auctionContext) {

return CompositeFuture.join(bidderResponses.stream()
.map(bidderResponse -> hookStageExecutor.executeProcessedBidderResponseStage(
bidderResponse,
auctionContext.getBidRequest(),
auctionContext.getAccount(),
auctionContext.getHookExecutionContext())
.map(stageResult -> rejectBidderResponseOrProceed(stageResult, bidderResponse)))
.collect(Collectors.toList()))
.map(CompositeFuture::list);
}

private static BidderResponse rejectBidderResponseOrProceed(
HookStageExecutionResult<BidderResponsePayload> stageResult,
BidderResponse bidderResponse) {

final List<BidderBid> bids =
stageResult.isShouldReject() ? Collections.emptyList() : stageResult.getPayload().bids();

return bidderResponse
.with(bidderResponse.getSeatBid()
.with(bids));
}

private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidderResponses,
AuctionContext auctionContext,
BidRequestCacheInfo cacheInfo,
Expand All @@ -265,10 +299,10 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
final Set<BidInfo> winningBidInfos = targeting == null
? null
: bidderResponseToTargetingBidInfos.values().stream()
.flatMap(Collection::stream)
.filter(TargetingBidInfo::isWinningBid)
.map(TargetingBidInfo::getBidInfo)
.collect(Collectors.toSet());
.flatMap(Collection::stream)
.filter(TargetingBidInfo::isWinningBid)
.map(TargetingBidInfo::getBidInfo)
.collect(Collectors.toSet());

final Set<BidInfo> bidsToCache = cacheInfo.isShouldCacheWinningBidsOnly() ? winningBidInfos : bidInfos;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.prebid.server.bidder.model;

import lombok.AllArgsConstructor;
import lombok.Value;
import org.prebid.server.bidder.Bidder;
import org.prebid.server.proto.openrtb.ext.response.ExtHttpCall;
Expand All @@ -14,8 +13,7 @@
* This is distinct from the {@link com.iab.openrtb.response.SeatBid} so that the prebid-server ext can be passed
* back with type safety.
*/
@AllArgsConstructor(staticName = "of")
@Value
@Value(staticConstructor = "of")
public class BidderSeatBid {

/**
Expand Down Expand Up @@ -43,6 +41,10 @@ public class BidderSeatBid {
*/
List<BidderError> errors;

public BidderSeatBid with(List<BidderBid> bids) {
return BidderSeatBid.of(bids, this.getHttpCalls(), this.getErrors());
}

public static BidderSeatBid empty() {
return of(Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import io.vertx.core.MultiMap;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.auction.model.BidderRequest;
import org.prebid.server.auction.model.BidderResponse;
import org.prebid.server.bidder.model.BidderBid;
import org.prebid.server.hooks.execution.model.EndpointExecutionPlan;
import org.prebid.server.hooks.execution.model.ExecutionPlan;
import org.prebid.server.hooks.execution.model.HookExecutionContext;
Expand All @@ -15,13 +17,15 @@
import org.prebid.server.hooks.v1.auction.AuctionRequestPayload;
import org.prebid.server.hooks.v1.auction.AuctionResponsePayload;
import org.prebid.server.hooks.v1.bidder.BidderRequestPayload;
import org.prebid.server.hooks.v1.bidder.BidderResponsePayload;
import org.prebid.server.hooks.v1.entrypoint.EntrypointPayload;
import org.prebid.server.json.DecodeException;
import org.prebid.server.json.JacksonMapper;
import org.prebid.server.model.Endpoint;
import org.prebid.server.settings.model.Account;
import org.prebid.server.settings.model.AccountHooksConfiguration;

import java.util.List;
import java.util.Objects;

public class HookStageExecutor {
Expand Down Expand Up @@ -80,6 +84,20 @@ public BidRequest bidRequest() {
}));
}

public Future<HookStageExecutionResult<BidderResponsePayload>> executeProcessedBidderResponseStage(
BidderResponse bidderResponse,
BidRequest bidRequest,
Account account,
HookExecutionContext context) {

return Future.succeededFuture(HookStageExecutionResult.of(false, new BidderResponsePayload() {
@Override
public List<BidderBid> bids() {
return bidderResponse.getSeatBid().getBids();
}
}));
}

public Future<HookStageExecutionResult<AuctionResponsePayload>> executeAuctionResponseStage(
BidResponse bidResponse,
HookExecutionContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@ BidResponseCreator bidResponseCreator(
StoredRequestProcessor storedRequestProcessor,
WinningBidComparator winningBidComparator,
IdGenerator bidIdGenerator,
HookStageExecutor hookStageExecutor,
@Value("${settings.targeting.truncate-attr-chars}") int truncateAttrChars,
Clock clock,
JacksonMapper mapper) {
Expand All @@ -505,6 +506,7 @@ BidResponseCreator bidResponseCreator(
storedRequestProcessor,
winningBidComparator,
bidIdGenerator,
hookStageExecutor,
truncateAttrChars,
clock,
mapper);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import com.iab.openrtb.response.Response;
import com.iab.openrtb.response.SeatBid;
import io.vertx.core.Future;
import lombok.Value;
import lombok.experimental.Accessors;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -44,6 +46,9 @@
import org.prebid.server.events.EventsService;
import org.prebid.server.execution.Timeout;
import org.prebid.server.execution.TimeoutFactory;
import org.prebid.server.hooks.execution.HookStageExecutor;
import org.prebid.server.hooks.execution.model.HookStageExecutionResult;
import org.prebid.server.hooks.v1.bidder.BidderResponsePayload;
import org.prebid.server.identity.IdGenerator;
import org.prebid.server.identity.IdGeneratorType;
import org.prebid.server.proto.openrtb.ext.ExtPrebid;
Expand Down Expand Up @@ -105,6 +110,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidAdservertargetingRule.Source.xStatic;
Expand Down Expand Up @@ -136,6 +142,8 @@ public class BidResponseCreatorTest extends VertxTest {
private StoredRequestProcessor storedRequestProcessor;
@Mock
private IdGenerator idGenerator;
@Mock
private HookStageExecutor hookStageExecutor;

private WinningBidComparator winningBidComparator;

Expand All @@ -155,6 +163,12 @@ public void setUp() {
.willReturn(Future.succeededFuture(VideoStoredDataResult.empty()));
given(idGenerator.getType()).willReturn(IdGeneratorType.none);

given(hookStageExecutor.executeProcessedBidderResponseStage(any(), any(), any(), any()))
.willAnswer(invocation -> Future.succeededFuture(HookStageExecutionResult.of(
false,
BidderResponsePayloadImpl.of(
invocation.<BidderResponse>getArgument(0).getSeatBid().getBids()))));

winningBidComparator = new WinningBidComparator();
clock = Clock.fixed(Instant.ofEpochMilli(1000L), ZoneOffset.UTC);

Expand All @@ -166,13 +180,75 @@ public void setUp() {
storedRequestProcessor,
winningBidComparator,
idGenerator,
hookStageExecutor,
0,
clock,
jacksonMapper);

timeout = new TimeoutFactory(Clock.fixed(Instant.now(), ZoneId.systemDefault())).create(500);
}

@Test
public void shouldSkipBidderWhenRejectedByProcessedBidderResponseHooks() {
// given
doAnswer(invocation -> Future.succeededFuture(HookStageExecutionResult.of(true, null)))
.when(hookStageExecutor).executeProcessedBidderResponseStage(any(), any(), any(), any());

final AuctionContext auctionContext = givenAuctionContext(givenBidRequest(givenImp()));

final Bid bid = Bid.builder()
.id("bidId1")
.impid(IMP_ID)
.price(BigDecimal.valueOf(5.67))
.build();
final List<BidderResponse> bidderResponses = singletonList(
BidderResponse.of("bidder1", givenSeatBid(BidderBid.of(bid, banner, "USD")), 100));

// when
final BidResponse bidResponse =
bidResponseCreator.create(bidderResponses, auctionContext, CACHE_INFO, MULTI_BIDS, false).result();

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid)
.isEmpty();
}

@Test
public void shouldPassRequestModifiedByBidderRequestHooks() {
doAnswer(invocation -> Future.succeededFuture(HookStageExecutionResult.of(
false,
BidderResponsePayloadImpl.of(singletonList(BidderBid.of(
Bid.builder()
.id("bidIdModifiedByHook")
.impid(IMP_ID)
.price(BigDecimal.valueOf(1.23))
.build(),
video,
"EUR"))))))
.when(hookStageExecutor).executeProcessedBidderResponseStage(any(), any(), any(), any());

final AuctionContext auctionContext = givenAuctionContext(givenBidRequest(givenImp()));

final Bid bid = Bid.builder()
.id("bidId1")
.impid(IMP_ID)
.price(BigDecimal.valueOf(5.67))
.build();
final List<BidderResponse> bidderResponses = singletonList(
BidderResponse.of("bidder1", givenSeatBid(BidderBid.of(bid, banner, "USD")), 100));

// when
final BidResponse bidResponse =
bidResponseCreator.create(bidderResponses, auctionContext, CACHE_INFO, MULTI_BIDS, false).result();

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid)
.extracting(Bid::getId, Bid::getImpid, Bid::getPrice)
.containsOnly(tuple("bidIdModifiedByHook", IMP_ID, BigDecimal.valueOf(1.23)));
}

@Test
public void shouldPassOriginalTimeoutToCacheServiceIfCachingIsRequested() {
// given
Expand Down Expand Up @@ -1055,6 +1131,7 @@ public void shouldTruncateTargetingKeywordsByGlobalConfig() {
storedRequestProcessor,
winningBidComparator,
idGenerator,
hookStageExecutor,
20,
clock,
jacksonMapper);
Expand Down Expand Up @@ -2336,4 +2413,11 @@ private static ObjectNode extWithTargeting(String targetBidderCode, Map<String,
private static <T> List<T> mutableList(T... values) {
return Arrays.stream(values).collect(Collectors.toList());
}

@Accessors(fluent = true)
@Value(staticConstructor = "of")
private static class BidderResponsePayloadImpl implements BidderResponsePayload {

List<BidderBid> bids;
}
}