This is an automated email from the ASF dual-hosted git repository.
erickguan pushed a commit to branch ruby-binding-io
in repository https://gitbox.apache.org/repos/asf/opendal.git
commit 0840c3aee45dc9caf32b2cc366f19abbc40276a3
Author: Erick Guan <297343+erickg...@users.noreply.github.com>
AuthorDate: Sun Mar 30 14:54:09 2025 +0200
feat(bindings/ruby): support Ruby's IO#new parameters
---
bindings/ruby/src/io.rs | 248 +++++++++++++++++++++++++-----------------
bindings/ruby/src/operator.rs | 16 ++-
2 files changed, 162 insertions(+), 102 deletions(-)
diff --git a/bindings/ruby/src/io.rs b/bindings/ruby/src/io.rs
index ca04c2545..082ff1b9e 100644
--- a/bindings/ruby/src/io.rs
+++ b/bindings/ruby/src/io.rs
@@ -23,7 +23,6 @@
#![allow(rustdoc::bare_urls, reason = "YARD's syntax for documentation")]
use std::cell::RefCell;
-use std::collections::HashSet;
use std::io::BufRead;
use std::io::Read;
use std::io::Seek;
@@ -31,10 +30,13 @@ use std::io::SeekFrom;
use std::io::Write;
use magnus::class;
+use magnus::io::FMode;
use magnus::method;
use magnus::prelude::*;
use magnus::Error;
+use magnus::RHash;
use magnus::RModule;
+use magnus::Value;
use crate::*;
@@ -49,15 +51,30 @@ use crate::*;
/// `OpenDAL::IO` is similar to Ruby's `IO` and `StringIO` for accessing files.
///
/// You can't create an instance of `OpenDAL::IO` except using
{OpenDAL::Operator#open}.
+///
+/// Constraints:
+/// - Only available for reading and writing
+/// - Writing doesn't support seek.
#[magnus::wrap(class = "OpenDAL::IO", free_immediately, size)]
-pub struct Io(RefCell<FileState>);
+pub struct Io(RefCell<IoHandle>);
enum FileState {
- Reader(ocore::blocking::StdReader, bool), // bool indicates binary mode
- Writer(ocore::blocking::StdWriter, bool), // bool indicates binary mode
+ Reader(ocore::blocking::StdReader),
+ Writer(ocore::blocking::StdWriter),
Closed,
}
+struct IoHandle {
+ state: FileState,
+ fmode: FMode,
+ #[allow(dead_code)]
+ external_encoding_name: Option<String>,
+ #[allow(dead_code)]
+ internal_encoding_name: Option<String>,
+ #[allow(dead_code)]
+ encoding_flags: i32,
+}
+
pub fn format_io_error(err: std::io::Error) -> Error {
Error::new(exception::runtime_error(), err.to_string())
}
@@ -70,42 +87,44 @@ impl Io {
ruby: &Ruby,
operator: ocore::blocking::Operator,
path: String,
- mode: String,
+ mut mode: Value,
+ mut permission: Value,
+ kwargs: RHash,
) -> Result<Self, Error> {
- let mut mode_flags = HashSet::new();
- let is_unique = mode.chars().all(|c| mode_flags.insert(c));
- if !is_unique {
- return Err(Error::new(
- ruby.exception_arg_error(),
- format!("Invalid access mode {mode}"),
- ));
- }
-
- let binary_mode = mode_flags.contains(&'b');
+ let (_open_flags, fmode, encoding) =
+ ruby.io_extract_modeenc(&mut mode, &mut permission, &kwargs)?;
- if mode_flags.contains(&'r') {
- Ok(Self(RefCell::new(FileState::Reader(
- operator
- .reader(&path)
- .map_err(format_magnus_error)?
- .into_std_read(..)
- .map_err(format_magnus_error)?,
- binary_mode,
- ))))
- } else if mode_flags.contains(&'w') {
- Ok(Self(RefCell::new(FileState::Writer(
+ let state =
+ // Create reader if mode supports reading
+ if fmode.contains(FMode::READ) {
+ FileState::Reader(
+ operator
+ .reader(&path)
+ .map_err(format_magnus_error)?
+ .into_std_read(..)
+ .map_err(format_magnus_error)?,
+ )
+ } else if fmode.contains(FMode::WRITE) {
+ FileState::Writer(
operator
.writer(&path)
.map_err(format_magnus_error)?
.into_std_write(),
- binary_mode,
- ))))
+ )
} else {
- Err(Error::new(
- ruby.exception_runtime_error(),
- format!("OpenDAL doesn't support mode: {mode}"),
- ))
- }
+ return Err(Error::new(
+ ruby.exception_arg_error(),
+ "Invalid mode: must open for reading or writing",
+ ));
+ };
+
+ Ok(Self(RefCell::new(IoHandle {
+ state,
+ fmode,
+ external_encoding_name: encoding.external.map(|e| e.to_string()),
+ internal_encoding_name: encoding.internal.map(|e| e.to_string()),
+ encoding_flags: encoding.flags,
+ })))
}
/// @yard
@@ -114,18 +133,12 @@ impl Io {
/// @return [nil]
/// @raise [IOError] when operate on a closed stream
fn binary_mode(ruby: &Ruby, rb_self: &Self) -> Result<(), Error> {
- let mut cell = rb_self.0.borrow_mut();
- match &mut *cell {
- FileState::Reader(_, ref mut is_binary_mode) => {
- *is_binary_mode = true;
- Ok(())
- }
- FileState::Writer(_, ref mut is_binary_mode) => {
- *is_binary_mode = true;
- Ok(())
- }
- FileState::Closed => Err(Error::new(ruby.exception_io_error(),
"closed stream")),
+ let mut handle = rb_self.0.borrow_mut();
+ if let FileState::Closed = handle.state {
+ return Err(Error::new(ruby.exception_io_error(), "closed stream"));
}
+ handle.fmode = FMode::new(handle.fmode.bits() | FMode::BINARY_MODE);
+ Ok(())
}
/// @yard
@@ -134,11 +147,11 @@ impl Io {
/// @return [Boolean]
/// @raise [IOError] when operate on a closed stream
fn is_binary_mode(ruby: &Ruby, rb_self: &Self) -> Result<bool, Error> {
- match *rb_self.0.borrow() {
- FileState::Reader(_, is_binary_mode) => Ok(is_binary_mode),
- FileState::Writer(_, is_binary_mode) => Ok(is_binary_mode),
- FileState::Closed => Err(Error::new(ruby.exception_io_error(),
"closed stream")),
+ let handle = rb_self.0.borrow();
+ if let FileState::Closed = handle.state {
+ return Err(Error::new(ruby.exception_io_error(), "closed stream"));
}
+ Ok(handle.fmode.contains(FMode::BINARY_MODE))
}
/// @yard
@@ -146,12 +159,11 @@ impl Io {
/// Close streams.
/// @return [nil]
fn close(&self) -> Result<(), Error> {
- // skips closing reader because `StdReader` doesn't have `close()`.
- let mut cell = self.0.borrow_mut();
- if let FileState::Writer(writer, _) = &mut *cell {
+ let mut handle = self.0.borrow_mut();
+ if let FileState::Writer(writer) = &mut handle.state {
writer.close().map_err(format_io_error)?;
}
- *cell = FileState::Closed;
+ handle.state = FileState::Closed;
Ok(())
}
@@ -160,7 +172,10 @@ impl Io {
/// Closes the read stream.
/// @return [nil]
fn close_read(&self) -> Result<(), Error> {
- *self.0.borrow_mut() = FileState::Closed;
+ let mut handle = self.0.borrow_mut();
+ if let FileState::Reader(_) = &handle.state {
+ handle.state = FileState::Closed;
+ }
Ok(())
}
@@ -169,11 +184,11 @@ impl Io {
/// Closes the write stream.
/// @return [nil]
fn close_write(&self) -> Result<(), Error> {
- let mut cell = self.0.borrow_mut();
- if let FileState::Writer(writer, _) = &mut *cell {
+ let mut handle = self.0.borrow_mut();
+ if let FileState::Writer(writer) = &mut handle.state {
writer.close().map_err(format_io_error)?;
+ handle.state = FileState::Closed;
}
- *cell = FileState::Closed;
Ok(())
}
@@ -182,7 +197,8 @@ impl Io {
/// Returns if streams are closed.
/// @return [Boolean]
fn is_closed(&self) -> Result<bool, Error> {
- Ok(matches!(*self.0.borrow(), FileState::Closed))
+ let handle = self.0.borrow();
+ Ok(matches!(handle.state, FileState::Closed))
}
/// @yard
@@ -190,7 +206,8 @@ impl Io {
/// Returns if the read stream is closed.
/// @return [Boolean]
fn is_closed_read(&self) -> Result<bool, Error> {
- Ok(!matches!(*self.0.borrow(), FileState::Reader(_, _)))
+ let handle = self.0.borrow();
+ Ok(!matches!(handle.state, FileState::Reader(_)))
}
/// @yard
@@ -198,7 +215,8 @@ impl Io {
/// Returns if the write stream is closed.
/// @return [Boolean]
fn is_closed_write(&self) -> Result<bool, Error> {
- Ok(!matches!(*self.0.borrow(), FileState::Writer(_, _)))
+ let handle = self.0.borrow();
+ Ok(!matches!(handle.state, FileState::Writer(_)))
}
}
@@ -210,8 +228,9 @@ impl Io {
///
/// @param size The maximum number of bytes to read. Reads all data if
`None`.
fn read(ruby: &Ruby, rb_self: &Self, size: Option<usize>) ->
Result<bytes::Bytes, Error> {
- // FIXME: consider what to return exactly
- if let FileState::Reader(reader, _) = &mut *rb_self.0.borrow_mut() {
+ let mut handle = rb_self.0.borrow_mut();
+
+ if let FileState::Reader(reader) = &mut handle.state {
let buffer = match size {
Some(size) => {
let mut bs = vec![0; size];
@@ -230,7 +249,7 @@ impl Io {
} else {
Err(Error::new(
ruby.exception_runtime_error(),
- "I/O operation failed for reading on closed file.",
+ "I/O operation failed for reading on write-only file.",
))
}
}
@@ -241,7 +260,9 @@ impl Io {
/// @return [String]
// TODO: extend readline with parameters
fn readline(ruby: &Ruby, rb_self: &Self) -> Result<String, Error> {
- if let FileState::Reader(reader, _) = &mut *rb_self.0.borrow_mut() {
+ let mut handle = rb_self.0.borrow_mut();
+
+ if let FileState::Reader(reader) = &mut handle.state {
let mut buffer = String::new();
let size = reader.read_line(&mut buffer).map_err(format_io_error)?;
if size == 0 {
@@ -255,7 +276,7 @@ impl Io {
} else {
Err(Error::new(
ruby.exception_runtime_error(),
- "I/O operation failed for reading on closed file.",
+ "I/O operation failed for reading on write-only file.",
))
}
}
@@ -266,15 +287,16 @@ impl Io {
/// @param buffer [String]
/// @return [Integer] the written byte size
fn write(ruby: &Ruby, rb_self: &Self, bs: String) -> Result<usize, Error> {
- if let FileState::Writer(writer, _) = &mut *rb_self.0.borrow_mut() {
- Ok(writer
- .write_all(bs.as_bytes())
- .map(|_| bs.len())
- .map_err(format_io_error)?)
+ let mut handle = rb_self.0.borrow_mut();
+
+ if let FileState::Writer(writer) = &mut handle.state {
+ let bytes_written = bs.len();
+ writer.write_all(bs.as_bytes()).map_err(format_io_error)?;
+ Ok(bytes_written)
} else {
Err(Error::new(
ruby.exception_runtime_error(),
- "I/O operation failed for reading on write only file.",
+ "I/O operation failed for writing on read-only file.",
))
}
}
@@ -289,49 +311,77 @@ impl Io {
/// - 0 = IO:SEEK_SET (Start)
/// - 1 = IO:SEEK_CUR (Current position)
/// - 2 = IO:SEEK_END (From the end)
- /// @return [Integer] always 0 if the seek operation is successful
+ ///
+ /// @return [Integer] always 0 if the seek operation is successful
fn seek(ruby: &Ruby, rb_self: &Self, offset: i64, whence: u8) ->
Result<u8, Error> {
- match &mut *rb_self.0.borrow_mut() {
- FileState::Reader(reader, _) => {
- let whence = match whence {
- 0 => SeekFrom::Start(offset as u64),
- 1 => SeekFrom::Current(offset),
- 2 => SeekFrom::End(offset),
- _ => return Err(Error::new(ruby.exception_arg_error(),
"invalid whence")),
- };
-
- reader.seek(whence).map_err(format_io_error)?;
-
- Ok(0)
- }
- FileState::Writer(_, _) => Err(Error::new(
- ruby.exception_runtime_error(),
- "I/O operation failed for reading on write only file.",
- )),
- FileState::Closed => Err(Error::new(
+ let mut handle = rb_self.0.borrow_mut();
+
+ if let FileState::Reader(reader) = &mut handle.state {
+ // Calculate the new position first
+ let new_reader_position = match whence {
+ 0 => {
+ // SEEK_SET - absolute position
+ if offset < 0 {
+ return Err(Error::new(
+ ruby.exception_runtime_error(),
+ "Cannot seek to negative reader_position.",
+ ));
+ }
+ offset as u64
+ }
+ 1 => {
+ // SEEK_CUR - relative to current position
+ let position =
reader.stream_position().map_err(format_io_error)?;
+ if offset < 0 && (-offset as u64) > position {
+ return Err(Error::new(
+ ruby.exception_runtime_error(),
+ "Cannot seek before start of stream.",
+ ));
+ }
+ (position as i64 + offset) as u64
+ }
+ 2 => {
+ // SEEK_END
+ let end_pos =
reader.seek(SeekFrom::End(0)).map_err(format_io_error)?;
+ if offset < 0 && (-offset as u64) > end_pos {
+ return Err(Error::new(
+ ruby.exception_runtime_error(),
+ "Cannot seek before start of stream.",
+ ));
+ }
+ (end_pos as i64 + offset) as u64
+ }
+ _ => return Err(Error::new(ruby.exception_arg_error(),
"invalid whence")),
+ };
+
+ let _ = reader.seek(std::io::SeekFrom::Start(new_reader_position));
+ } else {
+ return Err(Error::new(
ruby.exception_runtime_error(),
- "I/O operation failed for reading on closed file.",
- )),
+ "Cannot seek from end on write-only stream.",
+ ));
}
+
+ Ok(0)
}
/// @yard
/// @def tell
- /// Returns the current position of the file pointer in the stream.
- /// @return [Integer] the current position in bytes
+ /// Returns the current reader_position of the file pointer in the stream.
+ /// @return [Integer] the current reader_position in bytes
/// @raise [IOError] when cannot operate on the operation mode
fn tell(ruby: &Ruby, rb_self: &Self) -> Result<u64, Error> {
- match &mut *rb_self.0.borrow_mut() {
- FileState::Reader(reader, _) => {
- Ok(reader.stream_position().map_err(format_io_error)?)
- }
- FileState::Writer(_, _) => Err(Error::new(
+ let mut handle = rb_self.0.borrow_mut();
+
+ match &mut handle.state {
+ FileState::Reader(reader) =>
Ok(reader.stream_position().map_err(format_io_error)?),
+ FileState::Writer(_) => Err(Error::new(
ruby.exception_runtime_error(),
"I/O operation failed for reading on write only file.",
)),
FileState::Closed => Err(Error::new(
ruby.exception_runtime_error(),
- "I/O operation failed for reading on closed file.",
+ "I/O operation failed for tell on closed stream.",
)),
}
}
diff --git a/bindings/ruby/src/operator.rs b/bindings/ruby/src/operator.rs
index a75082bbc..5d91f74a9 100644
--- a/bindings/ruby/src/operator.rs
+++ b/bindings/ruby/src/operator.rs
@@ -31,6 +31,7 @@ use magnus::prelude::*;
use magnus::scan_args::get_kwargs;
use magnus::scan_args::scan_args;
use magnus::Error;
+use magnus::RHash;
use magnus::RModule;
use magnus::RString;
use magnus::Ruby;
@@ -233,9 +234,18 @@ impl Operator {
/// @param mode [String] operation mode, e.g., `r`, `w`, or `rb`.
/// @raise [ArgumentError] invalid mode, or when the mode is not unique
/// @return [OpenDAL::IO]
- fn open(ruby: &Ruby, rb_self: &Self, path: String, mode: String) ->
Result<Io, Error> {
+ fn open(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result<Io, Error> {
+ let args = scan_args::<(String,), (Option<Value>, Option<Value>), (),
(), RHash, ()>(args)?;
+ let (path,) = args.required;
+ let (option_mode, option_permission) = args.optional;
+ let kwargs = args.keywords;
+
+ // Ruby handles Qnil safely (will not assign to Qnil)
+ let mode = option_mode.unwrap_or(ruby.str_new("r").as_value());
+ let permission = option_permission.unwrap_or(ruby.qnil().as_value());
+
let operator = rb_self.blocking_op.clone();
- Io::new(ruby, operator, path, mode)
+ Io::new(ruby, operator, path, mode, permission, kwargs)
}
/// @yard
@@ -293,7 +303,7 @@ pub fn include(gem_module: &RModule) -> Result<(), Error> {
class.define_method("rename", method!(Operator::rename, 2))?;
class.define_method("remove_all", method!(Operator::remove_all, 1))?;
class.define_method("copy", method!(Operator::copy, 2))?;
- class.define_method("open", method!(Operator::open, 2))?;
+ class.define_method("open", method!(Operator::open, -1))?;
class.define_method("list", method!(Operator::list, -1))?;
class.define_method("info", method!(Operator::info, 0))?;
