Avoid writing empty extensions object

CesiumGS · Nov 15, 2024 · 231f018 · 231f018
1 parent 0bc7245
commit 231f018
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 31 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -10,6 +10,7 @@
 ##### Fixes :wrench:
 
 - Updated the CMake install process to install the vcpkg-built Debug binaries in Debug builds. Previously the Release binaries were installed instead.
+- Fixed a bug where an empty `extensions` object gets written when an `ExtensibleObject` only has unregistered extensions.
 
 ### v0.41.0 - 2024-11-01
 

diff --git a/Cesium3DTilesWriter/generated/src/TilesetJsonWriter.cpp b/Cesium3DTilesWriter/generated/src/TilesetJsonWriter.cpp
@@ -268,7 +268,7 @@ void writeExtensibleObject(
     CesiumJsonWriter::JsonWriter& jsonWriter,
     const CesiumJsonWriter::ExtensionWriterContext& context) {
 
-  if (!obj.extensions.empty()) {
+  if (hasWritableExtensions(obj, jsonWriter, context)) {
     jsonWriter.Key("extensions");
     writeJsonExtensions(obj, jsonWriter, context);
   }

diff --git a/CesiumGltfWriter/generated/src/ModelJsonWriter.cpp b/CesiumGltfWriter/generated/src/ModelJsonWriter.cpp
@@ -509,7 +509,7 @@ void writeExtensibleObject(
     CesiumJsonWriter::JsonWriter& jsonWriter,
     const CesiumJsonWriter::ExtensionWriterContext& context) {
 
-  if (!obj.extensions.empty()) {
+  if (hasWritableExtensions(obj, jsonWriter, context)) {
     jsonWriter.Key("extensions");
     writeJsonExtensions(obj, jsonWriter, context);
   }

diff --git a/CesiumJsonWriter/include/CesiumJsonWriter/writeJsonExtensions.h b/CesiumJsonWriter/include/CesiumJsonWriter/writeJsonExtensions.h
@@ -20,19 +20,38 @@ void writeJsonExtensions(
         item.first,
         item.second,
         TExtended::TypeName);
-    if (!handler) {
-      if (context.getExtensionState(item.first) != ExtensionState::Disabled) {
-        jsonWriter.emplaceWarning(fmt::format(
-            "Encountered unregistered extension {}. This extension will be "
-            "ignored. To silence this warning, disable the extension with "
-            "ExtensionWriterContext::setExtensionState.",
-            item.first));
-      }
-      continue;
+    if (handler) {
+      jsonWriter.Key(item.first);
+      handler(item.second, jsonWriter, context);
     }
-    jsonWriter.Key(item.first);
-    handler(item.second, jsonWriter, context);
   }
   jsonWriter.EndObject();
 }
+
+template <typename TExtended>
+bool hasWritableExtensions(
+    const TExtended& obj,
+    JsonWriter& jsonWriter,
+    const ExtensionWriterContext& context) {
+  bool hasWritableExtensions = false;
+
+  for (const auto& item : obj.extensions) {
+    auto handler = context.createExtensionHandler(
+        item.first,
+        item.second,
+        TExtended::TypeName);
+    if (handler) {
+      hasWritableExtensions = true;
+    } else if (
+        context.getExtensionState(item.first) != ExtensionState::Disabled) {
+      jsonWriter.emplaceWarning(fmt::format(
+          "Encountered unregistered extension {}. This extension will be "
+          "ignored. To silence this warning, disable the extension with "
+          "ExtensionWriterContext::setExtensionState.",
+          item.first));
+    }
+  }
+
+  return hasWritableExtensions;
+}
 } // namespace CesiumJsonWriter
diff --git a/CesiumQuantizedMeshTerrain/generated/src/LayerJsonWriter.cpp b/CesiumQuantizedMeshTerrain/generated/src/LayerJsonWriter.cpp
@@ -130,7 +130,7 @@ void writeExtensibleObject(
     CesiumJsonWriter::JsonWriter& jsonWriter,
     const CesiumJsonWriter::ExtensionWriterContext& context) {
 
-  if (!obj.extensions.empty()) {
+  if (hasWritableExtensions(obj, jsonWriter, context)) {
     jsonWriter.Key("extensions");
     writeJsonExtensions(obj, jsonWriter, context);
   }

diff --git a/tools/generate-classes/generateCombinedWriter.js b/tools/generate-classes/generateCombinedWriter.js
@@ -36,7 +36,7 @@ function generateCombinedWriter(options) {
             .join("\n")}
         } // namespace ${namespace}
 
-        namespace ${writerNamespace} {          
+        namespace ${writerNamespace} {
           ${writers
             .map((writer) => {
               return writer.writeDeclaration;
@@ -49,9 +49,9 @@ function generateCombinedWriter(options) {
   const implementation = `
         // This file was generated by generate-classes.
         // DO NOT EDIT THIS FILE!
-        
+
         #include "${name}JsonWriter.h"
-        
+
         #include <CesiumUtility/JsonValue.h>
         #include <CesiumJsonWriter/ExtensionWriterContext.h>
         #include <CesiumJsonWriter/writeJsonExtensions.h>
@@ -63,17 +63,17 @@ function generateCombinedWriter(options) {
             return writer.writeInclude;
           })
           .join("\n")}
-        
+
         namespace ${writerNamespace} {
-        
+
         namespace {
-        
+
         ${writers
           .map((writer) => {
             return writer.writeJsonDeclaration;
           })
           .join("\n")}
-        
+
         // Forward declaration to avoid circular dependency since some properties
         // are vector of unordered_map and others are unordered_map of vector
         template <typename T>
@@ -88,21 +88,21 @@ function generateCombinedWriter(options) {
             const CesiumJsonWriter::ExtensionWriterContext& /* context */) {
           jsonWriter.String(str);
         }
-        
+
         [[maybe_unused]] void writeJson(
             double val,
             CesiumJsonWriter::JsonWriter& jsonWriter,
             const CesiumJsonWriter::ExtensionWriterContext& /* context */) {
           jsonWriter.Double(val);
         }
-        
+
         [[maybe_unused]] void writeJson(
             bool val,
             CesiumJsonWriter::JsonWriter& jsonWriter,
             const CesiumJsonWriter::ExtensionWriterContext& /* context */) {
           jsonWriter.Bool(val);
         }
-        
+
         [[maybe_unused]] void writeJson(
             int64_t val,
             CesiumJsonWriter::JsonWriter& jsonWriter,
@@ -116,7 +116,7 @@ function generateCombinedWriter(options) {
             const CesiumJsonWriter::ExtensionWriterContext& /* context */) {
           jsonWriter.Int64(val);
         }
-        
+
         [[maybe_unused]] void writeJson(
             const CesiumUtility::JsonValue::Object& obj,
             CesiumJsonWriter::JsonWriter& jsonWriter,
@@ -128,13 +128,13 @@ function generateCombinedWriter(options) {
           }
           jsonWriter.EndObject();
         }
-        
+
         [[maybe_unused]] void writeJson(
             const CesiumUtility::JsonValue& value,
             CesiumJsonWriter::JsonWriter& jsonWriter,
             const CesiumJsonWriter::ExtensionWriterContext& /* context */) {
             CesiumJsonWriter::writeJsonValue(value, jsonWriter);
-        }      
+        }
 
         template <typename T>
         [[maybe_unused]] void writeJson(
@@ -160,7 +160,7 @@ function generateCombinedWriter(options) {
           }
           jsonWriter.EndArray();
         }
-        
+
         template <typename T>
         [[maybe_unused]] void writeJson(
             const std::optional<T>& val,
@@ -179,7 +179,7 @@ function generateCombinedWriter(options) {
             CesiumJsonWriter::JsonWriter& jsonWriter,
             const CesiumJsonWriter::ExtensionWriterContext& context) {
 
-          if (!obj.extensions.empty()) {
+          if (hasWritableExtensions(obj, jsonWriter, context)) {
             jsonWriter.Key("extensions");
             writeJsonExtensions(obj, jsonWriter, context);
           }
@@ -203,7 +203,7 @@ function generateCombinedWriter(options) {
 
           writeExtensibleObject(obj, jsonWriter, context);
         }
-        
+
         ${writers
           .map((writer) => {
             return writer.writeBaseJsonDefinition ? writer.writeBaseJsonDefinition : "";
@@ -223,7 +223,7 @@ function generateCombinedWriter(options) {
             return writer.writeDefinition;
           })
           .join("\n")}
-        
+
         } // namespace ${writerNamespace}
   `;