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

Extend PG Targeting to match single String or Integer value #1653

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
94 changes: 59 additions & 35 deletions src/main/java/org/prebid/server/deals/targeting/RequestContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.iab.openrtb.request.App;
import com.iab.openrtb.request.Banner;
import com.iab.openrtb.request.BidRequest;
import com.iab.openrtb.request.Data;
import com.iab.openrtb.request.Device;
import com.iab.openrtb.request.Geo;
import com.iab.openrtb.request.Imp;
Expand All @@ -13,6 +14,7 @@
import com.iab.openrtb.request.Site;
import com.iab.openrtb.request.User;
import org.apache.commons.collections4.ListUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.prebid.server.deals.model.TxnLog;
Expand All @@ -37,7 +39,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -65,13 +66,15 @@ public RequestContext(BidRequest bidRequest, Imp imp, TxnLog txnLog, JacksonMapp
this.imp = Objects.requireNonNull(imp);
this.deviceExt = getExtNode(bidRequest.getDevice(), Device::getExt, mapper);
this.geoExt = getExtNode(bidRequest.getDevice(),
dev -> getIfNotNull(getIfNotNull(dev, Device::getGeo), Geo::getExt), mapper);
device -> getIfNotNull(getIfNotNull(device, Device::getGeo), Geo::getExt), mapper);
this.userExt = getExtNode(bidRequest.getUser(), User::getExt, mapper);
this.txnLog = Objects.requireNonNull(txnLog);
}

