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

Processed auction request stage #1241

Merged
merged 20 commits into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d226e04
Flesh out machinery for running entrypoint hooks
Apr 9, 2021
479bea1
Unit tests for hooks execution machinery
Apr 13, 2021
0dc62df
Merge branch 'hooks-framework' into hook-execution-plan-impl
Apr 13, 2021
4f270d7
Move HookCatalog up in package hierarchy
Apr 13, 2021
70d303f
Make checkstyle happy
Apr 13, 2021
58d7708
Split hook execution functionality into smaller and manageable pieces
Apr 14, 2021
6e42750
Extend hook execution onto raw auction request stage
Apr 15, 2021
468df11
Move HookCatalog close to other classes responsible for hooks execution
Apr 15, 2021
2f8d55c
Put hooks-related interface implementations to more granular package …
Apr 15, 2021
23f19fb
Implement bidder request hooks execution
Apr 15, 2021
9f5fa60
Implement auction response hooks execution
Apr 16, 2021
3eb289c
Simplify stage executor bootstrapping
Apr 16, 2021
3537364
Check that response phase hooks are not able to cause request rejection
Apr 16, 2021
a942123
Merge branch 'hooks-framework' into hook-execution-plan-impl
Apr 16, 2021
7c9bad3
Implement (raw and processed) bidder response hooks execution
Apr 19, 2021
c08b6ee
Merge remote-tracking branch 'open-source/hooks-framework' into hook-…
DGarbar Apr 21, 2021
7fd1663
Add ProcessedAuctionRequestHook and Execution plan for it
DGarbar Apr 22, 2021
bc91713
Merge remote-tracking branch 'open-source/hooks-framework' into proce…
DGarbar Apr 22, 2021
45be360
Added tests
DGarbar Apr 22, 2021
b6716c1
Migrate ProcessedAuctionRequestStage to the Factories
DGarbar Apr 26, 2021
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
11 changes: 6 additions & 5 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,8 @@ private Future<List<BidderResponse>> invokeProcessedBidderResponseHooks(List<Bid

return CompositeFuture.join(bidderResponses.stream()
.map(bidderResponse -> hookStageExecutor.executeProcessedBidderResponseStage(
bidderResponse,
bidderResponse.getSeatBid().getBids(),
bidderResponse.getBidder(),
auctionContext.getBidRequest(),
auctionContext.getAccount(),
auctionContext.getHookExecutionContext())
Expand Down Expand Up @@ -299,10 +300,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
8 changes: 4 additions & 4 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -1004,10 +1004,10 @@ private Future<BidderResponse> requestBidsOrRejectBidder(
return hookStageResult.isShouldReject()
? Future.succeededFuture(BidderResponse.of(bidderRequest.getBidder(), BidderSeatBid.empty(), 0))
: requestBids(
bidderRequest.with(hookStageResult.getPayload().bidRequest()),
timeout,
debugEnabled,
aliases);
bidderRequest.with(hookStageResult.getPayload().bidRequest()),
timeout,
debugEnabled,
aliases);
}

private BidderResponse rejectBidderResponseOrProceed(HookStageExecutionResult<BidderResponsePayload> stageResult,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ public Future<AuctionContext> fromRequest(RoutingContext routingContext, long st
ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(
auctionContext.getBidRequest(),
auctionContext.getAccount(),
auctionContext.getPrivacyContext())));
auctionContext.getPrivacyContext())))

.compose(auctionContext -> ortb2RequestFactory.executeProcessedAuctionRequestHooks(auctionContext)
.map(auctionContext::with));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ public Future<AuctionContext> fromRequest(RoutingContext routingContext, long st
hookExecutionContext,
errors)))

.compose(auctionContext -> ortb2RequestFactory.executeRawAuctionRequestHooks(auctionContext,
hookExecutionContext)
.compose(auctionContext -> ortb2RequestFactory.executeRawAuctionRequestHooks(auctionContext)
.map(auctionContext::with))

.compose(auctionContext -> updateBidRequest(auctionContext)
Expand All @@ -102,7 +101,10 @@ public Future<AuctionContext> fromRequest(RoutingContext routingContext, long st
ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(
auctionContext.getBidRequest(),
auctionContext.getAccount(),
auctionContext.getPrivacyContext())));
auctionContext.getPrivacyContext())))

