FIX: Cache missing inline oneboxes (#12953)

FIX: Cache missing inline oneboxes (#12953)

  • FIX: Cache missing inline oneboxes

Some inline oneboxes were not cached when the server did not return an answer for an URL and the queried URL and the absolute URL were different.

For example, if user typed www.example.com, the client asked the server for http://www.example.com and if the server returned an empty response, then the client would keep requesting an inline onebox everytime the composer changed.

In other words, the key used for reading (the absolute URL) and the one used for writing (the URL as typed by the user) were not the same when the server returned an empty response.

  • DEV: Check cache before making request

There is another cache check in PrettyText, but that is not enough if multiple requests are pending. This problem was made obvious in tests, but can happen for users with slow connections.

diff --git a/app/assets/javascripts/discourse/app/lib/load-oneboxes.js b/app/assets/javascripts/discourse/app/lib/load-oneboxes.js
index 29ced7b..0be1065 100644
--- a/app/assets/javascripts/discourse/app/lib/load-oneboxes.js
+++ b/app/assets/javascripts/discourse/app/lib/load-oneboxes.js
@@ -23,19 +23,20 @@ export function loadOneboxes(
   container
     .querySelectorAll(`a.onebox, a.inline-onebox-loading`)
     .forEach((link) => {
-      const text = link.textContent;
-      const isInline = link.getAttribute("class") === "inline-onebox-loading";
-      const m = isInline ? inlineOneboxes : oneboxes;
+      const isInline = link.classList.contains("inline-onebox-loading");
+
+      // maps URLs to their link elements
+      const map = isInline ? inlineOneboxes : oneboxes;
 
       if (loadedOneboxes < maxOneboxes) {
-        if (m[text] === undefined) {
-          m[text] = [];
+        if (map[link.href] === undefined) {
+          map[link.href] = [];
           loadedOneboxes++;
         }
-        m[text].push(link);
+        map[link.href].push(link);
       } else {
-        if (m[text] !== undefined) {
-          m[text].push(link);
+        if (map[link.href] !== undefined) {
+          map[link.href].push(link);
         } else if (isInline) {
           link.classList.remove("inline-onebox-loading");
         }
diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-onebox-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-onebox-test.js
index 04ad220..cc8ea8e 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/composer-onebox-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/composer-onebox-test.js
@@ -41,3 +41,42 @@ This is another test <a href=\"http://www.example.com/has-title.html\" class=\"i
     );
   });
 });
+
+acceptance("Composer - Inline Onebox", function (needs) {
+  needs.user();
+  needs.settings({
+    max_oneboxes_per_post: 2,
+    enable_markdown_linkify: true,
+    markdown_linkify_tlds: "com",
+  });
+
+  let requestsCount;
+
+  needs.pretender((server, helper) => {
+    server.get("/inline-onebox", () => {
+      ++requestsCount;
+      return helper.response({ "inline-oneboxes": [] });
+    });
+  });
+
+  test("Uses cached inline onebox", async function (assert) {
+    requestsCount = 0;
+
+    await visit("/t/internationalization-localization/280");
+    await click("#topic-footer-buttons .btn.create");
+
+    await fillIn(".d-editor-input", `Test www.example.com/page`);
+    assert.equal(requestsCount, 1);
+    assert.equal(
+      queryAll(".d-editor-preview").html().trim(),
+      '<p>Test <a href="http://www.example.com/page" class="inline-onebox-loading">www.example.com/page</a></p>'
+    );
+
+    await fillIn(".d-editor-input", `Test www.example.com/page Test`);
+    assert.equal(requestsCount, 1);
+    assert.equal(
+      queryAll(".d-editor-preview").html().trim(),
+      '<p>Test <a href="http://www.example.com/page">www.example.com/page</a> Test</p>'
+    );
+  });
+});
diff --git a/app/assets/javascripts/pretty-text/addon/inline-oneboxer.js b/app/assets/javascripts/pretty-text/addon/inline-oneboxer.js
index cbed77d..010b6a9 100644
--- a/app/assets/javascripts/pretty-text/addon/inline-oneboxer.js
+++ b/app/assets/javascripts/pretty-text/addon/inline-oneboxer.js
@@ -3,14 +3,20 @@ const _cache = {};
 export function applyInlineOneboxes(inline, ajax, opts) {
   opts = opts || {};
 
-  Object.keys(inline).forEach((url) => {
+  const urls = Object.keys(inline).filter((url) => !_cache[url]);
+
+  urls.forEach((url) => {
     // cache a blank locally, so we never trigger a lookup
     _cache[url] = {};
   });
 
-  return ajax("/inline-onebox", {
+  if (urls.length === 0) {
+    return;
+  }
+
+  ajax("/inline-onebox", {
     data: {
-      urls: Object.keys(inline),
+      urls,
       category_id: opts.categoryId,
       topic_id: opts.topicId,
     },

GitHub sha: ecc3c404

This commit appears in #12953 which was approved by ZogStriP. It was merged by nbianca.