Skip to content

Commit bf503d2

Browse files
AntoxaAntoxicAntoxaAntoxic
authored
Fix Stored Request Merging (#3931)
1 parent 46220c7 commit bf503d2

8 files changed

Lines changed: 350 additions & 1 deletion

File tree

‎src/main/java/org/prebid/server/json/JsonMerger.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
import com.fasterxml.jackson.core.JsonProcessingException;
44
import com.fasterxml.jackson.databind.JsonNode;
55
import com.github.fge.jsonpatch.JsonPatchException;
6-
import com.github.fge.jsonpatch.mergepatch.JsonMergePatch;
76
import org.apache.commons.lang3.ObjectUtils;
87
import org.prebid.server.exception.InvalidRequestException;
8+
import org.prebid.server.json.merge.JsonMergePatch;
99

1010
import java.io.IOException;
1111
import java.util.Objects;

‎src/main/java/org/prebid/server/json/merge/JsonMergePatch.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package org.prebid.server.json.merge;
2+
3+
import com.fasterxml.jackson.databind.JsonNode;
4+
import com.fasterxml.jackson.databind.JsonSerializable;
5+
import com.fasterxml.jackson.databind.ObjectMapper;
6+
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
7+
import com.github.fge.jsonpatch.JsonPatchException;
8+
import com.github.fge.jsonpatch.JsonPatchMessages;
9+
import com.github.fge.jsonpatch.Patch;
10+
import com.github.fge.msgsimple.bundle.MessageBundle;
11+
import com.github.fge.msgsimple.load.MessageBundles;
12+
import org.prebid.server.json.ObjectMapperProvider;
13+
14+
import java.io.IOException;
15+
16+
/**
17+
* Json merge patch implementation that uses the application-wide object mapper.
18+
* Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.JsonMergePatch}.
19+
*/
20+
@JsonDeserialize(using = JsonMergePatchDeserializer.class)
21+
public abstract class JsonMergePatch implements JsonSerializable, Patch {
22+
23+
private static final ObjectMapper MAPPER = ObjectMapperProvider.mapper();
24+
public static final MessageBundle BUNDLE = MessageBundles.getBundle(JsonPatchMessages.class);
25+
26+
public static JsonMergePatch fromJson(JsonNode node) throws JsonPatchException {
27+
BUNDLE.checkNotNull(node, "jsonPatch.nullInput");
28+
try {
29+
return MAPPER.readValue(node.traverse(), JsonMergePatch.class);
30+
} catch (IOException e) {
31+
throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.deserFailed"), e);
32+
}
33+
}
34+
35+
@Override
36+
public abstract JsonNode apply(JsonNode input) throws JsonPatchException;
37+
}

‎src/main/java/org/prebid/server/json/merge/JsonMergePatchDeserializer.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package org.prebid.server.json.merge;
2+
3+
import com.fasterxml.jackson.core.JsonParser;
4+
import com.fasterxml.jackson.core.ObjectCodec;
5+
import com.fasterxml.jackson.databind.DeserializationContext;
6+
import com.fasterxml.jackson.databind.JsonDeserializer;
7+
import com.fasterxml.jackson.databind.JsonNode;
8+
import com.fasterxml.jackson.databind.node.NullNode;
9+
import org.prebid.server.json.ObjectMapperProvider;
10+
11+
import java.io.IOException;
12+
import java.util.HashMap;
13+
import java.util.HashSet;
14+
import java.util.Iterator;
15+
import java.util.Map;
16+
import java.util.Set;
17+
18+
/**
19+
* Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.JsonMergePatchDeserializer}
20+
*/
21+
final class JsonMergePatchDeserializer extends JsonDeserializer<JsonMergePatch> {
22+
23+
/*
24+
* FIXME! UGLY! HACK!
25+
*
26+
* We MUST have an ObjectCodec ready so that the parser in .deserialize()
27+
* can actually do something useful -- for instance, deserializing even a
28+
* JsonNode.
29+
*
30+
* Jackson does not do this automatically; I don't know why...
31+
*/
32+
private static final ObjectCodec CODEC = ObjectMapperProvider.mapper();
33+
34+
@Override
35+
public JsonMergePatch deserialize(final JsonParser jp, final DeserializationContext ctxt) throws IOException {
36+
// FIXME: see comment above
37+
jp.setCodec(CODEC);
38+
final JsonNode node = jp.readValueAsTree();
39+
40+
/*
41+
* Not an object: the simple case
42+
*/
43+
if (!node.isObject()) {
44+
return new NonObjectMergePatch(node);
45+
}
46+
47+
/*
48+
* The complicated case...
49+
*
50+
* We have to build a set of removed members, plus a map of modified
51+
* members.
52+
*/
53+
54+
final Set<String> removedMembers = new HashSet<>();
55+
final Map<String, JsonMergePatch> modifiedMembers = new HashMap<>();
56+
final Iterator<Map.Entry<String, JsonNode>> iterator = node.fields();
57+
58+
Map.Entry<String, JsonNode> entry;
59+
60+
while (iterator.hasNext()) {
61+
entry = iterator.next();
62+
if (entry.getValue().isNull()) {
63+
removedMembers.add(entry.getKey());
64+
} else {
65+
final JsonMergePatch value = deserialize(entry.getValue().traverse(), ctxt);
66+
modifiedMembers.put(entry.getKey(), value);
67+
}
68+
}
69+
70+
return new ObjectMergePatch(removedMembers, modifiedMembers);
71+
}
72+
73+
/*
74+
* This method MUST be overriden... The default is to return null, which is
75+
* not what we want.
76+
*/
77+
@Override
78+
@SuppressWarnings("deprecation")
79+
public JsonMergePatch getNullValue() {
80+
return new NonObjectMergePatch(NullNode.getInstance());
81+
}
82+
}

‎src/main/java/org/prebid/server/json/merge/NonObjectMergePatch.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package org.prebid.server.json.merge;
2+
3+
import com.fasterxml.jackson.core.JsonGenerator;
4+
import com.fasterxml.jackson.databind.JsonNode;
5+
import com.fasterxml.jackson.databind.SerializerProvider;
6+
import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
7+
8+
import java.io.IOException;
9+
10+
/**
11+
* Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.NonObjectMergePatch}
12+
*/
13+
final class NonObjectMergePatch extends JsonMergePatch {
14+
15+
private final JsonNode node;
16+
17+
NonObjectMergePatch(final JsonNode node) {
18+
if (node == null) {
19+
throw new NullPointerException();
20+
}
21+
this.node = node;
22+
}
23+
24+
@Override
25+
public JsonNode apply(final JsonNode input) {
26+
BUNDLE.checkNotNull(input, "jsonPatch.nullValue");
27+
return node;
28+
}
29+
30+
@Override
31+
public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException {
32+
jgen.writeTree(node);
33+
}
34+
35+
@Override
36+
public void serializeWithType(final JsonGenerator jgen,
37+
final SerializerProvider provider,
38+
final TypeSerializer typeSer) throws IOException {
39+
40+
serialize(jgen, provider);
41+
}
42+
}

‎src/main/java/org/prebid/server/json/merge/ObjectMergePatch.java‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package org.prebid.server.json.merge;
2+
3+
import com.fasterxml.jackson.core.JsonGenerator;
4+
import com.fasterxml.jackson.databind.JsonNode;
5+
import com.fasterxml.jackson.databind.SerializerProvider;
6+
import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
7+
import com.fasterxml.jackson.databind.node.NullNode;
8+
import com.fasterxml.jackson.databind.node.ObjectNode;
9+
import com.github.fge.jackson.JacksonUtils;
10+
import com.github.fge.jsonpatch.JsonPatchException;
11+
12+
import java.io.IOException;
13+
import java.util.Map;
14+
import java.util.Set;
15+
16+
/**
17+
* Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.ObjectMergePatch}
18+
*/
19+
final class ObjectMergePatch extends JsonMergePatch {
20+
21+
private final Set<String> removedMembers;
22+
private final Map<String, JsonMergePatch> modifiedMembers;
23+
24+
ObjectMergePatch(final Set<String> removedMembers, final Map<String, JsonMergePatch> modifiedMembers) {
25+
26+
this.removedMembers = Set.copyOf(removedMembers);
27+
this.modifiedMembers = Map.copyOf(modifiedMembers);
28+
}
29+
30+
@Override
31+
public JsonNode apply(final JsonNode input)
32+
throws JsonPatchException {
33+
BUNDLE.checkNotNull(input, "jsonPatch.nullValue");
34+
/*
35+
* If the input is an object, we make a deep copy of it
36+
*/
37+
final ObjectNode ret = input.isObject() ? (ObjectNode) input.deepCopy()
38+
: JacksonUtils.nodeFactory().objectNode();
39+
40+
/*
41+
* Our result is now a JSON Object; first, add (or modify) existing
42+
* members in the result
43+
*/
44+
String key;
45+
JsonNode value;
46+
for (final Map.Entry<String, JsonMergePatch> entry : modifiedMembers.entrySet()) {
47+
48+
key = entry.getKey();
49+
/*
50+
* FIXME: ugly...
51+
*
52+
* We treat missing keys as null nodes; this "works" because in
53+
* the modifiedMembers map, values are JsonMergePatch instances:
54+
*
55+
* * if it is a NonObjectMergePatch, the value is replaced
56+
* unconditionally;
57+
* * if it is an ObjectMergePatch, we get back here; the value will
58+
* be replaced with a JSON Object anyway before being processed.
59+
*/
60+
final JsonNode jsonNode = ret.get(key);
61+
value = jsonNode != null ? jsonNode : NullNode.getInstance();
62+
ret.replace(key, entry.getValue().apply(value));
63+
}
64+
65+
ret.remove(removedMembers);
66+
67+
return ret;
68+
}
69+
70+
@Override
71+
public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException {
72+
jgen.writeStartObject();
73+
74+
/*
75+
* Write removed members as JSON nulls
76+
*/
77+
for (final String member : removedMembers) {
78+
jgen.writeNullField(member);
79+
}
80+
81+
/*
82+
* Write modified members; delegate to serialization for writing values
83+
*/
84+
for (final Map.Entry<String, JsonMergePatch> entry : modifiedMembers.entrySet()) {
85+
jgen.writeFieldName(entry.getKey());
86+
entry.getValue().serialize(jgen, provider);
87+
}
88+
89+
jgen.writeEndObject();
90+
}
91+
92+
@Override
93+
public void serializeWithType(final JsonGenerator jgen,
94+
final SerializerProvider provider,
95+
final TypeSerializer typeSer) throws IOException {
96+
97+
serialize(jgen, provider);
98+
}
99+
}

‎src/test/groovy/org/prebid/server/functional/model/bidder/AppNexus.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ class AppNexus implements BidderAdapter {
1414
String trafficSourceCode
1515
Boolean isAmp
1616
String hbSource
17+
Double reserve
1718

1819
static AppNexus getDefault() {
1920
new AppNexus().tap {

‎src/test/groovy/org/prebid/server/functional/tests/BidderParamsSpec.groovy‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.prebid.server.functional.tests
22

3+
import org.prebid.server.functional.model.bidder.AppNexus
34
import org.prebid.server.functional.model.bidder.BidderName
45
import org.prebid.server.functional.model.bidder.Generic
56
import org.prebid.server.functional.model.db.Account
@@ -23,6 +24,7 @@ import org.prebid.server.functional.model.request.auction.Native
2324
import org.prebid.server.functional.model.request.auction.PrebidOptions
2425
import org.prebid.server.functional.model.request.auction.PrebidStoredRequest
2526
import org.prebid.server.functional.model.request.auction.Site
27+
import org.prebid.server.functional.model.request.auction.Source
2628
import org.prebid.server.functional.model.request.auction.Targeting
2729
import org.prebid.server.functional.model.request.vtrack.VtrackRequest
2830
import org.prebid.server.functional.model.request.vtrack.xml.Vast
@@ -1739,4 +1741,53 @@ class BidderParamsSpec extends BaseSpec {
17391741
cleanup: "Stop and remove pbs container"
17401742
pbsServiceFactory.removeContainer(pbsConfig)
17411743
}
1744+
1745+
def "PBS should merge stored imp with appnexus bidder requested when reserve field specified"() {
1746+
given: "Pbs default config with appnexus"
1747+
def pbsConfig = ["adapters.${APPNEXUS.value}.enabled" : "true",
1748+
"adapters.${APPNEXUS.value}.endpoint": "$networkServiceContainer.rootUri/auction".toString()]
1749+
def defaultPbsService = pbsServiceFactory.getService(pbsConfig)
1750+
1751+
and: "Default stored request with specified stored imps and request"
1752+
def storedRequestId = PBSUtils.randomString
1753+
def bidRequest = BidRequest.getDefaultBidRequest().tap {
1754+
imp[0].ext.prebid.bidder.generic = null
1755+
imp[0].ext.prebid.bidder.appNexus = AppNexus.getDefault().tap {
1756+
reserve = PBSUtils.getRandomDecimal() as Double
1757+
}
1758+
imp[0].ext.prebid.storedRequest = new PrebidStoredRequest(id: PBSUtils.randomString)
1759+
ext.prebid.storedRequest = new PrebidStoredRequest(id: storedRequestId)
1760+
}
1761+
1762+
and: "Save storedImp into DB"
1763+
def storedImp = StoredImp.getStoredImp(bidRequest).tap {
1764+
impData = Imp.defaultImpression
1765+
}
1766+
storedImpDao.save(storedImp)
1767+
1768+
and: "Save stored request with source.tid and cur"
1769+
def storedBidRequest = new BidRequest(cur: [USD], source: new Source(tid: PBSUtils.randomString))
1770+
def storedRequest = StoredRequest.getStoredRequest(storedRequestId, storedBidRequest)
1771+
storedRequestDao.save(storedRequest)
1772+
1773+
and: "Default basic bid with bid.ext"
1774+
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest, APPNEXUS).tap {
1775+
seatbid[0].bid[0].ext = new BidExt()
1776+
}
1777+
bidder.setResponse(bidRequest.id, bidResponse)
1778+
1779+
when: "PBS processes auction request"
1780+
def response = defaultPbsService.sendAuctionRequest(bidRequest)
1781+
1782+
then: "Bid response should contain appnexus and generic bidder"
1783+
assert response.seatbid.size() == 2
1784+
assert response.seatbid.seat.sort() == [APPNEXUS, BidderName.GENERIC].sort()
1785+
1786+
and: "Bidder requests should perform two bidder call"
1787+
def bidderRequests = bidder.getBidderRequests(bidRequest.id)
1788+
assert bidderRequests.size() == 2
1789+
1790+
cleanup: "Stop and remove pbs container"
1791+
pbsServiceFactory.removeContainer(pbsConfig)
1792+
}
17421793
}

‎src/test/java/org/prebid/server/json/merge/JsonMergePatchTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package org.prebid.server.json.merge;
2+
3+
import com.fasterxml.jackson.core.JsonProcessingException;
4+
import com.fasterxml.jackson.databind.JsonNode;
5+
import com.github.fge.jsonpatch.JsonPatchException;
6+
import org.junit.jupiter.api.Test;
7+
import org.prebid.server.VertxTest;
8+
9+
import static org.assertj.core.api.Assertions.assertThat;
10+
11+
public class JsonMergePatchTest extends VertxTest {
12+
13+
@Test
14+
public void fromJsonShouldParseDoublesCorrectly() throws JsonProcessingException, JsonPatchException {
15+
// given
16+
final String source = """
17+
{
18+
"object": {
19+
"property": 0.08
20+
}
21+
}
22+
""";
23+
24+
final JsonNode givenSource = mapper.readTree(source);
25+
final JsonNode givenTarget = mapper.readTree("{}");
26+
27+
// when
28+
final JsonNode oldPatch = com.github.fge.jsonpatch.mergepatch.JsonMergePatch.fromJson(givenSource)
29+
.apply(givenTarget);
30+
final JsonNode newPatch = JsonMergePatch.fromJson(givenSource)
31+
.apply(givenTarget);
32+
33+
// then
34+
assertThat(givenSource).isEqualTo(newPatch);
35+
assertThat(givenSource).isNotEqualTo(oldPatch);
36+
}
37+
}

0 commit comments

Comments
 (0)