Skip to content

Commit ce9a909

Browse files
osulzhenkoosulzhenko
committed
Merge branch 'refs/heads/auction-imp-limit' into functional-tests/auction-imp-limit
2 parents 6e2d40b + e952960 commit ce9a909

File tree

10 files changed

+120
-102
lines changed

10 files changed

+120
-102
lines changed

‎src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,10 @@ private Future<BidRequest> updateBidRequest(AuctionContext auctionContext) {
414414
.map(bidRequest -> overrideParameters(bidRequest, httpRequest, auctionContext.getPrebidErrors()))
415415
.map(bidRequest -> paramsResolver.resolve(bidRequest, auctionContext, ENDPOINT, true))
416416
.map(bidRequest -> ortb2RequestFactory.removeEmptyEids(bidRequest, auctionContext.getDebugWarnings()))
417+
.compose(resolvedBidRequest -> ortb2RequestFactory.limitImpressions(
418+
account,
419+
resolvedBidRequest,
420+
auctionContext.getDebugWarnings()))
417421
.compose(resolvedBidRequest -> ortb2RequestFactory.validateRequest(
418422
account,
419423
resolvedBidRequest,

‎src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ private Future<BidRequest> updateAndValidateBidRequest(AuctionContext auctionCon
241241

242242
return storedRequestProcessor.processAuctionRequest(account.getId(), auctionContext.getBidRequest())
243243
.compose(auctionStoredResult -> updateBidRequest(auctionStoredResult, auctionContext))
244+
.compose(bidRequest -> ortb2RequestFactory.limitImpressions(account, bidRequest, debugWarnings))
244245
.compose(bidRequest -> ortb2RequestFactory.validateRequest(
245246
account, bidRequest, httpRequest, auctionContext.getDebugContext(), debugWarnings))
246247
.map(interstitialProcessor::process);

‎src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import com.iab.openrtb.request.Dooh;
77
import com.iab.openrtb.request.Eid;
88
import com.iab.openrtb.request.Geo;
9+
import com.iab.openrtb.request.Imp;
910
import com.iab.openrtb.request.Publisher;
1011
import com.iab.openrtb.request.Regs;
1112
import com.iab.openrtb.request.Site;
@@ -192,6 +193,23 @@ public Future<ActivityInfrastructure> activityInfrastructureFrom(AuctionContext
192193
auctionContext.getDebugContext().getTraceLevel()));
193194
}
194195

196+
public Future<BidRequest> limitImpressions(Account account, BidRequest bidRequest, List<String> warnings) {
197+
final List<Imp> imps = bidRequest.getImp();
198+
final int impsLimit = Optional.ofNullable(account)
199+
.map(Account::getAuction)
200+
.map(AccountAuctionConfig::getImpressionLimit)
201+
.orElse(0);
202+
203+
if (impsLimit > 0 && imps.size() > impsLimit) {
204+
metrics.updateImpsDroppedMetric(imps.size() - impsLimit);
205+
warnings.add(("Only first %d impressions were kept due to the limit, "
206+
+ "all the subsequent impressions have been dropped for the auction").formatted(impsLimit));
207+
return Future.succeededFuture(bidRequest.toBuilder().imp(imps.stream().limit(impsLimit).toList()).build());
208+
}
209+
210+
return Future.succeededFuture(bidRequest);
211+
}
212+
195213
public Future<BidRequest> validateRequest(Account account,
196214
BidRequest bidRequest,
197215
HttpRequestContext httpRequestContext,

‎src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ public Future<WithPodErrors<AuctionContext>> fromRequest(RoutingContext routingC
119119

120120
.map(auctionContext -> auctionContext.with(debugResolver.debugContextFrom(auctionContext)))
121121

122+
.compose(auctionContext -> ortb2RequestFactory.limitImpressions(
123+
auctionContext.getAccount(),
124+
auctionContext.getBidRequest(),
125+
auctionContext.getDebugWarnings())
126+
.map(auctionContext::with))
127+
122128
.compose(auctionContext -> ortb2RequestFactory.validateRequest(
123129
auctionContext.getAccount(),
124130
auctionContext.getBidRequest(),

‎src/main/java/org/prebid/server/validation/RequestValidator.java‎

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.prebid.server.proto.openrtb.ext.request.ImpMediaType;
4747
import org.prebid.server.proto.openrtb.ext.response.BidType;
4848
import org.prebid.server.settings.model.Account;
49-
import org.prebid.server.settings.model.AccountAuctionConfig;
5049
import org.prebid.server.util.HttpUtil;
5150
import org.prebid.server.validation.model.ValidationResult;
5251

@@ -154,25 +153,28 @@ public ValidationResult validate(Account account,
154153
validateSchains(extRequestPrebid.getSchains());
155154
}
156155

157-
final List<Imp> imps = bidRequest.getImp();
158-
if (CollectionUtils.isEmpty(imps)) {
156+
if (CollectionUtils.isEmpty(bidRequest.getImp())) {
159157
throw new ValidationException("request.imp must contain at least one element");
160158
}
161159

162-
BidRequest requestWithImpsDropped = bidRequest;
163-
final int impsLimit = Optional.ofNullable(account)
164-
.map(Account::getAuction)
165-
.map(AccountAuctionConfig::getImpressionLimit)
166-
.orElse(0);
167-
if (impsLimit > 0 && imps.size() > impsLimit) {
168-
metrics.updateImpsDroppedMetric(imps.size() - impsLimit);
169-
warnings.add(("Only first %d impressions were kept due to the limit, "
170-
+ "all the subsequent impressions have been dropped for the auction").formatted(impsLimit));
171-
requestWithImpsDropped = bidRequest.toBuilder().imp(imps.stream().limit(impsLimit).toList()).build();
160+
final List<Imp> imps = bidRequest.getImp();
161+
final List<String> errors = new ArrayList<>();
162+
final Map<String, Integer> uniqueImps = new HashMap<>();
163+
for (int i = 0; i < imps.size(); i++) {
164+
final String impId = imps.get(i).getId();
165+
if (uniqueImps.get(impId) != null) {
166+
errors.add("request.imp[%d].id and request.imp[%d].id are both \"%s\". Imp IDs must be unique."
167+
.formatted(uniqueImps.get(impId), i, impId));
168+
}
169+
170+
uniqueImps.put(impId, i);
172171
}
173172

174-
validateUniqueImps(requestWithImpsDropped.getImp());
175-
impValidator.validateImps(requestWithImpsDropped.getImp(), aliases, warnings);
173+
if (CollectionUtils.isNotEmpty(errors)) {
174+
throw new ValidationException(String.join(System.lineSeparator(), errors));
175+
}
176+
177+
impValidator.validateImps(bidRequest.getImp(), aliases, warnings);
176178

177179
final List<String> channels = new ArrayList<>();
178180
Optional.ofNullable(bidRequest.getApp()).ifPresent(ignored -> channels.add("request.app"));
@@ -359,24 +361,6 @@ private void validateSchains(List<ExtRequestPrebidSchain> schains) throws Valida
359361
}
360362
}
361363

362-
private static void validateUniqueImps(List<Imp> imps) throws ValidationException {
363-
final List<String> errors = new ArrayList<>();
364-
final Map<String, Integer> uniqueImps = new HashMap<>();
365-
for (int i = 0; i < imps.size(); i++) {
366-
final String impId = imps.get(i).getId();
367-
if (uniqueImps.get(impId) != null) {
368-
errors.add("request.imp[%d].id and request.imp[%d].id are both \"%s\". Imp IDs must be unique."
369-
.formatted(uniqueImps.get(impId), i, impId));
370-
}
371-
372-
uniqueImps.put(impId, i);
373-
}
374-
375-
if (CollectionUtils.isNotEmpty(errors)) {
376-
throw new ValidationException(String.join(System.lineSeparator(), errors));
377-
}
378-
}
379-
380364
private void validateExtBidPrebidData(ExtRequestPrebidData data,
381365
Map<String, String> aliases,
382366
boolean isDebugEnabled,

‎src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,6 +1757,8 @@ private void givenBidRequest(
17571757

17581758
given(ortb2ImplicitParametersResolver.resolve(any(), any(), any(), anyBoolean())).willAnswer(
17591759
answerWithFirstArgument());
1760+
given(ortb2RequestFactory.limitImpressions(any(), any(), any()))
1761+
.willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(1)));
17601762
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any()))
17611763
.willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(1)));
17621764

‎src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ public void setUp() {
167167
given(ortb2RequestFactory.executeRawAuctionRequestHooks(any()))
168168
.willAnswer(invocation -> Future.succeededFuture(
169169
((AuctionContext) invocation.getArgument(0)).getBidRequest()));
170+
given(ortb2RequestFactory.limitImpressions(any(), any(), any()))
171+
.willAnswer(invocationOnMock -> Future.succeededFuture(invocationOnMock.getArgument(1)));
170172
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any()))
171173
.willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(1)));
172174
given(ortb2RequestFactory.removeEmptyEids(any(), any()))

