Skip to content

优化代码，增加 Ruby4 CI #245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OuYangJinTing wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

优化代码，增加 Ruby4 CI #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/test-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
strategy:
fail-fast: false
matrix:
ruby-versions: ['2.4', '2.5', '2.6', '2.7', '3.0', '3.1', '3.2', 'ruby-head', 'jruby-head']
ruby-versions: ['2.4', '2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '4.0', 'ruby-head', 'jruby-head']

runs-on: ubuntu-latest
steps:
Expand All @@ -24,12 +24,12 @@ jobs:

- name: Test
run: bundle exec rspec

env:
QINIU_ACCESS_KEY: ${{ secrets.QINIU_ACCESS_KEY }}
QINIU_SECRET_KEY: ${{ secrets.QINIU_SECRET_KEY }}
QINIU_TEST_BUCKET: ${{ secrets.QINIU_TEST_BUCKET }}
QINIU_TEST_DOMAIN: ${{ secrets.QINIU_TEST_DOMAIN }}
BUCKET: ${{ secrets.BUCKET }}
BUCKET_NA0: ${{ secrets.BUCKET_NA0 }}
BUCKET_BC: ${{ secrets.BUCKET_BC }}

- name: After_success
Copy link
Copy Markdown

@fennoai fennoai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR switches the test stack from codecov to simplecov-cobertura, but CI still runs bash <(curl -s https://codecov.io/bash). That means the workflow still depends on the deprecated Codecov uploader, and the newly generated Cobertura report is not actually consumed anywhere. If the intent is to stop relying on Codecov, this step needs to be updated or removed as part of the same change.

run: bash <(curl -s https://codecov.io/bash)
52 changes: 24 additions & 28 deletions lib/qiniu/auth.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,6 @@ module Auth
AUTHORIZATION_PREFIX_QBOX = 'QBox '.freeze
AUTHORIZATION_PREFIX_QINIU = 'Qiniu '.freeze

class << self
def calculate_deadline(expires_in, deadline = nil)
### 授权期计算
if expires_in.is_a?(Integer) && expires_in > 0 then
# 指定相对时间，单位：秒
return Time.now.to_i + expires_in
elsif deadline.is_a?(Integer) then
# 指定绝对时间，常用于调试和单元测试
return deadline
end

# 默认授权期1小时
return Time.now.to_i + DEFAULT_AUTH_SECONDS
end # calculate_deadline

def calculate_hmac_sha1_digest(sk, str)
raise ArgumentError, "Please set Qiniu's access_key and secret_key before authorize any tokens." if sk.nil?
OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), sk, str)
end
end # class << self

class PutPolicy
private
def initialize(bucket,
Expand Down Expand Up @@ -96,13 +75,11 @@ def scope!(bucket, key = nil)
@scope = "#{bucket}:#{key}"
end

if Config.settings[:multi_region]
begin
@uphosts = Config.host_manager.up_hosts(bucket)
@global = Config.host_manager.global(bucket)
rescue
# Do nothing
end
begin
@uphosts = Config.host_manager.up_hosts(bucket)
@global = Config.host_manager.global(bucket)
rescue
# Do nothing
end
end # scope!

Expand Down Expand Up @@ -162,6 +139,25 @@ def to_json
class << self
EMPTY_ARGS = {}

def calculate_deadline(expires_in, deadline = nil)
### 授权期计算
if expires_in.is_a?(Integer) && expires_in > 0 then
# 指定相对时间，单位：秒
return Time.now.to_i + expires_in
elsif deadline.is_a?(Integer) then
# 指定绝对时间，常用于调试和单元测试
return deadline
end

# 默认授权期1小时
return Time.now.to_i + DEFAULT_AUTH_SECONDS
end # calculate_deadline

def calculate_hmac_sha1_digest(sk, str)
raise ArgumentError, "Please set Qiniu's access_key and secret_key before authorize any tokens." if sk.nil?
OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), sk, str)
end

### 生成下载授权URL
def authorize_download_url(url, args = EMPTY_ARGS)
### 提取AK/SK信息
Expand Down
3 changes: 1 addition & 2 deletions lib/qiniu/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ def default_options
:block_size => 1024*1024*4,
:chunk_size => 1024*256,
:enable_debug => true,
:tmpdir => Dir.tmpdir + File::SEPARATOR + 'QiniuRuby',
:multi_region => true
:tmpdir => Dir.tmpdir + File::SEPARATOR + 'QiniuRuby'
}.freeze
end

