-
Notifications
You must be signed in to change notification settings - Fork 242
Prebid Mobile: makes creative size transformation based on pxRatio. #4510
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,32 @@ | ||
| package org.prebid.server.proto.openrtb.ext.request; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import lombok.Value; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| /** | ||
| * Defines the contract for bidrequest.ext.prebid.sdk | ||
| */ | ||
| @Value(staticConstructor = "of") | ||
| @Value | ||
| public class ExtRequestPrebidSdk { | ||
|
|
||
| /** | ||
| * Defines the contract for bidrequest.ext.prebid.sdk.renderers | ||
| */ | ||
| List<ExtRequestPrebidSdkRenderer> renderers; | ||
|
|
||
| /** | ||
| * Defines the contract for bidrequest.ext.prebid.sdk.usepxratio | ||
| */ | ||
| @JsonProperty("usepxratio") | ||
| Boolean usePxRatio; | ||
|
|
||
| public static ExtRequestPrebidSdk of(List<ExtRequestPrebidSdkRenderer> renderers) { | ||
| return new ExtRequestPrebidSdk(renderers, null); | ||
| } | ||
|
Comment on lines
+25
to
+27
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this static constructor and update all usages providing |
||
|
|
||
| public static ExtRequestPrebidSdk of(List<ExtRequestPrebidSdkRenderer> renderers, Boolean usePxRatio) { | ||
| return new ExtRequestPrebidSdk(renderers, usePxRatio); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,11 @@ | |
| import org.prebid.server.proto.openrtb.ext.request.ExtDevice; | ||
| import org.prebid.server.proto.openrtb.ext.request.ExtDeviceInt; | ||
| import org.prebid.server.proto.openrtb.ext.request.ExtDevicePrebid; | ||
| import org.prebid.server.proto.openrtb.ext.request.ExtRequest; | ||
| import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; | ||
| import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidSdk; | ||
|
|
||
| import java.math.BigDecimal; | ||
|
|
||
| import static java.util.Arrays.asList; | ||
| import static java.util.Collections.singletonList; | ||
|
|
@@ -71,6 +76,28 @@ public void processShouldReturnBidRequestUpdatedWithInterstitialFormatsUsingSize | |
| Format.builder().w(320).h(481).build()); | ||
| } | ||
|
|
||
| @Test | ||
| public void processShouldReturnBidRequestUpdatedWithInterstitialFormatsUsingDeviceSizeInDipsWhenFormatIsEmpty() { | ||
| // given | ||
| final BidRequest bidRequest = BidRequest.builder() | ||
| .imp(singletonList(Imp.builder().banner(Banner.builder().build()).instl(1).build())) | ||
| .device(Device.builder().w(1080).h(1920).pxratio(BigDecimal.valueOf(3)) | ||
| .ext(ExtDevice.of(null, ExtDevicePrebid.of(ExtDeviceInt.of(60, 60)))) | ||
| .build()) | ||
|
Comment on lines
+84
to
+86
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| .ext(usePxRatioExt()) | ||
| .build(); | ||
|
|
||
| // when | ||
| final BidRequest result = interstitialProcessor.process(bidRequest); | ||
|
|
||
| // then | ||
| assertThat(result.getImp()) | ||
| .extracting(Imp::getBanner) | ||
| .flatExtracting(Banner::getFormat) | ||
| .contains(Format.builder().w(320).h(480).build()) | ||
| .doesNotContain(Format.builder().w(768).h(1024).build()); | ||
|
Comment on lines
+97
to
+98
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Merge into |
||
| } | ||
|
|
||
| @Test | ||
| public void processShouldReturnBidRequestUpdatedWithInterstitialFormatsUsingSizesFromDeviceWhenFormatIsOneToOne() { | ||
| // given | ||
|
|
@@ -96,6 +123,97 @@ public void processShouldReturnBidRequestUpdatedWithInterstitialFormatsUsingSize | |
| Format.builder().w(320).h(481).build()); | ||
| } | ||
|
|
||
| @Test | ||
| public void processShouldReturnBidRequestUpdatedWithInterstitialFormatsUsingDeviceSizeInDipsWhenFormatIsOneToOne() { | ||
| // given | ||
| final BidRequest bidRequest = BidRequest.builder() | ||
| .imp(singletonList(Imp.builder().banner(Banner.builder().format(singletonList( | ||
| Format.builder().w(1).h(1).build())).build()).instl(1).build())) | ||
| .device(Device.builder().w(1080).h(1920).pxratio(BigDecimal.valueOf(3)) | ||
| .ext(ExtDevice.of(null, ExtDevicePrebid.of(ExtDeviceInt.of(60, 60)))) | ||
| .build()) | ||
|
Comment on lines
+130
to
+134
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| .ext(usePxRatioExt()) | ||
| .build(); | ||
|
|
||
| // when | ||
| final BidRequest result = interstitialProcessor.process(bidRequest); | ||
|
|
||
| // then | ||
| assertThat(result.getImp()) | ||
| .extracting(Imp::getBanner) | ||
| .flatExtracting(Banner::getFormat) | ||
| .contains(Format.builder().w(320).h(480).build()) | ||
| .doesNotContain(Format.builder().w(768).h(1024).build()); | ||
|
Comment on lines
+145
to
+146
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| } | ||
|
|
||
| @Test | ||
| public void processShouldNotConvertExplicitFormatSizeUsingDevicePxratio() { | ||
| // given | ||
| final BidRequest bidRequest = BidRequest.builder() | ||
| .imp(singletonList(Imp.builder().banner(Banner.builder() | ||
| .format(singletonList(Format.builder().w(400).h(600).build())).build()).instl(1) | ||
| .build())) | ||
| .device(Device.builder().w(1080).h(1920).pxratio(BigDecimal.valueOf(3)) | ||
| .ext(ExtDevice.of(null, ExtDevicePrebid.of(ExtDeviceInt.of(80, 80)))) | ||
| .build()) | ||
|
Comment on lines
+153
to
+158
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| .ext(usePxRatioExt()) | ||
| .build(); | ||
|
|
||
| // when | ||
| final BidRequest result = interstitialProcessor.process(bidRequest); | ||
|
|
||
| // then | ||
| assertThat(result.getImp()) | ||
| .extracting(Imp::getBanner) | ||
| .flatExtracting(Banner::getFormat) | ||
| .containsOnly(Format.builder().w(320).h(480).build(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| Format.builder().w(336).h(544).build(), | ||
| Format.builder().w(320).h(568).build(), | ||
| Format.builder().w(320).h(500).build(), | ||
| Format.builder().w(320).h(481).build()); | ||
| } | ||
|
|
||
| @Test | ||
| public void processShouldKeepCurrentDeviceSizeBehaviorWhenUsePxRatioIsTrueAndDevicePxRatioIsAbsent() { | ||
| // given | ||
| final BidRequest bidRequest = BidRequest.builder() | ||
| .imp(singletonList(Imp.builder().banner(Banner.builder().build()).instl(1).build())) | ||
| .device(Device.builder().w(1080).h(1920) | ||
| .ext(ExtDevice.of(null, ExtDevicePrebid.of(ExtDeviceInt.of(1, 1)))) | ||
| .build()) | ||
|
Comment on lines
+181
to
+183
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| .ext(usePxRatioExt()) | ||
| .build(); | ||
|
|
||
| // when | ||
| final BidRequest result = interstitialProcessor.process(bidRequest); | ||
|
|
||
| // then | ||
| assertThat(result.getImp()) | ||
| .extracting(Imp::getBanner) | ||
| .flatExtracting(Banner::getFormat) | ||
| .contains(Format.builder().w(768).h(1024).build()); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| } | ||
|
|
||
| @Test | ||
| public void processShouldKeepCurrentDeviceSizeBehaviorWhenUsePxRatioIsAbsent() { | ||
| // given | ||
| final BidRequest bidRequest = BidRequest.builder() | ||
| .imp(singletonList(Imp.builder().banner(Banner.builder().build()).instl(1).build())) | ||
| .device(Device.builder().w(1080).h(1920).pxratio(BigDecimal.valueOf(3)) | ||
| .ext(ExtDevice.of(null, ExtDevicePrebid.of(ExtDeviceInt.of(1, 1)))) | ||
| .build()) | ||
|
Comment on lines
+202
to
+204
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| .build(); | ||
|
|
||
| // when | ||
| final BidRequest result = interstitialProcessor.process(bidRequest); | ||
|
|
||
| // then | ||
| assertThat(result.getImp()) | ||
| .extracting(Imp::getBanner) | ||
| .flatExtracting(Banner::getFormat) | ||
| .contains(Format.builder().w(768).h(1024).build()); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| } | ||
|
|
||
| @Test | ||
| public void processShouldReturnBidRequestUpdatedWithInterstitialFormatsLimitedByTen() { | ||
| // given | ||
|
|
@@ -266,4 +384,10 @@ public void processShouldNotUpdateImpWhenInterstitialSizesWereNotFound() { | |
| .build()) | ||
| .build()); | ||
| } | ||
|
|
||
| private static ExtRequest usePxRatioExt() { | ||
| return ExtRequest.of(ExtRequestPrebid.builder() | ||
| .sdk(ExtRequestPrebidSdk.of(null, true)) | ||
| .build()); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make your change consistent with the project's style, old code needs to be refactored. So, to simplify your work and the review process, please copy the attached file :)
InterstitialProcessor.java