From 64df5b09c3e0f95772d52e2bc728c4a805f5b606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Thu, 28 Jun 2018 18:31:58 -0700 Subject: [PATCH] Add `RelationalView.addData` (#442) Now that we want to implement RelationalView de/serialization, we need a way to construct one without adding data to it. Now that we're allowing `addData` to be called explicitly, we also want to make sure it's idempotent, which necessitated a small change to reference handling. A new test verifies idempotency. Test plan: travis Paired with @wchargin --- src/v3/plugins/github/example/example.js | 4 +++- src/v3/plugins/github/relationalView.js | 12 +++++++++++- src/v3/plugins/github/relationalView.test.js | 12 +++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/v3/plugins/github/example/example.js b/src/v3/plugins/github/example/example.js index 7057fdf..fdf9e45 100644 --- a/src/v3/plugins/github/example/example.js +++ b/src/v3/plugins/github/example/example.js @@ -11,7 +11,9 @@ export function exampleData(): GithubResponseJSON { } export function exampleRelationalView(): RelationalView { - return new RelationalView(exampleData()); + const rv = new RelationalView(); + rv.addData(exampleData()); + return rv; } export function exampleGraph(): Graph { diff --git a/src/v3/plugins/github/relationalView.js b/src/v3/plugins/github/relationalView.js index 9bd7efe..546c60a 100644 --- a/src/v3/plugins/github/relationalView.js +++ b/src/v3/plugins/github/relationalView.js @@ -32,7 +32,7 @@ export class RelationalView { _mapReferences: Map; _mapReferencedBy: Map; - constructor(data: Q.GithubResponseJSON) { + constructor() { this._repos = new Map(); this._issues = new Map(); this._pulls = new Map(); @@ -41,6 +41,13 @@ export class RelationalView { this._userlikes = new Map(); this._mapReferences = new Map(); this._mapReferencedBy = new Map(); + } + + addData(data: Q.GithubResponseJSON) { + // Warning: calling `addData` can put the RelationalView in an inconistent + // state. for example, if called with {repo: {issues: [1,2,3]}} and then with + // {repo: {issues: [4, 5]}}, then calls to repo.issues() will only give back + // issues 4 and 5 (although issues 1, 2, and 3 will still be in the view) this._addRepo(data.repository); this._addReferences(); } @@ -281,6 +288,9 @@ export class RelationalView { } _addReferences() { + // TODO(perf): _addReferences regenerates all refs from scratch + this._mapReferences = new Map(); + this._mapReferencedBy = new Map(); // refToAddress maps a "referencing string" to the address that string refers to. // There are 3 kinds of valid referencing strings: // - A canonical URL pointing to a GitHub entity, e.g. diff --git a/src/v3/plugins/github/relationalView.test.js b/src/v3/plugins/github/relationalView.test.js index da53373..929d076 100644 --- a/src/v3/plugins/github/relationalView.test.js +++ b/src/v3/plugins/github/relationalView.test.js @@ -2,7 +2,7 @@ import * as R from "./relationalView"; import * as N from "./nodes"; -import {exampleRelationalView} from "./example/example"; +import {exampleData, exampleRelationalView} from "./example/example"; describe("plugins/github/relationalView", () => { // Sharing this state is OK because it's just a view - no mutation allowed! @@ -213,4 +213,14 @@ describe("plugins/github/relationalView", () => { expect(nFoundReferences).toEqual(nReferences); }); }); + + it("addData is idempotent", () => { + const rv1 = new R.RelationalView(); + rv1.addData(exampleData()); + const rv2 = new R.RelationalView(); + rv2.addData(exampleData()); + rv2.addData(exampleData()); + // may be fragile + expect(rv1).toEqual(rv2); + }); });