‎src/test/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactoryTest.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.iab.openrtb.request.Dooh;
88
import com.iab.openrtb.request.Eid;
99
import com.iab.openrtb.request.Geo;
10+
import com.iab.openrtb.request.Imp;
1011
import com.iab.openrtb.request.Publisher;
1112
import com.iab.openrtb.request.Regs;
1213
import com.iab.openrtb.request.Site;
@@ -83,19 +84,22 @@
8384
import java.util.List;
8485
import java.util.function.UnaryOperator;
8586

87+
import static java.util.Arrays.asList;
8688
import static java.util.Collections.emptyList;
8789
import static java.util.Collections.emptyMap;
8890
import static java.util.Collections.singletonList;
8991
import static java.util.function.UnaryOperator.identity;
9092
import static org.assertj.core.api.Assertions.assertThat;
9193
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
9294
import static org.mockito.ArgumentMatchers.any;
95+
import static org.mockito.ArgumentMatchers.anyInt;
9396
import static org.mockito.ArgumentMatchers.anyLong;
9497
import static org.mockito.ArgumentMatchers.anyString;
9598
import static org.mockito.ArgumentMatchers.eq;
9699
import static org.mockito.BDDMockito.given;
97100
import static org.mockito.Mock.Strictness.LENIENT;
98101
import static org.mockito.Mockito.mock;
102+
import static org.mockito.Mockito.never;
99103
import static org.mockito.Mockito.verify;
100104
import static org.mockito.Mockito.verifyNoInteractions;
101105
import static org.prebid.server.assertion.FutureAssertion.assertThat;
@@ -1708,6 +1712,70 @@ public void removeEmptyEidsShouldRemoveEmptyUidsOnly() {
17081712
"removed EID source3 due to empty ID");
17091713
}
17101714

