(MINOR) Refactor how property values are returned from accessor funct…

…ions (#81) Introduces `XmpValue` as a templated type with accessors for the various flags that the C++ XMP Toolkit can return. `XmpMeta::property` now returns `Option<XmpValue<String>>` which allows it to pass through the various "option" flags that C++ XMP Toolkit provides.
adobe · Oct 12, 2022 · 0634185 · 0634185
1 parent 4f3f10e
commit 0634185
Show file tree

Hide file tree

Showing 10 changed files with 452 additions and 54 deletions.
diff --git a/README.md b/README.md
@@ -53,7 +53,13 @@ xmp_toolkit = "0.5.3"
 
 ### Upgrading to 0.6 from earlier versions
 
-This version increases the minimum supported Rust version (MSRV) to 1.56.0.
+The `XmpMeta::property` value has been changed to return `Option<XmpValue<String>>`
+instead of `Option<String>`. You may need to add a `.value` dereference to get the
+string value from existing calls to the `property` accessor. The XMP value flags
+(known as `XMP_OptionBits` in the C++ XMP Toolkit) are now available via accessors
+on the new `XmpValue` struct.
+
+This version also increases the minimum supported Rust version (MSRV) to 1.56.0.
 
 ### Upgrading to 0.5 from earlier releases
 

diff --git a/src/ffi.cpp b/src/ffi.cpp
@@ -326,17 +326,24 @@ extern "C" {
     }
 
     const char* CXmpMetaGetProperty(CXmpMeta* m,
+                                    CXmpError* outError,
                                     const char* schemaNS,
-                                    const char* propName) {
+                                    const char* propName,
+                                    AdobeXMPCommon::uint32* outOptions) {
+        *outOptions = 0;
+
         #ifndef NOOP_FFI
             try {
                 std::string propValue;
-                if (m->m.GetProperty(schemaNS, propName, &propValue, NULL /* options */)) {
+                if (m->m.GetProperty(schemaNS, propName, &propValue, outOptions)) {
                     return copyStringForResult(propValue);
                 }
             }
+            catch (XMP_Error& e) {
+                copyErrorForResult(e, outError);
+            }
             catch (...) {
-                // Intentional no-op.
+                signalUnknownError(outError);
             }
         #endif
 

diff --git a/src/ffi.rs b/src/ffi.rs
@@ -77,8 +77,10 @@ extern "C" {
 
     pub(crate) fn CXmpMetaGetProperty(
         meta: *mut CXmpMeta,
+        out_error: *mut CXmpError,
         schema_ns: *const c_char,
         prop_name: *const c_char,
+        out_options: *mut u32,
     ) -> *mut c_char;
 
     pub(crate) fn CXmpMetaSetProperty(

diff --git a/src/lib.rs b/src/lib.rs
@@ -24,11 +24,13 @@ mod xmp_error;
 mod xmp_file;
 mod xmp_meta;
 pub mod xmp_ns;
+mod xmp_value;
 
 pub use xmp_date_time::XmpDateTime;
 pub use xmp_error::{XmpError, XmpErrorType, XmpResult};
 pub use xmp_file::{OpenFileOptions, XmpFile};
-pub use xmp_meta::{ArrayProperty, XmpMeta, XmpValue};
+pub use xmp_meta::{ArrayProperty, XmpMeta};
+pub use xmp_value::XmpValue;
 
 #[cfg(test)]
 mod tests;
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
@@ -24,3 +24,4 @@ mod xmp_error;
 mod xmp_error_type;
 mod xmp_file;
 mod xmp_meta;
+mod xmp_value;
diff --git a/src/tests/xmp_file.rs b/src/tests/xmp_file.rs
@@ -13,7 +13,7 @@
 
 use tempfile::tempdir;
 
-use crate::{tests::fixtures::*, xmp_ns, OpenFileOptions, XmpDateTime, XmpFile, XmpMeta};
+use crate::{tests::fixtures::*, xmp_ns, OpenFileOptions, XmpDateTime, XmpFile, XmpMeta, XmpValue};
 
 #[test]
 fn open_and_edit_file() {
@@ -67,7 +67,10 @@ fn open_and_edit_file() {
         assert_eq!(
             m.property("http://purl.org/dc/terms/", "provenance")
                 .unwrap(),
-            "blah"
+            XmpValue {
+                value: "blah".to_owned(),
+                options: 0
+            }
         );
         assert_eq!(m.property("http://purl.org/dc/terms/", "provenancx"), None);
     }

diff --git a/src/tests/xmp_meta.rs b/src/tests/xmp_meta.rs
@@ -23,7 +23,7 @@ fn new_empty() {
 mod from_file {
     use std::path::PathBuf;
 
-    use crate::{tests::fixtures::*, XmpErrorType, XmpMeta};
+    use crate::{tests::fixtures::*, XmpErrorType, XmpMeta, XmpValue};
 
     #[test]
     fn happy_path() {
@@ -32,13 +32,19 @@ mod from_file {
         assert_eq!(
             m.property("http://ns.adobe.com/xap/1.0/", "CreatorTool")
                 .unwrap(),
-            "Adobe Photoshop CS2 Windows"
+            XmpValue {
+                value: "Adobe Photoshop CS2 Windows".to_owned(),
+                options: 0
+            }
         );
 
         assert_eq!(
             m.property("http://ns.adobe.com/photoshop/1.0/", "ICCProfile")
                 .unwrap(),
-            "Dell 1905FP Color Profile"
+            XmpValue {
+                value: "Dell 1905FP Color Profile".to_owned(),
+                options: 0
+            }
         );
 
         assert!(m
@@ -70,7 +76,7 @@ mod from_file {
 mod from_str {
     use std::str::FromStr;
 
-    use crate::{tests::fixtures::*, XmpMeta};
+    use crate::{tests::fixtures::*, XmpMeta, XmpValue};
 
     #[test]
     fn happy_path() {
@@ -79,13 +85,19 @@ mod from_str {
         assert_eq!(
             m.property("http://ns.adobe.com/xap/1.0/", "CreatorTool")
                 .unwrap(),
-            "Adobe Photoshop CS2 Windows"
+            XmpValue {
+                value: "Adobe Photoshop CS2 Windows".to_owned(),
+                options: 0
+            }
         );
 
         assert_eq!(
             m.property("http://ns.adobe.com/photoshop/1.0/", "ICCProfile")
                 .unwrap(),
-            "Dell 1905FP Color Profile"
+            XmpValue {
+                value: "Dell 1905FP Color Profile".to_owned(),
+                options: 0
+            }
         );
 
         assert!(m
@@ -155,14 +167,17 @@ mod register_namespace {
 }
 
 mod property {
-    use crate::{tests::fixtures::*, xmp_ns, XmpMeta};
+    use crate::{tests::fixtures::*, xmp_ns, XmpMeta, XmpValue};
 
     #[test]
     fn happy_path() {
         let m = XmpMeta::from_file(fixture_path("Purple Square.psd")).unwrap();
         assert_eq!(
             m.property(xmp_ns::XMP, "CreatorTool"),
-            Some("Adobe Photoshop CS2 Windows".to_owned())
+            Some(XmpValue {
+                value: "Adobe Photoshop CS2 Windows".to_owned(),
+                options: 0
+            })
         );
     }
 
@@ -192,7 +207,7 @@ mod property {
 }
 
 mod set_property {
-    use crate::{tests::fixtures::*, XmpErrorType, XmpMeta};
+    use crate::{tests::fixtures::*, XmpErrorType, XmpMeta, XmpValue};
 
     #[test]
     fn happy_path() {
@@ -206,7 +221,10 @@ mod set_property {
         assert_eq!(
             m.property("http://purl.org/dc/terms/", "provenance")
                 .unwrap(),
-            "blah"
+            XmpValue {
+                value: "blah".to_owned(),
+                options: 0
+            }
         );
     }
 
@@ -323,7 +341,7 @@ mod array_property {
     fn happy_path_creator_seq() {
         let m = XmpMeta::from_str(PURPLE_SQUARE_XMP).unwrap();
 
-        let mut creators: Vec<XmpValue> = m
+        let mut creators: Vec<XmpValue<String>> = m
             .array_property("http://purl.org/dc/elements/1.1/", "creator")
             .collect();
 
@@ -363,13 +381,3 @@ mod array_property {
         assert!(first_creator.is_none());
     }
 }
-
-mod xmp_options {
-    use crate::xmp_meta::XmpOptions;
-
-    #[test]
-    fn default() {
-        let o = XmpOptions::default();
-        assert_eq!(o.options, 0);
-    }
-}