Improve api oembed controller (#3450)
* Add StreamEntryFinder class to parse URLs * Use StreamEntryFinder and clean up api/oembed controller
This commit is contained in:
		@@ -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
 | 
			
		||||
 
 | 
			
		||||
							
								
								
									
										34
									
								
								app/lib/stream_entry_finder.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										34
									
								
								app/lib/stream_entry_finder.rb
									
									
									
									
									
										Normal file
									
								
							@@ -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
 | 
			
		||||
@@ -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) }
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
							
								
								
									
										47
									
								
								spec/lib/stream_entry_finder_spec.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										47
									
								
								spec/lib/stream_entry_finder_spec.rb
									
									
									
									
									
										Normal file
									
								
							@@ -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
 | 
			
		||||
		Reference in New Issue
	
	Block a user