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.

Sign up for GitHub

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

Overwrite Bid Id #616

Merged
merged 17 commits into from
May 20, 2020
Merged

Overwrite Bid Id #616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
06f93c6
Bid Id can be overwritten if application.yaml auction.generate-bid-id…
apaliy Feb 7, 2020
99ee0ef
Merge branch 'master' into pbs-id-generation
apaliy Feb 7, 2020
df3ef9f
Merge branch 'master' into pbs-id-generation
apaliy Feb 7, 2020
0e9eea0
Overwriting exp.prebid.bid_id
apaliy Mar 16, 2020
81ad0a7
Overwriting exp.prebid.bid_id
apaliy Mar 16, 2020
91f51d5
Overwriting exp.prebid.bid_id
apaliy Mar 16, 2020
76140ca
PR fix
apaliy Mar 24, 2020
485e411
test fix
apaliy Mar 24, 2020
85dbf6e
applying change request
apaliy Mar 26, 2020
61471de
change request
apaliy Mar 30, 2020
813a7d1
change request
apaliy Mar 31, 2020
42c412e
simplify code
apaliy Mar 31, 2020
fb1cef4
simplify code
apaliy Mar 31, 2020
40eae2a
Merge branch 'master' into pbs-id-generation
May 8, 2020
87ee7d1
Minor changes
May 8, 2020
5273bac
Fix org.prebid.server.proto.openrtb.ext.response.ExtBidPrebid.bidid
May 8, 2020
9867e30
Fix tests
May 8, 2020
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
15 changes: 12 additions & 3 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.commons.collections4.ListUtils;
import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.prebid.server.auction.model.BidRequestCacheInfo;
import org.prebid.server.auction.model.BidderResponse;
import org.prebid.server.bidder.BidderCatalog;
Expand Down Expand Up @@ -72,6 +73,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -86,18 +88,21 @@ public class BidResponseCreator {
private final BidderCatalog bidderCatalog;
private final EventsService eventsService;
private final StoredRequestProcessor storedRequestProcessor;
private final boolean generateBidId;
private final JacksonMapper mapper;

private final String cacheHost;
private final String cachePath;
private final String cacheAssetUrlTemplate;

public BidResponseCreator(CacheService cacheService, BidderCatalog bidderCatalog, EventsService eventsService,
StoredRequestProcessor storedRequestProcessor, JacksonMapper mapper) {
StoredRequestProcessor storedRequestProcessor,
boolean generateBidId, JacksonMapper mapper) {
this.cacheService = Objects.requireNonNull(cacheService);
this.bidderCatalog = Objects.requireNonNull(bidderCatalog);
this.eventsService = Objects.requireNonNull(eventsService);
this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor);
this.generateBidId = generateBidId;
this.mapper = Objects.requireNonNull(mapper);

cacheHost = Objects.requireNonNull(cacheService.getEndpointHost());
Expand Down Expand Up @@ -658,12 +663,16 @@ private Bid toBid(BidderBid bidderBid, String bidder, ExtRequestTargeting target
cache = null;
}

final String generatedBidId = generateBidId ? UUID.randomUUID().toString() : null;
final String eventBidId = ObjectUtils.defaultIfNull(generatedBidId, bid.getId());
final Video storedVideo = impIdToStoredVideo.get(bid.getImpid());
final Events events = eventsEnabled && eventsAllowedByRequest
? eventsService.createEvent(bid.getId(), bidder, account.getId(), auctionTimestamp)
? eventsService.createEvent(eventBidId, bidder, account.getId(), auctionTimestamp)
: null;

final ExtBidPrebid prebidExt = ExtBidPrebid.of(bidType, targetingKeywords, cache, storedVideo, events, null);
final ExtBidPrebid prebidExt = ExtBidPrebid.of(
generatedBidId, bidType, targetingKeywords, cache, storedVideo, events, null);

final ExtPrebid<ExtBidPrebid, ObjectNode> bidExt = ExtPrebid.of(prebidExt, bid.getExt());
bid.setExt(mapper.mapper().valueToTree(bidExt));

Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
@Value
public class ExtBidPrebid {

String bidid;

BidType type;

Map<String, String> targeting;
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,9 +469,11 @@ BidResponseCreator bidResponseCreator(
BidderCatalog bidderCatalog,
EventsService eventsService,
StoredRequestProcessor storedRequestProcessor,
@Value("${auction.generate-bid-id}") boolean generateBidId,
JacksonMapper mapper) {

return new BidResponseCreator(cacheService, bidderCatalog, eventsService, storedRequestProcessor, mapper);
return new BidResponseCreator(cacheService, bidderCatalog, eventsService, storedRequestProcessor,
generateBidId, mapper);
}

@Bean
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ auction:
timeout-adjustment-ms: 30
stored-requests-timeout-ms: 50
max-request-size: 262144
generate-bid-id: false
cache:
expected-request-time-ms: 10
only-winning-bids: false
Expand Down
45 changes: 41 additions & 4 deletions src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
Expand Down Expand Up @@ -125,7 +126,7 @@ public void setUp() {
.willReturn(Future.succeededFuture(VideoStoredDataResult.empty()));

bidResponseCreator = new BidResponseCreator(cacheService, bidderCatalog, eventsService, storedRequestProcessor,
jacksonMapper);
false, jacksonMapper);

timeout = new TimeoutFactory(Clock.fixed(Instant.now(), ZoneId.systemDefault())).create(500);
}
Expand Down Expand Up @@ -221,7 +222,7 @@ public void shouldRequestCacheServiceWithWinningBidsOnlyWhenWinningonlyIsTrue()
// just a stub to get through method call chain
givenCacheServiceResult(singletonMap(bid1, CacheIdInfo.of(null, null)));

// when\
// when
bidResponseCreator.create(bidderResponses, bidRequest, targeting, cacheInfo, ACCOUNT, false, 1000L, false,
timeout);

Expand Down Expand Up @@ -414,6 +415,41 @@ public void shouldSkipBidderResponsesWhereSeatBidContainEmptyBids() {
verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any());
}

