Handle empty object case for Mongo update (#179)

* Handle empty object case for Mongo update
hypertrace · Oct 26, 2023 · 85aa1ea · 85aa1ea
1 parent 5e587b2
commit 85aa1ea
Show file tree

Hide file tree

Showing 4 changed files with 313 additions and 2 deletions.
diff --git a/...store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java b/...store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java
@@ -1,5 +1,7 @@
 package org.hypertrace.core.documentstore;
 
+import static java.util.Collections.emptyMap;
+import static java.util.Map.entry;
 import static java.util.Spliterators.spliteratorUnknownSize;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.stream.Collectors.toUnmodifiableList;
@@ -2293,6 +2295,33 @@ public void testAtomicUpdateDocumentWithoutSelections(final String datastoreName
 
   @Nested
   class UpdateOperatorTest {
+    @ParameterizedTest
+    @ArgumentsSource(AllProvider.class)
+    void testUpdateSetEmptyObject(final String datastoreName) throws IOException {
+      final Collection collection = getCollection(datastoreName, UPDATABLE_COLLECTION_NAME);
+      createCollectionData("query/updatable_collection_data.json", UPDATABLE_COLLECTION_NAME);
+
+      final SubDocumentUpdate set =
+          SubDocumentUpdate.builder()
+              .subDocument("props.new_property.with.empty.object")
+              .operator(SET)
+              .subDocumentValue(
+                  SubDocumentValue.of(
+                      new JSONDocument(
+                          Map.ofEntries(
+                              entry("hello", "world"), entry("emptyObject", emptyMap())))))
+              .build();
+
+      final Query query = Query.builder().build();
+      final List<SubDocumentUpdate> updates = List.of(set);
+
+      final CloseableIterator<Document> iterator =
+          collection.bulkUpdate(
+              query, updates, UpdateOptions.builder().returnDocumentType(AFTER_UPDATE).build());
+      assertDocsAndSizeEqualWithoutOrder(
+          datastoreName, iterator, "query/update_operator/updated3.json", 9);
+    }
+
     @ParameterizedTest
     @ArgumentsSource(AllProvider.class)
     void testUpdateWithAllOperators(final String datastoreName) throws IOException {

diff --git a/document-store/src/integrationTest/resources/query/update_operator/updated3.json b/document-store/src/integrationTest/resources/query/update_operator/updated3.json
@@ -0,0 +1,273 @@
+[
+  {
+    "item": "Soap",
+    "price": 10,
+    "quantity": 2,
+    "date": "2014-03-01T08:00:00Z",
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      },
+      "brand": "Dettol",
+      "size": "M",
+      "seller": {
+        "name": "Metro Chemicals Pvt. Ltd.",
+        "address": {
+          "city": "Mumbai",
+          "pincode": 400004
+        }
+      }
+    },
+    "sales": [
+      {
+        "city": "delhi",
+        "medium": [
+          {
+            "type": "distributionChannel",
+            "volume": 1000
+          },
+          {
+            "type": "retail",
+            "volume": 500
+          },
+          {
+            "type": "online",
+            "volume": 1000
+          }
+        ]
+      },
+      {
+        "city": "pune",
+        "medium": [
+          {
+            "type": "distributionChannel",
+            "volume": 300
+          },
+          {
+            "type": "online",
+            "volume": 2000
+          }
+        ]
+      }
+    ]
+  },
+  {
+    "item": "Mirror",
+    "price": 20,
+    "quantity": 1,
+    "date": "2014-03-01T09:00:00Z",
+    "sales": [
+      {
+        "city": "delhi",
+        "medium": []
+      }
+    ],
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      }
+    }
+  },
+  {
+    "item": "Shampoo",
+    "price": 5,
+    "quantity": 10,
+    "date": "2014-03-15T09:00:00Z",
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      },
+      "brand": "Sunsilk",
+      "size": "L",
+      "seller": {
+        "name": "Metro Chemicals Pvt. Ltd.",
+        "address": {
+          "city": "Mumbai",
+          "pincode": 400004
+        }
+      }
+    },
+    "sales": [
+      {
+        "city": "delhi",
+        "medium": [
+          {
+            "type": "distributionChannel",
+            "volume": 3000
+          },
+          {
+            "type": "retail",
+            "volume": 500
+          },
+          {
+            "type": "online",
+            "volume": 1000
+          }
+        ]
+      },
+      {
+        "city": "mumbai",
+        "medium": [
+          {
+            "type": "distributionChannel",
+            "volume": 700
+          },
+          {
+            "type": "retail",
+            "volume": 500
+          },
+          {
+            "type": "online",
+            "volume": 5000
+          }
+        ]
+      }
+    ]
+  },
+  {
+    "item": "Shampoo",
+    "price": 5,
+    "quantity": 20,
+    "date": "2014-04-04T11:21:39.736Z",
+    "sales": [],
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      }
+    }
+  },
+  {
+    "item": "Soap",
+    "price": 20,
+    "quantity": 5,
+    "date": "2014-04-04T21:23:13.331Z",
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      },
+      "brand": "Lifebuoy",
+      "size": "S",
+      "seller": {
+        "name": "Hans and Co.",
+        "address": {
+          "city": "Kolkata",
+          "pincode": 700007
+        }
+      }
+    }
+  },
+  {
+    "item": "Comb",
+    "price": 7.5,
+    "quantity": 5,
+    "date": "2015-06-04T05:08:13Z",
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      }
+    }
+  },
+  {
+    "item": "Comb",
+    "price": 7.5,
+    "quantity": 10,
+    "date": "2015-09-10T08:43:00Z",
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      },
+      "seller": {
+        "name": "Go Go Plastics",
+        "address": {
+          "city": "Kolkata",
+          "pincode": 700007
+        }
+      }
+    }
+  },
+  {
+    "item": "Soap",
+    "price": 10,
+    "quantity": 5,
+    "date": "2016-02-06T20:20:13Z",
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      }
+    }
+  },
+  {
+    "item": "Soap",
+    "price": 88,
+    "quantity": 50,
+    "date": "2023-08-09T18:53:17Z",
+    "props": {
+      "new_property": {
+        "with": {
+          "empty": {
+            "object": {
+              "hello": "world",
+              "emptyObject": {}
+            }
+          }
+        }
+      }
+    }
+  }
+]
diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/mongo/MongoUtils.java b/document-store/src/main/java/org/hypertrace/core/documentstore/mongo/MongoUtils.java
@@ -3,6 +3,7 @@
 import static com.mongodb.client.model.ReturnDocument.AFTER;
 import static com.mongodb.client.model.ReturnDocument.BEFORE;
 import static java.util.Map.entry;
