Bug 27677910: Fix ReplaceOne(expr()).

Author: silvakid

devapi/crud.cc

@@ -142,14 +142,187 @@ struct Upsert_cmd : public Executable<Result, Upsert_cmd>
 };
 
 
+struct Value_expr_check_id
+  : cdk::Expression
+    , cdk::Expression::Processor
+    , cdk::Expression::Processor::Doc_prc
+{
+  Value_expr &m_expr;
+  bool m_is_expr;
+
+  Processor *m_prc;
+  Doc_prc *m_doc_prc;
+
+  struct Any_processor_check
+      : cdk::Expression::Processor::Doc_prc::Any_prc
+      , cdk::Expression::Processor::Doc_prc::Any_prc::Scalar_prc
+      , cdk::Expression::Processor::Doc_prc::Any_prc::Scalar_prc::Value_prc
+  {
+    Any_prc *m_id_prc;
+    Scalar_prc *m_scalar_prc;
+    Value_prc *m_value_prc;
+    const string& m_id;
+
+    Any_processor_check(const string& id)
+      : m_id(id)
+    {}
+
+    // Any processor implementation
+
+    Scalar_prc* scalar() override
+    {
+      m_scalar_prc = m_id_prc->scalar();
+      return m_scalar_prc ? this : nullptr;
+    }
+
+    List_prc* arr() override
+    {
+      return m_id_prc->arr();
+    }
+
+    Doc_prc* doc() override
+    {
+      return m_id_prc->doc();
+    }
+
+    //Scalar processor implementation
+
+    Value_prc* val() override
+    {
+      m_value_prc = m_scalar_prc->val();
+      return m_value_prc ? this : nullptr;
+    }
+
+    Args_prc* op(const char *name) override
+    {
+      return m_scalar_prc->op(name);
+    }
+    Args_prc* call(const Object_ref&obj) override
+    {
+      return m_scalar_prc->call(obj);
+    }
+
+    void ref(const Column_ref &col, const Doc_path *path) override
+    {
+      return m_scalar_prc->ref(col, path);
+    }
+    void ref(const Doc_path &path) override
+    {
+      return m_scalar_prc->ref(path);
+    }
+
+    void param(const string &val) override
+    {
+      return m_scalar_prc->param(val);
+    }
+
+    void param(uint16_t val) override
+    {
+      return m_scalar_prc->param(val);
+    }
+
+    void var(const string &name) override
+    {
+      m_scalar_prc->var(name);
+    }
+
+    // Value processor implementation
+    virtual void null() override { m_value_prc->null();}
+
+    virtual void value(cdk::Type_info type,
+                       const cdk::Format_info &format,
+                       cdk::foundation::bytes val) override
+    {
+      m_value_prc->value(type, format, val);
+    }
+
+    virtual void str(const string &val)
+    {
+      if (m_id != val)
+        throw Error(R"(Document "_id" and replace id are different!)");
+      m_value_prc->str(val);
+    }
+    virtual void num(int64_t  val) { m_value_prc->num(val); }
+    virtual void num(uint64_t val) { m_value_prc->num(val); }
+    virtual void num(float    val) { m_value_prc->num(val); }
+    virtual void num(double   val) { m_value_prc->num(val); }
+    virtual void yesno(bool   val) { m_value_prc->yesno(val); }
+
+  };
+
+  Any_processor_check m_any_prc;
+
+  Value_expr_check_id(Value_expr &expr, bool is_expr, const string& id)
+    : m_expr(expr)
+    , m_is_expr(is_expr)
+    , m_any_prc(id)
+  {}
+
+  // Expression implementation
+
+  void process(Processor& prc) const override
+  {
+    auto self = const_cast<Value_expr_check_id*>(this);
+    self->m_prc = &prc;
+    m_expr.process(*self);
+  }
+
+  // Expression processor implementation
+
+  Scalar_prc* scalar() override
+  {
+    return m_prc->scalar();
+  }
+
+  List_prc* arr() override
+  {
+    return m_prc->arr();
+  }
+
+  Doc_prc* doc() override
+  {
+    m_doc_prc = m_prc->doc();
+    return m_doc_prc ? this : nullptr;
+  }
+
+  // Doc_prc implementation
+
+  void doc_begin() override
+  {
+    m_doc_prc->doc_begin();
+  }
+
+  void doc_end() override
+  {
+    m_doc_prc->doc_end();
+  }
+
+  Any_prc* key_val(const string &key) override
+  {
+    if (string("_id") == key )
+    {
+      if (m_is_expr)
+        throw Error(R"(Document "_id" will be replaced by expression "_id")");
+
+      m_any_prc.m_id_prc = m_doc_prc->key_val(key);
+      return m_any_prc.m_id_prc ? &m_any_prc : nullptr;
+    }
+    return m_doc_prc->key_val(key);
+  }
+
+};
+
+
 Result
 internal::Collection_detail::add_or_replace_one(
   const string &id, Value &&doc, bool replace
 )
 {
   Object_ref coll(get_schema().m_name, m_name);
 
-  if (doc.getType() == Value::STRING)
+
+  if (!Value::Access::is_expr(doc) &&
+      doc.getType() == Value::STRING)
   {
     doc = DbDoc(doc.get<string>());
   }
@@ -158,22 +331,9 @@ internal::Collection_detail::add_or_replace_one(
 
   if (replace)
   {
-    string doc_id;
-    bool has_id = true;
-
-    try {
-      doc_id = doc.get<DbDoc>()["_id"];
-    } catch (std::out_of_range)
-    {
-      has_id = false;
-    }
-
-    if (has_id && (doc_id != id))
-    {
-      throw Error(R"(Document "_id" and replace id are different!)");
-    }
+    Value_expr_check_id check_id(expr, Value::Access::is_expr(doc), id);
 
-    Replace_cmd cmd(m_sess, coll, std::string(id), expr);
+    Replace_cmd cmd(m_sess, coll, std::string(id), check_id);
     return cmd.execute();
   }
   else

devapi/impl.h

@@ -57,6 +57,16 @@ namespace mysqlx {
 
 struct Value::Access
 {
+
+  /*
+    Check if Value is an expression
+  */
+
+  static bool is_expr(const Value &val)
+  {
+    return val.is_expr();
+  }
+
   /*
     Build document value from a JSON string which is
     assumed to describe a document.

devapi/tests/crud-t.cc

@@ -2251,9 +2251,9 @@ TEST_F(Crud, single_document)
 
   coll.remove("true").execute();
 
-  coll.add("{\"_id\":\"id1\", \"name\":\"foo\" }" )
-      .add("{\"_id\":\"id2\", \"name\":\"bar\" }" )
-      .add("{\"_id\":\"id3\", \"name\":\"baz\" }" )
+  coll.add(R"({"_id":"id1", "name":"foo", "age": 1 })" )
+      .add(R"({"_id":"id2", "name":"bar", "age": 2 })" )
+      .add(R"({"_id":"id3", "name":"baz", "age": 3 })" )
       .execute();
 
   EXPECT_EQ(string("foo"), coll.getOne("id1")["name"].get<string>());
@@ -2266,9 +2266,12 @@ TEST_F(Crud, single_document)
   EXPECT_TRUE(coll.getOne("id1").isNull());
 
   // Replace existing document
-  EXPECT_EQ(1, coll.replaceOne("id3", expr("{\"name\": \"qux\" }"))
-                   .getAffectedItemsCount());
+  EXPECT_EQ(1, coll.replaceOne(
+              "id3", expr(R"({"name": "qux",
+                          "age": cast(age+1 AS UNSIGNED INT) })"))
+            .getAffectedItemsCount());
   EXPECT_EQ(string("qux"), coll.getOne("id3")["name"].get<string>());
+  EXPECT_EQ(4, coll.getOne("id3")["age"].get<int>());
 
   // Setting a different _id on document should throw error
   // Document passed as string
@@ -2290,7 +2293,7 @@ TEST_F(Crud, single_document)
 
   // should affect none
   EXPECT_EQ(0,
-    coll.replaceOne("id4", expr("{\"_id\":\"id4\", \"name\": \"baz\" }"))
+    coll.replaceOne("id4", expr(R"({"name": "baz" })"))
         .getAffectedItemsCount());
 }