diff options
author | Matt Jankowski <mjankowski@thoughtbot.com> | 2017-05-30 16:30:06 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-05-30 16:30:06 -0400 |
commit | 3576fa0d591db69a1727153a1130ff5bebf37167 (patch) | |
tree | 93fd386972bcf128b91d0296865239631286ef11 | |
parent | 1dcfb902024a7d4049306d9bc48c499856e2e429 (diff) |
Improve api oembed controller (#3450)
* Add StreamEntryFinder class to parse URLs
* Use StreamEntryFinder and clean up api/oembed controller
-rw-r--r-- | app/controllers/api/oembed_controller.rb | 26 | ||||
-rw-r--r-- | app/lib/stream_entry_finder.rb | 34 | ||||
-rw-r--r-- | spec/controllers/api/oembed_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/stream_entry_finder_spec.rb | 47 |
4 files changed, 92 insertions, 17 deletions
diff --git a/app/controllers/api/oembed_controller.rb b/app/controllers/api/oembed_controller.rb index 656851f43b9..57618835312 100644 --- a/app/controllers/api/oembed_controller.rb +++ b/app/controllers/api/oembed_controller.rb @@ -4,30 +4,22 @@ class Api::OEmbedController < ApiController respond_to :json def show - @stream_entry = stream_entry_from_url(params[:url]) - @width = params[:maxwidth].present? ? params[:maxwidth].to_i : 400 - @height = params[:maxheight].present? ? params[:maxheight].to_i : nil + @stream_entry = find_stream_entry.stream_entry + @width = maxwidth_or_default + @height = maxheight_or_default end private - def stream_entry_from_url(url) - params = Rails.application.routes.recognize_path(url) - - raise ActiveRecord::RecordNotFound unless recognized_stream_entry_url?(params) - - stream_entry(params) + def find_stream_entry + StreamEntryFinder.new(params[:url]) end - def recognized_stream_entry_url?(params) - %w(stream_entries statuses).include?(params[:controller]) && params[:action] == 'show' + def maxwidth_or_default + (params[:maxwidth].presence || 400).to_i end - def stream_entry(params) - if params[:controller] == 'stream_entries' - StreamEntry.find(params[:id]) - else - Status.find(params[:id]).stream_entry - end + def maxheight_or_default + params[:maxheight].present? ? params[:maxheight].to_i : nil end end diff --git a/app/lib/stream_entry_finder.rb b/app/lib/stream_entry_finder.rb new file mode 100644 index 00000000000..0ea33229ce9 --- /dev/null +++ b/app/lib/stream_entry_finder.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class StreamEntryFinder + attr_reader :url + + def initialize(url) + @url = url + end + + def stream_entry + verify_action! + + case recognized_params[:controller] + when 'stream_entries' + StreamEntry.find(recognized_params[:id]) + when 'statuses' + Status.find(recognized_params[:id]).stream_entry + else + raise ActiveRecord::RecordNotFound + end + end + + private + + def recognized_params + Rails.application.routes.recognize_path(url) + end + + def verify_action! + unless recognized_params[:action] == 'show' + raise ActiveRecord::RecordNotFound + end + end +end diff --git a/spec/controllers/api/oembed_controller_spec.rb b/spec/controllers/api/oembed_controller_spec.rb index 511cdb4639c..43631a7e5a0 100644 --- a/spec/controllers/api/oembed_controller_spec.rb +++ b/spec/controllers/api/oembed_controller_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' RSpec.describe Api::OEmbedController, type: :controller do + render_views + let(:alice) { Fabricate(:account, username: 'alice') } let(:status) { Fabricate(:status, text: 'Hello world', account: alice) } diff --git a/spec/lib/stream_entry_finder_spec.rb b/spec/lib/stream_entry_finder_spec.rb new file mode 100644 index 00000000000..88ba72b1151 --- /dev/null +++ b/spec/lib/stream_entry_finder_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe StreamEntryFinder do + include RoutingHelper + + describe '#stream_entry' do + context 'with a status url' do + let(:status) { Fabricate(:status) } + let(:url) { short_account_status_url(account_username: status.account.username, id: status.id) } + subject { described_class.new(url) } + + it 'finds the stream entry' do + expect(subject.stream_entry).to eq(status.stream_entry) + end + end + + context 'with a stream entry url' do + let(:stream_entry) { Fabricate(:stream_entry) } + let(:url) { account_stream_entry_url(stream_entry.account, stream_entry) } + subject { described_class.new(url) } + + it 'finds the stream entry' do + expect(subject.stream_entry).to eq(stream_entry) + end + end + + context 'with a plausible url' do + let(:url) { 'https://example.com/users/test/updates/123/embed' } + subject { described_class.new(url) } + + it 'raises an error' do + expect { subject.stream_entry }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'with an unrecognized url' do + let(:url) { 'https://example.com/about' } + subject { described_class.new(url) } + + it 'raises an error' do + expect { subject.stream_entry }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end +end |