Fix AMP Targeting FPD

src/main/java/org/prebid/server/auction/FpdResolver.java

@@ -9,28 +9,18 @@
 import com.iab.openrtb.request.Dooh;
 import com.iab.openrtb.request.Site;
 import com.iab.openrtb.request.User;
-import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.lang3.ObjectUtils;
 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.ExtBidderConfig;
-import org.prebid.server.proto.openrtb.ext.request.ExtBidderConfigOrtb;
 import org.prebid.server.proto.openrtb.ext.request.ExtDooh;
-import org.prebid.server.proto.openrtb.ext.request.ExtRequest;
-import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid;
-import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidBidderConfig;
-import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidData;
 import org.prebid.server.proto.openrtb.ext.request.ExtSite;
 import org.prebid.server.proto.openrtb.ext.request.ExtUser;
-import org.prebid.server.proto.request.Targeting;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
-import java.util.TreeSet;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.StreamSupport;
@@ -43,7 +33,6 @@ public class FpdResolver {
  private static final String APP = "app";
  private static final String DOOH = "dooh";
  private static final Set<String> KNOWN_FPD_ATTRIBUTES = Set.of(USER, SITE, APP, DOOH, BIDDERS);
- private static final String ALLOW_ALL_BIDDERS = "*";
  private static final String EXT = "ext";
  private static final String CONTEXT = "context";
  private static final String DATA = "data";
@@ -289,64 +278,6 @@ private void removeOrReplace(ObjectNode impExt, String field, JsonNode jsonNode)
  }
  }
 
