From c022b3f4d04ad8ff27399053a8c97076675bb3f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Thu, 28 Jun 2018 13:32:47 -0700 Subject: [PATCH] `RelationalView` tracks GitHub references (#431) For every `TextContentEntity` (`Issue`, `Pull`, `Review`, `Comment`), this commit adds a `references` method that iterates over the entities that the text content entity references. For every `ReferentEntity` (actually, every entity), this commit adds a `referencedBy` method which iterates over the text content entities that reference that referent entity. This method also adds `referentEntities` and `textContentEntities` methods to the `RelationalView`, as they are used in both implementation and test code. Test plan: The snapshot tests include every reference, in a format that is very convenient for inspecting the ground truth on GitHub. For every reference, it's easy to check that the reference actually exists by copying the `from` url and pasting it into the browser. I've done this and they check out. (It's not easy to prove that there are no missing references, but I'm pretty confident that this code is working.) Unit tests ensure that the `references` and `referencedBy` methods are consistent. --- .../__snapshots__/relationalView.test.js.snap | 69 +++++++ src/v3/plugins/github/relationalView.js | 185 ++++++++++++++++++ src/v3/plugins/github/relationalView.test.js | 43 ++++ 3 files changed, 297 insertions(+) diff --git a/src/v3/plugins/github/__snapshots__/relationalView.test.js.snap b/src/v3/plugins/github/__snapshots__/relationalView.test.js.snap index 226f1fa..70390e4 100644 --- a/src/v3/plugins/github/__snapshots__/relationalView.test.js.snap +++ b/src/v3/plugins/github/__snapshots__/relationalView.test.js.snap @@ -237,3 +237,72 @@ exports[`plugins/github/relationalView Review has url 1`] = `"https://github.com exports[`plugins/github/relationalView Userlike has login 1`] = `"wchargin"`; exports[`plugins/github/relationalView Userlike has url 1`] = `"https://github.com/wchargin"`; + +exports[`plugins/github/relationalView reference detection references match snapshot 1`] = ` +Array [ + Object { + "from": "https://github.com/sourcecred/example-github/issues/2", + "to": "https://github.com/sourcecred/example-github/issues/1", + }, + Object { + "from": "https://github.com/sourcecred/example-github/pull/5", + "to": "https://github.com/wchargin", + }, + Object { + "from": "https://github.com/sourcecred/example-github/pull/9", + "to": "https://github.com/wchargin", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-373768703", + "to": "https://github.com/sourcecred/example-github/issues/6", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-373768850", + "to": "https://github.com/sourcecred/example-github/issues/6#issuecomment-373768538", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576185", + "to": "https://github.com/sourcecred/example-github/pull/5", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576220", + "to": "https://github.com/sourcecred/example-github/pull/5#pullrequestreview-100313899", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576248", + "to": "https://github.com/sourcecred/example-github/pull/5#discussion_r171460198", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576273", + "to": "https://github.com/wchargin", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576920", + "to": "https://github.com/sourcecred/example-github/issues/1", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576920", + "to": "https://github.com/sourcecred/example-github/issues/2", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576920", + "to": "https://github.com/sourcecred/example-github/pull/3", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576920", + "to": "https://github.com/sourcecred/example-github/pull/5#discussion_r171460198", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/2#issuecomment-385576920", + "to": "https://github.com/sourcecred/example-github/pull/5#pullrequestreview-100313899", + }, + Object { + "from": "https://github.com/sourcecred/example-github/issues/6#issuecomment-385223316", + "to": "https://github.com/sourcecred/example-github/issues/2", + }, + Object { + "from": "https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222", + "to": "https://github.com/sourcecred/example-github/issues/2", + }, +] +`; diff --git a/src/v3/plugins/github/relationalView.js b/src/v3/plugins/github/relationalView.js index a01a7f7..f76cd2f 100644 --- a/src/v3/plugins/github/relationalView.js +++ b/src/v3/plugins/github/relationalView.js @@ -1,6 +1,7 @@ // @flow import stringify from "json-stable-stringify"; +import {parseReferences} from "./parseReferences"; import * as N from "./nodes"; // Workaround for https://github.com/facebook/flow/issues/6538 import type { @@ -28,6 +29,8 @@ export class RelationalView { _comments: Map; _reviews: Map; _userlikes: Map; + _mapReferences: Map; + _mapReferencedBy: Map; constructor(data: Q.GithubResponseJSON) { this._repos = new Map(); @@ -36,7 +39,10 @@ export class RelationalView { this._comments = new Map(); this._reviews = new Map(); this._userlikes = new Map(); + this._mapReferences = new Map(); + this._mapReferencedBy = new Map(); this._addRepo(data.repository); + this._addReferences(); } *repos(): Iterator { @@ -105,6 +111,22 @@ export class RelationalView { return entry == null ? entry : new Userlike(this, entry); } + *referentEntities(): Iterator { + yield* this.repos(); + yield* this.issues(); + yield* this.pulls(); + yield* this.reviews(); + yield* this.comments(); + yield* this.userlikes(); + } + + *textContentEntities(): Iterator { + yield* this.issues(); + yield* this.pulls(); + yield* this.reviews(); + yield* this.comments(); + } + _addRepo(json: Q.RepositoryJSON) { const address: RepoAddress = { type: N.REPO_TYPE, @@ -227,6 +249,136 @@ export class RelationalView { return address; } } + + _addReferences() { + // 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. + // https://github.com/sourcecred/sourcecred/pull/416 + // - A # followed by a number, such as #416 + // - An @ followed by a login name, such as @decentralion + const refToAddress: Map = new Map(); + for (const e: ReferentEntity of this.referentEntities()) { + const a = e.address(); + refToAddress.set(e.url(), a); + if (e instanceof Userlike) { + refToAddress.set(`@${e.login()}`, a); + } + if (e instanceof Issue || e instanceof Pull) { + refToAddress.set(`#${e.number()}`, a); + } + } + for (const e of this.textContentEntities()) { + const srcAddress = e.address(); + for (const ref of parseReferences(e.body())) { + const refAddress = refToAddress.get(ref); + if (refAddress != null) { + this._addReference(srcAddress, refAddress); + } + } + } + } + + _addReference(src: N.TextContentAddress, dst: N.ReferentAddress) { + const srcRaw = N.toRaw(src); + const referencesForSrc = this._mapReferences.get(srcRaw); + if (referencesForSrc == null) { + this._mapReferences.set(srcRaw, [dst]); + } else { + referencesForSrc.push(dst); + } + const dstRaw = N.toRaw(dst); + const referencedByForDst = this._mapReferencedBy.get(dstRaw); + if (referencedByForDst == null) { + this._mapReferencedBy.set(dstRaw, [src]); + } else { + referencedByForDst.push(src); + } + } + + *_referencedBy(e: ReferentEntity): Iterator { + const refs = this._mapReferencedBy.get(N.toRaw(e.address())); + if (refs == null) { + return; + } else { + for (const address of refs) { + let entity: ?TextContentEntity; + switch (address.type) { + case "ISSUE": + entity = this.issue(address); + break; + case "PULL": + entity = this.pull(address); + break; + case "REVIEW": + entity = this.review(address); + break; + case "COMMENT": + entity = this.comment(address); + break; + default: + // eslint-disable-next-line no-unused-expressions + (address.type: empty); + throw new Error( + `Unexpected referrer address type: ${address.type}` + ); + } + if (entity == null) { + throw new Error( + `Invariant error: Reference from non-existent entity: ${stringify( + address + )}` + ); + } + yield entity; + } + } + } + + *_references(e: TextContentEntity): Iterator { + const refs = this._mapReferences.get(N.toRaw(e.address())); + if (refs == null) { + return; + } else { + for (const address of refs) { + let entity: ?ReferentEntity; + switch (address.type) { + case "REPO": + entity = this.repo(address); + break; + case "ISSUE": + entity = this.issue(address); + break; + case "PULL": + entity = this.pull(address); + break; + case "REVIEW": + entity = this.review(address); + break; + case "COMMENT": + entity = this.comment(address); + break; + case "USERLIKE": + entity = this.userlike(address); + break; + default: + // eslint-disable-next-line no-unused-expressions + (address.type: empty); + throw new Error( + `Unexpected referent address type: ${address.type}` + ); + } + if (entity == null) { + throw new Error( + `Invariant error: Reference to non-existent entity: ${stringify( + address + )}` + ); + } + yield entity; + } + } + } } type Entry = @@ -281,6 +433,9 @@ export class Repo extends Entity { yield assertExists(pull, address); } } + referencedBy(): Iterator { + return this._view._referencedBy(this); + } } type IssueEntry = {| @@ -319,6 +474,12 @@ export class Issue extends Entity { authors(): Iterator { return getAuthors(this._view, this._entry); } + references(): Iterator { + return this._view._references(this); + } + referencedBy(): Iterator { + return this._view._referencedBy(this); + } } type PullEntry = {| @@ -368,6 +529,12 @@ export class Pull extends Entity { authors(): Iterator { return getAuthors(this._view, this._entry); } + references(): Iterator { + return this._view._references(this); + } + referencedBy(): Iterator { + return this._view._referencedBy(this); + } } type ReviewEntry = {| @@ -403,6 +570,12 @@ export class Review extends Entity { authors(): Iterator { return getAuthors(this._view, this._entry); } + references(): Iterator { + return this._view._references(this); + } + referencedBy(): Iterator { + return this._view._referencedBy(this); + } } type CommentEntry = {| @@ -442,6 +615,12 @@ export class Comment extends Entity { authors(): Iterator { return getAuthors(this._view, this._entry); } + references(): Iterator { + return this._view._references(this); + } + referencedBy(): Iterator { + return this._view._referencedBy(this); + } } type UserlikeEntry = {| @@ -456,6 +635,9 @@ export class Userlike extends Entity { login(): string { return this.address().login; } + referencedBy(): Iterator { + return this._view._referencedBy(this); + } } function assertExists(item: ?T, address: N.StructuredAddress): T { @@ -477,3 +659,6 @@ function* getAuthors( yield assertExists(author, address); } } + +export type TextContentEntity = Issue | Pull | Review | Comment; +export type ReferentEntity = Repo | Issue | Pull | Review | Comment | Userlike; diff --git a/src/v3/plugins/github/relationalView.test.js b/src/v3/plugins/github/relationalView.test.js index 1fb2add..a1b8ae6 100644 --- a/src/v3/plugins/github/relationalView.test.js +++ b/src/v3/plugins/github/relationalView.test.js @@ -151,4 +151,47 @@ describe("plugins/github/relationalView", () => { hasCorrectParent("pull", pull); hasCorrectParent("review", review); }); + + describe("reference detection", () => { + // create url->url reference maps, for convenient snapshot readability + const allReferences: Map> = new Map(); + let nReferences = 0; + for (const referrer of view.textContentEntities()) { + const references = new Set(); + allReferences.set(referrer.url(), references); + for (const referenced of referrer.references()) { + references.add(referenced.url()); + nReferences++; + } + } + + it("references match snapshot", () => { + const everyReference = []; + for (const [referrerUrl, referencedUrls] of allReferences) { + for (const referencedUrl of referencedUrls) { + everyReference.push({from: referrerUrl, to: referencedUrl}); + } + } + expect(everyReference).toMatchSnapshot(); + }); + + it("correspondence between references() and referencedBy()", () => { + let nFoundReferences = 0; + for (const referent of view.referentEntities()) { + for (const referrer of referent.referencedBy()) { + nFoundReferences++; + const srcUrl = referrer.url(); + const dstUrl = referent.url(); + const actualRefsFromSrc = allReferences.get(srcUrl); + if (actualRefsFromSrc == null) { + throw new Error(`Expected refs for ${srcUrl}`); + } + if (!actualRefsFromSrc.has(dstUrl)) { + throw new Error(`Expected ref from ${srcUrl} to ${dstUrl}`); + } + } + } + expect(nFoundReferences).toEqual(nReferences); + }); + }); });