From ed3397f6549565420c7137e45006fe2b64eb9fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Thu, 14 Jun 2018 15:01:33 -0700 Subject: [PATCH] Add GitHub prefixes and const types (#395) - Switch string constant node and edge types (e.g. "REPO") to exported consts (eg `export const REPO_TYPE`). - Add (and internally use) a `_Prefix` psuedomodule which contains per-type address prefixes - Test that constructing a StructuredAddress with the wrong type is an error. Test plan: Unit tests pass, snapshots unchanged. Paired with @wchargin --- src/v3/plugins/github/edges.js | 78 ++++++++------- src/v3/plugins/github/edges.test.js | 32 ++++--- src/v3/plugins/github/nodes.js | 143 +++++++++++++++++----------- src/v3/plugins/github/nodes.test.js | 54 ++++++++--- 4 files changed, 192 insertions(+), 115 deletions(-) diff --git a/src/v3/plugins/github/edges.js b/src/v3/plugins/github/edges.js index 7ad2c6a..8ef61df 100644 --- a/src/v3/plugins/github/edges.js +++ b/src/v3/plugins/github/edges.js @@ -11,21 +11,39 @@ import * as GithubNode from "./nodes"; export opaque type RawAddress: EdgeAddressT = EdgeAddressT; +export const AUTHORS_TYPE = "AUTHORS"; +export const MERGED_AS_TYPE = "MERGED_AS"; +export const HAS_PARENT_TYPE = "HAS_PARENT"; +export const REFERENCES_TYPE = "REFERENCES"; + +const GITHUB_PREFIX = EdgeAddress.fromParts(["sourcecred", "github"]); +function githubEdgeAddress(...parts: string[]): RawAddress { + return EdgeAddress.append(GITHUB_PREFIX, ...parts); +} + +export const _Prefix = Object.freeze({ + base: GITHUB_PREFIX, + authors: githubEdgeAddress(AUTHORS_TYPE), + mergedAs: githubEdgeAddress(MERGED_AS_TYPE), + references: githubEdgeAddress(REFERENCES_TYPE), + hasParent: githubEdgeAddress(HAS_PARENT_TYPE), +}); + export type AuthorsAddress = {| - +type: "AUTHORS", + +type: typeof AUTHORS_TYPE, +author: GithubNode.UserlikeAddress, +content: GithubNode.AuthorableAddress, |}; export type MergedAsAddress = {| - +type: "MERGED_AS", + +type: typeof MERGED_AS_TYPE, +pull: GithubNode.PullAddress, |}; export type HasParentAddress = {| - +type: "HAS_PARENT", + +type: typeof HAS_PARENT_TYPE, +child: GithubNode.ChildAddress, |}; export type ReferencesAddress = {| - +type: "REFERENCES", + +type: typeof REFERENCES_TYPE, +referrer: GithubNode.TextContentAddress, +referent: GithubNode.ReferentAddress, |}; @@ -41,7 +59,7 @@ export const createEdge = Object.freeze({ author: GithubNode.UserlikeAddress, content: GithubNode.AuthorableAddress ): Edge => ({ - address: toRaw({type: "AUTHORS", author, content}), + address: toRaw({type: AUTHORS_TYPE, author, content}), src: GithubNode.toRaw(author), dst: GithubNode.toRaw(content), }), @@ -49,7 +67,7 @@ export const createEdge = Object.freeze({ pull: GithubNode.PullAddress, commitAddress: NodeAddressT /* TODO: Make this a Git commit node address. */ ): Edge => ({ - address: toRaw({type: "MERGED_AS", pull}), + address: toRaw({type: MERGED_AS_TYPE, pull}), src: GithubNode.toRaw(pull), dst: commitAddress, }), @@ -57,7 +75,7 @@ export const createEdge = Object.freeze({ child: GithubNode.ChildAddress, parent: GithubNode.ParentAddress ): Edge => ({ - address: toRaw({type: "HAS_PARENT", child}), + address: toRaw({type: HAS_PARENT_TYPE, child}), src: GithubNode.toRaw(child), dst: GithubNode.toRaw(parent), }), @@ -65,7 +83,7 @@ export const createEdge = Object.freeze({ referrer: GithubNode.TextContentAddress, referent: GithubNode.ReferentAddress ): Edge => ({ - address: toRaw({type: "REFERENCES", referrer, referent}), + address: toRaw({type: REFERENCES_TYPE, referrer, referent}), src: GithubNode.toRaw(referrer), dst: GithubNode.toRaw(referent), }), @@ -74,10 +92,6 @@ export const createEdge = Object.freeze({ const NODE_PREFIX_LENGTH = NodeAddress.toParts(GithubNode._githubAddress()) .length; -const GITHUB_PREFIX = EdgeAddress.fromParts(["sourcecred", "github"]); -function githubEdgeAddress(...parts: string[]): RawAddress { - return EdgeAddress.append(GITHUB_PREFIX, ...parts); -} function lengthEncode(x: GithubNode.RawAddress): $ReadOnlyArray { const baseParts = NodeAddress.toParts(x).slice(NODE_PREFIX_LENGTH); return [String(baseParts.length), ...baseParts]; @@ -121,7 +135,7 @@ export function fromRaw(x: RawAddress): StructuredAddress { } const [_unused_sc, _unused_gh, kind, ...rest] = EdgeAddress.toParts(x); switch (kind) { - case "AUTHORS": { + case AUTHORS_TYPE: { const parts = multiLengthDecode(rest, fail); if (parts.length !== 2) { throw fail(); @@ -133,9 +147,9 @@ export function fromRaw(x: RawAddress): StructuredAddress { const content: GithubNode.AuthorableAddress = (GithubNode.fromRaw( GithubNode._githubAddress(...contentParts) ): any); - return ({type: "AUTHORS", author, content}: AuthorsAddress); + return ({type: AUTHORS_TYPE, author, content}: AuthorsAddress); } - case "MERGED_AS": { + case MERGED_AS_TYPE: { const parts = multiLengthDecode(rest, fail); if (parts.length !== 1) { throw fail(); @@ -144,9 +158,9 @@ export function fromRaw(x: RawAddress): StructuredAddress { const pull: GithubNode.PullAddress = (GithubNode.fromRaw( GithubNode._githubAddress(...pullParts) ): any); - return ({type: "MERGED_AS", pull}: MergedAsAddress); + return ({type: MERGED_AS_TYPE, pull}: MergedAsAddress); } - case "HAS_PARENT": { + case HAS_PARENT_TYPE: { const parts = multiLengthDecode(rest, fail); if (parts.length !== 1) { throw fail(); @@ -155,9 +169,9 @@ export function fromRaw(x: RawAddress): StructuredAddress { const child: GithubNode.ChildAddress = (GithubNode.fromRaw( GithubNode._githubAddress(...childParts) ): any); - return ({type: "HAS_PARENT", child}: HasParentAddress); + return ({type: HAS_PARENT_TYPE, child}: HasParentAddress); } - case "REFERENCES": { + case REFERENCES_TYPE: { const parts = multiLengthDecode(rest, fail); if (parts.length !== 2) { throw fail(); @@ -169,7 +183,7 @@ export function fromRaw(x: RawAddress): StructuredAddress { const referent: GithubNode.ReferentAddress = (GithubNode.fromRaw( GithubNode._githubAddress(...referentParts) ): any); - return ({type: "REFERENCES", referrer, referent}: ReferencesAddress); + return ({type: REFERENCES_TYPE, referrer, referent}: ReferencesAddress); } default: throw fail(); @@ -178,25 +192,25 @@ export function fromRaw(x: RawAddress): StructuredAddress { export function toRaw(x: StructuredAddress): RawAddress { switch (x.type) { - case "AUTHORS": - return githubEdgeAddress( - "AUTHORS", + case AUTHORS_TYPE: + return EdgeAddress.append( + _Prefix.authors, ...lengthEncode(GithubNode.toRaw(x.author)), ...lengthEncode(GithubNode.toRaw(x.content)) ); - case "MERGED_AS": - return githubEdgeAddress( - "MERGED_AS", + case MERGED_AS_TYPE: + return EdgeAddress.append( + _Prefix.mergedAs, ...lengthEncode(GithubNode.toRaw(x.pull)) ); - case "HAS_PARENT": - return githubEdgeAddress( - "HAS_PARENT", + case HAS_PARENT_TYPE: + return EdgeAddress.append( + _Prefix.hasParent, ...lengthEncode(GithubNode.toRaw(x.child)) ); - case "REFERENCES": - return githubEdgeAddress( - "REFERENCES", + case REFERENCES_TYPE: + return EdgeAddress.append( + _Prefix.references, ...lengthEncode(GithubNode.toRaw(x.referrer)), ...lengthEncode(GithubNode.toRaw(x.referent)) ); diff --git a/src/v3/plugins/github/edges.test.js b/src/v3/plugins/github/edges.test.js index 0aca42d..35028c0 100644 --- a/src/v3/plugins/github/edges.test.js +++ b/src/v3/plugins/github/edges.test.js @@ -2,57 +2,63 @@ import {NodeAddress, EdgeAddress, edgeToString} from "../../core/graph"; import {createEdge, fromRaw, toRaw} from "./edges"; +import * as GE from "./edges"; +import * as GN from "./nodes"; describe("plugins/github/edges", () => { const nodeExamples = { repo: () => ({ - type: "REPO", + type: GN.REPO_TYPE, owner: "sourcecred", name: "example-github", }), - issue: () => ({type: "ISSUE", repo: nodeExamples.repo(), number: "2"}), - pull: () => ({type: "PULL", repo: nodeExamples.repo(), number: "5"}), + issue: () => ({ + type: GN.ISSUE_TYPE, + repo: nodeExamples.repo(), + number: "2", + }), + pull: () => ({type: GN.PULL_TYPE, repo: nodeExamples.repo(), number: "5"}), review: () => ({ - type: "REVIEW", + type: GN.REVIEW_TYPE, pull: nodeExamples.pull(), id: "100313899", }), issueComment: () => ({ - type: "COMMENT", + type: GN.COMMENT_TYPE, parent: nodeExamples.issue(), id: "373768703", }), pullComment: () => ({ - type: "COMMENT", + type: GN.COMMENT_TYPE, parent: nodeExamples.pull(), id: "396430464", }), reviewComment: () => ({ - type: "COMMENT", + type: GN.COMMENT_TYPE, parent: nodeExamples.review(), id: "171460198", }), - user: () => ({type: "USERLIKE", login: "decentralion"}), + user: () => ({type: GN.USERLIKE_TYPE, login: "decentralion"}), }; const edgeExamples = { authors: () => ({ - type: "AUTHORS", + type: GE.AUTHORS_TYPE, author: nodeExamples.user(), content: nodeExamples.pull(), }), mergedAs: () => ({ - type: "MERGED_AS", + type: GE.MERGED_AS_TYPE, pull: nodeExamples.pull(), }), hasParent: () => ({ - type: "HAS_PARENT", + type: GE.HAS_PARENT_TYPE, child: nodeExamples.reviewComment(), }), references: () => ({ - type: "REFERENCES", + type: GE.REFERENCES_TYPE, referrer: nodeExamples.issue(), - referent: {type: "ISSUE", repo: nodeExamples.repo(), number: "1"}, + referent: {type: GN.ISSUE_TYPE, repo: nodeExamples.repo(), number: "1"}, }), }; diff --git a/src/v3/plugins/github/nodes.js b/src/v3/plugins/github/nodes.js index c33746a..e59c42b 100644 --- a/src/v3/plugins/github/nodes.js +++ b/src/v3/plugins/github/nodes.js @@ -9,33 +9,53 @@ export function _githubAddress(...parts: string[]): RawAddress { return NodeAddress.append(GITHUB_PREFIX, ...parts); } +export const REPO_TYPE: "REPO" = "REPO"; +export const ISSUE_TYPE: "ISSUE" = "ISSUE"; +export const PULL_TYPE: "PULL" = "PULL"; +export const REVIEW_TYPE: "REVIEW" = "REVIEW"; +export const COMMENT_TYPE: "COMMENT" = "COMMENT"; +export const USERLIKE_TYPE: "USERLIKE" = "USERLIKE"; + +export const _Prefix = Object.freeze({ + base: GITHUB_PREFIX, + repo: _githubAddress(REPO_TYPE), + issue: _githubAddress(ISSUE_TYPE), + pull: _githubAddress(PULL_TYPE), + review: _githubAddress(REVIEW_TYPE), + comment: _githubAddress(COMMENT_TYPE), + userlike: _githubAddress(USERLIKE_TYPE), + reviewComment: _githubAddress(COMMENT_TYPE, REVIEW_TYPE), + issueComment: _githubAddress(COMMENT_TYPE, ISSUE_TYPE), + pullComment: _githubAddress(COMMENT_TYPE, PULL_TYPE), +}); + export type RepoAddress = {| - +type: "REPO", + +type: typeof REPO_TYPE, +owner: string, +name: string, |}; export type IssueAddress = {| - +type: "ISSUE", + +type: typeof ISSUE_TYPE, +repo: RepoAddress, +number: string, |}; export type PullAddress = {| - +type: "PULL", + +type: typeof PULL_TYPE, +repo: RepoAddress, +number: string, |}; export type ReviewAddress = {| - +type: "REVIEW", + +type: typeof REVIEW_TYPE, +pull: PullAddress, +id: string, |}; export type CommentAddress = {| - +type: "COMMENT", + +type: typeof COMMENT_TYPE, +parent: CommentableAddress, +id: string, |}; export type UserlikeAddress = {| - +type: "USERLIKE", + +type: typeof USERLIKE_TYPE, +login: string, |}; @@ -105,82 +125,82 @@ export function fromRaw(x: RawAddress): StructuredAddress { } const [_unused_sc, _unused_gh, kind, ...rest] = NodeAddress.toParts(x); switch (kind) { - case "REPO": { + case REPO_TYPE: { if (rest.length !== 2) { throw fail(); } const [owner, name] = rest; - return {type: "REPO", owner, name}; + return {type: REPO_TYPE, owner, name}; } - case "ISSUE": { + case ISSUE_TYPE: { if (rest.length !== 3) { throw fail(); } const [owner, name, number] = rest; - const repo = {type: "REPO", owner, name}; - return {type: "ISSUE", repo, number}; + const repo = {type: REPO_TYPE, owner, name}; + return {type: ISSUE_TYPE, repo, number}; } - case "PULL": { + case PULL_TYPE: { if (rest.length !== 3) { throw fail(); } const [owner, name, number] = rest; - const repo = {type: "REPO", owner, name}; - return {type: "PULL", repo, number}; + const repo = {type: REPO_TYPE, owner, name}; + return {type: PULL_TYPE, repo, number}; } - case "REVIEW": { + case REVIEW_TYPE: { if (rest.length !== 4) { throw fail(); } const [owner, name, pullNumber, id] = rest; - const repo = {type: "REPO", owner, name}; - const pull = {type: "PULL", repo, number: pullNumber}; - return {type: "REVIEW", pull, id}; + const repo = {type: REPO_TYPE, owner, name}; + const pull = {type: PULL_TYPE, repo, number: pullNumber}; + return {type: REVIEW_TYPE, pull, id}; } - case "COMMENT": { + case COMMENT_TYPE: { if (rest.length < 1) { throw fail(); } const [subkind, ...subrest] = rest; switch (subkind) { - case "ISSUE": { + case ISSUE_TYPE: { if (subrest.length !== 4) { throw fail(); } const [owner, name, issueNumber, id] = subrest; - const repo = {type: "REPO", owner, name}; - const issue = {type: "ISSUE", repo, number: issueNumber}; - return {type: "COMMENT", parent: issue, id}; + const repo = {type: REPO_TYPE, owner, name}; + const issue = {type: ISSUE_TYPE, repo, number: issueNumber}; + return {type: COMMENT_TYPE, parent: issue, id}; } - case "PULL": { + case PULL_TYPE: { if (subrest.length !== 4) { throw fail(); } const [owner, name, pullNumber, id] = subrest; - const repo = {type: "REPO", owner, name}; - const pull = {type: "PULL", repo, number: pullNumber}; - return {type: "COMMENT", parent: pull, id}; + const repo = {type: REPO_TYPE, owner, name}; + const pull = {type: PULL_TYPE, repo, number: pullNumber}; + return {type: COMMENT_TYPE, parent: pull, id}; } - case "REVIEW": { + case REVIEW_TYPE: { if (subrest.length !== 5) { throw fail(); } const [owner, name, pullNumber, reviewFragment, id] = subrest; - const repo = {type: "REPO", owner, name}; - const pull = {type: "PULL", repo, number: pullNumber}; - const review = {type: "REVIEW", pull, id: reviewFragment}; - return {type: "COMMENT", parent: review, id}; + const repo = {type: REPO_TYPE, owner, name}; + const pull = {type: PULL_TYPE, repo, number: pullNumber}; + const review = {type: REVIEW_TYPE, pull, id: reviewFragment}; + return {type: COMMENT_TYPE, parent: review, id}; } default: throw fail(); } } - case "USERLIKE": { + case USERLIKE_TYPE: { if (rest.length !== 1) { throw fail(); } const [login] = rest; - return {type: "USERLIKE", login}; + return {type: USERLIKE_TYPE, login}; } default: throw fail(); @@ -189,44 +209,51 @@ export function fromRaw(x: RawAddress): StructuredAddress { export function toRaw(x: StructuredAddress): RawAddress { switch (x.type) { - case "REPO": - return _githubAddress("REPO", x.owner, x.name); - case "ISSUE": - return _githubAddress("ISSUE", x.repo.owner, x.repo.name, x.number); - case "PULL": - return _githubAddress("PULL", x.repo.owner, x.repo.name, x.number); - case "REVIEW": - return _githubAddress( - "REVIEW", + case REPO_TYPE: + return NodeAddress.append(_Prefix.repo, x.owner, x.name); + case ISSUE_TYPE: + return NodeAddress.append( + _Prefix.issue, + x.repo.owner, + x.repo.name, + x.number + ); + case PULL_TYPE: + return NodeAddress.append( + _Prefix.pull, + x.repo.owner, + x.repo.name, + x.number + ); + case REVIEW_TYPE: + return NodeAddress.append( + _Prefix.review, x.pull.repo.owner, x.pull.repo.name, x.pull.number, x.id ); - case "COMMENT": + case COMMENT_TYPE: switch (x.parent.type) { - case "ISSUE": - return _githubAddress( - "COMMENT", - "ISSUE", + case ISSUE_TYPE: + return NodeAddress.append( + _Prefix.issueComment, x.parent.repo.owner, x.parent.repo.name, x.parent.number, x.id ); - case "PULL": - return _githubAddress( - "COMMENT", - "PULL", + case PULL_TYPE: + return NodeAddress.append( + _Prefix.pullComment, x.parent.repo.owner, x.parent.repo.name, x.parent.number, x.id ); - case "REVIEW": - return _githubAddress( - "COMMENT", - "REVIEW", + case REVIEW_TYPE: + return NodeAddress.append( + _Prefix.reviewComment, x.parent.pull.repo.owner, x.parent.pull.repo.name, x.parent.pull.number, @@ -238,8 +265,8 @@ export function toRaw(x: StructuredAddress): RawAddress { (x.parent.type: empty); throw new Error(`Bad comment parent type: ${x.parent.type}`); } - case "USERLIKE": - return _githubAddress("USERLIKE", x.login); + case USERLIKE_TYPE: + return NodeAddress.append(_Prefix.userlike, x.login); default: // eslint-disable-next-line no-unused-expressions (x.type: empty); diff --git a/src/v3/plugins/github/nodes.test.js b/src/v3/plugins/github/nodes.test.js index 941916b..ff7eb7e 100644 --- a/src/v3/plugins/github/nodes.test.js +++ b/src/v3/plugins/github/nodes.test.js @@ -1,33 +1,50 @@ // @flow import {NodeAddress} from "../../core/graph"; +import * as GN from "./nodes"; import {fromRaw, toRaw} from "./nodes"; describe("plugins/github/nodes", () => { - const repo = () => ({ - type: "REPO", + const repo = (): GN.RepoAddress => ({ + type: GN.REPO_TYPE, owner: "sourcecred", name: "example-github", }); - const issue = () => ({type: "ISSUE", repo: repo(), number: "2"}); - const pull = () => ({type: "PULL", repo: repo(), number: "5"}); - const review = () => ({type: "REVIEW", pull: pull(), id: "100313899"}); - const issueComment = () => ({ - type: "COMMENT", + const issue = (): GN.IssueAddress => ({ + type: GN.ISSUE_TYPE, + repo: repo(), + number: "2", + }); + const pull = (): GN.PullAddress => ({ + type: GN.PULL_TYPE, + repo: repo(), + number: "5", + }); + const review = (): GN.ReviewAddress => ({ + type: GN.REVIEW_TYPE, + pull: pull(), + id: "100313899", + }); + const issueComment = (): GN.CommentAddress => ({ + type: GN.COMMENT_TYPE, parent: issue(), id: "373768703", }); - const pullComment = () => ({ - type: "COMMENT", + const pullComment = (): GN.CommentAddress => ({ + type: GN.COMMENT_TYPE, parent: pull(), id: "396430464", }); - const reviewComment = () => ({ - type: "COMMENT", + const reviewComment = (): GN.CommentAddress => ({ + type: GN.COMMENT_TYPE, parent: review(), id: "171460198", }); - const user = () => ({type: "USERLIKE", login: "decentralion"}); + const user = (): GN.UserlikeAddress => ({ + type: GN.USERLIKE_TYPE, + login: "decentralion", + }); + const examples = { repo, issue, @@ -39,6 +56,19 @@ describe("plugins/github/nodes", () => { user, }; + // Incorrect types should be caught statically, either due to being + // totally invalid... + // $ExpectFlowError + const _unused_badRepo: GN.RepoAddress = { + type: "REPOSITORY", + owner: "foo", + name: "bar", + }; + // ...or due to being annotated with the type of a distinct structured + // address: + // $ExpectFlowError + const _unused_badIssue: GN.IssueAddress = {...pull()}; + describe("`fromRaw` after `toRaw` is identity", () => { Object.keys(examples).forEach((example) => { it(example, () => {