.compose(auctionContext -> ortb2RequestFactory.executeProcessedAuctionRequestHooks(auctionContext)
.map(auctionContext::with));
}

private String extractAndValidateBody(RoutingContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,24 @@ public Future<HttpRequestWrapper> executeEntrypointHooks(RoutingContext routingC
.map(stageResult -> toHttpRequest(stageResult, routingContext, hookExecutionContext));
}

public Future<BidRequest> executeRawAuctionRequestHooks(AuctionContext auctionContext,
HookExecutionContext hookExecutionContext) {
public Future<BidRequest> executeRawAuctionRequestHooks(AuctionContext auctionContext) {
final BidRequest bidRequest = auctionContext.getBidRequest();
final Account account = auctionContext.getAccount();
final HookExecutionContext hookExecutionContext = auctionContext.getHookExecutionContext();

return hookStageExecutor.executeRawAuctionRequestStage(bidRequest, account, hookExecutionContext)
.map(stageResult -> toBidRequest(stageResult, hookExecutionContext));
}

public Future<BidRequest> executeProcessedAuctionRequestHooks(AuctionContext auctionContext) {
final BidRequest bidRequest = auctionContext.getBidRequest();
final Account account = auctionContext.getAccount();
final HookExecutionContext hookExecutionContext = auctionContext.getHookExecutionContext();

return hookStageExecutor.executeProcessedAuctionRequestStage(bidRequest, account, hookExecutionContext)
.map(stageResult -> toBidRequest(stageResult, hookExecutionContext));
}

private BidRequest toBidRequest(HookStageExecutionResult<AuctionRequestPayload> stageResult,
HookExecutionContext hookExecutionContext) {
if (stageResult.isShouldReject()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ public Future<WithPodErrors<AuctionContext>> fromRequest(RoutingContext routingC
auctionContext.getAccount(),
auctionContext.getPrivacyContext())))

.compose(auctionContext -> ortb2RequestFactory.executeProcessedAuctionRequestHooks(auctionContext)
.map(auctionContext::with))

.map(auctionContext -> WithPodErrors.of(auctionContext, podErrors));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.prebid.server.hooks.v1.Hook;
import org.prebid.server.hooks.v1.Module;
import org.prebid.server.hooks.v1.auction.AuctionResponseHook;
import org.prebid.server.hooks.v1.auction.ProcessedAuctionRequestHook;
import org.prebid.server.hooks.v1.auction.RawAuctionRequestHook;
import org.prebid.server.hooks.v1.bidder.BidderRequestHook;
import org.prebid.server.hooks.v1.bidder.ProcessedBidderResponseHook;
Expand Down Expand Up @@ -31,6 +32,10 @@ public RawAuctionRequestHook rawAuctionRequestHookBy(String moduleCode, String h
return getHookBy(moduleCode, hookImplCode, RawAuctionRequestHook.class);
}

public ProcessedAuctionRequestHook processedAuctionRequestHookBy(String moduleCode, String hookImplCode) {
return getHookBy(moduleCode, hookImplCode, ProcessedAuctionRequestHook.class);
}

public BidderRequestHook bidderRequestHookBy(String moduleCode, String hookImplCode) {
return getHookBy(moduleCode, hookImplCode, BidderRequestHook.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,23 @@ public Future<HookStageExecutionResult<AuctionRequestPayload>> executeRawAuction
.execute();
}

public Future<HookStageExecutionResult<AuctionRequestPayload>> executeProcessedAuctionRequestStage(
BidRequest bidRequest,
Account account,
HookExecutionContext context) {

final Endpoint endpoint = context.getEndpoint();
final Stage stage = Stage.processed_auction_request;

return this.<AuctionRequestPayload, AuctionInvocationContext>stageExecutor(stage, account, endpoint, context)
.withInitialPayload(AuctionRequestPayloadImpl.of(bidRequest))
.withHookProvider(hookId ->
hookCatalog.processedAuctionRequestHookBy(hookId.getModuleCode(), hookId.getHookImplCode()))
.withInvocationContextProvider(auctionInvocationContextProvider(endpoint, bidRequest, account))
.withRejectAllowed(true)
.execute();
}

public Future<HookStageExecutionResult<BidderRequestPayload>> executeBidderRequestStage(
BidderRequest bidderRequest,
Account account,
Expand Down Expand Up @@ -180,20 +197,6 @@ public Future<HookStageExecutionResult<BidderResponsePayload>> executeProcessedB
.execute();
}

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,
BidRequest bidRequest,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.prebid.server.hooks.v1.auction;

import org.prebid.server.hooks.v1.Hook;

public interface ProcessedAuctionRequestHook extends Hook<AuctionRequestPayload, AuctionInvocationContext> {

}
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,10 @@ public void setUp() {
.willReturn(Future.succeededFuture(VideoStoredDataResult.empty()));
given(idGenerator.getType()).willReturn(IdGeneratorType.none);

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

winningBidComparator = new WinningBidComparator();
clock = Clock.fixed(Instant.ofEpochMilli(1000L), ZoneOffset.UTC);
Expand All @@ -192,7 +191,7 @@ public void setUp() {
public void shouldSkipBidderWhenRejectedByProcessedBidderResponseHooks() {
// given
doAnswer(invocation -> Future.succeededFuture(HookStageExecutionResult.of(true, null)))
.when(hookStageExecutor).executeProcessedBidderResponseStage(any(), any(), any(), any());
.when(hookStageExecutor).executeProcessedBidderResponseStage(any(), any(), any(), any(), any());

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

Expand Down Expand Up @@ -226,7 +225,7 @@ public void shouldPassRequestModifiedByBidderRequestHooks() {
.build(),
video,
"EUR"))))))
.when(hookStageExecutor).executeProcessedBidderResponseStage(any(), any(), any(), any());
.when(hookStageExecutor).executeProcessedBidderResponseStage(any(), any(), any(), any(), any());

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.prebid.server.auction.TimeoutResolver;
import org.prebid.server.auction.model.AuctionContext;
import org.prebid.server.exception.InvalidRequestException;
import org.prebid.server.exception.RejectedRequestException;
import org.prebid.server.geolocation.model.GeoInfo;
import org.prebid.server.metric.MetricName;
import org.prebid.server.model.Endpoint;
Expand Down Expand Up @@ -1598,6 +1599,47 @@ public void shouldPassHookExecutionContextWithAmpEndpoint() {
any());
}

