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
+80 −27
Merged

Overwrite Bid Id #616

Show file tree
Hide file tree
Changes from 6 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
56 changes: 53 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 @@ -70,6 +70,8 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -89,9 +91,11 @@ public class BidResponseCreator {
private final String cachePath;
private final String cacheAssetUrlTemplate;
private final StoredRequestProcessor storedRequestProcessor;
private final Consumer<BidResponse> overwriteBidIdConsumer;

public BidResponseCreator(CacheService cacheService, BidderCatalog bidderCatalog, EventsService eventsService,
StoredRequestProcessor storedRequestProcessor, JacksonMapper mapper) {
StoredRequestProcessor storedRequestProcessor,
JacksonMapper mapper, boolean generateBidId) {
apaliy marked this conversation as resolved.
Show resolved Hide resolved
this.cacheService = Objects.requireNonNull(cacheService);
this.bidderCatalog = Objects.requireNonNull(bidderCatalog);
this.eventsService = Objects.requireNonNull(eventsService);
Expand All @@ -100,6 +104,45 @@ public BidResponseCreator(CacheService cacheService, BidderCatalog bidderCatalog
this.cacheAssetUrlTemplate = Objects.requireNonNull(cacheService.getCachedAssetURLTemplate());
this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor);
this.mapper = Objects.requireNonNull(mapper);

if (generateBidId) {
apaliy marked this conversation as resolved.
Show resolved Hide resolved
overwriteBidIdConsumer = overwriteBidIdConsumer();
} else {
overwriteBidIdConsumer = bidderResponses -> {
};
}

}

private Consumer<BidResponse> overwriteBidIdConsumer() {
return bidderResponses -> {
bidderResponses.getSeatbid().stream()
.map(SeatBid::getBid)
.forEach(bids -> bids.stream()
.filter(Objects::nonNull)
.forEach(this::replaceBidId));

};
}

private void replaceBidId(Bid bid) {
final ObjectNode ext = bid.getExt();
final ExtBidPrebid extBidPrebid = parsePrebidExt(ext);
if (extBidPrebid != null) {
extBidPrebid.setBidId(UUID.randomUUID().toString());
try {
bid.setExt(ext.set(PREBID_EXT, mapper.mapper().readTree(mapper.encode(extBidPrebid))));
apaliy marked this conversation as resolved.
Show resolved Hide resolved
} catch (JsonProcessingException e) {
}
apaliy marked this conversation as resolved.
Show resolved Hide resolved
}
}

private ExtBidPrebid parsePrebidExt(ObjectNode ext) {
try {
return mapper.decodeValue(ext.get(PREBID_EXT).toString(), ExtBidPrebid.class);
apaliy marked this conversation as resolved.
Show resolved Hide resolved
} catch (Exception e) {
return null;
}
}

/**
Expand Down Expand Up @@ -514,12 +557,14 @@ private BidResponse toBidResponse(
toExtBidResponse(bidderResponses, bidRequest, cacheResult, videoStoredDataResult,
debugEnabled, bidErrors);

return BidResponse.builder()
BidResponse response = BidResponse.builder()
.id(bidRequest.getId())
.cur(bidRequest.getCur().get(0))
.seatbid(seatBids)
.ext(mapper.mapper().valueToTree(extBidResponse))
.build();
overwriteBidIdConsumer.accept(response);
apaliy marked this conversation as resolved.
Show resolved Hide resolved
return response;
}

private Future<VideoStoredDataResult> videoStoredDataResult(List<Imp> imps, Timeout timeout) {
Expand Down Expand Up @@ -641,7 +686,12 @@ private Bid toBid(BidderBid bidderBid, String bidder, ExtRequestTargeting target
final Video storedVideo = impIdToStoredVideo.get(bid.getImpid());
final Events events = eventsEnabled ? eventsService.createEvent(bid.getId(), account.getId()) : null;
apaliy marked this conversation as resolved.
Show resolved Hide resolved

final ExtBidPrebid prebidExt = ExtBidPrebid.of(bidType, targetingKeywords, cache, storedVideo, events, null);
final ExtBidPrebid prebidExt = ExtBidPrebid.builder().type(bidType)
.targeting(targetingKeywords)
.cache(cache)
.storedRequestAttributes(storedVideo)
.events(events)
.build();
final ExtPrebid<ExtBidPrebid, ObjectNode> bidExt = ExtPrebid.of(prebidExt, bid.getExt());
bid.setExt(mapper.mapper().valueToTree(bidExt));

Expand Down
10 changes: 6 additions & 4 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 @@ -2,18 +2,20 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import com.iab.openrtb.request.Video;
import lombok.AllArgsConstructor;
import lombok.Value;
import lombok.Builder;
import lombok.Data;

import java.util.Map;

/**
* Defines the contract for bidresponse.seatbid.bid[i].ext.prebid
*/
@AllArgsConstructor(staticName = "of")
@Value
@Builder(toBuilder = true)
@Data
apaliy marked this conversation as resolved.
Show resolved Hide resolved
public class ExtBidPrebid {

String bidId;

BidType type;

Map<String, String> targeting;
Expand Down
6 changes: 4 additions & 2 deletions 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 @@ -406,9 +406,11 @@ BidResponseCreator bidResponseCreator(
BidderCatalog bidderCatalog,
EventsService eventsService,
StoredRequestProcessor storedRequestProcessor,
JacksonMapper mapper) {
JacksonMapper mapper,
@Value("${auction.generate-bid-id}") Boolean generateBidId) {

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

@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
44 changes: 38 additions & 6 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 @@ -86,8 +86,7 @@
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.prebid.server.proto.openrtb.ext.response.BidType.banner;
import static org.prebid.server.proto.openrtb.ext.response.BidType.xNative;
import static org.prebid.server.proto.openrtb.ext.response.BidType.*;
apaliy marked this conversation as resolved.
Show resolved Hide resolved

public class BidResponseCreatorTest extends VertxTest {

Expand Down Expand Up @@ -120,8 +119,7 @@ public void setUp() {
.willReturn(Future.succeededFuture(VideoStoredDataResult.empty()));

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

timeout = new TimeoutFactory(Clock.fixed(Instant.now(), ZoneId.systemDefault())).create(500);
}
Expand Down Expand Up @@ -371,6 +369,40 @@ public void shouldSkipBidderResponsesWhereSeatBidContainEmptyBids() {
verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any());
}

@Test
public void shouldOverwriteBidderIdToUUID() throws JsonProcessingException {
// given
final BidRequest bidRequest = givenBidRequest();

final ExtPrebid<ExtBidPrebid, ?> prebid = ExtPrebid.of(ExtBidPrebid.builder().type(banner).build(), null);
final Bid bid = Bid.builder()
.id("123")
.impid("imp123")
.w(123)
.h(123)
.price(BigDecimal.ONE)
apaliy marked this conversation as resolved.
Show resolved Hide resolved
.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, jacksonMapper, true);
// when
final BidResponse bidResponse = bidResponseCreator.create(bidderResponses, bidRequest,
null, CACHE_INFO, ACCOUNT, timeout, false).result();

// then
assertThat(bidResponse.getSeatbid()).hasSize(1);
assertThat(bidResponse.getSeatbid().get(0).getBid()).hasSize(1);
apaliy marked this conversation as resolved.
Show resolved Hide resolved
final ExtBidPrebid ext = mapper.readValue(bidResponse.getSeatbid().get(0).getBid().get(0)
apaliy marked this conversation as resolved.
Show resolved Hide resolved
.getExt().get("prebid").toString(), ExtBidPrebid.class);
assertThat(ext.getBidId()).isNotNull();
apaliy marked this conversation as resolved.
Show resolved Hide resolved

System.out.println(ext.getBidId());
apaliy marked this conversation as resolved.
Show resolved Hide resolved
verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any());
}

@Test
public void shouldSetExpectedResponseSeatBidAndBidFields() {
// given
Expand All @@ -394,7 +426,7 @@ 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.builder().type(banner).build(), singletonMap("bidExt", 1))))
.build()))
.build());

