Fix Stored Request Merging

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

@@ -3,9 +3,9 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.github.fge.jsonpatch.JsonPatchException;
-import com.github.fge.jsonpatch.mergepatch.JsonMergePatch;
 import org.apache.commons.lang3.ObjectUtils;
 import org.prebid.server.exception.InvalidRequestException;
+import org.prebid.server.json.merge.JsonMergePatch;
 
 import java.io.IOException;
 import java.util.Objects;

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

@@ -0,0 +1,37 @@
+package org.prebid.server.json.merge;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.JsonSerializable;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.github.fge.jsonpatch.JsonPatchException;
+import com.github.fge.jsonpatch.JsonPatchMessages;
+import com.github.fge.jsonpatch.Patch;
+import com.github.fge.msgsimple.bundle.MessageBundle;
+import com.github.fge.msgsimple.load.MessageBundles;
+import org.prebid.server.json.ObjectMapperProvider;
+
+import java.io.IOException;
+
+/**
+ * Json merge patch implementation that uses the application-wide object mapper.
+ * Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.JsonMergePatch}.
+ */
+@JsonDeserialize(using = JsonMergePatchDeserializer.class)
+public abstract class JsonMergePatch implements JsonSerializable, Patch {
+
+    private static final ObjectMapper MAPPER = ObjectMapperProvider.mapper();
+    public static final MessageBundle BUNDLE = MessageBundles.getBundle(JsonPatchMessages.class);
+
+    public static JsonMergePatch fromJson(JsonNode node) throws JsonPatchException {
+        BUNDLE.checkNotNull(node, "jsonPatch.nullInput");
+        try {
+            return MAPPER.readValue(node.traverse(), JsonMergePatch.class);
+        } catch (IOException e) {
+            throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.deserFailed"), e);
+        }
+    }
+
+    @Override
+    public abstract JsonNode apply(JsonNode input) throws JsonPatchException;
+}

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

@@ -0,0 +1,82 @@
+package org.prebid.server.json.merge;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.ObjectCodec;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.NullNode;
+import org.prebid.server.json.ObjectMapperProvider;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.JsonMergePatchDeserializer}
+ */
+final class JsonMergePatchDeserializer extends JsonDeserializer<JsonMergePatch> {
+
+    /*
+     * FIXME! UGLY! HACK!
+     *
+     * We MUST have an ObjectCodec ready so that the parser in .deserialize()
+     * can actually do something useful -- for instance, deserializing even a
+     * JsonNode.
+     *
+     * Jackson does not do this automatically; I don't know why...
+     */
+    private static final ObjectCodec CODEC = ObjectMapperProvider.mapper();
+
+    @Override
+    public JsonMergePatch deserialize(final JsonParser jp, final DeserializationContext ctxt) throws IOException {
+        // FIXME: see comment above
+        jp.setCodec(CODEC);
+        final JsonNode node = jp.readValueAsTree();
+
+        /*
+         * Not an object: the simple case
+         */
+        if (!node.isObject()) {
+            return new NonObjectMergePatch(node);
+        }
+
+        /*
+         * The complicated case...
+         *
+         * We have to build a set of removed members, plus a map of modified
+         * members.
+         */
+
+        final Set<String> removedMembers = new HashSet<>();
+        final Map<String, JsonMergePatch> modifiedMembers = new HashMap<>();
+        final Iterator<Map.Entry<String, JsonNode>> iterator = node.fields();
+
+        Map.Entry<String, JsonNode> entry;
+
+        while (iterator.hasNext()) {
+            entry = iterator.next();
+            if (entry.getValue().isNull()) {
+                removedMembers.add(entry.getKey());
+            } else {
+                final JsonMergePatch value = deserialize(entry.getValue().traverse(), ctxt);
+                modifiedMembers.put(entry.getKey(), value);
+            }
+        }
+
+        return new ObjectMergePatch(removedMembers, modifiedMembers);
+    }
+
+    /*
+     * This method MUST be overriden... The default is to return null, which is
+     * not what we want.
+     */
+    @Override
+    @SuppressWarnings("deprecation")
+    public JsonMergePatch getNullValue() {
+        return new NonObjectMergePatch(NullNode.getInstance());
+    }
+}

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

