From f946db4afe5af02539437ed221893cf1362fb6db Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Tue, 30 Jan 2018 08:39:41 +1100
Subject: [PATCH] FIX: inline oneboxer min title length of 2

also: cache mini onebox misses as well to cut down traffic
---
 .../engines/discourse-markdown/onebox.js.es6  |  6 +++---
 .../pretty-text/inline-oneboxer.js.es6        |  6 ++++++
 lib/inline_oneboxer.rb                        |  5 ++++-
 lib/retrieve_title.rb                         |  2 +-
 spec/components/inline_oneboxer_spec.rb       | 19 +++++++++++++++++++
 5 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6
index 2ff5c2a1204..d229837d4cc 100644
--- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6
+++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6
@@ -96,15 +96,15 @@ function applyOnebox(state, silent) {
 
             if (!isTopLevel(href)) {
               let onebox = cachedInlineOnebox(href);
-
               let options = state.md.options.discourse;
+
               if (options.lookupInlineOnebox) {
                 onebox = options.lookupInlineOnebox(href);
               }
 
-              if (onebox) {
+              if (onebox && onebox.title) {
                 text.content = onebox.title;
-              } else if (state.md.options.discourse.previewing) {
+              } else if (state.md.options.discourse.previewing && !onebox) {
                 attrs.push(["class", "inline-onebox-loading"]);
               }
             }
diff --git a/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6 b/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6
index b8be79f60e7..171611506a1 100644
--- a/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6
+++ b/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6
@@ -1,6 +1,12 @@
 let _cache = {};
 
 export function applyInlineOneboxes(inline, ajax) {
+
+  Object.keys(inline).forEach(url => {
+    // cache a blank locally, so we never trigger a lookup
+    _cache[url] = {};
+  });
+
   return ajax("/inline-onebox", {
     data: { urls: Object.keys(inline) },
   }).then(result => {
diff --git a/lib/inline_oneboxer.rb b/lib/inline_oneboxer.rb
index 4c71cba888d..ccc2b547fe9 100644
--- a/lib/inline_oneboxer.rb
+++ b/lib/inline_oneboxer.rb
@@ -2,6 +2,8 @@ require_dependency 'retrieve_title'
 
 class InlineOneboxer
 
+  MIN_TITLE_LENGTH = 2
+
   def initialize(urls, opts = nil)
     @urls = urls
     @opts = opts || {}
@@ -46,6 +48,7 @@ class InlineOneboxer
         uri.hostname.present? &&
         (always_allow || domains.include?(uri.hostname)) &&
         title = RetrieveTitle.crawl(url)
+        title = nil if title && title.length < MIN_TITLE_LENGTH
         return onebox_for(url, title, opts)
       end
     end
@@ -58,7 +61,7 @@ class InlineOneboxer
     def self.onebox_for(url, title, opts)
       onebox = {
         url: url,
-        title: Emoji.gsub_emoji_to_unicode(title)
+        title: title && Emoji.gsub_emoji_to_unicode(title)
       }
       unless opts[:skip_cache]
         Rails.cache.write(cache_key(url), onebox, expires_in: 1.day)
diff --git a/lib/retrieve_title.rb b/lib/retrieve_title.rb
index 26df654e8ef..c8e5f8a7ed9 100644
--- a/lib/retrieve_title.rb
+++ b/lib/retrieve_title.rb
@@ -67,6 +67,6 @@ module RetrieveTitle
         title = extract_title(current)
         throw :done if title || max_size < current.length
       end
-      return title || ""
+      return title
     end
 end
diff --git a/spec/components/inline_oneboxer_spec.rb b/spec/components/inline_oneboxer_spec.rb
index 491f322a0cd..33db691bf08 100644
--- a/spec/components/inline_oneboxer_spec.rb
+++ b/spec/components/inline_oneboxer_spec.rb
@@ -87,6 +87,25 @@ describe InlineOneboxer do
       expect(onebox[:title]).to eq("a blog")
     end
 
+    it "will not return a onebox if it does not meet minimal length" do
+      SiteSetting.enable_inline_onebox_on_all_domains = true
+
+      # Final destination does a HEAD and a GET
+      stub_request(:head, "https://eviltrout.com/some-path").to_return(status: 200)
+
+      stub_request(:get, "https://eviltrout.com/some-path").
+        to_return(status: 200, body: "<html><head><title>a</title></head></html>", headers: {})
+
+      onebox = InlineOneboxer.lookup(
+        "https://eviltrout.com/some-path",
+        skip_cache: true
+      )
+
+      expect(onebox).to be_present
+      expect(onebox[:url]).to eq("https://eviltrout.com/some-path")
+      expect(onebox[:title]).to eq(nil)
+    end
+
     it "will lookup whitelisted domains" do
       SiteSetting.inline_onebox_domains_whitelist = "eviltrout.com"
       RetrieveTitle.stubs(:crawl).returns("Evil Trout's Blog")