@Test
public void shouldOverrideBidIdWhenGenerateBidIdIsTurnedOn() {
// given
final BidRequest bidRequest = givenBidRequest();

final ExtPrebid<ExtBidPrebid, ?> prebid = ExtPrebid.of(ExtBidPrebid.of(null, banner, null,
null, null, null, null), null);
final Bid bid = Bid.builder()
.id("123")
.impid("imp123")
.ext(mapper.valueToTree(prebid))
.build();
final List<BidderResponse> bidderResponses = singletonList(
BidderResponse.of("bidder2", givenSeatBid(BidderBid.of(bid, banner, "USD")), 0));

final BidResponseCreator bidResponseCreator = new BidResponseCreator(cacheService, bidderCatalog, eventsService,
storedRequestProcessor, true, jacksonMapper);
// when
final BidResponse bidResponse = bidResponseCreator.create(bidderResponses, bidRequest,
null, CACHE_INFO, ACCOUNT, false, 1000L, false, timeout).result();

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid)
.extracting(Bid::getExt)
.extracting(ext -> ext.get("prebid"))
.extracting(extPrebid -> mapper.treeToValue(extPrebid, ExtBidPrebid.class))
.extracting(ExtBidPrebid::getBidid)
.hasSize(1)
.first()
.satisfies(bidId -> assertThat(UUID.fromString(bidId)).isInstanceOf(UUID.class));

verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any());
}

@Test
public void shouldSetExpectedResponseSeatBidAndBidFields() {
// given
Expand All @@ -437,7 +473,8 @@ public void shouldSetExpectedResponseSeatBidAndBidFields() {
.price(BigDecimal.ONE)
.adm("adm")
.ext(mapper.valueToTree(ExtPrebid.of(
ExtBidPrebid.of(banner, null, null, null, null, null), singletonMap("bidExt", 1))))
ExtBidPrebid.of(null, banner, null, null, null, null, null),
singletonMap("bidExt", 1))))
.build()))
.build());

Expand Down Expand Up @@ -572,7 +609,7 @@ public void shouldTolerateMissingExtInSeatBidAndBid() {
.id("bidId")
.price(BigDecimal.ONE)
.ext(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(banner, null, null, null, null, null), null)))
ExtPrebid.of(ExtBidPrebid.of(null, banner, null, null, null, null, null), null)))
.build());

verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any());
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/prebid/server/auction/ExchangeServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1883,7 +1883,7 @@ private static Bid givenBid(Function<Bid.BidBuilder, Bid.BidBuilder> bidBuilder)
return bidBuilder.apply(Bid.builder()
.id("bidId")
.price(BigDecimal.ONE)
.ext(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))))
.ext(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))))
.build();
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ public void mergeWithBidderResponsesShouldResolveBidTypeFromStoredBidExt() {
singletonList(BidderBid.of(Bid.builder().id("bid1").build(), BidType.banner, "USD")), emptyList(),
emptyList()), 100));