+import static java.util.function.UnaryOperator.identity;
 import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.toUnmodifiableMap;
 import static org.hypertrace.core.documentstore.model.options.ReturnDocumentType.AFTER_UPDATE;
@@ -72,6 +73,14 @@ public static String sanitizeJsonString(final String jsonString) throws JsonProc
     return MAPPER.writeValueAsString(sanitizedJsonNode);
   }
 
+  public static String sanitizeJsonStringWithoutLiteralWrapping(final String jsonString)
+      throws JsonProcessingException {
+    final JsonNode jsonNode = MAPPER.readTree(jsonString);
+    // escape "." and "$" in field names since Mongo DB does not like them
+    final JsonNode sanitizedJsonNode = recursiveClone(jsonNode, MongoUtils::encodeKey, identity());
+    return MAPPER.writeValueAsString(sanitizedJsonNode);
+  }
+
   public static BasicDBObject merge(final List<BasicDBObject> basicDbObjects) {
     return basicDbObjects.stream()
         .map(Map::entrySet)

diff --git a/...main/java/org/hypertrace/core/documentstore/mongo/subdoc/MongoSubDocumentValueParser.java b/...main/java/org/hypertrace/core/documentstore/mongo/subdoc/MongoSubDocumentValueParser.java
@@ -3,7 +3,7 @@
 import static org.hypertrace.core.documentstore.model.subdoc.UpdateOperator.ADD_TO_LIST_IF_ABSENT;
 import static org.hypertrace.core.documentstore.model.subdoc.UpdateOperator.APPEND_TO_LIST;
 import static org.hypertrace.core.documentstore.model.subdoc.UpdateOperator.REMOVE_ALL_FROM_LIST;
-import static org.hypertrace.core.documentstore.mongo.MongoUtils.sanitizeJsonString;
+import static org.hypertrace.core.documentstore.mongo.MongoUtils.sanitizeJsonStringWithoutLiteralWrapping;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.mongodb.BasicDBObject;
@@ -54,7 +54,7 @@ public Object visit(final NullSubDocumentValue value) {
 
   private BasicDBObject parse(final String jsonValue) {
     try {
-      return BasicDBObject.parse(sanitizeJsonString(jsonValue));
+      return BasicDBObject.parse(sanitizeJsonStringWithoutLiteralWrapping(jsonValue));
     } catch (final JsonProcessingException e) {
       throw new RuntimeException(e);
     }