From f3ea8f424fb79d3f0143ccde6e170965b23926be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sch=C3=B6lling?= Date: Sun, 25 Sep 2011 21:59:25 +0200 Subject: [PATCH 1/8] Added oEmbed support --- Gemfile | 1 + app/helpers/markdownify_helper.rb | 6 +- app/models/jobs/gather_o_embed_data.rb | 38 ++++++ app/models/o_embed_cache.rb | 3 + app/models/post.rb | 5 + app/views/comments/_comment.html.haml | 2 +- app/views/messages/_message.haml | 2 +- app/views/people/_profile_sidebar.html.haml | 4 +- app/views/photos/show.html.haml | 2 +- .../status_messages/_status_message.haml | 2 +- config/initializers/oembed.rb | 4 + .../20110924112840_create_o_embed_caches.rb | 14 ++ lib/diaspora/markdownify.rb | 3 +- spec/helpers/markdownify_helper_spec.rb | 128 ++++++++++++++++++ spec/models/jobs/gather_o_embed_data.rb | 59 ++++++++ spec/models/o_embed_cache_spec.rb | 5 + spec/models/post_spec.rb | 11 ++ 17 files changed, 280 insertions(+), 9 deletions(-) create mode 100644 app/models/jobs/gather_o_embed_data.rb create mode 100644 app/models/o_embed_cache.rb create mode 100644 config/initializers/oembed.rb create mode 100644 db/migrate/20110924112840_create_o_embed_caches.rb create mode 100644 spec/models/jobs/gather_o_embed_data.rb create mode 100644 spec/models/o_embed_cache_spec.rb diff --git a/Gemfile b/Gemfile index a32c2ca19..66326040a 100644 --- a/Gemfile +++ b/Gemfile @@ -64,6 +64,7 @@ gem 'rails-i18n' gem 'nokogiri' gem 'redcarpet', "2.0.0b5" gem 'roxml', :git => 'git://github.com/Empact/roxml.git', :ref => '7ea9a9ffd2338aaef5b0' +gem 'ruby-oembed' # queue diff --git a/app/helpers/markdownify_helper.rb b/app/helpers/markdownify_helper.rb index cf012b581..4b87e6210 100644 --- a/app/helpers/markdownify_helper.rb +++ b/app/helpers/markdownify_helper.rb @@ -33,7 +33,11 @@ module MarkdownifyHelper return '' if message.blank? #renderer = Redcarpet::Render::HTML.new(render_options) - renderer = Diaspora::Markdownify::HTML.new(render_options) + if render_options[:oembed] + renderer = Diaspora::Markdownify::HTMLwithOEmbed.new(render_options) + else + renderer = Diaspora::Markdownify::HTML.new(render_options) + end markdown = Redcarpet::Markdown.new(renderer, markdown_options) message = markdown.render(message).html_safe diff --git a/app/models/jobs/gather_o_embed_data.rb b/app/models/jobs/gather_o_embed_data.rb new file mode 100644 index 000000000..ed70e176e --- /dev/null +++ b/app/models/jobs/gather_o_embed_data.rb @@ -0,0 +1,38 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. +# + +module Jobs + class GatherOEmbedData < Base + @queue = :http_service + + class GatherOEmbedRenderer < Redcarpet::Render::HTML + include ActionView::Helpers::TextHelper + include ActionView::Helpers::TagHelper + + def autolink(link, type) + return link if OEmbedCache.exists?(:url => link) + + begin + res = ::OEmbed::Providers.get(link, {:maxwidth => 420, :maxheight => 420, :frame => 1, :iframe => 1}) + rescue Exception => e + # noop + else + data = res.fields + data['trusted_endpoint_url'] = res.provider.endpoint + cache = OEmbedCache.new(:url => link, :data => data) + cache.save + end + + return link + end + end + + def self.perform(text) + renderer = GatherOEmbedRenderer.new({}) + markdown = Redcarpet::Markdown.new(renderer, {:autolink => true}) + message = markdown.render(text) + end + end +end diff --git a/app/models/o_embed_cache.rb b/app/models/o_embed_cache.rb new file mode 100644 index 000000000..79f472f8c --- /dev/null +++ b/app/models/o_embed_cache.rb @@ -0,0 +1,3 @@ +class OEmbedCache < ActiveRecord::Base + serialize :data +end diff --git a/app/models/post.rb b/app/models/post.rb index 178167438..20613b5a5 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -66,6 +66,11 @@ class Post < ActiveRecord::Base @reshare_count ||= Post.where(:root_guid => self.guid).count end + def text= nd + Resque.enqueue(Jobs::GatherOEmbedData, nd) + write_attribute(:text, nd) + end + def diaspora_handle= nd self.author = Person.where(:diaspora_handle => nd).first write_attribute(:diaspora_handle, nd) diff --git a/app/views/comments/_comment.html.haml b/app/views/comments/_comment.html.haml index fd8a9834e..e0955bde3 100644 --- a/app/views/comments/_comment.html.haml +++ b/app/views/comments/_comment.html.haml @@ -12,7 +12,7 @@ = person_link(comment.author, :class => "hovercardable") %span{:class => direction_for(comment.text)} - = markdownify(comment, :youtube_maps => comment.youtube_titles) + = markdownify(comment, :oembed => true, :youtube_maps => comment.youtube_titles) .comment_info %time.timeago{:datetime => comment.created_at} diff --git a/app/views/messages/_message.haml b/app/views/messages/_message.haml index f36ebbc55..ad44c9b79 100644 --- a/app/views/messages/_message.haml +++ b/app/views/messages/_message.haml @@ -12,5 +12,5 @@ = how_long_ago(message) %div{ :class => direction_for(message.text) } - = markdownify(message) + = markdownify(message, :oembed => true) diff --git a/app/views/people/_profile_sidebar.html.haml b/app/views/people/_profile_sidebar.html.haml index c6d85c205..b3c5a7739 100644 --- a/app/views/people/_profile_sidebar.html.haml +++ b/app/views/people/_profile_sidebar.html.haml @@ -22,13 +22,13 @@ %h4 =t('.bio') %div{ :class => direction_for(person.profile.bio) } - = markdownify(person.profile.bio, :newlines => true) + = markdownify(person.profile.bio, :oembed => true, :newlines => true) - unless person.profile.location.blank? %li %h4 =t('.location') %div{ :class => direction_for(person.profile.location) } - = markdownify(person.profile.location, :newlines => true) + = markdownify(person.profile.location, :oembed => true, :newlines => true) %li - unless person.profile.gender.blank? diff --git a/app/views/photos/show.html.haml b/app/views/photos/show.html.haml index 283580bb0..f9805f232 100644 --- a/app/views/photos/show.html.haml +++ b/app/views/photos/show.html.haml @@ -40,7 +40,7 @@ .span-8.last %p - = markdownify(photo.status_message) + = markdownify(photo.status_message, :oembed => true) %span{:style=>'font-size:smaller'} =link_to t('.collection_permalink'), post_path(photo.status_message) %br diff --git a/app/views/status_messages/_status_message.haml b/app/views/status_messages/_status_message.haml index ec8343327..2a6347d66 100644 --- a/app/views/status_messages/_status_message.haml +++ b/app/views/status_messages/_status_message.haml @@ -16,4 +16,4 @@ = link_to (image_tag photo.url(:thumb_small), :class => 'stream-photo thumb_small', 'data-small-photo' => photo.url(:thumb_medium), 'data-full-photo' => photo.url), photo_path(photo), :class => 'stream-photo-link' %div{:class => direction_for(post.text)} - != markdownify(post, :youtube_maps => post[:youtube_titles]) + != markdownify(post, :oembed => true, :youtube_maps => post[:youtube_titles]) diff --git a/config/initializers/oembed.rb b/config/initializers/oembed.rb new file mode 100644 index 000000000..5db195434 --- /dev/null +++ b/config/initializers/oembed.rb @@ -0,0 +1,4 @@ +require 'oembed' +OEmbed::Providers.register_all +OEmbed::Providers.register_fallback(OEmbed::ProviderDiscovery) + diff --git a/db/migrate/20110924112840_create_o_embed_caches.rb b/db/migrate/20110924112840_create_o_embed_caches.rb new file mode 100644 index 000000000..78a549e81 --- /dev/null +++ b/db/migrate/20110924112840_create_o_embed_caches.rb @@ -0,0 +1,14 @@ +class CreateOEmbedCaches < ActiveRecord::Migration + def self.up + create_table :o_embed_caches do |t| + t.string :url, :limit => 1024, :null => false, :unique => true + t.text :data, :null => false + end + add_index :o_embed_caches, :url + end + + def self.down + remove_index :o_embed_caches, :column => :url + drop_table :o_embed_caches + end +end diff --git a/lib/diaspora/markdownify.rb b/lib/diaspora/markdownify.rb index 900436eb8..bdd24da54 100644 --- a/lib/diaspora/markdownify.rb +++ b/lib/diaspora/markdownify.rb @@ -3,12 +3,11 @@ require 'erb' module Diaspora module Markdownify class HTML < Redcarpet::Render::HTML - include ActionView::Helpers::TextHelper include ActionView::Helpers::TagHelper def autolink(link, type) - auto_link(link, :link => :urls, :html => { :target => "_blank" }) + auto_link(link, :link => :urls) end end end diff --git a/spec/helpers/markdownify_helper_spec.rb b/spec/helpers/markdownify_helper_spec.rb index ed73c7d1b..af88b27ba 100644 --- a/spec/helpers/markdownify_helper_spec.rb +++ b/spec/helpers/markdownify_helper_spec.rb @@ -58,6 +58,134 @@ describe MarkdownifyHelper do formatted = markdownify(message) formatted.should =~ /hovercard/ end + + context 'when posting a link with oEmbed support' do + scenarios = { + "photo" => { + "oembed_data" => { + "trusted_endpoint_url" => "__!SPOOFED!__", + "version" => "1.0", + "type" => "photo", + "title" => "ZB8T0193", + "width" => "240", + "height" => "160", + "url" => "http://farm4.static.flickr.com/3123/2341623661_7c99f48bbf_m.jpg" + }, + "link_url" => 'http://www.flickr.com/photos/bees/2341623661', + "oembed_get_request" => "http://www.flickr.com/services/oembed/?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://www.flickr.com/photos/bees/2341623661", + }, + + "unsupported" => { + "oembed_data" => "", + "oembed_get_request" => 'http://www.we-do-not-support-oembed.com/index.html', + "link_url" => 'http://www.we-do-not-support-oembed.com/index.html' + }, + + "secure_video" => { + "oembed_data" => { + "version" => "1.0", + "type" => "video", + "width" => 425, + "height" => 344, + "title" => "Amazing Nintendo Facts", + "html" => " + + + + + ", + }, + "link_url" => "http://youtube.com/watch?v=M3r2XDceM6A&format=json", + "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", + }, + + "unsecure_video" => { + "oembed_data" => { + "version" => "1.0", + "type" => "video", + "title" => "This is a video from an unsecure source", + "html" => " + + + + + ", + }, + "link_url" => "http://mytube.com/watch?v=M3r2XDceM6A&format=json", + "discovery_data" => '', + "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", + }, + + "secure_rich" => { + "oembed_data" => { + "version" => "1.0", + "type" => "rich", + "width" => 425, + "height" => 344, + "title" => "Amazing Nintendo Facts", + "html" => " + + + + + ", + }, + "link_url" => "http://youtube.com/watch?v=M3r2XDceM6A&format=json", + "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", + }, + + "unsecure_rich" => { + "oembed_data" => { + "version" => "1.0", + "type" => "rich", + "title" => "This is a video from an unsecure source", + "html" => " + + + + + ", + }, + "link_url" => "http://mytube.com/watch?v=M3r2XDceM6A&format=json", + "discovery_data" => '', + "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", + }, + } + + scenarios.each do |type, data| + specify 'for type "'+type+'"' do + stub_request(:get, data['oembed_get_request']).to_return(:status => 200, :body => data['oembed_data'].to_json.to_s) + stub_request(:get, data['link_url']).to_return(:status => 200, :body => data['discovery_data']) if data.has_key?('discovery_data') + + message = "Look at this! "+data['link_url'] + Jobs::GatherOEmbedData.perform(message) + OEmbedCache.find_by_url(data['link_url']).should_not be_nil unless type == 'unsupported' + + formatted = markdownify(message, :oembed => true) + case type + when 'photo' + formatted.should =~ /#{data['oembed_data']['url']}/ + when 'unsupported' + formatted.should =~ /#{data['link_url']}/ + when 'secure_video', 'secure_rich' + formatted.should =~ /#{data['oembed_data']['html']}/ + when 'unsecure_video', 'unsecure_rich' + formatted.should_not =~ /#{data['oembed_data']['html']}/ + formatted.should =~ /#{data['oembed_data']['title']}/ + formatted.should =~ /#{data['oembed_data']['url']}/ + end + end + end + + end end end end diff --git a/spec/models/jobs/gather_o_embed_data.rb b/spec/models/jobs/gather_o_embed_data.rb new file mode 100644 index 000000000..5d95a2ff8 --- /dev/null +++ b/spec/models/jobs/gather_o_embed_data.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +describe Jobs::GatherOEmbedData do + before do + @flickr_oembed_data = { + "trusted_endpoint_url" => "__!SPOOFED!__", + "version" => "1.0", + "type" => "photo", + "author_url" => "http://www.flickr.com/photos/bees/", + "cache_age" => 3600, + "provider_name" => "Flickr", + "provider_url" => "http://www.flickr.com/", + "title" => "ZB8T0193", + "author_name" => "Bees", + "width" => "240", + "height" => "160", + "url" => "https://farm4.static.flickr.com/3123/2341623661_7c99f48bbf_m.jpg" + } + + @flickr_oembed_url = 'http://www.flickr.com/services/oembed/' + @flickr_photo_url = 'http://www.flickr.com/photos/bees/2341623661' + @flickr_oembed_get_request = @flickr_oembed_url+"?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url="+@flickr_photo_url + + @no_oembed_url = 'http://www.we-do-not-support-oembed.com/index.html' + + stub_request(:get, @flickr_oembed_get_request).to_return(:status => 200, :body => @flickr_oembed_data.to_json) + stub_request(:get, @no_oembed_url).to_return(:status => 200, :body => 'hello there') + end + + describe '.perform' do + it 'requests not data from the internet' do + Jobs::GatherOEmbedData.perform("Look at this! "+@flickr_photo_url) + + a_request(:get, @flickr_oembed_get_request).should have_been_made + end + + it 'requests not data from the internet only once' do + Jobs::GatherOEmbedData.perform("Look at this! "+@flickr_photo_url) + Jobs::GatherOEmbedData.perform("Look at this! "+@flickr_photo_url) + + a_request(:get, @flickr_oembed_get_request).should have_been_made.times(1) + end + + it 'creates one cache entry' do + Jobs::GatherOEmbedData.perform("Look at this! "+@flickr_photo_url) + + expected_data = @flickr_oembed_data + expected_data['trusted_endpoint_url'] = @flickr_oembed_url + OEmbedCache.find_by_url(@flickr_photo_url).data.should == expected_data + + Jobs::GatherOEmbedData.perform("Look at this! "+@flickr_photo_url) + OEmbedCache.count(:conditions => {:url => @flickr_photo_url}).should == 1 + end + + it 'creates no cache entry for unsupported pages' do + Jobs::GatherOEmbedData.perform("This page is lame! It does not support oEmbed: "+@no_oembed_url) + OEmbedCache.find_by_url(@no_oembed_url).should be_nil + end + end +end diff --git a/spec/models/o_embed_cache_spec.rb b/spec/models/o_embed_cache_spec.rb new file mode 100644 index 000000000..e54417962 --- /dev/null +++ b/spec/models/o_embed_cache_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe OEmbedCache do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index fade3ab7a..10a769bd8 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -68,6 +68,17 @@ describe Post do end end + describe 'oEmbed' do + it 'should gather oEmbed data' do + stub_request(:get, "http://gdata.youtube.com/feeds/api/videos/M3r2XDceM6A?v=2").to_return(:status => 200, :body => "") + + text = 'http://youtube.com/watch?v=M3r2XDceM6A&format=json' + Resque.should_receive(:enqueue).with(Jobs::GatherOEmbedData, text) + post = Factory.create(:status_message, :author => @user.person, :text => text) + post.save! + end + end + describe 'serialization' do it 'should serialize the handle and not the sender' do post = @user.post :status_message, :text => "hello", :to => @aspect.id From 3561021a90ecd7ae24be7853c3c9c2d2817337b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sch=C3=B6lling?= Date: Tue, 27 Sep 2011 19:56:44 +0200 Subject: [PATCH 2/8] Added cucumber tests for oEmbed feature --- app/models/jobs/gather_o_embed_data.rb | 15 ++- features/oembed.feature | 55 +++++++++++ features/step_definitions/oembed.rb | 118 ++++++++++++++++++++++++ spec/helpers/markdownify_helper_spec.rb | 3 +- 4 files changed, 186 insertions(+), 5 deletions(-) create mode 100644 features/oembed.feature create mode 100644 features/step_definitions/oembed.rb diff --git a/app/models/jobs/gather_o_embed_data.rb b/app/models/jobs/gather_o_embed_data.rb index ed70e176e..fd07f7462 100644 --- a/app/models/jobs/gather_o_embed_data.rb +++ b/app/models/jobs/gather_o_embed_data.rb @@ -11,21 +11,28 @@ module Jobs include ActionView::Helpers::TextHelper include ActionView::Helpers::TagHelper + def isHttp?(url) + URI.parse(url).scheme.downcase == 'http' + end + def autolink(link, type) - return link if OEmbedCache.exists?(:url => link) + url = auto_link(link, :link => :urls).scan(/href=["']?((?:.(?!["']?\s+(?:\S+)=|[>"']))+.)["']?/).first.first + url = CGI::unescapeHTML(url) + + return url if OEmbedCache.exists?(:url => url) or not isHttp?(url) begin - res = ::OEmbed::Providers.get(link, {:maxwidth => 420, :maxheight => 420, :frame => 1, :iframe => 1}) + res = ::OEmbed::Providers.get(url, {:maxwidth => 420, :maxheight => 420, :frame => 1, :iframe => 1}) rescue Exception => e # noop else data = res.fields data['trusted_endpoint_url'] = res.provider.endpoint - cache = OEmbedCache.new(:url => link, :data => data) + cache = OEmbedCache.new(:url => url, :data => data) cache.save end - return link + return url end end diff --git a/features/oembed.feature b/features/oembed.feature new file mode 100644 index 000000000..6aefc2a18 --- /dev/null +++ b/features/oembed.feature @@ -0,0 +1,55 @@ +@javascript +Feature: oembed + In order to make videos easy accessible + As a user + I want the links in my posts be replaced by their oEmbed representation + + Background: + Given a user named "Alice Smith" with email "alice@alice.alice" + And I have several oEmbed data in cache + When I sign in as "alice@alice.alice" + And I am on the home page + + Scenario: Post a secure video link + Given I expand the publisher + When I fill in "status_message_fake_text" with "http://youtube.com/watch?v=M3r2XDceM6A&format=json" + And I press "Share" + + And I follow "Your Aspects" + Then I should see a video player + + Scenario: Post an unsecure video link + Given I expand the publisher + When I fill in "status_message_fake_text" with "http://mytube.com/watch?v=M3r2XDceM6A&format=json" + And I press "Share" + + And I follow "Your Aspects" + Then I should not see a video player + And I should see "http://mytube.com/watch?v=M3r2XDceM6A&format=json" + + Scenario: Post an unsecure rich-typed link + Given I expand the publisher + When I fill in "status_message_fake_text" with "http://myrichtube.com/watch?v=M3r2XDceM6A&format=json" + And I press "Share" + + And I follow "Your Aspects" + Then I should not see a video player + And I should see "http://myrichtube.com/watch?v=M3r2XDceM6A&format=json" + + Scenario: Post a photo link + Given I expand the publisher + When I fill in "status_message_fake_text" with "http://farm4.static.flickr.com/3123/2341623661_7c99f48bbf_m.jpg" + And I press "Share" + + And I follow "Your Aspects" + Then I should see a "img" within ".stream_element" + + Scenario: Post an unsupported text link + Given I expand the publisher + When I fill in "status_message_fake_text" with "http://www.we-do-not-support-oembed.com/index.html" + And I press "Share" + + And I follow "Your Aspects" + Then I should see "http://www.we-do-not-support-oembed.com/index.html" within ".stream_element" + + diff --git a/features/step_definitions/oembed.rb b/features/step_definitions/oembed.rb new file mode 100644 index 000000000..ade28cc3c --- /dev/null +++ b/features/step_definitions/oembed.rb @@ -0,0 +1,118 @@ +Given /^I have several oEmbed data in cache$/ do + scenarios = { + "photo" => { + "oembed_data" => { + "trusted_endpoint_url" => "__!SPOOFED!__", + "version" => "1.0", + "type" => "photo", + "title" => "ZB8T0193", + "width" => "240", + "height" => "160", + "url" => "http://farm4.static.flickr.com/3123/2341623661_7c99f48bbf_m.jpg" + }, + "link_url" => 'http://www.flickr.com/photos/bees/2341623661', + "oembed_get_request" => "http://www.flickr.com/services/oembed/?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://www.flickr.com/photos/bees/2341623661", + }, + + "unsupported" => { + "oembed_data" => {}, + "oembed_get_request" => 'http://www.we-do-not-support-oembed.com/index.html', + "link_url" => 'http://www.we-do-not-support-oembed.com/index.html', + "discovery_data" => 'no LINK tag!', + }, + + "secure_video" => { + "oembed_data" => { + "version" => "1.0", + "type" => "video", + "width" => 425, + "height" => 344, + "title" => "Amazing Nintendo Facts", + "html" => " + + + + + ", + }, + "link_url" => "http://youtube.com/watch?v=M3r2XDceM6A&format=json", + "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", + }, + + "unsecure_video" => { + "oembed_data" => { + "version" => "1.0", + "type" => "video", + "title" => "This is a video from an unsecure source", + "html" => " + + + + + ", + }, + "link_url" => "http://myrichtube.com/watch?v=M3r2XDceM6A&format=json", + "discovery_data" => '', + "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", + }, + + "secure_rich" => { + "oembed_data" => { + "version" => "1.0", + "type" => "rich", + "width" => 425, + "height" => 344, + "title" => "Amazing Nintendo Facts", + "html" => " + + + + + ", + }, + "link_url" => "http://yourichtube.com/watch?v=M3r2XDceM6A&format=json", + "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", + }, + + "unsecure_rich" => { + "oembed_data" => { + "version" => "1.0", + "type" => "rich", + "title" => "This is a video from an unsecure source", + "html" => " + + + + + ", + }, + "link_url" => "http://mytube.com/watch?v=M3r2XDceM6A&format=json", + "discovery_data" => '', + "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", + }, + } + scenarios.each do |type, data| + unless type=='unsupported' + url = data['oembed_get_request'].split('?')[0] + store_data = data['oembed_data'].merge('trusted_endpoint_url' => url) + OEmbedCache.new(:url => data['link_url'], :data => store_data.to_json); + end + end +end + +Then /^I should see a video player$/ do + page.has_css?('object') +end + +Then /^I should not see a video player$/ do + page.has_no_css?('object') +end + diff --git a/spec/helpers/markdownify_helper_spec.rb b/spec/helpers/markdownify_helper_spec.rb index af88b27ba..ca1641b96 100644 --- a/spec/helpers/markdownify_helper_spec.rb +++ b/spec/helpers/markdownify_helper_spec.rb @@ -162,8 +162,9 @@ describe MarkdownifyHelper do scenarios.each do |type, data| specify 'for type "'+type+'"' do - stub_request(:get, data['oembed_get_request']).to_return(:status => 200, :body => data['oembed_data'].to_json.to_s) + url = stub_request(:get, data['link_url']).to_return(:status => 200, :body => data['discovery_data']) if data.has_key?('discovery_data') + stub_request(:get, data['oembed_get_request']).to_return(:status => 200, :body => data['oembed_data'].to_json.to_s) message = "Look at this! "+data['link_url'] Jobs::GatherOEmbedData.perform(message) From b334a048bf36091907e4525dca21675e5d377c62 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Fri, 7 Oct 2011 14:21:32 -0700 Subject: [PATCH 3/8] merged --- db/schema.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/db/schema.rb b/db/schema.rb index 57297a4e7..5b997a392 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -178,6 +178,13 @@ ActiveRecord::Schema.define(:version => 20111003232053) do add_index "notifications", ["target_id"], :name => "index_notifications_on_target_id" add_index "notifications", ["target_type", "target_id"], :name => "index_notifications_on_target_type_and_target_id" + create_table "o_embed_caches", :force => true do |t| + t.string "url", :limit => 1024, :null => false + t.text "data", :null => false + end + + add_index "o_embed_caches", ["url"], :name => "index_o_embed_caches_on_url", :length => {"url"=>255} + create_table "oauth_access_tokens", :force => true do |t| t.integer "authorization_id", :null => false t.string "access_token", :limit => 32, :null => false From 963d5c1a691fb3b78df4dab5826ca0f8e7dabcce Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Fri, 7 Oct 2011 15:51:13 -0700 Subject: [PATCH 4/8] wip --- app/helpers/markdownify_helper.rb | 3 +- lib/diaspora/markdownify.rb | 68 ++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/app/helpers/markdownify_helper.rb b/app/helpers/markdownify_helper.rb index 4b87e6210..eba13fc7d 100644 --- a/app/helpers/markdownify_helper.rb +++ b/app/helpers/markdownify_helper.rb @@ -19,7 +19,6 @@ module MarkdownifyHelper render_options[:filter_html] = true render_options[:hard_wrap] ||= true - # This ugly little hack basically means # "Give me the rawest contents of target available" if target.respond_to?(:raw_message) @@ -34,8 +33,10 @@ module MarkdownifyHelper #renderer = Redcarpet::Render::HTML.new(render_options) if render_options[:oembed] + puts "oembed" renderer = Diaspora::Markdownify::HTMLwithOEmbed.new(render_options) else + puts "not oembed" renderer = Diaspora::Markdownify::HTML.new(render_options) end markdown = Redcarpet::Markdown.new(renderer, markdown_options) diff --git a/lib/diaspora/markdownify.rb b/lib/diaspora/markdownify.rb index bdd24da54..5958b195b 100644 --- a/lib/diaspora/markdownify.rb +++ b/lib/diaspora/markdownify.rb @@ -7,8 +7,74 @@ module Diaspora include ActionView::Helpers::TagHelper def autolink(link, type) - auto_link(link, :link => :urls) + auto_link(link, :link => :urls, :html => { :target => "_blank" }) end end + + class HTMLwithOEmbed < Redcarpet::Render::HTML + include ActionView::Helpers::UrlHelper + include ActionView::Helpers::TextHelper + include ActionView::Helpers::TagHelper + include ActionView::Helpers::AssetTagHelper + include ActionView::Helpers::RawOutputHelper + + def autolink(link, type) + #auto_link(link, :link => :urls, :html => { :target => "_blank" }) + + title = link + url = auto_link(link, :link => :urls).scan(/href=["']?((?:.(?!["']?\s+(?:\S+)=|[>"']))+.)["']?/).first.first + url = CGI::unescapeHTML(url) + + # SECURITY NOTICE! CROSS-SITE SCRIPTING! + # these endpoints may inject html code into our page + secure_endpoints = [::OEmbed::Providers::Youtube.endpoint, + ::OEmbed::Providers::Viddler.endpoint, + ::OEmbed::Providers::Qik.endpoint, + ::OEmbed::Providers::Revision3.endpoint, + ::OEmbed::Providers::Hulu.endpoint, + ::OEmbed::Providers::Vimeo.endpoint, + 'http://soundcloud.com/oembed', + ] + + # note that 'trusted_endpoint_url' is the only information + # in OEmbed that we can trust. anything else may be spoofed! + cache = OEmbedCache.find_by_url(url) + if not cache.nil? and cache.data.has_key?('type') + case cache.data['type'] + when 'video', 'rich' + if secure_endpoints.include?(cache.data['trusted_endpoint_url']) and cache.data.has_key?('html') + rep = raw(cache.data['html']) + elsif cache.data.has_key?('thumbnail_url') + img_options = {} + img_options.merge!({:height => cache.data['thumbnail_height'], + :width => cache.data['thumbnail_width']}) if cache.data.has_key?('thumbnail_width') and cache.data.has_key?('thumbnail_height') + img_options[:alt] = cache.data['title'] if cache.data.has_key?('title') + rep = link_to(image_tag(cache.data['thumbnail_url'], img_options), + url, :target => '_blank') + end + + when 'photo' + if cache.data.has_key?('url') + img_options = {} + img_options.merge!({:height => cache.data['height'], + :width => cache.data['width']}) if cache.data.has_key?('width') and cache.data.has_key?('height') + img_options[:alt] = cache.data['title'] if cache.data.has_key?('title') + rep = link_to(image_tag(cache.data['url'], img_options), + url, :target => '_blank') + end + else + puts "mega derp" + end + + title = cache.data['title'] \ + if cache.data.has_key?('title') and \ + not cache.data['title'].blank? + end + + rep ||= link_to(title, url, :target => '_blank') if rep.blank? + return rep + end + end +>>>>>>> wip end end From 139ddd726a8f103a52e7f0096b8e470d2bee0160 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Tue, 11 Oct 2011 13:16:06 -0700 Subject: [PATCH 5/8] wip oembed refactor. still need to make a oembed_helper, and move some tests to their new home, but e but we are looking preeeetttttyyyy good. --- app/helpers/markdownify_helper.rb | 9 +---- app/models/jobs/gather_o_embed_data.rb | 37 ++----------------- app/models/o_embed_cache.rb | 22 +++++++++++ app/models/post.rb | 7 +--- app/models/status_message.rb | 13 +++++++ config/initializers/oembed.rb | 18 ++++++++- ...0111011193702_add_oembed_cache_to_posts.rb | 9 +++++ db/schema.rb | 3 +- lib/diaspora/markdownify.rb | 15 +------- spec/models/status_message_spec.rb | 8 ++++ 10 files changed, 79 insertions(+), 62 deletions(-) create mode 100644 db/migrate/20111011193702_add_oembed_cache_to_posts.rb diff --git a/app/helpers/markdownify_helper.rb b/app/helpers/markdownify_helper.rb index eba13fc7d..9ba17f019 100644 --- a/app/helpers/markdownify_helper.rb +++ b/app/helpers/markdownify_helper.rb @@ -31,14 +31,7 @@ module MarkdownifyHelper return '' if message.blank? - #renderer = Redcarpet::Render::HTML.new(render_options) - if render_options[:oembed] - puts "oembed" - renderer = Diaspora::Markdownify::HTMLwithOEmbed.new(render_options) - else - puts "not oembed" - renderer = Diaspora::Markdownify::HTML.new(render_options) - end + renderer = Diaspora::Markdownify::HTML.new(render_options) markdown = Redcarpet::Markdown.new(renderer, markdown_options) message = markdown.render(message).html_safe diff --git a/app/models/jobs/gather_o_embed_data.rb b/app/models/jobs/gather_o_embed_data.rb index fd07f7462..72fe83d29 100644 --- a/app/models/jobs/gather_o_embed_data.rb +++ b/app/models/jobs/gather_o_embed_data.rb @@ -7,39 +7,10 @@ module Jobs class GatherOEmbedData < Base @queue = :http_service - class GatherOEmbedRenderer < Redcarpet::Render::HTML - include ActionView::Helpers::TextHelper - include ActionView::Helpers::TagHelper - - def isHttp?(url) - URI.parse(url).scheme.downcase == 'http' - end - - def autolink(link, type) - url = auto_link(link, :link => :urls).scan(/href=["']?((?:.(?!["']?\s+(?:\S+)=|[>"']))+.)["']?/).first.first - url = CGI::unescapeHTML(url) - - return url if OEmbedCache.exists?(:url => url) or not isHttp?(url) - - begin - res = ::OEmbed::Providers.get(url, {:maxwidth => 420, :maxheight => 420, :frame => 1, :iframe => 1}) - rescue Exception => e - # noop - else - data = res.fields - data['trusted_endpoint_url'] = res.provider.endpoint - cache = OEmbedCache.new(:url => url, :data => data) - cache.save - end - - return url - end - end - - def self.perform(text) - renderer = GatherOEmbedRenderer.new({}) - markdown = Redcarpet::Markdown.new(renderer, {:autolink => true}) - message = markdown.render(text) + def self.perform(post_id, url) + post = Post.find(post_id) + post.o_embed_cache = OEmbedCache.find_or_create_by_url(url) + post.save end end end diff --git a/app/models/o_embed_cache.rb b/app/models/o_embed_cache.rb index 79f472f8c..6ae29b8a3 100644 --- a/app/models/o_embed_cache.rb +++ b/app/models/o_embed_cache.rb @@ -1,3 +1,25 @@ class OEmbedCache < ActiveRecord::Base serialize :data + attr_accessible :url + + has_many :posts + + def self.find_or_create_by_url(url) + cache = OEmbedCache.find_or_build_by_url(url) + return cache if cache.persisted? + cache.fetch_and_save_oembed_data! + cache + end + + def fetch_and_save_oembed_data! + begin + response = OEmbed::Providers.get(self.url, {:maxwidth => 420, :maxheight => 420, :frame => 1, :iframe => 1}) + rescue Exception => e + # noop + else + self.data = response.fields + self.data['trusted_endpoint_url'] = response.provider.endpoint + cache.save + end + end end diff --git a/app/models/post.rb b/app/models/post.rb index 20613b5a5..7cf9d4185 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -28,6 +28,8 @@ class Post < ActiveRecord::Base has_many :reshares, :class_name => "Reshare", :foreign_key => :root_guid, :primary_key => :guid has_many :resharers, :class_name => 'Person', :through => :reshares, :source => :author + has_one :o_embed_cache + belongs_to :author, :class_name => 'Person' validates :guid, :uniqueness => true @@ -66,11 +68,6 @@ class Post < ActiveRecord::Base @reshare_count ||= Post.where(:root_guid => self.guid).count end - def text= nd - Resque.enqueue(Jobs::GatherOEmbedData, nd) - write_attribute(:text, nd) - end - def diaspora_handle= nd self.author = Person.where(:diaspora_handle => nd).first write_attribute(:diaspora_handle, nd) diff --git a/app/models/status_message.rb b/app/models/status_message.rb index 1c0f6b0be..72ac5cf67 100644 --- a/app/models/status_message.rb +++ b/app/models/status_message.rb @@ -22,10 +22,13 @@ class StatusMessage < Post validate :presence_of_content attr_accessible :text, :provider_display_name + attr_accessor :oembed_url serialize :youtube_titles, Hash after_create :create_mentions + after_create :queue_gather_oembed_data, :if => :contains_oembed_url_in_text? + #scopes scope :where_person_is_mentioned, lambda{|person| joins(:mentions).where(:mentions => {:person_id => person.id})} @@ -153,6 +156,16 @@ class StatusMessage < Post self.text.blank? && self.photos.blank? end + def queue_gather_oembed_data + Resque.enqueue(Jobs::GatherOEmbedData, self.oembed_url) + end + + def contains_oembed_url_in_text? + require 'uri' + urls = URI.extract(self.raw_message, ['http', 'https']) + self.oembed_url = urls.find{|url| ENDPOINT_HOSTS_STRING.match(URI.parse(url).host)} + end + protected def presence_of_content diff --git a/config/initializers/oembed.rb b/config/initializers/oembed.rb index 5db195434..8947649c8 100644 --- a/config/initializers/oembed.rb +++ b/config/initializers/oembed.rb @@ -1,4 +1,20 @@ require 'oembed' +require 'uri' + OEmbed::Providers.register_all OEmbed::Providers.register_fallback(OEmbed::ProviderDiscovery) - +# +# SECURITY NOTICE! CROSS-SITE SCRIPTING! +# these endpoints may inject html code into our page +# note that 'trusted_endpoint_url' is the only information +# in OEmbed that we can trust. anything else may be spoofed! +SECURE_ENDPOINTS = [::OEmbed::Providers::Youtube.endpoint, + ::OEmbed::Providers::Viddler.endpoint, + ::OEmbed::Providers::Qik.endpoint, + ::OEmbed::Providers::Revision3.endpoint, + ::OEmbed::Providers::Hulu.endpoint, + ::OEmbed::Providers::Vimeo.endpoint, + 'http://soundcloud.com/oembed', + 'http://cubbi.es/oembed' + ] +ENDPOINT_HOSTS_STRING = SECURE_ENDPOINTS.map{|e| URI.parse(e.split('{')[0]).host}.to_s diff --git a/db/migrate/20111011193702_add_oembed_cache_to_posts.rb b/db/migrate/20111011193702_add_oembed_cache_to_posts.rb new file mode 100644 index 000000000..371faef7c --- /dev/null +++ b/db/migrate/20111011193702_add_oembed_cache_to_posts.rb @@ -0,0 +1,9 @@ +class AddOembedCacheToPosts < ActiveRecord::Migration + def self.up + add_column :posts, :o_embed_cache_id, :integer + end + + def self.down + remove_column :posts, :o_embed_cache_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 5b997a392..547f8cf09 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20111003232053) do +ActiveRecord::Schema.define(:version => 20111011193702) do create_table "aspect_memberships", :force => true do |t| t.integer "aspect_id", :null => false @@ -290,6 +290,7 @@ ActiveRecord::Schema.define(:version => 20111003232053) do t.string "status_message_guid" t.integer "likes_count", :default => 0 t.integer "comments_count", :default => 0 + t.integer "o_embed_cache_id" end add_index "posts", ["author_id", "root_guid"], :name => "index_posts_on_author_id_and_root_guid", :unique => true diff --git a/lib/diaspora/markdownify.rb b/lib/diaspora/markdownify.rb index 5958b195b..974f97e8a 100644 --- a/lib/diaspora/markdownify.rb +++ b/lib/diaspora/markdownify.rb @@ -25,24 +25,11 @@ module Diaspora url = auto_link(link, :link => :urls).scan(/href=["']?((?:.(?!["']?\s+(?:\S+)=|[>"']))+.)["']?/).first.first url = CGI::unescapeHTML(url) - # SECURITY NOTICE! CROSS-SITE SCRIPTING! - # these endpoints may inject html code into our page - secure_endpoints = [::OEmbed::Providers::Youtube.endpoint, - ::OEmbed::Providers::Viddler.endpoint, - ::OEmbed::Providers::Qik.endpoint, - ::OEmbed::Providers::Revision3.endpoint, - ::OEmbed::Providers::Hulu.endpoint, - ::OEmbed::Providers::Vimeo.endpoint, - 'http://soundcloud.com/oembed', - ] - - # note that 'trusted_endpoint_url' is the only information - # in OEmbed that we can trust. anything else may be spoofed! cache = OEmbedCache.find_by_url(url) if not cache.nil? and cache.data.has_key?('type') case cache.data['type'] when 'video', 'rich' - if secure_endpoints.include?(cache.data['trusted_endpoint_url']) and cache.data.has_key?('html') + if SECURE_ENDPOINTS.include?(cache.data['trusted_endpoint_url']) and cache.data.has_key?('html') rep = raw(cache.data['html']) elsif cache.data.has_key?('thumbnail_url') img_options = {} diff --git a/spec/models/status_message_spec.rb b/spec/models/status_message_spec.rb index b4d653488..5f1e1a4ac 100644 --- a/spec/models/status_message_spec.rb +++ b/spec/models/status_message_spec.rb @@ -312,4 +312,12 @@ STR @status_message.after_dispatch(alice) end end + + describe '#contains_url_in_text?' do + it 'returns an array of all urls found in the raw message' do + sm = Factory(:status_message, :text => 'http://youtube.com is so cool. so is https://joindiaspora.com') + sm.contains_oembed_url_in_text?.should_not be_nil + sm.oembed_url.should == 'http://youtube.com' + end + end end From cfb28db00fef93c4899d9f290e2e4f364f28e2f7 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Tue, 11 Oct 2011 14:29:18 -0700 Subject: [PATCH 6/8] wip, but the refactored code works --- app/helpers/o_embed_helper.rb | 32 +++++++++++ app/models/o_embed_cache.rb | 21 +++++++- app/models/post.rb | 2 +- app/models/status_message.rb | 2 +- .../status_messages/_status_message.haml | 2 +- .../status_messages/_status_message.html.haml | 2 + lib/diaspora/markdownify.rb | 53 ------------------- 7 files changed, 56 insertions(+), 58 deletions(-) create mode 100644 app/helpers/o_embed_helper.rb diff --git a/app/helpers/o_embed_helper.rb b/app/helpers/o_embed_helper.rb new file mode 100644 index 000000000..aa6559faa --- /dev/null +++ b/app/helpers/o_embed_helper.rb @@ -0,0 +1,32 @@ +module OEmbedHelper + def o_embed_html(cache) + data = cache.data + title = data.fetch('title', 'an awesome post') + html ||= link_to(title, cache.url, :target => '_blank') + return nil unless data.has_key?('type') + case data['type'] + when 'video', 'rich' + if cache.is_trusted_and_has_html? + html = data['html'] + elsif data.has_key?('thumbnail_url') + html = link_to_oembed_image(cache) + end + when 'photo' + if data.has_key?('url') + img_options = cache.options_hash('') + html = link_to_oembed_image(cache, '') + end + else + end + + return html.html_safe + end + + def link_to_oembed_image(cache, prefix = 'thumbnail_') + link_to(oembed_image_tag(cache, prefix), cache.url, :target => '_blank') + end + + def oembed_image_tag(cache, prefix) + image_tag(cache.data[prefix + 'url'], cache.image_options_hash(prefix)) + end +end diff --git a/app/models/o_embed_cache.rb b/app/models/o_embed_cache.rb index 6ae29b8a3..277564d8b 100644 --- a/app/models/o_embed_cache.rb +++ b/app/models/o_embed_cache.rb @@ -5,7 +5,7 @@ class OEmbedCache < ActiveRecord::Base has_many :posts def self.find_or_create_by_url(url) - cache = OEmbedCache.find_or_build_by_url(url) + cache = OEmbedCache.find_or_initialize_by_url(url) return cache if cache.persisted? cache.fetch_and_save_oembed_data! cache @@ -19,7 +19,24 @@ class OEmbedCache < ActiveRecord::Base else self.data = response.fields self.data['trusted_endpoint_url'] = response.provider.endpoint - cache.save + self.save end end + + def is_trusted_and_has_html? + self.from_trusted? and self.data.has_key?('html') + end + + def from_trusted? + SECURE_ENDPOINTS.include?(self.data['trusted_endpoint_url']) + end + + def options_hash(prefix = 'thumbnail_') + return nil unless self.data.has_key?(prefix + 'url') + { + :height => self.data.fetch(prefix + 'height', ''), + :width => self.data.fetch(prefix + 'width', ''), + :alt => self.data.fetch('title', ''), + } + end end diff --git a/app/models/post.rb b/app/models/post.rb index 7cf9d4185..9199a5b15 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -28,7 +28,7 @@ class Post < ActiveRecord::Base has_many :reshares, :class_name => "Reshare", :foreign_key => :root_guid, :primary_key => :guid has_many :resharers, :class_name => 'Person', :through => :reshares, :source => :author - has_one :o_embed_cache + belongs_to :o_embed_cache belongs_to :author, :class_name => 'Person' diff --git a/app/models/status_message.rb b/app/models/status_message.rb index 72ac5cf67..9054eaf61 100644 --- a/app/models/status_message.rb +++ b/app/models/status_message.rb @@ -157,7 +157,7 @@ class StatusMessage < Post end def queue_gather_oembed_data - Resque.enqueue(Jobs::GatherOEmbedData, self.oembed_url) + Resque.enqueue(Jobs::GatherOEmbedData, self.id, self.oembed_url) end def contains_oembed_url_in_text? diff --git a/app/views/status_messages/_status_message.haml b/app/views/status_messages/_status_message.haml index 2a6347d66..6048f085a 100644 --- a/app/views/status_messages/_status_message.haml +++ b/app/views/status_messages/_status_message.haml @@ -16,4 +16,4 @@ = link_to (image_tag photo.url(:thumb_small), :class => 'stream-photo thumb_small', 'data-small-photo' => photo.url(:thumb_medium), 'data-full-photo' => photo.url), photo_path(photo), :class => 'stream-photo-link' %div{:class => direction_for(post.text)} - != markdownify(post, :oembed => true, :youtube_maps => post[:youtube_titles]) + != markdownify(post, :youtube_maps => post[:youtube_titles]) diff --git a/app/views/status_messages/_status_message.html.haml b/app/views/status_messages/_status_message.html.haml index ec8343327..193b13b66 100644 --- a/app/views/status_messages/_status_message.html.haml +++ b/app/views/status_messages/_status_message.html.haml @@ -17,3 +17,5 @@ %div{:class => direction_for(post.text)} != markdownify(post, :youtube_maps => post[:youtube_titles]) + - if post.o_embed_cache_id.present? + = o_embed_html(post.o_embed_cache) diff --git a/lib/diaspora/markdownify.rb b/lib/diaspora/markdownify.rb index 974f97e8a..0349db4ed 100644 --- a/lib/diaspora/markdownify.rb +++ b/lib/diaspora/markdownify.rb @@ -10,58 +10,5 @@ module Diaspora auto_link(link, :link => :urls, :html => { :target => "_blank" }) end end - - class HTMLwithOEmbed < Redcarpet::Render::HTML - include ActionView::Helpers::UrlHelper - include ActionView::Helpers::TextHelper - include ActionView::Helpers::TagHelper - include ActionView::Helpers::AssetTagHelper - include ActionView::Helpers::RawOutputHelper - - def autolink(link, type) - #auto_link(link, :link => :urls, :html => { :target => "_blank" }) - - title = link - url = auto_link(link, :link => :urls).scan(/href=["']?((?:.(?!["']?\s+(?:\S+)=|[>"']))+.)["']?/).first.first - url = CGI::unescapeHTML(url) - - cache = OEmbedCache.find_by_url(url) - if not cache.nil? and cache.data.has_key?('type') - case cache.data['type'] - when 'video', 'rich' - if SECURE_ENDPOINTS.include?(cache.data['trusted_endpoint_url']) and cache.data.has_key?('html') - rep = raw(cache.data['html']) - elsif cache.data.has_key?('thumbnail_url') - img_options = {} - img_options.merge!({:height => cache.data['thumbnail_height'], - :width => cache.data['thumbnail_width']}) if cache.data.has_key?('thumbnail_width') and cache.data.has_key?('thumbnail_height') - img_options[:alt] = cache.data['title'] if cache.data.has_key?('title') - rep = link_to(image_tag(cache.data['thumbnail_url'], img_options), - url, :target => '_blank') - end - - when 'photo' - if cache.data.has_key?('url') - img_options = {} - img_options.merge!({:height => cache.data['height'], - :width => cache.data['width']}) if cache.data.has_key?('width') and cache.data.has_key?('height') - img_options[:alt] = cache.data['title'] if cache.data.has_key?('title') - rep = link_to(image_tag(cache.data['url'], img_options), - url, :target => '_blank') - end - else - puts "mega derp" - end - - title = cache.data['title'] \ - if cache.data.has_key?('title') and \ - not cache.data['title'].blank? - end - - rep ||= link_to(title, url, :target => '_blank') if rep.blank? - return rep - end - end ->>>>>>> wip end end From 8acd9acb08944c92f34e264f7a744c160e1f3bf2 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Tue, 11 Oct 2011 16:00:10 -0700 Subject: [PATCH 7/8] tests moved and passing --- app/helpers/o_embed_helper.rb | 9 +- spec/helpers/markdownify_helper_spec.rb | 129 ------------------------ spec/helpers/o_embed_helper_spec.rb | 128 +++++++++++++++++++++++ spec/models/post_spec.rb | 11 -- spec/models/status_message_spec.rb | 8 ++ 5 files changed, 141 insertions(+), 144 deletions(-) create mode 100644 spec/helpers/o_embed_helper_spec.rb diff --git a/app/helpers/o_embed_helper.rb b/app/helpers/o_embed_helper.rb index aa6559faa..70d489e62 100644 --- a/app/helpers/o_embed_helper.rb +++ b/app/helpers/o_embed_helper.rb @@ -1,9 +1,10 @@ module OEmbedHelper def o_embed_html(cache) data = cache.data - title = data.fetch('title', 'an awesome post') - html ||= link_to(title, cache.url, :target => '_blank') - return nil unless data.has_key?('type') + data = {} if data.blank? + title = data.fetch('title', cache.url) + html = link_to(title, cache.url, :target => '_blank') + return html unless data.has_key?('type') case data['type'] when 'video', 'rich' if cache.is_trusted_and_has_html? @@ -27,6 +28,6 @@ module OEmbedHelper end def oembed_image_tag(cache, prefix) - image_tag(cache.data[prefix + 'url'], cache.image_options_hash(prefix)) + image_tag(cache.data[prefix + 'url'], cache.options_hash(prefix)) end end diff --git a/spec/helpers/markdownify_helper_spec.rb b/spec/helpers/markdownify_helper_spec.rb index ca1641b96..ed73c7d1b 100644 --- a/spec/helpers/markdownify_helper_spec.rb +++ b/spec/helpers/markdownify_helper_spec.rb @@ -58,135 +58,6 @@ describe MarkdownifyHelper do formatted = markdownify(message) formatted.should =~ /hovercard/ end - - context 'when posting a link with oEmbed support' do - scenarios = { - "photo" => { - "oembed_data" => { - "trusted_endpoint_url" => "__!SPOOFED!__", - "version" => "1.0", - "type" => "photo", - "title" => "ZB8T0193", - "width" => "240", - "height" => "160", - "url" => "http://farm4.static.flickr.com/3123/2341623661_7c99f48bbf_m.jpg" - }, - "link_url" => 'http://www.flickr.com/photos/bees/2341623661', - "oembed_get_request" => "http://www.flickr.com/services/oembed/?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://www.flickr.com/photos/bees/2341623661", - }, - - "unsupported" => { - "oembed_data" => "", - "oembed_get_request" => 'http://www.we-do-not-support-oembed.com/index.html', - "link_url" => 'http://www.we-do-not-support-oembed.com/index.html' - }, - - "secure_video" => { - "oembed_data" => { - "version" => "1.0", - "type" => "video", - "width" => 425, - "height" => 344, - "title" => "Amazing Nintendo Facts", - "html" => " - - - - - ", - }, - "link_url" => "http://youtube.com/watch?v=M3r2XDceM6A&format=json", - "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", - }, - - "unsecure_video" => { - "oembed_data" => { - "version" => "1.0", - "type" => "video", - "title" => "This is a video from an unsecure source", - "html" => " - - - - - ", - }, - "link_url" => "http://mytube.com/watch?v=M3r2XDceM6A&format=json", - "discovery_data" => '', - "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", - }, - - "secure_rich" => { - "oembed_data" => { - "version" => "1.0", - "type" => "rich", - "width" => 425, - "height" => 344, - "title" => "Amazing Nintendo Facts", - "html" => " - - - - - ", - }, - "link_url" => "http://youtube.com/watch?v=M3r2XDceM6A&format=json", - "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", - }, - - "unsecure_rich" => { - "oembed_data" => { - "version" => "1.0", - "type" => "rich", - "title" => "This is a video from an unsecure source", - "html" => " - - - - - ", - }, - "link_url" => "http://mytube.com/watch?v=M3r2XDceM6A&format=json", - "discovery_data" => '', - "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", - }, - } - - scenarios.each do |type, data| - specify 'for type "'+type+'"' do - url = - stub_request(:get, data['link_url']).to_return(:status => 200, :body => data['discovery_data']) if data.has_key?('discovery_data') - stub_request(:get, data['oembed_get_request']).to_return(:status => 200, :body => data['oembed_data'].to_json.to_s) - - message = "Look at this! "+data['link_url'] - Jobs::GatherOEmbedData.perform(message) - OEmbedCache.find_by_url(data['link_url']).should_not be_nil unless type == 'unsupported' - - formatted = markdownify(message, :oembed => true) - case type - when 'photo' - formatted.should =~ /#{data['oembed_data']['url']}/ - when 'unsupported' - formatted.should =~ /#{data['link_url']}/ - when 'secure_video', 'secure_rich' - formatted.should =~ /#{data['oembed_data']['html']}/ - when 'unsecure_video', 'unsecure_rich' - formatted.should_not =~ /#{data['oembed_data']['html']}/ - formatted.should =~ /#{data['oembed_data']['title']}/ - formatted.should =~ /#{data['oembed_data']['url']}/ - end - end - end - - end end end end diff --git a/spec/helpers/o_embed_helper_spec.rb b/spec/helpers/o_embed_helper_spec.rb new file mode 100644 index 000000000..b984e8d51 --- /dev/null +++ b/spec/helpers/o_embed_helper_spec.rb @@ -0,0 +1,128 @@ +require 'spec_helper' + +describe OEmbedHelper do + describe 'o_embed_html' do + scenarios = { + "photo" => { + "oembed_data" => { + "trusted_endpoint_url" => "__!SPOOFED!__", + "version" => "1.0", + "type" => "photo", + "title" => "ZB8T0193", + "width" => "240", + "height" => "160", + "url" => "http://farm4.static.flickr.com/3123/2341623661_7c99f48bbf_m.jpg" + }, + "link_url" => 'http://www.flickr.com/photos/bees/2341623661', + "oembed_get_request" => "http://www.flickr.com/services/oembed/?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://www.flickr.com/photos/bees/2341623661", + }, + + "unsupported" => { + "oembed_data" => "", + "oembed_get_request" => 'http://www.we-do-not-support-oembed.com/index.html', + "link_url" => 'http://www.we-do-not-support-oembed.com/index.html' + }, + + "secure_video" => { + "oembed_data" => { + "version" => "1.0", + "type" => "video", + "width" => 425, + "height" => 344, + 'trusted_endpoint_url' => ::OEmbed::Providers::Youtube.endpoint, + "title" => "Amazing Nintendo Facts", + "html" => " + + + + + ", + }, + "link_url" => "http://youtube.com/watch?v=M3r2XDceM6A&format=json", + "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", + }, + + "unsecure_video" => { + "oembed_data" => { + "version" => "1.0", + "type" => "video", + "title" => "This is a video from an unsecure source", + "html" => " + + + + + ", + }, + "link_url" => "http://mytube.com/watch?v=M3r2XDceM6A&format=json", + "discovery_data" => '', + "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", + }, + + "secure_rich" => { + "oembed_data" => { + "version" => "1.0", + "type" => "rich", + "width" => 425, + "height" => 344, + 'trusted_endpoint_url' => ::OEmbed::Providers::Youtube.endpoint, + "title" => "Amazing Nintendo Facts", + "html" => " + + + + + ", + }, + "link_url" => "http://youtube.com/watch?v=M3r2XDceM6A&format=json", + "oembed_get_request" => "http://www.youtube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://youtube.com/watch?v=M3r2XDceM6A", + }, + + "unsecure_rich" => { + "oembed_data" => { + "version" => "1.0", + "type" => "rich", + "title" => "This is a video from an unsecure source", + "html" => " + + + + + ", + }, + "link_url" => "http://mytube.com/watch?v=M3r2XDceM6A&format=json", + "discovery_data" => '', + "oembed_get_request" => "http://www.mytube.com/oembed?format=json&frame=1&iframe=1&maxheight=420&maxwidth=420&url=http://mytube.com/watch?v=M3r2XDceM6A", + }, + } + + scenarios.each do |type, data| + specify 'for type "'+type+'"' do + real_data = data['oembed_data'] + cache = OEmbedCache.new(:url => data['link_url']) + cache.data = real_data + formatted = o_embed_html(cache) + case type + when 'photo' + formatted.should =~ /#{data['oembed_data']['url']}/ + when 'unsupported' + formatted.should =~ /#{data['link_url']}/ + when 'secure_video', 'secure_rich' + formatted.should =~ /#{data['oembed_data']['html']}/ + when 'unsecure_video', 'unsecure_rich' + formatted.should_not =~ /#{data['oembed_data']['html']}/ + formatted.should =~ /#{data['oembed_data']['title']}/ + formatted.should =~ /#{data['oembed_data']['url']}/ + end + end + end + end +end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 10a769bd8..fade3ab7a 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -68,17 +68,6 @@ describe Post do end end - describe 'oEmbed' do - it 'should gather oEmbed data' do - stub_request(:get, "http://gdata.youtube.com/feeds/api/videos/M3r2XDceM6A?v=2").to_return(:status => 200, :body => "") - - text = 'http://youtube.com/watch?v=M3r2XDceM6A&format=json' - Resque.should_receive(:enqueue).with(Jobs::GatherOEmbedData, text) - post = Factory.create(:status_message, :author => @user.person, :text => text) - post.save! - end - end - describe 'serialization' do it 'should serialize the handle and not the sender' do post = @user.post :status_message, :text => "hello", :to => @aspect.id diff --git a/spec/models/status_message_spec.rb b/spec/models/status_message_spec.rb index 5f1e1a4ac..005e0afd0 100644 --- a/spec/models/status_message_spec.rb +++ b/spec/models/status_message_spec.rb @@ -320,4 +320,12 @@ STR sm.oembed_url.should == 'http://youtube.com' end end + + describe 'oembed' do + it 'should queue a GatherOembedData if it includes a link' do + sm = Factory.build(:status_message, :text => 'http://youtube.com is so cool. so is https://joindiaspora.com') + Resque.should_receive(:enqueue).with(Jobs::GatherOEmbedData, instance_of(Fixnum), instance_of(String)) + sm.save + end + end end From 9fd1d7db630719dcebac5892e850c2c5fa0a5842 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Tue, 11 Oct 2011 16:53:37 -0700 Subject: [PATCH 8/8] updating gemfile and schema.rb --- Gemfile.lock | 2 ++ app/models/post.rb | 2 +- spec/controllers/tags_controller_spec.rb | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 68bab4162..c1982af08 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -406,6 +406,7 @@ GEM linecache19 (>= 0.5.11) ruby-debug-base19 (>= 0.11.19) ruby-hmac (0.4.0) + ruby-oembed (0.8.3) ruby-openid (2.1.8) ruby-openid-apps-discovery (1.2.0) ruby-openid (>= 2.1.7) @@ -532,6 +533,7 @@ DEPENDENCIES rspec-rails (>= 2.0.0) ruby-debug ruby-debug19 + ruby-oembed sass (= 3.1.7) selenium-webdriver (~> 2.7.0) settingslogic (= 2.0.6) diff --git a/app/models/post.rb b/app/models/post.rb index 9199a5b15..c9929185b 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -38,7 +38,7 @@ class Post < ActiveRecord::Base #scopes scope :all_public, where(:public => true, :pending => false) - scope :includes_for_a_stream, includes({:author => :profile}, :mentions => {:person => :profile}) #note should include root and photos, but i think those are both on status_message + scope :includes_for_a_stream, includes(:o_embed_cache, {:author => :profile}, :mentions => {:person => :profile}) #note should include root and photos, but i think those are both on status_message def self.for_a_stream(max_time, order) by_max_time(max_time, order). diff --git a/spec/controllers/tags_controller_spec.rb b/spec/controllers/tags_controller_spec.rb index 16007efaa..d79ccec7d 100644 --- a/spec/controllers/tags_controller_spec.rb +++ b/spec/controllers/tags_controller_spec.rb @@ -16,7 +16,6 @@ describe TagsController do it 'responds with json' do get :index, :q => "ra", :format => 'json' #parse json - response.body.should include("#diaspora") response.body.should include("#rad") end