Skip to content

Commit

Permalink
[FIX] ui5 config: Allow usage of all Configuration options (#645)
Browse files Browse the repository at this point in the history 
Options are now properly read from the @ui5/project Configuration class
in order to be decoupled from future changes.

Some code could be removed as the checks are done via yargs.

See: SAP/ui5-project#642

JIRA: CPOUI5FOUNDATION-716

---------

Co-authored-by: Merlin Beutlberger <[email protected]>
  • Loading branch information
@matz3 @RandomByte
matz3 and RandomByte authored Aug 19, 2023
1 parent 4ca30c3 commit 78e032e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 63 deletions.
63 changes: 32 additions & 31 deletions lib/cli/commands/config.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import chalk from "chalk";
import process from "node:process";
import baseMiddleware from "../middlewares/base.js";
import Configuration from "@ui5/project/config/Configuration";

const configCommand = {
command: "config",
Expand All @@ -12,13 +13,17 @@ const configCommand = {
configCommand.builder = function(cli) {
return cli
.demandCommand(1, "Command required. Available commands are 'set', 'get', and 'list'")
.command("set <key> [value]", "Set the value for a given configuration key. " +
.command("set <option> [value]", "Set the value for a given configuration option. " +
"Clear an existing configuration by omitting the value", {
handler: handleConfig,
builder: noop,
builder: (cli) => {
cli.positional("option", {
choices: Configuration.OPTIONS
});
},
middlewares: [baseMiddleware],
})
.command("get <key>", "Get the value for a given configuration key", {
.command("get <option>", "Get the value for a given configuration option", {
handler: handleConfig,
builder: noop,
middlewares: [baseMiddleware],
Expand All @@ -28,51 +33,47 @@ configCommand.builder = function(cli) {
builder: noop,
middlewares: [baseMiddleware],
})
.example("$0 config set mavenSnapshotEndpointUrl http://example.com/snapshots/",
"Set a value for the mavenSnapshotEndpointUrl configuration")
.example("$0 config set mavenSnapshotEndpointUrl",
"Unset the current value of the mavenSnapshotEndpointUrl configuration");
.example("$0 config set ui5DataDir /path/to/.ui5",
"Set a value for the ui5DataDir configuration")
.example("$0 config set ui5DataDir",
"Unset the current value of the ui5DataDir configuration");
};

function noop() {}

async function handleConfig(argv) {
const {_: commandArgs, key, value} = argv;
const {_: commandArgs, option, value} = argv;
const command = commandArgs[commandArgs.length - 1];

const {default: Configuration} = await import( "@ui5/project/config/Configuration");
const allowedKeys = ["mavenSnapshotEndpointUrl"];

if (["set", "get"].includes(command) && !allowedKeys.includes(key)) {
throw new Error(
`The provided key is not a valid configuration option. Valid options are: ${allowedKeys.join(", ")}`);
}
// Yargs ensures that:
// - "option" only contains valid values (defined as "choices" in command builder)
// - "command" is one of "list", "get", "set"

const config = await Configuration.fromFile();
if (command === "list") {
let jsonConfig;

switch (command) {
case "list":
// Print all configuration values to stdout
process.stdout.write(formatJsonForOutput(config.toJson()));
} else if (command === "get") {
break;
case "get":
// Get a single configuration value and print to stdout
let configValue = config.toJson()[key];
if (configValue === undefined) {
configValue = "";
}
process.stdout.write(`${configValue}\n`);
} else if (command === "set") {
const jsonConfig = config.toJson();
process.stdout.write(`${config.toJson()[option] ?? ""}\n`);
break;
case "set":
jsonConfig = config.toJson();
if (value === undefined || value === "") {
delete jsonConfig[key];
process.stderr.write(`Configuration option ${chalk.bold(key)} has been unset\n`);
delete jsonConfig[option];
process.stderr.write(`Configuration option ${chalk.bold(option)} has been unset\n`);
} else {
jsonConfig[key] = value;
process.stderr.write(`Configuration option ${chalk.bold(key)} has been updated:
${formatJsonForOutput(jsonConfig, key)}`);
jsonConfig[option] = value;
process.stderr.write(`Configuration option ${chalk.bold(option)} has been updated:
${formatJsonForOutput(jsonConfig, option)}`);
}

await Configuration.toFile(new Configuration(jsonConfig));
} else {
throw new Error(`Unknown 'ui5 config' command '${command}'`);
break;
}
}

Expand Down
67 changes: 35 additions & 32 deletions test/lib/cli/commands/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ import test from "ava";
import sinon from "sinon";
import esmock from "esmock";
import chalk from "chalk";
import path from "node:path";
import {fileURLToPath} from "node:url";
import {execa} from "execa";
import stripAnsi from "strip-ansi";

const __dirname = path.dirname(fileURLToPath(import.meta.url));
const ui5Cli = path.join(__dirname, "..", "..", "..", "..", "bin", "ui5.cjs");
const ui5 = (args, options = {}) => execa(ui5Cli, args, options);

function getDefaultArgv() {
// This has been taken from the actual argv object yargs provides
Expand Down Expand Up @@ -93,7 +101,7 @@ test.serial("ui5 config get", async (t) => {
}));

argv["_"] = ["get"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["option"] = "mavenSnapshotEndpointUrl";
await config.handler(argv);

t.is(stdoutWriteStub.firstCall.firstArg, "my/url\n",
Expand All @@ -105,7 +113,7 @@ test.serial("ui5 config get: Empty value", async (t) => {
const {config, argv, stdoutWriteStub} = t.context;

argv["_"] = ["get"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["option"] = "mavenSnapshotEndpointUrl";
await config.handler(argv);

t.is(stdoutWriteStub.firstCall.firstArg, "\n", "Logged no value to console");
Expand All @@ -122,7 +130,7 @@ test.serial("ui5 config set", async (t) => {
Configuration.fromFile.resolves(configurationStub);

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["option"] = "mavenSnapshotEndpointUrl";
argv["value"] = "https://_snapshot_endpoint_";
await config.handler(argv);

Expand All @@ -141,7 +149,7 @@ test.serial("ui5 config set without a value should delete the configuration", as
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["option"] = "mavenSnapshotEndpointUrl";
argv["value"] = undefined; // Simulating no value parameter provided to Yargs
await config.handler(argv);

Expand All @@ -159,7 +167,7 @@ test.serial("ui5 config set with an empty value should delete the configuration"
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["option"] = "mavenSnapshotEndpointUrl";
argv["value"] = ""; // Simulating empty value provided to Yargs using quotes ""
await config.handler(argv);

Expand All @@ -177,7 +185,7 @@ test.serial("ui5 config set null should update the configuration", async (t) =>
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["option"] = "mavenSnapshotEndpointUrl";

// Yargs would never provide us with other types than string. Still, our code should
// check for empty strings and nothing else (like falsy)
Expand All @@ -199,7 +207,7 @@ test.serial("ui5 config set false should update the configuration", async (t) =>
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["option"] = "mavenSnapshotEndpointUrl";

// Yargs would never provide us with other types than string. Still, our code should
// check for empty strings and nothing else (like falsyness)
Expand All @@ -217,39 +225,34 @@ test.serial("ui5 config set false should update the configuration", async (t) =>
false, "Configuration#toFile got called with expected argument");
});

test.serial("ui5 config invalid key", async (t) => {
const {config, argv} = t.context;

argv["_"] = ["get"];
argv["key"] = "_invalid_key_";
argv["value"] = "https://_snapshot_endpoint_";

await t.throwsAsync(config.handler(argv), {
message:
"The provided key is not a valid configuration option. Valid options are: mavenSnapshotEndpointUrl",
test.serial("ui5 config invalid option", async (t) => {
await t.throwsAsync(ui5(["config", "set", "_invalid_key_", "https://_snapshot_endpoint_"]), {
message: ($) => {
return $.includes("Command failed with exit code 1") &&
$.includes("Invalid values") &&
$.includes(`Given: "_invalid_key_", Choices: "`);
}
});
});

test.serial("ui5 config empty key", async (t) => {
const {config, argv} = t.context;

argv["_"] = ["set"];
argv["key"] = "";
argv["value"] = undefined;
test.serial("ui5 config empty option", async (t) => {
await t.throwsAsync(ui5(["config", "set"]), {
message: ($) => stripAnsi($) ===
`Command failed with exit code 1: ${ui5Cli} config set
Command Failed:
Not enough non-option arguments: got 0, need at least 1
await t.throwsAsync(config.handler(argv), {
message:
"The provided key is not a valid configuration option. Valid options are: mavenSnapshotEndpointUrl",
See 'ui5 --help'`
});
});

test.serial("ui5 config unknown command", async (t) => {
const {config, argv} = t.context;

argv["_"] = ["foo"];
await t.throwsAsync(ui5(["config", "foo"]), {
message: ($) => stripAnsi($) ===
`Command failed with exit code 1: ${ui5Cli} config foo
Command Failed:
Unknown argument: foo
await t.throwsAsync(config.handler(argv), {
message:
"Unknown 'ui5 config' command 'foo'",
See 'ui5 --help'`
});
});

0 comments on commit 78e032e

Please sign in to comment.