From 663f8b52fb98ee760a04740e470f96f6a74d8780 Mon Sep 17 00:00:00 2001 From: Dubyk Danylo <45672370+CTMBNara@users.noreply.github.com> Date: Wed, 8 Jan 2025 17:03:34 +0100 Subject: [PATCH] Refine FPD (#3653) --- .../prebid/server/auction/FpdResolver.java | 229 +------- .../server/auction/OrtbTypesResolver.java | 496 +++++++++--------- .../openrtb/ext/request/ExtBidderConfig.java | 9 +- .../ext/request/ExtBidderConfigFpd.java | 20 - .../server/functional/tests/AmpFpdSpec.groovy | 81 ++- .../server/auction/ExchangeServiceTest.java | 16 +- .../server/auction/FpdResolverTest.java | 126 +---- .../server/auction/OrtbTypesResolverTest.java | 35 +- 8 files changed, 388 insertions(+), 624 deletions(-) delete mode 100644 src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfigFpd.java diff --git a/src/main/java/org/prebid/server/auction/FpdResolver.java b/src/main/java/org/prebid/server/auction/FpdResolver.java index e1e4fa1e436..f0ade099ece 100644 --- a/src/main/java/org/prebid/server/auction/FpdResolver.java +++ b/src/main/java/org/prebid/server/auction/FpdResolver.java @@ -1,29 +1,20 @@ package org.prebid.server.auction; -import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.NullNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.iab.openrtb.request.App; -import com.iab.openrtb.request.Data; import com.iab.openrtb.request.Dooh; import com.iab.openrtb.request.Site; import com.iab.openrtb.request.User; -import org.apache.commons.lang3.ObjectUtils; +import org.prebid.server.exception.InvalidRequestException; import org.prebid.server.json.JacksonMapper; import org.prebid.server.json.JsonMerger; -import org.prebid.server.proto.openrtb.ext.request.ExtApp; -import org.prebid.server.proto.openrtb.ext.request.ExtDooh; -import org.prebid.server.proto.openrtb.ext.request.ExtSite; -import org.prebid.server.proto.openrtb.ext.request.ExtUser; -import java.util.Collections; -import java.util.List; import java.util.Objects; import java.util.Set; -import java.util.function.Function; -import java.util.function.Predicate; -import java.util.stream.StreamSupport; public class FpdResolver { @@ -33,17 +24,8 @@ public class FpdResolver { private static final String APP = "app"; private static final String DOOH = "dooh"; private static final Set KNOWN_FPD_ATTRIBUTES = Set.of(USER, SITE, APP, DOOH, BIDDERS); - private static final String EXT = "ext"; private static final String CONTEXT = "context"; private static final String DATA = "data"; - private static final Set USER_DATA_ATTR = Collections.singleton("geo"); - private static final Set APP_DATA_ATTR = Set.of("id", "content", "publisher", "privacypolicy"); - private static final Set SITE_DATA_ATTR = Set.of("id", "content", "publisher", "privacypolicy", "mobile"); - private static final Set DOOH_DATA_ATTR = Set.of("id", "content", "publisher", "privacypolicy"); - - private static final TypeReference> USER_DATA_TYPE_REFERENCE = - new TypeReference<>() { - }; private final JacksonMapper jacksonMapper; private final JsonMerger jsonMerger; @@ -54,146 +36,37 @@ public FpdResolver(JacksonMapper jacksonMapper, JsonMerger jsonMerger) { } public User resolveUser(User originUser, ObjectNode fpdUser) { - if (fpdUser == null) { - return originUser; - } - final User resultUser = originUser == null ? User.builder().build() : originUser; - final ExtUser resolvedExtUser = resolveUserExt(fpdUser, resultUser); - return resultUser.toBuilder() - .keywords(ObjectUtils.defaultIfNull(getString(fpdUser, "keywords"), resultUser.getKeywords())) - .gender(ObjectUtils.defaultIfNull(getString(fpdUser, "gender"), resultUser.getGender())) - .yob(ObjectUtils.defaultIfNull(getInteger(fpdUser, "yob"), resultUser.getYob())) - .data(ObjectUtils.defaultIfNull(getFpdUserData(fpdUser), resultUser.getData())) - .ext(resolvedExtUser) - .build(); - } - - private ExtUser resolveUserExt(ObjectNode fpdUser, User originUser) { - final ExtUser originExtUser = originUser.getExt(); - final ObjectNode resolvedData = - mergeExtData(fpdUser.path(EXT).path(DATA), originExtUser != null ? originExtUser.getData() : null); - - return updateUserExtDataWithFpdAttr(fpdUser, originExtUser, resolvedData); - } - - private ExtUser updateUserExtDataWithFpdAttr(ObjectNode fpdUser, ExtUser originExtUser, ObjectNode extData) { - final ObjectNode resultData = extData != null ? extData : jacksonMapper.mapper().createObjectNode(); - USER_DATA_ATTR.forEach(attribute -> setAttr(fpdUser, resultData, attribute)); - return originExtUser != null - ? originExtUser.toBuilder().data(resultData.isEmpty() ? null : resultData).build() - : resultData.isEmpty() ? null : ExtUser.builder().data(resultData).build(); - } - - private List getFpdUserData(ObjectNode fpdUser) { - final ArrayNode fpdUserDataNode = getValueFromJsonNode( - fpdUser, DATA, node -> (ArrayNode) node, JsonNode::isArray); - - return toList(fpdUserDataNode, USER_DATA_TYPE_REFERENCE); + return mergeFpd(originUser, fpdUser, User.class); } public App resolveApp(App originApp, ObjectNode fpdApp) { - if (fpdApp == null) { - return originApp; - } - final App resultApp = originApp == null ? App.builder().build() : originApp; - final ExtApp resolvedExtApp = resolveAppExt(fpdApp, resultApp); - return resultApp.toBuilder() - .name(ObjectUtils.defaultIfNull(getString(fpdApp, "name"), resultApp.getName())) - .bundle(ObjectUtils.defaultIfNull(getString(fpdApp, "bundle"), resultApp.getBundle())) - .storeurl(ObjectUtils.defaultIfNull(getString(fpdApp, "storeurl"), resultApp.getStoreurl())) - .domain(ObjectUtils.defaultIfNull(getString(fpdApp, "domain"), resultApp.getDomain())) - .cat(ObjectUtils.defaultIfNull(getStrings(fpdApp, "cat"), resultApp.getCat())) - .sectioncat(ObjectUtils.defaultIfNull(getStrings(fpdApp, "sectioncat"), resultApp.getSectioncat())) - .pagecat(ObjectUtils.defaultIfNull(getStrings(fpdApp, "pagecat"), resultApp.getPagecat())) - .keywords(ObjectUtils.defaultIfNull(getString(fpdApp, "keywords"), resultApp.getKeywords())) - .ext(resolvedExtApp) - .build(); - } - - private ExtApp resolveAppExt(ObjectNode fpdApp, App originApp) { - final ExtApp originExtApp = originApp.getExt(); - final ObjectNode resolvedData = - mergeExtData(fpdApp.path(EXT).path(DATA), originExtApp != null ? originExtApp.getData() : null); - - return updateAppExtDataWithFpdAttr(fpdApp, originExtApp, resolvedData); - } - - private ExtApp updateAppExtDataWithFpdAttr(ObjectNode fpdApp, ExtApp originExtApp, ObjectNode extData) { - final ObjectNode resultData = extData != null ? extData : jacksonMapper.mapper().createObjectNode(); - APP_DATA_ATTR.forEach(attribute -> setAttr(fpdApp, resultData, attribute)); - return originExtApp != null - ? ExtApp.of(originExtApp.getPrebid(), resultData.isEmpty() ? null : resultData) - : resultData.isEmpty() ? null : ExtApp.of(null, resultData); + return mergeFpd(originApp, fpdApp, App.class); } public Site resolveSite(Site originSite, ObjectNode fpdSite) { - if (fpdSite == null) { - return originSite; - } - final Site resultSite = originSite == null ? Site.builder().build() : originSite; - final ExtSite resolvedExtSite = resolveSiteExt(fpdSite, resultSite); - return resultSite.toBuilder() - .name(ObjectUtils.defaultIfNull(getString(fpdSite, "name"), resultSite.getName())) - .domain(ObjectUtils.defaultIfNull(getString(fpdSite, "domain"), resultSite.getDomain())) - .cat(ObjectUtils.defaultIfNull(getStrings(fpdSite, "cat"), resultSite.getCat())) - .sectioncat(ObjectUtils.defaultIfNull(getStrings(fpdSite, "sectioncat"), resultSite.getSectioncat())) - .pagecat(ObjectUtils.defaultIfNull(getStrings(fpdSite, "pagecat"), resultSite.getPagecat())) - .page(ObjectUtils.defaultIfNull(getString(fpdSite, "page"), resultSite.getPage())) - .keywords(ObjectUtils.defaultIfNull(getString(fpdSite, "keywords"), resultSite.getKeywords())) - .ref(ObjectUtils.defaultIfNull(getString(fpdSite, "ref"), resultSite.getRef())) - .search(ObjectUtils.defaultIfNull(getString(fpdSite, "search"), resultSite.getSearch())) - .ext(resolvedExtSite) - .build(); - } - - private ExtSite resolveSiteExt(ObjectNode fpdSite, Site originSite) { - final ExtSite originExtSite = originSite.getExt(); - final ObjectNode resolvedData = - mergeExtData(fpdSite.path(EXT).path(DATA), originExtSite != null ? originExtSite.getData() : null); - - return updateSiteExtDataWithFpdAttr(fpdSite, originExtSite, resolvedData); - } - - private ExtSite updateSiteExtDataWithFpdAttr(ObjectNode fpdSite, ExtSite originExtSite, ObjectNode extData) { - final ObjectNode resultData = extData != null ? extData : jacksonMapper.mapper().createObjectNode(); - SITE_DATA_ATTR.forEach(attribute -> setAttr(fpdSite, resultData, attribute)); - return originExtSite != null - ? ExtSite.of(originExtSite.getAmp(), resultData.isEmpty() ? null : resultData) - : resultData.isEmpty() ? null : ExtSite.of(null, resultData); + return mergeFpd(originSite, fpdSite, Site.class); } public Dooh resolveDooh(Dooh originDooh, ObjectNode fpdDooh) { - if (fpdDooh == null) { - return originDooh; - } - final Dooh resultDooh = originDooh == null ? Dooh.builder().build() : originDooh; - final ExtDooh resolvedExtDooh = resolveDoohExt(fpdDooh, resultDooh); - return resultDooh.toBuilder() - .name(ObjectUtils.defaultIfNull(getString(fpdDooh, "name"), resultDooh.getName())) - .venuetype(ObjectUtils.defaultIfNull(getStrings(fpdDooh, "venuetype"), resultDooh.getVenuetype())) - .venuetypetax(ObjectUtils.defaultIfNull( - getInteger(fpdDooh, "venuetypetax"), - resultDooh.getVenuetypetax())) - .domain(ObjectUtils.defaultIfNull(getString(fpdDooh, "domain"), resultDooh.getDomain())) - .keywords(ObjectUtils.defaultIfNull(getString(fpdDooh, "keywords"), resultDooh.getKeywords())) - .ext(resolvedExtDooh) - .build(); + return mergeFpd(originDooh, fpdDooh, Dooh.class); } - private ExtDooh resolveDoohExt(ObjectNode fpdDooh, Dooh originDooh) { - final ExtDooh originExtDooh = originDooh.getExt(); - final ObjectNode resolvedData = - mergeExtData(fpdDooh.path(EXT).path(DATA), originExtDooh != null ? originExtDooh.getData() : null); + private T mergeFpd(T original, ObjectNode fpd, Class tClass) { + if (fpd == null || fpd.isNull() || fpd.isMissingNode()) { + return original; + } - return updateDoohExtDataWithFpdAttr(fpdDooh, originExtDooh, resolvedData); - } + final ObjectMapper mapper = jacksonMapper.mapper(); - private ExtDooh updateDoohExtDataWithFpdAttr(ObjectNode fpdDooh, ExtDooh originExtDooh, ObjectNode extData) { - final ObjectNode resultData = extData != null ? extData : jacksonMapper.mapper().createObjectNode(); - DOOH_DATA_ATTR.forEach(attribute -> setAttr(fpdDooh, resultData, attribute)); - return originExtDooh != null - ? ExtDooh.of(resultData.isEmpty() ? null : resultData) - : resultData.isEmpty() ? null : ExtDooh.of(resultData); + final JsonNode originalAsJsonNode = original != null + ? mapper.valueToTree(original) + : NullNode.getInstance(); + final JsonNode merged = jsonMerger.merge(fpd, originalAsJsonNode); + try { + return mapper.treeToValue(merged, tClass); + } catch (JsonProcessingException e) { + throw new InvalidRequestException("Can't convert merging result class " + tClass.getName()); + } } public ObjectNode resolveImpExt(ObjectNode impExt, ObjectNode targeting) { @@ -277,62 +150,4 @@ private void removeOrReplace(ObjectNode impExt, String field, JsonNode jsonNode) impExt.set(field, jsonNode); } } - - private ObjectNode mergeExtData(JsonNode fpdData, JsonNode originData) { - if (fpdData.isMissingNode() || !fpdData.isObject()) { - return originData != null && originData.isObject() ? ((ObjectNode) originData).deepCopy() : null; - } - - if (originData != null && originData.isObject()) { - return (ObjectNode) jsonMerger.merge(fpdData, originData); - } - return fpdData.isObject() ? (ObjectNode) fpdData : null; - } - - private static void setAttr(ObjectNode source, ObjectNode dest, String fieldName) { - final JsonNode field = source.get(fieldName); - if (field != null) { - dest.set(fieldName, field); - } - } - - private static List getStrings(JsonNode firstItem, String fieldName) { - final JsonNode valueNode = firstItem.get(fieldName); - final ArrayNode arrayNode = valueNode != null && valueNode.isArray() ? (ArrayNode) valueNode : null; - return arrayNode != null && isTextualArray(arrayNode) - ? StreamSupport.stream(arrayNode.spliterator(), false) - .map(JsonNode::asText) - .toList() - : null; - } - - private static boolean isTextualArray(ArrayNode arrayNode) { - return StreamSupport.stream(arrayNode.spliterator(), false).allMatch(JsonNode::isTextual); - } - - private static String getString(ObjectNode firstItem, String fieldName) { - return getValueFromJsonNode(firstItem, fieldName, JsonNode::asText, JsonNode::isTextual); - } - - private static Integer getInteger(ObjectNode firstItem, String fieldName) { - return getValueFromJsonNode(firstItem, fieldName, JsonNode::asInt, JsonNode::isInt); - } - - private List toList(JsonNode node, TypeReference> listTypeReference) { - try { - return jacksonMapper.mapper().convertValue(node, listTypeReference); - } catch (IllegalArgumentException e) { - return null; - } - } - - private static T getValueFromJsonNode(ObjectNode firstItem, String fieldName, - Function nodeConverter, - Predicate isCorrectType) { - final JsonNode valueNode = firstItem.get(fieldName); - return valueNode != null && isCorrectType.test(valueNode) - ? nodeConverter.apply(valueNode) - : null; - } - } diff --git a/src/main/java/org/prebid/server/auction/OrtbTypesResolver.java b/src/main/java/org/prebid/server/auction/OrtbTypesResolver.java index 35668bbf0f4..07ac2ce7015 100644 --- a/src/main/java/org/prebid/server/auction/OrtbTypesResolver.java +++ b/src/main/java/org/prebid/server/auction/OrtbTypesResolver.java @@ -1,5 +1,6 @@ package org.prebid.server.auction; +import com.fasterxml.jackson.core.JsonPointer; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; @@ -14,19 +15,18 @@ import org.prebid.server.log.ConditionalLogger; import org.prebid.server.log.Logger; import org.prebid.server.log.LoggerFactory; +import org.prebid.server.util.StreamUtil; 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.Optional; import java.util.Set; -import java.util.function.Supplier; +import java.util.function.BiFunction; +import java.util.function.Function; import java.util.stream.Collectors; -import java.util.stream.StreamSupport; /** * Service resolves types inconsistency and cast them if possible to ortb2 protocol. @@ -40,36 +40,32 @@ 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 EXT = "ext"; + private static final String DATA = "data"; + private static final String CONFIG = "config"; + private static final String FPD = "fpd"; + private static final String ORTB2 = "ortb2"; private static final String CONTEXT = "context"; - private static final String BIDREQUEST = "bidrequest"; - private static final String TARGETING = "targeting"; private static final String UNKNOWN_REFERER = "unknown referer"; - private static final String DATA = "data"; - private static final String EXT = "ext"; - private static final Map> FIRST_ARRAY_ELEMENT_STANDARD_FIELDS; - private static final Map> FIRST_ARRAY_ELEMENT_REQUEST_FIELDS; + private static final JsonPointer EXT_PREBID_BIDDER_CONFIG = JsonPointer.valueOf("/ext/prebid/bidderconfig"); + private static final JsonPointer CONFIG_ORTB2 = JsonPointer.valueOf("/config/ortb2"); + private static final JsonPointer APP_BUNDLE = JsonPointer.valueOf("/app/bundle"); + private static final JsonPointer SITE_PAGE = JsonPointer.valueOf("/site/page"); + + private static final Map> FIRST_ARRAY_ELEMENT_FIELDS; private static final Map> COMMA_SEPARATED_ELEMENT_FIELDS; static { - FIRST_ARRAY_ELEMENT_REQUEST_FIELDS = new HashMap<>(); - FIRST_ARRAY_ELEMENT_REQUEST_FIELDS.put(USER, new HashSet<>(Collections.singleton("gender"))); - FIRST_ARRAY_ELEMENT_REQUEST_FIELDS.put(APP, new HashSet<>(Arrays.asList("id", "name", "bundle", "storeurl", - "domain"))); - FIRST_ARRAY_ELEMENT_REQUEST_FIELDS.put(SITE, new HashSet<>(Arrays.asList("id", "name", "domain", "page", - "ref", "search"))); - - FIRST_ARRAY_ELEMENT_STANDARD_FIELDS = new HashMap<>(); - FIRST_ARRAY_ELEMENT_STANDARD_FIELDS.put(USER, new HashSet<>(Collections.singleton("gender"))); - FIRST_ARRAY_ELEMENT_STANDARD_FIELDS.put(APP, new HashSet<>(Arrays.asList("name", "bundle", "storeurl", - "domain"))); - FIRST_ARRAY_ELEMENT_STANDARD_FIELDS.put(SITE, new HashSet<>(Arrays.asList("name", "domain", "page", "ref", - "search"))); - - COMMA_SEPARATED_ELEMENT_FIELDS = new HashMap<>(); - COMMA_SEPARATED_ELEMENT_FIELDS.put(USER, Collections.singleton("keywords")); - COMMA_SEPARATED_ELEMENT_FIELDS.put(APP, Collections.singleton("keywords")); - COMMA_SEPARATED_ELEMENT_FIELDS.put(SITE, Collections.singleton("keywords")); + FIRST_ARRAY_ELEMENT_FIELDS = Map.of( + USER, Collections.singleton("gender"), + APP, Set.of("id", "name", "bundle", "storeurl", "domain"), + SITE, Set.of("id", "name", "domain", "page", "ref", "search")); + + COMMA_SEPARATED_ELEMENT_FIELDS = Map.of( + USER, Collections.singleton("keywords"), + APP, Collections.singleton("keywords"), + SITE, Collections.singleton("keywords")); } private final double logSamplingRate; @@ -83,291 +79,306 @@ public OrtbTypesResolver(double logSamplingRate, JacksonMapper jacksonMapper, Js this.jsonMerger = Objects.requireNonNull(jsonMerger); } - /** - * Resolves fields types inconsistency to ortb2 protocol for {@param bidRequest} for bidRequest level parameters - * and bidderconfig. - * Mutates both parameters, {@param fpdContainerNode} and {@param warnings}. - */ public void normalizeBidRequest(JsonNode bidRequest, List warnings, String referer) { final List resolverWarnings = new ArrayList<>(); - final String rowOriginBidRequest = getOriginalRowContainerNode(bidRequest); - normalizeRequestFpdFields(bidRequest, resolverWarnings); - final JsonNode bidderConfigs = bidRequest.path("ext").path("prebid").path("bidderconfig"); + + normalizeFpdFields(bidRequest, "bidrequest.", resolverWarnings); + + final String source = source(bidRequest); + final JsonNode bidderConfigs = bidRequest.at(EXT_PREBID_BIDDER_CONFIG); if (!bidderConfigs.isMissingNode() && bidderConfigs.isArray()) { for (JsonNode bidderConfig : bidderConfigs) { + mergeFpdFieldsToOrtb2(bidderConfig, source); - mergeFpdFieldsToOrtb2(bidderConfig); - - final JsonNode ortb2Config = bidderConfig.path("config").path("ortb2"); + final JsonNode ortb2Config = bidderConfig.at(CONFIG_ORTB2); if (!ortb2Config.isMissingNode()) { - normalizeStandardFpdFields(ortb2Config, resolverWarnings, "bidrequest.ext.prebid.bidderconfig"); + normalizeFpdFields(ortb2Config, "bidrequest.ext.prebid.bidderconfig.", resolverWarnings); } } } - processWarnings(resolverWarnings, warnings, rowOriginBidRequest, referer, BIDREQUEST); + + processWarnings(resolverWarnings, warnings, referer, "bidrequest", getOriginalRowContainerNode(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"); + private void normalizeFpdFields(JsonNode fpdContainerNode, String prefix, List warnings) { + if (fpdContainerNode != null && fpdContainerNode.isObject()) { + final ObjectNode fpdContainerObjectNode = (ObjectNode) fpdContainerNode; + updateFpdWithNormalizedNode(fpdContainerObjectNode, USER, warnings, prefix); + updateFpdWithNormalizedNode(fpdContainerObjectNode, APP, warnings, prefix); + updateFpdWithNormalizedNode(fpdContainerObjectNode, SITE, warnings, prefix); } } - /** - * Merges fpd fields into ortb2: - * config.fpd.context -> config.ortb2.site - * config.fpd.user -> config.ortb2.user - */ - private void mergeFpdFieldsToOrtb2(JsonNode bidderConfig) { - final JsonNode config = bidderConfig.path("config"); - final JsonNode configFpd = config.path("fpd"); - - if (configFpd.isMissingNode()) { - return; - } + private static String source(JsonNode bidRequest) { + return Optional.ofNullable(stringAt(bidRequest, APP_BUNDLE)) + .orElseGet(() -> stringAt(bidRequest, SITE_PAGE)); + } - final JsonNode configOrtb = config.path("ortb2"); + private static String stringAt(JsonNode node, JsonPointer path) { + final JsonNode at = node.at(path); + return at.isMissingNode() || at.isNull() || !at.isTextual() + ? null + : at.textValue(); + } - final JsonNode fpdContext = configFpd.get(CONTEXT); - final JsonNode ortbSite = configOrtb.get(SITE); - final JsonNode updatedOrtbSite = ortbSite == null - ? fpdContext - : fpdContext != null ? jsonMerger.merge(fpdContext, ortbSite) : null; + private void updateFpdWithNormalizedNode(ObjectNode containerNode, + String nodeNameToNormalize, + List warnings, + String nodePrefix) { + + updateWithNormalizedNode( + containerNode, + nodeNameToNormalize, + normalizeNode( + containerNode.get(nodeNameToNormalize), + nodeNameToNormalize, + warnings, + nodePrefix)); + } - final JsonNode fpdUser = configFpd.get(USER); - final JsonNode ortbUser = configOrtb.get(USER); - final JsonNode updatedOrtbUser = ortbUser == null - ? fpdUser - : fpdUser != null ? jsonMerger.merge(fpdUser, ortbUser) : null; + private static void updateWithNormalizedNode(ObjectNode containerNode, + String fieldName, + JsonNode normalizedNode) { - if (updatedOrtbUser == null && updatedOrtbSite == null) { - return; + if (normalizedNode == null) { + containerNode.remove(fieldName); + } else { + containerNode.set(fieldName, normalizedNode); } + } - final ObjectNode ortbObjectNode = configOrtb.isMissingNode() - ? jacksonMapper.mapper().createObjectNode() - : (ObjectNode) configOrtb; - - if (updatedOrtbSite != null) { - ortbObjectNode.set(SITE, updatedOrtbSite); + private JsonNode normalizeNode(JsonNode containerNode, String nodeName, List warnings, String nodePrefix) { + if (containerNode == null) { + return null; } + if (!containerNode.isObject()) { + warnings.add("%s%s field ignored. Expected type is object, but was `%s`." + .formatted(nodePrefix, nodeName, containerNode.getNodeType().name())); - if (updatedOrtbUser != null) { - ortbObjectNode.set(USER, updatedOrtbUser); + return null; } - ((ObjectNode) config).set("ortb2", ortbObjectNode); - } + final ObjectNode containerObjectNode = (ObjectNode) containerNode; - /** - * Resolves fields types inconsistency to ortb2 protocol for {@param targeting}. - * Mutates both parameters, {@param targeting} and {@param warnings}. - */ - public void normalizeTargeting(JsonNode targeting, List warnings, String referer) { - final List resolverWarnings = new ArrayList<>(); - final String rowOriginTargeting = getOriginalRowContainerNode(targeting); - normalizeStandardFpdFields(targeting, resolverWarnings, TARGETING); - processWarnings(resolverWarnings, warnings, rowOriginTargeting, referer, TARGETING); - } + normalizeFields( + FIRST_ARRAY_ELEMENT_FIELDS, + nodeName, + containerObjectNode, + (name, node) -> toFirstElementTextNode(name, node, warnings, nodePrefix, nodeName)); + normalizeFields( + COMMA_SEPARATED_ELEMENT_FIELDS, + nodeName, + containerObjectNode, + (name, node) -> toCommaSeparatedTextNode(name, node, warnings, nodePrefix, nodeName)); - /** - * Resolves fields types inconsistency to ortb2 protocol for {@param fpdContainerNode}. - * Mutates both parameters, {@param fpdContainerNode} and {@param warnings}. - */ - private void normalizeStandardFpdFields(JsonNode fpdContainerNode, List warnings, String nodePrefix) { - final String normalizedNodePrefix = nodePrefix.endsWith(".") ? nodePrefix : nodePrefix.concat("."); - if (fpdContainerNode != null && fpdContainerNode.isObject()) { - final ObjectNode fpdContainerObjectNode = (ObjectNode) fpdContainerNode; - updateWithNormalizedNode(fpdContainerObjectNode, USER, FIRST_ARRAY_ELEMENT_STANDARD_FIELDS, - COMMA_SEPARATED_ELEMENT_FIELDS, normalizedNodePrefix, warnings); - updateWithNormalizedNode(fpdContainerObjectNode, APP, FIRST_ARRAY_ELEMENT_STANDARD_FIELDS, - COMMA_SEPARATED_ELEMENT_FIELDS, normalizedNodePrefix, warnings); - updateWithNormalizedNode(fpdContainerObjectNode, SITE, FIRST_ARRAY_ELEMENT_STANDARD_FIELDS, - COMMA_SEPARATED_ELEMENT_FIELDS, normalizedNodePrefix, warnings); - } - } + normalizeDataExtension(containerObjectNode, warnings, nodePrefix, nodeName); - private void normalizeRequestFpdFields(JsonNode fpdContainerNode, List 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, bidRequestPrefix, warnings); - updateWithNormalizedNode(fpdContainerObjectNode, APP, FIRST_ARRAY_ELEMENT_REQUEST_FIELDS, - COMMA_SEPARATED_ELEMENT_FIELDS, bidRequestPrefix, warnings); - updateWithNormalizedNode(fpdContainerObjectNode, SITE, FIRST_ARRAY_ELEMENT_REQUEST_FIELDS, - COMMA_SEPARATED_ELEMENT_FIELDS, bidRequestPrefix, warnings); - } + return containerNode; } - private void updateWithNormalizedNode(ObjectNode containerNode, String nodeNameToNormalize, - Map> firstArrayElementsFields, - Map> commaSeparatedElementFields, - String nodePrefix, List 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 static void normalizeFields(Map> nodeNameToFields, + String nodeName, + ObjectNode containerObjectNode, + BiFunction fieldNormalizer) { - private JsonNode normalizeNode(JsonNode containerNode, String nodeName, - Map> firstArrayElementsFields, - Map> commaSeparatedElementFields, - String nodePrefix, List 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))); - - normalizeDataExtension(containerObjectNode, nodeName, nodePrefix, warnings); - } else { - warnings.add("%s%s field ignored. Expected type is object, but was `%s`." - .formatted(nodePrefix, nodeName, containerNode.getNodeType().name())); - return null; - } - } - return containerNode; + nodeNameToFields.get(nodeName) + .forEach(fieldName -> updateWithNormalizedNode( + containerObjectNode, + fieldName, + fieldNormalizer.apply(fieldName, containerObjectNode.get(fieldName)))); } - private void updateWithNormalizedField(ObjectNode containerNode, String fieldName, - Supplier normalizationSupplier) { - final JsonNode normalizedField = normalizationSupplier.get(); - if (normalizedField == null) { - containerNode.remove(fieldName); - } else { - containerNode.set(fieldName, normalizedField); - } + private static TextNode toFirstElementTextNode(String fieldName, + JsonNode fieldNode, + List warnings, + String nodePrefix, + String containerName) { + + return toTextNode( + fieldName, + fieldNode, + arrayNode -> arrayNode.get(0).asText(), + warnings, + nodePrefix, + containerName, + "Converted to string by taking first element of array."); } - private JsonNode toFirstElementTextNode(ObjectNode containerNode, - String fieldName, - String containerName, - String nodePrefix, - List warnings) { + private static TextNode toTextNode(String fieldName, + JsonNode fieldNode, + Function mapper, + List warnings, + String nodePrefix, + String containerName, + String action) { - final JsonNode node = containerNode.get(fieldName); - if (node == null || node.isNull() || node.isTextual()) { - return node; + if (fieldNode == null || fieldNode.isNull()) { + return null; } - final boolean isArray = node.isArray(); - final ArrayNode arrayNode = isArray ? (ArrayNode) node : null; - final boolean isTextualArray = arrayNode != null && isTextualArray(arrayNode) && !arrayNode.isEmpty(); + if (fieldNode.isTextual()) { + return (TextNode) fieldNode; + } + + final ArrayNode arrayNode = fieldNode.isArray() ? (ArrayNode) fieldNode : null; + final boolean isTextualArray = arrayNode != null && !arrayNode.isEmpty() && isTextualArray(arrayNode); - if (isTextualArray && !arrayNode.isEmpty()) { + if (isTextualArray) { warnings.add(""" 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.""" - .formatted(nodePrefix, containerName, fieldName)); - return new TextNode(arrayNode.get(0).asText()); + but was an array of strings. %s""" + .formatted(nodePrefix, containerName, fieldName, action)); + + return new TextNode(mapper.apply(arrayNode)); } else { - warnForExpectedStringArrayType(fieldName, containerName, warnings, nodePrefix, node.getNodeType()); + warnForExpectedStringArrayType(warnings, nodePrefix, containerName, fieldName, fieldNode.getNodeType()); return null; } } - private JsonNode toCommaSeparatedTextNode(ObjectNode containerNode, - String fieldName, - String containerName, - String nodePrefix, - List warnings) { + private static boolean isTextualArray(ArrayNode arrayNode) { + return StreamUtil.asStream(arrayNode.iterator()).allMatch(JsonNode::isTextual); + } - final JsonNode node = containerNode.get(fieldName); - if (node == null || node.isNull() || node.isTextual()) { - return node; - } + private static void warnForExpectedStringArrayType(List warnings, + String nodePrefix, + String containerName, + String fieldName, + JsonNodeType nodeType) { - final boolean isArray = node.isArray(); - final ArrayNode arrayNode = isArray ? (ArrayNode) node : null; - final boolean isTextualArray = arrayNode != null && isTextualArray(arrayNode) && !arrayNode.isEmpty(); + warnings.add(""" + Incorrect type for first party data field %s%s.%s, expected strings, \ + but was `%s`. Failed to convert to correct type.""".formatted( + nodePrefix, + containerName, + fieldName, + nodeType == JsonNodeType.ARRAY ? "ARRAY of different types" : nodeType.name())); + } - if (isTextualArray) { - warnings.add(""" - 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.""" - .formatted(nodePrefix, containerName, fieldName)); + private static TextNode toCommaSeparatedTextNode(String fieldName, + JsonNode fieldNode, + List warnings, + String nodePrefix, + String containerName) { - 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; - } + return toTextNode( + fieldName, + fieldNode, + arrayNode -> StreamUtil.asStream(arrayNode.spliterator()) + .map(TextNode.class::cast) + .map(TextNode::textValue) + .collect(Collectors.joining(",")), + warnings, + nodePrefix, + containerName, + "Converted to string by separating values with comma."); } - private void normalizeDataExtension(ObjectNode containerNode, String containerName, String nodePrefix, - List warnings) { + private void normalizeDataExtension(ObjectNode containerNode, + List warnings, + String nodePrefix, + String containerName) { + final JsonNode data = containerNode.get(DATA); if (data == null || !data.isObject()) { return; } + final JsonNode extData = containerNode.path(EXT).path(DATA); final JsonNode ext = containerNode.get(EXT); if (!extData.isNull() && !extData.isMissingNode()) { final JsonNode resolvedExtData = jsonMerger.merge(data, extData); ((ObjectNode) ext).set(DATA, resolvedExtData); } else { - copyDataToExtData(containerNode, containerName, nodePrefix, warnings, data); + copyDataToExtData(containerNode, data, warnings, nodePrefix, containerName); } + containerNode.remove(DATA); } - private void copyDataToExtData(ObjectNode containerNode, String containerName, String nodePrefix, - List warnings, JsonNode data) { + private void copyDataToExtData(ObjectNode containerNode, + JsonNode data, + List warnings, + String nodePrefix, + String containerName) { + final JsonNode ext = containerNode.get(EXT); - if (ext != null && ext.isObject()) { - ((ObjectNode) ext).set(DATA, data); - } else if (ext != null && !ext.isObject()) { + if (ext == null) { + createExtAndCopyData(containerNode, data); + } else if (!ext.isObject()) { warnings.add(""" Incorrect type for first party data field %s%s.%s, \ expected is object, but was %s. Replaced with object""" .formatted(nodePrefix, containerName, EXT, ext.getNodeType())); - containerNode.set(EXT, jacksonMapper.mapper().createObjectNode().set(DATA, data)); + createExtAndCopyData(containerNode, data); } else { - containerNode.set(EXT, jacksonMapper.mapper().createObjectNode().set(DATA, data)); + ((ObjectNode) ext).set(DATA, data); } } - private void warnForExpectedStringArrayType(String fieldName, String containerName, List warnings, - String nodePrefix, JsonNodeType nodeType) { - warnings.add(""" - Incorrect type for first party data field %s%s.%s, expected strings, \ - but was `%s`. Failed to convert to correct type.""".formatted( - nodePrefix, - containerName, - fieldName, - nodeType == JsonNodeType.ARRAY ? "ARRAY of different types" : nodeType.name())); + private void createExtAndCopyData(ObjectNode containerNode, JsonNode data) { + containerNode.set(EXT, jacksonMapper.mapper().createObjectNode().set(DATA, data)); } - private static boolean isTextualArray(ArrayNode arrayNode) { - return StreamSupport.stream(arrayNode.spliterator(), false).allMatch(JsonNode::isTextual); + private void mergeFpdFieldsToOrtb2(JsonNode bidderConfig, String source) { + final JsonNode config = bidderConfig.path(CONFIG); + final JsonNode configFpd = config.path(FPD); + + if (configFpd.isMissingNode()) { + return; + } + + logDeprecatedFpdConfig(source); + + final JsonNode configOrtb = config.path(ORTB2); + final JsonNode updatedOrtbSite = updatedOrtb2Node(configFpd, CONTEXT, configOrtb, SITE); + final JsonNode updatedOrtbUser = updatedOrtb2Node(configFpd, USER, configOrtb, USER); + + if (updatedOrtbUser == null && updatedOrtbSite == null) { + return; + } + + final ObjectNode ortbObjectNode = configOrtb.isMissingNode() + ? jacksonMapper.mapper().createObjectNode() + : (ObjectNode) configOrtb; + + setIfNotNull(ortbObjectNode, SITE, updatedOrtbSite); + setIfNotNull(ortbObjectNode, USER, updatedOrtbUser); + + ((ObjectNode) config).set(ORTB2, ortbObjectNode); + } + + private void logDeprecatedFpdConfig(String source) { + final String messagePart = source != null ? " on " + source : StringUtils.EMPTY; + ORTB_TYPES_RESOLVING_LOGGER.warn("Usage of deprecated FPD config path" + messagePart, logSamplingRate); + } + + private JsonNode updatedOrtb2Node(JsonNode configFpd, String fpdField, JsonNode configOrtb, String ortbField) { + final JsonNode fpdNode = configFpd.get(fpdField); + final JsonNode ortbNode = configOrtb.get(ortbField); + return ortbNode == null + ? fpdNode + : fpdNode != null ? jsonMerger.merge(ortbNode, fpdNode) : null; + } + + private static void setIfNotNull(ObjectNode destination, String fieldName, JsonNode data) { + if (data != null) { + destination.set(fieldName, data); + } } - private void processWarnings(List resolverWarning, List warnings, String containerValue, - String referer, String containerName) { - if (CollectionUtils.isNotEmpty(resolverWarning)) { - warnings.addAll(updateWithWarningPrefix(resolverWarning)); - // log only 1% of cases + private void processWarnings(List resolverWarnings, + List warnings, + String referer, + String containerName, + String containerValue) { + + if (CollectionUtils.isNotEmpty(resolverWarnings)) { + warnings.addAll(updateWithWarningPrefix(resolverWarnings)); + ORTB_TYPES_RESOLVING_LOGGER.warn( "WARNINGS: %s. \n Referer = %s and %s = %s".formatted( - String.join("\n", resolverWarning), + String.join("\n", resolverWarnings), StringUtils.isNotBlank(referer) ? referer : UNKNOWN_REFERER, containerName, containerValue), @@ -375,7 +386,22 @@ private void processWarnings(List resolverWarning, List warnings } } - private List updateWithWarningPrefix(List resolverWarning) { + private static List updateWithWarningPrefix(List resolverWarning) { return resolverWarning.stream().map(warning -> "WARNING: " + warning).toList(); } + + 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"); + } + } + + public void normalizeTargeting(JsonNode targeting, List warnings, String referer) { + final List resolverWarnings = new ArrayList<>(); + normalizeFpdFields(targeting, "targeting.", resolverWarnings); + processWarnings(resolverWarnings, warnings, referer, "targeting", getOriginalRowContainerNode(targeting)); + } } diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfig.java b/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfig.java index 3a66ac2374e..49c68685625 100644 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfig.java +++ b/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfig.java @@ -1,17 +1,10 @@ package org.prebid.server.proto.openrtb.ext.request; -import lombok.AllArgsConstructor; import lombok.Value; -@AllArgsConstructor(staticName = "of") -@Value +@Value(staticConstructor = "of") public class ExtBidderConfig { - /** - * Defines the contract for bidrequest.ext.prebid.bidderconfig.config.fpd - */ - ExtBidderConfigFpd fpd; - /** * Defines the contract for bidrequest.ext.prebid.bidderconfig.config.ortb2 */ diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfigFpd.java b/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfigFpd.java deleted file mode 100644 index 27748982011..00000000000 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtBidderConfigFpd.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.prebid.server.proto.openrtb.ext.request; - -import com.fasterxml.jackson.databind.JsonNode; -import lombok.AllArgsConstructor; -import lombok.Value; - -@AllArgsConstructor(staticName = "of") -@Value -public class ExtBidderConfigFpd { - - /** - * Defines the contract for bidrequest.ext.prebid.bidderconfig.config.fpd.context - */ - JsonNode context; - - /** - * Defines the contract for bidrequest.ext.prebid.bidderconfig.config.fpd.user - */ - JsonNode user; -} diff --git a/src/test/groovy/org/prebid/server/functional/tests/AmpFpdSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/AmpFpdSpec.groovy index 762c55fe879..e376f611e84 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/AmpFpdSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/AmpFpdSpec.groovy @@ -13,6 +13,7 @@ import org.prebid.server.functional.model.request.auction.Geo import org.prebid.server.functional.model.request.auction.ImpExtContext import org.prebid.server.functional.model.request.auction.ImpExtContextData import org.prebid.server.functional.model.request.auction.ImpExtContextDataAdServer +import org.prebid.server.functional.model.request.auction.Publisher import org.prebid.server.functional.model.request.auction.Site import org.prebid.server.functional.model.request.auction.User import org.prebid.server.functional.service.PrebidServerException @@ -333,11 +334,14 @@ class AmpFpdSpec extends BaseSpec { given: "AMP request" def ampRequest = new AmpRequest(tagId: PBSUtils.randomString) + and: "Amp stored request with FPD data" + def fpdSite = Site.rootFPDSite + def fpdUser = User.rootFPDUser def ampStoredRequest = BidRequest.getDefaultBidRequest(SITE).tap { ext.prebid.tap { data = new ExtRequestPrebidData(bidders: [extRequestPrebidDataBidder]) bidderConfig = [new ExtPrebidBidderConfig(bidders: [prebidBidderConfigBidder], config: new BidderConfig( - ortb2: new BidderConfigOrtb(site: Site.configFPDSite, user: User.configFPDUser)))] + ortb2: new BidderConfigOrtb(site: fpdSite, user: fpdUser)))] } } @@ -350,25 +354,24 @@ class AmpFpdSpec extends BaseSpec { then: "Bidder request should contain certain FPD field from the stored request" def bidderRequest = bidder.getBidderRequest(ampStoredRequest.id) - def ortb2 = ampStoredRequest.ext.prebid.bidderConfig[0].config.ortb2 verifyAll(bidderRequest) { - ortb2.site.name == site.name - ortb2.site.domain == site.domain - ortb2.site.cat == site.cat - ortb2.site.sectionCat == site.sectionCat - ortb2.site.pageCat == site.pageCat - ortb2.site.page == site.page - ortb2.site.ref == site.ref - ortb2.site.search == site.search - ortb2.site.keywords == site.keywords - ortb2.site.ext.data.language == site.ext.data.language - - ortb2.user.yob == user.yob - ortb2.user.gender == user.gender - ortb2.user.keywords == user.keywords - ortb2.user.ext.data.keywords == user.ext.data.keywords - ortb2.user.ext.data.buyeruid == user.ext.data.buyeruid - ortb2.user.ext.data.buyeruids == user.ext.data.buyeruids + it.site.name == fpdSite.name + it.site.domain == fpdSite.domain + it.site.cat == fpdSite.cat + it.site.sectionCat == fpdSite.sectionCat + it.site.pageCat == fpdSite.pageCat + it.site.page == fpdSite.page + it.site.ref == fpdSite.ref + it.site.search == fpdSite.search + it.site.keywords == fpdSite.keywords + it.site.ext.data.language == fpdSite.ext.data.language + + it.user.yob == fpdUser.yob + it.user.gender == fpdUser.gender + it.user.keywords == fpdUser.keywords + it.user.ext.data.keywords == fpdUser.ext.data.keywords + it.user.ext.data.buyeruid == fpdUser.ext.data.buyeruid + it.user.ext.data.buyeruids == fpdUser.ext.data.buyeruids } and: "Bidder request shouldn't contain imp[0].ext.rp" @@ -413,17 +416,18 @@ class AmpFpdSpec extends BaseSpec { } } - def "PBS should fill unknown FPD when unknown FPD data present"() { + def "PBS should ignore any not FPD data value in bidderconfig.config when merging values"() { given: "AMP request" def ampRequest = new AmpRequest(tagId: PBSUtils.randomString) and: "Stored request" - def fpdGeo = Geo.FPDGeo + def fpdSite = Site.rootFPDSite + def fpdUser = User.rootFPDUser def ampStoredRequest = BidRequest.getDefaultBidRequest(SITE).tap { - site = Site.rootFPDSite - user = User.rootFPDUser + site = fpdSite + user = fpdUser ext.prebid.bidderConfig = [new ExtPrebidBidderConfig(bidders: [GENERIC], config: new BidderConfig(ortb2: - new BidderConfigOrtb(user: new User(geo: fpdGeo))))] + new BidderConfigOrtb(user: new User(geo: Geo.FPDGeo), site: new Site(publisher: new Publisher(name: PBSUtils.randomString)))))] } and: "Save stored request in DB" @@ -433,13 +437,32 @@ class AmpFpdSpec extends BaseSpec { when: "PBS processes amp request" defaultPbsService.sendAmpRequest(ampRequest) - then: "Bidder request should contain certain FPD field from the stored request" + then: "Bidder request should contain FPD field from the stored request" def bidderRequest = bidder.getBidderRequest(ampStoredRequest.id) verifyAll(bidderRequest) { - user.ext.data.geo.country == fpdGeo.country - user.ext.data.geo.zip == fpdGeo.zip - user.geo.country == ampStoredRequest.user.geo.country - user.geo.zip == ampStoredRequest.user.geo.zip + it.site.name == fpdSite.name + it.site.domain == fpdSite.domain + it.site.cat == fpdSite.cat + it.site.sectionCat == fpdSite.sectionCat + it.site.pageCat == fpdSite.pageCat + it.site.page == fpdSite.page + it.site.ref == fpdSite.ref + it.site.search == fpdSite.search + it.site.keywords == fpdSite.keywords + it.site.ext.data.language == fpdSite.ext.data.language + + it.user.yob == fpdUser.yob + it.user.gender == fpdUser.gender + it.user.keywords == fpdUser.keywords + it.user.ext.data.keywords == fpdUser.ext.data.keywords + it.user.ext.data.buyeruid == fpdUser.ext.data.buyeruid + it.user.ext.data.buyeruids == fpdUser.ext.data.buyeruids + } + + and: "Should should ignore any non FPD data" + verifyAll(bidderRequest) { + !it.user.ext.data.geo + !it.site.ext.data.publisher } } diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index fe57f123d9b..11346bc5957 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -2678,12 +2678,12 @@ public void shouldUseConcreteOverGeneralSiteWithExtPrebidBidderConfigIgnoringCas final ObjectNode siteWithPage = mapper.valueToTree(Site.builder().page("testPage").build()); final ExtBidderConfig extBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(siteWithPage, null, null, null)); + ExtBidderConfigOrtb.of(siteWithPage, null, null, null)); final ExtRequestPrebidBidderConfig concreteFpdConfig = ExtRequestPrebidBidderConfig.of( singletonList("SoMeBiDdEr"), extBidderConfig); final ObjectNode siteWithDomain = mapper.valueToTree(Site.builder().domain("notUsed").build()); final ExtBidderConfig allExtBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(siteWithDomain, null, null, null)); + ExtBidderConfigOrtb.of(siteWithDomain, null, null, null)); final ExtRequestPrebidBidderConfig allFpdConfig = ExtRequestPrebidBidderConfig.of(singletonList("*"), allExtBidderConfig); @@ -2725,12 +2725,12 @@ public void shouldUseConcreteOverGeneralDoohWithExtPrebidBidderConfig() { final ObjectNode doohWithVenueType = mapper.valueToTree(Dooh.builder().venuetype(List.of("venuetype")).build()); final ExtBidderConfig extBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(null, null, doohWithVenueType, null)); + ExtBidderConfigOrtb.of(null, null, doohWithVenueType, null)); final ExtRequestPrebidBidderConfig concreteFpdConfig = ExtRequestPrebidBidderConfig.of( singletonList("someBidder"), extBidderConfig); final ObjectNode doohWithDomain = mapper.valueToTree(Dooh.builder().domain("notUsed").build()); final ExtBidderConfig allExtBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(null, null, doohWithDomain, null)); + ExtBidderConfigOrtb.of(null, null, doohWithDomain, null)); final ExtRequestPrebidBidderConfig allFpdConfig = ExtRequestPrebidBidderConfig.of( singletonList("*"), allExtBidderConfig); @@ -2774,7 +2774,7 @@ public void shouldUseConcreteOverGeneralAppWithExtPrebidBidderConfigIgnoringCase final Publisher publisherWithId = Publisher.builder().id("testId").build(); final ObjectNode appWithPublisherId = mapper.valueToTree(App.builder().publisher(publisherWithId).build()); final ExtBidderConfig extBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(null, appWithPublisherId, null, null)); + ExtBidderConfigOrtb.of(null, appWithPublisherId, null, null)); final ExtRequestPrebidBidderConfig concreteFpdConfig = ExtRequestPrebidBidderConfig.of( singletonList("SoMeBiDdEr"), extBidderConfig); @@ -2782,7 +2782,7 @@ public void shouldUseConcreteOverGeneralAppWithExtPrebidBidderConfigIgnoringCase final ObjectNode appWithUpdatedPublisher = mapper.valueToTree( App.builder().publisher(publisherWithIdAndDomain).build()); final ExtBidderConfig allExtBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(null, appWithUpdatedPublisher, null, null)); + ExtBidderConfigOrtb.of(null, appWithUpdatedPublisher, null, null)); final ExtRequestPrebidBidderConfig allFpdConfig = ExtRequestPrebidBidderConfig.of(singletonList("*"), allExtBidderConfig); @@ -2821,13 +2821,13 @@ public void shouldUseConcreteOverGeneralUserWithExtPrebidBidderConfig() { givenBidder("someBidder", bidder, givenEmptySeatBid()); final ObjectNode bidderConfigUser = mapper.valueToTree(User.builder().id("userFromConfig").build()); final ExtBidderConfig extBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(null, null, null, bidderConfigUser)); + ExtBidderConfigOrtb.of(null, null, null, bidderConfigUser)); final ExtRequestPrebidBidderConfig concreteFpdConfig = ExtRequestPrebidBidderConfig.of( singletonList("SomMeBiDdEr"), extBidderConfig); final ObjectNode emptyUser = mapper.valueToTree(User.builder().build()); final ExtBidderConfig allExtBidderConfig = ExtBidderConfig.of( - null, ExtBidderConfigOrtb.of(null, null, null, emptyUser)); + ExtBidderConfigOrtb.of(null, null, null, emptyUser)); final ExtRequestPrebidBidderConfig allFpdConfig = ExtRequestPrebidBidderConfig.of(singletonList("*"), allExtBidderConfig); final User requestUser = User.builder().id("erased").buyeruid("testBuyerId").build(); diff --git a/src/test/java/org/prebid/server/auction/FpdResolverTest.java b/src/test/java/org/prebid/server/auction/FpdResolverTest.java index 07909bd09f5..d3b7530ab84 100644 --- a/src/test/java/org/prebid/server/auction/FpdResolverTest.java +++ b/src/test/java/org/prebid/server/auction/FpdResolverTest.java @@ -23,7 +23,6 @@ import java.util.Collections; import java.util.List; -import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; @@ -67,16 +66,14 @@ public void resolveUserShouldOverrideFpdFieldsFromFpdUser() { // then assertThat(resultUser).isEqualTo(User.builder() - .id("id") + .id("fpdid") .keywords("fpdkeywords") .yob(2) .gender("fpdgender") - .buyeruid("buyeruid") - .customdata("customdata") - .geo(Geo.builder().country("country").build()) + .buyeruid("fpdbuyeruid") + .customdata("fpdcustomdata") + .geo(Geo.builder().country("fpdcountry").build()) .data(Collections.singletonList(Data.builder().id("fpdid").build())) - .ext(ExtUser.builder().data(mapper.createObjectNode() - .set("geo", mapper.createObjectNode().put("country", "fpdcountry"))).build()) .build()); } @@ -92,69 +89,6 @@ public void resolveUserShouldReturnFpdUserIfOriginUserIsNull() { .isEqualTo(User.builder().gender("male").build()); } - @Test - public void resolveUserShouldAddExtDataAttributesIfOriginDoesNotHaveExtData() { - // given - final User originUser = User.builder() - .build(); - - final User fpdUser = User.builder() - .geo(Geo.builder().country("country").build()) - .build(); - - // when - final User resultUser = target.resolveUser(originUser, mapper.valueToTree(fpdUser)); - - // then - assertThat(resultUser).isEqualTo(User.builder() - .ext(ExtUser.builder().data(mapper.createObjectNode() - .set("geo", mapper.createObjectNode().put("country", "country"))) - .build()) - .build()); - } - - @Test - public void resolveAppShouldAddExtDataAttributesIfOriginDoesNotHaveExtData() { - // given - final App originApp = App.builder().build(); - final App fpdApp = App.builder().id("id").build(); - - // when - final App resultApp = target.resolveApp(originApp, mapper.valueToTree(fpdApp)); - - // then - assertThat(resultApp).isEqualTo(App.builder().ext(ExtApp.of(null, mapper.createObjectNode() - .put("id", "id"))).build()); - } - - @Test - public void resolveSiteShouldAddExtDataAttributesIfOriginDoesNotHaveExtData() { - // given - final Site originSite = Site.builder().build(); - final Site fpdSite = Site.builder().id("id").build(); - - // when - final Site resultSite = target.resolveSite(originSite, mapper.valueToTree(fpdSite)); - - // then - assertThat(resultSite).isEqualTo(Site.builder().ext(ExtSite.of(null, mapper.createObjectNode() - .put("id", "id"))).build()); - } - - @Test - public void resolveDoohShouldAddExtDataAttributesIfOriginDoesNotHaveExtData() { - // given - final Dooh originDooh = Dooh.builder().build(); - final Dooh fpdDooh = Dooh.builder().id("id").build(); - - // when - final Dooh resultDooh = target.resolveDooh(originDooh, mapper.valueToTree(fpdDooh)); - - // then - assertThat(resultDooh).isEqualTo( - Dooh.builder().ext(ExtDooh.of(mapper.createObjectNode().put("id", "id"))).build()); - } - @Test public void resolveUserShouldNotChangeOriginExtDataIfFPDDoesNotHaveExt() { // given @@ -218,6 +152,7 @@ public void resolveAppShouldOverrideFpdFieldsFromFpdApp() { .publisher(Publisher.builder().id("originId").build()) .content(Content.builder().language("originLan").build()) .keywords("originKeywords") + .ext(ExtApp.of(ExtAppPrebid.of("originalSource", null), mapper.createObjectNode())) .build(); final App fpdApp = App.builder() @@ -235,17 +170,18 @@ public void resolveAppShouldOverrideFpdFieldsFromFpdApp() { .publisher(Publisher.builder().id("fpdId").build()) .content(Content.builder().language("fpdLan").build()) .keywords("fpdKeywords") + .ext(ExtApp.of( + ExtAppPrebid.of(null, "fpdVersion"), + mapper.createObjectNode().put("data", "fpdData"))) .build(); + // when final App resultApp = target.resolveApp(originApp, mapper.valueToTree(fpdApp)); // then - final ObjectNode dataResult = mapper.createObjectNode().put("id", "fpdId").put("privacypolicy", 2) - .set("publisher", mapper.createObjectNode().put("id", "fpdId")); - dataResult.set("content", mapper.createObjectNode().put("language", "fpdLan")); assertThat(resultApp) .isEqualTo(App.builder() - .id("originId") + .id("fpdId") .name("fpdName") .bundle("fpdBundle") .domain("fpdDomain") @@ -253,13 +189,15 @@ public void resolveAppShouldOverrideFpdFieldsFromFpdApp() { .cat(Collections.singletonList("fpdCat")) .sectioncat(Collections.singletonList("fpdSectionCat")) .pagecat(Collections.singletonList("fpdPageCat")) - .publisher(Publisher.builder().id("originId").build()) - .content(Content.builder().language("originLan").build()) - .ver("originVer") - .privacypolicy(1) - .paid(1) + .publisher(Publisher.builder().id("fpdId").build()) + .content(Content.builder().language("fpdLan").build()) + .ver("fpdVer") + .privacypolicy(2) + .paid(2) .keywords("fpdKeywords") - .ext(ExtApp.of(null, dataResult)) + .ext(ExtApp.of( + ExtAppPrebid.of("originalSource", "fpdVersion"), + mapper.createObjectNode().put("data", "fpdData"))) .build()); } @@ -343,9 +281,6 @@ public void resolveSiteShouldOverrideFpdFieldsFromFpdSite() { final Site resultSite = target.resolveSite(originSite, mapper.valueToTree(fpdSite)); // then - final ObjectNode extData = mapper.createObjectNode().put("id", "fpdId").put("privacypolicy", 2).put("mobile", 2) - .set("publisher", mapper.createObjectNode().put("id", "fpdId")); - extData.set("content", mapper.createObjectNode().put("language", "fpdLan")); assertThat(resultSite).isEqualTo( Site.builder() .name("fpdName") @@ -357,12 +292,11 @@ public void resolveSiteShouldOverrideFpdFieldsFromFpdSite() { .ref("fpdRef") .search("fpdSearch") .keywords("fpdKeywords") - .id("originId") - .content(Content.builder().language("originLan").build()) - .publisher(Publisher.builder().id("originId").build()) - .privacypolicy(1) - .mobile(1) - .ext(ExtSite.of(null, extData)) + .id("fpdId") + .content(Content.builder().language("fpdLan").build()) + .publisher(Publisher.builder().id("fpdId").build()) + .privacypolicy(2) + .mobile(2) .build()); } @@ -434,22 +368,16 @@ public void resolveDoohShouldOverrideFpdFieldsFromFpdDooh() { final Dooh resultDooh = target.resolveDooh(originDooh, mapper.valueToTree(fpdDooh)); // then - final ObjectNode extData = mapper.createObjectNode() - .put("id", "fpdId") - .setAll(Map.of( - "publisher", mapper.createObjectNode().put("id", "fpdId"), - "content", mapper.createObjectNode().put("language", "fpdLan"))); assertThat(resultDooh).isEqualTo( Dooh.builder() + .id("fpdId") .name("fpdName") .domain("fpdDomain") - .keywords("fpdKeywords") - .id("originId") .venuetype(List.of("fpdVenuetype")) .venuetypetax(2) - .content(Content.builder().language("originLan").build()) - .publisher(Publisher.builder().id("originId").build()) - .ext(ExtDooh.of(extData)) + .publisher(Publisher.builder().id("fpdId").build()) + .content(Content.builder().language("fpdLan").build()) + .keywords("fpdKeywords") .build()); } diff --git a/src/test/java/org/prebid/server/auction/OrtbTypesResolverTest.java b/src/test/java/org/prebid/server/auction/OrtbTypesResolverTest.java index f49a03fcdee..7c0797ffacd 100644 --- a/src/test/java/org/prebid/server/auction/OrtbTypesResolverTest.java +++ b/src/test/java/org/prebid/server/auction/OrtbTypesResolverTest.java @@ -133,7 +133,7 @@ public void normalizeTargetingShouldNormalizeFieldsForUser() { } @Test - public void normalizeTargetingShouldNormalizeFieldsForAppExceptId() { + public void normalizeTargetingShouldNormalizeFieldsForApp() { // given final ObjectNode app = mapper.createObjectNode(); app.set("id", array("id1", "id2")); @@ -149,16 +149,16 @@ public void normalizeTargetingShouldNormalizeFieldsForAppExceptId() { // then assertThat(containerNode).isEqualTo(mapper.createObjectNode().set("app", mapper.createObjectNode() + .put("id", "id1") .put("name", "name1") .put("bundle", "bundle1") .put("storeurl", "storeurl1") .put("domain", "domain1") - .put("keywords", "keyword1,keyword2") - .set("id", array("id1", "id2")))); + .put("keywords", "keyword1,keyword2"))); } @Test - public void normalizeTargetingShouldNormalizeFieldsForSiteExceptId() { + public void normalizeTargetingShouldNormalizeFieldsForSite() { // given final ObjectNode site = mapper.createObjectNode(); site.set("id", array("id1", "id2")); @@ -175,14 +175,13 @@ public void normalizeTargetingShouldNormalizeFieldsForSiteExceptId() { // then assertThat(containerNode).isEqualTo(mapper.createObjectNode().set("site", mapper.createObjectNode() + .put("id", "id1") .put("name", "name1") .put("page", "page1") .put("ref", "ref1") .put("domain", "domain1") .put("search", "search1") - .put("keywords", "keyword1,keyword2") - .set("id", array("id1", "id2"))) - ); + .put("keywords", "keyword1,keyword2"))); } @Test @@ -409,22 +408,22 @@ public void normalizeBidRequestShouldResolveEmptyOrtbWithFpdFieldsWithIdForReque assertThat(ortb2.path("site")) .isEqualTo(mapper.createObjectNode() + .put("id", "id1") .put("name", "name1") .put("domain", "domain1") .put("page", "page1") .put("ref", "ref1") .put("search", "search1") - .put("keywords", "keyword1,keyword2") - .set("id", array("id1", "id2"))); + .put("keywords", "keyword1,keyword2")); assertThat(ortb2.path("app")) .isEqualTo(mapper.createObjectNode() + .put("id", "id1") .put("name", "name1") .put("bundle", "bundle1") .put("storeurl", "storeurl1") .put("domain", "domain1") - .put("keywords", "keyword1,keyword2") - .set("id", array("id1", "id2"))); + .put("keywords", "keyword1,keyword2")); assertThat(ortb2.path("user")) .isEqualTo(mapper.createObjectNode() @@ -472,12 +471,12 @@ public void normalizeBidRequestShouldBeMergedWithFpdContextToOrtbSite() { .put("fpdData", "data_value"); final ObjectNode expectedOrtb = mapper.createObjectNode() - .put("name", "name1") - .put("domain", "domain1") - .put("page", "page1") - .put("ref", "ortb_ref1") - .put("keywords", "ortb_keyword1,ortb_keyword2"); - expectedOrtb.set("id", array("id1", "id2")); + .put("id", "ortb_id") + .put("name", "name1") + .put("domain", "ortb_domain1") + .put("page", "ortb_page1") + .put("ref", "ortb_ref1") + .put("keywords", "ortb_keyword1,ortb_keyword2"); expectedOrtb.set("ext", obj("data", expectedOrtbExtData)); assertThat(ortb2.path("site")).isEqualTo(expectedOrtb); @@ -518,7 +517,7 @@ public void normalizeBidRequestShouldBeMergedWithFpdUserToOrtbUser() { assertThat(ortb2.path("user")) .isEqualTo(mapper.createObjectNode() - .put("gender", "gender1") + .put("gender", "ortb_gender1") .put("keywords", "ortb_keyword1,ortb_keyword2") .set("ext", obj("data", expectedOrtbExtData)));