1715+
@Test
1716+
public void validateShouldDropImpressionsOverAccountLimitAndReturnWarning() {
1717+
// given
1718+
final Imp imp1 = Imp.builder().id("1").build();
1719+
final Imp imp2 = Imp.builder().id("2").build();
1720+
final BidRequest bidRequest = givenBidRequest(request -> request.imp(asList(imp1, imp2)));
1721+
final List<String> warning = new ArrayList<>();
1722+
1723+
final Account givenAccount = Account.builder()
1724+
.id(ACCOUNT_ID)
1725+
.auction(AccountAuctionConfig.builder().impressionLimit(1).build())
1726+
.build();
1727+
1728+
// when
1729+
final BidRequest result = target.limitImpressions(givenAccount, bidRequest, warning).result();
1730+
1731+
// then
1732+
assertThat(warning).hasSize(1)
1733+
.containsOnly("Only first 1 impressions were kept due to the limit, "
1734+
+ "all the subsequent impressions have been dropped for the auction");
1735+
1736+
verify(metrics).updateImpsDroppedMetric(1);
1737+
assertThat(result.getImp()).containsOnly(imp1);
1738+
}
1739+
1740+
@Test
1741+
public void validateShouldNotDropImpressionsReturnWarningWhenAccountLimitIsSetToZero() {
1742+
// given
1743+
final Imp imp1 = Imp.builder().id("1").build();
1744+
final Imp imp2 = Imp.builder().id("2").build();
1745+
final BidRequest bidRequest = givenBidRequest(request -> request.imp(asList(imp1, imp2)));
1746+
final List<String> warning = new ArrayList<>();
1747+
1748+
final Account givenAccount = Account.builder()
1749+
.id(ACCOUNT_ID)
1750+
.auction(AccountAuctionConfig.builder().impressionLimit(0).build())
1751+
.build();
1752+
1753+
// when
1754+
final BidRequest result = target.limitImpressions(givenAccount, bidRequest, warning).result();
1755+
1756+
// then
1757+
assertThat(warning).isEmpty();
1758+
assertThat(result).isEqualTo(bidRequest);
1759+
verify(metrics, never()).updateImpsDroppedMetric(anyInt());
1760+
}
1761+
1762+
@Test
1763+
public void validateShouldNotDropImpressionsReturnWarningWhenAccountLimitIsNotSet() {
1764+
// given
1765+
final Imp imp1 = Imp.builder().id("1").build();
1766+
final Imp imp2 = Imp.builder().id("2").build();
1767+
final BidRequest bidRequest = givenBidRequest(request -> request.imp(asList(imp1, imp2)));
1768+
final List<String> warning = new ArrayList<>();
1769+
1770+
// when
1771+
final BidRequest result = target.limitImpressions(Account.empty(ACCOUNT_ID), bidRequest, warning).result();
1772+
1773+
// then
1774+
assertThat(warning).isEmpty();
1775+
assertThat(result).isEqualTo(bidRequest);
1776+
verify(metrics, never()).updateImpsDroppedMetric(anyInt());
1777+
}
1778+
17111779
private void givenTarget(int timeoutAdjustmentFactor) {
17121780
target = new Ortb2RequestFactory(
17131781
timeoutAdjustmentFactor,

‎src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,8 @@ private void givenBidRequest(BidRequest bidRequest, List<PodError> podErrors) {
408408
.build());
409409
given(ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(any())).willReturn(Future.succeededFuture());
410410

411+
given(ortb2RequestFactory.limitImpressions(any(), any(), any()))
412+
.willAnswer(invocationOnMock -> Future.succeededFuture(invocationOnMock.getArgument(1)));
411413
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any()))
412414
.willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(1)));
413415

