Skip to content

Commit

Permalink
Merge pull request #694 from sul-dlss/remove-url-normalization
Browse files Browse the repository at this point in the history
Remove url normalization from our HTTP calls to the imageserver
  • Loading branch information
cbeer authored Oct 1, 2020
2 parents 59a2adf + b2728cc commit 6149c23
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 15 deletions.
4 changes: 3 additions & 1 deletion app/models/source_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ def initialize(id:, file_name:, transformation:); end
def response
with_retries max_tries: 3, rescue: [HTTP::ConnectionError] do
benchmark "Fetch #{image_url}" do
HTTP.get(image_url)
# Disable url normalization as an upstream bug in addressable causes issues for `+`
# https://github.com/sporkmonger/addressable/issues/386
HTTP.use({ normalize_uri: { normalizer: lambda(&:itself) } }).get(image_url)
end
end
end
Expand Down
6 changes: 5 additions & 1 deletion app/services/iiif_metadata_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ def image_height
# @return [String] the IIIF info response
def retrieve
with_retries max_tries: 3, rescue: [HTTP::ConnectionError] do
handle_response(HTTP.get(@url))
handle_response(
# Disable url normalization as an upstream bug in addressable causes issues for `+`
# https://github.com/sporkmonger/addressable/issues/386
HTTP.use({ normalize_uri: { normalizer: lambda(&:itself) } }).get(@url)
)
end
end

Expand Down
31 changes: 21 additions & 10 deletions spec/models/projection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
let(:image) { StacksImage.new }
let(:instance) { described_class.new(image, transformation) }
let(:transformation) { IIIF::Image::OptionDecoder.decode(options) }
let(:http_client) { instance_double(HTTP::Client) }

before do
allow(image).to receive_messages(image_width: 800, image_height: 600)
Expand Down Expand Up @@ -105,9 +106,11 @@
let(:options) { { size: 'max', region: 'full' } }

it 'allows the user to see the full-resolution image' do
allow(HTTP).to receive(:get).and_return(double(body: nil))
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
expect(HTTP).to have_received(:get).with(%r{/full/max/0/default.jpg})
expect(http_client).to have_received(:get).with(%r{/full/max/0/default.jpg})
end
end
end
Expand All @@ -121,39 +124,47 @@
let(:options) { { size: 'max', region: 'full' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:get).and_return(double(body: nil))
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
expect(HTTP).to have_received(:get).with(%r{/full/!400,400/0/default.jpg})
expect(http_client).to have_received(:get).with(%r{/full/!400,400/0/default.jpg})
end
end

context "smaller-than-a-thumbnail size" do
let(:options) { { size: '!100,100', region: 'full' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:get).and_return(double(body: nil))
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
expect(HTTP).to have_received(:get).with(%r{/full/!100,100/0/default.jpg})
expect(http_client).to have_received(:get).with(%r{/full/!100,100/0/default.jpg})
end
end

context "best fit size" do
let(:options) { { size: '!800,880', region: 'full' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:get).and_return(double(body: nil))
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
expect(HTTP).to have_received(:get).with(%r{/full/!400,400/0/default.jpg})
expect(http_client).to have_received(:get).with(%r{/full/!400,400/0/default.jpg})
end
end

context "square region" do
let(:options) { { size: '100,100', region: 'square' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:get).and_return(double(body: nil))
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
expect(HTTP).to have_received(:get).with(%r{/square/100,100/0/default.jpg})
expect(http_client).to have_received(:get).with(%r{/square/100,100/0/default.jpg})
end
end
end
Expand Down
5 changes: 4 additions & 1 deletion spec/requests/iiif_auth_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
let(:path) { "/stacks/nr/349/ct/7889/nr349ct7889_00_0001" }
let(:perms) { nil }
let(:current_image) { StacksImage.new(params_hash) }
let(:http_client) { instance_double(HTTP::Client) }

before(:each) do
allow(File).to receive(:world_readable?).with(path).and_return(perms)
Expand All @@ -30,7 +31,9 @@
describe "#show" do
before(:each) do
allow_any_instance_of(Projection).to receive(:valid?).and_return(true)
allow(HTTP).to receive(:get).and_return(instance_double(HTTP::Response, status: 200, body: StringIO.new))
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(instance_double(HTTP::Response, status: 200, body: StringIO.new))

allow_any_instance_of(IiifController).to receive(:current_user).and_return(current_user)
allow_any_instance_of(IiifController).to receive(:current_image).and_return(current_image)
Expand Down
10 changes: 8 additions & 2 deletions spec/services/iiif_metadata_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
end
let(:base_uri) { 'https://sul-imageserver-uat.stanford.edu/cantaloupe/iiif/2/' } # 'image-server-path'
let(:service) { described_class.new(image_id: identifier, canonical_url: 'foo', base_uri: base_uri) }
let(:http_client) { instance_double(HTTP::Client) }

context "When a valid JSON response is received" do
let(:json) do
Expand All @@ -19,8 +20,11 @@
'"sizes":[{"width":1916,"height":1276}]}'
end
let(:response) { instance_double(HTTP::Response, code: 200, body: json) }

before do
allow(HTTP).to receive(:get)
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get)
.with('https://sul-imageserver-uat.stanford.edu/cantaloupe/iiif/2/nr%2F349%2Fct%2F7889%2Fnr349ct7889_00_0001.jp2/info.json')
.and_return(response)
end
Expand Down Expand Up @@ -54,7 +58,9 @@
let(:empty_json) { '' }
let(:bad_response) { instance_double(HTTP::Response, code: 200, body: empty_json) }
before do
allow(HTTP).to receive(:get)
allow(HTTP).to receive(:use)
.and_return(http_client)
allow(http_client).to receive(:get)
.with('https://sul-imageserver-uat.stanford.edu/cantaloupe/iiif/2/nr%2F349%2Fct%2F7889%2Fnr349ct7889_00_0001.jp2/info.json')
.and_return(bad_response)
end
Expand Down

0 comments on commit 6149c23

Please sign in to comment.