final ExtBidPrebid extBidPrebid = ExtBidPrebid.of(BidType.video, null, null, null, null, null);
final ExtBidPrebid extBidPrebid = ExtBidPrebid.of(null, BidType.video, null, null, null, null, null);

final List<SeatBid> seatBid = singletonList(SeatBid.builder()
.seat("rubicon").bid(singletonList(Bid.builder().ext(mapper.createObjectNode()
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,20 @@ public void shouldReturnExpectedVideoResponse() {
final Bid bid0 = Bid.builder()
.impid("0_0")
.ext(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, targeting, null, null, null, null),
ExtPrebid.of(ExtBidPrebid.of(null, null, targeting, null, null, null, null),
mapper.createObjectNode())))
.build();
final Bid bid1 = Bid.builder()
.impid("1_1")
.ext(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, targeting, null, null, null, null),
ExtPrebid.of(ExtBidPrebid.of(null, null, targeting, null, null, null, null),
mapper.createObjectNode())))
.build();
final Bid bid2 = Bid.builder()
.impid("2_1")
.ext(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), mapper.createObjectNode())))
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null),
mapper.createObjectNode())))
.build();

final BidResponse bidResponse = BidResponse.builder()
Expand Down
29 changes: 15 additions & 14 deletions src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ public void shouldRespondWithExpectedResponse() {
targeting.put("hb_cache_id_bidder1", "value2");
given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of(
null, targeting, null, null, null, null), null))));
null, null, targeting, null, null, null, null), null))));

// when
ampHandler.handle(routingContext);
Expand Down Expand Up @@ -377,7 +377,8 @@ public void shouldRespondWithCustomTargetingIncluded() {
.seat("bidder1")
.bid(singletonList(Bid.builder()
.ext(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, targeting, null, null, null, null),
ExtPrebid.of(
ExtBidPrebid.of(null, null, targeting, null, null, null, null),
mapper.createObjectNode())))
.build()))
.build()))
Expand Down Expand Up @@ -457,7 +458,7 @@ public void shouldIncrementOkAmpRequestMetrics() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -474,7 +475,7 @@ public void shouldIncrementAppRequestMetrics() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -491,7 +492,7 @@ public void shouldIncrementNoCookieMetrics() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

given(uidsCookie.hasLiveUids()).willReturn(false);

Expand All @@ -514,7 +515,7 @@ public void shouldIncrementImpsRequestedMetrics() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -533,7 +534,7 @@ public void shouldIncrementImpsTypesMetrics() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

// when
ampHandler.handle(routingContext);
Expand Down Expand Up @@ -617,7 +618,7 @@ public void shouldUpdateNetworkErrorMetric() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

// simulate calling exception handler that is supposed to update networkerr timer value
given(httpResponse.exceptionHandler(any())).willAnswer(inv -> {
Expand All @@ -640,7 +641,7 @@ public void shouldNotUpdateNetworkErrorMetricIfResponseSucceeded() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -657,7 +658,7 @@ public void shouldUpdateNetworkErrorMetricIfClientClosedConnection() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null))));
ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null))));

given(routingContext.response().closed()).willReturn(true);

Expand Down Expand Up @@ -724,8 +725,8 @@ public void shouldPassSuccessfulEventToAnalyticsReporter() {

given(exchangeService.holdAuction(any()))
.willReturn(givenBidResponse(mapper.valueToTree(
ExtPrebid.of(ExtBidPrebid.of(null, singletonMap("hb_cache_id_bidder1", "value1"), null, null,
null, null),
ExtPrebid.of(ExtBidPrebid.of(null, null, singletonMap("hb_cache_id_bidder1", "value1"),
null, null, null, null),
null))));

// when
Expand All @@ -743,8 +744,8 @@ public void shouldPassSuccessfulEventToAnalyticsReporter() {
.bidResponse(BidResponse.builder().seatbid(singletonList(SeatBid.builder()
.bid(singletonList(Bid.builder()
.ext(mapper.valueToTree(ExtPrebid.of(
ExtBidPrebid.of(null, singletonMap("hb_cache_id_bidder1", "value1"), null,
null, null, null),
ExtBidPrebid.of(null, null, singletonMap("hb_cache_id_bidder1", "value1"),
null, null, null, null),
null)))
.build()))
.build()))
Expand Down