‎src/test/java/org/prebid/server/validation/RequestValidatorTest.java‎

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import org.prebid.server.proto.openrtb.ext.request.ExtUserPrebid;
4646
import org.prebid.server.proto.openrtb.ext.request.ImpMediaType;
4747
import org.prebid.server.settings.model.Account;
48-
import org.prebid.server.settings.model.AccountAuctionConfig;
4948
import org.prebid.server.validation.model.ValidationResult;
5049

5150
import java.math.BigDecimal;
@@ -62,13 +61,11 @@
6261
import static java.util.Collections.singletonMap;
6362
import static org.assertj.core.api.Assertions.assertThat;
6463
import static org.mockito.ArgumentMatchers.any;
65-
import static org.mockito.ArgumentMatchers.anyInt;
6664
import static org.mockito.ArgumentMatchers.eq;
6765
import static org.mockito.BDDMockito.given;
6866
import static org.mockito.Mock.Strictness.LENIENT;
6967
import static org.mockito.Mockito.doAnswer;
7068
import static org.mockito.Mockito.doThrow;
71-
import static org.mockito.Mockito.never;
7269
import static org.mockito.Mockito.verify;
7370

7471
@ExtendWith(MockitoExtension.class)
@@ -565,72 +562,6 @@ public void validateShouldReturnValidationMessageWhenRequestHaveDuplicatedImpIds
565562
.containsOnly("request.imp[0].id and request.imp[1].id are both \"11\". Imp IDs must be unique.");
566563
}
567564

568-
@Test
569-
public void validateShouldDropImpressionsOverAccountLimitAndReturnWarning() throws ValidationException {
570-
// given
571-
final Imp imp1 = Imp.builder().id("1").build();
572-
final Imp imp2 = Imp.builder().id("2").build();
573-
final BidRequest bidRequest = validBidRequestBuilder().imp(asList(imp1, imp2)).build();
574-
575-
final Account givenAccount = Account.builder()
576-
.id(ACCOUNT_ID)
577-
.auction(AccountAuctionConfig.builder().impressionLimit(1).build())
578-
.build();
579-
580-
// when
581-
final ValidationResult result = target.validate(givenAccount, bidRequest, null, null);
582-
583-
// then
584-
assertThat(result.getWarnings()).hasSize(1)
585-
.containsOnly("Only first 1 impressions were kept due to the limit, "
586-
+ "all the subsequent impressions have been dropped for the auction");
587-
assertThat(result.getErrors()).isEmpty();
588-
589-
verify(metrics).updateImpsDroppedMetric(1);
590-
verify(impValidator).validateImps(eq(singletonList(imp1)), any(), any());
591-
}
592-
593-
@Test
594-
public void validateShouldNotDropImpressionsReturnWarningWhenAccountLimitIsSetToZero() throws ValidationException {
595-
// given
596-
final Imp imp1 = Imp.builder().id("1").build();
597-
final Imp imp2 = Imp.builder().id("2").build();
598-
final BidRequest bidRequest = validBidRequestBuilder().imp(asList(imp1, imp2)).build();
599-
600-
final Account givenAccount = Account.builder()
601-
.id(ACCOUNT_ID)
602-
.auction(AccountAuctionConfig.builder().impressionLimit(0).build())
603-
.build();
604-
605-
// when
606-
final ValidationResult result = target.validate(givenAccount, bidRequest, null, null);
607-
608-
// then
609-
assertThat(result.getWarnings()).isEmpty();
610-
assertThat(result.getErrors()).isEmpty();
611-
612-
verify(metrics, never()).updateImpsDroppedMetric(anyInt());
613-
verify(impValidator).validateImps(eq(bidRequest.getImp()), any(), any());
614-
}
615-
616-
@Test
617-
public void validateShouldNotDropImpressionsReturnWarningWhenAccountLimitIsNotSet() throws ValidationException {
618-
// given
619-
final Imp imp1 = Imp.builder().id("1").build();
620-
final Imp imp2 = Imp.builder().id("2").build();
621-
final BidRequest bidRequest = validBidRequestBuilder().imp(asList(imp1, imp2)).build();
622-
623-
// when
624-
final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null);
625-
626-
// then
627-
assertThat(result.getWarnings()).isEmpty();
628-
assertThat(result.getErrors()).isEmpty();
629-
630-
verify(metrics, never()).updateImpsDroppedMetric(anyInt());
631-
verify(impValidator).validateImps(eq(bidRequest.getImp()), any(), any());
632-
}
633-
634565
@Test
635566
public void validateShouldNotReturnValidationMessageIfUserExtIsEmptyJsonObject() {
636567
// given

0 commit comments

Comments
 (0)