Merge pull request #50 from pulsar-edit/refactor-git

Laying the groundwork for the `git` refactor

codeql-config.yml

@@ -7,4 +7,5 @@ queries:
 paths-ignore:
   - ./src/tests
   - ./src/tests_integration
+  - ./src/vcs_providers_tests
 # https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors

docs/reference/structures.md

@@ -0,0 +1,46 @@
+This document aims to take some of the mystery out of objects used within the backend.
+
+Since a lot of what's done on the backend is mutating object structures or working with them, it can be helpful to now document what structures some important functions expect, or will return.
+
+## `database.insertNewPackageVersion()`
+
+This function expects it's first argument to be one of `packJSON` and expects the following format:
+
+* Essentially this object should be the full `package.json` of the version we want.
+* There are some values that are required, and used by the database function:
+  - `.name`: Expected to be the `package.json` `name` key
+  - `.license`: Expected to contain the license of the package. If none is provided it defaults to `defaultLicense`
+  - `.engines`: Expected to contain the packages engines field. If none is provided it defaults to `defaultEngine`
+  - `.version`: Required, no default provided. If this key doesn't exist the database call *will* fail
+* There are other values that are required that the database function *doesn't* use, but will be used later on.
+  - `.sha`: This should be the SHA value of the tarball download for this version.
+  - `.tarball_url`: This should be the GitHub (Or other VCS service) URL to download this versions code.
+  - `.dist.tarball`: This value is not required. It is injected when the packages version data is returned. Migrated packages will already have this key, but will be pointing to an `atom.io` URL, and will need to be overridden when returning the package data.
+
+A full example is below:
+
+```json
+{
+  "sha": "4fd4a4942dc0136c982277cdd59351bb71eb776d",
+  "dist": {
+    "tarball": "https://www.atom.io/api/packages/codelink-tools/versions/0.14.0/tarball"
+  },
+  "main": "./lib/codelink-tools",
+  "name": "codelink-tools",
+  "version": "0.14.0",
+  "keywords": [],
+  "repository": "https://github.com/j-woudenberg/codelink-tools",
+  "description": "An Atom package for CodeLink that includes a compiler, debugger, and a solution explorer",
+  "tarball_url": "https://api.github.com/repos/j-woudenberg/codelink-tools/tarball/refs/tags/v0.14.0",
+  "dependencies": {},
+  "deserializers": {
+    "codelink-tools/Parser": "deserializeParser"
+  },
+  "activationHooks": [
+    "source.codelink:root-scope-used"
+  ],
+  "activationCommands": {
+    "atom-workspace": "codelink-tools:toggle"
+  }
+}
+```

docs/resources/jsdoc_typedef.js

@@ -0,0 +1,53 @@
+/**
+* This Document is intended to house all `typedef` comments used within
+* the codebase.
+* It's existance and location serves two purposes:
+*   - Having this located within docs lets it be properly discoverable as
+*     documentation about the object structures we use.
+*   - But having it as a JavaScript file means it can be included into our
+*     JSDoc Source Code Documentation - ../reference/Source_Documentation.md
+*/
+
+/**
+  * The Generic Object that should be returned by nearly every function
+  * within every module. Allows ease of bubbling errors to the HTTP Handler.
+  * @see {@link docs/reference/bubbled_errors.md}
+ * @typedef {object} ServerStatusObject
+ * @property {boolean} ok - Indicates if the overall function was successful.
+ * @property {*} content - The returned data of the request. Can be anything.
+ * @property {string} [short] - Only included if `ok` is false. Includes a
+ * generic reason the request failed.
+ */
+
+// =============================================================================
+// ========================= VCS ===============================================
+// =============================================================================
+
+/**
+ * The Server Status Object returned by `vcs.newVersionData()` containing all
+ * the data needed to update a packages version.
+ * @typedef {object} SSO_VCS_newVersionData
+ * @property {boolean} ok - Indicates if the overall function was successful.
+ * @property {string} [short] - Only included if `ok: false`. Includes the generic
+ * reason the request failed.
+ * @property {string|object} content - When `ok: false` returns a string error
+ * but when `ok: true` returns an object further documented below.
+ * @property {string} content.name - The Lowercase string of the packages name.
+ * As taken from the `package.json` content at it's remote repository.
+ * @property {object} content.repository - The returned repository object as
+ * returned by `vcs.determineProvider()` when passed the remote `package.json`s
+ * `repository` key.
+ * @property {string} content.repository.type - A string representing the service
+ * vcs name of where the repo is located. One of the valid types returned by
+ * `vcs.determineProvider()`
+ * @property {string} content.repository.url - A String URL of where the remote
+ * repository is located.
+ * @property {string} content.readme - The Text based readme of the package, as
+ * received from it's remote repository.
+ * @property {object} content.metadata - Largely made up of the remote `package.json`
+ * Where it will include all fields as found in the remote file. While additionally
+ * adding a few others which will be documented here.
+ * @property {string} content.metadata.tarball_url - The URL of the tarball download
+ * for the newest tag published for the package.
+ * @property {string} content.metadata.sha - The SHA hash of the `tarball_url`
+ */