@Test
public void shouldUseBidRequestModifiedByProcessedAuctionRequestHooks() {
// given
givenBidRequest(
builder -> builder.site(Site.builder().domain("example.com").build()).ext(ExtRequest.empty()),
Imp.builder().build());

final BidRequest modifiedBidRequest = BidRequest.builder()
.app(App.builder().bundle("org.company.application").build())
.build();
doAnswer(invocation -> Future.succeededFuture(modifiedBidRequest))
.when(ortb2RequestFactory)
.executeProcessedAuctionRequestHooks(any());

// when
final Future<AuctionContext> result = target.fromRequest(routingContext, 0L);

// then

final BidRequest resultBidRequest = result.result().getBidRequest();
assertThat(resultBidRequest.getSite()).isNull();
assertThat(resultBidRequest.getApp()).isEqualTo(App.builder().bundle("org.company.application").build());
}

@Test
public void shouldReturnFailedFutureIfProcessedAuctionRequestHookRejectRequest() {
// given
givenBidRequest(builder -> builder.ext(ExtRequest.empty()), Imp.builder().build());

doAnswer(invocation -> Future.failedFuture(new RejectedRequestException(null)))
.when(ortb2RequestFactory)
.executeProcessedAuctionRequestHooks(any());

// when
final Future<?> future = target.fromRequest(routingContext, 0L);

// then
assertThat(future.failed()).isTrue();
assertThat(future.cause()).isInstanceOf(RejectedRequestException.class);
}

