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

Updating admixer bidder #1401

Merged
merged 19 commits into from
Sep 24, 2021
Merged

Updating admixer bidder #1401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4a47351
Adding headers to vtrack and status
javaAndScriptDeveloper Aug 2, 2021
375223b
This commit added headers to /vtrack and /status
javaAndScriptDeveloper Aug 2, 2021
d65aeba
Working
javaAndScriptDeveloper Aug 3, 2021
1a93c52
Continue working progress
javaAndScriptDeveloper Aug 3, 2021
3ce34b5
Updated tests and moved adding headers to lambdas
javaAndScriptDeveloper Aug 3, 2021
3236a95
Updated admixer bidder and admixer.json
javaAndScriptDeveloper Aug 4, 2021
62a5aec
microfix
javaAndScriptDeveloper Aug 5, 2021
deeee79
added 2 tests
javaAndScriptDeveloper Aug 5, 2021
f02b043
Fixed and added new tests
javaAndScriptDeveloper Aug 6, 2021
d04a855
Cropped unnecessary
javaAndScriptDeveloper Aug 10, 2021
dc3c31a
Merge branch 'master' into updatingAdmixerBidder
javaAndScriptDeveloper Aug 10, 2021
457a004
Update VtrackHandlerTest.java
javaAndScriptDeveloper Aug 11, 2021
094401a
Update StatusHandlerTest.java
javaAndScriptDeveloper Aug 11, 2021
3f7c8a6
Update StatusHandlerTest.java
javaAndScriptDeveloper Aug 11, 2021
5ace28c
Fixed
javaAndScriptDeveloper Aug 13, 2021
fc4f9e3
fixed
javaAndScriptDeveloper Aug 13, 2021
5d3aa28
fixed resolveBidFloor method
javaAndScriptDeveloper Aug 20, 2021
b94cf67
minor fixes
javaAndScriptDeveloper Sep 13, 2021
9f14274
Merge branch 'master' into updatingAdmixerBidder
Sep 24, 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
20 changes: 16 additions & 4 deletions src/main/java/org/prebid/server/bidder/admixer/AdmixerBidder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.iab.openrtb.response.SeatBid;
import io.vertx.core.http.HttpMethod;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.bidder.Bidder;
import org.prebid.server.bidder.model.BidderBid;
Expand Down Expand Up @@ -85,23 +86,34 @@ private ExtImpAdmixer parseImpExt(Imp imp) {
} catch (IllegalArgumentException e) {
throw new PreBidException(String.format("Wrong Admixer bidder ext in imp with id : %s", imp.getId()));
}
if (StringUtils.length(extImpAdmixer.getZone()) != 36) {
final String zoneId = extImpAdmixer.getZone();

if (StringUtils.length(zoneId) < 32 || StringUtils.length(zoneId) > 36) {
throw new PreBidException("ZoneId must be UUID/GUID");
}

return extImpAdmixer;
}

private Imp processImp(Imp imp, ExtImpAdmixer extImpAdmixer) {
final Double extImpFloor = extImpAdmixer.getCustomFloor();
final BigDecimal customFloor = extImpFloor != null ? BigDecimal.valueOf(extImpFloor) : BigDecimal.ZERO;
return imp.toBuilder()
.tagid(extImpAdmixer.getZone())
.bidfloor(customFloor)
.bidfloor(resolveBidFloor(extImpAdmixer.getCustomFloor(), imp.getBidfloor()))
.ext(makeImpExt(extImpAdmixer.getCustomParams()))
.build();
}

private static BigDecimal resolveBidFloor(BigDecimal customBidFloor, BigDecimal bidFloor) {
final BigDecimal resolvedCustomBidFloor = isValidBidFloor(customBidFloor) ? customBidFloor : null;
final BigDecimal resolvedBidFloor = isValidBidFloor(bidFloor) ? bidFloor : null;

return ObjectUtils.defaultIfNull(resolvedBidFloor, resolvedCustomBidFloor);
}

private static boolean isValidBidFloor(BigDecimal bidFloor) {
return bidFloor != null && bidFloor.compareTo(BigDecimal.ZERO) > 0;
}

