From 38827221951c94158417e8932a2025c497788588 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 19 Jul 2017 15:08:54 -0400 Subject: [PATCH] FEATURE: Inline (Mini) Oneboxing see: https://meta.discourse.org/t/mini-inline-onebox-support-rfc/66400?source_topic_id=66066 --- .../components/composer-editor.js.es6 | 18 +++++ app/assets/javascripts/markdown-it-bundle.js | 1 + app/assets/javascripts/pretty-text-bundle.js | 1 + .../discourse-markdown/inline-onebox.js.es6 | 67 +++++++++++++++++++ .../pretty-text/inline-oneboxer.js.es6 | 19 ++++++ .../pretty-text/pretty-text.js.es6 | 8 ++- .../pretty-text/white-lister.js.es6 | 1 + app/controllers/inline_onebox_controller.rb | 10 +++ app/models/topic_link.rb | 7 +- config/routes.rb | 1 + lib/discourse.rb | 17 +++++ lib/inline_oneboxer.rb | 47 +++++++++++++ lib/pretty_text.rb | 1 + lib/pretty_text/helpers.rb | 6 ++ lib/pretty_text/shims.js | 4 ++ spec/components/inline_oneboxer_spec.rb | 54 +++++++++++++++ spec/components/pretty_text_spec.rb | 15 +++++ .../inline_onebox_controller_spec.rb | 37 ++++++++++ 18 files changed, 306 insertions(+), 8 deletions(-) create mode 100644 app/assets/javascripts/pretty-text/engines/discourse-markdown/inline-onebox.js.es6 create mode 100644 app/assets/javascripts/pretty-text/inline-oneboxer.js.es6 create mode 100644 app/controllers/inline_onebox_controller.rb create mode 100644 lib/inline_oneboxer.rb create mode 100644 spec/components/inline_oneboxer_spec.rb create mode 100644 spec/controllers/inline_onebox_controller_spec.rb diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index 3373ccd79d0..b4edf877513 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -5,6 +5,7 @@ import { linkSeenCategoryHashtags, fetchUnseenCategoryHashtags } from 'discourse import { linkSeenTagHashtags, fetchUnseenTagHashtags } from 'discourse/lib/link-tag-hashtag'; import Composer from 'discourse/models/composer'; import { load } from 'pretty-text/oneboxer'; +import { applyInlineOneboxes } from 'pretty-text/inline-oneboxer'; import { ajax } from 'discourse/lib/ajax'; import InputValidation from 'discourse/models/input-validation'; import { findRawTemplate } from 'discourse/lib/raw-templates'; @@ -58,6 +59,8 @@ export default Ember.Component.extend({ @computed markdownOptions() { return { + previewing: true, + lookupAvatarByPostNumber: (postNumber, topicId) => { const topic = this.get('topic'); if (!topic) { return; } @@ -171,6 +174,10 @@ export default Ember.Component.extend({ }); }, + _loadInlineOneboxes(inline) { + applyInlineOneboxes(inline, ajax); + }, + _loadOneboxes($oneboxes) { const post = this.get('composer.post'); let refresh = false; @@ -572,6 +579,17 @@ export default Ember.Component.extend({ Ember.run.debounce(this, this._loadOneboxes, $oneboxes, 450); } + let inline = {}; + $('a.inline-onebox-loading', $preview).each(function(index, link) { + let $link = $(link); + let text = $link.text(); + inline[text] = inline[text] || []; + inline[text].push($link); + }); + if (Object.keys(inline).length > 0) { + Ember.run.debounce(this, this._loadInlineOneboxes, inline, 450); + } + this.trigger('previewRefreshed', $preview); this.sendAction('afterRefresh', $preview); }, diff --git a/app/assets/javascripts/markdown-it-bundle.js b/app/assets/javascripts/markdown-it-bundle.js index 81c05f719fd..4ac2c201c8c 100644 --- a/app/assets/javascripts/markdown-it-bundle.js +++ b/app/assets/javascripts/markdown-it-bundle.js @@ -4,6 +4,7 @@ //= require ./pretty-text/engines/discourse-markdown/quotes //= require ./pretty-text/engines/discourse-markdown/emoji //= require ./pretty-text/engines/discourse-markdown/onebox +//= require ./pretty-text/engines/discourse-markdown/inline-onebox //= require ./pretty-text/engines/discourse-markdown/bbcode-block //= require ./pretty-text/engines/discourse-markdown/bbcode-inline //= require ./pretty-text/engines/discourse-markdown/code diff --git a/app/assets/javascripts/pretty-text-bundle.js b/app/assets/javascripts/pretty-text-bundle.js index 869631eda19..a572344df81 100644 --- a/app/assets/javascripts/pretty-text-bundle.js +++ b/app/assets/javascripts/pretty-text-bundle.js @@ -9,3 +9,4 @@ //= require ./pretty-text/white-lister //= require ./pretty-text/sanitizer //= require ./pretty-text/oneboxer +//= require ./pretty-text/inline-oneboxer diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/inline-onebox.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/inline-onebox.js.es6 new file mode 100644 index 00000000000..acf33e072f3 --- /dev/null +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/inline-onebox.js.es6 @@ -0,0 +1,67 @@ +import { cachedInlineOnebox } from 'pretty-text/inline-oneboxer'; + +function applyInlineOnebox(state, silent) { + if (silent || !state.tokens) { + return; + } + + for (let i=1; i children.length-3) { + continue; + } + + let text = children[j+1]; + let close = children[j+2]; + + // check attrs only include a href + let attrs = child.attrs; + if (!attrs || attrs.length !== 1 || attrs[0][0] !== "href") { + continue; + } + + let href = attrs[0][1]; + if (!/^http|^\/\//i.test(href)) { + continue; + } + + // we already know text matches cause it is an auto link + if (!close || close.type !== "link_close") { + continue; + } + + // link must be the same as the href + if (!text || text.content !== href) { + continue; + } + + // check for href + let onebox = cachedInlineOnebox(href); + + let options = state.md.options.discourse; + if (options.lookupInlineOnebox) { + onebox = options.lookupInlineOnebox(href); + } + + if (onebox) { + text.content = onebox.title; + } else if (state.md.options.discourse.previewing) { + attrs.push(["class", "inline-onebox-loading"]); + } + } + } + } + } +} + +export function setup(helper) { + helper.registerPlugin(md => { + md.core.ruler.after('linkify', 'inline-onebox', applyInlineOnebox); + }); +} diff --git a/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6 b/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6 new file mode 100644 index 00000000000..b8be79f60e7 --- /dev/null +++ b/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6 @@ -0,0 +1,19 @@ +let _cache = {}; + +export function applyInlineOneboxes(inline, ajax) { + return ajax("/inline-onebox", { + data: { urls: Object.keys(inline) }, + }).then(result => { + result['inline-oneboxes'].forEach(onebox => { + _cache[onebox.url] = onebox; + let links = inline[onebox.url] || []; + links.forEach(link => { + link.text(onebox.title); + }); + }); + }); +}; + +export function cachedInlineOnebox(url) { + return _cache[url]; +} diff --git a/app/assets/javascripts/pretty-text/pretty-text.js.es6 b/app/assets/javascripts/pretty-text/pretty-text.js.es6 index f341e7c4c7c..3859fde27e8 100644 --- a/app/assets/javascripts/pretty-text/pretty-text.js.es6 +++ b/app/assets/javascripts/pretty-text/pretty-text.js.es6 @@ -19,7 +19,9 @@ export function buildOptions(state) { getCurrentUser, currentUser, lookupAvatarByPostNumber, - emojiUnicodeReplacer + emojiUnicodeReplacer, + lookupInlineOnebox, + previewing } = state; let features = { @@ -52,8 +54,10 @@ export function buildOptions(state) { lookupAvatarByPostNumber, mentionLookup: state.mentionLookup, emojiUnicodeReplacer, + lookupInlineOnebox, allowedHrefSchemes: siteSettings.allowed_href_schemes ? siteSettings.allowed_href_schemes.split('|') : null, - markdownIt: true + markdownIt: true, + previewing }; // note, this will mutate options due to the way the API is designed diff --git a/app/assets/javascripts/pretty-text/white-lister.js.es6 b/app/assets/javascripts/pretty-text/white-lister.js.es6 index 04578037f8c..30f8f23c966 100644 --- a/app/assets/javascripts/pretty-text/white-lister.js.es6 +++ b/app/assets/javascripts/pretty-text/white-lister.js.es6 @@ -112,6 +112,7 @@ const DEFAULT_LIST = [ 'a.mention', 'a.mention-group', 'a.onebox', + 'a.inline-onebox-loading', 'a[data-bbcode]', 'a[name]', 'a[rel=nofollow]', diff --git a/app/controllers/inline_onebox_controller.rb b/app/controllers/inline_onebox_controller.rb new file mode 100644 index 00000000000..67afcf01b24 --- /dev/null +++ b/app/controllers/inline_onebox_controller.rb @@ -0,0 +1,10 @@ +require_dependency 'inline_oneboxer' + +class InlineOneboxController < ApplicationController + before_filter :ensure_logged_in + + def show + oneboxes = InlineOneboxer.new(params[:urls]).process + render json: { "inline-oneboxes" => oneboxes } + end +end diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 0669f786765..37d7b656dde 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -123,17 +123,12 @@ SQL internal = false topic_id = nil post_number = nil - parsed_path = parsed.path || "" if Discourse.store.has_been_uploaded?(url) internal = Discourse.store.internal? - elsif (parsed.host == Discourse.current_hostname && parsed_path.start_with?(Discourse.base_uri)) || !parsed.host + elsif route = Discourse.route_for(parsed) internal = true - parsed_path.slice!(Discourse.base_uri) - - route = Rails.application.routes.recognize_path(parsed_path) - # We aren't interested in tracking internal links to users next if route[:controller] == 'users' diff --git a/config/routes.rb b/config/routes.rb index b3efb5f486f..037a1d3b0eb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -664,6 +664,7 @@ Discourse::Application.routes.draw do end get "onebox" => "onebox#show" + get "inline-onebox" => "inline_onebox#show" get "exception" => "list#latest" diff --git a/lib/discourse.rb b/lib/discourse.rb index 086ed28b72b..7a305879b50 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -214,6 +214,23 @@ module Discourse base_url_no_prefix + base_uri end + def self.route_for(uri) + + uri = URI(uri) rescue nil unless (uri.is_a?(URI)) + return unless uri + + path = uri.path || "" + if (uri.host == Discourse.current_hostname && + path.start_with?(Discourse.base_uri)) || + !uri.host + + path.slice!(Discourse.base_uri) + return Rails.application.routes.recognize_path(path) + end + + nil + end + READONLY_MODE_KEY_TTL ||= 60 READONLY_MODE_KEY ||= 'readonly_mode'.freeze PG_READONLY_MODE_KEY ||= 'readonly_mode:postgres'.freeze diff --git a/lib/inline_oneboxer.rb b/lib/inline_oneboxer.rb new file mode 100644 index 00000000000..7b726d07613 --- /dev/null +++ b/lib/inline_oneboxer.rb @@ -0,0 +1,47 @@ +class InlineOneboxer + + def initialize(urls) + @urls = urls + end + + def process + @urls.map {|url| InlineOneboxer.lookup(url) }.compact + end + + def self.clear_cache! + end + + def self.cache_lookup(url) + Rails.cache.read(cache_key(url)) + end + + def self.lookup(url) + cached = cache_lookup(url) + return cached if cached.present? + + if route = Discourse.route_for(url) + if route[:controller] == "topics" && + route[:action] == "show" && + + topic = Topic.where(id: route[:topic_id].to_i).first + + # Only public topics + if Guardian.new.can_see?(topic) + onebox = { url: url, title: topic.title } + Rails.cache.write(cache_key(url), onebox, expires_in: 1.day) + return onebox + end + end + end + + nil + end + + private + + def self.cache_key(url) + "inline_onebox:#{url}" + end + +end + diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 4c0d9550792..50131970efa 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -164,6 +164,7 @@ module PrettyText __optInput.mentionLookup = __mentionLookup; __optInput.customEmoji = #{custom_emoji.to_json}; __optInput.emojiUnicodeReplacer = __emojiUnicodeReplacer; + __optInput.lookupInlineOnebox = __lookupInlineOnebox; JS if opts[:topicId] diff --git a/lib/pretty_text/helpers.rb b/lib/pretty_text/helpers.rb index abcb5b6665c..c9859e04ad7 100644 --- a/lib/pretty_text/helpers.rb +++ b/lib/pretty_text/helpers.rb @@ -1,3 +1,5 @@ +require_dependency 'inline_oneboxer' + module PrettyText module Helpers extend self @@ -43,6 +45,10 @@ module PrettyText end end + def lookup_inline_onebox(url) + InlineOneboxer.lookup(url) + end + def get_topic_info(topic_id) return unless topic_id.is_a?(Integer) # TODO this only handles public topics, secured one do not get this diff --git a/lib/pretty_text/shims.js b/lib/pretty_text/shims.js index 93a1ef592fd..dc0ede53de7 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -49,6 +49,10 @@ function __getURL(url) { return url; } +function __lookupInlineOnebox(url) { + return __helpers.lookup_inline_onebox(url); +} + function __getTopicInfo(i) { return __helpers.get_topic_info(i); } diff --git a/spec/components/inline_oneboxer_spec.rb b/spec/components/inline_oneboxer_spec.rb new file mode 100644 index 00000000000..817c5020cdd --- /dev/null +++ b/spec/components/inline_oneboxer_spec.rb @@ -0,0 +1,54 @@ +require 'rails_helper' +require_dependency 'inline_oneboxer' + +describe InlineOneboxer do + + before do + InlineOneboxer.clear_cache! + end + + it "should return nothing with empty input" do + expect(InlineOneboxer.new([]).process).to be_blank + end + + it "can onebox a topic" do + topic = Fabricate(:topic) + results = InlineOneboxer.new([topic.url]).process + expect(results).to be_present + expect(results[0][:url]).to eq(topic.url) + expect(results[0][:title]).to eq(topic.title) + end + + it "doesn't onebox private messages" do + topic = Fabricate(:private_message_topic) + results = InlineOneboxer.new([topic.url]).process + expect(results).to be_blank + end + + context "caching" do + it "puts an entry in the cache" do + topic = Fabricate(:topic) + expect(InlineOneboxer.cache_lookup(topic.url)).to be_blank + + result = InlineOneboxer.lookup(topic.url) + expect(result).to be_present + + cached = InlineOneboxer.cache_lookup(topic.url) + expect(cached).to be_present + expect(cached[:url]).to eq(topic.url) + expect(cached[:title]).to eq(topic.title) + end + end + + context ".lookup" do + it "can lookup one link at a time" do + topic = Fabricate(:topic) + onebox = InlineOneboxer.lookup(topic.url) + expect(onebox).to be_present + expect(onebox[:url]).to eq(topic.url) + expect(onebox[:title]).to eq(topic.title) + end + end + +end + diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index fcb85b78b8b..2ff341b69a9 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -951,4 +951,19 @@ HTML expect(cooked).to eq(html.strip) end end + + describe "inline onebox" do + it "includes the topic title" do + topic = Fabricate(:topic) + + raw = "Hello #{topic.url}" + + cooked = <<~HTML +

