AVRO-4026: [c++] Add new methods to CustomAttributes to allow non-string custom attributes in schemas

lang/c++/impl/Compiler.cc

@@ -277,7 +277,7 @@ static void getCustomAttributes(const Object &m, CustomAttributes &customAttribu
     const std::unordered_set<std::string> &kKnownFields = getKnownFields();
     for (const auto &entry : m) {
         if (kKnownFields.find(entry.first) == kKnownFields.end()) {
-            customAttributes.addAttribute(entry.first, entry.second.stringValue());
+            customAttributes.addAttribute(entry.first, entry.second.toString());
         }
     }
 }
@@ -300,7 +300,7 @@ static Field makeField(const Entity &e, SymbolTable &st, const string &ns) {
     }
     GenericDatum d = (it2 == m.end()) ? GenericDatum() : makeGenericDatum(node, it2->second, st);
     // Get custom attributes
-    CustomAttributes customAttributes;
+    CustomAttributes customAttributes(CustomAttributes::json);
     getCustomAttributes(m, customAttributes);
     return Field(std::move(n), std::move(aliases), node, d, customAttributes);
 }
@@ -424,7 +424,7 @@ static NodePtr makeArrayNode(const Entity &e, const Object &m,
     if (containsField(m, "doc")) {
         node->setDoc(getDocField(e, m));
     }
-    CustomAttributes customAttributes;
+    CustomAttributes customAttributes(CustomAttributes::json);
     getCustomAttributes(m, customAttributes);
     node->addCustomAttributesForField(customAttributes);
     return node;

lang/c++/impl/CustomAttributes.cc

@@ -16,26 +16,57 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "CustomAttributes.hh"
-#include "Exception.hh"
 #include <map>
 #include <memory>
 
+#include "CustomAttributes.hh"
+#include "Exception.hh"
+
+#include "json/JsonDom.hh"
+
 namespace avro {
 
+CustomAttributes::CustomAttributes(ValueMode valueMode) {
+    switch (valueMode) {
+    case CustomAttributes::string:
+    case CustomAttributes::json:
+        valueMode_ = valueMode;
+        break;
+    default:
+        throw Exception("invalid ValueMode: " + std::to_string(valueMode));
+    }
+}
+
 std::optional<std::string> CustomAttributes::getAttribute(const std::string &name) const {
-    std::optional<std::string> result;
-    std::map<std::string, std::string>::const_iterator iter =
-        attributes_.find(name);
+    auto iter = attributes_.find(name);
     if (iter == attributes_.end()) {
-        return result;
+        return {};
     }
-    result = iter->second;
-    return result;
+    return iter->second;
 }
 
 void CustomAttributes::addAttribute(const std::string &name,
                                     const std::string &value) {
+    // Validate the incoming value.
+    //
+    // NOTE: This is a bit annoying that we accept the data as a string instead of
+    // as an Entity. That means the compiler must convert the value to a string only
+    // for this method to convert it back. But we can't directly refer to the
+    // json::Entity type in the signatures for this class (and thus cannot accept
+    // that type directly as a parameter) because then it would need to be included
+    // from a header file: CustomAttributes.hh. But the json header files are not
+    // part of the Avro distribution, so CustomAttributes.hh cannot #include any of
+    // the json header files.
+    if (valueMode_ == CustomAttributes::string) {
+        try {
+            json::loadEntity(("\"" + value + "\"").c_str());
+        } catch (json::TooManyValuesException e) {
+            throw Exception("string has malformed or missing escapes");
+        }
+    } else {
+        json::loadEntity(value.c_str());
+    }
+
     auto iter_and_find =
         attributes_.insert(std::pair<std::string, std::string>(name, value));
     if (!iter_and_find.second) {
@@ -45,9 +76,15 @@ void CustomAttributes::addAttribute(const std::string &name,
 
 void CustomAttributes::printJson(std::ostream &os,
                                  const std::string &name) const {
-    if (attributes().find(name) == attributes().end()) {
+    auto iter = attributes_.find(name);
+    if (iter == attributes_.end()) {
         throw Exception(name + " doesn't exist");
     }
-    os << "\"" << name << "\": \"" << attributes().at(name) << "\"";
+    os << json::Entity(std::make_shared<std::string>(name)).toString() << ": ";
+    if (valueMode_ == CustomAttributes::string) {
+        os << "\"" << iter->second << "\"";
+    } else {
+        os << iter->second;
+    }
 }
 } // namespace avro

lang/c++/impl/json/JsonDom.cc

@@ -92,7 +92,11 @@ Entity loadEntity(const char *text) {
 Entity loadEntity(InputStream &in) {
     JsonParser p;
     p.init(in);
-    return readEntity(p);
+    Entity e = readEntity(p);
+    if (p.hasMore()) {
+        throw TooManyValuesException();
+    }
+    return e;
 }
 
 Entity loadEntity(const uint8_t *text, size_t len) {

lang/c++/impl/json/JsonDom.hh

@@ -177,6 +177,12 @@ AVRO_DECL Entity loadEntity(const uint8_t *text, size_t len);
 
 void writeEntity(JsonGenerator<JsonNullFormatter> &g, const Entity &n);
 
+class AVRO_DECL TooManyValuesException : public virtual std::runtime_error {
+public:
+    explicit TooManyValuesException() :
+        std::runtime_error("invalid JSON document: expecting a single JSON value but found more than one") {}
+};
+
 } // namespace json
 } // namespace avro
 

lang/c++/impl/json/JsonIO.cc

@@ -49,6 +49,28 @@ char JsonParser::next() {
     return ch;
 }
 
+bool JsonParser::hasMore() {
+    if (peeked || hasNext && !isspace(nextChar)) {
+        return true;
+    }
+    if (!hasNext) {
+        nextChar = ' ';
+    }
+    // We return true if there are any more tokens; we ignore
+    // any trailing whitespace.
+    while (isspace(nextChar)) {
+        if (nextChar == '\n') {
+            line_++;
+        }
+        if (!in_.hasMore()) {
+            return false;
+        }
+        nextChar = in_.read();
+    }
+    hasNext = true;
+    return true;
+}
+
 void JsonParser::expectToken(Token tk) {
     if (advance() != tk) {
         if (tk == Token::Double) {

lang/c++/impl/json/JsonIO.hh

@@ -116,6 +116,8 @@ public:
         return curToken;
     }
 
+    bool hasMore();
+
     void expectToken(Token tk);
 
     bool boolValue() const {

lang/c++/include/avro/CustomAttributes.hh

@@ -19,27 +19,76 @@
 #ifndef avro_CustomAttributes_hh__
 #define avro_CustomAttributes_hh__
 
-#include "Config.hh"
 #include <iostream>
 #include <map>
 #include <optional>
 #include <string>
 
+#include "Config.hh"
+
 namespace avro {
 
 // CustomAttributes class stores avro custom attributes.
 // Each attribute is represented by a unique name and value.
 // User is supposed to create CustomAttributes object and then add it to Schema.
 class AVRO_DECL CustomAttributes {
+
 public:
-    // Retrieves the custom attribute json entity for that attributeName, returns an
-    // null if the attribute doesn't exist.
+    enum ValueMode {
+        // When a CustomAttributes is created using this mode, all values are strings.
+        // The value should not be quoted, but any interior quotes and special
+        // characters (such as newlines) must be escaped.
+        string,
+        // When a CustomAttributes is created using this mode, all values are JSON
+        // values. String values must be quoted and escaped.
+        json
+    };
+
+    // Creates a new CustomAttributes object where all values are strings.
+    // All values passed to addAttribute() and returned from getAttribute() or the
+    // attributes() map will not be enclosed in quotes. However, any internal quotes
+    // WILL be escaped and other special characters MAY be escaped.
+    //
+    // To support non-string values, use CustomAttributes(CustomAttributes::json) instead.
+    CustomAttributes() : CustomAttributes(string) {}
+
+    // Creates a new CustomAttributes object.
+    //
+    // If the given mode is string, all values must be strings. All values passed to
+    // addAttribute() and returned from getAttribute() or the attributes() map will not
+    // be enclosed in quotes. However, any internal quotes and newlines WILL be escaped
+    // and other special characters MAY be escaped.
+    //
+    // If the given mode is json, the values support any valid JSON type. In this more,
+    // all values passed to addAttribute() and returned from getAttribute() or the
+    // attributes() map must be valid JSON values; string values must be quoted and
+    // escaped.
+    CustomAttributes(ValueMode valueMode);
+
+    // Retrieves the custom attribute string for the given name. Returns an empty
+    // value if the attribute doesn't exist.
+    //
+    // If this CustomAttributes was created in json mode, the returned string will
+    // be a JSON document (and string values will be quoted and escaped). Otherwise,
+    // the returned string will be a string value, though interior quotes and newlines
+    // will be escaped and other special characters may be escaped.
     std::optional<std::string> getAttribute(const std::string &name) const;
 
-    // Adds a custom attribute. If the attribute already exists, throw an exception.
+    // Adds a custom attribute.
+    //
+    // If this CustomAttributes was create in json mode, the given value string must
+    // be a valid JSON document. So if the value is a string, it must be quoted and
+    // escaped. Otherwise, the given value string is an unquoted string value (though
+    // interior quotes and newlines must still be escaped).
+    //
+    // If the attribute already exists or if the given value is not a valid JSON
+    // document or not a correctly escaped string, throw an exception.
     void addAttribute(const std::string &name, const std::string &value);
 
     // Provides a way to iterate over the custom attributes or check attribute size.
+    // The values in this map are the same as those returned from getAttribute. So
+    // the value may be a JSON document or an unquoted string, depending on whether
+    // this CustomAttributes was created in json or string mode.
     const std::map<std::string, std::string> &attributes() const {
         return attributes_;
     }
@@ -48,6 +97,7 @@ public:
     void printJson(std::ostream &os, const std::string &name) const;
 
 private:
+    ValueMode valueMode_;
     std::map<std::string, std::string> attributes_;
 };
 

lang/c++/test/SchemaTests.cc

@@ -495,7 +495,7 @@ static void testLogicalTypes() {
     const char *durationType = R"({"type": "fixed","size": 12,"name": "durationType","logicalType": "duration"})";
     const char *uuidType = R"({"type": "string","logicalType": "uuid"})";
     // AVRO-2923 Union with LogicalType
-    const char *unionType = R"([{"type":"string", "logicalType":"uuid"},"null"]})";
+    const char *unionType = R"([{"type":"string", "logicalType":"uuid"},"null"])";
     {
         BOOST_TEST_CHECKPOINT(bytesDecimalType);
         ValidSchema schema1 = compileJsonSchemaFromString(bytesDecimalType);

lang/c++/test/unittest.cc

@@ -440,29 +440,31 @@ struct TestSchema {
         std::vector<GenericDatum> defaultValues;
         concepts::MultiAttribute<CustomAttributes> customAttributes;
 
-        CustomAttributes cf;
-        cf.addAttribute("stringField", std::string("\\\"field value with \\\"double quotes\\\"\\\""));
-        cf.addAttribute("booleanField", std::string("true"));
-        cf.addAttribute("numberField", std::string("1.23"));
-        cf.addAttribute("nullField", std::string("null"));
-        cf.addAttribute("arrayField", std::string("[1]"));
-        cf.addAttribute("mapField", std::string("{\\\"key1\\\":\\\"value1\\\", \\\"key2\\\":\\\"value2\\\"}"));
+        CustomAttributes ca(CustomAttributes::json);
+        ca.addAttribute("stringField", std::string("\"foobar\""));
+        ca.addAttribute("stringFieldComplex", std::string("\"\\\" a field value with \\\"double quotes\\\" \\\"\""));
+        ca.addAttribute("booleanField", std::string("true"));
+        ca.addAttribute("numberField", std::string("1.23"));
+        ca.addAttribute("nullField", std::string("null"));
+        ca.addAttribute("arrayField", std::string("[1]"));
+        ca.addAttribute("mapField", std::string("{\"key1\":\"value1\", \"key2\":\"value2\"}"));
         fieldNames.add("f1");
         fieldValues.add(NodePtr(new NodePrimitive(Type::AVRO_LONG)));
-        customAttributes.add(cf);
+        customAttributes.add(ca);
 
         NodeRecord nodeRecordWithCustomAttribute(nameConcept, fieldValues,
                                                  fieldNames, fieldAliases, defaultValues,
                                                  customAttributes);
         std::string expectedJsonWithCustomAttribute =
             "{\"type\": \"record\", \"name\": \"Test\",\"fields\": "
             "[{\"name\": \"f1\", \"type\": \"long\", "
-            "\"arrayField\": \"[1]\", "
-            "\"booleanField\": \"true\", "
-            "\"mapField\": \"{\\\"key1\\\":\\\"value1\\\", \\\"key2\\\":\\\"value2\\\"}\", "
-            "\"nullField\": \"null\", "
-            "\"numberField\": \"1.23\", "
-            "\"stringField\": \"\\\"field value with \\\"double quotes\\\"\\\"\""
+            "\"arrayField\": [1], "
+            "\"booleanField\": true, "
+            "\"mapField\": {\"key1\":\"value1\", \"key2\":\"value2\"}, "
+            "\"nullField\": null, "
+            "\"numberField\": 1.23, "
+            "\"stringField\": \"foobar\", "
+            "\"stringFieldComplex\": \"\\\" a field value with \\\"double quotes\\\" \\\"\""
             "}]}";
         testNodeRecord(nodeRecordWithCustomAttribute,
                        expectedJsonWithCustomAttribute);
@@ -489,12 +491,26 @@ struct TestSchema {
                        expectedJsonWithoutCustomAttribute);
     }
 
-    void checkCustomAttributes_getAttribute() {
-        CustomAttributes cf;
-        cf.addAttribute("field1", std::string("1"));
+    void checkCustomAttributes_addAndGetAttributeJson() {
+        CustomAttributes ca(CustomAttributes::json);
+        ca.addAttribute("field1", std::string("true"));
 
-        BOOST_CHECK_EQUAL(std::string("1"), *cf.getAttribute("field1"));
-        BOOST_CHECK_EQUAL(false, cf.getAttribute("not_existing").has_value());
+        BOOST_CHECK_EQUAL(std::string("true"), *ca.getAttribute("field1"));
+        BOOST_CHECK_EQUAL(false, ca.getAttribute("not_existing").has_value());
+    }
+
+    void checkCustomAttributes_addAndGetAttributeString() {
+        CustomAttributes ca;
+        ca.addAttribute("field1", std::string("true"));
+        ca.addAttribute("field2", std::string("value with \\\"quotes\\\""));
+
+        BOOST_CHECK_EQUAL(std::string("true"), *ca.getAttribute("field1"));
+        BOOST_CHECK_EQUAL(std::string("value with \\\"quotes\\\""), *ca.getAttribute("field2"));
+        BOOST_CHECK_EQUAL(false, ca.getAttribute("not_existing").has_value());
+
+        std::ostringstream oss;
+        ca.printJson(oss, "field2");
+        BOOST_CHECK_EQUAL(std::string("\"field2\": \"value with \\\"quotes\\\"\""), oss.str());
     }
 
     void test() {
@@ -521,7 +537,8 @@ struct TestSchema {
 
         checkNodeRecordWithoutCustomAttribute();
         checkNodeRecordWithCustomAttribute();
-        checkCustomAttributes_getAttribute();
+        checkCustomAttributes_addAndGetAttributeJson();
+        checkCustomAttributes_addAndGetAttributeString();
     }
 
     ValidSchema schema_;