From e674608d10e3c0381d2e8172580b36eb553b5279 Mon Sep 17 00:00:00 2001 From: "Akihiko Odaki (@fn_aki@pawoo.net)" Date: Sun, 4 Jun 2017 21:59:40 +0900 Subject: [PATCH] A minor change for ProviderDiscovery and spec (#3543) * Do not default the format in ProviderDiscovery The format should be determined when discovering, as it is in the current implementation, and it is a flaw if it is not determined. * Spec ProviderDiscovery --- app/lib/provider_discovery.rb | 2 +- .../fixtures/requests/oembed_invalid_xml.html | 7 ++ spec/fixtures/requests/oembed_json.html | 7 ++ spec/fixtures/requests/oembed_json_xml.html | 8 ++ .../requests/oembed_undiscoverable.html | 5 + spec/fixtures/requests/oembed_xml.html | 7 ++ spec/lib/provider_discovery_spec.rb | 118 ++++++++++++++++++ 7 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/requests/oembed_invalid_xml.html create mode 100644 spec/fixtures/requests/oembed_json.html create mode 100644 spec/fixtures/requests/oembed_json_xml.html create mode 100644 spec/fixtures/requests/oembed_undiscoverable.html create mode 100644 spec/fixtures/requests/oembed_xml.html create mode 100644 spec/lib/provider_discovery_spec.rb diff --git a/app/lib/provider_discovery.rb b/app/lib/provider_discovery.rb index 3d0712a5ca..6d48cae2f0 100644 --- a/app/lib/provider_discovery.rb +++ b/app/lib/provider_discovery.rb @@ -31,7 +31,7 @@ class ProviderDiscovery < OEmbed::ProviderDiscovery raise OEmbed::NotFound, url end - OEmbed::Provider.new(provider_endpoint, format || OEmbed::Formatter.default) + OEmbed::Provider.new(provider_endpoint, format) end end end diff --git a/spec/fixtures/requests/oembed_invalid_xml.html b/spec/fixtures/requests/oembed_invalid_xml.html new file mode 100644 index 0000000000..405834f129 --- /dev/null +++ b/spec/fixtures/requests/oembed_invalid_xml.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/spec/fixtures/requests/oembed_json.html b/spec/fixtures/requests/oembed_json.html new file mode 100644 index 0000000000..773a4f92a2 --- /dev/null +++ b/spec/fixtures/requests/oembed_json.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/spec/fixtures/requests/oembed_json_xml.html b/spec/fixtures/requests/oembed_json_xml.html new file mode 100644 index 0000000000..b5fc9bed09 --- /dev/null +++ b/spec/fixtures/requests/oembed_json_xml.html @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/spec/fixtures/requests/oembed_undiscoverable.html b/spec/fixtures/requests/oembed_undiscoverable.html new file mode 100644 index 0000000000..a4acdc47a5 --- /dev/null +++ b/spec/fixtures/requests/oembed_undiscoverable.html @@ -0,0 +1,5 @@ + + + + + diff --git a/spec/fixtures/requests/oembed_xml.html b/spec/fixtures/requests/oembed_xml.html new file mode 100644 index 0000000000..5d7633e713 --- /dev/null +++ b/spec/fixtures/requests/oembed_xml.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/spec/lib/provider_discovery_spec.rb b/spec/lib/provider_discovery_spec.rb new file mode 100644 index 0000000000..12e2616c9c --- /dev/null +++ b/spec/lib/provider_discovery_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ProviderDiscovery do + describe 'discover_provider' do + context 'when status code is 200 and MIME type is text/html' do + context 'Both of JSON and XML provider are discoverable' do + before do + stub_request(:get, 'https://host/oembed.html').to_return( + status: 200, + headers: { 'Content-Type': 'text/html' }, + body: request_fixture('oembed_json_xml.html') + ) + end + + it 'returns new OEmbed::Provider for JSON provider if :format option is set to :json' do + provider = ProviderDiscovery.discover_provider('https://host/oembed.html', format: :json) + expect(provider.endpoint).to eq 'https://host/provider.json' + expect(provider.format).to eq :json + end + + it 'returns new OEmbed::Provider for XML provider if :format option is set to :xml' do + provider = ProviderDiscovery.discover_provider('https://host/oembed.html', format: :xml) + expect(provider.endpoint).to eq 'https://host/provider.xml' + expect(provider.format).to eq :xml + end + end + + context 'JSON provider is discoverable while XML provider is not' do + before do + stub_request(:get, 'https://host/oembed.html').to_return( + status: 200, + headers: { 'Content-Type': 'text/html' }, + body: request_fixture('oembed_json.html') + ) + end + + it 'returns new OEmbed::Provider for JSON provider' do + provider = ProviderDiscovery.discover_provider('https://host/oembed.html') + expect(provider.endpoint).to eq 'https://host/provider.json' + expect(provider.format).to eq :json + end + end + + context 'XML provider is discoverable while JSON provider is not' do + before do + stub_request(:get, 'https://host/oembed.html').to_return( + status: 200, + headers: { 'Content-Type': 'text/html' }, + body: request_fixture('oembed_xml.html') + ) + end + + it 'returns new OEmbed::Provider for XML provider' do + provider = ProviderDiscovery.discover_provider('https://host/oembed.html') + expect(provider.endpoint).to eq 'https://host/provider.xml' + expect(provider.format).to eq :xml + end + end + + context 'Invalid XML provider is discoverable while JSON provider is not' do + before do + stub_request(:get, 'https://host/oembed.html').to_return( + status: 200, + headers: { 'Content-Type': 'text/html' }, + body: request_fixture('oembed_invalid_xml.html') + ) + end + + it 'raises OEmbed::NotFound' do + expect { ProviderDiscovery.discover_provider('https://host/oembed.html') }.to raise_error OEmbed::NotFound + end + end + + context 'Neither of JSON and XML provider is discoverable' do + before do + stub_request(:get, 'https://host/oembed.html').to_return( + status: 200, + headers: { 'Content-Type': 'text/html' }, + body: request_fixture('oembed_undiscoverable.html') + ) + end + + it 'raises OEmbed::NotFound' do + expect { ProviderDiscovery.discover_provider('https://host/oembed.html') }.to raise_error OEmbed::NotFound + end + end + end + + context 'when status code is not 200' do + before do + stub_request(:get, 'https://host/oembed.html').to_return( + status: 400, + headers: { 'Content-Type': 'text/html' }, + body: request_fixture('oembed_xml.html') + ) + end + + it 'raises OEmbed::NotFound' do + expect { ProviderDiscovery.discover_provider('https://host/oembed.html') }.to raise_error OEmbed::NotFound + end + end + + context 'when MIME type is not text/html' do + before do + stub_request(:get, 'https://host/oembed.html').to_return( + status: 200, + body: request_fixture('oembed_xml.html') + ) + end + + it 'raises OEmbed::NotFound' do + expect { ProviderDiscovery.discover_provider('https://host/oembed.html') }.to raise_error OEmbed::NotFound + end + end + end +end