Fix #135: XmpMeta::from_str_with_options returns empty data model i…

…f no `xmp_meta` wrapper exists
adobe · Apr 10, 2023 · f3d45bd · f3d45bd
1 parent ae81da3
commit f3d45bd
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 24 deletions.
diff --git a/src/tests/xmp_core_coverage.rs b/src/tests/xmp_core_coverage.rs
@@ -1098,21 +1098,19 @@ fn xmp_core_coverage() {
     write_major_label("Test parsing with multiple buffers and various options");
 
     {
-        // TODO (https://github.com/adobe/xmp-toolkit-rs/issues/135):
-        // I think this should be an error response, not a silent
-        // Ok(default) response.
-        let meta = XmpMeta::from_str_with_options(
+        let err = XmpMeta::from_str_with_options(
             SIMPLE_RDF,
             FromStrOptions::default().require_xmp_meta(),
         )
-        .unwrap();
+        .unwrap_err();
 
-        println!(
-            "Parse and require xmpmeta element, which is missing = {:#?}",
-            meta
+        assert_eq!(
+            err,
+            XmpError {
+                error_type: XmpErrorType::XmpMetaElementMissing,
+                debug_message: "x:xmpmeta element not found".to_owned()
+            }
         );
-
-        assert_eq!(meta.to_string(), "<x:xmpmeta xmlns:x=\"adobe:ns:meta/\" x:xmptk=\"XMP Core 6.0.0\"> <rdf:RDF xmlns:rdf=\"http://www.w3.org/1999/02/22-rdf-syntax-ns#\"> <rdf:Description rdf:about=\"\"/> </rdf:RDF> </x:xmpmeta>");
     }
 
     {

diff --git a/src/tests/xmp_meta.rs b/src/tests/xmp_meta.rs
@@ -247,20 +247,17 @@ mod from_str_with_options {
 
     #[test]
     fn missing_xmp_meta_required() {
-        // TODO (https://github.com/adobe/xmp-toolkit-rs/issues/135):
-        // I think this should be an error response, not a silent
-        // Ok(default) response.
-        assert!(XmpMeta::from_str_with_options(
-            NO_META,
-            FromStrOptions::default().require_xmp_meta()
-        )
-        .is_ok());
+        let err =
+            XmpMeta::from_str_with_options(NO_META, FromStrOptions::default().require_xmp_meta())
+                .unwrap_err();
 
-        // Should be:
-        // XmpError {
-        //     error_type: XmpErrorType::BadSerialize,
-        //     debug_message: "x".to_owned()
-        // }
+        assert_eq!(
+            err,
+            XmpError {
+                error_type: XmpErrorType::XmpMetaElementMissing,
+                debug_message: "x:xmpmeta element not found".to_owned()
+            }
+        );
     }
 
     #[test]

diff --git a/src/xmp_error.rs b/src/xmp_error.rs
@@ -289,6 +289,17 @@ pub enum XmpErrorType {
     /// C++ toolkit did not initialize properly.
     #[error("C++ XMP toolkit did not initialize properly")]
     NoCppToolkit = -433,
+
+    /// An `x:xmpmeta` wrapper was required, but not found.
+    ///
+    /// This error can only occur if you call
+    /// [`XmpMeta::from_str_with_options()`]
+    /// with [`FromStrOptions::require_xmp_meta()`].
+    ///
+    /// [`XmpMeta::from_str_with_options()`]: crate::XmpMeta::from_str_with_options
+    /// [`FromStrOptions::require_xmp_meta()`]: crate::FromStrOptions::require_xmp_meta
+    #[error("x:xmpmeta element not found")]
+    XmpMetaElementMissing = -434,
 }
 
 /// A specialized `Result` type for XMP Toolkit operations.

diff --git a/src/xmp_meta.rs b/src/xmp_meta.rs
@@ -1882,7 +1882,26 @@ impl XmpMeta {
 
         XmpError::raise_from_c(&err)?;
 
-        Ok(XmpMeta { m: Some(m) })
+        let result = XmpMeta { m: Some(m) };
+
+        if options.options & 0x01 != 0 {
+            // Caller has asked that we require an `<x:xmpmeta>` element
+            // when parsing this XMP payload. If no such element is found,
+            // the C++ XMP Toolkit will "succeed" and return an `SXMPMeta`
+            // object with no content. In Rust, we translate that to
+            // an error condition signaling that the `<x:xmpmeta>` element
+            // was missing.
+
+            let mut prop_iter = result.iter(IterOptions::default());
+            if prop_iter.next().is_none() {
+                return Err(XmpError {
+                    error_type: XmpErrorType::XmpMetaElementMissing,
+                    debug_message: "x:xmpmeta element not found".to_owned(),
+                });
+            }
+        }
+
+        Ok(result)
     }
 
     /// Converts metadata in this XMP object into a string as RDF.