Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: faster overwrites and string min max length support #495

Merged
merged 2 commits into from
Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
"@sapphire/result": "^1.1.1",
"@sapphire/stopwatch": "^1.4.1",
"@sapphire/utilities": "^3.7.0",
"@types/object-hash": "^2.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

types in regular deps?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah otherwise people need to install them themselves or use skipLibCheck and seeing as we're a type-first framework that would be bad DX

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, thanks

"lexure": "^0.17.0",
"object-hash": "^3.0.0",
"tslib": "^2.4.0"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/structures/Command.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AliasPiece, AliasPieceJSON, AliasStore, PieceContext } from '@sapphire/pieces';
import { Awaitable, isNullish, NonNullObject } from '@sapphire/utilities';
import type { LocalizationMap } from 'discord-api-types/v9';
import type { LocalizationMap } from 'discord-api-types/v10';
import {
AutocompleteInteraction,
CommandInteraction,
Expand Down
19 changes: 14 additions & 5 deletions src/lib/types/Enums.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
export const enum CooldownLevel {
export enum CooldownLevel {
Author = 'author',
Channel = 'channel',
Guild = 'guild'
}

export const enum PluginHook {
export enum PluginHook {
PreGenericsInitialization = 'preGenericsInitialization',
PreInitialization = 'preInitialization',
PostInitialization = 'postInitialization',
Expand All @@ -15,7 +15,7 @@ export const enum PluginHook {
/**
* The scope the cooldown applies to.
*/
export const enum BucketScope {
export enum BucketScope {
/**
* Per channel cooldowns.
*/
Expand All @@ -34,9 +34,18 @@ export const enum BucketScope {
User
}

export const enum RegisterBehavior {
export enum RegisterBehavior {
Overwrite = 'OVERWRITE',
LogToConsole = 'LOG_TO_CONSOLE'
LogToConsole = 'LOG_TO_CONSOLE',
/**
* Finds all differences in the commands provided using our internal computation method, and logs them to the console, while applying them.
*
* @danger This can potentially cause slowdowns when booting up your bot as computing differences on big commands can take a while.
* We recommend you use `OVERWRITE` instead in production.
* @deprecated This mode should be considered "deprecated" in the sense it won't get the same attention as the other modes. It's still usable, but it might
* also remain behind compared to API updates.
*/
VerboseOverwrite = 'VERBOSE_OVERWRITE'
}

export const enum InternalRegistryAPIType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ApplicationCommandRegistry } from './ApplicationCommandRegistry';
import { emitRegistryError } from './emitRegistryError';
import { getNeededRegistryParameters } from './getNeededParameters';

export let defaultBehaviorWhenNotIdentical = RegisterBehavior.LogToConsole;
export let defaultBehaviorWhenNotIdentical = RegisterBehavior.Overwrite;

export const registries = new Map<string, ApplicationCommandRegistry>();

Expand All @@ -28,10 +28,10 @@ export function acquire(commandName: string) {
/**
* Sets the default behavior when registered commands aren't identical to provided data.
* @param behavior The default behavior to have. Set this to `null` to reset it to the default
* of `RegisterBehavior.LogToConsole`.
* of `RegisterBehavior.Overwrite`.
*/
export function setDefaultBehaviorWhenNotIdentical(behavior?: RegisterBehavior | null) {
defaultBehaviorWhenNotIdentical = behavior ?? RegisterBehavior.LogToConsole;
defaultBehaviorWhenNotIdentical = behavior ?? RegisterBehavior.Overwrite;
}

export function getDefaultBehaviorWhenNotIdentical() {
Expand Down
64 changes: 46 additions & 18 deletions src/lib/utils/application-commands/ApplicationCommandRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
ApplicationCommandType,
RESTPostAPIChatInputApplicationCommandsJSONBody,
RESTPostAPIContextMenuApplicationCommandsJSONBody
} from 'discord-api-types/v9';
} from 'discord-api-types/v10';
import type {
ApplicationCommand,
ApplicationCommandManager,
Expand All @@ -19,6 +19,7 @@ import type {
MessageApplicationCommandData,
UserApplicationCommandData
} from 'discord.js';
import objectHash from 'object-hash';
import { InternalRegistryAPIType, RegisterBehavior } from '../../types/Enums';
import { getDefaultBehaviorWhenNotIdentical } from './ApplicationCommandRegistries';
import { CommandDifference, getCommandDifferences } from './computeDifferences';
Expand Down Expand Up @@ -322,27 +323,53 @@ export class ApplicationCommandRegistry {
behaviorIfNotEqual: RegisterBehavior,
guildId?: string
) {
const now = Date.now();
let differences: CommandDifference[] = [];

// Step 0: compute differences
const differences = getCommandDifferences(convertApplicationCommandToApiData(applicationCommand), apiData);
if (behaviorIfNotEqual === RegisterBehavior.VerboseOverwrite) {
const now = Date.now();

const later = Date.now() - now;
this.debug(`Took ${later}ms to process differences`);
// Step 0: compute differences
differences = getCommandDifferences(convertApplicationCommandToApiData(applicationCommand), apiData);

// Step 1: if there are no differences, return
if (!differences.length) {
this.debug(
`${guildId ? 'Guild command' : 'Command'} "${apiData.name}" is identical to command "${applicationCommand.name}" (${
applicationCommand.id
})`
);
return;
const later = Date.now() - now;
this.debug(`Took ${later}ms to process differences via computing differences`);

// Step 1: if there are no differences, return
if (!differences.length) {
this.debug(
`${guildId ? 'Guild command' : 'Command'} "${apiData.name}" is identical to command "${applicationCommand.name}" (${
applicationCommand.id
})`
);
return;
}
}

// Run the fast path even if the user wants to just log if the command has a difference
if (behaviorIfNotEqual === RegisterBehavior.Overwrite || behaviorIfNotEqual === RegisterBehavior.LogToConsole) {
const now = Date.now();

// Step 0: compute differences
const oldHash = objectHash(convertApplicationCommandToApiData(applicationCommand));
const newHash = objectHash(apiData);

const later = Date.now() - now;
this.debug(`Took ${later}ms to process differences via object hashing`);

// Step 1: if there are no differences, return
if (oldHash === newHash) {
this.debug(
`${guildId ? 'Guild command' : 'Command'} "${apiData.name}" is identical to command "${applicationCommand.name}" (${
applicationCommand.id
})`
);
return;
}
}

this.logCommandDifferences(differences, applicationCommand, behaviorIfNotEqual === RegisterBehavior.LogToConsole);
this.logCommandDifferencesFound(applicationCommand, behaviorIfNotEqual === RegisterBehavior.LogToConsole, differences);

// Step 2: if the behavior is to log to console, log the differences
// Step 2: if the behavior is to log to console, only log the differences
if (behaviorIfNotEqual === RegisterBehavior.LogToConsole) {
return;
}
Expand All @@ -356,7 +383,7 @@ export class ApplicationCommandRegistry {
}
}

private logCommandDifferences(differences: CommandDifference[], applicationCommand: ApplicationCommand, logAsWarn: boolean) {
private logCommandDifferencesFound(applicationCommand: ApplicationCommand, logAsWarn: boolean, differences: CommandDifference[]) {
const finalMessage: string[] = [];
const pad = ' '.repeat(5);

Expand All @@ -371,7 +398,8 @@ export class ApplicationCommandRegistry {
);
}

const header = `Found differences for command "${applicationCommand.name}" (${applicationCommand.id}) versus provided api data\n`;
const finalMessageNewLine = finalMessage.length ? '\n' : '';
const header = `Found differences for command "${applicationCommand.name}" (${applicationCommand.id}) versus provided api data.${finalMessageNewLine}`;

logAsWarn ? this.warn(header, ...finalMessage) : this.debug(header, ...finalMessage);
}
Expand Down
61 changes: 60 additions & 1 deletion src/lib/utils/application-commands/computeDifferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
RESTPostAPIApplicationCommandsJSONBody,
RESTPostAPIChatInputApplicationCommandsJSONBody,
RESTPostAPIContextMenuApplicationCommandsJSONBody
} from 'discord-api-types/v9';
} from 'discord-api-types/v10';
import type { InternalAPICall } from './ApplicationCommandRegistry';

const optionTypeToPrettyName = new Map([
Expand Down Expand Up @@ -644,6 +644,61 @@ function* reportOptionDifferences({
}
}
}

if (hasMinMaxLengthSupport(option)) {
favna marked this conversation as resolved.
Show resolved Hide resolved
// Check min and max_value
const existingCasted = existingOption as APIApplicationCommandStringOption;

// 0. No min_length and now we have min_length
if (existingCasted.min_length === undefined && option.min_length !== undefined) {
yield {
key: `${keyPath(currentIndex)}.min_length`,
expected: 'min_length present',
original: 'no min_length present'
};
}
// 1. Have min_length and now we don't
else if (existingCasted.min_length !== undefined && option.min_length === undefined) {
yield {
key: `${keyPath(currentIndex)}.min_length`,
expected: 'no min_length present',
original: 'min_length present'
};
}
// 2. Equality check
else if (existingCasted.min_length !== option.min_length) {
yield {
key: `${keyPath(currentIndex)}.min_length`,
original: String(existingCasted.min_length),
expected: String(option.min_length)
};
}

// 0. No max_length and now we have max_length
if (existingCasted.max_length === undefined && option.max_length !== undefined) {
yield {
key: `${keyPath(currentIndex)}.max_length`,
expected: 'max_length present',
original: 'no max_length present'
};
}
// 1. Have max_length and now we don't
else if (existingCasted.max_length !== undefined && option.max_length === undefined) {
yield {
key: `${keyPath(currentIndex)}.max_length`,
expected: 'no max_length present',
original: 'max_length present'
};
}
// 2. Equality check
else if (existingCasted.max_length !== option.max_length) {
yield {
key: `${keyPath(currentIndex)}.max_length`,
original: String(existingCasted.max_length),
expected: String(option.max_length)
};
}
}
}

function hasMinMaxValueSupport(option: APIApplicationCommandOption): option is APIApplicationCommandNumericTypes {
Expand All @@ -653,3 +708,7 @@ function hasMinMaxValueSupport(option: APIApplicationCommandOption): option is A
function hasChoicesAndAutocompleteSupport(option: APIApplicationCommandOption): option is APIApplicationCommandChoosableAndAutocompletableTypes {
return [ApplicationCommandOptionType.Integer, ApplicationCommandOptionType.Number, ApplicationCommandOptionType.String].includes(option.type);
}

function hasMinMaxLengthSupport(option: APIApplicationCommandOption): option is APIApplicationCommandStringOption {
return option.type === ApplicationCommandOptionType.String;
}
2 changes: 1 addition & 1 deletion src/lib/utils/application-commands/normalizeInputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
RESTPostAPIApplicationCommandsJSONBody,
RESTPostAPIChatInputApplicationCommandsJSONBody,
RESTPostAPIContextMenuApplicationCommandsJSONBody
} from 'discord-api-types/v9';
} from 'discord-api-types/v10';
import {
ApplicationCommand,
ChatInputApplicationCommandData,
Expand Down
Loading