Skip to content

Commit

Permalink
(tests not working) remove need for clone for KeysIter
Browse files Browse the repository at this point in the history
  • Loading branch information
@Quba1
Quba1 committed Feb 3, 2024
1 parent f3a26be commit 05cf0c3
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 162 deletions.
14 changes: 0 additions & 14 deletions src/codes_handle/iterator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,6 @@ impl FallibleStreamingIterator for CodesHandle<GribFile> {

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(())
Expand Down Expand Up @@ -136,11 +131,6 @@ impl FallibleStreamingIterator for CodesHandle<CodesIndex> {

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(())
Expand Down Expand Up @@ -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]
Expand Down
186 changes: 97 additions & 89 deletions src/codes_handle/keyed_message/iterator.rs
View file
Original file line number Diff line number Diff line change
@@ -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<KeysIteratorFlags>,
namespace: String,
) -> Result<KeysIterator, CodesError> {
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<KeysIterator, CodesError> {
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
Expand Down Expand Up @@ -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<Option<Self::Item>, 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 {
Expand All @@ -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<KeysIteratorFlags>, 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!()
}
}

Expand Down
40 changes: 8 additions & 32 deletions src/codes_handle/keyed_message/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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);

Expand Down
10 changes: 6 additions & 4 deletions src/codes_handle/keyed_message/read.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down
Loading

0 comments on commit 05cf0c3

Please sign in to comment.