Expand Down
40 changes: 10 additions & 30 deletions lib/qiniu/host_manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,41 +13,25 @@ def initialize(config)
end

def up_host(bucket, opts = {})
if !multi_region_support?
"#{extract_protocol(opts)}://up.qiniup.com"
elsif bucket
host = hosts(bucket)
"#{extract_protocol(opts)}://" + host.dig('up', 'acc', 'main', 0) rescue "#{extract_protocol(opts)}://" + host.dig('up', 'src', 'main', 0)
end
host = hosts(bucket)
Copy link
Copy Markdown

@fennoai fennoai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up_host and fetch_host used to short-circuit when bucket was nil (elsif bucket in the old implementation). After this change they always call hosts(bucket), so Config.up_host(nil)/fetch_host(nil) now issues a UC query with an empty bucket and raises Host query is failed instead of returning nil (or the legacy fixed host when multi_region was disabled). Since these methods are public API, this is a breaking behavior change unless you add an explicit guard or raise a clearer argument error.

"#{extract_protocol(opts)}://" + host.dig('up', 'acc', 'main', 0) rescue "#{extract_protocol(opts)}://" + host.dig('up', 'src', 'main', 0)
end
Comment on lines 15 to 18
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

移除 bucket 检查并简化逻辑引入了回归风险和潜在的运行时错误：

  1. 回归风险：以前如果 bucket 为 nil，该方法会返回 nil。现在它会调用 hosts(nil)，这可能会触发 API 错误并抛出异常。
  2. 脆弱的 Rescue 逻辑：行内的 rescue 仅能处理第一个 dig 返回 nil 时（导致与字符串相加产生 TypeError）的情况。如果第二个 dig 也返回 nil，产生的 TypeError 将不会被捕获，从而导致程序崩溃。
  3. 健壮性：建议使用字符串插值并提供显式的回退逻辑，这样更安全且代码意图更清晰。
    def up_host(bucket, opts = {})
      return nil if bucket.nil?
      host = hosts(bucket)
      up_host = host.dig('up', 'acc', 'main', 0) || host.dig('up', 'src', 'main', 0)
      "#{extract_protocol(opts)}://#{up_host}" if up_host
    end


def fetch_host(bucket, opts = {})
if !multi_region_support?
"#{extract_protocol(opts)}://iovip.qbox.me"
elsif bucket
host = hosts(bucket)
"#{extract_protocol(opts)}://" + host.dig('io', 'acc', 'main', 0) rescue "#{extract_protocol(opts)}://" + host.dig('io', 'src', 'main', 0)
end
host = hosts(bucket)
"#{extract_protocol(opts)}://" + host.dig('io', 'acc', 'main', 0) rescue "#{extract_protocol(opts)}://" + host.dig('io', 'src', 'main', 0)
end
Comment on lines 20 to 23
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

与 up_host 类似，此方法现在缺少对 bucket 是否为 nil 的检查，并且使用了脆弱的 rescue 逻辑。如果 API 响应中同时缺少 acc 和 src 主机配置，程序将会崩溃。

    def fetch_host(bucket, opts = {})
      return nil if bucket.nil?
      host = hosts(bucket)
      fetch_host = host.dig('io', 'acc', 'main', 0) || host.dig('io', 'src', 'main', 0)
      "#{extract_protocol(opts)}://#{fetch_host}" if fetch_host
    end


def up_hosts(bucket, opts = {})
if multi_region_support?
host = hosts(bucket)['up']
multi_region_hosts = []
multi_region_hosts |= host.dig('acc', 'main') || []
multi_region_hosts |= host.dig('src', 'main') || []
return multi_region_hosts
else
raise 'HostManager#up_hosts: multi_region must be enabled'
end
host = hosts(bucket)['up']
up_hosts = []
up_hosts |= host.dig('acc', 'main') || []
up_hosts |= host.dig('src', 'main') || []
up_hosts
end
Comment on lines 25 to 31
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

如果 hosts(bucket) 的响应不包含 'up' 键，或者 bucket 为 nil，此方法会抛出 NoMethodError（因为 host 为 nil）。建议增加卫语句并为 host 变量提供默认的空哈希以提高代码的健壮性。

    def up_hosts(bucket, opts = {})
      return [] if bucket.nil?
      host = hosts(bucket)['up'] || {}
      up_hosts = []
      up_hosts |= host.dig('acc', 'main') || []
      up_hosts |= host.dig('src', 'main') || []
      up_hosts
    end


