From 3e4ead61a13a170518ad4568059250e5362505a0 Mon Sep 17 00:00:00 2001 From: Erick Guan Date: Tue, 22 Oct 2024 22:39:18 +0200 Subject: [PATCH 1/7] Bump Ruby bindings dependencies with minimal Ruby 3.1.3 - Ruby 3.0 is EoL. Since OpenDAL gem is fresh new, use 3.1 and up - Use a Ruby with rubygems that can compile Rust gem --- bindings/ruby/.standard.yml | 2 +- bindings/ruby/Cargo.toml | 4 ++-- bindings/ruby/Gemfile | 14 ++++++++------ bindings/ruby/Rakefile | 5 ++++- bindings/ruby/extconf.rb | 3 +++ bindings/ruby/opendal.gemspec | 24 +++++++++++++----------- 6 files changed, 31 insertions(+), 21 deletions(-) diff --git a/bindings/ruby/.standard.yml b/bindings/ruby/.standard.yml index 3450e5599330..c199ca278772 100644 --- a/bindings/ruby/.standard.yml +++ b/bindings/ruby/.standard.yml @@ -17,4 +17,4 @@ # For available configuration options, see: # https://github.com/testdouble/standard -ruby_version: 2.6 +ruby_version: 3.1 diff --git a/bindings/ruby/Cargo.toml b/bindings/ruby/Cargo.toml index 129d7a12eac2..3556720386ba 100644 --- a/bindings/ruby/Cargo.toml +++ b/bindings/ruby/Cargo.toml @@ -33,7 +33,7 @@ doc = false name = "opendal_ruby" [dependencies] -magnus = { version = "0.5", features = ["bytes-crate"] } +magnus = { version = "0.7.1", features = ["bytes"] } # this crate won't be published, we always use the local version opendal = { version = ">=0", path = "../../core", features = [ # These are default features before v0.46. TODO: change to optional features @@ -53,7 +53,7 @@ opendal = { version = ">=0", path = "../../core", features = [ "services-webhdfs", "services-azfile", ] } -rb-sys = { version = "0.9.77", default-features = false } +rb-sys = { version = "0.9.102", default-features = false } [build-dependencies] rb-sys-env = "0.1.2" diff --git a/bindings/ruby/Gemfile b/bindings/ruby/Gemfile index 0d12b266c4b6..917f2329b5bd 100644 --- a/bindings/ruby/Gemfile +++ b/bindings/ruby/Gemfile @@ -19,11 +19,13 @@ source "https://rubygems.org" -# Specify your gem's dependencies in opendal.gemspec +# Specify runtime dependencies from opendal.gemspec gemspec -gem "rake", "~> 13.0" -gem "rake-compiler", "~> 1.2.0" -gem "rspec", "~> 3.12.0" -gem "standard", "~> 1.3" -gem "rb_sys", "~> 0.9.39" +group :development, :test do + gem "rake", ">= 13.0" + gem "rb_sys", "~> 0.9.102" # for Makefile generation in extconf.rb + gem "rake-compiler", "~> 1.2.0" + gem "rspec", "~> 3.12.0" + gem "standard", "~> 1.3" +end diff --git a/bindings/ruby/Rakefile b/bindings/ruby/Rakefile index 16d56ec23286..875938c76046 100644 --- a/bindings/ruby/Rakefile +++ b/bindings/ruby/Rakefile @@ -23,9 +23,12 @@ require "rspec/core/rake_task" require "bundler/gem_tasks" require "standard/rake" +GEMSPEC = Gem::Specification.load("opendal.gemspec") +CRATE_PACKAGE_NAME = "opendal-ruby" + RSpec::Core::RakeTask.new(:spec) -Rake::ExtensionTask.new do |ext| +Rake::ExtensionTask.new(CRATE_PACKAGE_NAME, GEMSPEC) do |ext| ext.name = "opendal_ruby" ext.ext_dir = "." ext.lib_dir = "lib/opendal_ruby" diff --git a/bindings/ruby/extconf.rb b/bindings/ruby/extconf.rb index 98ec2ea83d32..7b6e02c41198 100644 --- a/bindings/ruby/extconf.rb +++ b/bindings/ruby/extconf.rb @@ -16,6 +16,9 @@ # under the License. require "mkmf" +# We use rb_sys for makefile generation only. +# We can use `RB_SYS_CARGO_PROFILE` to choose Cargo profile +# Read more https://github.com/oxidize-rb/rb-sys/blob/main/gem/README.md require "rb_sys/mkmf" create_rust_makefile("opendal_ruby/opendal_ruby") diff --git a/bindings/ruby/opendal.gemspec b/bindings/ruby/opendal.gemspec index 59c25e684b76..df716d91f6c7 100644 --- a/bindings/ruby/opendal.gemspec +++ b/bindings/ruby/opendal.gemspec @@ -27,7 +27,6 @@ Gem::Specification.new do |spec| spec.summary = "OpenDAL Ruby Binding" spec.homepage = "https://opendal.apache.org/" - spec.required_ruby_version = ">= 2.6.0" spec.metadata["allowed_push_host"] = "TODO: Set to your gem server 'https://example.com'" @@ -46,16 +45,19 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] - # Uncomment to register a new dependency of your gem - # spec.add_dependency "example-gem", "~> 1.0" spec.extensions = ["./extconf.rb"] - # needed until rubygems supports Rust support is out of beta - spec.add_dependency "rb_sys", "~> 0.9.39" - - # only needed when developing or packaging your gem - spec.add_development_dependency "rake-compiler", "~> 1.2.0" - - # For more information and examples about making a new gem, check out our - # guide at: https://bundler.io/guides/creating_gem.html + # Rubygems is a default gem that is a part of Ruby core. + # Rubygems 3.3.11 supports building gem with Cargo. + # Read more https://github.com/rubygems/rubygems/blob/master/CHANGELOG.md#3311--2022-04-07 + # + # Ruby 3.1.3 includes Rubygems 3.3.26 + # Read more https://stdgems.org/3.1.3/ + # + # use a Ruby version which: + # - supports new Rubygems with the ability of compilation of Rust gem + # - not end of life + spec.required_ruby_version = ">= 3.1.3" + + # intentionally skipping rb_sys gem because newer Rubygems will be present end From 8e8934313548994792a4681495542a6468831101 Mon Sep 17 00:00:00 2001 From: Erick Guan Date: Tue, 22 Oct 2024 22:44:47 +0200 Subject: [PATCH 2/7] Prefer minitest for simplicity --- bindings/ruby/.gitignore | 3 +-- bindings/ruby/.rspec | 3 --- bindings/ruby/Gemfile | 4 +++- bindings/ruby/Rakefile | 15 ++++++++---- .../blocking_op_test.rb} | 15 ++++++------ .../spec_helper.rb => test/test_helper.rb} | 23 +++++++++++-------- 6 files changed, 36 insertions(+), 27 deletions(-) delete mode 100644 bindings/ruby/.rspec rename bindings/ruby/{spec/blocking_op_spec.rb => test/blocking_op_test.rb} (80%) rename bindings/ruby/{spec/spec_helper.rb => test/test_helper.rb} (64%) diff --git a/bindings/ruby/.gitignore b/bindings/ruby/.gitignore index f96f9a8ab876..6661c2f1f89a 100644 --- a/bindings/ruby/.gitignore +++ b/bindings/ruby/.gitignore @@ -4,10 +4,9 @@ /coverage/ /doc/ /pkg/ -/spec/reports/ /tmp/ +/target/ *.bundle *.so -.rspec_status Gemfile.lock Cargo.lock diff --git a/bindings/ruby/.rspec b/bindings/ruby/.rspec deleted file mode 100644 index 34c5164d9b56..000000000000 --- a/bindings/ruby/.rspec +++ /dev/null @@ -1,3 +0,0 @@ ---format documentation ---color ---require spec_helper diff --git a/bindings/ruby/Gemfile b/bindings/ruby/Gemfile index 917f2329b5bd..f1acfa1e79b5 100644 --- a/bindings/ruby/Gemfile +++ b/bindings/ruby/Gemfile @@ -26,6 +26,8 @@ group :development, :test do gem "rake", ">= 13.0" gem "rb_sys", "~> 0.9.102" # for Makefile generation in extconf.rb gem "rake-compiler", "~> 1.2.0" - gem "rspec", "~> 3.12.0" + gem "minitest", "~> 5.25.0" + gem "minitest-reporters", "~> 1.7.1" + gem "activesupport", "~> 7.2.1.1" gem "standard", "~> 1.3" end diff --git a/bindings/ruby/Rakefile b/bindings/ruby/Rakefile index 875938c76046..802064805b2f 100644 --- a/bindings/ruby/Rakefile +++ b/bindings/ruby/Rakefile @@ -17,21 +17,26 @@ # frozen_string_literal: true +require "bundler/gem_tasks" require "rake/testtask" require "rake/extensiontask" -require "rspec/core/rake_task" -require "bundler/gem_tasks" require "standard/rake" GEMSPEC = Gem::Specification.load("opendal.gemspec") CRATE_PACKAGE_NAME = "opendal-ruby" -RSpec::Core::RakeTask.new(:spec) - Rake::ExtensionTask.new(CRATE_PACKAGE_NAME, GEMSPEC) do |ext| ext.name = "opendal_ruby" ext.ext_dir = "." ext.lib_dir = "lib/opendal_ruby" end -task default: %i[clobber compile spec standard] +Rake::Task[:test].prerequisites << :compile + +Rake::TestTask.new do |t| + t.libs << "lib" + t.libs << "test" + t.pattern = "test/**/*_test.rb" +end + +task default: %i[clobber compile test standard] diff --git a/bindings/ruby/spec/blocking_op_spec.rb b/bindings/ruby/test/blocking_op_test.rb similarity index 80% rename from bindings/ruby/spec/blocking_op_spec.rb rename to bindings/ruby/test/blocking_op_test.rb index 8d555ddf4534..07793f75fd2b 100644 --- a/bindings/ruby/spec/blocking_op_spec.rb +++ b/bindings/ruby/test/blocking_op_test.rb @@ -17,20 +17,21 @@ # frozen_string_literal: true -RSpec.describe OpenDAL do - before :each do +require "test_helper" + +class OpenDalTest < ActiveSupport::TestCase + setup do @op = OpenDAL::Operator.new("memory", nil) end - it "should perform basic ops" do + test "should perform basic ops" do path = "/path/to/file" content = "OpenDAL Ruby is ready." @op.write(path, content) stat = @op.stat(path) - expect(stat.is_file).to eq(true) - expect(stat.content_length).to eq(content.length) - - expect(@op.read(path)).to eq(content) + assert stat.is_file + assert_equal content.length, stat.content_length + assert_equal content, @op.read(path), content end end diff --git a/bindings/ruby/spec/spec_helper.rb b/bindings/ruby/test/test_helper.rb similarity index 64% rename from bindings/ruby/spec/spec_helper.rb rename to bindings/ruby/test/test_helper.rb index 16a653f4ca15..b10afd56aa8a 100644 --- a/bindings/ruby/spec/spec_helper.rb +++ b/bindings/ruby/test/test_helper.rb @@ -17,16 +17,21 @@ # frozen_string_literal: true -require "opendal" +require "active_support" +require "minitest/autorun" +require "minitest/reporters" -RSpec.configure do |config| - # Enable flags like --only-failures and --next-failure - config.example_status_persistence_file_path = ".rspec_status" +Minitest::Reporters.use!([Minitest::Reporters::DefaultReporter.new(:color => true)]) - # Disable RSpec exposing methods globally on `Module` and `main` - config.disable_monkey_patching! +require "opendal" - config.expect_with :rspec do |c| - c.syntax = :expect - end +# Uses `ActiveSupport::TestCase` for additional features including: +# - additional assertions +# - file fixtures +# - parallel worker +# +# Read more https://edgeapi.rubyonrails.org/classes/ActiveSupport/TestCase.html +class ActiveSupport::TestCase + parallelize(workers: :number_of_processors) end + From d14fec9a9ead7cbe7ae33ce1ccfa65a57a32f5b4 Mon Sep 17 00:00:00 2001 From: Erick Guan <297343+erickguan@users.noreply.github.com> Date: Sat, 26 Oct 2024 10:51:40 +0200 Subject: [PATCH 3/7] Put Ruby class implementation in Rust modules --- bindings/ruby/Cargo.toml | 2 + bindings/ruby/Gemfile | 4 +- bindings/ruby/README.md | 4 +- bindings/ruby/src/lib.rs | 120 +++---------------------- bindings/ruby/src/metadata.rs | 86 ++++++++++++++++++ bindings/ruby/src/operator.rs | 86 ++++++++++++++++++ bindings/ruby/test/blocking_op_test.rb | 2 +- bindings/ruby/test/test_helper.rb | 3 +- 8 files changed, 192 insertions(+), 115 deletions(-) create mode 100644 bindings/ruby/src/metadata.rs create mode 100644 bindings/ruby/src/operator.rs diff --git a/bindings/ruby/Cargo.toml b/bindings/ruby/Cargo.toml index 3556720386ba..c5f30b38418b 100644 --- a/bindings/ruby/Cargo.toml +++ b/bindings/ruby/Cargo.toml @@ -54,6 +54,8 @@ opendal = { version = ">=0", path = "../../core", features = [ "services-azfile", ] } rb-sys = { version = "0.9.102", default-features = false } +# should be in sync with opendal +bytes = "1.6" [build-dependencies] rb-sys-env = "0.1.2" diff --git a/bindings/ruby/Gemfile b/bindings/ruby/Gemfile index f1acfa1e79b5..5f8523302722 100644 --- a/bindings/ruby/Gemfile +++ b/bindings/ruby/Gemfile @@ -19,7 +19,7 @@ source "https://rubygems.org" -# Specify runtime dependencies from opendal.gemspec +# Includes runtime dependencies from opendal.gemspec gemspec group :development, :test do @@ -28,6 +28,6 @@ group :development, :test do gem "rake-compiler", "~> 1.2.0" gem "minitest", "~> 5.25.0" gem "minitest-reporters", "~> 1.7.1" - gem "activesupport", "~> 7.2.1.1" + gem "activesupport", "~> 7.2.1" gem "standard", "~> 1.3" end diff --git a/bindings/ruby/README.md b/bindings/ruby/README.md index f45b455c312a..334fa8936abc 100644 --- a/bindings/ruby/README.md +++ b/bindings/ruby/README.md @@ -17,13 +17,13 @@ bundle Build bindings: ```shell -rake compile +bundle exec rake compile ``` Run tests: ```shell -rake spec +bundle exec rake spec ``` ## License and Trademarks diff --git a/bindings/ruby/src/lib.rs b/bindings/ruby/src/lib.rs index c47e9af88a5a..7bdffc2bb829 100644 --- a/bindings/ruby/src/lib.rs +++ b/bindings/ruby/src/lib.rs @@ -15,124 +15,28 @@ // specific language governing permissions and limitations // under the License. -use std::collections::HashMap; -use std::str::FromStr; - -use magnus::class; -use magnus::define_module; -use magnus::error::Result; use magnus::exception; use magnus::function; -use magnus::method; -use magnus::prelude::*; use magnus::Error; -use magnus::RString; -use opendal as od; - -#[magnus::wrap(class = "OpenDAL::Operator", free_immediately, size)] -#[derive(Clone, Debug)] -pub struct Operator(od::BlockingOperator); - -impl Operator { - pub fn new(scheme: String, options: Option>) -> Result { - let scheme = od::Scheme::from_str(&scheme) - .map_err(|err| { - od::Error::new(od::ErrorKind::Unexpected, "unsupported scheme").set_source(err) - }) - .map_err(format_magnus_error)?; - let options = options.unwrap_or_default(); - - let op = od::Operator::via_iter(scheme, options) - .map_err(format_magnus_error)? - .blocking(); - Ok(Operator(op)) - } - - /// Read the whole path into string. - pub fn read(&self, path: String) -> Result { - let bytes = self.0.read(&path).map_err(format_magnus_error)?; - Ok(RString::from_slice(&bytes.to_vec())) - } - - /// Write string into given path. - pub fn write(&self, path: String, bs: RString) -> Result<()> { - self.0 - .write(&path, bs.to_bytes()) - .map_err(format_magnus_error) - } - - /// Get current path's metadata **without cache** directly. - pub fn stat(&self, path: String) -> Result { - self.0 - .stat(&path) - .map_err(format_magnus_error) - .map(Metadata) - } -} +use magnus::Ruby; -#[magnus::wrap(class = "OpenDAL::Metadata", free_immediately, size)] -pub struct Metadata(od::Metadata); +// We will use `ocore::` to represents opendal rust core functionalities. +// This convention aligns with the Python binding. +pub use ::opendal as ocore; -impl Metadata { - /// Content-Disposition of this object - pub fn content_disposition(&self) -> Option<&str> { - self.0.content_disposition() - } - - /// Content length of this entry. - pub fn content_length(&self) -> u64 { - self.0.content_length() - } - - /// Content MD5 of this entry. - pub fn content_md5(&self) -> Option<&str> { - self.0.content_md5() - } - - /// Content Type of this entry. - pub fn content_type(&self) -> Option<&str> { - self.0.content_type() - } - - /// ETag of this entry. - pub fn etag(&self) -> Option<&str> { - self.0.etag() - } - - /// Returns `True` if this is a file. - pub fn is_file(&self) -> bool { - self.0.is_file() - } - - /// Returns `True` if this is a directory. - pub fn is_dir(&self) -> bool { - self.0.is_dir() - } -} +mod metadata; +mod operator; -fn format_magnus_error(err: od::Error) -> Error { +pub fn format_magnus_error(err: ocore::Error) -> Error { Error::new(exception::runtime_error(), err.to_string()) } +/// Apache OpenDAL™ Ruby binding #[magnus::init] -fn init() -> Result<()> { - let namespace = define_module("OpenDAL")?; - let operator_class = namespace.define_class("Operator", class::object())?; - operator_class.define_singleton_method("new", function!(Operator::new, 2))?; - operator_class.define_method("read", method!(Operator::read, 1))?; - operator_class.define_method("write", method!(Operator::write, 2))?; - operator_class.define_method("stat", method!(Operator::stat, 1))?; +fn init(ruby: &Ruby) -> Result<(), Error> { + let gem_module = ruby.define_module("OpenDAL")?; + let _ = operator::include(&gem_module); + let _ = metadata::include(&gem_module); - let metadata_class = namespace.define_class("Metadata", class::object())?; - metadata_class.define_method( - "content_disposition", - method!(Metadata::content_disposition, 0), - )?; - metadata_class.define_method("content_length", method!(Metadata::content_length, 0))?; - metadata_class.define_method("content_md5", method!(Metadata::content_md5, 0))?; - metadata_class.define_method("content_type", method!(Metadata::content_type, 0))?; - metadata_class.define_method("etag", method!(Metadata::etag, 0))?; - metadata_class.define_method("is_file", method!(Metadata::is_file, 0))?; - metadata_class.define_method("is_dir", method!(Metadata::is_dir, 0))?; Ok(()) } diff --git a/bindings/ruby/src/metadata.rs b/bindings/ruby/src/metadata.rs new file mode 100644 index 000000000000..c9e495772811 --- /dev/null +++ b/bindings/ruby/src/metadata.rs @@ -0,0 +1,86 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use magnus::class; +use magnus::method; +use magnus::prelude::*; +use magnus::Error; +use magnus::RModule; + +use crate::*; + +#[magnus::wrap(class = "OpenDAL::Metadata", free_immediately, size)] +pub struct Metadata(ocore::Metadata); + +impl Metadata { + pub fn new(metadata: ocore::Metadata) -> Self { + Self(metadata) + } +} + +impl Metadata { + /// Content-Disposition of this object + pub fn content_disposition(&self) -> Option<&str> { + self.0.content_disposition() + } + + /// Content length of this entry. + pub fn content_length(&self) -> u64 { + self.0.content_length() + } + + /// Content MD5 of this entry. + pub fn content_md5(&self) -> Option<&str> { + self.0.content_md5() + } + + /// Content Type of this entry. + pub fn content_type(&self) -> Option<&str> { + self.0.content_type() + } + + /// ETag of this entry. + pub fn etag(&self) -> Option<&str> { + self.0.etag() + } + + /// Returns `True` if this is a file. + pub fn is_file(&self) -> bool { + self.0.is_file() + } + + /// Returns `True` if this is a directory. + pub fn is_dir(&self) -> bool { + self.0.is_dir() + } +} + +pub fn include(gem_module: &RModule) -> Result<(), Error> { + let class = gem_module.define_class("Metadata", class::object())?; + class.define_method( + "content_disposition", + method!(Metadata::content_disposition, 0), + )?; + class.define_method("content_length", method!(Metadata::content_length, 0))?; + class.define_method("content_md5", method!(Metadata::content_md5, 0))?; + class.define_method("content_type", method!(Metadata::content_type, 0))?; + class.define_method("etag", method!(Metadata::etag, 0))?; + class.define_method("file?", method!(Metadata::is_file, 0))?; + class.define_method("dir?", method!(Metadata::is_dir, 0))?; + + Ok(()) +} diff --git a/bindings/ruby/src/operator.rs b/bindings/ruby/src/operator.rs new file mode 100644 index 000000000000..48a8da34b5e8 --- /dev/null +++ b/bindings/ruby/src/operator.rs @@ -0,0 +1,86 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::collections::HashMap; +use std::str::FromStr; + +use magnus::class; +use magnus::exception; +use magnus::method; +use magnus::prelude::*; +use magnus::Error; +use magnus::RModule; +use magnus::RString; + +use crate::metadata::Metadata; +use crate::*; + +#[magnus::wrap(class = "OpenDAL::Operator", free_immediately, size)] +#[derive(Clone, Debug)] +struct Operator(ocore::BlockingOperator); + +fn format_magnus_error(err: ocore::Error) -> Error { + Error::new(exception::runtime_error(), err.to_string()) +} + +impl Operator { + fn new(scheme: String, options: Option>) -> Result { + let scheme = ocore::Scheme::from_str(&scheme) + .map_err(|err| { + ocore::Error::new(ocore::ErrorKind::Unexpected, "unsupported scheme") + .set_source(err) + }) + .map_err(format_magnus_error)?; + let options = options.unwrap_or_default(); + + let op = ocore::Operator::via_iter(scheme, options) + .map_err(format_magnus_error)? + .blocking(); + Ok(Operator(op)) + } + + /// Reads the whole path into string. + fn read(&self, path: String) -> Result { + let buffer = self.0.read(&path).map_err(format_magnus_error)?; + Ok(buffer.to_bytes()) + } + + /// Writes string into given path. + fn write(&self, path: String, bs: RString) -> Result<(), Error> { + self.0 + .write(&path, bs.to_bytes()) + .map_err(format_magnus_error) + } + + /// Gets current path's metadata **without cache** directly. + fn stat(&self, path: String) -> Result { + self.0 + .stat(&path) + .map_err(format_magnus_error) + .map(Metadata::new) + } +} + +pub fn include(gem_module: &RModule) -> Result<(), Error> { + let class = gem_module.define_class("Operator", class::object())?; + class.define_singleton_method("new", function!(Operator::new, 2))?; + class.define_method("read", method!(Operator::read, 1))?; + class.define_method("write", method!(Operator::write, 2))?; + class.define_method("stat", method!(Operator::stat, 1))?; + + Ok(()) +} diff --git a/bindings/ruby/test/blocking_op_test.rb b/bindings/ruby/test/blocking_op_test.rb index 07793f75fd2b..ef9214912ca1 100644 --- a/bindings/ruby/test/blocking_op_test.rb +++ b/bindings/ruby/test/blocking_op_test.rb @@ -30,7 +30,7 @@ class OpenDalTest < ActiveSupport::TestCase @op.write(path, content) stat = @op.stat(path) - assert stat.is_file + assert stat.file? assert_equal content.length, stat.content_length assert_equal content, @op.read(path), content end diff --git a/bindings/ruby/test/test_helper.rb b/bindings/ruby/test/test_helper.rb index b10afd56aa8a..10ae46e8b5fb 100644 --- a/bindings/ruby/test/test_helper.rb +++ b/bindings/ruby/test/test_helper.rb @@ -21,7 +21,7 @@ require "minitest/autorun" require "minitest/reporters" -Minitest::Reporters.use!([Minitest::Reporters::DefaultReporter.new(:color => true)]) +Minitest::Reporters.use!([Minitest::Reporters::DefaultReporter.new(color: true)]) require "opendal" @@ -34,4 +34,3 @@ class ActiveSupport::TestCase parallelize(workers: :number_of_processors) end - From f86323e18aab1273562a87cd5d8b93abc33a65e5 Mon Sep 17 00:00:00 2001 From: Erick Guan <297343+erickguan@users.noreply.github.com> Date: Sat, 26 Oct 2024 15:15:52 +0200 Subject: [PATCH 4/7] Add capability in Ruby binding --- bindings/ruby/src/capability.rs | 156 ++++++++++++++++++++++++++ bindings/ruby/src/lib.rs | 2 + bindings/ruby/src/operator.rs | 8 ++ bindings/ruby/test/capability_test.rb | 42 +++++++ 4 files changed, 208 insertions(+) create mode 100644 bindings/ruby/src/capability.rs create mode 100644 bindings/ruby/test/capability_test.rb diff --git a/bindings/ruby/src/capability.rs b/bindings/ruby/src/capability.rs new file mode 100644 index 000000000000..01a9676b2a97 --- /dev/null +++ b/bindings/ruby/src/capability.rs @@ -0,0 +1,156 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use magnus::class; +use magnus::method; +use magnus::prelude::*; +use magnus::Error; +use magnus::RModule; + +use crate::*; + +// This name follows `attr_accessor` in Ruby +macro_rules! define_accessors { + ($struct:ty, { $( $field:ident : $type:ty ),+ $(,)? }) => { + impl $struct { + $( + pub fn $field(&self) -> $type { + self.0.$field + } + )+ + } + }; +} +macro_rules! bind_methods_to_ruby { + ($ruby_class:ident, { $( $field:ident ),+ $(,)? }) => { + $( + $ruby_class.define_method(stringify!($field), method!(Capability::$field, 0))?; + )+ + }; +} + +/// Capability describes OpenDAL supported operations by current Operator. +#[magnus::wrap(class = "OpenDAL::Capability", free_immediately, size)] +pub struct Capability(ocore::Capability); + +impl Capability { + pub fn new(capability: ocore::Capability) -> Self { + Self(capability) + } +} + +define_accessors!(Capability, { + stat: bool, + stat_with_if_match: bool, + stat_with_if_none_match: bool, + stat_with_override_cache_control: bool, + stat_with_override_content_disposition: bool, + stat_with_override_content_type: bool, + stat_with_version: bool, + read: bool, + read_with_if_match: bool, + read_with_if_none_match: bool, + read_with_override_cache_control: bool, + read_with_override_content_disposition: bool, + read_with_override_content_type: bool, + read_with_version: bool, + write: bool, + write_can_multi: bool, + write_can_empty: bool, + write_can_append: bool, + write_with_content_type: bool, + write_with_content_disposition: bool, + write_with_cache_control: bool, + write_with_if_none_match: bool, + write_with_user_metadata: bool, + write_multi_max_size: Option, + write_multi_min_size: Option, + write_multi_align_size: Option, + write_total_max_size: Option, + create_dir: bool, + delete: bool, + delete_with_version: bool, + copy: bool, + rename: bool, + list: bool, + list_with_limit: bool, + list_with_start_after: bool, + list_with_recursive: bool, + list_with_version: bool, + presign: bool, + presign_read: bool, + presign_stat: bool, + presign_write: bool, + batch: bool, + batch_delete: bool, + batch_max_operations: Option, + blocking: bool +}); + +// includes class into the Ruby module +pub fn include(gem_module: &RModule) -> Result<(), Error> { + let class = gem_module.define_class("Capability", class::object())?; + bind_methods_to_ruby!(class, { + stat, + stat_with_if_match, + stat_with_if_none_match, + stat_with_override_cache_control, + stat_with_override_content_disposition, + stat_with_override_content_type, + stat_with_version, + read, + read_with_if_match, + read_with_if_none_match, + read_with_override_cache_control, + read_with_override_content_disposition, + read_with_override_content_type, + read_with_version, + write, + write_can_multi, + write_can_empty, + write_can_append, + write_with_content_type, + write_with_content_disposition, + write_with_cache_control, + write_with_if_none_match, + write_with_user_metadata, + write_multi_max_size, + write_multi_min_size, + write_multi_align_size, + write_total_max_size, + create_dir, + delete, + delete_with_version, + copy, + rename, + list, + list_with_limit, + list_with_start_after, + list_with_recursive, + list_with_version, + presign, + presign_read, + presign_stat, + presign_write, + batch, + batch_delete, + batch_max_operations, + blocking + }); + + Ok(()) +} diff --git a/bindings/ruby/src/lib.rs b/bindings/ruby/src/lib.rs index 7bdffc2bb829..5de42706927a 100644 --- a/bindings/ruby/src/lib.rs +++ b/bindings/ruby/src/lib.rs @@ -24,6 +24,7 @@ use magnus::Ruby; // This convention aligns with the Python binding. pub use ::opendal as ocore; +mod capability; mod metadata; mod operator; @@ -37,6 +38,7 @@ fn init(ruby: &Ruby) -> Result<(), Error> { let gem_module = ruby.define_module("OpenDAL")?; let _ = operator::include(&gem_module); let _ = metadata::include(&gem_module); + let _ = capability::include(&gem_module); Ok(()) } diff --git a/bindings/ruby/src/operator.rs b/bindings/ruby/src/operator.rs index 48a8da34b5e8..11517c7a7133 100644 --- a/bindings/ruby/src/operator.rs +++ b/bindings/ruby/src/operator.rs @@ -26,6 +26,7 @@ use magnus::Error; use magnus::RModule; use magnus::RString; +use crate::capability::Capability; use crate::metadata::Metadata; use crate::*; @@ -73,6 +74,12 @@ impl Operator { .map_err(format_magnus_error) .map(Metadata::new) } + + /// Gets capabilities of the current operator + fn capability(&self) -> Result { + let capability = self.0.info().full_capability(); + Ok(Capability::new(capability)) + } } pub fn include(gem_module: &RModule) -> Result<(), Error> { @@ -81,6 +88,7 @@ pub fn include(gem_module: &RModule) -> Result<(), Error> { class.define_method("read", method!(Operator::read, 1))?; class.define_method("write", method!(Operator::write, 2))?; class.define_method("stat", method!(Operator::stat, 1))?; + class.define_method("capability", method!(Operator::capability, 0))?; Ok(()) } diff --git a/bindings/ruby/test/capability_test.rb b/bindings/ruby/test/capability_test.rb new file mode 100644 index 000000000000..f1bf43fe4d70 --- /dev/null +++ b/bindings/ruby/test/capability_test.rb @@ -0,0 +1,42 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# frozen_string_literal: true + +require "test_helper" + +class CapabilityTest < ActiveSupport::TestCase + setup do + @op = OpenDAL::Operator.new("memory", nil) + end + + test "has read capability" do + capability = @op.capability + + assert_not capability.nil? + assert capability.read + end + + test "doesn't respond to undefined capability" do + capability = @op.capability + + assert_not capability.nil? + assert_raises(NoMethodError) do + capability.not_exist + end + end +end From 39ea07e1d14d61488b5bfa873d78802172778892 Mon Sep 17 00:00:00 2001 From: Erick Guan <297343+erickguan@users.noreply.github.com> Date: Sat, 26 Oct 2024 15:28:45 +0200 Subject: [PATCH 5/7] Add more operations: - create_dir - delete - exist? - rename - remove_all - copy Also refactor tests with Ruby stdlib --- bindings/ruby/src/operator.rs | 37 +++++++++++++ bindings/ruby/test/blocking_op_test.rb | 77 +++++++++++++++++++++++--- 2 files changed, 106 insertions(+), 8 deletions(-) diff --git a/bindings/ruby/src/operator.rs b/bindings/ruby/src/operator.rs index 11517c7a7133..2a07d68b2c70 100644 --- a/bindings/ruby/src/operator.rs +++ b/bindings/ruby/src/operator.rs @@ -80,6 +80,37 @@ impl Operator { let capability = self.0.info().full_capability(); Ok(Capability::new(capability)) } + + /// Creates directory recursively similar as `mkdir -p` + /// The ending path must be `/`. Otherwise, OpenDAL throws `NotADirectory` error. + fn create_dir(&self, path: String) -> Result<(), Error> { + self.0.create_dir(&path).map_err(format_magnus_error) + } + + /// Deletes given path + fn delete(&self, path: String) -> Result<(), Error> { + self.0.delete(&path).map_err(format_magnus_error) + } + + /// Returns if this path exists + fn exists(&self, path: String) -> Result { + self.0.exists(&path).map_err(format_magnus_error) + } + + /// Renames a file from `from` to `to` + fn rename(&self, from: String, to: String) -> Result<(), Error> { + self.0.rename(&from, &to).map_err(format_magnus_error) + } + + /// Removes the path and all nested directories and files recursively + fn remove_all(&self, path: String) -> Result<(), Error> { + self.0.remove_all(&path).map_err(format_magnus_error) + } + + /// Copies a file from `from` to `to`. + fn copy(&self, from: String, to: String) -> Result<(), Error> { + self.0.copy(&from, &to).map_err(format_magnus_error) + } } pub fn include(gem_module: &RModule) -> Result<(), Error> { @@ -89,6 +120,12 @@ pub fn include(gem_module: &RModule) -> Result<(), Error> { class.define_method("write", method!(Operator::write, 2))?; class.define_method("stat", method!(Operator::stat, 1))?; class.define_method("capability", method!(Operator::capability, 0))?; + class.define_method("create_dir", method!(Operator::create_dir, 1))?; + class.define_method("delete", method!(Operator::delete, 1))?; + class.define_method("exist?", method!(Operator::exists, 1))?; + 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))?; Ok(()) } diff --git a/bindings/ruby/test/blocking_op_test.rb b/bindings/ruby/test/blocking_op_test.rb index ef9214912ca1..b6cee4d716c4 100644 --- a/bindings/ruby/test/blocking_op_test.rb +++ b/bindings/ruby/test/blocking_op_test.rb @@ -18,20 +18,81 @@ # frozen_string_literal: true require "test_helper" +require "tmpdir" class OpenDalTest < ActiveSupport::TestCase setup do - @op = OpenDAL::Operator.new("memory", nil) + @root = Dir.mktmpdir + File.write("#{@root}/sample", "Sample data for testing") + @op = OpenDAL::Operator.new("fs", {"root" => @root}) end - test "should perform basic ops" do - path = "/path/to/file" - content = "OpenDAL Ruby is ready." - @op.write(path, content) + teardown do + FileUtils.remove_entry(@root) if File.exist?(@root) + end + + test "write writes to a file" do + @op.write("/file", "OpenDAL Ruby is ready.") + + assert_equal "OpenDAL Ruby is ready.", File.read("/#{@root}/file") + end + + test "write writes binary" do + # writes 32-bit signed integers + @op.write("/file", [67305985, -50462977].pack("l*")) + + assert_equal [67305985, -50462977], File.binread("/#{@root}/file").unpack("l*") + end - stat = @op.stat(path) + test "read reads file data" do + data = @op.read("/sample") + + assert_equal "Sample data for testing", data + end + + test "stat returns file metadata" do + stat = @op.stat("sample") + + assert stat.is_a?(OpenDAL::Metadata) + assert_equal 23, stat.content_length assert stat.file? - assert_equal content.length, stat.content_length - assert_equal content, @op.read(path), content + end + + test "create_dir creates directory" do + @op.create_dir("/new/directory/") + assert File.directory?("#{@root}/new/directory/") + end + + test "exists returns existence" do + assert @op.exist?("sample") + end + + test "delete removes file" do + @op.delete("/sample") + + assert_not File.exist?("#{@root}/sample") + end + + test "rename renames file" do + @op.rename("/sample", "/new_name") + + assert_not File.exist?("#{@root}/sample") + assert File.exist?("#{@root}/new_name") + end + + test "remove_all removes files" do + @op.create_dir("/nested/directory/") + @op.write("/nested/directory/text", "content") + + @op.remove_all("/") + + assert_not File.exist?(@root) + end + + test "copy copies file" do + @op.copy("/sample", "/new_name") + + assert File.exist?("#{@root}/sample") + assert File.exist?("#{@root}/new_name") end end From efe2e27eb3b7246e51282821662e5d8e183ba7f7 Mon Sep 17 00:00:00 2001 From: Erick Guan <297343+erickguan@users.noreply.github.com> Date: Sat, 26 Oct 2024 21:39:44 +0200 Subject: [PATCH 6/7] Test supported Rubies on GitHub Action --- .github/workflows/ci_bindings_ruby.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci_bindings_ruby.yml b/.github/workflows/ci_bindings_ruby.yml index 5dcaf608de0f..13238e65c834 100644 --- a/.github/workflows/ci_bindings_ruby.yml +++ b/.github/workflows/ci_bindings_ruby.yml @@ -42,17 +42,27 @@ permissions: jobs: test: runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + ruby-version: ["3.1", "3.2", "3.3"] + env: # $BUNDLE_GEMFILE must be set at the job level, so it is set for all steps BUNDLE_GEMFILE: ${{ github.workspace }}/bindings/ruby/Gemfile + steps: - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@v1 with: - ruby-version: '3.1' + ruby-version: ${{ matrix.ruby-version }} bundler-cache: true # runs 'bundle install' and caches installed gems automatically + - name: Setup Rust toolchain uses: ./.github/actions/setup - - name: Bundle with rake - working-directory: "bindings/ruby" + + - name: Run tests and lint + working-directory: bindings/ruby run: | bundle exec rake From eb472ced8af5595fa308e3fa8f74a378753eeb22 Mon Sep 17 00:00:00 2001 From: Erick Guan <297343+erickguan@users.noreply.github.com> Date: Tue, 5 Nov 2024 10:53:13 +0100 Subject: [PATCH 7/7] fixup! Add more operations: --- bindings/ruby/src/operator.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/bindings/ruby/src/operator.rs b/bindings/ruby/src/operator.rs index 2a07d68b2c70..997b55a607b6 100644 --- a/bindings/ruby/src/operator.rs +++ b/bindings/ruby/src/operator.rs @@ -19,7 +19,6 @@ use std::collections::HashMap; use std::str::FromStr; use magnus::class; -use magnus::exception; use magnus::method; use magnus::prelude::*; use magnus::Error; @@ -34,10 +33,6 @@ use crate::*; #[derive(Clone, Debug)] struct Operator(ocore::BlockingOperator); -fn format_magnus_error(err: ocore::Error) -> Error { - Error::new(exception::runtime_error(), err.to_string()) -} - impl Operator { fn new(scheme: String, options: Option>) -> Result { let scheme = ocore::Scheme::from_str(&scheme)