Hello #{topic.title}

+ HTML + + expect(PrettyText.cook(raw)).to eq(cooked.strip) + end + end + end diff --git a/spec/controllers/inline_onebox_controller_spec.rb b/spec/controllers/inline_onebox_controller_spec.rb new file mode 100644 index 00000000000..3170c3e3e72 --- /dev/null +++ b/spec/controllers/inline_onebox_controller_spec.rb @@ -0,0 +1,37 @@ +require 'rails_helper' + +describe InlineOneboxController do + + it "requires the user to be logged in" do + expect { xhr :get, :show, urls: [] }.to raise_error(Discourse::NotLoggedIn) + end + + context "logged in" do + let!(:user) { log_in(:user) } + + it "returns empty JSON for empty input" do + xhr :get, :show, urls: [] + expect(response).to be_success + json = JSON.parse(response.body) + expect(json['inline-oneboxes']).to eq([]) + end + + context "topic link" do + let(:topic) { Fabricate(:topic) } + + it "returns information for a valid link" do + xhr :get, :show, urls: [ topic.url ] + expect(response).to be_success + json = JSON.parse(response.body) + onebox = json['inline-oneboxes'][0] + + expect(onebox).to be_present + expect(onebox['url']).to eq(topic.url) + expect(onebox['title']).to eq(topic.title) + end + end + + end + +end +