private ObjectNode makeImpExt(Map<String, JsonNode> customParams) {
return customParams != null
? mapper.mapper().valueToTree(ExtImpAdmixer.of(null, null, customParams))
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/prebid/server/proto/openrtb/ext/request/admixer/ExtImpAdmixer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import lombok.AllArgsConstructor;
import lombok.Value;

import java.math.BigDecimal;
import java.util.Map;

@Value
Expand All @@ -14,7 +15,7 @@ public class ExtImpAdmixer {
String zone;

@JsonProperty("customFloor")
Double customFloor;
BigDecimal customFloor;

@JsonProperty("customParams")
Map<String, JsonNode> customParams;
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/static/bidder-params/admixer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"zone": {
"type": "string",
"description": "Zone ID.",
"pattern": "^([a-fA-F\\d\\-]{36})$"
"pattern": "^([a-fA-F\\d\\-]{32,36})$"
},
"customFloor": {
"type": "number",
Expand Down
172 changes: 119 additions & 53 deletions src/test/java/org/prebid/server/bidder/admixer/AdmixerBidderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
import org.prebid.server.proto.openrtb.ext.request.admixer.ExtImpAdmixer;

import java.math.BigDecimal;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.function.UnaryOperator;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -54,71 +56,34 @@ public void creationShouldFailOnInvalidEndpointUrl() {
assertThatIllegalArgumentException().isThrownBy(() -> new AdmixerBidder("invalid_url", jacksonMapper));
}

@Test
public void shouldSetBidfloorToZeroIfExtImpFloorValuIsNull() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(Imp.builder()
.id("123")
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpAdmixer.of("tentententtentententtentententetetet", null,
givenCustomParams("foo1", singletonList("bar1"))))))
.build()))
.build();

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

// then
final Imp expectedImp = Imp.builder()
.id("123")
.tagid("tentententtentententtentententetetet")
.bidfloor(BigDecimal.ZERO)
.ext(mapper.valueToTree(ExtImpAdmixer.of(null, null,
givenCustomParams("foo1", singletonList("bar1")))))
.build();
assertThat(result.getErrors()).hasSize(0);
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.containsExactly(expectedImp);
}

@Test
public void shouldSetExtToNullIfCustomParamsAreNotPresent() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(Imp.builder()
.id("123")
.imp(Collections.singletonList(givenImp(builder -> builder
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpAdmixer.of("tentententtentententtentententetetet", null, null))))
.build()))
ExtImpAdmixer.of("veryVeryVerySuperLongZoneIdValue", null, null)))))))
.build();

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

// then
final Imp expectedImp = Imp.builder()
.id("123")
.tagid("tentententtentententtentententetetet")
.bidfloor(BigDecimal.ZERO)
.ext(null)
.build();
assertThat(result.getErrors()).hasSize(0);
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.containsExactly(expectedImp);
.containsExactly(givenImp(builder -> builder
.tagid("veryVeryVerySuperLongZoneIdValue")
.ext(null)));
}

@Test
public void makeHttpRequestsShouldReturnErrorIfImpExtCouldNotBeParsed() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(Imp.builder()
.id("123")
.ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode()))).build()))
.imp(singletonList(givenImp(builder -> builder
.ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode()))))))
.build();

// when
Expand All @@ -134,10 +99,9 @@ public void makeHttpRequestsShouldReturnErrorIfImpExtCouldNotBeParsed() {
public void makeHttpRequestsShouldReturnErrorIfZoneIdNotHaveLength() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(Imp.builder()
.id("123")
.ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpAdmixer.of("zoneId", 36D,
givenCustomParams("foo1", singletonList("bar1")))))).build()))
.imp(singletonList(givenImp(builder -> builder
.ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpAdmixer.of("zoneId", BigDecimal.ONE,
givenCustomParams("foo1", singletonList("bar1")))))))))
.build();

// when
Expand Down Expand Up @@ -218,7 +182,7 @@ public void makeBidsShouldReturnBannerBidByDefault() throws JsonProcessingExcept
// when
final Result<List<BidderBid>> result = admixerBidder.makeBids(httpCall,
BidRequest.builder()
.imp(singletonList(Imp.builder().id("123").build()))
.imp(singletonList(givenImp(UnaryOperator.identity())))
.build());

// then
Expand Down Expand Up @@ -256,7 +220,7 @@ public void makeBidsShouldReturnBannerBidIfVideoIsPresentInRequestImp() throws J
// when
final Result<List<BidderBid>> result = admixerBidder.makeBids(httpCall,
BidRequest.builder()
.imp(singletonList(Imp.builder().id("123").video(Video.builder().build()).build()))
.imp(singletonList(givenImp(builder -> builder.video(Video.builder().build()))))
.build());

