Skip to content

Commit

Permalink
fix(computeDifferences): ensure Overwrite strategy is actually fast…
Browse files Browse the repository at this point in the history 
…er (#586)
  • Loading branch information
@vladfrangu
vladfrangu committed Jan 4, 2023
1 parent dd052dd commit bfa3561
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 53 deletions.
2 changes: 0 additions & 2 deletions src/lib/types/Enums.ts
View file
Expand Up @@ -42,8 +42,6 @@ export enum RegisterBehavior {
*
* @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'
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/utils/application-commands/ApplicationCommandRegistry.ts
View file
Expand Up @@ -344,7 +344,7 @@ export class ApplicationCommandRegistry {
const now = Date.now();

// Step 0: compute differences
differences = getCommandDifferences(convertApplicationCommandToApiData(applicationCommand), apiData, guildId !== null);
differences = [...getCommandDifferences(convertApplicationCommandToApiData(applicationCommand), apiData, guildId !== null)];

const later = Date.now() - now;
this.debug(`Took ${later}ms to process differences via computing differences`);
Expand Down
92 changes: 43 additions & 49 deletions src/lib/utils/application-commands/computeDifferences.ts
View file
Expand Up @@ -52,192 +52,188 @@ export function getCommandDifferencesFast(
return false;
}

export function getCommandDifferences(
export function* getCommandDifferences(
existingCommand: RESTPostAPIApplicationCommandsJSONBody,
apiData: InternalAPICall['builtData'],
guildCommand: boolean
) {
const differences: CommandDifference[] = [];

): Generator<CommandDifference> {
if (existingCommand.type !== ApplicationCommandType.ChatInput && existingCommand.type) {
// Check context menus
if (contextMenuTypes.includes(existingCommand.type ?? ApplicationCommandType.ChatInput)) {
const casted = apiData as RESTPostAPIContextMenuApplicationCommandsJSONBody;

// Check name
if (existingCommand.name !== casted.name) {
differences.push({
yield {
key: 'name',
original: existingCommand.name,
expected: casted.name
});
};
}

// Check defaultPermissions
// TODO(vladfrangu): This will be deprecated
if ((existingCommand.default_permission ?? true) !== (casted.default_permission ?? true)) {
differences.push({
yield {
key: 'defaultPermission',
original: String(existingCommand.default_permission ?? true),
expected: String(casted.default_permission ?? true)
});
};
}

// Check dmPermission only for non-guild commands
if (!guildCommand && (existingCommand.dm_permission ?? true) !== (casted.dm_permission ?? true)) {
differences.push({
yield {
key: 'dmPermission',
original: String(existingCommand.dm_permission ?? true),
expected: String(casted.dm_permission ?? true)
});
};
}

// Check defaultMemberPermissions
if (existingCommand.default_member_permissions !== casted.default_member_permissions) {
differences.push({
yield {
key: 'defaultMemberPermissions',
original: String(existingCommand.default_member_permissions),
expected: String(casted.default_member_permissions)
});
};
}

// Check localized names
const originalLocalizedNames = existingCommand.name_localizations;
const expectedLocalizedNames = casted.name_localizations;

if (!originalLocalizedNames && expectedLocalizedNames) {
differences.push({
yield {
key: 'nameLocalizations',
original: 'no localized names',
expected: 'localized names'
});
};
} else if (originalLocalizedNames && !expectedLocalizedNames) {
differences.push({
yield {
key: 'nameLocalizations',
original: 'localized names',
expected: 'no localized names'
});
};
} else if (originalLocalizedNames && expectedLocalizedNames) {
differences.push(...reportLocalizationMapDifferences(originalLocalizedNames, expectedLocalizedNames, 'nameLocalizations'));
yield* reportLocalizationMapDifferences(originalLocalizedNames, expectedLocalizedNames, 'nameLocalizations');
}
}

return differences;
return;
}

const casted = apiData as RESTPostAPIChatInputApplicationCommandsJSONBody;

// Check name
if (existingCommand.name.toLowerCase() !== casted.name.toLowerCase()) {
differences.push({
yield {
key: 'name',
original: existingCommand.name,
expected: casted.name
});
};
}

// Check localized names
const originalLocalizedNames = existingCommand.name_localizations;
const expectedLocalizedNames = casted.name_localizations;

if (!originalLocalizedNames && expectedLocalizedNames) {
differences.push({
yield {
key: 'nameLocalizations',
original: 'no localized names',
expected: 'localized names'
});
};
} else if (originalLocalizedNames && !expectedLocalizedNames) {
differences.push({
yield {
key: 'nameLocalizations',
original: 'localized names',
expected: 'no localized names'
});
};
} else if (originalLocalizedNames && expectedLocalizedNames) {
differences.push(...reportLocalizationMapDifferences(originalLocalizedNames, expectedLocalizedNames, 'nameLocalizations'));
yield* reportLocalizationMapDifferences(originalLocalizedNames, expectedLocalizedNames, 'nameLocalizations');
}

// Check defaultPermissions
// TODO(vladfrangu): This will be deprecated
if ((existingCommand.default_permission ?? true) !== (casted.default_permission ?? true)) {
differences.push({
yield {
key: 'defaultPermission',
original: String(existingCommand.default_permission ?? true),
expected: String(casted.default_permission ?? true)
});
};
}

// Check dmPermission
if (!guildCommand && (existingCommand.dm_permission ?? true) !== (casted.dm_permission ?? true)) {
differences.push({
yield {
key: 'dmPermission',
original: String(existingCommand.dm_permission ?? true),
expected: String(casted.dm_permission ?? true)
});
};
}

// Check defaultMemberPermissions
if (existingCommand.default_member_permissions !== casted.default_member_permissions) {
differences.push({
yield {
key: 'defaultMemberPermissions',
original: String(existingCommand.default_member_permissions),
expected: String(casted.default_member_permissions)
});
};
}

// Check description
if (existingCommand.description !== casted.description) {
differences.push({
yield {
key: 'description',
original: existingCommand.description,
expected: casted.description
});
};
}

// Check localized descriptions
const originalLocalizedDescriptions = existingCommand.description_localizations;
const expectedLocalizedDescriptions = casted.description_localizations;

if (!originalLocalizedDescriptions && expectedLocalizedDescriptions) {
differences.push({
yield {
key: 'descriptionLocalizations',
original: 'no localized descriptions',
expected: 'localized descriptions'
});
};
} else if (originalLocalizedDescriptions && !expectedLocalizedDescriptions) {
differences.push({
yield {
key: 'descriptionLocalizations',
original: 'localized descriptions',
expected: 'no localized descriptions'
});
};
} else if (originalLocalizedDescriptions && expectedLocalizedDescriptions) {
differences.push(
...reportLocalizationMapDifferences(originalLocalizedDescriptions, expectedLocalizedDescriptions, 'descriptionLocalizations')
);
yield* reportLocalizationMapDifferences(originalLocalizedDescriptions, expectedLocalizedDescriptions, 'descriptionLocalizations');
}

// 0. No existing options and now we have options
if (!existingCommand.options?.length && casted.options?.length) {
differences.push({
yield {
key: 'options',
original: 'no options present',
expected: 'options present'
});
};
}
// 1. Existing options and now we have no options
else if (existingCommand.options?.length && !casted.options?.length) {
differences.push({
yield {
key: 'options',
original: 'options present',
expected: 'no options present'
});
};
}
// 2. Iterate over each option if we have any and see what's different
else if (casted.options?.length) {
let index = 0;
for (const option of casted.options) {
const currentIndex = index++;
const existingOption = existingCommand.options![currentIndex];
differences.push(...reportOptionDifferences({ currentIndex, option, existingOption }));
yield* reportOptionDifferences({ currentIndex, option, existingOption });
}

// If we went through less options than we previously had, report that
Expand All @@ -247,18 +243,16 @@ export function getCommandDifferences(
const expectedType =
optionTypeToPrettyName.get(option.type) ?? `unknown (${option.type}); please contact Sapphire developers about this!`;

differences.push({
yield {
key: `existing command option at index ${index}`,
expected: 'no option present',
original: `${expectedType} with name ${option.name}`
});
};

index++;
}
}
}

return differences;
}

function* reportLocalizationMapDifferences(
Expand Down
6 changes: 5 additions & 1 deletion tests/application-commands/computeDifferences.test.ts
View file
@@ -1,5 +1,9 @@
import { ApplicationCommandOptionType, ApplicationCommandType, RESTPostAPIChatInputApplicationCommandsJSONBody } from 'discord-api-types/v10';
import { getCommandDifferences } from '../../src/lib/utils/application-commands/computeDifferences';
import { getCommandDifferences as getCommandDifferencesRaw } from '../../src/lib/utils/application-commands/computeDifferences';

function getCommandDifferences(...args: Parameters<typeof getCommandDifferencesRaw>) {
return [...getCommandDifferencesRaw(...args)];
}

describe('Compute differences for provided application commands', () => {
test('GIVEN two identical context menu commands THEN do not return any difference', () => {
Expand Down

0 comments on commit bfa3561

Please sign in to comment.