about summary refs log tree commit diff
path: root/app
diff options
context:
space:
mode:
authorEugen Rochko <eugen@zeonfederated.com>2017-09-01 16:20:16 +0200
committerGitHub <noreply@github.com>2017-09-01 16:20:16 +0200
commit7dc5035031a697e7a2726fcd787fc9c294751027 (patch)
treead2e2fe24ba604c07e4c329315efdf1479759cc8 /app
parent2305f7c391325c7abf8746ebb2bb560c13df4437 (diff)
Make PreviewCard records reuseable between statuses (#4642)
* Make PreviewCard records reuseable between statuses

**Warning!** Migration truncates preview_cards tablec

* Allow a wider thumbnail for link preview, display it in horizontal layout (#4648)

* Delete preview cards files before truncating

* Rename old table instead of truncating it

* Add mastodon:maintenance:remove_deprecated_preview_cards

* Ignore deprecated_preview_cards in schema definition

* Fix null behaviour
Diffstat (limited to 'app')
-rw-r--r--app/controllers/api/v1/statuses_controller.rb2
-rw-r--r--app/javascript/mastodon/features/status/components/card.js9
-rw-r--r--app/javascript/styles/components.scss12
-rw-r--r--app/models/media_attachment.rb3
-rw-r--r--app/models/preview_card.rb31
-rw-r--r--app/models/status.rb3
-rw-r--r--app/services/fetch_link_card_service.rb100
7 files changed, 108 insertions, 52 deletions
diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb
index 9c7124d0f..544a4ce21 100644
--- a/app/controllers/api/v1/statuses_controller.rb
+++ b/app/controllers/api/v1/statuses_controller.rb
@@ -29,7 +29,7 @@ class Api::V1::StatusesController < Api::BaseController
   end
 
   def card
-    @card = PreviewCard.find_by(status: @status)
+    @card = @status.preview_cards.first
 
     if @card.nil?
       render_empty
diff --git a/app/javascript/mastodon/features/status/components/card.js b/app/javascript/mastodon/features/status/components/card.js
index bfb40468b..6b13e15cc 100644
--- a/app/javascript/mastodon/features/status/components/card.js
+++ b/app/javascript/mastodon/features/status/components/card.js
@@ -1,6 +1,7 @@
 import React from 'react';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import punycode from 'punycode';
+import classnames from 'classnames';
 
 const IDNA_PREFIX = 'xn--';
 
@@ -32,7 +33,7 @@ export default class Card extends React.PureComponent {
     if (card.get('image')) {
       image = (
         <div className='status-card__image'>
-          <img src={card.get('image')} alt={card.get('title')} className='status-card__image-image' />
+          <img src={card.get('image')} alt={card.get('title')} className='status-card__image-image' width={card.get('width')} height={card.get('height')} />
         </div>
       );
     }
@@ -41,8 +42,12 @@ export default class Card extends React.PureComponent {
       provider = decodeIDNA(getHostname(card.get('url')));
     }
 
+    const className = classnames('status-card', {
+      'horizontal': card.get('width') > card.get('height'),
+    });
+
     return (
-      <a href={card.get('url')} className='status-card' target='_blank' rel='noopener'>
+      <a href={card.get('url')} className={className} target='_blank' rel='noopener'>
         {image}
 
         <div className='status-card__content'>
diff --git a/app/javascript/styles/components.scss b/app/javascript/styles/components.scss
index 8b932e77c..4c913a931 100644
--- a/app/javascript/styles/components.scss
+++ b/app/javascript/styles/components.scss
@@ -2057,6 +2057,18 @@ button.icon-button.active i.fa-retweet {
   background: lighten($ui-base-color, 8%);
 }
 
+.status-card.horizontal {
+  display: block;
+
+  .status-card__image {
+    width: 100%;
+  }
+
+  .status-card__image-image {
+    border-radius: 4px 4px 0 0;
+  }
+}
+
 .status-card__image-image {
   border-radius: 4px 0 0 4px;
   display: block;
diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index 1e8c6d00a..d83ca44f1 100644
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -142,9 +142,11 @@ class MediaAttachment < ApplicationRecord
 
   def populate_meta
     meta = {}
+
     file.queued_for_write.each do |style, file|
       begin
         geo = Paperclip::Geometry.from_file file
+
         meta[style] = {
           width: geo.width.to_i,
           height: geo.height.to_i,
@@ -155,6 +157,7 @@ class MediaAttachment < ApplicationRecord
         meta[style] = {}
       end
     end
+
     meta
   end
 
diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb
index c334c48aa..b7efac354 100644
--- a/app/models/preview_card.rb
+++ b/app/models/preview_card.rb
@@ -4,16 +4,13 @@
 # Table name: preview_cards
 #
 #  id                 :integer          not null, primary key
-#  status_id          :integer
 #  url                :string           default(""), not null
-#  title              :string
-#  description        :string
+#  title              :string           default(""), not null
+#  description        :string           default(""), not null
 #  image_file_name    :string
 #  image_content_type :string
 #  image_file_size    :integer
 #  image_updated_at   :datetime
-#  created_at         :datetime         not null
-#  updated_at         :datetime         not null
 #  type               :integer          default("link"), not null
 #  html               :text             default(""), not null
 #  author_name        :string           default(""), not null
@@ -22,6 +19,8 @@
 #  provider_url       :string           default(""), not null
 #  width              :integer          default(0), not null
 #  height             :integer          default(0), not null
+#  created_at         :datetime         not null
+#  updated_at         :datetime         not null
 #
 
 class PreviewCard < ApplicationRecord
@@ -31,21 +30,37 @@ class PreviewCard < ApplicationRecord
 
   enum type: [:link, :photo, :video, :rich]
 
-  belongs_to :status
+  has_and_belongs_to_many :statuses
 
-  has_attached_file :image, styles: { original: '120x120#' }, convert_options: { all: '-quality 80 -strip' }
+  has_attached_file :image, styles: { original: '280x120>' }, convert_options: { all: '-quality 80 -strip' }
 
   include Attachmentable
   include Remotable
 
-  validates :url, presence: true
+  validates :url, presence: true, uniqueness: true
   validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES
   validates_attachment_size :image, less_than: 1.megabytes
 
+  before_save :extract_dimensions, if: :link?
+
   def save_with_optional_image!
     save!
   rescue ActiveRecord::RecordInvalid
     self.image = nil
     save!
   end
+
+  private
+
+  def extract_dimensions
+    file = image.queued_for_write[:original]
+
+    return if file.nil?
+
+    geo         = Paperclip::Geometry.from_file(file)
+    self.width  = geo.width.to_i
+    self.height = geo.height.to_i
+  rescue Paperclip::Errors::NotIdentifiedByImageMagickError
+    nil
+  end
 end
diff --git a/app/models/status.rb b/app/models/status.rb
index abd902cd7..f44f79aaf 100644
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -47,10 +47,11 @@ class Status < ApplicationRecord
   has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread
   has_many :mentions, dependent: :destroy
   has_many :media_attachments, dependent: :destroy
+
   has_and_belongs_to_many :tags
+  has_and_belongs_to_many :preview_cards
 
   has_one :notification, as: :activity, dependent: :destroy
-  has_one :preview_card, dependent: :destroy
   has_one :stream_entry, as: :activity, inverse_of: :status
 
   validates :uri, uniqueness: true, unless: :local?
diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb
index 20c85e0ea..c38e9e7df 100644
--- a/app/services/fetch_link_card_service.rb
+++ b/app/services/fetch_link_card_service.rb
@@ -4,29 +4,45 @@ class FetchLinkCardService < BaseService
   URL_PATTERN = %r{https?://\S+}
 
   def call(status)
-    # Get first http/https URL that isn't local
-    url = parse_urls(status)
+    @status = status
+    @url    = parse_urls
 
-    return if url.nil?
+    return if @url.nil? || @status.preview_cards.any?
 
-    url  = url.to_s
-    card = PreviewCard.where(status: status).first_or_initialize(status: status, url: url)
-    res  = Request.new(:head, url).perform
+    @url = @url.to_s
 
-    return if res.code != 200 || res.mime_type != 'text/html'
+    RedisLock.acquire(lock_options) do |lock|
+      if lock.acquired?
+        @card = PreviewCard.find_by(url: @url)
+        process_url if @card.nil?
+      end
+    end
 
-    attempt_opengraph(card, url) unless attempt_oembed(card, url)
+    attach_card unless @card.nil?
   rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError
     nil
   end
 
   private
 
-  def parse_urls(status)
-    if status.local?
-      urls = status.text.match(URL_PATTERN).to_a.map { |uri| Addressable::URI.parse(uri).normalize }
+  def process_url
+    @card = PreviewCard.new(url: @url)
+    res   = Request.new(:head, @url).perform
+
+    return if res.code != 200 || res.mime_type != 'text/html'
+
+    attempt_oembed || attempt_opengraph
+  end
+
+  def attach_card
+    @status.preview_cards << @card
+  end
+
+  def parse_urls
+    if @status.local?
+      urls = @status.text.match(URL_PATTERN).to_a.map { |uri| Addressable::URI.parse(uri).normalize }
     else
-      html  = Nokogiri::HTML(status.text)
+      html  = Nokogiri::HTML(@status.text)
       links = html.css('a')
       urls  = links.map { |a| Addressable::URI.parse(a['href']).normalize unless skip_link?(a) }.compact
     end
@@ -44,41 +60,41 @@ class FetchLinkCardService < BaseService
     a['rel']&.include?('tag') || a['class']&.include?('u-url')
   end
 
-  def attempt_oembed(card, url)
-    response = OEmbed::Providers.get(url)
+  def attempt_oembed
+    response = OEmbed::Providers.get(@url)
 
-    card.type          = response.type
-    card.title         = response.respond_to?(:title)         ? response.title         : ''
-    card.author_name   = response.respond_to?(:author_name)   ? response.author_name   : ''
-    card.author_url    = response.respond_to?(:author_url)    ? response.author_url    : ''
-    card.provider_name = response.respond_to?(:provider_name) ? response.provider_name : ''
-    card.provider_url  = response.respond_to?(:provider_url)  ? response.provider_url  : ''
-    card.width         = 0
-    card.height        = 0
+    @card.type          = response.type
+    @card.title         = response.respond_to?(:title)         ? response.title         : ''
+    @card.author_name   = response.respond_to?(:author_name)   ? response.author_name   : ''
+    @card.author_url    = response.respond_to?(:author_url)    ? response.author_url    : ''
+    @card.provider_name = response.respond_to?(:provider_name) ? response.provider_name : ''
+    @card.provider_url  = response.respond_to?(:provider_url)  ? response.provider_url  : ''
+    @card.width         = 0
+    @card.height        = 0
 
-    case card.type
+    case @card.type
     when 'link'
-      card.image = URI.parse(response.thumbnail_url) if response.respond_to?(:thumbnail_url)
+      @card.image = URI.parse(response.thumbnail_url) if response.respond_to?(:thumbnail_url)
     when 'photo'
-      card.url    = response.url
-      card.width  = response.width.presence  || 0
-      card.height = response.height.presence || 0
+      @card.url    = response.url
+      @card.width  = response.width.presence  || 0
+      @card.height = response.height.presence || 0
     when 'video'
-      card.width  = response.width.presence  || 0
-      card.height = response.height.presence || 0
-      card.html   = Formatter.instance.sanitize(response.html, Sanitize::Config::MASTODON_OEMBED)
+      @card.width  = response.width.presence  || 0
+      @card.height = response.height.presence || 0
+      @card.html   = Formatter.instance.sanitize(response.html, Sanitize::Config::MASTODON_OEMBED)
     when 'rich'
       # Most providers rely on <script> tags, which is a no-no
       return false
     end
 
-    card.save_with_optional_image!
+    @card.save_with_optional_image!
   rescue OEmbed::NotFound
     false
   end
 
-  def attempt_opengraph(card, url)
-    response = Request.new(:get, url).perform
+  def attempt_opengraph
+    response = Request.new(:get, @url).perform
 
     return if response.code != 200 || response.mime_type != 'text/html'
 
@@ -88,19 +104,23 @@ class FetchLinkCardService < BaseService
     detector.strip_tags = true
 
     guess = detector.detect(html, response.charset)
-    page = Nokogiri::HTML(html, nil, guess&.fetch(:encoding))
+    page  = Nokogiri::HTML(html, nil, guess&.fetch(:encoding))
 
-    card.type             = :link
-    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_remote_url = meta_property(page, 'og:image') if meta_property(page, 'og:image')
+    @card.type             = :link
+    @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_remote_url = meta_property(page, 'og:image') if meta_property(page, 'og:image')
 
-    return if card.title.blank?
+    return if @card.title.blank?
 
-    card.save_with_optional_image!
+    @card.save_with_optional_image!
   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
+
+  def lock_options
+    { redis: Redis.current, key: "fetch:#{@url}" }
+  end
 end