From e092b32bca5e5ddaf9200344511a5e3b6e2a9138 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dandelion=20Man=C3=A9?= <decentralion@dandelion.io>
Date: Mon, 4 Jun 2018 15:19:24 -0700
Subject: [PATCH] Add `addressAppend` (#332)

Consider a case where a user wants to construct many addresses sharing a
common prefix. For example, every GitHub entity may have an address
starting with the component sequence `["sourcecred", "github"]`.

Currently, users can implement this with `toParts`:

```js
const GITHUB_ADDRESS = Address.nodeAddress(["sourcecred", "github"]);
function createGithubAddress(ids: $ReadOnlyArray<string>): NodeAddress {
  return nodeAddress([...Address.toParts(GITHUB_ADDRESS), ...ids]);
}
```

This is unfortunate from a performance standpoint, as we needlessly
perform string and array operations, when under the hood this is
basically a string concatenation.

This commit fixes this by adding functions `nodeAppend` and
`edgeAppend`, which take a node (resp. edge) and some components to
append, returning a new address. Consider how straightforward our
example case becomes:

```js
const GITHUB_NODE_PREFIX = Address.nodeAddress(["sourcecred", "github"]);
const ISSUE_NODE_PREFIX = Address.nodeAppend(GITHUB_NODE_PREFIX, "issue");

// There is no longer any need for an explicit creation function
const anIssueAddress = Address.nodeAppend(ISSUE_NODE_PREFIX, someIssueID);
```

Paired with @wchargin.

Test Plan:
Unit tests added; run `yarn travis`.
---
 src/v3/core/_address.js      | 26 ++++++++++++--
 src/v3/core/_address.test.js | 67 +++++++++++++++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/src/v3/core/_address.js b/src/v3/core/_address.js
index fc5913f..e69864c 100644
--- a/src/v3/core/_address.js
+++ b/src/v3/core/_address.js
@@ -86,14 +86,18 @@ export function assertAddressArray(arr: $ReadOnlyArray<string>) {
   });
 }
 
+function nullDelimited(components: $ReadOnlyArray<string>): string {
+  return [...components, ""].join(SEPARATOR);
+}
+
 export function nodeAddress(arr: $ReadOnlyArray<string>): NodeAddress {
   assertAddressArray(arr);
-  return [NODE_PREFIX, ...arr, ""].join(SEPARATOR);
+  return NODE_PREFIX + SEPARATOR + nullDelimited(arr);
 }
 
 export function edgeAddress(arr: $ReadOnlyArray<string>): EdgeAddress {
   assertAddressArray(arr);
-  return [EDGE_PREFIX, ...arr, ""].join(SEPARATOR);
+  return EDGE_PREFIX + SEPARATOR + nullDelimited(arr);
 }
 
 export function toParts(a: GenericAddress): string[] {
@@ -101,3 +105,21 @@ export function toParts(a: GenericAddress): string[] {
   const parts = a.split(SEPARATOR);
   return parts.slice(1, parts.length - 1);
 }
+
+export function nodeAppend(
+  base: NodeAddress,
+  ...components: string[]
+): NodeAddress {
+  assertNodeAddress(base);
+  assertAddressArray(components);
+  return base + nullDelimited(components);
+}
+
+export function edgeAppend(
+  base: EdgeAddress,
+  ...components: string[]
+): EdgeAddress {
+  assertEdgeAddress(base);
+  assertAddressArray(components);
+  return base + nullDelimited(components);
+}
diff --git a/src/v3/core/_address.test.js b/src/v3/core/_address.test.js
index bd38890..f8b2e26 100644
--- a/src/v3/core/_address.test.js
+++ b/src/v3/core/_address.test.js
@@ -1,10 +1,13 @@
 // @flow
 
+import type {NodeAddress, EdgeAddress} from "./_address";
 import {
   assertNodeAddress,
   assertEdgeAddress,
-  nodeAddress,
   edgeAddress,
+  edgeAppend,
+  nodeAddress,
+  nodeAppend,
   toParts,
 } from "./_address";
 
@@ -79,6 +82,68 @@ describe("core/address", () => {
     });
   });
 
+  function checkAppend<
+    Good: NodeAddress | EdgeAddress,
+    Bad: NodeAddress | EdgeAddress
+  >(
+    f: (Good, ...string[]) => Good,
+    goodConstructor: (string[]) => Good,
+    badConstructor: (string[]) => Bad
+  ) {
+    describe(f.name, () => {
+      describe("errors on", () => {
+        [null, undefined].forEach((bad) => {
+          it(`${String(bad)} base input`, () => {
+            // $ExpectFlowError
+            expect(() => f(bad, "foo")).toThrow(String(bad));
+          });
+          it(`${String(bad)} component`, () => {
+            // $ExpectFlowError
+            expect(() => f(goodConstructor(["foo"]), bad)).toThrow(String(bad));
+          });
+        });
+        it("malformed base", () => {
+          // $ExpectFlowError
+          expect(() => f("foo", "foo")).toThrow("Bad address");
+        });
+        it("base of wrong kind", () => {
+          // $ExpectFlowError
+          expect(() => f(badConstructor(["foo"]), "foo")).toThrow(
+            /Expected.*Address/
+          );
+        });
+        it("invalid component", () => {
+          expect(() => f(goodConstructor(["foo"]), "foo\0oo"));
+        });
+      });
+
+      describe("works on", () => {
+        function check(
+          description: string,
+          baseComponents: string[],
+          ...components: string[]
+        ) {
+          test(description, () => {
+            const base = goodConstructor(baseComponents);
+            const expectedParts = [...baseComponents, ...components];
+            expect(toParts(f(base, ...components))).toEqual(expectedParts);
+          });
+        }
+        check("the base address with no extra component", []);
+        check("the base address with empty component", [], "");
+        check("the base address with nonempty component", [], "a");
+        check("the base address with lots of components", [], "a", "b");
+
+        check("a longer address with no extra component", ["a", ""]);
+        check("a longer address with empty component", ["a", ""], "");
+        check("a longer address with nonempty component", ["a", ""], "b");
+        check("a longer address with lots of components", ["a", ""], "b", "c");
+      });
+    });
+  }
+  checkAppend(nodeAppend, nodeAddress, edgeAddress);
+  checkAppend(edgeAppend, edgeAddress, nodeAddress);
+
   describe("type assertions", () => {
     function checkAssertion(f, good, bad, badMsg) {
       describe(f.name, () => {