diff --git a/src/codes_handle/iterator.rs b/src/codes_handle/iterator.rs index 9bc7caa..9f72f13 100644 --- a/src/codes_handle/iterator.rs +++ b/src/codes_handle/iterator.rs @@ -98,11 +98,6 @@ impl FallibleStreamingIterator for CodesHandle { self.unsafe_message = KeyedMessage { message_handle: new_eccodes_handle, - iterator_flags: None, - iterator_namespace: None, - keys_iterator: None, - keys_iterator_next_item_exists: false, - nearest_handle: None, }; Ok(()) @@ -136,11 +131,6 @@ impl FallibleStreamingIterator for CodesHandle { self.unsafe_message = KeyedMessage { message_handle: new_eccodes_handle, - iterator_flags: None, - iterator_namespace: None, - keys_iterator: None, - keys_iterator_next_item_exists: false, - nearest_handle: None, }; Ok(()) @@ -235,10 +225,6 @@ mod tests { let current_message = handle.next().unwrap().unwrap(); assert!(!current_message.message_handle.is_null()); - assert!(current_message.iterator_flags.is_none()); - assert!(current_message.iterator_namespace.is_none()); - assert!(current_message.keys_iterator.is_none()); - assert!(!current_message.keys_iterator_next_item_exists); } #[test] diff --git a/src/codes_handle/keyed_message/iterator.rs b/src/codes_handle/keyed_message/iterator.rs index 18ff274..e35e8ba 100644 --- a/src/codes_handle/keyed_message/iterator.rs +++ b/src/codes_handle/keyed_message/iterator.rs @@ -1,17 +1,93 @@ -use eccodes_sys::codes_keys_iterator; use fallible_iterator::FallibleIterator; +use log::warn; +use std::ptr::null_mut; use crate::{ - codes_handle::{Key, KeyedMessage}, + codes_handle::{Key, KeyedMessage, KeysIterator}, errors::CodesError, intermediate_bindings::{ - codes_keys_iterator_get_name, codes_keys_iterator_new, codes_keys_iterator_next, + codes_keys_iterator_delete, codes_keys_iterator_get_name, codes_keys_iterator_new, + codes_keys_iterator_next, }, }; use super::KeysIteratorFlags; -///`FallibleIterator` implementation for `KeyedMessage` to access keyes inside message. +impl KeyedMessage { + ///Function that allows to set the flags and namespace for `FallibleIterator`. + ///**Must be called before calling the iterator.** Changing the parameters + ///after first call of `next()` will have no effect on the iterator. + /// + ///The flags are set by providing any combination of [`KeysIteratorFlags`] + ///inside a vector. Check the documentation for the details of each flag meaning. + /// + ///Namespace is set simply as string, eg. `"ls"`, `"time"`, `"parameter"`, `"geography"`, `"statistics"`. + ///Invalid namespace will result in empty iterator. + /// + ///Default parameters are [`AllKeys`](KeysIteratorFlags::AllKeys) flag and `""` namespace, + ///which implies iteration over all keys available in the message. + /// + ///### Example + /// + ///``` + ///# use eccodes::codes_handle::{ProductKind, CodesHandle, KeyedMessage, KeysIteratorFlags}; + ///# use std::path::Path; + ///# use eccodes::codes_handle::KeyType::Str; + ///# use eccodes::FallibleIterator; + ///let file_path = Path::new("./data/iceland.grib"); + ///let product_kind = ProductKind::GRIB; + /// + ///let mut handle = CodesHandle::new_from_file(file_path, product_kind).unwrap(); + ///let mut current_message = handle.next().unwrap().unwrap(); + /// + /// + ///let flags = vec![ + /// KeysIteratorFlags::AllKeys, + /// KeysIteratorFlags::SkipOptional, + /// KeysIteratorFlags::SkipReadOnly, + /// KeysIteratorFlags::SkipDuplicates, + ///]; + /// + ///let namespace = "geography".to_owned(); + /// + ///current_message.set_iterator_parameters(flags, namespace); + /// + /// + ///while let Some(key) = current_message.next().unwrap() { + /// println!("{:?}", key); + ///} + ///``` + pub fn new_keys_iterator( + &self, + flags: Vec, + namespace: String, + ) -> Result { + let flags = flags.iter().map(|f| *f as u32).sum(); + + let iterator_handle = + unsafe { codes_keys_iterator_new(self.message_handle, flags, &namespace)? }; + let next_item_exists = unsafe { codes_keys_iterator_next(iterator_handle) }; + + Ok(KeysIterator { + parent_message: self, + iterator_handle, + next_item_exists, + }) + } + + pub fn default_keys_iterator(&self) -> Result { + let iterator_handle = unsafe { codes_keys_iterator_new(self.message_handle, 0, "")? }; + let next_item_exists = unsafe { codes_keys_iterator_next(iterator_handle) }; + + Ok(KeysIterator { + parent_message: self, + iterator_handle, + next_item_exists, + }) + } +} + +///`FallibleIterator` implementation for `KeysIterator` to iterate through keys inside `KeyedMessage`. ///Mainly useful to discover what keys are present inside the message. /// ///This function internally calls [`read_key()`](KeyedMessage::read_key()) function @@ -44,25 +120,23 @@ use super::KeysIteratorFlags; ///## Errors ///The `next()` method will return [`CodesInternal`](crate::errors::CodesInternal) ///when internal ecCodes function returns non-zero code. -impl FallibleIterator for KeyedMessage { +impl FallibleIterator for KeysIterator<'_> { type Item = Key; type Error = CodesError; fn next(&mut self) -> Result, Self::Error> { - let itr = self.keys_iterator()?; - - if self.keys_iterator_next_item_exists { + if self.next_item_exists { let key_name; let next_item_exists; unsafe { - key_name = codes_keys_iterator_get_name(itr)?; - next_item_exists = codes_keys_iterator_next(itr); + key_name = codes_keys_iterator_get_name(self.iterator_handle)?; + next_item_exists = codes_keys_iterator_next(self.iterator_handle); } - let key = KeyedMessage::read_key(self, &key_name)?; + let key = KeyedMessage::read_key(self.parent_message, &key_name)?; - self.keys_iterator_next_item_exists = next_item_exists; + self.next_item_exists = next_item_exists; Ok(Some(key)) } else { @@ -71,86 +145,20 @@ impl FallibleIterator for KeyedMessage { } } -impl KeyedMessage { - ///Function that allows to set the flags and namespace for `FallibleIterator`. - ///**Must be called before calling the iterator.** Changing the parameters - ///after first call of `next()` will have no effect on the iterator. - /// - ///The flags are set by providing any combination of [`KeysIteratorFlags`] - ///inside a vector. Check the documentation for the details of each flag meaning. - /// - ///Namespace is set simply as string, eg. `"ls"`, `"time"`, `"parameter"`, `"geography"`, `"statistics"`. - ///Invalid namespace will result in empty iterator. - /// - ///Default parameters are [`AllKeys`](KeysIteratorFlags::AllKeys) flag and `""` namespace, - ///which implies iteration over all keys available in the message. - /// - ///### Example - /// - ///``` - ///# use eccodes::codes_handle::{ProductKind, CodesHandle, KeyedMessage, KeysIteratorFlags}; - ///# use std::path::Path; - ///# use eccodes::codes_handle::KeyType::Str; - ///# use eccodes::FallibleIterator; - ///let file_path = Path::new("./data/iceland.grib"); - ///let product_kind = ProductKind::GRIB; - /// - ///let mut handle = CodesHandle::new_from_file(file_path, product_kind).unwrap(); - ///let mut current_message = handle.next().unwrap().unwrap(); - /// - /// - ///let flags = vec![ - /// KeysIteratorFlags::AllKeys, - /// KeysIteratorFlags::SkipOptional, - /// KeysIteratorFlags::SkipReadOnly, - /// KeysIteratorFlags::SkipDuplicates, - ///]; - /// - ///let namespace = "geography".to_owned(); - /// - ///current_message.set_iterator_parameters(flags, namespace); - /// - /// - ///while let Some(key) = current_message.next().unwrap() { - /// println!("{:?}", key); - ///} - ///``` - pub fn set_iterator_parameters(&mut self, flags: Vec, namespace: String) { - self.iterator_namespace = Some(namespace); - - let mut flags_sum = 0; - - for flag in flags { - flags_sum += flag as u32; +impl Drop for KeysIterator<'_> { + fn drop(&mut self) { + unsafe { + codes_keys_iterator_delete(self.iterator_handle).unwrap_or_else(|error| { + warn!( + "codes_keys_iterator_delete() returned an error: {:?}", + &error + ); + }); } - self.iterator_flags = Some(flags_sum); - } + self.iterator_handle = null_mut(); - fn keys_iterator(&mut self) -> Result<*mut codes_keys_iterator, CodesError> { - self.keys_iterator.map_or_else( - || { - let flags = self.iterator_flags.unwrap_or(0); - - let namespace = match self.iterator_namespace.clone() { - Some(n) => n, - None => String::new(), - }; - - let itr; - let next_item; - unsafe { - itr = codes_keys_iterator_new(self.message_handle, flags, &namespace); - next_item = codes_keys_iterator_next(itr); - } - - self.keys_iterator_next_item_exists = next_item; - self.keys_iterator = Some(itr); - - Ok(itr) - }, - Ok, - ) + todo!() } } diff --git a/src/codes_handle/keyed_message/mod.rs b/src/codes_handle/keyed_message/mod.rs index 31019c4..53ee7ad 100644 --- a/src/codes_handle/keyed_message/mod.rs +++ b/src/codes_handle/keyed_message/mod.rs @@ -9,9 +9,7 @@ use std::ptr::null_mut; use crate::{ codes_handle::KeyedMessage, errors::CodesError, - intermediate_bindings::{ - codes_grib_nearest_new, codes_handle_clone, codes_handle_delete, codes_keys_iterator_delete, - }, + intermediate_bindings::{codes_grib_nearest_new, codes_handle_clone, codes_handle_delete}, }; use super::{CodesNearest, KeysIteratorFlags}; @@ -36,44 +34,25 @@ impl Clone for KeyedMessage { KeyedMessage { message_handle: new_handle, - iterator_flags: None, - iterator_namespace: None, - keys_iterator: None, - keys_iterator_next_item_exists: false, - nearest_handle: None, } } } impl Drop for KeyedMessage { ///Executes the destructor for this type. - ///This method calls `codes_handle_delete()`, `codes_keys_iterator_delete()` - ///`codes_grib_nearest_delete()` from ecCodes for graceful cleanup. - ///However in some edge cases ecCodes can return non-zero code. + ///This method calls destructor functions from ecCodes library. + ///In some edge cases these functions can return non-zero code. ///In such case all pointers and file descriptors are safely deleted. ///However memory leaks can still occur. /// ///If any function called in the destructor returns an error warning will appear in log. - ///If bugs occurs during `CodesHandle` drop please enable log output and post issue on [Github](https://github.com/ScaleWeather/eccodes). + ///If bugs occur during `CodesHandle` drop please enable log output and post issue on [Github](https://github.com/ScaleWeather/eccodes). /// ///Technical note: delete functions in ecCodes can only fail with [`CodesInternalError`](crate::errors::CodesInternal::CodesInternalError) ///when other functions corrupt the inner memory of pointer, in that case memory leak is possible. ///In case of corrupt pointer segmentation fault will occur. - ///The pointers are cleared at the end of drop as they are not functional despite the result of delete functions. + ///The pointers are cleared at the end of drop as they are not functional regardless of result of delete functions. fn drop(&mut self) { - if let Some(kiter) = self.keys_iterator { - unsafe { - codes_keys_iterator_delete(kiter).unwrap_or_else(|error| { - warn!( - "codes_keys_iterator_delete() returned an error: {:?}", - &error - ); - }); - } - } - - self.keys_iterator = Some(null_mut()); - unsafe { codes_handle_delete(self.message_handle).unwrap_or_else(|error| { warn!("codes_handle_delete() returned an error: {:?}", &error); @@ -87,7 +66,7 @@ impl Drop for KeyedMessage { #[cfg(test)] mod tests { use crate::codes_handle::{CodesHandle, ProductKind}; - use crate::{FallibleIterator, FallibleStreamingIterator}; + use crate::FallibleStreamingIterator; use anyhow::Result; use std::path::Path; use testing_logger; @@ -105,10 +84,6 @@ mod tests { current_message.message_handle, cloned_message.message_handle ); - assert!(cloned_message.iterator_flags.is_none()); - assert!(cloned_message.iterator_namespace.is_none()); - assert!(cloned_message.keys_iterator.is_none()); - assert!(!cloned_message.keys_iterator_next_item_exists); Ok(()) } @@ -143,7 +118,8 @@ mod tests { let mut handle = CodesHandle::new_from_file(file_path, product_kind)?; let mut current_message = handle.next()?.unwrap().clone(); - let _key = current_message.next()?.unwrap(); + let _kiter = current_message.default_keys_iterator()?; + let _niter = current_message.codes_nearest()?; drop(current_message); diff --git a/src/codes_handle/keyed_message/read.rs b/src/codes_handle/keyed_message/read.rs index 1fcf994..ca41489 100644 --- a/src/codes_handle/keyed_message/read.rs +++ b/src/codes_handle/keyed_message/read.rs @@ -229,10 +229,11 @@ mod tests { let product_kind = ProductKind::GRIB; let mut handle = CodesHandle::new_from_file(file_path, product_kind)?; - let mut current_message = handle.next()?.unwrap().clone(); + let current_message = handle.next()?.unwrap(); + let mut kiter = current_message.default_keys_iterator()?; for i in 0..=300 { - let key = current_message.next(); + let key = kiter.next()?.unwrap(); println!("{}: {:?}", i, key); } @@ -245,10 +246,11 @@ mod tests { let product_kind = ProductKind::GRIB; let mut handle = CodesHandle::new_from_file(file_path, product_kind)?; - let mut current_message = handle.next()?.unwrap().clone(); + let current_message = handle.next()?.unwrap(); + let mut kiter = current_message.default_keys_iterator()?; for i in 0..=300 { - let key = current_message.next(); + let key = kiter.next()?.unwrap(); println!("{}: {:?}", i, key); } diff --git a/src/codes_handle/mod.rs b/src/codes_handle/mod.rs index 627b6be..aea1da6 100644 --- a/src/codes_handle/mod.rs +++ b/src/codes_handle/mod.rs @@ -60,11 +60,6 @@ pub struct CodesHandle { #[derive(Hash, Debug)] pub struct KeyedMessage { message_handle: *mut codes_handle, - iterator_flags: Option, - iterator_namespace: Option, - keys_iterator: Option<*mut codes_keys_iterator>, - keys_iterator_next_item_exists: bool, - nearest_handle: Option<*mut codes_nearest>, } ///Structure representing a single key from the `KeyedMessage`. @@ -74,6 +69,13 @@ pub struct Key { pub value: KeyType, } +#[derive(Debug)] +pub struct KeysIterator<'a> { + parent_message: &'a KeyedMessage, + iterator_handle: *mut codes_keys_iterator, + next_item_exists: bool, +} + ///Enum to represent and contain all possible types of keys inside `KeyedMessage`. /// ///Messages inside GRIB files can contain arbitrary keys set by the file author. @@ -201,11 +203,6 @@ impl CodesHandle { product_kind, unsafe_message: KeyedMessage { message_handle: null_mut(), - iterator_flags: None, - iterator_namespace: None, - keys_iterator: None, - keys_iterator_next_item_exists: false, - nearest_handle: None, }, }) } @@ -262,11 +259,6 @@ impl CodesHandle { product_kind, unsafe_message: KeyedMessage { message_handle: null_mut(), - iterator_flags: None, - iterator_namespace: None, - keys_iterator: None, - keys_iterator_next_item_exists: false, - nearest_handle: None, }, }) } @@ -286,11 +278,6 @@ impl CodesHandle { product_kind, unsafe_message: KeyedMessage { message_handle: null_mut(), - iterator_flags: None, - iterator_namespace: None, - keys_iterator: None, - keys_iterator_next_item_exists: false, - nearest_handle: None, }, }; diff --git a/src/errors.rs b/src/errors.rs index 1f5aeb7..588ea97 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -52,6 +52,10 @@ pub enum CodesError { /// indicating issues with cloning the message. #[error("Cannot clone the message")] CloneFailed, + + /// Returned when codes_keys_iterator_new returns null pointer + #[error("Cannot create or manipulate keys iterator")] + KeysIteratorFailed, } #[derive(Copy, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, Error, Debug, FromPrimitive)] diff --git a/src/intermediate_bindings/mod.rs b/src/intermediate_bindings/mod.rs index 032db49..799e792 100644 --- a/src/intermediate_bindings/mod.rs +++ b/src/intermediate_bindings/mod.rs @@ -315,10 +315,16 @@ pub unsafe fn codes_keys_iterator_new( handle: *mut codes_handle, flags: u32, namespace: &str, -) -> *mut codes_keys_iterator { +) -> Result<*mut codes_keys_iterator, CodesError> { let namespace = CString::new(namespace).unwrap(); - eccodes_sys::codes_keys_iterator_new(handle, u64::from(flags), namespace.as_ptr()) + let kiter = eccodes_sys::codes_keys_iterator_new(handle, u64::from(flags), namespace.as_ptr()); + + if kiter.is_null() { + return Err(CodesError::KeysIteratorFailed); + } + + Ok(kiter) } pub unsafe fn codes_keys_iterator_delete( diff --git a/src/lib.rs b/src/lib.rs index 08348f6..db62f78 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -224,7 +224,8 @@ pub mod errors; mod intermediate_bindings; pub use codes_handle::{ - CodesHandle, Key, KeyType, KeyedMessage, KeysIteratorFlags, NearestGridpoint, ProductKind, + CodesHandle, CodesNearest, Key, KeyType, KeyedMessage, KeysIteratorFlags, NearestGridpoint, + ProductKind, }; #[cfg(feature = "experimental_index")] #[cfg_attr(docsrs, doc(cfg(feature = "experimental_index")))]