`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.
This commit is contained in:
Dandelion Mané 2018-06-28 13:32:47 -07:00 committed by GitHub
parent 6235febdac
commit c022b3f4d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 297 additions and 0 deletions

View File

@ -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",
},
]
`;

View File

@ -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<N.RawAddress, CommentEntry>;
_reviews: Map<N.RawAddress, ReviewEntry>;
_userlikes: Map<N.RawAddress, UserlikeEntry>;
_mapReferences: Map<N.RawAddress, N.ReferentAddress[]>;
_mapReferencedBy: Map<N.RawAddress, N.TextContentAddress[]>;
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<Repo> {
@ -105,6 +111,22 @@ export class RelationalView {
return entry == null ? entry : new Userlike(this, entry);
}
*referentEntities(): Iterator<ReferentEntity> {
yield* this.repos();
yield* this.issues();
yield* this.pulls();
yield* this.reviews();
yield* this.comments();
yield* this.userlikes();
}
*textContentEntities(): Iterator<TextContentEntity> {
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<string, N.StructuredAddress> = 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<TextContentEntity> {
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<ReferentEntity> {
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<RepoEntry> {
yield assertExists(pull, address);
}
}
referencedBy(): Iterator<ReferentEntity> {
return this._view._referencedBy(this);
}
}
type IssueEntry = {|
@ -319,6 +474,12 @@ export class Issue extends Entity<IssueEntry> {
authors(): Iterator<Userlike> {
return getAuthors(this._view, this._entry);
}
references(): Iterator<ReferentEntity> {
return this._view._references(this);
}
referencedBy(): Iterator<TextContentEntity> {
return this._view._referencedBy(this);
}
}
type PullEntry = {|
@ -368,6 +529,12 @@ export class Pull extends Entity<PullEntry> {
authors(): Iterator<Userlike> {
return getAuthors(this._view, this._entry);
}
references(): Iterator<ReferentEntity> {
return this._view._references(this);
}
referencedBy(): Iterator<TextContentEntity> {
return this._view._referencedBy(this);
}
}
type ReviewEntry = {|
@ -403,6 +570,12 @@ export class Review extends Entity<ReviewEntry> {
authors(): Iterator<Userlike> {
return getAuthors(this._view, this._entry);
}
references(): Iterator<ReferentEntity> {
return this._view._references(this);
}
referencedBy(): Iterator<TextContentEntity> {
return this._view._referencedBy(this);
}
}
type CommentEntry = {|
@ -442,6 +615,12 @@ export class Comment extends Entity<CommentEntry> {
authors(): Iterator<Userlike> {
return getAuthors(this._view, this._entry);
}
references(): Iterator<ReferentEntity> {
return this._view._references(this);
}
referencedBy(): Iterator<TextContentEntity> {
return this._view._referencedBy(this);
}
}
type UserlikeEntry = {|
@ -456,6 +635,9 @@ export class Userlike extends Entity<UserlikeEntry> {
login(): string {
return this.address().login;
}
referencedBy(): Iterator<TextContentEntity> {
return this._view._referencedBy(this);
}
}
function assertExists<T>(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;

View File

@ -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<string, Set<string>> = 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);
});
});
});