@@ -0,0 +1,42 @@
+package org.prebid.server.json.merge;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.SerializerProvider;
+import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
+
+import java.io.IOException;
+
+/**
+ * Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.NonObjectMergePatch}
+ */
+final class NonObjectMergePatch extends JsonMergePatch {
+
+    private final JsonNode node;
+
+    NonObjectMergePatch(final JsonNode node) {
+        if (node == null) {
+            throw new NullPointerException();
+        }
+        this.node = node;
+    }
+
+    @Override
+    public JsonNode apply(final JsonNode input) {
+        BUNDLE.checkNotNull(input, "jsonPatch.nullValue");
+        return node;
+    }
+
+    @Override
+    public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException {
+        jgen.writeTree(node);
+    }
+
+    @Override
+    public void serializeWithType(final JsonGenerator jgen,
+                                  final SerializerProvider provider,
+                                  final TypeSerializer typeSer) throws IOException {
+
+        serialize(jgen, provider);
+    }
+}

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

@@ -0,0 +1,99 @@
+package org.prebid.server.json.merge;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.SerializerProvider;
+import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
+import com.fasterxml.jackson.databind.node.NullNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.github.fge.jackson.JacksonUtils;
+import com.github.fge.jsonpatch.JsonPatchException;
+
+import java.io.IOException;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Replicates functionality from {@link com.github.fge.jsonpatch.mergepatch.ObjectMergePatch}
+ */
+final class ObjectMergePatch extends JsonMergePatch {
+
+    private final Set<String> removedMembers;
+    private final Map<String, JsonMergePatch> modifiedMembers;
+
+    ObjectMergePatch(final Set<String> removedMembers, final Map<String, JsonMergePatch> modifiedMembers) {
+
+        this.removedMembers = Set.copyOf(removedMembers);
+        this.modifiedMembers = Map.copyOf(modifiedMembers);
+    }
+
+    @Override
+    public JsonNode apply(final JsonNode input)
+            throws JsonPatchException {
+        BUNDLE.checkNotNull(input, "jsonPatch.nullValue");
+        /*
+         * If the input is an object, we make a deep copy of it
+         */
+        final ObjectNode ret = input.isObject() ? (ObjectNode) input.deepCopy()
+                : JacksonUtils.nodeFactory().objectNode();
+
+        /*
+         * Our result is now a JSON Object; first, add (or modify) existing
+         * members in the result
+         */
+        String key;
+        JsonNode value;
+        for (final Map.Entry<String, JsonMergePatch> entry : modifiedMembers.entrySet()) {
+
+            key = entry.getKey();
+            /*
+             * FIXME: ugly...
+             *
+             * We treat missing keys as null nodes; this "works" because in
+             * the modifiedMembers map, values are JsonMergePatch instances:
+             *
+             * * if it is a NonObjectMergePatch, the value is replaced
+             *   unconditionally;
+             * * if it is an ObjectMergePatch, we get back here; the value will
+             *   be replaced with a JSON Object anyway before being processed.
+             */
+            final JsonNode jsonNode = ret.get(key);
+            value = jsonNode != null ? jsonNode : NullNode.getInstance();
+            ret.replace(key, entry.getValue().apply(value));
+        }
+
+        ret.remove(removedMembers);
+
+        return ret;
+    }
+
+    @Override
+    public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException {
+        jgen.writeStartObject();
+
+        /*
+         * Write removed members as JSON nulls
+         */
+        for (final String member : removedMembers) {
+            jgen.writeNullField(member);
+        }
+
+        /*
+         * Write modified members; delegate to serialization for writing values
+         */
+        for (final Map.Entry<String, JsonMergePatch> entry : modifiedMembers.entrySet()) {
+            jgen.writeFieldName(entry.getKey());
+            entry.getValue().serialize(jgen, provider);
+        }
+
+        jgen.writeEndObject();
+    }
+
+    @Override
+    public void serializeWithType(final JsonGenerator jgen,
+                                  final SerializerProvider provider,
+                                  final TypeSerializer typeSer) throws IOException {
+
+        serialize(jgen, provider);
+    }
+}

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

