From f0de621e76b5a5ba3f7e67bd88c0183aac22b985 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 20 Jan 2017 01:00:14 +0100 Subject: [PATCH] Fix #463 - Fetch and display previews of URLs using OpenGraph tags --- Gemfile | 1 + Gemfile.lock | 2 + .../javascripts/components/actions/cards.jsx | 40 ++++++++ .../components/actions/statuses.jsx | 2 + .../features/status/components/card.jsx | 96 +++++++++++++++++++ .../status/components/detailed_status.jsx | 3 + .../status/containers/card_container.jsx | 8 ++ .../javascripts/components/reducers/cards.jsx | 14 +++ .../javascripts/components/reducers/index.jsx | 4 +- app/assets/stylesheets/components.scss | 6 ++ app/controllers/api/v1/statuses_controller.rb | 8 +- app/models/preview_card.rb | 20 ++++ app/models/status.rb | 1 + app/services/fetch_link_card_service.rb | 33 +++++++ app/services/post_status_service.rb | 1 + app/views/api/v1/statuses/card.rabl | 5 + app/workers/link_crawl_worker.rb | 13 +++ config/application.rb | 3 +- config/initializers/inflections.rb | 1 + config/routes.rb | 3 +- .../20170119214911_create_preview_cards.rb | 17 ++++ db/schema.rb | 16 +++- {app/lib => lib}/statsd_monitor.rb | 0 spec/fabricators/preview_card_fabricator.rb | 5 + spec/models/preview_card_spec.rb | 5 + spec/models/subscription_spec.rb | 2 +- 26 files changed, 302 insertions(+), 7 deletions(-) create mode 100644 app/assets/javascripts/components/actions/cards.jsx create mode 100644 app/assets/javascripts/components/features/status/components/card.jsx create mode 100644 app/assets/javascripts/components/features/status/containers/card_container.jsx create mode 100644 app/assets/javascripts/components/reducers/cards.jsx create mode 100644 app/models/preview_card.rb create mode 100644 app/services/fetch_link_card_service.rb create mode 100644 app/views/api/v1/statuses/card.rabl create mode 100644 app/workers/link_crawl_worker.rb create mode 100644 db/migrate/20170119214911_create_preview_cards.rb rename {app/lib => lib}/statsd_monitor.rb (100%) create mode 100644 spec/fabricators/preview_card_fabricator.rb create mode 100644 spec/models/preview_card_spec.rb diff --git a/Gemfile b/Gemfile index aa149c61e5..bab7cebb56 100644 --- a/Gemfile +++ b/Gemfile @@ -48,6 +48,7 @@ gem 'rails-settings-cached' gem 'pg_search' gem 'simple-navigation' gem 'statsd-instrument' +gem 'ruby-oembed', require: 'oembed' gem 'react-rails' gem 'browserify-rails' diff --git a/Gemfile.lock b/Gemfile.lock index 9b33580fcb..20ea37fcc4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -334,6 +334,7 @@ GEM rainbow (>= 1.99.1, < 3.0) ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) + ruby-oembed (0.10.1) ruby-progressbar (1.8.1) safe_yaml (1.0.4) sass (3.4.22) @@ -457,6 +458,7 @@ DEPENDENCIES rspec-rails rspec-sidekiq rubocop + ruby-oembed sass-rails (~> 5.0) sdoc (~> 0.4.0) sidekiq diff --git a/app/assets/javascripts/components/actions/cards.jsx b/app/assets/javascripts/components/actions/cards.jsx new file mode 100644 index 0000000000..808f1835b0 --- /dev/null +++ b/app/assets/javascripts/components/actions/cards.jsx @@ -0,0 +1,40 @@ +import api from '../api'; + +export const STATUS_CARD_FETCH_REQUEST = 'STATUS_CARD_FETCH_REQUEST'; +export const STATUS_CARD_FETCH_SUCCESS = 'STATUS_CARD_FETCH_SUCCESS'; +export const STATUS_CARD_FETCH_FAIL = 'STATUS_CARD_FETCH_FAIL'; + +export function fetchStatusCard(id) { + return (dispatch, getState) => { + dispatch(fetchStatusCardRequest(id)); + + api(getState).get(`/api/v1/statuses/${id}/card`).then(response => { + dispatch(fetchStatusCardSuccess(id, response.data)); + }).catch(error => { + dispatch(fetchStatusCardFail(id, error)); + }); + }; +}; + +export function fetchStatusCardRequest(id) { + return { + type: STATUS_CARD_FETCH_REQUEST, + id + }; +}; + +export function fetchStatusCardSuccess(id, card) { + return { + type: STATUS_CARD_FETCH_SUCCESS, + id, + card + }; +}; + +export function fetchStatusCardFail(id, error) { + return { + type: STATUS_CARD_FETCH_FAIL, + id, + error + }; +}; diff --git a/app/assets/javascripts/components/actions/statuses.jsx b/app/assets/javascripts/components/actions/statuses.jsx index 21a56381eb..9ac215727a 100644 --- a/app/assets/javascripts/components/actions/statuses.jsx +++ b/app/assets/javascripts/components/actions/statuses.jsx @@ -1,6 +1,7 @@ import api from '../api'; import { deleteFromTimelines } from './timelines'; +import { fetchStatusCard } from './cards'; export const STATUS_FETCH_REQUEST = 'STATUS_FETCH_REQUEST'; export const STATUS_FETCH_SUCCESS = 'STATUS_FETCH_SUCCESS'; @@ -31,6 +32,7 @@ export function fetchStatus(id) { api(getState).get(`/api/v1/statuses/${id}`).then(response => { dispatch(fetchStatusSuccess(response.data, skipLoading)); dispatch(fetchContext(id)); + dispatch(fetchStatusCard(id)); }).catch(error => { dispatch(fetchStatusFail(id, error, skipLoading)); }); diff --git a/app/assets/javascripts/components/features/status/components/card.jsx b/app/assets/javascripts/components/features/status/components/card.jsx new file mode 100644 index 0000000000..7161de3645 --- /dev/null +++ b/app/assets/javascripts/components/features/status/components/card.jsx @@ -0,0 +1,96 @@ +import PureRenderMixin from 'react-addons-pure-render-mixin'; +import ImmutablePropTypes from 'react-immutable-proptypes'; + +const outerStyle = { + display: 'flex', + cursor: 'pointer', + fontSize: '14px', + border: '1px solid #363c4b', + borderRadius: '4px', + color: '#616b86', + marginTop: '14px', + textDecoration: 'none', + overflow: 'hidden' +}; + +const contentStyle = { + flex: '2', + padding: '8px', + paddingLeft: '14px' +}; + +const titleStyle = { + display: 'block', + fontWeight: '500', + marginBottom: '5px', + color: '#d9e1e8' +}; + +const descriptionStyle = { + color: '#d9e1e8' +}; + +const imageOuterStyle = { + flex: '1', + background: '#373b4a' +}; + +const imageStyle = { + display: 'block', + width: '100%', + height: 'auto', + margin: '0', + borderRadius: '4px 0 0 4px' +}; + +const hostStyle = { + display: 'block', + marginTop: '5px', + fontSize: '13px' +}; + +const getHostname = url => { + const parser = document.createElement('a'); + parser.href = url; + return parser.hostname; +}; + +const Card = React.createClass({ + propTypes: { + card: ImmutablePropTypes.map + }, + + mixins: [PureRenderMixin], + + render () { + const { card } = this.props; + + if (card === null) { + return null; + } + + let image = ''; + + if (card.get('image')) { + image = ( +
+ {card.get('title')} +
+ ); + } + + return ( + + {image} + +
+ {card.get('title')} +

{card.get('description')}

+ {getHostname(card.get('url'))} +
+ + ); + } +}); + +export default Card; diff --git a/app/assets/javascripts/components/features/status/components/detailed_status.jsx b/app/assets/javascripts/components/features/status/components/detailed_status.jsx index 14a504c7cd..f2d6ae48a8 100644 --- a/app/assets/javascripts/components/features/status/components/detailed_status.jsx +++ b/app/assets/javascripts/components/features/status/components/detailed_status.jsx @@ -7,6 +7,7 @@ import MediaGallery from '../../../components/media_gallery'; import VideoPlayer from '../../../components/video_player'; import { Link } from 'react-router'; import { FormattedDate, FormattedNumber } from 'react-intl'; +import CardContainer from '../containers/card_container'; const DetailedStatus = React.createClass({ @@ -42,6 +43,8 @@ const DetailedStatus = React.createClass({ } else { media = ; } + } else { + media = ; } if (status.get('application')) { diff --git a/app/assets/javascripts/components/features/status/containers/card_container.jsx b/app/assets/javascripts/components/features/status/containers/card_container.jsx new file mode 100644 index 0000000000..5c8bfeec2a --- /dev/null +++ b/app/assets/javascripts/components/features/status/containers/card_container.jsx @@ -0,0 +1,8 @@ +import { connect } from 'react-redux'; +import Card from '../components/card'; + +const mapStateToProps = (state, { statusId }) => ({ + card: state.getIn(['cards', statusId], null) +}); + +export default connect(mapStateToProps)(Card); diff --git a/app/assets/javascripts/components/reducers/cards.jsx b/app/assets/javascripts/components/reducers/cards.jsx new file mode 100644 index 0000000000..3c9395011e --- /dev/null +++ b/app/assets/javascripts/components/reducers/cards.jsx @@ -0,0 +1,14 @@ +import { STATUS_CARD_FETCH_SUCCESS } from '../actions/cards'; + +import Immutable from 'immutable'; + +const initialState = Immutable.Map(); + +export default function cards(state = initialState, action) { + switch(action.type) { + case STATUS_CARD_FETCH_SUCCESS: + return state.set(action.id, Immutable.fromJS(action.card)); + default: + return state; + } +}; diff --git a/app/assets/javascripts/components/reducers/index.jsx b/app/assets/javascripts/components/reducers/index.jsx index 80c913d2d1..0798116c4b 100644 --- a/app/assets/javascripts/components/reducers/index.jsx +++ b/app/assets/javascripts/components/reducers/index.jsx @@ -13,6 +13,7 @@ import search from './search'; import notifications from './notifications'; import settings from './settings'; import status_lists from './status_lists'; +import cards from './cards'; export default combineReducers({ timelines, @@ -28,5 +29,6 @@ export default combineReducers({ relationships, search, notifications, - settings + settings, + cards }); diff --git a/app/assets/stylesheets/components.scss b/app/assets/stylesheets/components.scss index fedf73b1d5..7e61323abb 100644 --- a/app/assets/stylesheets/components.scss +++ b/app/assets/stylesheets/components.scss @@ -680,3 +680,9 @@ button.active i.fa-retweet { transition-duration: 0.9s; background-position: 0 -209px; } + +.status-card { + &:hover { + background: #363c4b; + } +} diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index c661d81c1f..37ed5e6dd6 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -3,8 +3,8 @@ class Api::V1::StatusesController < ApiController before_action -> { doorkeeper_authorize! :read }, except: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite] before_action -> { doorkeeper_authorize! :write }, only: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite] - before_action :require_user!, except: [:show, :context, :reblogged_by, :favourited_by] - before_action :set_status, only: [:show, :context, :reblogged_by, :favourited_by] + before_action :require_user!, except: [:show, :context, :card, :reblogged_by, :favourited_by] + before_action :set_status, only: [:show, :context, :card, :reblogged_by, :favourited_by] respond_to :json @@ -21,6 +21,10 @@ class Api::V1::StatusesController < ApiController set_counters_maps(statuses) end + def card + @card = PreviewCard.find_by!(status: @status) + end + def reblogged_by results = @status.reblogs.paginate_by_max_id(DEFAULT_ACCOUNTS_LIMIT, params[:max_id], params[:since_id]) accounts = Account.where(id: results.map(&:account_id)).map { |a| [a.id, a] }.to_h diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb new file mode 100644 index 0000000000..e59b05eb87 --- /dev/null +++ b/app/models/preview_card.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class PreviewCard < ApplicationRecord + IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif'].freeze + + belongs_to :status + + has_attached_file :image, styles: { original: '120x120#' }, convert_options: { all: '-quality 80 -strip' } + + validates :url, presence: true + validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES + validates_attachment_size :image, less_than: 1.megabytes + + def save_with_optional_image! + save! + rescue ActiveRecord::RecordInvalid + self.image = nil + save! + end +end diff --git a/app/models/status.rb b/app/models/status.rb index 5710f9ccac..d5f52b55c6 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -23,6 +23,7 @@ class Status < ApplicationRecord has_and_belongs_to_many :tags has_one :notification, as: :activity, dependent: :destroy + has_one :preview_card, dependent: :destroy validates :account, presence: true validates :uri, uniqueness: true, unless: 'local?' diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb new file mode 100644 index 0000000000..2779b79b5e --- /dev/null +++ b/app/services/fetch_link_card_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class FetchLinkCardService < BaseService + def call(status) + # Get first URL + url = URI.extract(status.text).reject { |uri| (uri =~ /\Ahttps?:\/\//).nil? }.first + + return if url.nil? + + response = http_client.get(url) + + return if response.code != 200 + + page = Nokogiri::HTML(response.to_s) + card = PreviewCard.where(status: status).first_or_initialize(status: status, url: url) + + card.title = meta_property(page, 'og:title') || page.at_xpath('//title')&.content + card.description = meta_property(page, 'og:description') || meta_property(page, 'description') + card.image = URI.parse(meta_property(page, 'og:image')) if meta_property(page, 'og:image') + + card.save_with_optional_image! + end + + private + + def http_client + HTTP.timeout(:per_operation, write: 10, connect: 10, read: 10).follow + end + + def meta_property(html, property) + html.at_xpath("//meta[@property=\"#{property}\"]")&.attribute('content')&.value || html.at_xpath("//meta[@name=\"#{property}\"]")&.attribute('content')&.value + end +end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index af31c923ff..8765ef5e35 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -22,6 +22,7 @@ class PostStatusService < BaseService process_mentions_service.call(status) process_hashtags_service.call(status) + LinkCrawlWorker.perform_async(status.id) DistributionWorker.perform_async(status.id) Pubsubhubbub::DistributionWorker.perform_async(status.stream_entry.id) diff --git a/app/views/api/v1/statuses/card.rabl b/app/views/api/v1/statuses/card.rabl new file mode 100644 index 0000000000..8ba8dcbb1e --- /dev/null +++ b/app/views/api/v1/statuses/card.rabl @@ -0,0 +1,5 @@ +object @card + +attributes :url, :title, :description + +node(:image) { |card| card.image? ? full_asset_url(card.image.url(:original)) : nil } diff --git a/app/workers/link_crawl_worker.rb b/app/workers/link_crawl_worker.rb new file mode 100644 index 0000000000..af3394b8b3 --- /dev/null +++ b/app/workers/link_crawl_worker.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class LinkCrawlWorker + include Sidekiq::Worker + + sidekiq_options retry: false + + def perform(status_id) + FetchLinkCardService.new.call(Status.find(status_id)) + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/config/application.rb b/config/application.rb index e97fb165ba..d0b06bf954 100644 --- a/config/application.rb +++ b/config/application.rb @@ -3,6 +3,7 @@ require_relative 'boot' require 'rails/all' require_relative '../app/lib/exceptions' +require_relative '../lib/statsd_monitor' # Require the gems listed in Gemfile, including any gems # you've limited to :test, :development, or :production. @@ -30,7 +31,7 @@ module Mastodon config.active_job.queue_adapter = :sidekiq - config.middleware.insert(0, 'StatsDMonitor') + config.middleware.insert(0, ::StatsDMonitor) config.middleware.insert_before 0, Rack::Cors do allow do diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 8fd1ae72c8..b5e43e705f 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/inflections.rb @@ -12,4 +12,5 @@ ActiveSupport::Inflector.inflections(:en) do |inflect| inflect.acronym 'StatsD' + inflect.acronym 'OEmbed' end diff --git a/config/routes.rb b/config/routes.rb index 42de503f0b..4606c663a1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -86,6 +86,7 @@ Rails.application.routes.draw do resources :statuses, only: [:create, :show, :destroy] do member do get :context + get :card get :reblogged_by get :favourited_by @@ -146,7 +147,7 @@ Rails.application.routes.draw do get '/about', to: 'about#index' get '/about/more', to: 'about#more' get '/terms', to: 'about#terms' - + root 'home#index' match '*unmatched_route', via: :all, to: 'application#raise_not_found' diff --git a/db/migrate/20170119214911_create_preview_cards.rb b/db/migrate/20170119214911_create_preview_cards.rb new file mode 100644 index 0000000000..70ed91bbd5 --- /dev/null +++ b/db/migrate/20170119214911_create_preview_cards.rb @@ -0,0 +1,17 @@ +class CreatePreviewCards < ActiveRecord::Migration[5.0] + def change + create_table :preview_cards do |t| + t.integer :status_id + t.string :url, null: false, default: '' + + # OpenGraph + t.string :title, null: true + t.string :description, null: true + t.attachment :image + + t.timestamps + end + + add_index :preview_cards, :status_id, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 37da0c44eb..abe6f1bfe3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170114203041) do +ActiveRecord::Schema.define(version: 20170119214911) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -157,6 +157,20 @@ ActiveRecord::Schema.define(version: 20170114203041) do t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true, using: :btree end + create_table "preview_cards", force: :cascade do |t| + t.integer "status_id" + t.string "url", default: "", null: false + t.string "title" + t.string "description" + t.string "image_file_name" + t.string "image_content_type" + t.integer "image_file_size" + t.datetime "image_updated_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["status_id"], name: "index_preview_cards_on_status_id", unique: true, using: :btree + end + create_table "pubsubhubbub_subscriptions", force: :cascade do |t| t.string "topic", default: "", null: false t.string "callback", default: "", null: false diff --git a/app/lib/statsd_monitor.rb b/lib/statsd_monitor.rb similarity index 100% rename from app/lib/statsd_monitor.rb rename to lib/statsd_monitor.rb diff --git a/spec/fabricators/preview_card_fabricator.rb b/spec/fabricators/preview_card_fabricator.rb new file mode 100644 index 0000000000..448a94e7ed --- /dev/null +++ b/spec/fabricators/preview_card_fabricator.rb @@ -0,0 +1,5 @@ +Fabricator(:preview_card) do + status_id 1 + url "MyString" + html "MyText" +end diff --git a/spec/models/preview_card_spec.rb b/spec/models/preview_card_spec.rb new file mode 100644 index 0000000000..14ef23923c --- /dev/null +++ b/spec/models/preview_card_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe PreviewCard, type: :model do + +end diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index d40bf0b447..9cb3d41ce9 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -1,5 +1,5 @@ require 'rails_helper' RSpec.describe Subscription, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + end