Skip to content

Commit

Permalink
feat: Remove attributes related to file fetching since it is not impl…
Browse files Browse the repository at this point in the history
…emented.

BREAKING CHANGE:
- The attributes related to fetching files has been removed from the respective types.
- The IGitHubRepository type has been replaced by Repository.
- The IGitHubPullRequestPatches type has been replaced by PullRequestPatches.

Signed-off-by: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com>
  • Loading branch information
yorinasub17 committed Oct 30, 2023
1 parent 49deefa commit 64e37fc
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 95 deletions.
9 changes: 0 additions & 9 deletions src/engine/interpreter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ test("sanity check", async () => {
ruleFn,
[
{
contentsID: "helloworld",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand All @@ -46,7 +45,6 @@ test("sanity check old version", async () => {
ruleFn,
[
{
contentsID: "helloworld",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand All @@ -70,7 +68,6 @@ test("ES5 minify", async () => {
ruleFn,
[
{
contentsID: "helloworld",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand All @@ -96,7 +93,6 @@ test("ES6 support", async () => {
ruleFn,
[
{
contentsID: "helloworld",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand Down Expand Up @@ -127,7 +123,6 @@ function main(inp: IPatch[], metadata: IChangeSetMetadata) {
ruleFn,
[
{
contentsID: "helloworld",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand Down Expand Up @@ -280,7 +275,6 @@ test("XMLHTTPRequest not supported", async () => {
ruleFn,
[
{
contentsID: "http://example.com/example.txt",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand All @@ -307,7 +301,6 @@ test("fetch is not supported", async () => {
ruleFn,
[
{
contentsID: "http://example.com/example.txt",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand All @@ -332,7 +325,6 @@ test("process is not supported", async () => {
ruleFn,
[
{
contentsID: "helloworld",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand All @@ -357,7 +349,6 @@ test("Deno is not supported", async () => {
ruleFn,
[
{
contentsID: "helloworld",
path: "foo.txt",
op: PatchOp.Insert,
additions: 0,
Expand Down
4 changes: 0 additions & 4 deletions src/engine/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,9 @@ export enum RuleLogMode {

/**
* Options for the rule interpreter engine.
* @property fileFetchMap The mapping from source platforms to the URL map for fetching file contents.
* @property fileFetchClients The authenticated API clients to use for fetching the files.
* @property logMode The logging mode that the interpreter should operate in.
*/
export interface IRuleInterpreterOpts {
fileFetchMap?: Record<string, Record<string, URL>>;
fileFetchClients?: IFileFetchClients;
logMode?: RuleLogMode;
}

Expand Down
3 changes: 0 additions & 3 deletions src/engine/patch_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ export enum PatchOp {

/**
* Represents updates to a single file that was done in the change set.
* @property contentsID A unique ID that can be used to retrieve the file contents. The ID is in the format
* <SOURCE_PLATFORM>:<CONTENTS_URL_HASH>.
* @property path The relative path (from the root of the repo) to the file that was updated in the patch.
* @property op The operation that was done on the file in the patch.
* @property additions The number of lines that were added in this patch.
Expand All @@ -74,7 +72,6 @@ export enum PatchOp {
* @property objectDiff If the file represents a parsable data file (e.g., json, yaml, toml), this will contain the object level diff.
*/
export interface IPatch {
contentsID: string;
path: string;
op: PatchOp;
additions: number;
Expand Down
3 changes: 0 additions & 3 deletions src/sourcer/from.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,10 @@ export type Repository = {
* Represents the decoded patches for the Pull Request. This also includes a mapping from patch IDs to the URL to
* retrieve the file contents.
* @property patchList The list of file patches that are included in this PR.
* @property patchFetchMap A mapping from a URL hash to the URL to fetch the contents for the file. The URL hash is
* the sha256 hash of the URL with a random salt.
*/
export type PullRequestPatches = {
metadata: IChangeSetMetadata;
patchList: IPatch[];
patchFetchMap: Record<string, URL>;
};

// eslint-disable-next-line no-var,@typescript-eslint/no-explicit-any
Expand Down
23 changes: 2 additions & 21 deletions src/sourcer/from_bitbucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ import {
PatchOp,
} from "../engine/patch_types.ts";

import { SourcePlatform, objectParserFromFilename } from "./from.ts";
import { objectParserFromFilename } from "./from.ts";
import type { Repository, PullRequestPatches } from "./from.ts";

type IBBFileDiff = {
unifiedTxt: string;
oFname: string;
oFhash: string;
tFname: string;
tFhash: string;
};

/**
Expand Down Expand Up @@ -59,9 +57,6 @@ export async function patchFromBitBucketPullRequest(
),
},
patchList: [],
// TODO (backward incompatible)
// This will be removed in the next backward incompatible update since it's not implemented
patchFetchMap: {},
};

const prDiffResp = await clt.directAPICall(pullReq.links.diff.href);
Expand Down Expand Up @@ -161,7 +156,6 @@ async function parseBitBucketDiff(
const out: IPatch[] = [];

const diffStartLineRE = /^diff --git a\/.+ b\/.+$/;
const indexLineRE = /^index ([0-9a-f]+)\.\.([0-9a-f]+) \d{6}$/;
const ofnameRE = /^--- ((a\/(.+))|(\/dev\/null))$/;
const tfnameRE = /^\+\+\+ ((b\/(.+))|(\/dev\/null))$/;
let fdiff: IBBFileDiff | null = null;
Expand All @@ -181,20 +175,14 @@ async function parseBitBucketDiff(
fdiff = {
unifiedTxt: line,
oFname: "",
oFhash: "",
tFname: "",
tFhash: "",
};
} else if (fdiff != null) {
fdiff.unifiedTxt += `\n${line}`;

const maybeIndex = indexLineRE.exec(line);
const maybeOFname = ofnameRE.exec(line);
const maybeTFname = tfnameRE.exec(line);
if (maybeIndex) {
fdiff.oFhash = maybeIndex[1];
fdiff.tFhash = maybeIndex[2];
} else if (maybeOFname && maybeOFname[1] !== "/dev/null") {
if (maybeOFname && maybeOFname[1] !== "/dev/null") {
fdiff.oFname = maybeOFname[3];
} else if (maybeTFname && maybeTFname[1] !== "/dev/null") {
fdiff.tFname = maybeTFname[3];
Expand Down Expand Up @@ -229,7 +217,6 @@ async function parseBitBucketFileDiff(
const isRename =
fdiff.oFname && fdiff.tFname && fdiff.oFname !== fdiff.tFname;
if (isRename) {
const fid = `${SourcePlatform.BitBucket}:${fdiff.tFhash}`;
let objectDiff: IObjectDiff | null = null;
if (fdiff.unifiedTxt) {
objectDiff = await getObjectDiff(
Expand All @@ -244,7 +231,6 @@ async function parseBitBucketFileDiff(
}
return [
{
contentsID: fid,
path: fdiff.oFname,
op: PatchOp.Delete,
diff: [],
Expand All @@ -254,7 +240,6 @@ async function parseBitBucketFileDiff(
deletions: 0,
},
{
contentsID: fid,
path: fdiff.tFname,
op: PatchOp.Insert,
diff: [],
Expand All @@ -264,7 +249,6 @@ async function parseBitBucketFileDiff(
deletions: 0,
},
{
contentsID: fid,
path: fdiff.tFname,
op: PatchOp.Modified,
diff,
Expand All @@ -280,11 +264,9 @@ async function parseBitBucketFileDiff(
const isDelete = fdiff.oFname && !fdiff.tFname;
let op = PatchOp.Modified;
let path = fdiff.oFname;
let fhash = fdiff.oFhash;
if (isAdd) {
op = PatchOp.Insert;
path = fdiff.tFname;
fhash = fdiff.tFhash;
} else if (isDelete) {
op = PatchOp.Delete;
}
Expand All @@ -300,7 +282,6 @@ async function parseBitBucketFileDiff(
);
return [
{
contentsID: `${SourcePlatform.BitBucket}:${fhash}`,
path,
op,
diff,
Expand Down
8 changes: 3 additions & 5 deletions src/sourcer/from_github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import { Octokit } from "@octokit/rest";

import { IPatch, PatchOp, LineOp, IObjectDiff } from "../engine/patch_types.ts";

import {
IGitHubRepository,
patchFromGitHubPullRequest,
} from "./from_github.ts";
import { patchFromGitHubPullRequest } from "./from_github.ts";
import type { Repository } from "./from.ts";

const maybeToken = process.env.GITHUB_TOKEN;
let octokit: Octokit;
Expand All @@ -18,7 +16,7 @@ if (maybeToken) {
} else {
octokit = new Octokit();
}
const testRepo: IGitHubRepository = {
const testRepo: Repository = {
owner: "fensak-test",
name: "test-fensak-rules-engine",
};
Expand Down
55 changes: 5 additions & 50 deletions src/sourcer/from_github.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Fensak, LLC.
// SPDX-License-Identifier: AGPL-3.0-or-later OR BUSL-1.1

import * as nodecrypto from "crypto";
import diff from "microdiff";

import { Octokit } from "@octokit/rest";
Expand All @@ -11,19 +10,16 @@ import {
extract as extractFrontMatter,
} from "@fensak-io/front-matter";

import { hexEncode, sha256 } from "../xtd/index.ts";
import { parseUnifiedDiff } from "../engine/patch.ts";
import {
ILinkedPR,
IChangeSetMetadata,
IPatch,
IObjectDiff,
PatchOp,
} from "../engine/patch_types.ts";

import { SourcePlatform, objectParserFromFilename } from "./from.ts";

const crypto = nodecrypto.webcrypto;
import type { Repository, PullRequestPatches } from "./from.ts";
import { objectParserFromFilename } from "./from.ts";

// A type utility to unpack the element type from an array type
// See https://stackoverflow.com/questions/43537520/how-do-i-extract-a-type-from-an-array-in-typescript
Expand All @@ -35,32 +31,6 @@ type PRFile = EleTypeUnpacked<
type PullReq =
Endpoints["GET /repos/{owner}/{repo}/pulls/{pull_number}"]["response"]["data"];

// TODO (backward incompatible)
// Update to use Repository and PullRequestPatches

/**
* Represents a repository hosted on GitHub.
* @property owner The owner of the repository.
* @property name The name of the repository.
*/
export interface IGitHubRepository {
owner: string;
name: string;
}

/**
* Represents the decoded patches for the Pull Request. This also includes a mapping from patch IDs to the URL to
* retrieve the file contents.
* @property patchList The list of file patches that are included in this PR.
* @property patchFetchMap A mapping from a URL hash to the URL to fetch the contents for the file. The URL hash is
* the sha256 hash of the URL with a random salt.
*/
export interface IGitHubPullRequestPatches {
metadata: IChangeSetMetadata;
patchList: IPatch[];
patchFetchMap: Record<string, URL>;
}

/**
* Pull in the changes contained in the Pull Request and create an IPatch array and a mapping from PR file IDs to the
* URL to fetch the contents.
Expand All @@ -71,9 +41,9 @@ export interface IGitHubPullRequestPatches {
*/
export async function patchFromGitHubPullRequest(
clt: Octokit,
repo: IGitHubRepository,
repo: Repository,
prNum: number,
): Promise<IGitHubPullRequestPatches> {
): Promise<PullRequestPatches> {
const { data: pullReq } = await clt.pulls.get({
owner: repo.owner,
repo: repo.name,
Expand All @@ -90,11 +60,7 @@ export async function patchFromGitHubPullRequest(
per_page: 100,
});

const a = new Uint8Array(8);
crypto.getRandomValues(a);
const fetchMapSalt = hexEncode(a);

const out: IGitHubPullRequestPatches = {
const out: PullRequestPatches = {
metadata: {
sourceBranch: pullReq.head.ref,
targetBranch: pullReq.base.ref,
Expand All @@ -107,17 +73,12 @@ export async function patchFromGitHubPullRequest(
),
},
patchList: [],
patchFetchMap: {},
};
for await (const { data: prFiles } of iter) {
for (const f of prFiles) {
const fContentsURL = new URL(f.contents_url);
const fContentsHash = await sha256(`${fetchMapSalt}:${fContentsURL}`);
out.patchFetchMap[fContentsHash] = fContentsURL;
const patches = await getPatchesFromPRFile(
clt,
f,
fContentsHash,
pullReq,
`${repo.owner}/${repo.name}`,
);
Expand All @@ -130,14 +91,11 @@ export async function patchFromGitHubPullRequest(
async function getPatchesFromPRFile(
clt: Octokit,
f: PRFile,
fContentsHash: string,
pullReq: PullReq,

// The following is only needed for error messaging
repoName: string,
): Promise<IPatch[]> {
const fid = `${SourcePlatform.GitHub}:${fContentsHash}`;

let op = PatchOp.Unknown;
switch (f.status) {
// This should never happen, so we throw an error
Expand All @@ -156,7 +114,6 @@ async function getPatchesFromPRFile(
}
return [
{
contentsID: fid,
path: f.previous_filename,
op: PatchOp.Delete,
// TODO: this requires pulling down the file contents
Expand All @@ -166,7 +123,6 @@ async function getPatchesFromPRFile(
objectDiff: null,
},
{
contentsID: fid,
path: f.filename,
op: PatchOp.Insert,
// TODO: this requires pulling down the file contents
Expand Down Expand Up @@ -194,7 +150,6 @@ async function getPatchesFromPRFile(

return [
{
contentsID: fid,
path: f.filename,
op: op,
additions: f.additions,
Expand Down

0 comments on commit 64e37fc

Please sign in to comment.