@@ -14,6 +14,7 @@ class AppNexus implements BidderAdapter {
     String trafficSourceCode
     Boolean isAmp
     String hbSource
+    Double reserve
 
     static AppNexus getDefault() {
         new AppNexus().tap {

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

@@ -1,5 +1,6 @@
 package org.prebid.server.functional.tests
 
+import org.prebid.server.functional.model.bidder.AppNexus
 import org.prebid.server.functional.model.bidder.BidderName
 import org.prebid.server.functional.model.bidder.Generic
 import org.prebid.server.functional.model.db.Account
@@ -23,6 +24,7 @@ import org.prebid.server.functional.model.request.auction.Native
 import org.prebid.server.functional.model.request.auction.PrebidOptions
 import org.prebid.server.functional.model.request.auction.PrebidStoredRequest
 import org.prebid.server.functional.model.request.auction.Site
+import org.prebid.server.functional.model.request.auction.Source
 import org.prebid.server.functional.model.request.auction.Targeting
 import org.prebid.server.functional.model.request.vtrack.VtrackRequest
 import org.prebid.server.functional.model.request.vtrack.xml.Vast
@@ -37,7 +39,6 @@ import static org.prebid.server.functional.model.Currency.CHF
 import static org.prebid.server.functional.model.Currency.EUR
 import static org.prebid.server.functional.model.Currency.JPY
 import static org.prebid.server.functional.model.Currency.USD
-import static org.prebid.server.functional.model.bidder.BidderName.ALIAS
 import static org.prebid.server.functional.model.bidder.BidderName.ALIAS_UPPER_CASE
 import static org.prebid.server.functional.model.bidder.BidderName.AMX
 import static org.prebid.server.functional.model.bidder.BidderName.APPNEXUS
@@ -1740,4 +1741,53 @@ class BidderParamsSpec extends BaseSpec {
         cleanup: "Stop and remove pbs container"
         pbsServiceFactory.removeContainer(pbsConfig)
     }
+
+    def "PBS should merge stored imp with appnexus bidder requested when reserve field specified"() {
+        given: "Pbs default config with appnexus"
+        def pbsConfig = ["adapters.${APPNEXUS.value}.enabled" : "true",
+                         "adapters.${APPNEXUS.value}.endpoint": "$networkServiceContainer.rootUri/auction".toString()]
+        def defaultPbsService = pbsServiceFactory.getService(pbsConfig)
+
+        and: "Default stored request with specified stored imps and request"
+        def storedRequestId = PBSUtils.randomString
+        def bidRequest = BidRequest.getDefaultBidRequest().tap {
+            imp[0].ext.prebid.bidder.generic = null
+            imp[0].ext.prebid.bidder.appNexus = AppNexus.getDefault().tap {
+                reserve = PBSUtils.getRandomDecimal() as Double
+            }
+            imp[0].ext.prebid.storedRequest = new PrebidStoredRequest(id: PBSUtils.randomString)
+            ext.prebid.storedRequest = new PrebidStoredRequest(id: storedRequestId)
+        }
+
+        and: "Save storedImp into DB"
+        def storedImp = StoredImp.getStoredImp(bidRequest).tap {
+            impData = Imp.defaultImpression
+        }
+        storedImpDao.save(storedImp)
+
+        and: "Save stored request with source.tid and cur"
+        def storedBidRequest = new BidRequest(cur: [USD], source: new Source(tid: PBSUtils.randomString))
+        def storedRequest = StoredRequest.getStoredRequest(storedRequestId, storedBidRequest)
+        storedRequestDao.save(storedRequest)
+
+        and: "Default basic bid with bid.ext"
+        def bidResponse = BidResponse.getDefaultBidResponse(bidRequest, APPNEXUS).tap {
+            seatbid[0].bid[0].ext = new BidExt()
+        }
+        bidder.setResponse(bidRequest.id, bidResponse)
+
+        when: "PBS processes auction request"
+        def response = defaultPbsService.sendAuctionRequest(bidRequest)
+
+        then: "Bid response should contain appnexus and generic bidder"
+        assert response.seatbid.size() == 2
+        assert response.seatbid.seat.sort() == [APPNEXUS, BidderName.GENERIC].sort()
+
+        and: "Bidder requests should perform two bidder call"
+        def bidderRequests = bidder.getBidderRequests(bidRequest.id)
+        assert bidderRequests.size() == 2
+
+        cleanup: "Stop and remove pbs container"
+        pbsServiceFactory.removeContainer(pbsConfig)
+    }
 }

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

@@ -0,0 +1,37 @@
+package org.prebid.server.json.merge;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.github.fge.jsonpatch.JsonPatchException;
+import org.junit.jupiter.api.Test;
+import org.prebid.server.VertxTest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class JsonMergePatchTest extends VertxTest {
+
+    @Test
+    public void fromJsonShouldParseDoublesCorrectly() throws JsonProcessingException, JsonPatchException {
+        // given
+        final String source = """
+                {
+                    "object": {
+                        "property": 0.08
+                    }
+                }
+                """;
+
+        final JsonNode givenSource = mapper.readTree(source);
+        final JsonNode givenTarget = mapper.readTree("{}");
+
+        // when
+        final JsonNode oldPatch = com.github.fge.jsonpatch.mergepatch.JsonMergePatch.fromJson(givenSource)
+                .apply(givenTarget);
+        final JsonNode newPatch = JsonMergePatch.fromJson(givenSource)
+                .apply(givenTarget);
+
+        // then
+        assertThat(givenSource).isEqualTo(newPatch);
+        assertThat(givenSource).isNotEqualTo(oldPatch);
+    }
+}