private static <T> ObjectNode getExtNode(T target, Function<T, FlexibleExtension> extExtractor,
private static <T> ObjectNode getExtNode(T target,
Function<T, FlexibleExtension> extExtractor,
JacksonMapper mapper) {

final FlexibleExtension ext = target != null ? extExtractor.apply(target) : null;
return ext != null ? (ObjectNode) mapper.mapper().valueToTree(ext) : null;
}
Expand All @@ -80,7 +83,7 @@ public String lookupString(TargetingCategory category) {
final TargetingCategory.Type type = category.type();
switch (type) {
case domain:
return org.apache.commons.lang3.ObjectUtils.defaultIfNull(
return ObjectUtils.defaultIfNull(
getIfNotNull(bidRequest.getSite(), Site::getDomain),
getIfNotNull(getIfNotNull(bidRequest.getSite(), Site::getPublisher), Publisher::getDomain));
case publisherDomain:
Expand All @@ -102,8 +105,8 @@ public String lookupString(TargetingCategory category) {
case bidderParam:
return impBidderAttributeReader.readFromExt(imp, category, RequestContext::nodeToString);
case userFirstPartyData:
return userAttributeReader.read(
bidRequest.getUser(), category, RequestContext::nodeToString, String.class);
return userAttributeReader.read(bidRequest.getUser(), category,
RequestContext::nodeToString, String.class);
case siteFirstPartyData:
return getSiteFirstPartyData(category, RequestContext::nodeToString);
default:
Expand All @@ -124,8 +127,8 @@ public Integer lookupInteger(TargetingCategory category) {
case bidderParam:
return impBidderAttributeReader.readFromExt(imp, category, RequestContext::nodeToInteger);
case userFirstPartyData:
return userAttributeReader.read(
bidRequest.getUser(), category, RequestContext::nodeToInteger, Integer.class);
return userAttributeReader.read(bidRequest.getUser(), category,
RequestContext::nodeToInteger, Integer.class);
case siteFirstPartyData:
return getSiteFirstPartyData(category, RequestContext::nodeToInteger);
default:
Expand All @@ -140,15 +143,14 @@ public List<String> lookupStrings(TargetingCategory category) {
case mediaType:
return getMediaTypes();
case bidderParam:
return impBidderAttributeReader
.readFromExt(imp, category, node -> nodeToList(node, RequestContext::nodeToString));
return impBidderAttributeReader.readFromExt(imp, category, RequestContext::nodeToListOfStrings);
case userSegment:
return getSegments(category);
case userFirstPartyData:
return userAttributeReader.readFromExt(
bidRequest.getUser(), category, node -> nodeToList(node, RequestContext::nodeToString));
return userAttributeReader.readFromExt(bidRequest.getUser(), category,
RequestContext::nodeToListOfStrings);
case siteFirstPartyData:
return getSiteFirstPartyData(category, node -> nodeToList(node, RequestContext::nodeToString));
return getSiteFirstPartyData(category, RequestContext::nodeToListOfStrings);
default:
throw new TargetingSyntaxException(
String.format("Unexpected category for fetching string values for: %s", type));
Expand All @@ -159,13 +161,12 @@ public List<Integer> lookupIntegers(TargetingCategory category) {
final TargetingCategory.Type type = category.type();
switch (type) {
case bidderParam:
return impBidderAttributeReader
.readFromExt(imp, category, node -> nodeToList(node, RequestContext::nodeToInteger));
return impBidderAttributeReader.readFromExt(imp, category, RequestContext::nodeToListOfIntegers);
case userFirstPartyData:
return userAttributeReader.readFromExt(
bidRequest.getUser(), category, node -> nodeToList(node, RequestContext::nodeToInteger));
return userAttributeReader.readFromExt(bidRequest.getUser(), category,
RequestContext::nodeToListOfIntegers);
case siteFirstPartyData:
return getSiteFirstPartyData(category, node -> nodeToList(node, RequestContext::nodeToInteger));
return getSiteFirstPartyData(category, RequestContext::nodeToListOfIntegers);
default:
throw new TargetingSyntaxException(
String.format("Unexpected category for fetching integer values for: %s", type));
Expand Down Expand Up @@ -204,7 +205,11 @@ public TxnLog txnLog() {
}

private String getFirstNonNullStringFromImpExt(String... path) {
return Arrays.stream(path).map(this::getStringFromImpExt).filter(Objects::nonNull).findFirst().orElse(null);
return Arrays.stream(path)
.map(this::getStringFromImpExt)
.filter(Objects::nonNull)
.findFirst()
.orElse(null);
}

private String getStringFromImpExt(String path) {
Expand Down Expand Up @@ -235,8 +240,10 @@ private List<String> getMediaTypes() {
return mediaTypes;
}

private static <T> T getValueFrom(ObjectNode objectNode, TargetingCategory category,
private static <T> T getValueFrom(ObjectNode objectNode,
TargetingCategory category,
Function<JsonNode, T> valueExtractor) {

final JsonNode jsonNode = getIfNotNull(objectNode, node -> node.at(toJsonPointer(category.path())));
return getIfNotNull(jsonNode, valueExtractor);
}
Expand All @@ -249,7 +256,10 @@ private <T> T getSiteFirstPartyData(TargetingCategory category, Function<JsonNod
}

public List<String> getSegments(TargetingCategory category) {
return ListUtils.emptyIfNull(getIfNotNull(getIfNotNull(bidRequest, BidRequest::getUser), User::getData))
final User user = getIfNotNull(bidRequest, BidRequest::getUser);
final List<Data> userData = getIfNotNull(user, User::getData);

return ListUtils.emptyIfNull(userData)
.stream()
.filter(Objects::nonNull)
.filter(data -> Objects.equals(data.getId(), category.path()))
Expand All @@ -260,7 +270,8 @@ public List<String> getSegments(TargetingCategory category) {
}

private static String toJsonPointer(String path) {
return Arrays.stream(path.split("\\.")).collect(Collectors.joining("/", "/", StringUtils.EMPTY));
return Arrays.stream(path.split("\\."))
.collect(Collectors.joining("/", "/", StringUtils.EMPTY));
}

private static String nodeToString(JsonNode node) {
Expand All @@ -271,6 +282,20 @@ private static Integer nodeToInteger(JsonNode node) {
return node.isInt() ? node.asInt() : null;
}

private static List<String> nodeToListOfStrings(JsonNode node) {
final Function<JsonNode, String> valueExtractor = RequestContext::nodeToString;
return node.isTextual()
? Collections.singletonList(valueExtractor.apply(node))
: nodeToList(node, valueExtractor);
}

private static List<Integer> nodeToListOfIntegers(JsonNode node) {
final Function<JsonNode, Integer> valueExtractor = RequestContext::nodeToInteger;
return node.isInt()
? Collections.singletonList(valueExtractor.apply(node))
: nodeToList(node, valueExtractor);
}

private static <T> List<T> nodeToList(JsonNode node, Function<JsonNode, T> valueExtractor) {
if (node.isArray()) {
return StreamUtil.asStream(node.spliterator())
Expand All @@ -284,8 +309,7 @@ private static <T> List<T> nodeToList(JsonNode node, Function<JsonNode, T> value

private static class AttributeReader<T> {

private static final Set<Class<?>> SUPPORTED_PROPERTY_TYPES = new HashSet<>(Arrays.asList(
String.class, Integer.class, int.class));
private static final Set<Class<?>> SUPPORTED_PROPERTY_TYPES = Set.of(String.class, Integer.class, int.class);

private static final String EXT_PREBID = "prebid";
private static final String EXT_BIDDER = "bidder";
Expand Down Expand Up @@ -336,8 +360,10 @@ public static AttributeReader<Imp> forImpBidder() {
node -> node.get(EXT_BIDDER)));
}

public <A> A read(
T target, TargetingCategory category, Function<JsonNode, A> valueExtractor, Class<A> attributeType) {
public <A> A read(T target,
TargetingCategory category,
Function<JsonNode, A> valueExtractor,
Class<A> attributeType) {

return ObjectUtil.firstNonNull(
// look in the object itself
Expand All @@ -347,17 +373,15 @@ public <A> A read(
}

public <A> A readFromObject(T target, TargetingCategory category, Class<A> attributeType) {
if (isTopLevelAttribute(category.path())) {
return getIfNotNull(target, user -> readProperty(user, category.path(), attributeType));
}
return null;
return isTopLevelAttribute(category.path())
? getIfNotNull(target, user -> readProperty(user, category.path(), attributeType))
: null;
}

public <A> A readFromExt(T target, TargetingCategory category, Function<JsonNode, A> valueExtractor) {
return getIfNotNull(
getIfNotNull(
getIfNotNull(target, extPathExtractor), node -> node.at(toJsonPointer(category.path()))),
valueExtractor);
final JsonNode path = getIfNotNull(target, extPathExtractor);
final JsonNode value = getIfNotNull(path, node -> node.at(toJsonPointer(category.path())));
return getIfNotNull(value, valueExtractor);
}

private boolean isTopLevelAttribute(String path) {
Expand All @@ -368,7 +392,7 @@ private static Map<String, PropertyDescriptor> supportedBeanProperties(Class<?>
try {
final BeanInfo beanInfo = Introspector.getBeanInfo(beanClass, Object.class);
return Arrays.stream(beanInfo.getPropertyDescriptors())
.filter(pd -> SUPPORTED_PROPERTY_TYPES.contains(pd.getPropertyType()))
.filter(descriptor -> SUPPORTED_PROPERTY_TYPES.contains(descriptor.getPropertyType()))
.collect(Collectors.toMap(FeatureDescriptor::getName, Function.identity()));
} catch (IntrospectionException e) {
return ExceptionUtils.rethrow(e);
Expand Down
53 changes: 53 additions & 0 deletions src/test/java/org/prebid/server/deals/TargetingServiceTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.prebid.server.deals;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.IntNode;
import com.fasterxml.jackson.databind.node.TextNode;
import com.iab.openrtb.request.Banner;
import com.iab.openrtb.request.BidRequest;
Expand Down Expand Up @@ -475,6 +476,58 @@ public void matchesTargetingShouldReturnTrue() {
assertThat(txnLog.lineItemsMatchedDomainTargeting()).containsOnly("lineItemId");
}

@Test
public void matchesTargetingShouldReturnTrueForIntersectsStringsOnSingleString() {
// given
final TargetingDefinition targetingDefinition = TargetingDefinition.of(
new IntersectsStrings(category(Type.userFirstPartyData, "segment"), asList("test", "111")));

final BidRequest bidRequest = BidRequest.builder()
.user(User.builder()
.ext(ExtUser.builder()
.data(mapper.createObjectNode().set("segment", TextNode.valueOf("test")))
.build())
.build())
.build();

final TxnLog txnLog = TxnLog.create();
final AuctionContext auctionContext = AuctionContext.builder()
.bidRequest(bidRequest)
.txnLog(txnLog)
.build();

final Imp imp = Imp.builder().build();

// when and then
assertThat(targetingService.matchesTargeting(auctionContext, imp, targetingDefinition)).isTrue();
}

@Test
public void matchesTargetingShouldReturnTrueForIntersectsIntegersOnSingleInteger() {
// given
final TargetingDefinition targetingDefinition = TargetingDefinition.of(
new IntersectsIntegers(category(Type.userFirstPartyData, "segment"), asList(123, 456)));

final BidRequest bidRequest = BidRequest.builder()
.user(User.builder()
.ext(ExtUser.builder()
.data(mapper.createObjectNode().set("segment", IntNode.valueOf(123)))
.build())
.build())
.build();

final TxnLog txnLog = TxnLog.create();
final AuctionContext auctionContext = AuctionContext.builder()
.bidRequest(bidRequest)
.txnLog(txnLog)
.build();

final Imp imp = Imp.builder().build();

// when and then
assertThat(targetingService.matchesTargeting(auctionContext, imp, targetingDefinition)).isTrue();
}

@Test
public void matchesTargetingShouldReturnTrueForNotInIntegers() throws IOException {
// given
Expand Down
Loading