Skip to content

Commit

Permalink
Tolerate root fpd fields wrong types (#908)
Browse files Browse the repository at this point in the history 
* Tolerate incorrect fpd types and remove these fields

* Fix ortb resolver debug messages and logs
  • Loading branch information
@BraslavskiyAndrey
BraslavskiyAndrey authored Sep 14, 2020
1 parent b46e78f commit 60ed657
Show file tree
Hide file tree
Showing 7 changed files with 193 additions and 110 deletions.
5 changes: 4 additions & 1 deletion src/main/java/org/prebid/server/auction/AmpRequestFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,22 @@ public class AmpRequestFactory {
private final StoredRequestProcessor storedRequestProcessor;
private final AuctionRequestFactory auctionRequestFactory;
private final OrtbTypesResolver ortbTypesResolver;
private final ImplicitParametersExtractor implicitParametersExtractor;
private final FpdResolver fpdResolver;
private final TimeoutResolver timeoutResolver;
private final JacksonMapper mapper;

public AmpRequestFactory(StoredRequestProcessor storedRequestProcessor,
AuctionRequestFactory auctionRequestFactory,
OrtbTypesResolver ortbTypesResolver,
ImplicitParametersExtractor implicitParametersExtractor,
FpdResolver fpdResolver,
TimeoutResolver timeoutResolver,
JacksonMapper mapper) {
this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor);
this.auctionRequestFactory = Objects.requireNonNull(auctionRequestFactory);
this.ortbTypesResolver = Objects.requireNonNull(ortbTypesResolver);
this.implicitParametersExtractor = Objects.requireNonNull(implicitParametersExtractor);
this.fpdResolver = Objects.requireNonNull(fpdResolver);
this.timeoutResolver = Objects.requireNonNull(timeoutResolver);
this.mapper = Objects.requireNonNull(mapper);
Expand Down Expand Up @@ -249,7 +252,7 @@ private BidRequest overrideParameters(BidRequest bidRequest, HttpServerRequest r

final String requestTargeting = request.getParam(TARGETING_REQUEST_PARAM);
final ObjectNode targetingNode = readTargeting(requestTargeting);
ortbTypesResolver.normalizeStandardFpdFields(targetingNode, errors, "targeting");
ortbTypesResolver.normalizeTargeting(targetingNode, errors, implicitParametersExtractor.refererFrom(request));
final Targeting targeting = parseTargeting(targetingNode);

final Site updatedSite = overrideSite(bidRequest.getSite(), request);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/prebid/server/auction/AuctionRequestFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ private BidRequest parseRequest(RoutingContext context, List<String> errors) {
throw new InvalidRequestException(String.format("Error decoding bidRequest: %s", e.getMessage()));
}

ortbTypesResolver.normalizeBidRequest(bidRequestNode, errors);
final String referer = paramsExtractor.refererFrom(context.request());
ortbTypesResolver.normalizeBidRequest(bidRequestNode, errors, referer);

try {
return mapper.mapper().treeToValue(bidRequestNode, BidRequest.class);
Expand Down
171 changes: 112 additions & 59 deletions src/main/java/org/prebid/server/auction/OrtbTypesResolver.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.prebid.server.auction;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.JsonNodeType;
Expand All @@ -8,13 +9,18 @@
import io.vertx.core.logging.Logger;
import io.vertx.core.logging.LoggerFactory;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.exception.InvalidRequestException;
import org.prebid.server.json.JacksonMapper;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
Expand All @@ -30,7 +36,9 @@ public class OrtbTypesResolver {
private static final String USER = "user";
private static final String APP = "app";
private static final String SITE = "site";
private static final String BIDREQUEST_PREFIX = "bidrequest.";
private static final String BIDREQUEST = "bidrequest";
private static final String TARGETING = "targeting";
private static final String UNKNOWN_REFERER = "unknown referer";

private static final Map<String, Set<String>> FIRST_ARRAY_ELEMENT_STANDARD_FIELDS;
private static final Map<String, Set<String>> FIRST_ARRAY_ELEMENT_REQUEST_FIELDS;
Expand All @@ -57,29 +65,58 @@ public class OrtbTypesResolver {
COMMA_SEPARATED_ELEMENT_FIELDS.put(SITE, Collections.singleton("keywords"));
}

private final JacksonMapper jacksonMapper;

public OrtbTypesResolver(JacksonMapper jacksonMapper) {
this.jacksonMapper = Objects.requireNonNull(jacksonMapper);
}

/**
* Resolves fields types inconsistency to ortb2 protocol for {@param bidRequest} for bidRequest level parameters
* and bidderconfig.
* Mutates both parameters, {@param fpdContainerNode} and {@param warnings}.
*/
void normalizeBidRequest(JsonNode bidRequest, List<String> warnings) {
normalizeRequestFpdFields(bidRequest, warnings);
void normalizeBidRequest(JsonNode bidRequest, List<String> warnings, String referer) {
final List<String> resolverWarnings = new ArrayList<>();
final String rowOriginBidRequest = getOriginalRowContainerNode(bidRequest);
normalizeRequestFpdFields(bidRequest, resolverWarnings);
final JsonNode bidderConfigs = bidRequest.path("ext").path("prebid").path("bidderconfig");
if (!bidderConfigs.isMissingNode() && bidderConfigs.isArray()) {
for (JsonNode bidderConfig : bidderConfigs) {
final JsonNode config = bidderConfig.path("config").path("fpd");
if (!config.isMissingNode()) {
normalizeStandardFpdFields(config, warnings, "bidrequest.ext.prebid.bidderconfig");
normalizeStandardFpdFields(config, resolverWarnings, "bidrequest.ext.prebid.bidderconfig");
}
}
}
processWarnings(resolverWarnings, warnings, rowOriginBidRequest, referer, BIDREQUEST);
}

private String getOriginalRowContainerNode(JsonNode bidRequest) {
try {
return jacksonMapper.mapper().writeValueAsString(bidRequest);
} catch (JsonProcessingException e) {
// should never happen
throw new InvalidRequestException("Failed to decode container node to string");
}
}

/**
* Resolves fields types inconsistency to ortb2 protocol for {@param targeting}.
* Mutates both parameters, {@param targeting} and {@param warnings}.
*/
void normalizeTargeting(JsonNode targeting, List<String> warnings, String referer) {
final List<String> resolverWarnings = new ArrayList<>();
final String rowOriginTargeting = getOriginalRowContainerNode(targeting);
normalizeStandardFpdFields(targeting, resolverWarnings, TARGETING);
processWarnings(resolverWarnings, warnings, rowOriginTargeting, referer, TARGETING);
}

/**
* Resolves fields types inconsistency to ortb2 protocol for {@param fpdContainerNode}.
* Mutates both parameters, {@param fpdContainerNode} and {@param warnings}.
*/
void normalizeStandardFpdFields(JsonNode fpdContainerNode, List<String> warnings, String nodePrefix) {
private void normalizeStandardFpdFields(JsonNode fpdContainerNode, List<String> warnings, String nodePrefix) {
final String normalizedNodePrefix = nodePrefix.endsWith(".") ? nodePrefix : nodePrefix.concat(".");
if (fpdContainerNode != null && fpdContainerNode.isObject()) {
final ObjectNode fpdContainerObjectNode = (ObjectNode) fpdContainerNode;
Expand All @@ -95,52 +132,61 @@ void normalizeStandardFpdFields(JsonNode fpdContainerNode, List<String> warnings
private void normalizeRequestFpdFields(JsonNode fpdContainerNode, List<String> warnings) {
if (fpdContainerNode != null && fpdContainerNode.isObject()) {
final ObjectNode fpdContainerObjectNode = (ObjectNode) fpdContainerNode;
final String bidRequestPrefix = BIDREQUEST + ".";
updateWithNormalizedNode(fpdContainerObjectNode, USER, FIRST_ARRAY_ELEMENT_REQUEST_FIELDS,
COMMA_SEPARATED_ELEMENT_FIELDS, BIDREQUEST_PREFIX, warnings);
COMMA_SEPARATED_ELEMENT_FIELDS, bidRequestPrefix, warnings);
updateWithNormalizedNode(fpdContainerObjectNode, APP, FIRST_ARRAY_ELEMENT_REQUEST_FIELDS,
COMMA_SEPARATED_ELEMENT_FIELDS, BIDREQUEST_PREFIX, warnings);
COMMA_SEPARATED_ELEMENT_FIELDS, bidRequestPrefix, warnings);
updateWithNormalizedNode(fpdContainerObjectNode, SITE, FIRST_ARRAY_ELEMENT_REQUEST_FIELDS,
COMMA_SEPARATED_ELEMENT_FIELDS, BIDREQUEST_PREFIX, warnings);
COMMA_SEPARATED_ELEMENT_FIELDS, bidRequestPrefix, warnings);
}
}

private void updateWithNormalizedNode(ObjectNode containerNode, String nodeNameToNormalize,
Map<String, Set<String>> firstArrayElementsFields,
Map<String, Set<String>> commaSeparatedElementFields,
String nodePrefix,
List<String> warnings) {
String nodePrefix, List<String> warnings) {
final JsonNode normalizedNode = normalizeNode(containerNode.get(nodeNameToNormalize), nodeNameToNormalize,
firstArrayElementsFields, commaSeparatedElementFields, nodePrefix, warnings);
if (normalizedNode != null) {
containerNode.set(nodeNameToNormalize, normalizedNode);
} else {
containerNode.remove(nodeNameToNormalize);
}
}

private JsonNode normalizeNode(JsonNode containerNode, String nodeName,
Map<String, Set<String>> firstArrayElementsFields,
Map<String, Set<String>> commaSeparatedElementFields,
String nodePrefix,
List<String> warnings) {
if (containerNode != null && containerNode.isObject()) {
final ObjectNode containerObjectNode = (ObjectNode) containerNode;

CollectionUtils.emptyIfNull(firstArrayElementsFields.get(nodeName))
.forEach(fieldName -> updateWithNormalizedField(containerObjectNode, fieldName,
() -> toFirstElementTextNode(containerObjectNode, fieldName, nodeName, nodePrefix,
warnings)));

CollectionUtils.emptyIfNull(commaSeparatedElementFields.get(nodeName))
.forEach(fieldName -> updateWithNormalizedField(containerObjectNode, fieldName,
() -> toCommaSeparatedTextNode(containerObjectNode, fieldName, nodeName, nodePrefix,
warnings)));
String nodePrefix, List<String> warnings) {
if (containerNode != null) {
if (containerNode.isObject()) {
final ObjectNode containerObjectNode = (ObjectNode) containerNode;

CollectionUtils.emptyIfNull(firstArrayElementsFields.get(nodeName))
.forEach(fieldName -> updateWithNormalizedField(containerObjectNode, fieldName,
() -> toFirstElementTextNode(containerObjectNode, fieldName, nodeName, nodePrefix,
warnings)));

CollectionUtils.emptyIfNull(commaSeparatedElementFields.get(nodeName))
.forEach(fieldName -> updateWithNormalizedField(containerObjectNode, fieldName,
() -> toCommaSeparatedTextNode(containerObjectNode, fieldName, nodeName, nodePrefix,
warnings)));
} else {
warnings.add(String.format("FDP warning: %s%s field ignored. Expected type is object, but was `%s`.",
nodePrefix, nodeName, containerNode.getNodeType().name()));
return null;
}
}
return containerNode;
}

private void updateWithNormalizedField(ObjectNode containerNode, String fieldName,
Supplier<JsonNode> normalizationSupplier) {
final JsonNode normalizedField = normalizationSupplier.get();
if (normalizedField != null) {
if (normalizedField == null) {
containerNode.remove(fieldName);
} else {
containerNode.set(fieldName, normalizedField);
}
}
Expand All @@ -152,18 +198,19 @@ private JsonNode toFirstElementTextNode(ObjectNode containerNode, String fieldNa
return node;
}

if (node.isArray()) {
final ArrayNode arrayNode = (ArrayNode) node;
if (!arrayNode.isEmpty() && isTextualArray(arrayNode)) {
handleWarning(String.format("Incorrect type for first party data field %s%s.%s, expected is string, but"
+ " was an array of strings. Converted to string by taking first element of array.",
nodePrefix, containerName, fieldName), warnings);
return new TextNode(arrayNode.get(0).asText());
}
final boolean isArray = node.isArray();
final ArrayNode arrayNode = isArray ? (ArrayNode) node : null;
final boolean isTextualArray = arrayNode != null && isTextualArray(arrayNode) && !arrayNode.isEmpty();

if (isTextualArray && !arrayNode.isEmpty()) {
warnings.add(String.format("Incorrect type for first party data field %s%s.%s, expected is"
+ " string, but was an array of strings. Converted to string by taking first element "
+ "of array.", nodePrefix, containerName, fieldName));
return new TextNode(arrayNode.get(0).asText());
} else {
warnForExpectedStringArrayType(fieldName, containerName, warnings, nodePrefix, node.getNodeType());
return null;
}
final JsonNodeType nodeType = node.getNodeType();
warnForExpectedStringArrayType(fieldName, containerName, warnings, nodePrefix, nodeType);
return node;
}

private JsonNode toCommaSeparatedTextNode(ObjectNode containerNode, String fieldName, String containerName,
Expand All @@ -173,40 +220,46 @@ private JsonNode toCommaSeparatedTextNode(ObjectNode containerNode, String field
return node;
}

if (node.isArray()) {
final ArrayNode arrayNode = (ArrayNode) node;
if (!arrayNode.isEmpty() && isTextualArray(arrayNode)) {
handleWarning(String.format("Incorrect type for first party data field %s%s.%s, expected is string, but"
+ " was an array of strings. Converted to string by separating values with comma.",
nodePrefix, containerName, fieldName), warnings);
return new TextNode(StreamSupport.stream(arrayNode.spliterator(), false)
.map(jsonNode -> (TextNode) jsonNode)
.map(TextNode::textValue)
.collect(Collectors.joining(",")));
}
final boolean isArray = node.isArray();
final ArrayNode arrayNode = isArray ? (ArrayNode) node : null;
final boolean isTextualArray = arrayNode != null && isTextualArray(arrayNode) && !arrayNode.isEmpty();

if (isTextualArray) {
warnings.add(String.format("Incorrect type for first party data field %s%s.%s, expected is "
+ "string, but was an array of strings. Converted to string by separating values with "
+ "comma.", nodePrefix, containerName, fieldName));
return new TextNode(StreamSupport.stream(arrayNode.spliterator(), false)
.map(jsonNode -> (TextNode) jsonNode)
.map(TextNode::textValue)
.collect(Collectors.joining(",")));
} else {
warnForExpectedStringArrayType(fieldName, containerName, warnings, nodePrefix, node.getNodeType());
return null;
}
final JsonNodeType nodeType = node.getNodeType();
warnForExpectedStringArrayType(fieldName, containerName, warnings, nodePrefix, nodeType);
return node;
}

private void warnForExpectedStringArrayType(String fieldName, String containerName, List<String> warnings,
String nodePrefix, JsonNodeType nodeType) {
handleWarning(String.format("Incorrect type for first party data field %s%s.%s, expected strings, but was `%s`."
+ " Failed to convert to correct type.", nodePrefix, containerName, fieldName,
nodeType == JsonNodeType.ARRAY ? "ARRAY of different types" : nodeType.name()), warnings);
warnings.add(String.format("Incorrect type for first party data field %s%s.%s, expected strings, but"
+ " was `%s`. Failed to convert to correct type.", nodePrefix, containerName, fieldName,
nodeType == JsonNodeType.ARRAY ? "ARRAY of different types" : nodeType.name()));
}

private static boolean isTextualArray(ArrayNode arrayNode) {
return StreamSupport.stream(arrayNode.spliterator(), false).allMatch(JsonNode::isTextual);
}

private void handleWarning(String message, List<String> warnings) {
warnings.add(message);

// log only 1% of cases
if (System.currentTimeMillis() % 100 == 0) {
logger.warn(message);
private void processWarnings(List<String> resolverWarning, List<String> warnings, String containerValue,
String referer, String containerName) {
if (CollectionUtils.isNotEmpty(resolverWarning)) {
warnings.addAll(resolverWarning);
// log only 1% of cases
if (System.currentTimeMillis() % 100 == 0) {
logger.info(String.format("%s. \n Referer = %s and %s = %s",
String.join("\n", resolverWarning),
StringUtils.isNotBlank(referer) ? referer : UNKNOWN_REFERER,
containerName, containerValue));
}
}
}
}
6 changes: 4 additions & 2 deletions src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ FpdResolver fpdResolver(JacksonMapper mapper) {
}

@Bean
OrtbTypesResolver ortbTypesResolver() {
return new OrtbTypesResolver();
OrtbTypesResolver ortbTypesResolver(JacksonMapper jacksonMapper) {
return new OrtbTypesResolver(jacksonMapper);
}

@Bean
Expand Down Expand Up @@ -236,6 +236,7 @@ private static List<String> splitCommaSeparatedString(String listString) {
AmpRequestFactory ampRequestFactory(StoredRequestProcessor storedRequestProcessor,
AuctionRequestFactory auctionRequestFactory,
OrtbTypesResolver ortbTypesResolver,
ImplicitParametersExtractor implicitParametersExtractor,
FpdResolver fpdResolver,
TimeoutResolver timeoutResolver,
JacksonMapper mapper) {
Expand All @@ -244,6 +245,7 @@ AmpRequestFactory ampRequestFactory(StoredRequestProcessor storedRequestProcesso
storedRequestProcessor,
auctionRequestFactory,
ortbTypesResolver,
implicitParametersExtractor,
fpdResolver,
timeoutResolver,
mapper);
Expand Down
Loading

0 comments on commit 60ed657

Please sign in to comment.