From 665db931f91590f201aa79f0e8be1be716824b3e Mon Sep 17 00:00:00 2001 From: Ashton Eby Date: Tue, 29 Oct 2024 12:18:17 -0700 Subject: [PATCH] address PR feedback - support colors on all calls to /validate and /status - fix schema push --active flag description - fix schema status confirmation prompts --- src/commands/schema/abandon.mjs | 9 ++---- src/commands/schema/commit.mjs | 9 ++---- src/commands/schema/diff.mjs | 6 ++-- src/commands/schema/pull.mjs | 11 ++----- src/commands/schema/push.mjs | 21 +++++++------ src/commands/schema/status.mjs | 7 ++--- src/lib/db.mjs | 17 +++++----- src/lib/schema.mjs | 7 ++--- test/general-cli.mjs | 12 ++++++- test/schema/pull.mjs | 5 ++- test/schema/push.mjs | 56 +++++++++++++++++++++++++++++++-- test/schema/status.mjs | 12 +++---- 12 files changed, 111 insertions(+), 61 deletions(-) diff --git a/src/commands/schema/abandon.mjs b/src/commands/schema/abandon.mjs index f6e3ddac..52df7175 100644 --- a/src/commands/schema/abandon.mjs +++ b/src/commands/schema/abandon.mjs @@ -14,9 +14,8 @@ async function doAbandon(argv) { }); await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: `/schema/1/staged/abandon?${params}`, - secret: argv.secret, method: "POST", }); logger.stdout("Schema has been abandoned"); @@ -26,9 +25,8 @@ async function doAbandon(argv) { if (argv.color) params.set("color", "ansi"); const response = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: `/schema/1/staged/status?${params}`, - secret: argv.secret, method: "GET", }); @@ -46,9 +44,8 @@ async function doAbandon(argv) { const params = new URLSearchParams({ version: response.version }); await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: `/schema/1/staged/abandon?${params}`, - secret: argv.secret, method: "POST", }); diff --git a/src/commands/schema/commit.mjs b/src/commands/schema/commit.mjs index 66d60e42..ae68b0c0 100644 --- a/src/commands/schema/commit.mjs +++ b/src/commands/schema/commit.mjs @@ -14,9 +14,8 @@ async function doCommit(argv) { }); await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: `/schema/1/staged/commit?${params}`, - secret: argv.secret, method: "POST", }); @@ -27,9 +26,8 @@ async function doCommit(argv) { if (argv.color) params.set("color", "ansi"); const response = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: `/schema/1/staged/status?${params}`, - secret: argv.secret, method: "GET", }); @@ -50,9 +48,8 @@ async function doCommit(argv) { const params = new URLSearchParams({ version: response.version }); await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: `/schema/1/staged/commit?${params}`, - secret: argv.secret, method: "POST", }); diff --git a/src/commands/schema/diff.mjs b/src/commands/schema/diff.mjs index 805cf264..534ea7ba 100644 --- a/src/commands/schema/diff.mjs +++ b/src/commands/schema/diff.mjs @@ -42,10 +42,9 @@ async function doDiff(argv) { if (target === "staged") params.set("diff", diffKind); const { version, status, diff } = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/staged/status", params, - secret: argv.secret, method: "GET", }); @@ -71,10 +70,9 @@ async function doDiff(argv) { } const { diff } = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/validate", params, - secret: argv.secret, body: files, method: "POST", }); diff --git a/src/commands/schema/pull.mjs b/src/commands/schema/pull.mjs index 48886867..cec54704 100644 --- a/src/commands/schema/pull.mjs +++ b/src/commands/schema/pull.mjs @@ -11,19 +11,17 @@ async function doPull(argv) { // fetch the list of remote FSL files const filesResponse = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/files", method: "GET", - secret: argv.secret, }); // check if there's a staged schema const statusResponse = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/staged/status", params: new URLSearchParams({ version: filesResponse.version }), method: "GET", - secret: argv.secret, }); // if there's a staged schema, cannot use the --active flag. @@ -88,10 +86,7 @@ async function doPull(argv) { const getAllSchemaFileContents = container.resolve( "getAllSchemaFileContents", ); - const contents = await getAllSchemaFileContents(filenames, { - secret: argv.secret, - baseUrl: argv.url, - }); + const contents = await getAllSchemaFileContents(filenames, argv); // don't start writing or deleting files until we've successfully fetched all // the remote schema files diff --git a/src/commands/schema/push.mjs b/src/commands/schema/push.mjs index d76f4b87..9445e3a4 100644 --- a/src/commands/schema/push.mjs +++ b/src/commands/schema/push.mjs @@ -11,6 +11,8 @@ async function doPush(argv) { const gatherFSL = container.resolve("gatherFSL"); const fsl = reformatFSL(await gatherFSL(argv.dir)); + const isStagedPush = !argv.active; + if (!argv.input) { const params = new URLSearchParams({ force: "true", @@ -18,11 +20,10 @@ async function doPush(argv) { }); await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/update", params, body: fsl, - secret: argv.secret, method: "POST", }); } else { @@ -35,21 +36,24 @@ async function doPush(argv) { if (argv.color) params.set("color", "ansi"); const response = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/validate", params, body: fsl, - secret: argv.secret, method: "POST", }); - let message = "Accept and push changes?"; + let message = isStagedPush + ? "Stage the above changes?" + : "Push the above changes?"; if (response.diff) { logger.stdout(`Proposed diff:\n`); logger.stdout(response.diff); } else { logger.stdout("No logical changes."); - message = "Push file contents anyway?"; + message = isStagedPush + ? "Stage the file contents anyway?" + : "Push the file contents anyway?"; } const confirm = container.resolve("confirm"); const confirmed = await confirm({ @@ -64,11 +68,10 @@ async function doPush(argv) { }); await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/update", params, body: fsl, - secret: argv.secret, method: "POST", }); } else { @@ -87,7 +90,7 @@ function buildPushCommand(yargs) { }, active: { description: - "Stages the schema change instead of applying it immediately", + "Immediately applies the schema change instead of staging it", type: "boolean", default: false, }, diff --git a/src/commands/schema/status.mjs b/src/commands/schema/status.mjs index ad4c6f78..7211c7c6 100644 --- a/src/commands/schema/status.mjs +++ b/src/commands/schema/status.mjs @@ -16,10 +16,9 @@ async function doStatus(argv) { const fsl = reformatFSL(await gatherFSL(argv.dir)); const statusResponse = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/staged/status", params, - secret: argv.secret, method: "GET", }); @@ -28,11 +27,11 @@ async function doStatus(argv) { staged: "true", version: statusResponse.version, }); + if (argv.color) params.set("color", "ansi"); const validationResponse = await makeFaunaRequest({ - baseUrl: argv.url, + argv, path: "/schema/1/validate", params, - secret: argv.secret, method: "POST", body: fsl, }); diff --git a/src/lib/db.mjs b/src/lib/db.mjs index 62b8cf60..75d30ae6 100644 --- a/src/lib/db.mjs +++ b/src/lib/db.mjs @@ -8,10 +8,9 @@ import { container } from "../cli.mjs"; /** * @typedef {Object} fetchParameters - * @property {string} secret - The secret to include in the AUTHORIZATION header of the request. - * @property {string} baseUrl - The base URL from the scheme up through the top level domain and optional port; defaults to "https://db.fauna.com:443". + * @property {Object} argv - The parsed argv from yargs; used to find the base url (`argv.url`), secret (`argv.secret`), and color support (`argv.color`). To overwrite, provided a modified argv to `makeFaunaRequest`. * @property {string} path - The path part of the URL. Added to the baseUrl and params to build the full URL. - * @property {URLSearchParams} [params] - The parameters (and their values) to set in the query string. + * @property {URLSearchParams|undefined} [params] - The parameters (and their values) to set in the query string. * @property {method} method - The HTTP method to use when making the request. * @property {object} [body] - The body to include in the request. * @property {boolean} [shouldThrow=true] - Whether or not to throw if the network request succeeds but is not a 2XX. If this is set to false, makeFaunaRequest will return the error instead of throwing. @@ -21,8 +20,7 @@ import { container } from "../cli.mjs"; * @param {fetchParameters} args */ export async function makeFaunaRequest({ - secret, - baseUrl, + argv, path, params = undefined, method, @@ -30,14 +28,17 @@ export async function makeFaunaRequest({ shouldThrow = true, }) { const fetch = container.resolve("fetch"); + const routesWithColor = ["/schema/1/staged/status", "/schema/1/validate"]; + if (params && argv.color && routesWithColor.includes(path)) + params.set("color", "ansi"); if (params && params.sort) params.sort(); const paramsString = params && params.size ? `?${params.toString()}` : ""; let fullUrl; try { - fullUrl = new URL(`${path}${paramsString}`, baseUrl).href; + fullUrl = new URL(`${path}${paramsString}`, argv.url).href; } catch (e) { - e.message = `Could not build valid URL out of base url (${baseUrl}), path (${path}), and params string (${paramsString}) built from params (${JSON.stringify( + e.message = `Could not build valid URL out of base url (${argv.url}), path (${path}), and params string (${paramsString}) built from params (${JSON.stringify( params, )}).`; throw e; @@ -45,7 +46,7 @@ export async function makeFaunaRequest({ const fetchArgs = { method, - headers: { AUTHORIZATION: `Bearer ${secret}` }, + headers: { AUTHORIZATION: `Bearer ${argv.secret}` }, }; if (body) fetchArgs.body = body; diff --git a/src/lib/schema.mjs b/src/lib/schema.mjs index 37619820..87b719c4 100644 --- a/src/lib/schema.mjs +++ b/src/lib/schema.mjs @@ -166,18 +166,17 @@ export async function writeSchemaFiles(dir, filenameToContentsDict) { /** * @param {string[]} filenames - A list of schema file names to fetch - * @param {Omit} overrides + * @param {object} argv * @returns {Promise>} A map of schema file names to their contents. */ -export async function getAllSchemaFileContents(filenames, { ...overrides }) { +export async function getAllSchemaFileContents(filenames, argv) { const promises = []; /** @type Record */ const fileContentCollection = {}; for (const filename of filenames) { promises.push( makeFaunaRequest({ - baseUrl: overrides.baseUrl, - secret: overrides.secret, + argv, path: `/schema/1/files/${encodeURIComponent(filename)}`, method: "GET", }).then(({ content }) => { diff --git a/test/general-cli.mjs b/test/general-cli.mjs index 82b2b041..363f3b86 100644 --- a/test/general-cli.mjs +++ b/test/general-cli.mjs @@ -7,9 +7,9 @@ import { expect } from "chai"; import chalk from "chalk"; import { stub } from "sinon"; -import { f } from "../helpers.mjs"; import { builtYargs, run } from "../src/cli.mjs"; import { setupTestContainer as setupContainer } from "../src/config/setup-test-container.mjs"; +import { f } from "./helpers.mjs"; describe("cli operations", function () { let container; @@ -84,7 +84,17 @@ describe("cli operations", function () { }); it("should check for updates when run", async function () { + const fetch = container.resolve("fetch"); fetch.onCall(0).resolves( + f({ + version: 0, + status: "none", + diff: "Staged schema: none", + pending_summary: "", + text_diff: "", + }), + ); + fetch.onCall(1).resolves( f({ version: 0, diff: "", diff --git a/test/schema/pull.mjs b/test/schema/pull.mjs index 5a1a93a3..526d54ca 100644 --- a/test/schema/pull.mjs +++ b/test/schema/pull.mjs @@ -101,7 +101,10 @@ describe("schema pull", function () { ); // the version param in the URL is important - we use it for optimistic locking expect(fetch).to.have.been.calledWith( - buildUrl("/schema/1/staged/status", { version: "194838274939473" }), + buildUrl("/schema/1/staged/status", { + version: "194838274939473", + color: "ansi", + }), commonFetchParams, ); expect(fetch).to.have.been.calledWith( diff --git a/test/schema/push.mjs b/test/schema/push.mjs index f225c077..3dc63d9b 100644 --- a/test/schema/push.mjs +++ b/test/schema/push.mjs @@ -95,6 +95,9 @@ describe("schema push", function () { expect(logger.stderr).to.not.be.called; expect(logger.stdout).to.have.been.calledWith("Proposed diff:\n"); + expect(confirm).to.have.been.calledWith( + sinon.match.has("message", "Stage the above changes?"), + ); expect(logger.stdout).to.have.been.calledWith(diffString); }); @@ -146,6 +149,9 @@ describe("schema push", function () { expect(logger.stderr).to.not.be.called; expect(logger.stdout).to.have.been.calledWith("Proposed diff:\n"); + expect(confirm).to.have.been.calledWith( + sinon.match.has("message", "Push the above changes?"), + ); expect(logger.stdout).to.have.been.calledWith(diffString); }); @@ -192,7 +198,7 @@ describe("schema push", function () { expect(gatherFSL).to.have.been.calledWith("/absolute/path/elsewhere"); }); - it("warns when attempting to push an empty diff", async function () { + it("warns when attempting to stage an empty diff", async function () { // user accepts the changes in the interactive prompt confirm.resolves(true); @@ -234,7 +240,53 @@ describe("schema push", function () { expect(logger.stderr).to.not.be.called; expect(logger.stdout).to.have.been.calledWith("No logical changes."); expect(confirm).to.have.been.calledWith( - sinon.match.has("message", "Push file contents anyway?"), + sinon.match.has("message", "Stage the file contents anyway?"), + ); + }); + + it("warns when attempting to push an empty diff", async function () { + // user accepts the changes in the interactive prompt + confirm.resolves(true); + + fetch.onCall(0).resolves( + f({ + // this is the version we provide when we mutate the resource + version: 1728675598430000, + // note: no diff + }), + ); + + await run(`schema push --secret "secret" --active`, container); + + expect(fetch).to.have.been.calledWith( + buildUrl("/schema/1/validate", { + force: "true", + staged: "false", + color: "ansi", + }), + { + method: "POST", + headers: { AUTHORIZATION: "Bearer secret" }, + body: reformatFSL(fsl), + }, + ); + + expect(fetch).to.have.been.calledWith( + buildUrl("/schema/1/update", { + version: "1728675598430000", + staged: "false", + }), + { + method: "POST", + headers: { AUTHORIZATION: "Bearer secret" }, + body: reformatFSL(fsl), + }, + ); + + expect(logger.stderr).to.not.be.called; + expect(logger.stdout).to.have.been.calledWith("No logical changes."); + expect(confirm).to.have.been.calledWith( + sinon.match.has("message", "Push the file contents anyway?"), ); }); diff --git a/test/schema/status.mjs b/test/schema/status.mjs index 9048f63a..cf60c878 100644 --- a/test/schema/status.mjs +++ b/test/schema/status.mjs @@ -96,6 +96,7 @@ describe("schema status", function () { diff: "summary", staged: "true", version: "0", + color: "ansi", }), { ...commonFetchParams, method: "POST", body: new FormData() }, ); @@ -138,6 +139,7 @@ describe("schema status", function () { diff: "summary", staged: "true", version: "0", + color: "ansi", }), { ...commonFetchParams, method: "POST", body: new FormData() }, ); @@ -186,14 +188,7 @@ describe("schema status", function () { diff: "summary", staged: "true", version: "0", - }), - { ...commonFetchParams, method: "POST", body: new FormData() }, - ); - expect(fetch).to.have.been.calledWith( - buildUrl("/schema/1/validate", { - diff: "summary", - staged: "true", - version: "0", + color: "ansi", }), { ...commonFetchParams, method: "POST", body: new FormData() }, ); @@ -239,6 +234,7 @@ describe("schema status", function () { diff: "summary", staged: "true", version: "0", + color: "ansi", }), { ...commonFetchParams, method: "POST", body: new FormData() }, );