private void givenBidRequest(
Function<BidRequest.BidRequestBuilder, BidRequest.BidRequestBuilder> bidRequestBuilderCustomizer,
Imp... imps) {
Expand All @@ -1623,6 +1665,10 @@ private void givenBidRequest(

given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any(), any(), any()))
.willAnswer(answerWithFirstArgument());
given(ortb2RequestFactory.executeProcessedAuctionRequestHooks(any()))
.willAnswer(invocation -> Future.succeededFuture(
((AuctionContext) invocation.getArgument(0)).getBidRequest()));

}

private Answer<Object> answerWithFirstArgument() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public void setUp() {

given(ortb2RequestFactory.executeEntrypointHooks(any(), any(), any()))
.willAnswer(invocation -> toHttpRequest(invocation.getArgument(0), invocation.getArgument(1)));
given(ortb2RequestFactory.executeRawAuctionRequestHooks(any(), any()))
given(ortb2RequestFactory.executeRawAuctionRequestHooks(any()))
.willAnswer(invocation -> Future.succeededFuture(
((AuctionContext) invocation.getArgument(0)).getBidRequest()));

Expand All @@ -134,6 +134,9 @@ public void setUp() {

given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any(), any(), any()))
.will(invocationOnMock -> invocationOnMock.getArgument(0));
given(ortb2RequestFactory.executeProcessedAuctionRequestHooks(any()))
.willAnswer(invocation -> Future.succeededFuture(
((AuctionContext) invocation.getArgument(0)).getBidRequest()));

target = new AuctionRequestFactory(
Integer.MAX_VALUE,
Expand Down Expand Up @@ -264,7 +267,7 @@ public void shouldUseBidRequestModifiedByRawAuctionRequestHooks() {
.build();
doAnswer(invocation -> Future.succeededFuture(modifiedBidRequest))
.when(ortb2RequestFactory)
.executeRawAuctionRequestHooks(any(), any());
.executeRawAuctionRequestHooks(any());

// when
target.fromRequest(routingContext, 0L);
Expand All @@ -285,7 +288,48 @@ public void shouldReturnFailedFutureIfRawAuctionRequestHookFailedRequest() {

doAnswer(invocation -> Future.failedFuture(new RejectedRequestException(null)))
.when(ortb2RequestFactory)
.executeRawAuctionRequestHooks(any(), any());
.executeRawAuctionRequestHooks(any());

// when
final Future<?> future = target.fromRequest(routingContext, 0L);

// then
assertThat(future.failed()).isTrue();
assertThat(future.cause()).isInstanceOf(RejectedRequestException.class);
}

@Test
public void shouldUseBidRequestModifiedByProcessedAuctionRequestHooks() {
// given
givenValidBidRequest(BidRequest.builder()
.site(Site.builder().domain("example.com").build())
.build());

final BidRequest modifiedBidRequest = BidRequest.builder()
.app(App.builder().bundle("org.company.application").build())
.build();
doAnswer(invocation -> Future.succeededFuture(modifiedBidRequest))
.when(ortb2RequestFactory)
.executeProcessedAuctionRequestHooks(any());

// when
final Future<AuctionContext> result = target.fromRequest(routingContext, 0L);

// then

final BidRequest resultBidRequest = result.result().getBidRequest();
assertThat(resultBidRequest.getSite()).isNull();
assertThat(resultBidRequest.getApp()).isEqualTo(App.builder().bundle("org.company.application").build());
}

@Test
public void shouldReturnFailedFutureIfProcessedAuctionRequestHookRejectRequest() {
// given
givenValidBidRequest(defaultBidRequest);

doAnswer(invocation -> Future.failedFuture(new RejectedRequestException(null)))
.when(ortb2RequestFactory)
.executeProcessedAuctionRequestHooks(any());

// when
final Future<?> future = target.fromRequest(routingContext, 0L);
Expand Down
Loading