remove fallible streaming iterator for using keyedmessage with phantomdata, all tests pass except doctests

Author: Quba1

Cargo.toml

@@ -26,7 +26,6 @@ errno = { version = "0.3", default-features = false }
 num-derive = { version = "0.4", default-features = false }
 num-traits = { version = "0.2", default-features = false }
 fallible-iterator = { version = "0.3", default-features = false }
-fallible-streaming-iterator = { version = "0.1.9", default-features = false }
 ndarray = { version = "0.16", default-features = false, optional = true, features = [
     "std",
 ] }

benches/main.rs

@@ -1,5 +1,5 @@
 use criterion::{Criterion, criterion_group, criterion_main};
-use eccodes::FallibleStreamingIterator;
+use eccodes::FallibleIterator;
 use eccodes::codes_handle::{CodesHandle, ProductKind};
 use std::hint::black_box;
 use std::path::Path;
@@ -11,7 +11,7 @@ pub fn key_reading(c: &mut Criterion) {
 
     let mut handle = CodesHandle::new_from_file(file_path, product_kind).unwrap();
 
-    let msg = handle.next().unwrap().unwrap();
+    let msg = handle.message_generator().next().unwrap().unwrap();
 
     c.bench_function("long reading", |b| {
         b.iter(|| msg.read_key_dynamic(black_box("dataDate")).unwrap())

src/codes_handle/iterator.rs

@@ -1,45 +1,48 @@
+use fallible_iterator::FallibleIterator;
+
 use crate::{CodesHandle, KeyedMessage, codes_handle::HandleGenerator, errors::CodesError};
-use fallible_streaming_iterator::FallibleStreamingIterator;
-use std::fmt::Debug;
+use std::marker::PhantomData;
+
+#[derive(Debug)]
+pub struct KeyedMessageGenerator<'a, S: HandleGenerator> {
+    codes_handle: &'a mut CodesHandle<S>,
+}
+
+impl<S: HandleGenerator> CodesHandle<S> {
+    pub fn message_generator<'a>(&'a mut self) -> KeyedMessageGenerator<'a, S> {
+        KeyedMessageGenerator { codes_handle: self }
+    }
+}
 
 /// # Errors
 ///
-/// The `advance()` and `next()` methods will return [`CodesInternal`](crate::errors::CodesInternal)
+/// The `next()` will return [`CodesInternal`](crate::errors::CodesInternal)
 /// when internal ecCodes function returns non-zero code.
