Improve handling of UTF8 error on Windows

realm · Sep 6, 2022 · 284bdcd · 284bdcd
1 parent c47cbdf
commit 284bdcd
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 132 deletions.
diff --git a/src/realm/exceptions.hpp b/src/realm/exceptions.hpp
@@ -176,6 +176,25 @@ class NoSubscriptionForWrite : public std::runtime_error {
     NoSubscriptionForWrite(const std::string& msg);
 };
 
+namespace query_parser {
+
+/// Exception thrown when parsing fails due to invalid syntax.
+struct SyntaxError : std::runtime_error {
+    using std::runtime_error::runtime_error;
+};
+
+/// Exception thrown when binding a syntactically valid query string in a
+/// context where it does not make sense.
+struct InvalidQueryError : std::runtime_error {
+    using std::runtime_error::runtime_error;
+};
+
+/// Exception thrown when there is a problem accessing the arguments in a query string
+struct InvalidQueryArgError : std::invalid_argument {
+    using std::invalid_argument::invalid_argument;
+};
+
+} // namespace query_parser
 
 /// The \c LogicError exception class is intended to be thrown only when
 /// applications (or bindings) violate rules that are stated (or ought to have

diff --git a/src/realm/parser/query_parser.hpp b/src/realm/parser/query_parser.hpp
@@ -31,22 +31,6 @@
 
 namespace realm::query_parser {
 
-/// Exception thrown when parsing fails due to invalid syntax.
-struct SyntaxError : std::runtime_error {
-    using std::runtime_error::runtime_error;
-};
-
-/// Exception thrown when binding a syntactically valid query string in a
-/// context where it does not make sense.
-struct InvalidQueryError : std::runtime_error {
-    using std::runtime_error::runtime_error;
-};
-
-/// Exception thrown when there is a problem accessing the arguments in a query string
-struct InvalidQueryArgError : std::invalid_argument {
-    using std::invalid_argument::invalid_argument;
-};
-
 struct AnyContext {
     template <typename T>
     T unbox(const std::any& wrapper)

diff --git a/src/realm/query.cpp b/src/realm/query.cpp
@@ -1827,20 +1827,6 @@ void* Query::query_thread(void* arg)
 
 #endif // REALM_MULTITHREADQUERY
 
-std::string Query::validate() const
-{
-    if (!m_groups.size())
-        return "";
-
-    if (error_code != "") // errors detected by QueryInterface
-        return error_code;
-
-    if (!root_node())
-        return "Syntax error";
-
-    return root_node()->validate(); // errors detected by QueryEngine
-}
-
 std::string Query::get_description(util::serializer::SerialisationState& state) const
 {
     std::string description;

diff --git a/src/realm/query.hpp b/src/realm/query.hpp
@@ -320,8 +320,6 @@ class Query final {
     // or empty vector if the query is not associated with a table.
     TableVersions sync_view_if_needed() const;
 
-    std::string validate() const;
-
     std::string get_description(const std::string& class_prefix = "") const;
     std::string get_description(util::serializer::SerialisationState& state) const;
 

diff --git a/src/realm/query_engine.hpp b/src/realm/query_engine.hpp
@@ -225,16 +225,6 @@ class ParentNode {
                                    ArrayPayload* source_column);
 
 
-    virtual std::string validate()
-    {
-        if (error_code != "")
-            return error_code;
-        if (m_child == nullptr)
-            return "";
-        else
-            return m_child->validate();
-    }
-
     ParentNode(const ParentNode& from);
 
     void add_child(std::unique_ptr<ParentNode> child)
@@ -320,7 +310,6 @@ class ParentNode {
     ConstTableRef m_table = ConstTableRef();
     const Cluster* m_cluster = nullptr;
     QueryStateBase* m_state = nullptr;
-    std::string error_code;
     static std::vector<ObjKey> s_dummy_keys;
 
     ColumnType get_real_column_type(ColKey key)
@@ -1582,7 +1571,7 @@ class StringNode : public StringNodeBase {
         auto upper = case_map(v, true);
         auto lower = case_map(v, false);
         if (!upper || !lower) {
-            error_code = "Malformed UTF-8: " + std::string(v);
+            throw query_parser::InvalidQueryError(util::format("Malformed UTF-8: %1", v));
         }
         else {
             m_ucase = std::move(*upper);
@@ -1707,7 +1696,7 @@ class StringNode<ContainsIns> : public StringNodeBase {
         auto upper = case_map(v, true);
         auto lower = case_map(v, false);
         if (!upper || !lower) {
-            error_code = "Malformed UTF-8: " + std::string(v);
+            throw query_parser::InvalidQueryError(util::format("Malformed UTF-8: %1", v));
         }
         else {
             m_ucase = std::move(*upper);
@@ -1921,7 +1910,7 @@ class StringNode<EqualIns> : public StringNodeEqualBase {
         auto upper = case_map(v, true);
         auto lower = case_map(v, false);
         if (!upper || !lower) {
-            error_code = "Malformed UTF-8: " + std::string(v);
+            throw query_parser::InvalidQueryError(util::format("Malformed UTF-8: %1", v));
         }
         else {
             m_ucase = std::move(*upper);
@@ -2106,27 +2095,6 @@ class OrNode : public ParentNode {
         return index;
     }
 
-    std::string validate() override
-    {
-        if (error_code != "")
-            return error_code;
-        if (m_conditions.size() == 0)
-            return "Missing left-hand side of OR";
-        if (m_conditions.size() == 1)
-            return "Missing right-hand side of OR";
-        std::string s;
-        if (m_child != 0)
-            s = m_child->validate();
-        if (s != "")
-            return s;
-        for (size_t i = 0; i < m_conditions.size(); ++i) {
-            s = m_conditions[i]->validate();
-            if (s != "")
-                return s;
-        }
-        return "";
-    }
-
     std::unique_ptr<ParentNode> clone() const override
     {
         return std::unique_ptr<ParentNode>(new OrNode(*this));
@@ -2166,6 +2134,9 @@ class NotNode : public ParentNode {
         : m_condition(std::move(condition))
     {
         m_dT = 50.0;
+        if (!m_condition) {
+            throw query_parser::InvalidQueryError("Missing argument to Not");
+        }
     }
 
     void table_changed() override
@@ -2194,23 +2165,6 @@ class NotNode : public ParentNode {
 
     size_t find_first_local(size_t start, size_t end) override;
 
-    std::string validate() override
-    {
-        if (error_code != "")
-            return error_code;
-        if (m_condition == 0)
-            return "Missing argument to Not";
-        std::string s;
-        if (m_child != 0)
-            s = m_child->validate();
-        if (s != "")
-            return s;
-        s = m_condition->validate();
-        if (s != "")
-            return s;
-        return "";
-    }
-
     std::string describe(util::serializer::SerialisationState& state) const override
     {
         if (m_condition) {

diff --git a/test/test_query2.cpp b/test/test_query2.cpp
@@ -864,54 +864,6 @@ TEST(Query_FindAllContainsUnicode)
     CHECK_EQUAL(3, tv2[3].get<Int>(col_id));
 }
 
-TEST(Query_SyntaxCheck)
-{
-    Table table;
-    auto col_int = table.add_column(type_Int, "1");
-    table.add_column(type_String, "2");
-
-    std::string s;
-
-    table.create_object().set_all(1, "a");
-    table.create_object().set_all(2, "a");
-    table.create_object().set_all(3, "X");
-
-    Query q1 = table.where().equal(col_int, 2).end_group();
-    s = q1.validate();
-    CHECK(s != "");
-
-    Query q2 = table.where().group().group().equal(col_int, 2).end_group();
-    s = q2.validate();
-    CHECK(s != "");
-
-    Query q3 = table.where().equal(col_int, 2).Or();
-    s = q3.validate();
-    CHECK(s != "");
-
-    Query q4 = table.where().Or().equal(col_int, 2);
-    s = q4.validate();
-    CHECK(s != "");
-
-    Query q5 = table.where().equal(col_int, 2);
-    s = q5.validate();
-    CHECK(s == "");
-
-    Query q6 = table.where().group().equal(col_int, 2);
-    s = q6.validate();
-    CHECK(s != "");
-
-    // FIXME: Work is currently underway to fully support locale
-    // independent case folding as defined by Unicode. Reenable this test
-    // when is becomes available.
-    /*
-    Query q7 = ttt.where().equal(1, "\xa0", false);
-#ifdef REALM_DEBUG
-    s = q7.verify();
-    CHECK(s != "");
-#endif
-    */
-}
-
 TEST(Query_TestTV_where)
 {
     // When using .where(&tv), tv can have any order, and the resulting view will retain its order