jest.config.js

@@ -11,12 +11,19 @@ const config = {
       globalSetup: "<rootDir>/node_modules/@databases/pg-test/jest/globalSetup",
       globalTeardown:
         "<rootDir>/node_modules/@databases/pg-test/jest/globalTeardown",
-      testMatch: ["<rootDir>/src/tests_integration/*.test.js"],
+      testMatch: ["<rootDir>/src/tests_integration/main.test.js"],
     },
     {
       displayName: "Unit-Tests",
       testMatch: ["<rootDir>/src/tests/*.test.js"],
     },
+    {
+      displayName: "VCS-Tests",
+      testMatch: [
+        "<rootDir>/src/vcs_providers_tests/**/*.test.js",
+        "<rootDir>/src/vcs_providers_tests/*.test.js"
+      ],
+    }
   ],
 };
 

package.json

@@ -12,10 +12,11 @@
     "test:integration": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Integration-Tests",
     "start:dev": "cross-env PULSAR_STATUS=dev node ./src/dev_server.js",
     "test": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest",
+    "test:vcs": "cross-env NODE_ENV=test PULSAR_STATUS=dev MOCK_GH=false jest --selectProjects VCS-Tests",
     "api-docs": "quick-webserver-docs -i ./src/main.js -o ./docs/reference/API_Definition.md",
     "lint": "prettier --check -u -w .",
     "complex": "cr --newmi --config .complexrc .",
-    "js-docs": "jsdoc2md -c ./jsdoc.conf.js ./src/*.js ./src/handlers/*.js > ./docs/reference/Source_Documentation.md",
+    "js-docs": "jsdoc2md -c ./jsdoc.conf.js ./src/*.js ./src/handlers/*.js ./docs/resources/jsdoc_typedef.js > ./docs/reference/Source_Documentation.md",
     "contributors:add": "all-contributors add",
     "test_search": "node ./scripts/tools/search.js",
     "migrations": "pg-migrations apply --directory ./src/dev-runner/migrations"

src/handlers/package_handler.js

@@ -13,7 +13,7 @@
 
 const common = require("./common_handler.js");
 const query = require("../query.js");
-const git = require("../git.js");
+const vcs = require("../vcs.js");
 const logger = require("../logger.js");
 const { server_url } = require("../config.js").getConfig();
 const utils = require("../utils.js");
@@ -172,7 +172,7 @@ async function postPackages(req, res) {
   }
 
   // Now we know the package doesn't exist. And we want to check that the user owns this repo on git.