- public ExtRequest resolveBidRequestExt(ExtRequest extRequest, Targeting targeting) {
- if (targeting == null) {
- return extRequest;
- }
-
- final ExtRequestPrebid extRequestPrebid = extRequest != null ? extRequest.getPrebid() : null;
- final ExtRequestPrebidData extRequestPrebidData = extRequestPrebid != null
- ? extRequestPrebid.getData()
- : null;
-
- final ExtRequestPrebidData resolvedExtRequestPrebidData = resolveExtRequestPrebidData(extRequestPrebidData,
- targeting.getBidders());
- final List<ExtRequestPrebidBidderConfig> resolvedBidderConfig = createAllowedAllBidderConfig(targeting);
-
- if (resolvedExtRequestPrebidData != null || resolvedBidderConfig != null) {
- final ExtRequestPrebid.ExtRequestPrebidBuilder prebidBuilder = extRequestPrebid != null
- ? extRequestPrebid.toBuilder()
- : ExtRequestPrebid.builder();
- return ExtRequest.of(prebidBuilder
- .data(resolvedExtRequestPrebidData != null
- ? resolvedExtRequestPrebidData
- : extRequestPrebidData)
- .bidderconfig(resolvedBidderConfig)
- .build());
- }
-
- return extRequest;
- }
-
- private ExtRequestPrebidData resolveExtRequestPrebidData(ExtRequestPrebidData data, List<String> fpdBidders) {
- if (CollectionUtils.isEmpty(fpdBidders)) {
- return null;
- }
- final List<String> originBidders = data != null ? data.getBidders() : Collections.emptyList();
- return CollectionUtils.isEmpty(originBidders)
- ? ExtRequestPrebidData.of(fpdBidders, null)
- : ExtRequestPrebidData.of(mergeBidders(fpdBidders, originBidders), null);
- }
-
- private List<String> mergeBidders(List<String> fpdBidders, List<String> originBidders) {
- final Set<String> resolvedBidders = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
- resolvedBidders.addAll(originBidders);
- resolvedBidders.addAll(fpdBidders);
- return new ArrayList<>(resolvedBidders);
- }
-
- private List<ExtRequestPrebidBidderConfig> createAllowedAllBidderConfig(Targeting targeting) {
- final ObjectNode userNode = targeting.getUser();
- final ObjectNode siteNode = targeting.getSite();
- if (userNode == null && siteNode == null) {
- return null;
- }
- final List<String> bidders = Collections.singletonList(ALLOW_ALL_BIDDERS);
-
- return Collections.singletonList(ExtRequestPrebidBidderConfig.of(bidders,
- ExtBidderConfig.of(null, ExtBidderConfigOrtb.of(siteNode, null, null, userNode))));
- }
-
  private ObjectNode mergeExtData(JsonNode fpdData, JsonNode originData) {
  if (fpdData.isMissingNode() || !fpdData.isObject()) {
  return originData != null && originData.isObject() ? ((ObjectNode) originData).deepCopy() : null;

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

@@ -50,7 +50,6 @@
 import org.prebid.server.proto.openrtb.ext.request.ExtSite;
 import org.prebid.server.proto.openrtb.ext.request.ExtStoredRequest;
 import org.prebid.server.proto.openrtb.ext.request.ExtUser;
-import org.prebid.server.proto.request.Targeting;
 import org.prebid.server.settings.model.Account;
 import org.prebid.server.util.HttpUtil;
 
@@ -500,19 +499,16 @@ private BidRequest fillExplicitParameters(BidRequest bidRequest) {
  private BidRequest overrideParameters(BidRequest bidRequest, HttpRequestContext httpRequest, List<String> errors) {
  final String requestTargeting = httpRequest.getQueryParams().get(TARGETING_REQUEST_PARAM);
  final ObjectNode targetingNode = readTargeting(requestTargeting);
- ortbTypesResolver.normalizeTargeting(
- targetingNode, errors, implicitParametersExtractor.refererFrom(httpRequest));
- final Targeting targeting = parseTargeting(targetingNode);
+ final String referer = implicitParametersExtractor.refererFrom(httpRequest);
+ ortbTypesResolver.normalizeTargeting(targetingNode, errors, referer);
 
  final Site updatedSite = overrideSite(bidRequest.getSite());
  final Imp updatedImp = overrideImp(bidRequest.getImp().get(0), httpRequest, targetingNode);
- final ExtRequest updatedExtBidRequest = overrideExtBidRequest(bidRequest.getExt(), targeting);
 
- if (ObjectUtils.anyNotNull(updatedSite, updatedImp, updatedExtBidRequest)) {
+ if (ObjectUtils.anyNotNull(updatedSite, updatedImp)) {
  return bidRequest.toBuilder()
  .site(updatedSite != null ? updatedSite : bidRequest.getSite())
  .imp(updatedImp != null ? Collections.singletonList(updatedImp) : bidRequest.getImp())
- .ext(updatedExtBidRequest != null ? updatedExtBidRequest : bidRequest.getExt())
  .build();
  }
 
@@ -540,16 +536,6 @@ private ObjectNode validateAndGetTargeting(JsonNode jsonNodeTargeting) {
  }
  }
 
- private Targeting parseTargeting(ObjectNode targetingNode) {
- try {
- return targetingNode == null
- ? Targeting.empty()
- : mapper.mapper().treeToValue(targetingNode, Targeting.class);
- } catch (JsonProcessingException e) {
- throw new InvalidRequestException("Error decoding targeting from url: " + e.getMessage());
- }
- }
-
  private Site overrideSite(Site site) {
  final boolean hasSite = site != null;
  final ExtSite siteExt = hasSite ? site.getExt() : null;
@@ -666,13 +652,6 @@ private static Banner overrideBanner(Banner banner, List<Format> formats) {
  : banner;
  }
 
- /**
- * Overrides {@link ExtRequest} with first party data.
- */
- private ExtRequest overrideExtBidRequest(ExtRequest extRequest, Targeting targeting) {
- return fpdResolver.resolveBidRequestExt(extRequest, targeting);
- }
-
  private static List<Format> parseMultiSizeParam(String ms) {
  final String[] formatStrings = ms.split(",", NO_LIMIT_SPLIT_MODE);
  final List<Format> formats = new ArrayList<>();

src/test/groovy/org/prebid/server/functional/tests/AmpFpdSpec.groovy

@@ -3,7 +3,6 @@ package org.prebid.server.functional.tests
 import org.prebid.server.functional.model.bidder.Generic
 import org.prebid.server.functional.model.db.StoredRequest
 import org.prebid.server.functional.model.request.amp.AmpRequest
-import org.prebid.server.functional.model.request.amp.Targeting
 import org.prebid.server.functional.model.request.auction.BidRequest
 import org.prebid.server.functional.model.request.auction.BidderConfig
 import org.prebid.server.functional.model.request.auction.BidderConfigOrtb
@@ -17,7 +16,6 @@ import org.prebid.server.functional.model.request.auction.ImpExtContextDataAdSer
 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
-import org.prebid.server.functional.util.HttpUtil
 import org.prebid.server.functional.util.PBSUtils
 
 import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST
@@ -113,64 +111,6 @@ class AmpFpdSpec extends BaseSpec {
  assert ampStoredRequest.user.data[0].name == bidderRequest.user.data[0].name
  }
 
- def "PBS should populate all FPD via targeting when targeting is present"() {
- given: "Init targeting"
- def targeting = new Targeting().tap {
- site = Site.configFPDSite
- user = User.configFPDUser
- keywords = [PBSUtils.randomString]
- bidders = [GENERIC]
- }
-
- and: "Encode targeting to String"
- def encodeTargeting = HttpUtil.encodeUrl(encode(targeting))
-
- and: "AMP request"
- def ampRequest = new AmpRequest(tagId: PBSUtils.randomString, targeting: encodeTargeting)
-
- and: "Stored request with FPD fields"
- def ampStoredRequest = BidRequest.getDefaultBidRequest(SITE).tap {
- site = new Site(page: PBSUtils.randomString)
- user = new User(keywords: PBSUtils.randomString)
- }
-
- and: "Stored request in DB"
- def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest)
- storedRequestDao.save(storedRequest)
-
- when: "PBS processes amp request"
- defaultPbsService.sendAmpRequest(ampRequest)
-
- then: "Bidder request should contain FPD field from the stored request"
- def bidderRequest = bidder.getBidderRequest(ampStoredRequest.id)
- verifyAll(bidderRequest) {
- targeting.site.name == site.name
- targeting.site.domain == site.domain
- targeting.site.cat == site.cat
- targeting.site.sectionCat == site.sectionCat
- targeting.site.pageCat == site.pageCat
- targeting.site.page == site.page
- targeting.site.ref == site.ref
- targeting.site.search == site.search
- targeting.site.keywords == site.keywords
- targeting.site.ext.data.language == site.ext.data.language
-
- targeting.user.yob == user.yob
- targeting.user.gender == user.gender
- targeting.user.keywords == user.keywords
- targeting.user.ext.data.keywords == user.ext.data.keywords
- targeting.user.ext.data.buyeruid == user.ext.data.buyeruid
- targeting.user.ext.data.buyeruids == user.ext.data.buyeruids
- }
-
- and: "Bidder request shouldn't contain imp[0].ext.rp"
- bidderRequest.each {
- verifyAll(it) {
- !imp[0].ext.rp
- }
- }
- }
-
  def "PBS should emit error when targeting field is invalid"() {
  given: "AMP request with invalid targeting"
  def invalidTargeting = "InvalidTargeting"
@@ -216,64 +156,6 @@ class AmpFpdSpec extends BaseSpec {
  assert ampStoredRequest.site.keywords == bidderRequest.site.keywords
  }
 
- def "PBS should take precedence target from request when stored request contain site/user"() {
- given: "Init targeting"
- def targeting = new Targeting().tap {
- site = Site.configFPDSite
- user = User.configFPDUser
- keywords = [PBSUtils.randomString]
- bidders = [GENERIC]
- }
-
- and: "Encode targeting to String"
- def encodeTargeting = HttpUtil.encodeUrl(encode(targeting))
-
- and: "Amp request"
- def ampRequest = new AmpRequest(tagId: PBSUtils.randomString, targeting: encodeTargeting)
-
- and: "Stored request with FPD fields"
- def ampStoredRequest = BidRequest.getDefaultBidRequest(SITE).tap {
- site = Site.rootFPDSite
- user = User.rootFPDUser
- }
-
- and: "Save stored request in DB"
- def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest)
- storedRequestDao.save(storedRequest)
-
- when: "PBS processes amp request"
- defaultPbsService.sendAmpRequest(ampRequest)
-
- then: "Bidder request should contain FPD field from the targeting"
- def bidderRequest = bidder.getBidderRequest(ampStoredRequest.id)
- verifyAll(bidderRequest) {
- targeting.site.name == site.name
- targeting.site.domain == site.domain
- targeting.site.cat == site.cat
- targeting.site.sectionCat == site.sectionCat
- targeting.site.pageCat == site.pageCat
- targeting.site.page == site.page
- targeting.site.ref == site.ref
- targeting.site.search == site.search
- targeting.site.keywords == site.keywords
- targeting.site.ext.data.language == site.ext.data.language
-
- targeting.user.yob == user.yob
- targeting.user.gender == user.gender
- targeting.user.keywords == user.keywords
- targeting.user.ext.data.keywords == user.ext.data.keywords
- targeting.user.ext.data.buyeruid == user.ext.data.buyeruid
- targeting.user.ext.data.buyeruids == user.ext.data.buyeruids
- }
-
- and: "Bidder request shouldn't contain imp[0].ext.rp"
- bidderRequest.each {
- verifyAll(it) {
- !imp[0].ext.rp
- }
- }
- }
-
  def "PBS should populate FPD via bidder config when config.ortb2 is present"() {
  given: "AMP request"
  def ampRequest = new AmpRequest(tagId: PBSUtils.randomString)

src/test/groovy/org/prebid/server/functional/util/HttpUtil.groovy

@@ -33,10 +33,6 @@ class HttpUtil implements ObjectMapperWrapper {
  [(COOKIE_HEADER): "$value1=$value2"]
  }
 
- static String encodeUrl(String url){
- URLEncoder.encode(url, UTF_8)
- }
-
  private static String decodeUrl(String url) {
  URLDecoder.decode(url, UTF_8)
  }