Skip to content

Commit

Permalink
refactor: Upgrade API for Settings#update (dirigeants#426)
Browse files Browse the repository at this point in the history
* refactor: No more _update

* cleanup: More re-organization

* src: More refactors

* Settings#{list,resolveString} -> Settings#display

* style: Rename constant name

* refactor: Re-add Conf#check and reduce duplicated code in Settings#reset

* fix: _resolveUpdateOverloads not parsing reset args correctly

* fix: Synchronize the Settings entry on update or reset instead of returning wrong results

* typings: Fixed QB types and updated everything

* docs: Added missing jsdocs and refactored Settings#get

* fix: conf and userconf always throwing on empty values

* fix: Better Schema#get

* fix: Settings#show using Map[key] instead of Map#get(key)

* fix: Settings#_resolveUpdateOverloads not checking for the lack of a key

* fix: conf command not passing all args for check

* fix: Fixed multiple things

* docs: Update guides to reflect latest changes

* pr: Update changelog
  • Loading branch information
kyranet authored and UnseenFaith committed Dec 11, 2018
1 parent 10e11eb commit 3c5820f
Show file tree
Hide file tree
Showing 9 changed files with 304 additions and 268 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ NOTE: For the contributors, you add new entries to this document following this

### Added

- [[#426](https://github.com/dirigeants/klasa/pull/426)] Added `Schema#configurableValues`, similar to `Schema#configurableKeys`. (kyranet)
- [[#426](https://github.com/dirigeants/klasa/pull/426)] Added `SettingsUpdateOptions.rejectOnError` to make `Settings#update` reject when any of the keys are invalid or the parse turns out wrong. (kyranet)
- [[#416](https://github.com/dirigeants/klasa/pull/416)] Added `KlasaClientOptions.createPiecesFolders` to not create pieces' folders if they do not exist. (kyranet)
- [[#398](https://github.com/dirigeants/klasa/pull/398)] Added the `Settings#update(entries: Array<[string, any]>);` overload. (kyranet)
- [[#392](https://github.com/dirigeants/klasa/pull/392)] Added support for empty prefixes. (kyranet)
Expand Down Expand Up @@ -125,6 +127,11 @@ NOTE: For the contributors, you add new entries to this document following this

### Changed

- [[#426](https://github.com/dirigeants/klasa/pull/426)] Made schema resolving much faster and reliable. (kyranet)
- [[#426](https://github.com/dirigeants/klasa/pull/426)] **[BREAKING]** `Settings#{list,resolveString}` -> `Settings#display`. (kyranet)
- [[#426](https://github.com/dirigeants/klasa/pull/426)] Tweaked conf and userconf commands to use restString for values. (kyranet)
- [[#426](https://github.com/dirigeants/klasa/pull/426)] Guild Settings will not longer need guild. (kyranet)
- [[#426](https://github.com/dirigeants/klasa/pull/426)] **[BREAKING]** Moved the guild parameter in Settings#update to `SettingsUpdateOptions`. (kyranet)
- [[#467](https://github.com/dirigeants/klasa/pull/467)] **[BREAKING]** Changed `GatewayDriver` to extend `Collection<string, Gateway>` as opposed to being a dictionary type. (kyranet)
- [[#392](https://github.com/dirigeants/klasa/pull/392)] Changed default prefix from `'!'` to `''`. (kyranet)
- [[#383](https://github.com/dirigeants/klasa/pull/383)] **[BREAKING]** Changed SchemaFolder to extend Schema, which extends Map. All keys are now stored inside the map as opposed to being properties. (Unseenfaith)
Expand Down Expand Up @@ -229,6 +236,7 @@ NOTE: For the contributors, you add new entries to this document following this

### Removed

- [[#426](https://github.com/dirigeants/klasa/pull/426)] Removed `Gateway#getPath`. They're now resolved inside Settings. (kyranet)
- [[#401](https://github.com/dirigeants/klasa/pull/401)] Removed `Settings#waitSync` in favor of `Settings#sync(?false);`. (kyranet)
- [[#398](https://github.com/dirigeants/klasa/pull/398)] Removed the `Settings#update(keys: string[], values: any[])` overload. (kyranet)
- [[#391](https://github.com/dirigeants/klasa/pull/391)] Removed `Util.getIdentifier()`. (kyranet)
Expand Down Expand Up @@ -292,6 +300,7 @@ NOTE: For the contributors, you add new entries to this document following this

### Fixed

- [[#426](https://github.com/dirigeants/klasa/pull/426)] Fixed typings for QueryBuilder. (kyranet)
- [[#383](https://github.com/dirigeants/klasa/pull/383)] Fixed abstract `SQLProvider#qb` property being missing in typings. (kyranet)
- [[#362](https://github.com/dirigeants/klasa/pull/362)] Fixed object mutation in `GatewayDriver#toJSON()`. (kyranet)
- [[#355](https://github.com/dirigeants/klasa/pull/355)] Fixed Schedule not deleting entries that do not exist in ClientStorage but are still cached in Schedule#tasks. (kyranet)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ Once we have our schema completed with all the keys, folders and types needed, w
// Updating the value of a key
// This key is contained in the roles folder, and the second value is a role id, we also need
// to pass a GuildResolvable.
message.guild.settings.update('roles.administrator', '339943234405007361', message.guild);
message.guild.settings.update('roles.administrator', '339943234405007361');

// For retrocompatibility, the object overload is still available, however, this is much slower.
// If you store objects literals in keys that do not take an array, this may break, prefer the
// other overload or use nested SchemaPieces for full consistency.
message.guild.settings.update({ roles: { administrator: '339943234405007361' } }, message.guild);
message.guild.settings.update({ roles: { administrator: '339943234405007361' } });

// Updating an array
// userBlacklist, as mentioned in another tutorial, it's a piece with an array of users. Using
Expand Down
37 changes: 19 additions & 18 deletions src/commands/Admin/conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module.exports = class extends Command {
guarded: true,
subcommands: true,
description: language => language.get('COMMAND_CONF_SERVER_DESCRIPTION'),
usage: '<set|show|remove|reset> (key:key) (value:value) [...]',
usage: '<set|show|remove|reset> (key:key) (value:value)',
usageDelim: ' '
});

Expand All @@ -19,42 +19,43 @@ module.exports = class extends Command {
throw message.language.get('COMMAND_CONF_NOKEY');
})
.createCustomResolver('value', (arg, possible, message, [action]) => {
if (!['set', 'remove'].includes(action) || arg) return arg;
if (!['set', 'remove'].includes(action)) return null;
if (arg) return this.client.arguments.get('...string').run(arg, possible, message);
throw message.language.get('COMMAND_CONF_NOVALUE');
});
}

show(message, [key]) {
const path = this.client.gateways.get('guilds').getPath(key, { avoidUnconfigurable: true, errors: false, piece: null });
if (!path) return message.sendLocale('COMMAND_CONF_GET_NOEXT', [key]);
if (path.piece.type === 'Folder') {
const piece = this.client.gateways.get('guilds').schema.get(key || '');
if (!piece || piece.type === 'Folder' ? !piece.configurableKeys.length : !piece.configurable) return message.sendLocale('COMMAND_CONF_GET_NOEXT', [key]);
if (piece.type === 'Folder') {
return message.sendLocale('COMMAND_CONF_SERVER', [
key ? `: ${key.split('.').map(toTitleCase).join('/')}` : '',
codeBlock('asciidoc', message.guild.settings.list(message, path.piece))
codeBlock('asciidoc', message.guild.settings.display(message, piece))
]);
}
return message.sendLocale('COMMAND_CONF_GET', [path.piece.path, message.guild.settings.resolveString(message, path.piece)]);
return message.sendLocale('COMMAND_CONF_GET', [piece.path, message.guild.settings.display(message, piece)]);
}

async set(message, [key, ...valueToSet]) {
const status = await message.guild.settings.update(key, valueToSet.join(' '), message.guild, { avoidUnconfigurable: true, action: 'add' });
return this.check(message, key, status) || message.sendLocale('COMMAND_CONF_UPDATED', [key, message.guild.settings.resolveString(message, status.updated[0].piece)]);
async set(message, [key, valueToSet]) {
const piece = this.check(message, key, await message.guild.settings.update(key, valueToSet, { avoidUnconfigurable: true, action: 'add', guild: message.guild }));
return message.sendLocale('COMMAND_CONF_UPDATED', [key, message.guild.settings.display(message, piece)]);
}

async remove(message, [key, ...valueToRemove]) {
const status = await message.guild.settings.update(key, valueToRemove.join(' '), message.guild, { avoidUnconfigurable: true, action: 'remove' });
return this.check(message, key, status) || message.sendLocale('COMMAND_CONF_UPDATED', [key, message.guild.settings.resolveString(message, status.updated[0].piece)]);
async remove(message, [key, valueToRemove]) {
const piece = this.check(message, key, await message.guild.settings.update(key, valueToRemove, { avoidUnconfigurable: true, action: 'remove', guild: message.guild }));
return message.sendLocale('COMMAND_CONF_UPDATED', [key, message.guild.settings.display(message, piece)]);
}

async reset(message, [key]) {
const status = await message.guild.settings.reset(key, message.guild, true);
return this.check(message, key, status) || message.sendLocale('COMMAND_CONF_RESET', [key, message.guild.settings.resolveString(message, status.updated[0].piece)]);
const piece = this.check(message, key, await message.guild.settings.reset(key, { guild: message.guild }));
return message.sendLocale('COMMAND_CONF_RESET', [key, message.guild.settings.display(message, piece)]);
}

check(message, key, { errors, updated }) {
if (errors.length) return message.sendMessage(String(errors[0]));
if (!updated.length) return message.sendLocale('COMMAND_CONF_NOCHANGE', [key]);
return null;
if (errors.length) throw String(errors[0]);
if (!updated.length) throw message.language.get('COMMAND_CONF_NOCHANGE', key);
return updated[0].piece;
}

};
37 changes: 19 additions & 18 deletions src/commands/General/User Settings/userconf.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module.exports = class extends Command {
guarded: true,
subcommands: true,
description: language => language.get('COMMAND_CONF_USER_DESCRIPTION'),
usage: '<set|show|remove|reset> (key:key) (value:value) [...]',
usage: '<set|show|remove|reset> (key:key) (value:value)',
usageDelim: ' '
});

Expand All @@ -17,42 +17,43 @@ module.exports = class extends Command {
throw message.language.get('COMMAND_CONF_NOKEY');
})
.createCustomResolver('value', (arg, possible, message, [action]) => {
if (!['set', 'remove'].includes(action) || arg) return arg;
if (!['set', 'remove'].includes(action)) return null;
if (arg) return this.client.arguments.get('...string').run(arg, possible, message);
throw message.language.get('COMMAND_CONF_NOVALUE');
});
}

show(message, [key]) {
const path = this.client.gateways.get('users').getPath(key, { avoidUnconfigurable: true, errors: false, piece: null });
if (!path) return message.sendLocale('COMMAND_CONF_GET_NOEXT', [key]);
if (path.piece.type === 'Folder') {
const piece = this.client.gateways.get('users').schema.get(key);
if (!piece || piece.type === 'Folder' ? !piece.configurableKeys.length : !piece.configurable) return message.sendLocale('COMMAND_CONF_GET_NOEXT', [key]);
if (piece.type === 'Folder') {
return message.sendLocale('COMMAND_CONF_USER', [
key ? `: ${key.split('.').map(toTitleCase).join('/')}` : '',
codeBlock('asciidoc', message.author.settings.list(message, path.piece))
codeBlock('asciidoc', message.author.settings.display(message, piece))
]);
}
return message.sendLocale('COMMAND_CONF_GET', [path.piece.path, message.author.settings.resolveString(message, path.piece)]);
return message.sendLocale('COMMAND_CONF_GET', [piece.path, message.author.settings.display(message, piece)]);
}

async set(message, [key, ...valueToSet]) {
const status = await message.author.settings.update(key, valueToSet.join(' '), message.guild, { avoidUnconfigurable: true, action: 'add' });
return this.check(message, key, status) || message.sendLocale('COMMAND_CONF_UPDATED', [key, message.author.settings.resolveString(message, status.updated[0].piece)]);
async set(message, [key, valueToSet]) {
const piece = this.check(message, key, await message.author.settings.update(key, valueToSet, { avoidUnconfigurable: true, action: 'add', guild: message.guild }));
return message.sendLocale('COMMAND_CONF_UPDATED', [key, message.author.settings.display(message, piece)]);
}

async remove(message, [key, ...valueToRemove]) {
const status = await message.author.settings.update(key, valueToRemove.join(' '), message.guild, { avoidUnconfigurable: true, action: 'remove' });
return this.check(message, key, status) || message.sendLocale('COMMAND_CONF_UPDATED', [key, message.author.settings.resolveString(message, status.updated[0].piece)]);
async remove(message, [key, valueToRemove]) {
const piece = this.check(message, key, await message.author.settings.update(key, valueToRemove, { avoidUnconfigurable: true, action: 'remove', guild: message.guild }));
return message.sendLocale('COMMAND_CONF_UPDATED', [key, message.author.settings.display(message, piece)]);
}

async reset(message, [key]) {
const status = await message.author.settings.reset(key, true);
return this.check(message, key, status) || message.sendLocale('COMMAND_CONF_RESET', [key, message.author.settings.resolveString(message, status.updated[0].piece)]);
const piece = this.check(message, key, await message.author.settings.reset(key));
return message.sendLocale('COMMAND_CONF_RESET', [key, message.author.settings.display(message, piece)]);
}

check(message, key, { errors, updated }) {
if (errors.length) return message.sendMessage(errors[0]);
if (!updated.length) return message.sendLocale('COMMAND_CONF_NOCHANGE', [key]);
return null;
if (errors.length) throw String(errors[0]);
if (!updated.length) throw message.language.get('COMMAND_CONF_NOCHANGE', key);
return updated[0].piece;
}

};
60 changes: 0 additions & 60 deletions src/lib/settings/GatewayStorage.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,5 @@
class GatewayStorage {

/**
* @typedef {Object} GatewayGetPathOptions
* @property {boolean} [avoidUnconfigurable=false] Whether the getPath should avoid unconfigurable keys
* @property {boolean} [piece=true] Whether the getPath should return pieces or folders
*/

/**
* @typedef {Object} GatewayGetPathResult
* @property {SchemaPiece} piece The piece resolved from the path
* @property {string[]} route The resolved path split by dots
*/

/**
* @typedef {Object} GatewayJSON
* @property {string} type The name of this gateway
Expand Down Expand Up @@ -89,54 +77,6 @@ class GatewayStorage {
return { ...this.schema.defaults, default: true };
}

/**
* Resolve a path from a string.
* @since 0.5.0
* @param {string} [key=null] A string to resolve
* @param {GatewayGetPathOptions} [options={}] Whether the Gateway should avoid configuring the selected key
* @returns {?GatewayGetPathResult}
*/
getPath(key = '', { avoidUnconfigurable = false, piece: requestPiece = true, errors = true } = {}) {
if (key === '' || key === '.') return { piece: this.schema, route: [] };
const route = key.split('.');
const piece = this.schema.get(route);

// The piece does not exist (invalid or non-existent path)
if (!piece) {
if (!errors) return null;
throw `The key ${key} does not exist in the schema.`;
}

if (requestPiece === null) requestPiece = piece.type !== 'Folder';

// GetPath expects a piece
if (requestPiece) {
// The piece is a key
if (piece.type !== 'Folder') {
// If the Piece is unconfigurable and avoidUnconfigurable is requested, throw
if (avoidUnconfigurable && !piece.configurable) {
if (!errors) return null;
throw `The key ${piece.path} is not configurable.`;
}
return { piece, route };
}

// The piece is a folder
if (!errors) return null;
const keys = avoidUnconfigurable ? piece.configurableKeys : [...piece.keys()];
throw keys.length ? `Please, choose one of the following keys: '${keys.join('\', \'')}'` : 'This group is not configurable.';
}

// GetPath does not expect a piece
if (piece.type !== 'Folder') {
// Remove leading key from the path
route.pop();
return { piece: piece.parent, route };
}

return { piece, route };
}

/**
* Inits the current Gateway.
* @since 0.5.0
Expand Down
Loading

0 comments on commit 3c5820f

Please sign in to comment.