Skip to content
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.

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

fix: deprecate domain sharding #43

Merged
merged 4 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ path.rect(x: 0, y: 50, width: 200, height: 300).to_url # Rect helper


## Domain Sharded URLs
**Warning: Domain Sharding has been deprecated and will be removed in the next major release**

Domain sharding enables you to spread image requests across multiple domains. This allows you to bypass the requests-per-host limits of browsers. We recommend 2-3 domain shards maximum if you are going to use domain sharding.

Expand Down
11 changes: 10 additions & 1 deletion lib/imgix/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Client
def initialize(options = {})
options = DEFAULTS.merge(options)

deprecate_warning!(options[:host],options[:hosts])
@hosts = Array(options[:host]) + Array(options[:hosts]) and validate_hosts!
@secure_url_token = options[:secure_url_token]
@api_key = options[:api_key]
Expand Down Expand Up @@ -59,7 +60,9 @@ def host_for_crc(path)
end

def host_for_cycle
@hosts_cycle = @hosts.cycle unless @hosts_cycle
Copy link
Contributor Author

@sherwinski sherwinski Apr 23, 2019

Choose a reason for hiding this comment

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

Incidentally, this line was throwing an initialization warning that got in the way when we used assert_output to check for the expected deprecation warning.

if not defined? @hosts_cycle
@hosts_cycle = @hosts.cycle
end
@hosts_cycle.next
end

Expand All @@ -82,5 +85,11 @@ def validate_hosts!
end
end

def deprecate_warning!(host, hosts)
has_many_domains = (host.kind_of?(Array) && host.length > 1 ) || (hosts.kind_of?(Array) && hosts.length > 1)
if has_many_domains
warn "Warning: Domain sharding has been deprecated and will be removed in the next major version."
end
end
end
end
113 changes: 66 additions & 47 deletions test/units/domains_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,67 +4,86 @@

class DomainsTest < Imgix::Test
def test_deterministically_choosing_a_path
client = Imgix::Client.new(hosts: [
"demos-1.imgix.net",
"demos-2.imgix.net",
"demos-3.imgix.net",
],
secure_url_token: '10adc394',
include_library_param: false)

path = client.path('/bridge.png')
assert_equal 'https://demos-1.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/flower.png')
assert_equal 'https://demos-2.imgix.net/flower.png?s=02105961388864f85c04121ea7b50e08', path.to_url
assert_output(nil, "Warning: Domain sharding has been deprecated and will be removed in the next major version.\n"){
client = Imgix::Client.new(hosts: [
"demos-1.imgix.net",
"demos-2.imgix.net",
"demos-3.imgix.net",
],
secure_url_token: '10adc394',
include_library_param: false)

path = client.path('/bridge.png')
assert_equal 'https://demos-1.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/flower.png')
assert_equal 'https://demos-2.imgix.net/flower.png?s=02105961388864f85c04121ea7b50e08', path.to_url
}
end

def test_cycling_choosing_domain_in_order
client = Imgix::Client.new(hosts: [
"demos-1.imgix.net",
"demos-2.imgix.net",
"demos-3.imgix.net",
],
secure_url_token: '10adc394',
shard_strategy: :cycle,
include_library_param: false)

path = client.path('/bridge.png')
assert_equal 'https://demos-1.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/bridge.png')
assert_equal 'https://demos-2.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/bridge.png')
assert_equal 'https://demos-3.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/bridge.png')
assert_equal 'https://demos-1.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url
assert_output(nil, "Warning: Domain sharding has been deprecated and will be removed in the next major version.\n"){
client = Imgix::Client.new(hosts: [
"demos-1.imgix.net",
"demos-2.imgix.net",
"demos-3.imgix.net",
],
secure_url_token: '10adc394',
shard_strategy: :cycle,
include_library_param: false)

path = client.path('/bridge.png')
assert_equal 'https://demos-1.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/bridge.png')
assert_equal 'https://demos-2.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/bridge.png')
assert_equal 'https://demos-3.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url

path = client.path('/bridge.png')
assert_equal 'https://demos-1.imgix.net/bridge.png?s=0233fd6de51f20f11cff6b452b7a9a05', path.to_url
}
end

def test_with_full_paths
client = Imgix::Client.new(hosts: [
"demos-1.imgix.net",
"demos-2.imgix.net",
"demos-3.imgix.net",
],
secure_url_token: '10adc394',
shard_strategy: :cycle,
include_library_param: false)

path = 'https://google.com/cats.gif'
assert_equal "https://demos-1.imgix.net/#{CGI.escape(path)}?s=e686099fbba86fc2b8141d3c1ff60605", client.path(path).to_url
assert_output(nil, "Warning: Domain sharding has been deprecated and will be removed in the next major version.\n"){
client = Imgix::Client.new(hosts: [
"demos-1.imgix.net",
"demos-2.imgix.net",
"demos-3.imgix.net",
],
secure_url_token: '10adc394',
shard_strategy: :cycle,
include_library_param: false)

path = 'https://google.com/cats.gif'
assert_equal "https://demos-1.imgix.net/#{CGI.escape(path)}?s=e686099fbba86fc2b8141d3c1ff60605", client.path(path).to_url
}
end

def test_invalid_domain_append_slash
assert_raises(ArgumentError) {Imgix::Client.new(hosts: ["assets.imgix.net/"])}
assert_raises(ArgumentError) {Imgix::Client.new(hosts: "assets.imgix.net/")}
end

def test_invalid_domain_prepend_scheme
assert_raises(ArgumentError) {Imgix::Client.new(hosts: ["https://assets.imgix.net"])}
assert_raises(ArgumentError) {Imgix::Client.new(hosts: "https://assets.imgix.net")}
end

def test_invalid_domain_append_dash
assert_raises(ArgumentError) {Imgix::Client.new(hosts: ["assets.imgix.net-"])}
assert_raises(ArgumentError) {Imgix::Client.new(hosts: "assets.imgix.net-")}
end

def test_domain_sharding_deprecation_host

assert_output(nil, "Warning: Domain sharding has been deprecated and will be removed in the next major version.\n"){
Imgix::Client.new(host: ["assets1.imgix.net", "assets2.imgix.net"])
}
end

def test_domain_sharding_deprecation_hosts
assert_output(nil, "Warning: Domain sharding has been deprecated and will be removed in the next major version.\n"){
Imgix::Client.new(hosts: ["assets1.imgix.net", "assets2.imgix.net"])
}
end
end