blackmwk commented on code in PR #3283:
URL: https://github.com/apache/iceberg-rust/pull/3283#discussion_r4536670668
##########
crates/iceberg/src/encryption/io.rs:
##########
@@ -273,25 +286,30 @@ mod tests {
#[tokio::test]
async fn test_invalid_file_length_is_rejected() {
let fileio = FileIO::new_with_memory();
+ let path = "memory:///test/invalid_length.bin";
Review Comment:
Is this change really necessary?
##########
crates/iceberg/src/encryption/io.rs:
##########
@@ -74,8 +74,21 @@ impl EncryptedInputFile {
/// Creates a reader that transparently decrypts on each read.
pub async fn reader(&self) -> Result<Box<dyn FileRead>> {
+ // Identity just returns the file reader it was given.
+ self.reader_with_wrapper(std::convert::identity).await
+ }
+
+ /// Creates a decrypting reader, letting the caller wrap the file reader
first.
+ ///
+ /// Calls `wrap` once with the file reader and uses the result for
decryption.
+ /// This lets callers add behavior such as logging reads or counting bytes
+ /// without changing the decryption code.
+ pub(crate) async fn reader_with_wrapper(
+ &self,
+ wrap: impl FnOnce(Box<dyn FileRead>) -> Box<dyn FileRead>,
+ ) -> Result<Box<dyn FileRead>> {
Review Comment:
```suggestion
pub(crate) async fn reader_with_wrapper(
&self,
wrap: F,
) -> Result<R>
where F: FnOnce(impl FileRead) -> R,
R: FileRead
{
```
##########
crates/iceberg/src/arrow/caching_delete_file_loader.rs:
##########
@@ -421,18 +423,34 @@ impl CachingDeleteFileLoader {
let input_file = basic_delete_file_loader
.file_io()
.new_input(task.file_path())?;
- let blob = match task.key_metadata() {
+ let key_metadata = task
+ .key_metadata()
+ .map(StandardKeyMetadata::decode)
+ .transpose()?;
+ let count_reads = |reader| -> Box<dyn FileRead> {
Review Comment:
See, I think this could be simplified as
```rust
|reader| -> CountFileRead::new(reader, xxx)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]