Added metadata key/value iteration (#28)

This commit adds the ability to get and iterate over metadata for Pact messages.

Author: alilleybrinker

rust/pact_matching_ffi/src/models/message.rs

@@ -9,6 +9,7 @@ use crate::util::*;
 use crate::{as_mut, as_ref, cstr, ffi, safe_str};
 use anyhow::{anyhow, Context};
 use libc::{c_char, c_int, c_uint, EXIT_FAILURE, EXIT_SUCCESS};
+use std::ops::Drop;
 
 /*===============================================================================================
  * # Re-Exports
@@ -110,8 +111,7 @@ pub unsafe extern "C" fn message_get_description(
         params: [message],
         op: {
             let message = as_ref!(message);
-            let description = string::into_leaked_cstring(&message.description)?;
-            Ok(description)
+            Ok(string::to_c(&message.description)? as *const c_char)
         },
         fail: {
             ptr::null_to::<c_char>()
@@ -222,15 +222,14 @@ pub unsafe extern "C" fn message_find_metadata(
         name: "message_find_metadata",
         params: [message, key],
         op: {
+            // Reconstitute the message.
             let message = as_ref!(message);
+            // Safely get a Rust String out of the key.
             let key = safe_str!(key);
-
-            match message.metadata.get(key) {
-                None => Ok(ptr::null_to::<c_char>()),
-                Some(value) => {
-                    Ok(string::into_leaked_cstring(value)?)
-                },
-            }
+            // Get the value, if present, for that key.
+            let value = message.metadata.get(key).ok_or(anyhow::anyhow!("invalid metadata key"))?;
+            // Leak the string to the C-side.
+            Ok(string::to_c(value)? as *const c_char)
         },
         fail: {
             ptr::null_to::<c_char>()
@@ -276,14 +275,13 @@ pub unsafe extern "C" fn message_insert_metadata(
     }
 }
 
-/*
-/// Get a copy of the metadata list from this message.
-/// It is in the form of a list of (key, value) pairs,
-/// in an unspecified order.
-/// The returned structure must be deleted with `metadata_list_delete`.
+/// Get an iterator over the metadata of a message.
 ///
-/// Since it is a copy, the returned structure may safely outlive
-/// the `Message`.
+/// This iterator carries a pointer to the message, and must
+/// not outlive the message.
+///
+/// The message metadata also must not be modified during iteration. If it is,
+/// the old iterator must be deleted and a new iterator created.
 ///
 /// # Errors
 ///
@@ -294,23 +292,165 @@ pub unsafe extern "C" fn message_insert_metadata(
 #[no_mangle]
 #[allow(clippy::missing_safety_doc)]
 #[allow(clippy::or_fun_call)]
-pub unsafe extern "C" fn message_get_metadata_list(
+pub unsafe extern "C" fn message_get_metadata_iter(
     message: *mut Message,
 ) -> *mut MetadataIterator {
     ffi! {
-        name: "message_get_metadata_list",
+        name: "message_get_metadata_iter",
         params: [message],
         op: {
             let message = as_mut!(message);
 
-            todo!()
+            let iter = MetadataIterator {
+                keys:  message.metadata.keys().cloned().collect(),
+                current: 0,
+                message: message as *const Message,
+            };
+
+            Ok(ptr::raw_to(iter))
+        },
+        fail: {
+            ptr::null_mut_to::<MetadataIterator>()
+        }
+    }
+}
+
+/// Get the next key and value out of the iterator, if possible
+///
+/// Returns a pointer to a heap allocated array of 2 elements, the pointer to the
+/// key string on the heap, and the pointer to the value string on the heap.
+///
+/// The user needs to free both the contained strings and the array.
+#[no_mangle]
+#[allow(clippy::missing_safety_doc)]
+#[allow(clippy::or_fun_call)]
+pub unsafe extern "C" fn metadata_iter_next(
+    iter: *mut MetadataIterator,
+) -> *mut MetadataPair {
+    ffi! {
+        name: "metadata_iter_next",
+        params: [iter],
+        op: {
+            // Reconstitute the iterator.
+            let iter = as_mut!(iter);
+
+            // Reconstitute the message.
+            let message = as_ref!(iter.message);
+
+            // Get the current key from the iterator.
+            let key = iter.next().ok_or(anyhow::anyhow!("iter past the end of metadata"))?;
+
+            // Get the value for the current key.
+            let (key, value) = message.metadata.get_key_value(key).ok_or(anyhow::anyhow!("iter provided invalid metadata key"))?;
+
+            // Package up for return.
+            let pair = MetadataPair::new(key, value)?;
+
+            // Leak the value out to the C-side.
+            Ok(ptr::raw_to(pair))
         },
         fail: {
-            ptr::null_to::<MetadataIterator>()
+            ptr::null_mut_to::<MetadataPair>()
         }
     }
 }
-*/
+
+/// Free the metadata iterator when you're done using it.
+#[no_mangle]
+#[allow(clippy::missing_safety_doc)]
+pub unsafe extern "C" fn metadata_iter_delete(
+    iter: *mut MetadataIterator,
+) -> c_int {
+    ffi! {
+        name: "metadata_iter_delete",
+        params: [iter],
+        op: {
+            ptr::drop_raw(iter);
+            Ok(EXIT_SUCCESS)
+        },
+        fail: {
+            EXIT_FAILURE
+        }
+    }
+}
+
+/// Free a pair of key and value returned from `message_next_metadata_iter`.
+#[no_mangle]
+#[allow(clippy::missing_safety_doc)]
+pub unsafe extern "C" fn metadata_pair_delete(
+    pair: *mut MetadataPair,
+) -> c_int {
+    ffi! {
+        name: "metadata_pair_delete",
+        params: [pair],
+        op: {
+            ptr::drop_raw(pair);
+            Ok(EXIT_SUCCESS)
+        },
+        fail: {
+            EXIT_FAILURE
+        }
+    }
+}
+
+/// An iterator that enables FFI iteration over metadata by putting all the keys on the heap
+/// and tracking which one we're currently at.
+///
+/// This assumes no mutation of the underlying metadata happens while the iterator is live.
+#[derive(Debug)]
+pub struct MetadataIterator {
+    /// The metadata keys
+    keys: Vec<String>,
+    /// The current key
+    current: usize,
+    /// Pointer to the message.
+    message: *const Message,
+}
+
+impl MetadataIterator {
+    fn next(&mut self) -> Option<&String> {
+        let idx = self.current;
+        self.current += 1;
+        self.keys.get(idx)
+    }
+}
+
+/// A single key-value pair exported to the C-side.
+#[derive(Debug)]
+#[repr(C)]
+#[allow(missing_copy_implementations)]
+pub struct MetadataPair {
+    key: *const c_char,
+    value: *const c_char,
+}
+
+impl MetadataPair {
+    fn new(key: &str, value: &str) -> anyhow::Result<MetadataPair> {
+        Ok(MetadataPair {
+            key: string::to_c(key)? as *const c_char,
+            value: string::to_c(value)? as *const c_char,
+        })
+    }
+}
+
+// Ensure that the owned strings are freed when the pair is dropped.
+//
+// Notice that we're casting from a `*const c_char` to a `*mut c_char`.
+// This may seem wrong, but is safe so long as it doesn't violate Rust's
+// guarantees around immutable references, which this doesn't. In this case,
+// the underlying data came from `CString::into_raw` which takes ownership
+// of the `CString` and hands it off via a `*mut pointer`. We cast that pointer
+// back to `*const` to limit the C-side from doing any shenanigans, since the
+// pointed-to values live inside of the `Message` metadata `HashMap`, but
+// cast back to `*mut` here so we can free the memory.
+//
+// The discussion here helps explain: https://github.com/rust-lang/rust-clippy/issues/4774
+impl Drop for MetadataPair {
+    fn drop(&mut self) {
+        string::string_delete(self.key as *mut c_char);
+        string::string_delete(self.value as *mut c_char);
+    }
+}
 
 /*===============================================================================================
  * # Status Types

rust/pact_matching_ffi/src/util/ptr.rs

@@ -36,7 +36,7 @@ pub(crate) fn null_mut_to<T>() -> *mut T {
 /// Get an immutable reference from a raw pointer
 #[macro_export]
 macro_rules! as_ref {
-    ( $name:ident ) => {{
+    ( $name:expr ) => {{
         $name
             .as_ref()
             .ok_or(anyhow!(concat!(stringify!($name), " is null")))?
@@ -46,7 +46,7 @@ macro_rules! as_ref {
 /// Get a mutable reference from a raw pointer
 #[macro_export]
 macro_rules! as_mut {
-    ( $name:ident ) => {{
+    ( $name:expr ) => {{
         $name
             .as_mut()
             .ok_or(anyhow!(concat!(stringify!($name), " is null")))?

rust/pact_matching_ffi/src/util/string.rs

@@ -1,24 +1,14 @@
 use libc::c_char;
 use std::ffi::CString;
-use std::mem;
 
 /// Converts the string into a C-compatible null terminated string,
 /// then forgets the container while returning a pointer to the
 /// underlying buffer.
 ///
 /// The returned pointer must be passed to CString::from_raw to
 /// prevent leaking memory.
-pub(crate) fn into_leaked_cstring(
-    t: &str,
-) -> anyhow::Result<*const c_char> {
-    let copy = CString::new(t)?;
-    let ptr = copy.as_ptr();
-
-    // Intentionally leak this memory so that it stays
-    // valid while C is using it.
-    mem::forget(copy);
-
-    Ok(ptr)
+pub(crate) fn to_c(t: &str) -> anyhow::Result<*mut c_char> {
+    Ok(CString::new(t)?.into_raw())
 }
 
 /// Delete a string previously returned by this FFI.