Skip to content

Commit

Permalink
Merge pull request #5413 from emberjs/fix-create-record-init
Browse files Browse the repository at this point in the history
[BUGFIX] Fix availability of properties in createRecord init
  • Loading branch information
runspired committed Apr 4, 2018
2 parents e82e20f + de55378 commit 7b5a885
Show file tree
Hide file tree
Showing 14 changed files with 504 additions and 118 deletions.
3 changes: 3 additions & 0 deletions addon/-private/system/many-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ export default EmberObject.extend(MutableArray, Evented, {
},

objectAt(index) {
if (this.relationship._willUpdateManyArray) {
this.relationship._flushPendingManyArrayUpdates();
}
let internalModel = this.currentState[index];
if (internalModel === undefined) { return; }

Expand Down
106 changes: 105 additions & 1 deletion addon/-private/system/model/internal-model.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { assign, merge } from '@ember/polyfills';
import { A } from '@ember/array';
import { set, get } from '@ember/object';
import { copy } from '@ember/object/internals';
import EmberError from '@ember/error';
Expand All @@ -13,6 +14,7 @@ import RootState from "./states";
import Relationships from "../relationships/state/create";
import Snapshot from "../snapshot";
import OrderedSet from "../ordered-set";
import isArrayLike from "../is-array-like";

import { getOwner } from '../../utils';

Expand Down Expand Up @@ -334,7 +336,7 @@ export default class InternalModel {
return this.currentState.dirtyType;
}

getRecord() {
getRecord(properties) {
if (!this._record && !this._isDematerializing) {
heimdall.increment(materializeRecord);
let token = heimdall.start('InternalModel.getRecord');
Expand All @@ -349,6 +351,40 @@ export default class InternalModel {
adapterError: this.error
};

if (properties !== undefined) {
assert(`You passed '${properties}' as properties for record creation instead of an object.`, typeof properties === 'object' && properties !== null);
let classFields = this.getFields();
let relationships = this._relationships;
let propertyNames = Object.keys(properties);

for (let i = 0; i < propertyNames.length; i++) {
let name = propertyNames[i];
let fieldType = classFields.get(name);
let propertyValue = properties[name];

if (name === 'id') {
this.setId(propertyValue);
continue;
}

switch (fieldType) {
case 'attribute':
this.setDirtyAttribute(name, propertyValue);
break;
case 'belongsTo':
this.setDirtyBelongsTo(name, propertyValue);
relationships.get(name).setHasData(true);
break;
case 'hasMany':
this.setDirtyHasMany(name, propertyValue);
relationships.get(name).setHasData(true);
break;
default:
createOptions[name] = propertyValue;
}
}
}

if (setOwner) {
// ensure that `getOwner(this)` works inside a model instance
setOwner(createOptions, getOwner(this.store));
Expand All @@ -365,6 +401,10 @@ export default class InternalModel {
return this._record;
}

getFields() {
return get(this.modelClass, 'fields');
}

resetRecord() {
this._record = null;
this.isReloading = false;
Expand Down Expand Up @@ -602,6 +642,70 @@ export default class InternalModel {
}
}

getAttributeValue(key) {
if (key in this._attributes) {
return this._attributes[key];
} else if (key in this._inFlightAttributes) {
return this._inFlightAttributes[key];
} else {
return this._data[key];
}
}

setDirtyHasMany(key, records) {
assert(`You must pass an array of records to set a hasMany relationship`, isArrayLike(records));
assert(`All elements of a hasMany relationship must be instances of DS.Model, you passed ${inspect(records)}`, (function() {
return A(records).every((record) => record.hasOwnProperty('_internalModel') === true);
})());

let relationship = this._relationships.get(key);
relationship.clear();
relationship.addInternalModels(records.map(record => get(record, '_internalModel')));
}

setDirtyBelongsTo(key, value) {
if (value === undefined) {
value = null;
}
if (value && value.then) {
this._relationships.get(key).setRecordPromise(value);
} else if (value) {
this._relationships.get(key).setInternalModel(value._internalModel);
} else {
this._relationships.get(key).setInternalModel(value);
}
}

setDirtyAttribute(key, value) {
if (this.isDeleted()) {
throw new EmberError(`Attempted to set '${key}' to '${value}' on the deleted record ${this}`);
}

let oldValue = this.getAttributeValue(key);
let originalValue;

if (value !== oldValue) {
// Add the new value to the changed attributes hash; it will get deleted by
// the 'didSetProperty' handler if it is no different from the original value
this._attributes[key] = value;

if (key in this._inFlightAttributes) {
originalValue = this._inFlightAttributes[key];
} else {
originalValue = this._data[key];
}

this.send('didSetProperty', {
name: key,
oldValue: oldValue,
originalValue: originalValue,
value: value
});
}

return value;
}

get isDestroyed() {
return this._isDestroyed;
}
Expand Down
11 changes: 1 addition & 10 deletions addon/-private/system/relationships/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,7 @@ export default function belongsTo(modelName, options) {
return this._internalModel._relationships.get(key).getRecord();
},
set(key, value) {
if (value === undefined) {
value = null;
}
if (value && value.then) {
this._internalModel._relationships.get(key).setRecordPromise(value);
} else if (value) {
this._internalModel._relationships.get(key).setInternalModel(value._internalModel);
} else {
this._internalModel._relationships.get(key).setInternalModel(value);
}
this._internalModel.setDirtyBelongsTo(key, value);

return this._internalModel._relationships.get(key).getRecord();
}
Expand Down
18 changes: 4 additions & 14 deletions addon/-private/system/relationships/has-many.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
/**
@module ember-data
*/

import { A } from '@ember/array';

import { get, computed } from '@ember/object';
import { computed } from '@ember/object';
import { assert, inspect } from '@ember/debug';
import normalizeModelName from "../normalize-model-name";
import isArrayLike from "../is-array-like";

/**
`DS.hasMany` is used to define One-To-Many and Many-To-Many
Expand Down Expand Up @@ -147,15 +143,9 @@ export default function hasMany(type, options) {
return this._internalModel._relationships.get(key).getRecords();
},
set(key, records) {
assert(`You must pass an array of records to set a hasMany relationship`, isArrayLike(records));
assert(`All elements of a hasMany relationship must be instances of DS.Model, you passed ${inspect(records)}`, (function() {
return A(records).every((record) => record.hasOwnProperty('_internalModel') === true);
})());

let relationship = this._internalModel._relationships.get(key);
relationship.clear();
relationship.addInternalModels(records.map(record => get(record, '_internalModel')));
return relationship.getRecords();
this._internalModel.setDirtyHasMany(key, records);

return this._internalModel._relationships.get(key).getRecords();
}
}).meta(meta);
}
44 changes: 42 additions & 2 deletions addon/-private/system/relationships/state/has-many.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export default class ManyRelationship extends Relationship {
// inverse internal models are unloaded.
this._retainedManyArray = null;
this.__loadingPromise = null;
this._willUpdateManyArray = false;
this._pendingManyArrayUpdates = null;
}

get _loadingPromise() { return this.__loadingPromise; }
Expand Down Expand Up @@ -109,8 +111,46 @@ export default class ManyRelationship extends Relationship {

assertPolymorphicType(this.internalModel, this.relationshipMeta, internalModel);
super.addInternalModel(internalModel, idx);
// make lazy later
this.manyArray._addInternalModels([internalModel], idx);
this.scheduleManyArrayUpdate(internalModel, idx);
}

scheduleManyArrayUpdate(internalModel, idx) {
// ideally we would early exit here, but some tests
// currently suggest that we cannot.
// if (!this._manyArray) {
// return;
// }

let pending = this._pendingManyArrayUpdates = this._pendingManyArrayUpdates || [];
pending.push(internalModel, idx);

if (this._willUpdateManyArray === true) {
return;
}

this._willUpdateManyArray = true;
let backburner = this.store._backburner;

backburner.join(() => {
backburner.schedule('syncRelationships', this, this._flushPendingManyArrayUpdates);
});
}

_flushPendingManyArrayUpdates() {
if (this._willUpdateManyArray === false) {
return;
}

let pending = this._pendingManyArrayUpdates;
this._pendingManyArrayUpdates = [];
this._willUpdateManyArray = false;

for (let i = 0; i < pending.length; i += 2) {
let internalModel = pending[i];
let idx = pending[i + 1];

this.manyArray._addInternalModels([internalModel], idx);
}
}

removeCanonicalInternalModelFromOwn(internalModel, idx) {
Expand Down
43 changes: 24 additions & 19 deletions addon/-private/system/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,13 @@ export default class Snapshot {
this._hasManyIds = Object.create(null);
this._internalModel = internalModel;

let record = internalModel.getRecord();
// TODO is there a way we can assign known attributes without
// using `eachAttribute`? This forces us to lookup the model-class
// but for findRecord / findAll these are empty and doing so at
// this point in time is unnecessary.
internalModel.eachAttribute((keyName) => this._attributes[keyName] = internalModel.getAttributeValue(keyName));

/**
The underlying record for this snapshot. Can be used to access methods and
properties defined on the record.
Example
```javascript
let json = snapshot.record.toJSON();
```
@property record
@type {DS.Model}
*/
this.record = record;
record.eachAttribute((keyName) => this._attributes[keyName] = get(record, keyName));

/**
/**O
The id of the snapshot's underlying record
Example
Expand Down Expand Up @@ -73,7 +61,24 @@ export default class Snapshot {
*/
this.modelName = internalModel.modelName;

this._changedAttributes = record.changedAttributes();
this._changedAttributes = internalModel.changedAttributes();
}

/**
The underlying record for this snapshot. Can be used to access methods and
properties defined on the record.
Example
```javascript
let json = snapshot.record.toJSON();
```
@property record
@type {DS.Model}
*/
get record() {
return this._internalModel.getRecord();
}

/**
Expand Down
37 changes: 15 additions & 22 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,34 +339,27 @@ Store = Service.extend({
createRecord(modelName, inputProperties) {
assert(`You need to pass a model name to the store's createRecord method`, isPresent(modelName));
assert(`Passing classes to store methods has been removed. Please pass a dasherized string instead of ${modelName}`, typeof modelName === 'string');
let normalizedModelName = normalizeModelName(modelName);
let properties = copy(inputProperties) || Object.create(null);

// If the passed properties do not include a primary key,
// give the adapter an opportunity to generate one. Typically,
// client-side ID generators will use something like uuid.js
// to avoid conflicts.
return this._backburner.join(() => {
let normalizedModelName = normalizeModelName(modelName);
let properties = copy(inputProperties) || Object.create(null);

if (isNone(properties.id)) {
properties.id = this._generateId(normalizedModelName, properties);
}
// If the passed properties do not include a primary key,
// give the adapter an opportunity to generate one. Typically,
// client-side ID generators will use something like uuid.js
// to avoid conflicts.

// Coerce ID to a string
properties.id = coerceId(properties.id);
if (isNone(properties.id)) {
properties.id = this._generateId(normalizedModelName, properties);
}

let internalModel = this._buildInternalModel(normalizedModelName, properties.id);
internalModel.loadedData();
let record = internalModel.getRecord();
record.setProperties(properties);
// Coerce ID to a string
properties.id = coerceId(properties.id);

// TODO @runspired this should also be coalesced into some form of internalModel.setState()
internalModel.eachRelationship((key, descriptor) => {
if (properties[key] !== undefined) {
internalModel._relationships.get(key).setHasData(true);
}
let internalModel = this._buildInternalModel(normalizedModelName, properties.id);
internalModel.loadedData();
return internalModel.getRecord(properties);
});

return record;
},

/**
Expand Down
Loading

0 comments on commit 7b5a885

Please sign in to comment.