-impl<S: HandleGenerator + Debug> FallibleStreamingIterator for CodesHandle<S> {
-    type Item = KeyedMessage;
-
+impl<'ch, S: HandleGenerator> FallibleIterator for KeyedMessageGenerator<'ch, S> {
+    type Item = KeyedMessage<'ch>;
     type Error = CodesError;
 
-    fn advance(&mut self) -> Result<(), Self::Error> {
-        // destructor of KeyedMessage calls ecCodes
+    fn next(&mut self) -> Result<Option<Self::Item>, Self::Error> {
+        let new_eccodes_handle = self.codes_handle.source.gen_codes_handle()?;
 
-        let new_eccodes_handle = self.source.gen_codes_handle()?;
-
-        self.current_message = if new_eccodes_handle.is_null() {
-            None
+        if new_eccodes_handle.is_null() {
+            Ok(None)
         } else {
-            Some(KeyedMessage {
+            Ok(Some(KeyedMessage {
+                parent_message: PhantomData,
                 message_handle: new_eccodes_handle,
-            })
-        };
-
-        Ok(())
-    }
-
-    fn get(&self) -> Option<&Self::Item> {
-        self.current_message.as_ref()
+            }))
+        }
     }
 }
 
 #[cfg(test)]
 mod tests {
     use crate::{
-        DynamicKeyType,
+        DynamicKeyType, FallibleIterator,
         codes_handle::{CodesHandle, ProductKind},
     };
     use anyhow::{Context, Ok, Result};
-    use fallible_streaming_iterator::FallibleStreamingIterator;
     use std::path::Path;
 
     #[test]
@@ -48,14 +51,26 @@ mod tests {
         let product_kind = ProductKind::GRIB;
         let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;
 
-        let msg1 = handle.next()?.context("Message not some")?;
+        let msg1 = handle
+            .message_generator()
+            .next()?
+            .context("Message not some")?;
         let key1 = msg1.read_key_dynamic("typeOfLevel")?;
+        drop(msg1);
 
-        let msg2 = handle.next()?.context("Message not some")?;
+        let msg2 = handle
+            .message_generator()
+            .next()?
+            .context("Message not some")?;
         let key2 = msg2.read_key_dynamic("typeOfLevel")?;
+        drop(msg2);
 
-        let msg3 = handle.next()?.context("Message not some")?;
+        let msg3 = handle
+            .message_generator()
+            .next()?
+            .context("Message not some")?;
         let key3 = msg3.read_key_dynamic("typeOfLevel")?;
+        drop(msg3);
 
         assert_eq!(key1, DynamicKeyType::Str("isobaricInhPa".to_string()));
         assert_eq!(key2, DynamicKeyType::Str("isobaricInhPa".to_string()));
@@ -71,7 +86,7 @@ mod tests {
 
         let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;
 
-        while let Some(msg) = handle.next()? {
+        while let Some(msg) = handle.message_generator().next()? {
             let key = msg.read_key_dynamic("shortName")?;
 
             match key {
@@ -91,7 +106,7 @@ mod tests {
 
         let mut handle_collected = vec![];
 
-        while let Some(msg) = handle.next()? {
+        while let Some(msg) = handle.message_generator().next()? {
             handle_collected.push(msg.try_clone()?);
         }
 
@@ -112,7 +127,7 @@ mod tests {
         let product_kind = ProductKind::GRIB;
 
         let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;
-        let current_message = handle.next()?.context("Message not some")?;
+        let current_message = handle.message_generator().next()?.context("Message not some")?;
 
         assert!(!current_message.message_handle.is_null());
 
@@ -125,17 +140,18 @@ mod tests {
         let product_kind = ProductKind::GRIB;
 
         let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;
+        let mut mgen = handle.message_generator();
 
-        assert!(handle.next()?.is_some());
-        assert!(handle.next()?.is_some());
-        assert!(handle.next()?.is_some());
-        assert!(handle.next()?.is_some());
-        assert!(handle.next()?.is_some());
+        assert!(mgen.next()?.is_some());
+        assert!(mgen.next()?.is_some());
+        assert!(mgen.next()?.is_some());
+        assert!(mgen.next()?.is_some());
+        assert!(mgen.next()?.is_some());
 
-        assert!(handle.next()?.is_none());
-        assert!(handle.next()?.is_none());
-        assert!(handle.next()?.is_none());
-        assert!(handle.next()?.is_none());
+        assert!(mgen.next()?.is_none());
+        assert!(mgen.next()?.is_none());
+        assert!(mgen.next()?.is_none());
+        assert!(mgen.next()?.is_none());
 
         Ok(())
     }
@@ -151,7 +167,7 @@ mod tests {
         // First, filter and collect the messages to get those that we want
         let mut level = vec![];
 
-        while let Some(msg) = handle.next()? {
+        while let Some(msg) = handle.message_generator().next()? {
             if msg.read_key_dynamic("shortName")? == DynamicKeyType::Str("msl".to_string())
                 && msg.read_key_dynamic("typeOfLevel")?
                     == DynamicKeyType::Str("surface".to_string())

src/codes_handle/mod.rs

@@ -3,9 +3,7 @@
 
 #[cfg(feature = "experimental_index")]
 use crate::codes_index::CodesIndex;
-use crate::{
-    CodesError, KeyedMessage, intermediate_bindings::codes_handle_new_from_file, pointer_guard,
-};
+use crate::{CodesError, intermediate_bindings::codes_handle_new_from_file, pointer_guard};
 use eccodes_sys::{ProductKind_PRODUCT_GRIB, codes_handle};
 use errno::errno;
 use libc::{FILE, c_char, c_void, size_t};
@@ -24,7 +22,7 @@ mod iterator;
 /// It is not intended to be used directly by the user.
 #[doc(hidden)]
 #[derive(Debug)]
-pub struct CodesFile<D> {
+pub struct CodesFile<D: Debug> {
     // fields dropped from top
     pointer: *mut FILE,
     product_kind: ProductKind,
@@ -33,11 +31,11 @@ pub struct CodesFile<D> {
 
 /// Internal trait implemented for types that can be called to generate `*mut codes_handle`.
 #[doc(hidden)]
-pub trait HandleGenerator {
+pub trait HandleGenerator: Debug {
     fn gen_codes_handle(&self) -> Result<*mut codes_handle, CodesError>;
 }
 
-impl<D> HandleGenerator for CodesFile<D> {
+impl<D: Debug> HandleGenerator for CodesFile<D> {
     fn gen_codes_handle(&self) -> Result<*mut codes_handle, CodesError> {
         unsafe { codes_handle_new_from_file(self.pointer, self.product_kind) }
     }
@@ -52,25 +50,17 @@ impl<D> HandleGenerator for CodesFile<D> {
 /// - From GRIB index using [`new_from_index()`](CodesHandle::new_from_index) (with `experimental_index` feature enabled)
 ///
 /// Destructor for this structure does not panic, but some internal functions may rarely fail
-/// leading to bugs. Errors encountered in the destructor are logged with [`log`].
+/// leading to bugs. Errors encountered in the destructor are logged with [`tracing`].
 ///
-/// # `FallibleStreamingIterator`
+/// # `FallibleIterator`
 ///
-/// This structure implements [`FallibleStreamingIterator`](crate::FallibleStreamingIterator) trait which allows to access GRIB messages.
+/// This structure implements [`FallibleIterator`](crate::FallibleStreamingIterator) trait which allows to access GRIB messages.
 ///
 /// To access GRIB messages the ecCodes library uses a method similar to a C-style iterator.
 /// It digests the `* FILE` multiple times, each time returning the `*mut codes_handle`
-/// to a message inside the file. The behavior of previous `*mut codes_handle` after next one is generated is undefined
-/// and we assume here that it is unsafe to use "old" `*mut codes_handle`.
-///
-/// In Rust, such pattern is best represented by a streaming iterator which returns a reference to the message,
-/// that is valid only until the next iteration. If you need to prolong the lifetime of the message, you can clone it.
-/// Internal ecCodes functions can fail, necessitating the streaming iterator to be implemented with
-/// [`FallibleStreamingIterator`](crate::FallibleStreamingIterator) trait.
+/// to a message inside the file.
 ///
-/// As of `0.10` release, none of the available streaming iterator crates utilises already stabilized GATs.
-/// This unfortunately significantly limits the number of methods available for `CodesHandle` iterator.
-/// Therefore the probably most versatile way to iterate over the messages is to use `while let` loop.
+/// This behaviour is represented in this crate by `FallibleIterator`, because generating `KeyedMessage` can fail.
 ///
 /// ```
 /// use eccodes::{ProductKind, CodesHandle, KeyRead};
@@ -120,9 +110,7 @@ impl<D> HandleGenerator for CodesFile<D> {
 ///
 /// All available methods for `CodesHandle` iterator can be found in [`FallibleStreamingIterator`](crate::FallibleStreamingIterator) trait.
 #[derive(Debug)]
-pub struct CodesHandle<S: Debug + HandleGenerator> {
-    // fields are dropped from top to bottom
-    current_message: Option<KeyedMessage>,
+pub struct CodesHandle<S: HandleGenerator> {
     source: S,
 }
 
@@ -198,7 +186,6 @@ impl CodesHandle<CodesFile<File>> {
                 pointer: file_pointer,
                 product_kind,
             },
-            current_message: None,
         })
     }
 }
@@ -250,7 +237,6 @@ impl CodesHandle<CodesFile<Vec<u8>>> {
                 product_kind,
                 pointer: file_pointer,
             },
-            current_message: None,
         })
     }
 }
@@ -340,7 +326,7 @@ mod tests {
     use crate::codes_index::{CodesIndex, Select};
     use anyhow::{Context, Result};
     use eccodes_sys::{ProductKind_PRODUCT_GRIB, grib_handle};
-    use fallible_streaming_iterator::FallibleStreamingIterator;
+    use fallible_iterator::FallibleIterator;
     use std::{
         fs::File,
         io::Read,
@@ -356,7 +342,6 @@ mod tests {
         let handle = CodesHandle::new_from_file(file_path, product_kind)?;
 
         assert!(!handle.source.pointer.is_null());
-        assert!(handle.current_message.is_none());
         assert_eq!(handle.source.product_kind as u32, {
             ProductKind_PRODUCT_GRIB
         });
@@ -438,7 +423,6 @@ mod tests {
 
         let handle = CodesHandle::new_from_memory(buf, product_kind)?;
         assert!(!handle.source.pointer.is_null());
-        assert!(handle.current_message.is_none());
         assert_eq!(handle.source.product_kind as u32, {
             ProductKind_PRODUCT_GRIB
         });
@@ -503,9 +487,10 @@ mod tests {
 
             let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;
 
-            let _ref_msg = handle.next()?.context("no message")?;
+            let _ref_msg = handle.message_generator().next()?.context("no message")?;
             let clone_msg = _ref_msg.try_clone()?;
-            let _oth_ref = handle.next()?.context("no message")?;
+            drop(_ref_msg);
+            let _oth_ref = handle.message_generator().next()?.context("no message")?;
 
             let _nrst = clone_msg.codes_nearest()?;
             let _kiter = clone_msg.default_keys_iterator()?;

src/codes_nearest.rs

@@ -1,7 +1,7 @@
 //! Definition and associated functions of `CodesNearest`
 //! used for finding nearest gridpoints in `KeyedMessage`
 
-use std::ptr::null_mut;
+use std::{fmt::Debug, ptr::null_mut};
 
 use eccodes_sys::codes_nearest;
 use tracing::{Level, event, instrument};
@@ -17,7 +17,7 @@ use crate::{
 #[derive(Debug)]
 pub struct CodesNearest<'a> {
     nearest_handle: *mut codes_nearest,
-    parent_message: &'a KeyedMessage,
+    parent_message: &'a KeyedMessage<'a>,
 }
 
 /// The structure returned by [`CodesNearest::find_nearest()`].
@@ -36,7 +36,7 @@ pub struct NearestGridpoint {
     pub value: f64,
 }
 
-impl KeyedMessage {
+impl KeyedMessage<'_> {
     /// Creates a new instance of [`CodesNearest`] for the `KeyedMessage`.
     /// [`CodesNearest`] can be used to find nearest gridpoints for given coordinates in the `KeyedMessage`
     /// by calling [`find_nearest()`](crate::CodesNearest::find_nearest).
@@ -126,7 +126,7 @@ mod tests {
     use std::path::Path;
 
     use anyhow::{Context, Result};
-    use fallible_streaming_iterator::FallibleStreamingIterator;
+    use fallible_iterator::FallibleIterator;
 
     use crate::{CodesHandle, ProductKind};
 
@@ -137,12 +137,18 @@ mod tests {
         let product_kind = ProductKind::GRIB;
 
         let mut handle1 = CodesHandle::new_from_file(file_path1, product_kind)?;
-        let msg1 = handle1.next()?.context("Message not some")?;
+        let msg1 = handle1
+            .message_generator()
+            .next()?
+            .context("Message not some")?;
         let nrst1 = msg1.codes_nearest()?;
         let out1 = nrst1.find_nearest(64.13, -21.89)?;
 
         let mut handle2 = CodesHandle::new_from_file(file_path2, product_kind)?;
-        let msg2 = handle2.next()?.context("Message not some")?;
+        let msg2 = handle2
+            .message_generator()
+            .next()?
+            .context("Message not some")?;
         let nrst2 = msg2.codes_nearest()?;
         let out2 = nrst2.find_nearest(64.13, -21.89)?;
 
@@ -161,13 +167,13 @@ mod tests {
         let product_kind = ProductKind::GRIB;
 
         let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;
-        let current_message = handle.next()?.context("Message not some")?;
+        let current_message = handle
+            .message_generator()
+            .next()?
+            .context("Message not some")?;
 
         let _nrst = current_message.codes_nearest()?;
 
-        drop(_nrst);
-        drop(handle);
-
         Ok(())
     }
 }