From 063418596d926e2334cde15a8b9ed0de6a4d33ea Mon Sep 17 00:00:00 2001 From: Eric Scouten Date: Wed, 12 Oct 2022 14:55:16 -0500 Subject: [PATCH] (MINOR) Refactor how property values are returned from accessor functions (#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>` which allows it to pass through the various "option" flags that C++ XMP Toolkit provides. --- README.md | 8 +- src/ffi.cpp | 13 ++- src/ffi.rs | 2 + src/lib.rs | 4 +- src/tests/mod.rs | 1 + src/tests/xmp_file.rs | 7 +- src/tests/xmp_meta.rs | 50 +++++----- src/tests/xmp_value.rs | 206 +++++++++++++++++++++++++++++++++++++++++ src/xmp_meta.rs | 46 ++++----- src/xmp_value.rs | 169 +++++++++++++++++++++++++++++++++ 10 files changed, 452 insertions(+), 54 deletions(-) create mode 100644 src/tests/xmp_value.rs create mode 100644 src/xmp_value.rs diff --git a/README.md b/README.md index a9b538b..0ccf335 100644 --- 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>` +instead of `Option`. 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 index 64f61b6..2f78ff2 100644 --- 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 index 6630909..6e5069a 100644 --- 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 index 6a2aa52..f464dea 100644 --- 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 index 689862d..f95d880 100644 --- 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 index 41052e7..86ca2c7 100644 --- 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 index ccd7850..d2fab93 100644 --- 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 = m + let mut creators: Vec> = 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); - } -} diff --git a/src/tests/xmp_value.rs b/src/tests/xmp_value.rs new file mode 100644 index 0000000..8f04dbc --- /dev/null +++ b/src/tests/xmp_value.rs @@ -0,0 +1,206 @@ +// Copyright 2020 Adobe. All rights reserved. +// This file is licensed to you under the Apache License, +// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) +// or the MIT license (http://opensource.org/licenses/MIT), +// at your option. + +// Unless required by applicable law or agreed to in writing, +// this software is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or +// implied. See the LICENSE-MIT and LICENSE-APACHE files for the +// specific language governing permissions and limitations under +// each license. + +/// Test mapping of C++ XMP Toolkit "options" value to `XmpValue`. +mod options { + use crate::{xmp_value::xmp_prop, XmpValue}; + + #[test] + fn default() { + let v = XmpValue::::default(); + assert_eq!(&v.value, ""); + assert_eq!(v.options, 0); + + assert!(v.has_no_flags()); + assert!(!v.is_uri()); + assert!(!v.has_qualifiers()); + assert!(!v.is_qualifier()); + assert!(!v.has_lang()); + assert!(!v.has_type()); + assert!(!v.is_struct()); + assert!(!v.is_array()); + assert!(!v.is_ordered()); + assert!(!v.is_alternate()); + assert!(!v.is_alt_text()); + assert!(!v.is_alias()); + assert!(!v.has_aliases()); + assert!(!v.is_internal()); + assert!(!v.is_stable()); + assert!(!v.is_derived()); + } + + #[test] + fn is_uri() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::VALUE_IS_URI, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_uri()); + } + + #[test] + fn has_qualifiers() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::HAS_QUALIFIERS, + }; + + assert!(!v.has_no_flags()); + assert!(v.has_qualifiers()); + } + + #[test] + fn is_qualifier() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::IS_QUALIFIER, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_qualifier()); + } + + #[test] + fn has_lang() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::HAS_LANG, + }; + + assert!(!v.has_no_flags()); + assert!(v.has_lang()); + } + + #[test] + fn has_type() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::HAS_TYPE, + }; + + assert!(!v.has_no_flags()); + assert!(v.has_type()); + } + + #[test] + fn is_struct() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::VALUE_IS_STRUCT, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_struct()); + } + + #[test] + fn is_array() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::VALUE_IS_ARRAY, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_array()); + } + + #[test] + fn is_ordered() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::ARRAY_IS_ORDERED, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_ordered()); + } + + #[test] + fn is_alternate() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::ARRAY_IS_ALTERNATE, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_alternate()); + } + + #[test] + fn is_alt_text() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::ARRAY_IS_ALT_TEXT, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_alt_text()); + } + + #[test] + fn is_alias() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::IS_ALIAS, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_alias()); + } + + #[test] + fn has_aliases() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::HAS_ALIASES, + }; + + assert!(!v.has_no_flags()); + assert!(v.has_aliases()); + } + + #[test] + fn is_internal() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::IS_INTERNAL, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_internal()); + } + + #[test] + fn is_stable() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::IS_STABLE, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_stable()); + } + + #[test] + fn is_derived() { + let v = XmpValue { + value: "".to_owned(), + options: xmp_prop::IS_DERIVED, + }; + + assert!(!v.has_no_flags()); + assert!(v.is_derived()); + } +} diff --git a/src/xmp_meta.rs b/src/xmp_meta.rs index 4903bc4..7eb868e 100644 --- a/src/xmp_meta.rs +++ b/src/xmp_meta.rs @@ -17,7 +17,9 @@ use std::{ str::FromStr, }; -use crate::{ffi, OpenFileOptions, XmpDateTime, XmpError, XmpErrorType, XmpFile, XmpResult}; +use crate::{ + ffi, OpenFileOptions, XmpDateTime, XmpError, XmpErrorType, XmpFile, XmpResult, XmpValue, +}; /// The `XmpMeta` struct allows access to the XMP Toolkit core services. /// @@ -128,17 +130,29 @@ impl XmpMeta { /// /// Any errors (for instance, empty or invalid namespace or property name) /// are ignored; the function will return `None` in such cases. - pub fn property(&self, schema_ns: &str, prop_name: &str) -> Option { + pub fn property(&self, schema_ns: &str, prop_name: &str) -> Option> { let c_ns = CString::new(schema_ns).unwrap_or_default(); let c_name = CString::new(prop_name).unwrap_or_default(); + let mut options: u32 = 0; + let mut err = ffi::CXmpError::default(); + unsafe { - let c_result = ffi::CXmpMetaGetProperty(self.m, c_ns.as_ptr(), c_name.as_ptr()); + let c_result = ffi::CXmpMetaGetProperty( + self.m, + &mut err, + c_ns.as_ptr(), + c_name.as_ptr(), + &mut options, + ); if c_result.is_null() { None } else { - Some(CStr::from_ptr(c_result).to_string_lossy().into_owned()) + Some(XmpValue { + value: CStr::from_ptr(c_result).to_string_lossy().into_owned(), + options, + }) } } } @@ -280,26 +294,6 @@ impl FromStr for XmpMeta { } } -/// An XMP value consists describes a simple property or an item in an -/// array property. -#[non_exhaustive] -pub struct XmpValue { - /// String value for this item. - pub value: String, - - /// Flags that further describe this item. (NOT YET IMPLEMENTED) - pub options: XmpOptions, -} - -/// Flags that provide additional description for an [`XmpValue`]. -/// -/// Not currently implemented. -#[derive(Default)] -pub struct XmpOptions { - #[allow(dead_code)] // TEMPORARY until we provide accessors for this - pub(crate) options: u32, -} - /// An iterator that provides access to items within a property array. /// /// Create via [`XmpMeta::array_property`]. @@ -311,7 +305,7 @@ pub struct ArrayProperty<'a> { } impl<'a> Iterator for ArrayProperty<'a> { - type Item = XmpValue; + type Item = XmpValue; fn next(&mut self) -> Option { unsafe { @@ -334,7 +328,7 @@ impl<'a> Iterator for ArrayProperty<'a> { } else { Some(XmpValue { value: CStr::from_ptr(c_result).to_string_lossy().into_owned(), - options: XmpOptions { options }, + options, }) } } diff --git a/src/xmp_value.rs b/src/xmp_value.rs new file mode 100644 index 0000000..fec95ee --- /dev/null +++ b/src/xmp_value.rs @@ -0,0 +1,169 @@ +// Copyright 2022 Adobe. All rights reserved. +// This file is licensed to you under the Apache License, +// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) +// or the MIT license (http://opensource.org/licenses/MIT), +// at your option. + +// Unless required by applicable law or agreed to in writing, +// this software is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or +// implied. See the LICENSE-MIT and LICENSE-APACHE files for the +// specific language governing permissions and limitations under +// each license. + +use std::fmt::Debug; + +/// An XMP value consists describes a simple property or an item in an +/// array property. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct XmpValue { + /// Core value for this item (typically a `String` or scalar value). + pub value: T, + + /// Raw bitflags that further describe this type. + pub(crate) options: u32, +} + +/// XMP_PROP_* constant values copied/renamed from XMP_Const.h. +pub(crate) mod xmp_prop { + pub(crate) const VALUE_IS_URI: u32 = 0x00000002; + pub(crate) const HAS_QUALIFIERS: u32 = 0x00000010; + pub(crate) const IS_QUALIFIER: u32 = 0x00000020; + pub(crate) const HAS_LANG: u32 = 0x00000040; + pub(crate) const HAS_TYPE: u32 = 0x00000080; + pub(crate) const VALUE_IS_STRUCT: u32 = 0x00000100; + pub(crate) const VALUE_IS_ARRAY: u32 = 0x00000200; + pub(crate) const ARRAY_IS_ORDERED: u32 = 0x00000400; + pub(crate) const ARRAY_IS_ALTERNATE: u32 = 0x00000800; + pub(crate) const ARRAY_IS_ALT_TEXT: u32 = 0x00001000; + pub(crate) const IS_ALIAS: u32 = 0x00010000; + pub(crate) const HAS_ALIASES: u32 = 0x00020000; + pub(crate) const IS_INTERNAL: u32 = 0x00040000; + pub(crate) const IS_STABLE: u32 = 0x00100000; + pub(crate) const IS_DERIVED: u32 = 0x00200000; +} + +impl XmpValue { + /// Returns `true` if none of the other `is...` or `has...` flags + /// for this value are true. + pub fn has_no_flags(&self) -> bool { + self.options == 0 + } + + /// Returns `true` if the XML string form of this property value is a URI, + /// meaning it uses the `rdf:resource` attribute. + /// + /// This is flagged as "discouraged" in the C++ XMP Toolkit API + /// documentation. + pub fn is_uri(&self) -> bool { + self.options & xmp_prop::VALUE_IS_URI != 0 + } + + // --- options relating to qualifiers attached to a property --- + + /// Returns `true` if the property has qualifiers, such as `rdf:type` + /// `xml:lang`. + pub fn has_qualifiers(&self) -> bool { + self.options & xmp_prop::HAS_QUALIFIERS != 0 + } + + /// Returns `true` if this property is a qualifier for some other property, + /// such as `rdf:type` and `xml:lang`. + /// + /// Qualifiers can have arbitrary structure, and can themselves have + /// qualifiers. If the qualifier itself has a structured value, this + /// flag is only set for the top node of the qualifier's subtree. + pub fn is_qualifier(&self) -> bool { + self.options & xmp_prop::IS_QUALIFIER != 0 + } + + /// Returns `true` if this property has an `xml:lang` qualifier. + /// + /// Implies `has_qualifiers` will also be true. + pub fn has_lang(&self) -> bool { + self.options & xmp_prop::HAS_LANG != 0 + } + + /// Returns `true` if this property has an `rdf:type` qualifier. + /// + /// Implies `has_qualifiers` will also be true. + pub fn has_type(&self) -> bool { + self.options & xmp_prop::HAS_TYPE != 0 + } + + // --- options relating to the data structure form --- + + /// Returns `true` if this value is a structure with nested fields. + pub fn is_struct(&self) -> bool { + self.options & xmp_prop::VALUE_IS_STRUCT != 0 + } + + /// Returns `true` if this value is an array (RDF alt/bag/seq). + /// + /// This may mean the array is ordered or unordered. Use the `is_ordered` + /// query to discern between the two. + pub fn is_array(&self) -> bool { + self.options & xmp_prop::VALUE_IS_ARRAY != 0 + } + + /// Returns `true` if the item order matters. In other words, this + /// array has been serialized using an `rdf:Seq` container. + /// + /// Implies that `is_array` is also `true`. + pub fn is_ordered(&self) -> bool { + self.options & xmp_prop::ARRAY_IS_ORDERED != 0 + } + + /// Returns `true` if the items in this array are alternates. In other + /// words, this array has been serialized using an `rdf:Alt` container. + /// + /// Implies that `is_array` and `is_ordered` are also `true`. + pub fn is_alternate(&self) -> bool { + self.options & xmp_prop::ARRAY_IS_ALTERNATE != 0 + } + + /// Returns `true` if items are localized text. Each array element will be + /// a simple property with an `xml:lang` attribute. + /// + /// Implies `is_alternate` is also true. + pub fn is_alt_text(&self) -> bool { + self.options & xmp_prop::ARRAY_IS_ALT_TEXT != 0 + } + + // -- other miscellaneous options -- + + /// Returns `true` if this property is an alias name for another property. + /// + /// This is only returned by [`XmpMeta::property`](crate::XmpMeta::property) + /// and then only if the property name is simple, not a path expression. + pub fn is_alias(&self) -> bool { + self.options & xmp_prop::IS_ALIAS != 0 + } + + /// Returns `true` if this property is the base value (actual) for a set + /// of aliases. + /// + /// This is only returned by [`XmpMeta::property`](crate::XmpMeta::property) + /// and then only if the property name is simple, not a path expression. + pub fn has_aliases(&self) -> bool { + self.options & xmp_prop::HAS_ALIASES != 0 + } + + /// Returns `true` if this property is "owned" by the application, + /// and should not generally be editable in a UI. + pub fn is_internal(&self) -> bool { + self.options & xmp_prop::IS_INTERNAL != 0 + } + + /// Returns `true` if the value of this property is not derived from + /// the document content. + pub fn is_stable(&self) -> bool { + self.options & xmp_prop::IS_STABLE != 0 + } + + /// Returns `true` if the value of this property is derived from the + /// document content. + pub fn is_derived(&self) -> bool { + self.options & xmp_prop::IS_DERIVED != 0 + } +}