url, internalLinks: Pipe realm as URL to isUrlOnRealm, `getPathsFro…

…mUrl`.

As in the previous and next commits, stringify the realm exactly
where we need it as a string.

This slightly expands the area of our codebase in which it's easy to
see that the realm is well-formed data.

src/message/messagesActions.js

@@ -44,11 +44,11 @@ export const messageLinkPress = (href: string) => async (
   const auth = getAuth(state);
   const usersById = getUsersById(state);
   const streamsById = getStreamsById(state);
-  const narrow = getNarrowFromLink(href, auth.realm.toString(), usersById, streamsById);
+  const narrow = getNarrowFromLink(href, auth.realm, usersById, streamsById);
   if (narrow) {
-    const anchor = getMessageIdFromLink(href, auth.realm.toString());
+    const anchor = getMessageIdFromLink(href, auth.realm);
     dispatch(doNarrow(narrow, anchor));
-  } else if (!isUrlOnRealm(href, auth.realm.toString())) {
+  } else if (!isUrlOnRealm(href, auth.realm)) {
     openLink(href);
   } else {
     const url =

src/utils/__tests__/internalLinks-test.js

@@ -15,148 +15,95 @@ import {
 } from '../internalLinks';
 import * as eg from '../../__tests__/lib/exampleData';
 
+const realm = new URL('https://example.com');
+
 describe('isInternalLink', () => {
   test('when link is external, return false', () => {
-    expect(isInternalLink('https://example.com', 'https://another.com')).toBe(false);
+    expect(isInternalLink('https://example.com', new URL('https://another.com'))).toBe(false);
   });
 
   test('when link is internal, but not in app, return false', () => {
-    expect(isInternalLink('https://example.com/user_uploads', 'https://example.com')).toBe(false);
+    expect(isInternalLink('https://example.com/user_uploads', realm)).toBe(false);
   });
 
   test('when link is internal and in app, return true', () => {
-    expect(isInternalLink('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
-      true,
-    );
+    expect(isInternalLink('https://example.com/#narrow/stream/jest', realm)).toBe(true);
   });
 
   test('when link is relative and in app, return true', () => {
-    expect(isInternalLink('#narrow/stream/jest/topic/topic1', 'https://example.com')).toBe(true);
-    expect(isInternalLink('/#narrow/stream/jest', 'https://example.com')).toBe(true);
+    expect(isInternalLink('#narrow/stream/jest/topic/topic1', realm)).toBe(true);
+    expect(isInternalLink('/#narrow/stream/jest', realm)).toBe(true);
   });
 
   test('links including IDs are also recognized', () => {
-    expect(isInternalLink('#narrow/stream/123-jest/topic/topic1', 'https://example.com')).toBe(
-      true,
-    );
-    expect(isInternalLink('/#narrow/stream/123-jest', 'https://example.com')).toBe(true);
-    expect(isInternalLink('/#narrow/pm-with/123-mark', 'https://example.com')).toBe(true);
+    expect(isInternalLink('#narrow/stream/123-jest/topic/topic1', realm)).toBe(true);
+    expect(isInternalLink('/#narrow/stream/123-jest', realm)).toBe(true);
+    expect(isInternalLink('/#narrow/pm-with/123-mark', realm)).toBe(true);
   });
 });
 
 describe('isMessageLink', () => {
   test('only in-app link containing "near/<message-id>" is a message link', () => {
-    expect(isMessageLink('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
-      false,
-    );
-    expect(isMessageLink('https://example.com/#narrow/#near/1', 'https://example.com')).toBe(true);
+    expect(isMessageLink('https://example.com/#narrow/stream/jest', realm)).toBe(false);
+    expect(isMessageLink('https://example.com/#narrow/#near/1', realm)).toBe(true);
   });
 });
 
 describe('getLinkType', () => {
   test('links to a different domain are of "external" type', () => {
-    expect(getLinkType('https://google.com/some-path', 'https://example.com')).toBe('external');
+    expect(getLinkType('https://google.com/some-path', realm)).toBe('external');
   });
 
   test('only in-app link containing "stream" is a stream link', () => {
-    expect(
-      getLinkType('https://example.com/#narrow/pm-with/1,2-group', 'https://example.com'),
-    ).toBe('pm');
-    expect(getLinkType('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
-      'stream',
-    );
-    expect(getLinkType('https://example.com/#narrow/stream/stream/', 'https://example.com')).toBe(
-      'stream',
-    );
+    expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group', realm)).toBe('pm');
+    expect(getLinkType('https://example.com/#narrow/stream/jest', realm)).toBe('stream');
+    expect(getLinkType('https://example.com/#narrow/stream/stream/', realm)).toBe('stream');
   });
 
   test('when a url is not a topic narrow return false', () => {
-    expect(
-      getLinkType('https://example.com/#narrow/pm-with/1,2-group', 'https://example.com'),
-    ).toBe('pm');
-    expect(getLinkType('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
-      'stream',
-    );
-    expect(
-      getLinkType(
-        'https://example.com/#narrow/stream/stream/topic/topic/near/',
-        'https://example.com',
-      ),
-    ).toBe('home');
-    expect(getLinkType('https://example.com/#narrow/stream/topic/', 'https://example.com')).toBe(
-      'stream',
+    expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group', realm)).toBe('pm');
+    expect(getLinkType('https://example.com/#narrow/stream/jest', realm)).toBe('stream');
+    expect(getLinkType('https://example.com/#narrow/stream/stream/topic/topic/near/', realm)).toBe(
+      'home',
     );
+    expect(getLinkType('https://example.com/#narrow/stream/topic/', realm)).toBe('stream');
   });
 
   test('when a url is a topic narrow return true', () => {
+    expect(getLinkType('https://example.com/#narrow/stream/jest/topic/test', realm)).toBe('topic');
     expect(
-      getLinkType('https://example.com/#narrow/stream/jest/topic/test', 'https://example.com'),
-    ).toBe('topic');
-    expect(
-      getLinkType(
-        'https://example.com/#narrow/stream/mobile/subject/topic/near/378333',
-        'https://example.com',
-      ),
-    ).toBe('topic');
-    expect(
-      getLinkType('https://example.com/#narrow/stream/mobile/topic/topic/', 'https://example.com'),
-    ).toBe('topic');
-    expect(
-      getLinkType(
-        'https://example.com/#narrow/stream/stream/topic/topic/near/1',
-        'https://example.com',
-      ),
+      getLinkType('https://example.com/#narrow/stream/mobile/subject/topic/near/378333', realm),
     ).toBe('topic');
+    expect(getLinkType('https://example.com/#narrow/stream/mobile/topic/topic/', realm)).toBe(
+      'topic',
+    );
+    expect(getLinkType('https://example.com/#narrow/stream/stream/topic/topic/near/1', realm)).toBe(
+      'topic',
+    );
     expect(
-      getLinkType(
-        'https://example.com/#narrow/stream/stream/subject/topic/near/1',
-        'https://example.com',
-      ),
+      getLinkType('https://example.com/#narrow/stream/stream/subject/topic/near/1', realm),
     ).toBe('topic');
 
-    expect(getLinkType('/#narrow/stream/stream/subject/topic', 'https://example.com')).toBe(
-      'topic',
-    );
+    expect(getLinkType('/#narrow/stream/stream/subject/topic', realm)).toBe('topic');
   });
 
   test('only in-app link containing "pm-with" is a group link', () => {
+    expect(getLinkType('https://example.com/#narrow/stream/jest/topic/test', realm)).toBe('topic');
+    expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group', realm)).toBe('pm');
+    expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group/near/1', realm)).toBe('pm');
     expect(
-      getLinkType('https://example.com/#narrow/stream/jest/topic/test', 'https://example.com'),
-    ).toBe('topic');
-    expect(
-      getLinkType('https://example.com/#narrow/pm-with/1,2-group', 'https://example.com'),
-    ).toBe('pm');
-    expect(
-      getLinkType('https://example.com/#narrow/pm-with/1,2-group/near/1', 'https://example.com'),
-    ).toBe('pm');
-    expect(
-      getLinkType(
-        'https://example.com/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3',
-        'https://example.com',
-      ),
+      getLinkType('https://example.com/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', realm),
     ).toBe('pm');
   });
 
   test('only in-app link containing "is" is a special link', () => {
-    expect(
-      getLinkType('https://example.com/#narrow/stream/jest/topic/test', 'https://example.com'),
-    ).toBe('topic');
-    expect(getLinkType('https://example.com/#narrow/is/private', 'https://example.com')).toBe(
-      'special',
-    );
-    expect(getLinkType('https://example.com/#narrow/is/starred', 'https://example.com')).toBe(
-      'special',
-    );
-    expect(getLinkType('https://example.com/#narrow/is/mentioned', 'https://example.com')).toBe(
-      'special',
-    );
-    expect(getLinkType('https://example.com/#narrow/is/men', 'https://example.com')).toBe('home');
-    expect(getLinkType('https://example.com/#narrow/is/men/stream', 'https://example.com')).toBe(
-      'home',
-    );
-    expect(getLinkType('https://example.com/#narrow/are/men/stream', 'https://example.com')).toBe(
-      'home',
-    );
+    expect(getLinkType('https://example.com/#narrow/stream/jest/topic/test', realm)).toBe('topic');
+    expect(getLinkType('https://example.com/#narrow/is/private', realm)).toBe('special');
+    expect(getLinkType('https://example.com/#narrow/is/starred', realm)).toBe('special');
+    expect(getLinkType('https://example.com/#narrow/is/mentioned', realm)).toBe('special');
+    expect(getLinkType('https://example.com/#narrow/is/men', realm)).toBe('home');
+    expect(getLinkType('https://example.com/#narrow/is/men/stream', realm)).toBe('home');
+    expect(getLinkType('https://example.com/#narrow/are/men/stream', realm)).toBe('home');
   });
 });
 
@@ -192,7 +139,7 @@ describe('getNarrowFromLink', () => {
   const get = (url, streams = []) =>
     getNarrowFromLink(
       url,
-      'https://example.com',
+      new URL('https://example.com'),
       usersById,
       new Map(streams.map(s => [s.stream_id, s])),
     );
@@ -343,26 +290,18 @@ describe('getNarrowFromLink', () => {
 
 describe('getMessageIdFromLink', () => {
   test('not message link', () => {
-    expect(
-      getMessageIdFromLink('https://example.com/#narrow/is/private', 'https://example.com'),
-    ).toBe(0);
+    expect(getMessageIdFromLink('https://example.com/#narrow/is/private', realm)).toBe(0);
   });
 
   test('when link is a group link, return anchor message id', () => {
     expect(
-      getMessageIdFromLink(
-        'https://example.com/#narrow/pm-with/1,3-group/near/1/',
-        'https://example.com',
-      ),
+      getMessageIdFromLink('https://example.com/#narrow/pm-with/1,3-group/near/1/', realm),
     ).toBe(1);
   });
 
   test('when link is a topic link, return anchor message id', () => {
     expect(
-      getMessageIdFromLink(
-        'https://example.com/#narrow/stream/jest/topic/test/near/1',
-        'https://example.com',
-      ),
+      getMessageIdFromLink('https://example.com/#narrow/stream/jest/topic/test/near/1', realm),
     ).toBe(1);
   });
 });

src/utils/__tests__/url-test.js

@@ -59,20 +59,20 @@ describe('getResource', () => {
 });
 
 describe('isUrlOnRealm', () => {
+  const realm = new URL('https://example.com');
+
   test('when link is on realm, return true', () => {
-    expect(isUrlOnRealm('/#narrow/stream/jest', 'https://example.com')).toBe(true);
+    expect(isUrlOnRealm('/#narrow/stream/jest', realm)).toBe(true);
 
-    expect(isUrlOnRealm('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
-      true,
-    );
+    expect(isUrlOnRealm('https://example.com/#narrow/stream/jest', realm)).toBe(true);
 
-    expect(isUrlOnRealm('#narrow/#near/1', 'https://example.com')).toBe(true);
+    expect(isUrlOnRealm('#narrow/#near/1', realm)).toBe(true);
   });
 
   test('when link is on not realm, return false', () => {
-    expect(isUrlOnRealm('https://demo.example.com', 'https://example.com')).toBe(false);
+    expect(isUrlOnRealm('https://demo.example.com', realm)).toBe(false);
 
-    expect(isUrlOnRealm('www.google.com', 'https://example.com')).toBe(false);
+    expect(isUrlOnRealm('www.google.com', realm)).toBe(false);
   });
 });
 

src/utils/internalLinks.js

@@ -4,9 +4,11 @@ import type { Narrow, Stream, User } from '../types';
 import { topicNarrow, streamNarrow, groupNarrow, specialNarrow } from './narrow';
 import { isUrlOnRealm } from './url';
 
-const getPathsFromUrl = (url: string = '', realm: string) => {
+// TODO: Work out what this does, write a jsdoc for its interface, and
+// reimplement using URL object (not just for the realm)
+const getPathsFromUrl = (url: string = '', realm: URL) => {
   const paths = url
-    .split(realm)
+    .split(realm.toString())
     .pop()
     .split('#narrow/')
     .pop()
@@ -20,16 +22,18 @@ const getPathsFromUrl = (url: string = '', realm: string) => {
 };
 
 /** PRIVATE -- exported only for tests. */
-export const isInternalLink = (url: string, realm: string): boolean =>
-  isUrlOnRealm(url, realm) ? /^(\/#narrow|#narrow)/i.test(url.split(realm).pop()) : false;
+export const isInternalLink = (url: string, realm: URL): boolean =>
+  isUrlOnRealm(url, realm)
+    ? /^(\/#narrow|#narrow)/i.test(url.split(realm.toString()).pop())
+    : false;
 
 /** PRIVATE -- exported only for tests. */
-export const isMessageLink = (url: string, realm: string): boolean =>
+export const isMessageLink = (url: string, realm: URL): boolean =>
   isInternalLink(url, realm) && url.includes('near');
 
 type LinkType = 'external' | 'home' | 'pm' | 'topic' | 'stream' | 'special';
 
-export const getLinkType = (url: string, realm: string): LinkType => {
+export const getLinkType = (url: string, realm: URL): LinkType => {
   if (!isInternalLink(url, realm)) {
     return 'external';
   }
@@ -115,7 +119,7 @@ const parsePmOperand = (operand, usersById) => {
 
 export const getNarrowFromLink = (
   url: string,
-  realm: string,
+  realm: URL,
   usersById: Map<number, User>,
   streamsById: Map<number, Stream>,
 ): Narrow | null => {
@@ -141,7 +145,7 @@ export const getNarrowFromLink = (
   }
 };
 
-export const getMessageIdFromLink = (url: string, realm: string): number => {
+export const getMessageIdFromLink = (url: string, realm: URL): number => {
   const paths = getPathsFromUrl(url, realm);
 
   return isMessageLink(url, realm) ? parseInt(paths[paths.lastIndexOf('near') + 1], 10) : 0;

src/utils/url.js

@@ -40,8 +40,10 @@ export const tryParseUrl = (url: string, base?: string | URL): URL | void => {
   }
 };
 
-export const isUrlOnRealm = (url: string = '', realm: string): boolean =>
-  url.startsWith('/') || url.startsWith(realm) || !/^(http|www.)/i.test(url);
+// TODO: Work out what this does, write a jsdoc for its interface, and
+// reimplement using URL object (not just for the realm)
+export const isUrlOnRealm = (url: string = '', realm: URL): boolean =>
+  url.startsWith('/') || url.startsWith(realm.toString()) || !/^(http|www.)/i.test(url);
 
 const getResourceWithAuth = (uri: string, auth: Auth) => ({
   uri: new URL(uri, auth.realm).toString(),
@@ -56,9 +58,7 @@ export const getResource = (
   uri: string,
   auth: Auth,
 ): {| uri: string, headers?: { [string]: string } |} =>
-  isUrlOnRealm(uri, auth.realm.toString())
-    ? getResourceWithAuth(uri, auth)
-    : getResourceNoAuth(uri);
+  isUrlOnRealm(uri, auth.realm) ? getResourceWithAuth(uri, auth) : getResourceNoAuth(uri);
 
 export type Protocol = 'https://' | 'http://';