Preview arbitrary urls within a document (#5598)

app/components/HoverPreview/Components.tsx

@@ -21,13 +21,13 @@ const StyledText = styled(Text)`
 `;
 
 export const Preview = styled(Link)`
-  cursor: var(--pointer);
+  cursor: ${(props: any) =>
+    props.as === "div" ? "default" : "var(--pointer)"};
   border-radius: 4px;
   box-shadow: 0 30px 90px -20px rgba(0, 0, 0, 0.3),
     0 0 1px 1px rgba(0, 0, 0, 0.05);
   overflow: hidden;
   position: absolute;
-  ${(props) => (!props.to ? "pointer-events: none;" : "")}
 `;
 
 export const Title = styled.h2`

app/components/HoverPreview/HoverPreviewLink.tsx

@@ -17,7 +17,7 @@ type Props = {
   url: string;
   /** Title for the preview card */
   title: string;
-  /** Url for thumbnail served by the link provider*/
+  /** Url for thumbnail served by the link provider */
   thumbnailUrl: string;
   /** Some description about the link provider */
   description: string;
@@ -25,7 +25,7 @@ type Props = {
 
 function HoverPreviewLink({ url, thumbnailUrl, title, description }: Props) {
   return (
-    <Preview to={{ pathname: url }} target="_blank" rel="noopener noreferrer">
+    <Preview as="a" href={url} target="_blank" rel="noopener noreferrer">
       <Flex column>
         {thumbnailUrl ? <Thumbnail src={thumbnailUrl} alt={""} /> : null}
         <Card>

app/components/HoverPreview/HoverPreviewMention.tsx

@@ -17,7 +17,7 @@ type Props = {
 
 function HoverPreviewMention({ url, title, info, color }: Props) {
   return (
-    <Preview to="">
+    <Preview as="div">
       <Card fadeOut={false}>
         <CardContent>
           <Flex gap={12}>

plugins/iframely/server/iframely.ts

@@ -1,30 +1,35 @@
 import fetch from "fetch-with-proxy";
 import env from "@server/env";
 import { InternalError } from "@server/errors";
+import Logger from "@server/logging/Logger";
 import Redis from "@server/redis";
 
 class Iframely {
   private static apiUrl = `${env.IFRAMELY_URL}/api`;
   private static apiKey = env.IFRAMELY_API_KEY;
   private static cacheKeyPrefix = "unfurl";
+  private static defaultCacheExpiry = 86400;
 
   private static cacheKey(url: string) {
     return `${this.cacheKeyPrefix}-${url}`;
   }
 
-  private static async cache(url: string) {
+  private static async cache(url: string, response: any) {
-    const data = await this.fetch(url);
+    // do not cache error responses
-
+    if (response.error) {
-    if (!data.error) {
+      return;
+    }
+    try {
       await Redis.defaultClient.set(
         this.cacheKey(url),
-        JSON.stringify(data),
+        JSON.stringify(response),
         "EX",
-        data.cache_age
+        response.cache_age || this.defaultCacheExpiry
       );
+    } catch (err) {
+      // just log it, can skip caching and directly return response
+      Logger.error("Could not cache Iframely response", err);
     }
-
-    return data;
   }
 
   private static async fetch(url: string, type = "oembed") {
@@ -37,16 +42,33 @@ class Iframely {
   }
 
   private static async cached(url: string) {
-    const val = await Redis.defaultClient.get(this.cacheKey(url));
+    try {
-    if (val) {
+      const val = await Redis.defaultClient.get(this.cacheKey(url));
-      return JSON.parse(val);
+      if (val) {
+        return JSON.parse(val);
+      }
+    } catch (err) {
+      // just log it, response can still be obtained using the fetch call
+      Logger.error("Could not fetch cached Iframely response", err);
     }
   }
 
+  /**
+   * Fetches the preview data for the given url
+   * using Iframely oEmbed API
+   *
+   * @param url
+   * @returns Preview data for the url
+   */
   public static async get(url: string) {
     try {
       const cached = await this.cached(url);
-      return cached ? cached : this.cache(url);
+      if (cached) {
+        return cached;
+      }
+      const res = await this.fetch(url);
+      await this.cache(url, res);
+      return res;
     } catch (err) {
       throw InternalError(err);
     }

server/env.ts

@@ -617,7 +617,7 @@ export class Environment {
    */
   @IsOptional()
   @CannotUseWithout("IFRAMELY_URL")
-  public IFRAMELY_API_KEY = process.env.IFRAMELY_API_KEY ?? "";
+  public IFRAMELY_API_KEY = this.toOptionalString(process.env.IFRAMELY_API_KEY);
 
   /**
    * The product name

server/routes/api/urls/urls.test.ts

@@ -1,7 +1,7 @@
 import { User } from "@server/models";
 import { buildDocument, buildUser } from "@server/test/factories";
 import { getTestServer } from "@server/test/support";
-import { Iframely } from "@server/utils/unfurl";
+import resolvers from "@server/utils/unfurl";
 
 jest.mock("@server/utils/unfurl", () => ({
   Iframely: {
@@ -146,7 +146,7 @@ describe("#urls.unfurl", () => {
   });
 
   it("should succeed with status 200 ok for a valid external url", async () => {
-    (Iframely.unfurl as jest.Mock).mockResolvedValue(
+    (resolvers.Iframely.unfurl as jest.Mock).mockResolvedValue(
       Promise.resolve({
         url: "https://www.flickr.com",
         type: "rich",
@@ -167,7 +167,9 @@ describe("#urls.unfurl", () => {
 
     const body = await res.json();
 
-    expect(Iframely.unfurl).toHaveBeenCalledWith("https://www.flickr.com");
+    expect(resolvers.Iframely.unfurl).toHaveBeenCalledWith(
+      "https://www.flickr.com"
+    );
     expect(res.status).toEqual(200);
     expect(body.url).toEqual("https://www.flickr.com");
     expect(body.type).toEqual("rich");
@@ -181,7 +183,7 @@ describe("#urls.unfurl", () => {
   });
 
   it("should succeed with status 204 no content for a non-existing external url", async () => {
-    (Iframely.unfurl as jest.Mock).mockResolvedValue(
+    (resolvers.Iframely.unfurl as jest.Mock).mockResolvedValue(
       Promise.resolve({
         status: 404,
         error:
@@ -196,7 +198,9 @@ describe("#urls.unfurl", () => {
       },
     });
 
-    expect(Iframely.unfurl).toHaveBeenCalledWith("https://random.url");
+    expect(resolvers.Iframely.unfurl).toHaveBeenCalledWith(
+      "https://random.url"
+    );
     expect(res.status).toEqual(204);
   });
 });

server/routes/api/urls/urls.ts

@@ -11,7 +11,7 @@ import { presentDocument, presentMention } from "@server/presenters/unfurls";
 import presentUnfurl from "@server/presenters/unfurls/unfurl";
 import { APIContext } from "@server/types";
 import { RateLimiterStrategy } from "@server/utils/RateLimiter";
-import { Iframely } from "@server/utils/unfurl";
+import resolvers from "@server/utils/unfurl";
 import * as T from "./schema";
 
 const router = new Router();
@@ -62,12 +62,14 @@ router.post(
       return;
     }
 
-    const data = await Iframely.unfurl(url);
+    if (resolvers.Iframely) {
-    if (data.error) {
+      const data = await resolvers.Iframely.unfurl(url);
-      ctx.response.status = 204;
+      return data.error
-      return;
+        ? (ctx.response.status = 204)
+        : (ctx.body = presentUnfurl(data));
     }
-    ctx.body = presentUnfurl(data);
+
+    return (ctx.response.status = 204);
   }
 );
 

server/utils/unfurl.ts

@@ -1,3 +1,4 @@
+import { existsSync } from "fs";
 import path from "path";
 import glob from "glob";
 import { startCase } from "lodash";
@@ -5,20 +6,36 @@ import env from "@server/env";
 import Logger from "@server/logging/Logger";
 import { UnfurlResolver } from "@server/types";
 
-const resolvers: Record<string, UnfurlResolver> = {};
 const rootDir = env.ENVIRONMENT === "test" ? "" : "build";
 
-glob
+const hasResolver = (plugin: string) => {
-  .sync(path.join(rootDir, "plugins/*/server/unfurl.js"))
+  // eslint-disable-next-line @typescript-eslint/no-var-requires
-  .forEach((filePath: string) => {
+  const config = require(path.join(process.cwd(), plugin, "plugin.json"));
+
+  return (
+    existsSync(resolverPath(plugin)) &&
+    (config.requiredEnvVars ?? []).every((name: string) => !!env[name])
+  );
+};
+
+const resolverPath = (plugin: string) =>
+  path.join(plugin, "server", "unfurl.js");
+
+const plugins = glob.sync(path.join(rootDir, "plugins/*"));
+const resolvers: Record<string, UnfurlResolver> = plugins
+  .filter(hasResolver)
+  .map(resolverPath)
+  .reduce((resolvers, resolverPath) => {
     // eslint-disable-next-line @typescript-eslint/no-var-requires
     const resolver: UnfurlResolver = require(path.join(
       process.cwd(),
-      filePath
+      resolverPath
     ));
-    const name = startCase(filePath.split("/")[2]);
+    const name = startCase(resolverPath.split("/")[2]);
     resolvers[name] = resolver;
-    Logger.debug("utils", `Registered unfurl resolver ${filePath}`);
+    Logger.debug("utils", `Registered unfurl resolver ${resolverPath}`);
-  });
 
-export const Iframely = resolvers["Iframely"];
+    return resolvers;
+  }, {});
+
+export default resolvers;