Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Expand urls in tweets #43

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/presenters/tweet/html_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,20 @@ def text
buffer = ActiveSupport::SafeBuffer.new
buffer << tweet.tweet_text

# make links anchors
buffer = buffer.gsub(/(https?[^\s]+)/o) do |url|
require Rails.root.join('lib/expand_url')
begin
expanded_url = ExpandUrl.expand_url(url)
rescue ExpandUrl::ExpansionError => e
STDERR.puts "#{e.class}: failed expanding #{url.inspect}"
expanded_url = url
end
%Q(<a href="#{expanded_url}" target="_blank">#{url}</a>)
end
# link hashtags
buffer.gsub! /#(\w+)/, '<a href="http://twitter.com/search?q=%23\\1">#\\1</a>'
# link users
buffer.gsub! /@(\w+)/, '<a href="http://twitter.com/\\1">@\\1</a>'

buffer
Expand Down
1 change: 1 addition & 0 deletions config/application.example.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Add application configuration variables here, as shown below.
#
# Rails config
debug: false
SECRET_TOKEN: f79b0679785f11473630bc31e8941adc3897d8f2ca91261bb68f782c2c665b2d0d45db4c1c9ee43a50f69f620c152a3cd91029d969f2a9072bf98f7b3ee6bfb6
# Twitter config
consumer_key: xxx
Expand Down
58 changes: 58 additions & 0 deletions lib/expand_url.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
require 'net/http'
require 'net/https'
require 'uri'

# e.g.
# url = http://t.co/z4t0E1vArh
# ExpandUrl.expand_url(url)
# => "http://www.haaretz.com/news/national/israel-s-ag-impels-ministers-to-crack-down-on-exclusion-of-women.premium-1.519917"
module ExpandUrl
class ExpansionError < StandardError; end
module ExpansionErrors
class BadUrl < ExpansionError; end
class BadResponse < ExpansionError; end
end
HTTP_ERRORS = [Timeout::Error, Errno::EINVAL, Errno::ECONNRESET,
Net::HTTPBadResponse, Net::HTTPHeaderSyntaxError, Net::ProtocolError]
class BasicResponse < Struct.new(:url, :code); end
extend self

# raises ExpandUrl::ExpansionError
def expand_url(url)
response = get_response(url)
case response.code
when '301'
log "url: #{url}\tresponse: #{response.inspect}"
expand_url(response['location'])
when '200'
log "url: #{url}\tresponse: #{response.inspect}"
url
else
log "url: #{url}\tresponse: #{response.inspect}"
expand_url(response['location'])
end
end

def get_response(url)
uri = url_to_uri(url)
Net::HTTP.get_response(uri)
rescue EOFError => e
BasicResponse.new(url, '200')
rescue *HTTP_ERRORS => e
log url.inspect + e.inspect
raise ExpansionErrors::BadResponse.new(e)
end

def url_to_uri(url)
begin
uri = URI.parse(url)
rescue URI::InvalidURIError, SocketError => e
raise ExpansionErrors::BadUrl.new(e)
end
end

def log(msg)
STDOUT.puts "#{msg}\t#{caller[1]}" if (ENV['debug'] == 'true')
end

end
7 changes: 7 additions & 0 deletions spec/lib/expand_url_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
require 'spec_helper'

describe ExpandUrl do

it 'should be tested'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Pretty please?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure. I was thinking of also moving the expansion to the tweet processing leve, but since that would be destructive, was considering adding an 'original tweet text' field. Unrelated, I want to add a 'context' field that represents the search term the tweet was found from, then... eventually extract most of this as an engine.. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm rewriting this to add a tweet_display_text field that is populated at import time.. should I continue to update this pull request?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just make it easy to merge and I'll do so.. Keep adding tests and I don't mind merging at a faster rate, given the feature is not going to bloat the code base -- aka, it is derived from something needed, with a reason that outweighs the expense of maintenance moving forward..


end
10 changes: 8 additions & 2 deletions spec/presenters/tweet/html_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@
subject { presenter }

let(:presenter) { described_class.new tweet }
let(:tweet_link) {
link = 'http://t.co/cyL9StoS'
ExpandUrl.stub(:expand_url).with(link).and_return(link)
link
}
let(:tweet) {
Tweet.new tweet_id: "263515718753079296",
tweet_text: "at @SteelCityRuby with @coreyhaines - one of my favorite #rubyfriends http://t.co/cyL9StoS",
Tweet.new(tweet_id: "263515718753079296",
tweet_text: "at @SteelCityRuby with @coreyhaines - one of my favorite #rubyfriends #{tweet_link}",
username: "joshsusser",
media_url: "http://p.twimg.com/A6gyJmlCUAA9Il2.jpg",
image: "A6gyJmlCUAA9Il2.jpg",
media_display_url: "pic.twitter.com/cyL9StoS"
)
}

its(:username) { should == "@joshsusser" }
Expand Down