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.

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

Eidpermissions Request Validation Update #3666

Merged
merged 2 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ private Future<BidRequest> updateBidRequest(AuctionContext auctionContext) {
.compose(resolvedBidRequest -> ortb2RequestFactory.validateRequest(
resolvedBidRequest,
auctionContext.getHttpRequest(),
auctionContext.getDebugContext(),
auctionContext.getDebugWarnings()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ private Future<BidRequest> updateAndValidateBidRequest(AuctionContext auctionCon

return storedRequestProcessor.processAuctionRequest(account.getId(), auctionContext.getBidRequest())
.compose(auctionStoredResult -> updateBidRequest(auctionStoredResult, auctionContext))
.compose(bidRequest -> ortb2RequestFactory.validateRequest(bidRequest, httpRequest, debugWarnings))
.compose(bidRequest -> ortb2RequestFactory.validateRequest(
bidRequest, httpRequest, auctionContext.getDebugContext(), debugWarnings))
.map(interstitialProcessor::process);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,11 @@ public Future<ActivityInfrastructure> activityInfrastructureFrom(AuctionContext

public Future<BidRequest> validateRequest(BidRequest bidRequest,
HttpRequestContext httpRequestContext,
DebugContext debugContext,
List<String> warnings) {

final ValidationResult validationResult = requestValidator.validate(bidRequest, httpRequestContext);
final ValidationResult validationResult = requestValidator.validate(
bidRequest, httpRequestContext, debugContext);

if (validationResult.hasWarnings()) {
warnings.addAll(validationResult.getWarnings());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,24 @@ public Future<WithPodErrors<AuctionContext>> fromRequest(RoutingContext routingC
return ortb2RequestFactory.executeEntrypointHooks(routingContext, body, initialAuctionContext)
.compose(httpRequest -> createBidRequest(httpRequest)
.map(bidRequest -> removeEmptyEids(bidRequest, initialAuctionContext.getDebugWarnings()))
.compose(bidRequest -> validateRequest(
bidRequest,
httpRequest,
initialAuctionContext.getDebugWarnings()))

.map(bidRequestWithErrors -> populatePodErrors(
bidRequestWithErrors.getPodErrors(), podErrors, bidRequestWithErrors))

.map(bidRequestWithErrors -> ortb2RequestFactory.enrichAuctionContext(
initialAuctionContext, httpRequest, bidRequestWithErrors.getData(), startTime)))

.compose(auctionContext -> ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(auctionContext)
.map(auctionContext -> auctionContext.with(debugResolver.debugContextFrom(auctionContext)))

.compose(auctionContext -> ortb2RequestFactory.validateRequest(
auctionContext.getBidRequest(),
auctionContext.getHttpRequest(),
auctionContext.getDebugContext(),
auctionContext.getDebugWarnings())
.map(auctionContext::with))

.map(auctionContext -> auctionContext.with(debugResolver.debugContextFrom(auctionContext)))
.compose(auctionContext -> ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(auctionContext)
.map(auctionContext::with))

.compose(auctionContext -> geoLocationServiceWrapper.lookup(auctionContext)
.map(auctionContext::with))
Expand Down Expand Up @@ -323,12 +326,4 @@ private WithPodErrors<BidRequest> fillImplicitParameters(HttpRequestContext http

return WithPodErrors.of(updatedWithDebugBidRequest, bidRequestToErrors.getPodErrors());
}

private Future<WithPodErrors<BidRequest>> validateRequest(WithPodErrors<BidRequest> requestWithPodErrors,
HttpRequestContext httpRequestContext,
List<String> warnings) {

return ortb2RequestFactory.validateRequest(requestWithPodErrors.getData(), httpRequestContext, warnings)
.map(bidRequest -> requestWithPodErrors);
}
}
40 changes: 27 additions & 13 deletions src/main/java/org/prebid/server/validation/RequestValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.auction.model.debug.DebugContext;
import org.prebid.server.bidder.BidderCatalog;
import org.prebid.server.json.JacksonMapper;
import org.prebid.server.log.ConditionalLogger;
Expand Down Expand Up @@ -96,8 +97,12 @@ public RequestValidator(BidderCatalog bidderCatalog,
* Validates the {@link BidRequest} against a list of validation checks, however, reports only one problem
* at a time.
*/
public ValidationResult validate(BidRequest bidRequest, HttpRequestContext httpRequestContext) {
public ValidationResult validate(BidRequest bidRequest,
HttpRequestContext httpRequestContext,
DebugContext debugContext) {

final List<String> warnings = new ArrayList<>();
final boolean isDebugEnabled = debugContext != null && debugContext.isDebugEnabled();
try {
if (StringUtils.isBlank(bidRequest.getId())) {
throw new ValidationException("request missing required field: \"id\"");
Expand All @@ -124,7 +129,7 @@ public ValidationResult validate(BidRequest bidRequest, HttpRequestContext httpR
validateAliases(aliases);
validateAliasesGvlIds(extRequestPrebid, aliases);
validateBidAdjustmentFactors(extRequestPrebid.getBidadjustmentfactors(), aliases);
validateExtBidPrebidData(extRequestPrebid.getData(), aliases);
validateExtBidPrebidData(extRequestPrebid.getData(), aliases, isDebugEnabled, warnings);
validateSchains(extRequestPrebid.getSchains());
}

Expand Down Expand Up @@ -313,15 +318,20 @@ private void validateSchains(List<ExtRequestPrebidSchain> schains) throws Valida
}
}

private void validateExtBidPrebidData(ExtRequestPrebidData data, Map<String, String> aliases)
throws ValidationException {
private void validateExtBidPrebidData(ExtRequestPrebidData data,
Map<String, String> aliases,
boolean isDebugEnabled,
List<String> warnings) throws ValidationException {

if (data != null) {
validateEidPermissions(data.getEidPermissions(), aliases);
validateEidPermissions(data.getEidPermissions(), aliases, isDebugEnabled, warnings);
}
}

private void validateEidPermissions(List<ExtRequestPrebidDataEidPermissions> eidPermissions,
Map<String, String> aliases) throws ValidationException {
Map<String, String> aliases,
boolean isDebugEnabled,
List<String> warnings) throws ValidationException {

if (eidPermissions == null) {
return;
Expand All @@ -333,7 +343,7 @@ private void validateEidPermissions(List<ExtRequestPrebidDataEidPermissions> eid
}

validateEidPermissionSource(eidPermission.getSource());
validateEidPermissionBidders(eidPermission.getBidders(), aliases);
validateEidPermissionBidders(eidPermission.getBidders(), aliases, isDebugEnabled, warnings);
}
}

Expand All @@ -344,18 +354,22 @@ private void validateEidPermissionSource(String source) throws ValidationExcepti
}

private void validateEidPermissionBidders(List<String> bidders,
Map<String, String> aliases) throws ValidationException {
Map<String, String> aliases,
boolean isDebugEnabled,
List<String> warnings) throws ValidationException {

if (CollectionUtils.isEmpty(bidders)) {
throw new ValidationException("request.ext.prebid.data.eidpermissions[].bidders[] required values"
+ " but was empty or null");
}

for (String bidder : bidders) {
if (!bidderCatalog.isValidName(bidder) && !bidderCatalog.isValidName(aliases.get(bidder))
&& ObjectUtils.notEqual(bidder, ASTERISK)) {
throw new ValidationException(
"request.ext.prebid.data.eidPermissions[].bidders[] unrecognized biddercode: '%s'", bidder);
if (isDebugEnabled) {
for (String bidder : bidders) {
if (!bidderCatalog.isValidName(bidder) && !bidderCatalog.isValidName(aliases.get(bidder))
&& ObjectUtils.notEqual(bidder, ASTERISK)) {
warnings.add("request.ext.prebid.data.eidPermissions[].bidders[] unrecognized biddercode: '%s'"
.formatted(bidder));
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1753,7 +1753,7 @@ private void givenBidRequest(

given(ortb2ImplicitParametersResolver.resolve(any(), any(), any(), anyBoolean())).willAnswer(
answerWithFirstArgument());
given(ortb2RequestFactory.validateRequest(any(), any(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
.willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0)));

given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public void setUp() {
given(ortb2RequestFactory.executeRawAuctionRequestHooks(any()))
.willAnswer(invocation -> Future.succeededFuture(
((AuctionContext) invocation.getArgument(0)).getBidRequest()));
given(ortb2RequestFactory.validateRequest(any(), any(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
.willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(0)));
given(ortb2RequestFactory.removeEmptyEids(any(), any()))
.willAnswer(invocationOnMock -> invocationOnMock.getArgument(0));
Expand Down Expand Up @@ -662,7 +662,7 @@ public void shouldReturnFailedFutureIfRequestValidationFailed() {
// given
givenValidBidRequest();

given(ortb2RequestFactory.validateRequest(any(), any(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
.willReturn(Future.failedFuture(new InvalidRequestException("errors")));

// when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -658,14 +658,15 @@ public void enrichAuctionContextShouldSetDebugOff() {
@Test
public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid() {
// given
given(requestValidator.validate(any(), any())).willReturn(ValidationResult.error("error"));
given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.error("error"));

final BidRequest bidRequest = givenBidRequest(identity());

// when
final Future<BidRequest> result = target.validateRequest(
bidRequest,
HttpRequestContext.builder().build(),
DebugContext.empty(),
new ArrayList<>());

// then
Expand All @@ -674,24 +675,25 @@ public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid(
.isInstanceOf(InvalidRequestException.class)
.hasMessage("error");

verify(requestValidator).validate(eq(bidRequest), any());
verify(requestValidator).validate(eq(bidRequest), any(), any());
}

@Test
public void validateRequestShouldReturnSameBidRequest() {
// given
given(requestValidator.validate(any(), any())).willReturn(ValidationResult.success());
given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.success());

final BidRequest bidRequest = givenBidRequest(identity());

// when
final BidRequest result = target.validateRequest(
bidRequest,
HttpRequestContext.builder().build(),
DebugContext.empty(),
new ArrayList<>()).result();

// then
verify(requestValidator).validate(eq(bidRequest), any());
verify(requestValidator).validate(eq(bidRequest), any(), any());

assertThat(result).isSameAs(bidRequest);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ public void shouldReturnExpectedResultAndReturnErrors() throws JsonProcessingExc
verify(ortb2RequestFactory).createAuctionContext(any(), eq(MetricName.video));
verify(ortb2RequestFactory).enrichAuctionContext(any(), any(), eq(bidRequest), eq(0L));
verify(ortb2RequestFactory).fetchAccountWithoutStoredRequestLookup(any());
verify(ortb2RequestFactory).validateRequest(eq(bidRequest), any(), any());
verify(ortb2RequestFactory).validateRequest(eq(bidRequest), any(), any(), any());
verify(paramsResolver)
.resolve(eq(bidRequest), any(), eq(Endpoint.openrtb2_video.value()), eq(false));
verify(ortb2RequestFactory).enrichBidRequestWithAccountAndPrivacyData(
Expand Down Expand Up @@ -404,7 +404,7 @@ private void givenBidRequest(BidRequest bidRequest, List<PodError> podErrors) {
.build());
given(ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(any())).willReturn(Future.succeededFuture());

given(ortb2RequestFactory.validateRequest(any(), any(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
.willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0)));

given(paramsResolver.resolve(any(), any(), any(), anyBoolean()))
Expand Down
Loading
Loading