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::ValueMode::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::ValueMode::JSON);
     getCustomAttributes(m, customAttributes);
     node->addCustomAttributesForField(customAttributes);
     return node;

lang/c++/impl/CustomAttributes.cc

@@ -16,26 +16,63 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "CustomAttributes.hh"
-#include "Exception.hh"
+
+#if defined(__clang__)
+// Even though CustomAttributes::ValueMode::STRING is deprecated, we still have to
+// handle/implement it.
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+#endif
+
 #include <map>
 #include <memory>
 
+#include "CustomAttributes.hh"
+#include "Exception.hh"
+
+#include "json/JsonDom.hh"
+
 namespace avro {
 
+CustomAttributes::CustomAttributes(ValueMode valueMode) {
+    switch (valueMode) {
+    case ValueMode::STRING:
+    case ValueMode::JSON:
+        valueMode_ = valueMode;
+        break;
+    default:
+        throw Exception("invalid ValueMode: " + std::to_string(static_cast<int>(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 (intentionally), so CustomAttributes.hh cannot
+    // #include any of the json header files.
+    const std::string &jsonVal = (valueMode_ == ValueMode::STRING)
+                ? std::move("\"" + value + "\"")
+                : value;
+    try {
+        json::loadEntity(jsonVal.c_str());
+    } catch (json::TooManyValuesException &e) {
+        throw Exception("string has malformed or missing escapes");
+    }
+
     auto iter_and_find =
         attributes_.insert(std::pair<std::string, std::string>(name, value));
     if (!iter_and_find.second) {
@@ -45,9 +82,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_ == ValueMode::STRING) {
+        os << "\"" << iter->second << "\"";
+    } else {
+        os << iter->second;
+    }
 }
 } // namespace avro

lang/c++/impl/Schema.cc

@@ -28,7 +28,7 @@ RecordSchema::RecordSchema(const std::string &name) : Schema(new NodeRecord) {
 }
 
 void RecordSchema::addField(const std::string &name, const Schema &fieldSchema) {
-    const CustomAttributes emptyCustomAttribute;
+    const CustomAttributes emptyCustomAttribute(CustomAttributes::ValueMode::JSON);
     addField(name, fieldSchema, emptyCustomAttribute);
 }
 

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,62 @@
 #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 class ValueMode : uint8_t {
+        // When a CustomAttributes is created using this mode, all values are expected
+        // to be strings. The value should not be quoted, but any interior quotes and
+        // special characters (such as newlines) must be escaped.
+        STRING
+        [[deprecated("The JSON ValueMode is less error-prone and less limited.")]],
+
+        // When a CustomAttributes is created using this mode, all values are formatted
+        // JSON values. So 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, one must instead use
+    // CustomAttributes(CustomAttributes::ValueMode::JSON)
+    [[deprecated("Use CustomAttributes(ValueMode) instead.")]]
+    CustomAttributes() : CustomAttributes(ValueMode::STRING) {}
+
+    // Creates a new CustomAttributes object with the given value mode.
+    CustomAttributes(ValueMode valueMode);
+
+    // Retrieves the custom attribute string for the given name. Returns an empty
+    // value if the attribute doesn't exist.
+    //
+    // See ValueMode for details on the format of the returned value.
     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.
+    //
+    // See ValueMode for details on the require format of the value parameter.
+    //
+    // 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.
     const std::map<std::string, std::string> &attributes() const {
         return attributes_;
     }
@@ -48,6 +83,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);