Skip to content

fix: do not render plugins with invalid configurations #129

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.

Sign up for GitHub

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

Open
cpruijsen wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: do not render plugins with invalid configurations #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 15 additions & 9 deletions src/plugins/data/utils.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,35 @@ const validateRequirements = (requiredTypes, widgetConfig) => Object.keys(requir

/**
* Called by organizePlugins to validate plugin configurations
* @returns {Boolean} - boolean if all types are correct and present, else throws an error
* @returns {Boolean} - true if all types are correct and present, otherwise logs an error and returns false
*/
export const validatePlugin = (pluginConfig) => {
let requiredTypes = {};
const { op } = pluginConfig;
let config = pluginConfig;

if (!op) { logError('There is a config with an invalid PLUGIN_OPERATION. Check to make sure it is configured correctly.'); }
if (!Object.values(PLUGIN_OPERATIONS).includes(op)) {
logError('There is a config with an invalid PLUGIN_OPERATION. Check to make sure it is configured correctly.');
return false;
}

let requiredTypes = requiredPluginTypes[op];

if (op === PLUGIN_OPERATIONS.Insert) {
config = config.widget;
if (!config) { logError('insert operation config is missing widget object'); }
if (!config) {
logError('insert operation config is missing widget object');
return false;
}

requiredTypes = {
...requiredPluginTypes[op].base,
...requiredPluginTypes[op][config.type?.toLowerCase()],
...requiredTypes.base,
...requiredTypes[config.type?.toLowerCase()],
};
} else {
requiredTypes = requiredPluginTypes[op];
}

if (!validateRequirements(requiredTypes, config)) {
logError(`the ${op} operation config is invalid for widget id: ${config.widgetId || config.id || 'MISSING ID'}`);
return false;
}

return true;
Expand All @@ -52,7 +58,7 @@ export const validatePlugin = (pluginConfig) => {
export const organizePlugins = (defaultContents, plugins) => {
const newContents = [...defaultContents];
plugins.forEach(change => {
validatePlugin(change);
if (!validatePlugin(change)) { return; }
if (change.op === PLUGIN_OPERATIONS.Insert) {
newContents.push(change.widget);
} else if (change.op === PLUGIN_OPERATIONS.Hide) {
Expand Down
162 changes: 101 additions & 61 deletions src/plugins/data/utils.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,79 @@ describe('organizePlugins', () => {
expect(plugins[1].id).toBe('default_contents');
expect(plugins[2].id).toBe('login');
});

it('should not insert a plugin that is missing a required property', () => {
const plugins = organizePlugins(mockDefaultContent, [
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'no_priority_plugin',
type: DIRECT_PLUGIN,
RenderWidget: mockRenderWidget,
},
},
]);
expect(plugins.length).toEqual(1);
expect(plugins.find((w) => w.id === 'no_priority_plugin')).toBeUndefined();
expect(logError).toHaveBeenCalledWith('the insert operation config is invalid for widget id: no_priority_plugin');
});

it('should ignore operations targeting a plugin that failed validation', () => {
const plugins = organizePlugins(mockDefaultContent, [
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'no_priority_plugin',
type: DIRECT_PLUGIN,
RenderWidget: mockRenderWidget,
},
},
{
op: PLUGIN_OPERATIONS.Modify,
widgetId: 'no_priority_plugin',
fn: mockModifyWidget,
},
{
op: PLUGIN_OPERATIONS.Wrap,
widgetId: 'no_priority_plugin',
wrapper: makeMockElementWrapper(),
},
]);
expect(plugins.length).toEqual(1);
expect(plugins[0].id).toBe('default_contents');
});

it('should preserve the priority ordering of valid plugins when an insert is invalid', () => {
const plugins = organizePlugins(mockDefaultContent, [
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'second_plugin',
priority: 30,
type: DIRECT_PLUGIN,
RenderWidget: mockRenderWidget,
},
},
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'no_priority_plugin',
type: DIRECT_PLUGIN,
RenderWidget: mockRenderWidget,
},
},
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'first_plugin',
priority: 10,
type: DIRECT_PLUGIN,
RenderWidget: mockRenderWidget,
},
},
]);
expect(plugins.map((w) => w.id)).toEqual(['first_plugin', 'second_plugin', 'default_contents']);
});
});
});

Expand Down Expand Up @@ -329,35 +402,20 @@ describe('validatePlugin', () => {
},
};

