Skip to content

Commit

Permalink
fixes #750
Browse files Browse the repository at this point in the history
  • Loading branch information
koskimas committed Jan 23, 2018
1 parent 5451ff4 commit f305d88
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 13 deletions.
8 changes: 4 additions & 4 deletions lib/model/modelSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@ function setJson(model, json, options) {
}

json = model.$parseJson(json, options);
const originalJson = json;
json = model.$validate(json, options);

model.$set(json);

if (!options.skipParseRelations) {
parseRelationsIntoModelInstances(model, originalJson, options);
parseRelationsIntoModelInstances(model, json, options);
}

return model;
Expand Down Expand Up @@ -101,8 +100,9 @@ function setDatabaseJson(model, json) {

function setFast(model, obj) {
if (obj) {
// Don't try to set read-only virtual properties. They can easily get here through `fromJson`
// when parsing an object that was previously serialized from a model instance.
// Don't try to set read-only virtual properties. They can easily get here
// through `fromJson` when parsing an object that was previously serialized
// from a model instance.
const readOnlyVirtuals = model.constructor.getReadOnlyVirtualAttributes();
const keys = Object.keys(obj);

Expand Down
24 changes: 15 additions & 9 deletions lib/model/modelValidate.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
const clone = require('./modelClone').clone;
const { defineNonEnumerableProperty } = require('./modelUtils');

function validate(model, json, options) {
function validate(model, json, options = {}) {
json = json || model;
options = options || {};
const inputJson = json;
const validatingModelInstance = inputJson && inputJson.$isObjectionModel;

if (json && json.$isObjectionModel) {
if (options.skipValidation) {
return json;
}

if (validatingModelInstance) {
// Strip away relations and other internal stuff.
json = clone(json, true, true);
// We can mutate `json` now that we took a copy of it.
options.mutable = true;
}

if (options.skipValidation) {
return json;
options = Object.assign({}, options, { mutable: true });
}

const ModelClass = model.constructor;
Expand All @@ -29,7 +30,12 @@ function validate(model, json, options) {
json = validator.validate(args);
validator.afterValidate(args);

return json;
if (validatingModelInstance) {
// If we cloned `json`, we need to copy the possible default values.
return inputJson.$set(json);
} else {
return json;
}
}

module.exports = {
Expand Down
25 changes: 25 additions & 0 deletions tests/unit/model/Model.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,31 @@ describe('Model', () => {
expect(model.c).not.to.equal(obj);
});

it('should merge default values from jsonSchema when validating a model instance', () => {
let obj = { a: 100, b: 200 };

Model1.jsonSchema = {
required: ['a'],
properties: {
a: { type: 'string', default: 'default string' },
b: { type: 'number', default: 666 },
c: { type: 'object', default: obj }
}
};

let model = Model1.fromJson({ a: 'str' }, { skipValidation: true });

expect(model.b).to.equal(undefined);
expect(model.c).to.equal(undefined);

model.$validate();

expect(model.a).to.equal('str');
expect(model.b).to.equal(666);
expect(model.c).to.eql(obj);
expect(model.c).not.to.equal(obj);
});

// regression introduced in 0.6
// https://github.com/Vincit/objection.js/issues/205
it('should not throw TypeError when jsonSchema.properties == undefined', () => {
Expand Down

0 comments on commit f305d88

Please sign in to comment.