// then
Expand All @@ -275,7 +239,7 @@ public void makeBidsShouldReturnBannerBidIfNativeIsPresentInRequestImp() throws
// when
final Result<List<BidderBid>> result = admixerBidder.makeBids(httpCall,
BidRequest.builder()
.imp(singletonList(Imp.builder().id("123").xNative(Native.builder().build()).build()))
.imp(singletonList(givenImp(builder -> builder.xNative(Native.builder().build()))))
.build());

// then
Expand All @@ -294,7 +258,7 @@ public void makeBidsShouldReturnBannerBidIfAudioIsPresentInRequestImp() throws J
// when
final Result<List<BidderBid>> result = admixerBidder.makeBids(httpCall,
BidRequest.builder()
.imp(singletonList(Imp.builder().id("123").audio(Audio.builder().build()).build()))
.imp(singletonList(givenImp(builder -> builder.audio(Audio.builder().build()))))
.build());

// then
Expand All @@ -303,6 +267,91 @@ public void makeBidsShouldReturnBannerBidIfAudioIsPresentInRequestImp() throws J
.containsExactly(BidderBid.of(Bid.builder().impid("123").build(), audio, "USD"));
}

@Test
public void makeHttpRequestsShouldReturnBidFloorAsNullIfGivenBidFloorNullCustomFloorNull() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(givenImp(builder -> builder.bidfloor(null))))
.build();

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

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.containsExactly(givenImpWithParsedTagID(builder -> builder.bidfloor(null)));
}

@Test
public void makeHttpRequestsShouldReturnBidFloorAsNullIfGivenBidFloorZeroCustomFloorNull() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(givenImp(builder -> builder.bidfloor(BigDecimal.ZERO))))
.build();

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

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.containsExactly(givenImpWithParsedTagID(builder -> builder.bidfloor(null)));
}

@Test
public void makeHttpRequestsShouldReturnBidFloorAsNullIfGivenBidFloorZeroCustomFloorZero() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(givenImp(builder -> builder
.bidfloor(BigDecimal.ZERO)
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpAdmixer.of("veryVeryVerySuperLongZoneIdValue", BigDecimal.ZERO,
givenCustomParams("foo1", singletonList("bar1")))))))))
.build();

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

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.containsExactly(givenImpWithParsedTagID(builder -> builder
.bidfloor(null)
.ext(mapper.valueToTree(ExtImpAdmixer.of(null, null,
givenCustomParams("foo1", singletonList("bar1")))))));
}

@Test
public void makeHttpRequestsShouldReturnBidFloorAsNullIfGivenBidFloorNullCustomFloorZero() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(singletonList(givenImp(builder -> builder
.bidfloor(null)
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpAdmixer.of("veryVeryVerySuperLongZoneIdValue", BigDecimal.ZERO,
givenCustomParams("foo1", singletonList("bar1")))))))))
.build();

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

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.containsExactly(givenImpWithParsedTagID(builder -> builder.bidfloor(null)
.ext(mapper.valueToTree(ExtImpAdmixer.of(null, null,
givenCustomParams("foo1", singletonList("bar1")))))));
}

private static BidResponse givenBidResponse(Function<Bid.BidBuilder, Bid.BidBuilder> bidCustomizer) {
return BidResponse.builder()
.cur("USD")
Expand All @@ -321,4 +370,21 @@ private static HttpCall<BidRequest> givenHttpCall(String body) {
private static Map<String, JsonNode> givenCustomParams(String key, Object values) {
return singletonMap(key, mapper.valueToTree(values));
}

private static Imp givenImp(UnaryOperator<Imp.ImpBuilder> impCustomizer) {
return impCustomizer.apply(Imp.builder()
.id("123")
.ext(mapper.valueToTree(ExtPrebid.of(null,
ExtImpAdmixer.of("veryVeryVerySuperLongZoneIdValue", null,
givenCustomParams("foo1", singletonList("bar1")))))))
.build();
}

//method where zoneId cut from ext and passed to tagId field
private static Imp givenImpWithParsedTagID(UnaryOperator<Imp.ImpBuilder> impCustomizer) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method almost duplicates the above.
Can we call givenImp(...) here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is that in givenImp tagId is located as a part of json in ext field, in givenImpWithParsedTagID tagId is cropped out of json and assigned to field tagId; givenImpWithParsedTagID is quite useful to compare imps after request(during request tagId is cropped and assigned)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but you can call givenImp(...) inside of givenImpWithParsedTagID(...).
So, you'll rid off the code duplication.

return givenImp(builder -> builder
.tagid("veryVeryVerySuperLongZoneIdValue")
.ext(mapper.valueToTree(ExtImpAdmixer.of(null, null,
givenCustomParams("foo1", singletonList("bar1"))))));
}
}