try {
validatePlugin(insertBrokenDirectConfig);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the insert configuration is invalid for widget id: MISSING ID');
}

try {
validatePlugin(insertBrokenDirectConfig2);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the insert configuration is invalid for widget id: new_plugin');
}

try {
validatePlugin(insertBrokenDirectConfig3);
} catch (error) {
expect(logError).toHaveBeenCalledWith('insert operation config is missing widget object');
}

try {
validatePlugin(insertBrokenIFrameConfig);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the insert configuration is invalid for widget id: new_iframe_plugin');
}

try {
validatePlugin(insertBrokenIFrameConfig2);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the insert configuration is invalid for widget id: new_iframe_plugin');
}
expect(validatePlugin(insertBrokenDirectConfig)).toBe(false);
expect(logError).toHaveBeenCalledWith('the insert operation config is invalid for widget id: MISSING ID');

expect(validatePlugin(insertBrokenDirectConfig2)).toBe(false);
expect(logError).toHaveBeenCalledWith('the insert operation config is invalid for widget id: new_plugin');

expect(validatePlugin(insertBrokenDirectConfig3)).toBe(false);
expect(logError).toHaveBeenCalledWith('insert operation config is missing widget object');

expect(validatePlugin(insertBrokenIFrameConfig)).toBe(false);
expect(logError).toHaveBeenCalledWith('the insert operation config is invalid for widget id: new_iframe_plugin');

expect(validatePlugin(insertBrokenIFrameConfig2)).toBe(false);
expect(logError).toHaveBeenCalledWith('the insert operation config is invalid for widget id: new_iframe_plugin');
});
});
describe('hide plugin configuration', () => {
Expand All @@ -373,11 +431,8 @@ describe('validatePlugin', () => {
op: PLUGIN_OPERATIONS.Hide,
};

try {
validatePlugin(invalidHideConfig);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the hide operation config is invalid for widget id: MISSING ID');
}
expect(validatePlugin(invalidHideConfig)).toBe(false);
expect(logError).toHaveBeenCalledWith('the hide operation config is invalid for widget id: MISSING ID');
});
});
describe('modify plugin configuration', () => {
Expand All @@ -399,16 +454,10 @@ describe('validatePlugin', () => {
fn: mockModifyWidget,
};

try {
validatePlugin(invalidModifyConfig1);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the modify operation config is invalid for widget id: random_plugin');
}
try {
validatePlugin(invalidModifyConfig2);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the modify operation config is invalid for widget id: MISSING ID');
}
expect(validatePlugin(invalidModifyConfig1)).toBe(false);
expect(logError).toHaveBeenCalledWith('the modify operation config is invalid for widget id: random_plugin');
expect(validatePlugin(invalidModifyConfig2)).toBe(false);
expect(logError).toHaveBeenCalledWith('the modify operation config is invalid for widget id: MISSING ID');
});
});
describe('wrap plugin configuration', () => {
Expand All @@ -430,30 +479,21 @@ describe('validatePlugin', () => {
wrapper: makeMockElementWrapper(),
};

try {
validatePlugin(invalidWrapConfig1);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the wrap operation config is invalid for widget id: random_plugin');
}
try {
validatePlugin(invalidWrapConfig2);
} catch (error) {
expect(logError).toHaveBeenCalledWith('the wrap operation config is invalid for widget id: MISSING ID');
}
expect(validatePlugin(invalidWrapConfig1)).toBe(false);
expect(logError).toHaveBeenCalledWith('the wrap operation config is invalid for widget id: random_plugin');
expect(validatePlugin(invalidWrapConfig2)).toBe(false);
expect(logError).toHaveBeenCalledWith('the wrap operation config is invalid for widget id: MISSING ID');
});
});
describe('an invalid plugin configuration', () => {
it('should raise an error for an operation that does not exist', () => {
it.each([undefined, 'destroy'])('should return false and raise an error for operation "%s"', (op) => {
const invalidPluginConfig = {
op: PLUGIN_OPERATIONS.Destroy,
op,
widgetId: 'drafts',
};

try {
validatePlugin(invalidPluginConfig);
} catch (error) {
expect(logError).toHaveBeenCalledWith('There is a config with an invalid PLUGIN_OPERATION. Check to make sure it is configured correctly.');
}
expect(validatePlugin(invalidPluginConfig)).toBe(false);
expect(logError).toHaveBeenCalledWith('There is a config with an invalid PLUGIN_OPERATION. Check to make sure it is configured correctly.');
});
});
});