def global(bucket, opts = {})
if multi_region_support?
!!hosts(bucket)['global']
else
raise 'HostManager#global: multi_region must be enabled'
end
!!hosts(bucket)['global']
end
Comment on lines 33 to 35
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

建议为 bucket 增加卫语句，以避免在缺少存储桶名称时进行不必要的 API 调用，并防止潜在的异常抛出。

    def global(bucket, opts = {})
      return false if bucket.nil?
      !!hosts(bucket)['global']
    end


private
Expand All @@ -56,10 +40,6 @@ def extract_protocol(opts)
(opts[:protocol] || @config[:protocol]).to_s
end

def multi_region_support?
@config[:multi_region]
end

def hosts(bucket)
host = read_host(bucket)
if host
Expand Down
8 changes: 5 additions & 3 deletions qiniu.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ Gem::Specification.new do |gem|
gem.add_development_dependency 'rake', '~> 12'
gem.add_development_dependency 'rspec', '~> 3.5'
gem.add_development_dependency 'webmock', '~> 2.3'
if Gem::Version.create(RUBY_VERSION) >= Gem::Version::create('2.4.0')
gem.add_development_dependency 'codecov', '~> 0.6.0'
gem.add_development_dependency 'simplecov', '~> 0.18.5'
if Gem::Version.create(RUBY_VERSION) >= Gem::Version::create('2.5.0')
gem.add_development_dependency 'simplecov', '~> 0.22'
gem.add_development_dependency 'simplecov-cobertura', '>= 2.0.0', '< 4.0.0'
end

gem.add_runtime_dependency 'base64', '< 1.0.0'
gem.add_runtime_dependency 'rexml', '~> 3.2'
gem.add_runtime_dependency 'rest-client', '~> 2.0'
gem.add_runtime_dependency 'mime-types', '~> 3.1'
Expand Down
10 changes: 7 additions & 3 deletions rails/qiniu.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ Gem::Specification.new do |gem|
gem.add_development_dependency 'rake', '~> 12'
gem.add_development_dependency 'rspec', '~> 3.5'
gem.add_development_dependency 'webmock', '~> 2.3'
if Gem::Version.create(RUBY_VERSION) >= Gem::Version::create('2.3.0')
gem.add_development_dependency 'codecov', '~> 0.2.5'
if Gem::Version.create(RUBY_VERSION) >= Gem::Version::create('2.5.0')
gem.add_development_dependency 'simplecov', '~> 0.22'
gem.add_development_dependency 'simplecov-cobertura', '>= 2.0.0', '< 4.0.0'
elsif Gem::Version.create(RUBY_VERSION) >= Gem::Version::create('2.3.0')
gem.add_development_dependency 'simplecov', '~> 0.18.5'
gem.add_development_dependency 'simplecov-cobertura', '~> 1.4'
else
gem.add_development_dependency 'codecov', '~> 0.1.20'
gem.add_development_dependency 'simplecov', '~> 0.17.1'
end

gem.add_runtime_dependency 'base64', '< 1.0.0'
gem.add_runtime_dependency 'rexml', '~> 3.2'
gem.add_runtime_dependency 'rest-client', '~> 2.0'
gem.add_runtime_dependency 'mime-types', '~> 3.1'
Expand Down
33 changes: 16 additions & 17 deletions spec/qiniu/auth_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Auth
describe Auth do

before :all do
@bucket = 'rubysdk'
@bucket = BUCKET
end
after :all do
end
Expand Down Expand Up @@ -68,22 +68,21 @@ module Auth
expect(code).to eq(200)
end

it "should generate uphosts and global for multi_region" do
origin_multi_region = Config.settings[:multi_region]
begin
Config.settings[:multi_region] = true
### 生成Key
key = 'a_private_file'
key = make_unique_key_in_bucket(key)
puts "key=#{key}"

### 生成 PutPolicy
pp = Auth::PutPolicy.new(@bucket, key)
expect(pp.instance_variable_get(:@uphosts)).to eq ["upload.qiniup.com", "up.qiniup.com"]
expect(pp.instance_variable_get(:@global)).to be false
ensure
Config.settings[:multi_region] = origin_multi_region
end
it "should generate uphosts and global" do
### 生成Key
key = 'a_private_file'
key = make_unique_key_in_bucket(key)
puts "key=#{key}"