-  const gitowner = await git.ownership(user.content, params.repository);
+  const gitowner = await vcs.ownership(user.content, params.repository);
 
   if (!gitowner.ok) {
     logger.generic(3, `postPackages-ownership Not OK: ${gitowner.content}`);
@@ -181,7 +181,8 @@ async function postPackages(req, res) {
   }
 
   // Now knowing they own the git repo, and it doesn't exist here, lets publish.
-  const newPack = await git.createPackage(params.repository, user.content);
+  // TODO: Stop hardcoding `git` as service
+  const newPack = await vcs.newPackageData(user.content, params.repository, "git");
 
   if (!newPack.ok) {
     logger.generic(3, `postPackages-createPackage Not OK: ${newPack.content}`);
@@ -424,19 +425,9 @@ async function deletePackagesName(req, res) {
     return;
   }
 
-  const packMetadata = packageExists.content?.versions[0]?.meta;
-
-  if (packMetadata === null) {
-    await common.handleError(req, res, {
-      ok: false,
-      short: "Not Found",
-      content: `Cannot retrieve metadata for ${params.packageName} package`,
-    });
-  }
-
-  const gitowner = await git.ownership(
+  const gitowner = await vcs.ownership(
     user.content,
-    utils.getOwnerRepoFromPackage(packMetadata)
+    packageExists.content
   );
 
   if (!gitowner.ok) {
@@ -639,74 +630,74 @@ async function postPackagesVersion(req, res) {
     return;
   }
 
-  const meta = packExists.content?.versions[0]?.meta;
-
-  if (meta === null) {
-    await common.handleError(req, res, {
-      ok: false,
-      short: "Not Found",
-      content: `Cannot retrieve metadata for ${params.packageName} package`,
-    });
-  }
-
   // Get `owner/repo` string format from package.
-  let ownerRepo = utils.getOwnerRepoFromPackage(meta);
+  let ownerRepo = utils.getOwnerRepoFromPackage(packExists.content.data);
 
-  // Now it's important to note, that getPackageJSON was intended to be an internal function.
-  // As such does not return a Server Status Object. This may change later, but for now,
-  // we will expect `undefined` to not be success.
-  const packJSON = await git.getPackageJSON(ownerRepo, user.content);
+  // Using our new VCS Service
+  // TODO: The "git" Service shouldn't always be hardcoded.
+  let packMetadata = await vcs.newVersionData(user.content, ownerRepo, "git");
 
-  if (packJSON === undefined) {
-    logger.generic(6, `Unable to get Package JSON from git with: ${ownerRepo}`);
-    await common.handleError(req, res, {
-      ok: false,
-      short: "Bad Package",
-      content: `Failed to get Package JSON: ${ownerRepo}`,
-    });
+  if (!packMetadata.ok) {
+    logger.generic(6, packMetadata.content);
+    await common.handleError(req, res, packMetadata);
     return;
   }
+  // Now it's important to note, that getPackageJSON was intended to be an internal function.
+  // As such does not return a Server Status Object. This may change later, but for now,
+  // we will expect `undefined` to not be success.
+  //const packJSON = await git.getPackageJSON(ownerRepo, user.content);
+
+  //if (packJSON === undefined) {
+  //  logger.generic(6, `Unable to get Package JSON from git with: ${ownerRepo}`);
+  //  await common.handleError(req, res, {
+  //    ok: false,
+  //    short: "Bad Package",
+  //    content: `Failed to get Package JSON: ${ownerRepo}`,
+  //  });
+  //  return;
+  //}
 
   // Now we will also need to get the packages data to update on the db
   // during version pushes.
 
-  const packReadme = await git.getRepoReadMe(ownerRepo, user.content);
+  //const packReadme = await git.getRepoReadMe(ownerRepo, user.content);
   // Again important to note, this was intended as an internal function of git
   // As such does not return a Server Status Object, and instead returns the obj or null
-  if (packReadme === undefined) {
-    logger.generic(
-      6,
-      `Unable to Get Package Readme from git with: ${ownerRepo}`
-    );
-    await common.handleError(req, res, {
-      ok: false,
-      short: "Bad Package",
-      content: `Failed to get Package Readme: ${ownerRepo}`,
-    });
-  }
-
-  const packMetadata = await git.metadataAppendTarballInfo(
-    packJSON,
-    packJSON.version,
-    user.content
-  );
-  if (packMetadata === undefined) {
-    await common.handleError(req, res, {
-      ok: false,
-      short: "Bad Package",
-      content: `Failed to get Package metadata info: ${ownerRepo}`,
-    });
-  }
+  //if (packReadme === undefined) {
+  //  logger.generic(
+  //    6,
+  //    `Unable to Get Package Readme from git with: ${ownerRepo}`
+  //  );
+  //  await common.handleError(req, res, {
+  //    ok: false,
+  //    short: "Bad Package",
+  //    content: `Failed to get Package Readme: ${ownerRepo}`,
+  //  });
+  //}
+
+  //const packMetadata = await git.metadataAppendTarballInfo(
+  //  packJSON,
+  //  packJSON.version,
+  //  user.content
+  //);
+  //if (packMetadata === undefined) {
+  //  await common.handleError(req, res, {
+  //    ok: false,
+  //    short: "Bad Package",
+  //    content: `Failed to get Package metadata info: ${ownerRepo}`,
+  //  });
+  //}
 
   // Now construct the object that will be used to update the `data` column.
-  const packageData = {
-    name: packMetadata.name.toLowerCase(),
-    repository: git.selectPackageRepository(packMetadata.repository),
-    readme: packReadme,
-    metadata: packMetadata,
-  };
+  //const packageData = {
+  //  name: packMetadata.name.toLowerCase(),
+  //  repository: git.selectPackageRepository(packMetadata.repository),
+  //  readme: packReadme,
+  //  metadata: packMetadata,
+  //};
+
+  const newName = packMetadata.content.name;
 
-  const newName = packageData.name;
   const currentName = packExists.content.name;
   if (newName !== currentName && !params.rename) {
     logger.generic(
@@ -729,9 +720,10 @@ async function postPackagesVersion(req, res) {
   // From the newest updated `package.json` info, just in case it's changed that will be
   // supported here
 
-  ownerRepo = utils.getOwnerRepoFromPackage(packJSON);
+  ownerRepo = utils.getOwnerRepoFromPackage(packMetadata.content.metadata);
 
-  const gitowner = await git.ownership(user.content, ownerRepo);
+  //const gitowner = await git.ownership(user.content, ownerRepo);
+  const gitowner = await vcs.ownership(user.content, packExists.content);
 
   if (!gitowner.ok) {
     logger.generic(6, `User Failed Git Ownership Check: ${gitowner.content}`);
@@ -784,7 +776,7 @@ async function postPackagesVersion(req, res) {
   // Now add the new Version key.
 
   const addVer = await database.insertNewPackageVersion(
-    packageData,
+    packMetadata.content.metadata,
     rename ? currentName : null
   );
 
@@ -794,6 +786,8 @@ async function postPackagesVersion(req, res) {
     return;
   }
 
+  // TODO: Additionally update things like the readme on the package here
+
   res.status(201).json(addVer.content);
   logger.httpLog(req, res);
 }
@@ -980,19 +974,23 @@ async function deletePackageVersion(req, res) {
     return;
   }
 
-  const packMetadata = packageExists.content?.versions[0]?.meta;
-
-  if (packMetadata === null) {
-    await common.handleError(req, res, {
-      ok: false,
-      short: "Not Found",
-      content: `Cannot retrieve metadata for ${params.packageName} package`,
-    });
-  }
-
-  const gitowner = await git.ownership(
+  //const packMetadata = packageExists.content?.versions[0]?.meta;
+
+  //if (packMetadata === null) {
+  //  await common.handleError(req, res, {
+  //    ok: false,
+  //    short: "Not Found",
+  //    content: `Cannot retrieve metadata for ${params.packageName} package`,
+  //  });
+  //}
+
+  //const gitowner = await git.ownership(
+  //  user.content,
+  //  utils.getOwnerRepoFromPackage(packMetadata)
+  //);
+  const gitowner = await vcs.ownership(
     user.content,
-    utils.getOwnerRepoFromPackage(packMetadata)
+    packageExists.content
   );
 
   if (!gitowner.ok) {