Expand Down Expand Up @@ -529,7 +561,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.builder().type(banner).build(), null)))
.build());

verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), 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 @@ -1769,7 +1769,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.builder().build(), 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 @@ -553,7 +553,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.builder().type(BidType.video).build();

final List<SeatBid> seatBid = singletonList(SeatBid.builder()
.seat("rubicon").bid(singletonList(Bid.builder().ext(mapper.createObjectNode()
Expand Down
6 changes: 3 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,19 @@ 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.builder().targeting(targeting).build(),
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.builder().targeting(targeting).build(),
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.builder().build(), mapper.createObjectNode())))
.build();

final BidResponse bidResponse = BidResponse.builder()
Expand Down
28 changes: 14 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 @@ -349,7 +349,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))));
mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.builder().targeting(targeting).build(), null))));

// when
ampHandler.handle(routingContext);
Expand Down Expand Up @@ -379,7 +379,7 @@ 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.builder().targeting(targeting).build(),
mapper.createObjectNode())))
.build()))
.build()))
Expand Down Expand Up @@ -459,7 +459,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.builder().build(), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -476,7 +476,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.builder().build(), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -493,7 +493,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.builder().build(), null))));

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

Expand All @@ -516,7 +516,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.builder().build(), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -535,7 +535,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.builder().build(), null))));

// when
ampHandler.handle(routingContext);
Expand Down Expand Up @@ -619,7 +619,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.builder().build(), null))));

// simulate calling exception handler that is supposed to update networkerr timer value
given(httpResponse.exceptionHandler(any())).willAnswer(inv -> {
Expand All @@ -642,7 +642,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.builder().build(), null))));

// when
ampHandler.handle(routingContext);
Expand All @@ -659,7 +659,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.builder().build(), null))));

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

Expand Down Expand Up @@ -726,8 +726,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.builder().targeting(singletonMap("hb_cache_id_bidder1", "value1"))
.build(),
null))));

// when
Expand All @@ -745,8 +745,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.builder().targeting(singletonMap("hb_cache_id_bidder1", "value1"))
.build(),
null)))
.build()))
.build()))
Expand Down