Skip to content

Commit

Permalink
Model Validations (#14991)
Browse files Browse the repository at this point in the history
* adds model-validations decorator and validators util

* converts key-mixin to decorator

* updates models to use validations decorator instead of ember-cp-validations

* updates invocation of model validations

* removes ember-cp-validations

* reverts secret-v2 model updates

* adds initials to TODO comment
  • Loading branch information
zofskeez authored Apr 11, 2022
1 parent 1090865 commit 4f8eeab
Show file tree
Hide file tree
Showing 15 changed files with 292 additions and 140 deletions.
16 changes: 8 additions & 8 deletions ui/app/components/generated-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,16 @@ export default Component.extend({
actions: {
onKeyUp(name, value) {
this.model.set(name, value);
if (this.model.validations) {
if (this.model.validate) {
// Set validation error message for updated attribute
this.model.validations.attrs[name] && this.model.validations.attrs[name].isValid
? set(this.validationMessages, name, '')
: set(this.validationMessages, name, this.model.validations.attrs[name].message);

const { isValid, state } = this.model.validate();
if (state[name]) {
state[name].isValid
? set(this.validationMessages, name, '')
: set(this.validationMessages, name, state[name].errors.join('. '));
}
// Set form button state
this.model.validate().then(({ validations }) => {
this.set('isFormInvalid', !validations.isValid);
});
this.set('isFormInvalid', !isValid);
} else {
this.set('isFormInvalid', false);
}
Expand Down
25 changes: 11 additions & 14 deletions ui/app/components/mount-backend-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export default Component.extend({

showEnable: false,

// cp-validation related properties
// validation related properties
validationMessages: null,
isFormInvalid: false,

Expand Down Expand Up @@ -166,27 +166,24 @@ export default Component.extend({

actions: {
onKeyUp(name, value) {
this.mountModel.set(name, value);
const {
isValid,
state: { path, maxVersions },
} = this.mountModel.validate();
// validate path
if (name === 'path') {
this.mountModel.set('path', value);
this.mountModel.validations.attrs.path.isValid
path.isValid
? set(this.validationMessages, 'path', '')
: set(this.validationMessages, 'path', this.mountModel.validations.attrs.path.message);
: set(this.validationMessages, 'path', path.errors.join('. '));
}
// check maxVersions is a number
if (name === 'maxVersions') {
this.mountModel.set('maxVersions', value);
this.mountModel.validations.attrs.maxVersions.isValid
maxVersions.isValid
? set(this.validationMessages, 'maxVersions', '')
: set(
this.validationMessages,
'maxVersions',
this.mountModel.validations.attrs.maxVersions.message
);
: set(this.validationMessages, 'maxVersions', maxVersions.errors.join('. '));
}
this.mountModel.validate().then(({ validations }) => {
this.set('isFormInvalid', !validations.isValid);
});
this.set('isFormInvalid', !isValid);
},
onTypeChange(path, value) {
if (path === 'type') {
Expand Down
8 changes: 6 additions & 2 deletions ui/app/components/secret-edit-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export default class SecretEditMetadata extends Component {
if (value) {
if (name === 'customMetadata') {
// cp validations won't work on an object so performing validations here
// JLR TODO: review this and incorporate into model-validations system
/* eslint-disable no-useless-escape */
let regex = /^[^\\]+$/g; // looking for a backward slash
value.match(regex)
Expand All @@ -62,9 +63,12 @@ export default class SecretEditMetadata extends Component {
}
if (name === 'maxVersions') {
this.args.model.maxVersions = value;
this.args.model.validations.attrs.maxVersions.isValid
const {
state: { maxVersions },
} = this.args.model.validate();
maxVersions.isValid
? set(this.validationMessages, name, '')
: set(this.validationMessages, name, this.args.model.validations.attrs.maxVersions.message);
: set(this.validationMessages, name, maxVersions.errors.join('. '));
}
}

Expand Down
58 changes: 58 additions & 0 deletions ui/app/decorators/model-validations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/* eslint-disable no-console */
import validators from 'vault/utils/validators';

export function withModelValidations(validations) {
return function decorator(SuperClass) {
return class ModelValidations extends SuperClass {
static _validations;

constructor() {
super(...arguments);
if (!validations || typeof validations !== 'object') {
throw new Error('Validations object must be provided to constructor for setup');
}
this._validations = validations;
}

validate() {
let isValid = true;
const state = {};

for (const key in this._validations) {
const rules = this._validations[key];

if (!Array.isArray(rules)) {
console.error(
`Must provide validations as an array for property "${key}" on ${this.modelName} model`
);
continue;
}

state[key] = { errors: [] };

for (const rule of rules) {
const { type, options, message } = rule;
if (!validators[type]) {
console.error(
`Validator type: "${type}" not found. Available validators: ${Object.keys(validators).join(
', '
)}`
);
continue;
}
if (!validators[type](this[key], options)) {
// consider setting a prop like validationErrors directly on the model
// for now return an errors object
state[key].errors.push(message);
if (isValid) {
isValid = false;
}
}
}
state[key].isValid = !state[key].errors.length;
}
return { isValid, state };
}
};
};
}
22 changes: 11 additions & 11 deletions ui/app/models/auth-method.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import Model, { hasMany, attr } from '@ember-data/model';
import { alias } from '@ember/object/computed';
import { computed } from '@ember/object';
import { alias } from '@ember/object/computed'; // eslint-disable-line
import { computed } from '@ember/object'; // eslint-disable-line
import { fragment } from 'ember-data-model-fragments/attributes';
import fieldToAttrs, { expandAttributeMeta } from 'vault/utils/field-to-attrs';
import { memberAction } from 'ember-api-actions';
import { validator, buildValidations } from 'ember-cp-validations';

import apiPath from 'vault/utils/api-path';
import attachCapabilities from 'vault/lib/attach-capabilities';
import { withModelValidations } from 'vault/decorators/model-validations';

const Validations = buildValidations({
path: validator('presence', {
presence: true,
message: "Path can't be blank.",
}),
});
const validations = {
path: [{ type: 'presence', message: "Path can't be blank." }],
};

let ModelExport = Model.extend(Validations, {
// unsure if ember-api-actions will work on native JS class model
// for now create class to use validations and then use classic extend pattern
@withModelValidations(validations)
class AuthMethodModel extends Model {}
const ModelExport = AuthMethodModel.extend({
authConfigs: hasMany('auth-config', { polymorphic: true, inverse: 'backend', async: false }),
path: attr('string'),
accessor: attr('string'),
Expand Down
31 changes: 11 additions & 20 deletions ui/app/models/secret-engine.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,25 @@
import Model, { attr } from '@ember-data/model';
import { computed } from '@ember/object';
import { equal } from '@ember/object/computed';
import { computed } from '@ember/object'; // eslint-disable-line
import { equal } from '@ember/object/computed'; // eslint-disable-line
import { fragment } from 'ember-data-model-fragments/attributes';
import fieldToAttrs, { expandAttributeMeta } from 'vault/utils/field-to-attrs';
import { validator, buildValidations } from 'ember-cp-validations';
import { withModelValidations } from 'vault/decorators/model-validations';

// identity will be managed separately and the inclusion
// of the system backend is an implementation detail
const LIST_EXCLUDED_BACKENDS = ['system', 'identity'];

const Validations = buildValidations({
path: validator('presence', {
presence: true,
message: "Path can't be blank.",
}),
const validations = {
path: [{ type: 'presence', message: "Path can't be blank." }],
maxVersions: [
validator('number', {
allowString: true,
integer: true,
message: 'Maximum versions must be a number.',
}),
validator('length', {
min: 1,
max: 16,
message: 'You cannot go over 16 characters.',
}),
{ type: 'number', options: { asString: true }, message: 'Maximum versions must be a number.' },
{ type: 'length', options: { min: 1, max: 16 }, message: 'You cannot go over 16 characters.' },
],
});
};

export default Model.extend(Validations, {
@withModelValidations(validations)
class SecretEngineModel extends Model {}
export default SecretEngineModel.extend({
path: attr('string'),
accessor: attr('string'),
name: attr('string'),
Expand Down
26 changes: 10 additions & 16 deletions ui/app/models/secret-v2.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,21 @@
import Model, { belongsTo, hasMany, attr } from '@ember-data/model';
import { computed } from '@ember/object';
import { alias } from '@ember/object/computed';
import { computed } from '@ember/object'; // eslint-disable-line
import { alias } from '@ember/object/computed'; // eslint-disable-line
import { expandAttributeMeta } from 'vault/utils/field-to-attrs';
import KeyMixin from 'vault/mixins/key-mixin';
import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities';
import { validator, buildValidations } from 'ember-cp-validations';
import { withModelValidations } from 'vault/decorators/model-validations';

const Validations = buildValidations({
const validations = {
maxVersions: [
validator('number', {
allowString: true,
integer: true,
message: 'Maximum versions must be a number.',
}),
validator('length', {
min: 1,
max: 16,
message: 'You cannot go over 16 characters.',
}),
{ type: 'number', options: { asString: true }, message: 'Maximum versions must be a number.' },
{ type: 'length', options: { min: 1, max: 16 }, message: 'You cannot go over 16 characters.' },
],
});
};

export default Model.extend(KeyMixin, Validations, {
@withModelValidations(validations)
class SecretV2Model extends Model {}
export default SecretV2Model.extend(KeyMixin, {
failedServerRead: attr('boolean'),
engine: belongsTo('secret-engine', { async: false }),
engineId: attr('string'),
Expand Down
16 changes: 13 additions & 3 deletions ui/app/services/path-help.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { resolve, reject } from 'rsvp';
import { debug } from '@ember/debug';
import { dasherize, capitalize } from '@ember/string';
import { singularize } from 'ember-inflector';
import buildValidations from 'vault/utils/build-api-validators';
import { withModelValidations } from 'vault/decorators/model-validations';

import generatedItemAdapter from 'vault/adapters/generated-item-list';
export function sanitizePath(path) {
Expand All @@ -36,6 +36,7 @@ export default Service.extend({
getNewModel(modelType, backend, apiPath, itemType) {
let owner = getOwner(this);
const modelName = `model:${modelType}`;

const modelFactory = owner.factoryFor(modelName);
let newModel, helpUrl;
// if we have a factory, we need to take the existing model into account
Expand Down Expand Up @@ -298,8 +299,17 @@ export default Service.extend({
// Build and add validations on model
// NOTE: For initial phase, initialize validations only for user pass auth
if (backend === 'userpass') {
let validations = buildValidations(fieldGroups);
newModel = newModel.extend(validations);
const validations = fieldGroups.reduce((obj, element) => {
if (element.default) {
element.default.forEach((v) => {
obj[v.name] = [{ type: 'presence', message: `${v.name} can't be black` }];
});
}
return obj;
}, {});
@withModelValidations(validations)
class GeneratedItemModel extends newModel {}
newModel = GeneratedItemModel;
}
}
} catch (err) {
Expand Down
30 changes: 0 additions & 30 deletions ui/app/utils/build-api-validators.js

This file was deleted.

23 changes: 23 additions & 0 deletions ui/app/utils/validators.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { isPresent } from '@ember/utils';

export const presence = (value) => isPresent(value);

export const length = (value, { nullable = false, min, max } = {}) => {
let isValid = nullable;
if (typeof value === 'string') {
const underMin = min && value.length < min;
const overMax = max && value.length > max;
isValid = underMin || overMax ? false : true;
}
return isValid;
};

export const number = (value, { nullable = false, asString } = {}) => {
if (!value) return nullable;
if (typeof value === 'string' && !asString) {
return false;
}
return !isNaN(value);
};

export default { presence, length, number };
1 change: 0 additions & 1 deletion ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@
"ember-composable-helpers": "^4.3.0",
"ember-concurrency": "^2.1.2",
"ember-copy": "2.0.1",
"ember-cp-validations": "^4.0.0-beta.12",
"ember-d3": "^0.5.1",
"ember-data": "~3.28.6",
"ember-data-model-fragments": "5.0.0-beta.5",
Expand Down
15 changes: 5 additions & 10 deletions ui/tests/acceptance/secrets/backend/kv/secret-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,16 +213,11 @@ module('Acceptance | secrets/secret/create', function (hooks) {

await editPage.toggleMetadata();
await settled();
/* TODO
* commenting out for now until ember-cp-validations is updated or removed
* throws an error when attempting to use isHTMLSafe which is imported from @ember/string in the cp-validations code
* in ember 3.28 the import changed to @ember/template
*/
// await typeIn('[data-test-input="maxVersions"]', 'abc');
// assert
// .dom('[data-test-input="maxVersions"]')
// .hasClass('has-error-border', 'shows border error on input with error');
// assert.dom('[data-test-secret-save]').isDisabled('Save button is disabled');
await typeIn('[data-test-input="maxVersions"]', 'abc');
assert
.dom('[data-test-input="maxVersions"]')
.hasClass('has-error-border', 'shows border error on input with error');
assert.dom('[data-test-secret-save]').isDisabled('Save button is disabled');
await fillIn('[data-test-input="maxVersions"]', 20); // fillIn replaces the text, whereas typeIn only adds to it.
await triggerKeyEvent('[data-test-input="maxVersions"]', 'keyup', 65);
await editPage.path(secretPath);
Expand Down
Loading

0 comments on commit 4f8eeab

Please sign in to comment.