### 生成 PutPolicy
pp = Auth::PutPolicy.new(@bucket, key)
expect(pp.instance_variable_get(:@global)).to be false
uphosts = pp.instance_variable_get(:@uphosts)
expect(uphosts.size).to eq 2
Copy link
Copy Markdown

@fennoai fennoai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion hard-codes the host list size to exactly 2 even though the API contract here is really “contains at least one upload-* host and one up-* host.” If UC starts returning additional upload endpoints for the bucket, the test will fail even though the SDK behavior is still correct. Dropping the exact-size check would make this integration spec much less brittle.

patterns = [/^upload-.*\.qiniup\.com$/, /^up-.*\.qiniup\.com$/]
expect(uphosts).to satisfy { |hosts|
patterns.all? { |pat| hosts.any? { |host| host.match?(pat) } }
}
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/qiniu/image_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Fop
describe Fop do

before :all do
@bucket = 'rubysdk'
@bucket = BUCKET
pic_fname = "image_logo_for_test.png"
@key = make_unique_key_in_bucket(pic_fname)

Expand Down
24 changes: 6 additions & 18 deletions spec/qiniu/management_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,30 +129,18 @@ module Storage
end
end

describe 'When multi_region is disabled' do
describe 'for z0 bucket' do
before :all do
Config.settings[:multi_region] = false
@bucket = 'rubysdk'
@bucket = BUCKET
end
include_examples 'Management Specs'
end

describe 'When multi_region is enabled' do
describe 'for z0 bucket' do
before :all do
Config.settings[:multi_region] = true
@bucket = 'rubysdk'
end
include_examples 'Management Specs'
end

describe 'for z1 bucket' do
before :all do
Config.settings[:multi_region] = true
@bucket = 'rubysdk-bc'
end
include_examples 'Management Specs'
describe 'for z1 bucket' do
before :all do
@bucket = BUCKET_BC
end
include_examples 'Management Specs'
end
end # module Storage
end # module Qiniu
2 changes: 1 addition & 1 deletion spec/qiniu/misc_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Misc
describe Misc do

before :all do
@bucket = 'rubysdk'
@bucket = BUCKET
end

after :all do
Expand Down
2 changes: 1 addition & 1 deletion spec/qiniu/pfop_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Persistance
describe Persistance do

before :all do
@bucket = 'rubysdk'
@bucket = BUCKET

pic_fname = "image_logo_for_test.png"
@key = make_unique_key_in_bucket(pic_fname)
Expand Down
4 changes: 1 addition & 3 deletions spec/qiniu/qiniu_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ module Qiniu
describe Qiniu do

before :all do
Config.settings[:multi_region] = true

@bucket = 'rubysdk-na0'
@bucket = BUCKET_NA0
@test_image_bucket = @bucket

@key = Digest::SHA1.hexdigest Time.now.to_s
Expand Down
4 changes: 1 addition & 3 deletions spec/qiniu/upload_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ module Qiniu
module Storage
shared_examples "Upload Specs" do
before :all do
Config.settings[:multi_region] = true

@key = Digest::SHA1.hexdigest((Time.now.to_i+rand(100)).to_s)
@key = make_unique_key_in_bucket(@key)
puts "key=#{@key}"
Expand Down Expand Up @@ -480,7 +478,7 @@ module Storage

describe 'for na0 bucket' do
before :all do
@bucket = 'rubysdk-na0'
@bucket = BUCKET_NA0
end
include_examples 'Upload Specs'
end
Expand Down
12 changes: 7 additions & 5 deletions spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
require 'rspec'
require 'webmock'

if RUBY_ENGINE == 'jruby' && Gem::Version.create(RUBY_VERSION) < Gem::Version::create('2.3.0')
# Do nothing
else
BUCKET = ENV['BUCKET'] || 'rubysdk'
BUCKET_NA0 = ENV['BUCKET_NA0'] || 'rubysdk-na0'
BUCKET_BC = ENV['BUCKET_BC'] || 'rubysdk-bc'

if RUBY_ENGINE != 'jruby' && Gem::Version.create(RUBY_VERSION) >= Gem::Version::create('2.5.0')
require 'simplecov'
require 'codecov'
require 'simplecov-cobertura'

SimpleCov.formatter = SimpleCov::Formatter::CoberturaFormatter
SimpleCov.start
SimpleCov.formatter = SimpleCov::Formatter::Codecov
end

RSpec.configure do |config|
Expand Down