From 75d0f50d5d00005ef3fb661c7da04dcbff4fc81a Mon Sep 17 00:00:00 2001 From: Aaron Heesakkers Date: Tue, 30 Aug 2016 16:25:19 +0200 Subject: [PATCH 1/2] Use related-proxy's metadata to determine types/urls for related-operations. --- addon/adapters/application.js | 12 ++- addon/models/resource.js | 13 ++- fixtures/api/employees.json | 48 +++++++++ fixtures/api/employees/1.json | 19 ++++ fixtures/api/supervisors/2.json | 20 ++++ tests/dummy/app/models/employee.js | 2 +- tests/dummy/app/models/supervisor.js | 2 +- tests/unit/adapters/application-test.js | 134 ++++++++++++++++-------- tests/unit/models/resource-test.js | 11 ++ 9 files changed, 209 insertions(+), 52 deletions(-) create mode 100644 fixtures/api/employees.json create mode 100644 fixtures/api/employees/1.json create mode 100644 fixtures/api/supervisors/2.json diff --git a/addon/adapters/application.js b/addon/adapters/application.js index 720d92f..bbabe6f 100644 --- a/addon/adapters/application.js +++ b/addon/adapters/application.js @@ -261,10 +261,11 @@ export default Ember.Object.extend(FetchMixin, Evented, { @return {Promise} */ patchRelationship(resource, relationship) { + let meta = resource.constructor.metaForProperty(relationship); return this.fetch(this._urlForRelationship(resource, relationship), { method: 'PATCH', body: JSON.stringify({ - data: resource.get(['relationships', relationship, 'data'].join('.')) + data: resource.get(['relationships', meta.relation, 'data'].join('.')) }) }); }, @@ -307,7 +308,8 @@ export default Ember.Object.extend(FetchMixin, Evented, { @return {String} url */ _urlForRelationship(resource, relationship) { - let url = resource.get(['relationships', relationship, 'links', 'self'].join('.')); + let meta = resource.constructor.metaForProperty(relationship); + let url = resource.get(['relationships', meta.relation, 'links', 'self'].join('.')); return url || [this.get('url'), resource.get('id'), 'relationships', relationship].join('/'); }, @@ -320,8 +322,10 @@ export default Ember.Object.extend(FetchMixin, Evented, { @return {Object} payload */ _payloadForRelationship(resource, relationship, id) { - let data = resource.get(['relationships', relationship, 'data'].join('.')); - let resourceObject = { type: pluralize(relationship), id: id.toString() }; + // actual resource type of this relationship is found in related-proxy's meta. + let meta = resource.constructor.metaForProperty(relationship); + let data = resource.get(['relationships', meta.relation, 'data'].join('.')); + let resourceObject = { type: pluralize(meta.type), id: id.toString() }; return { data: (Array.isArray(data)) ? [resourceObject] : resourceObject }; }, diff --git a/addon/models/resource.js b/addon/models/resource.js index 73f97bd..64ea8d2 100644 --- a/addon/models/resource.js +++ b/addon/models/resource.js @@ -190,9 +190,12 @@ const Resource = Ember.Object.extend(ResourceOperationsMixin, { */ addRelationship(related, id) { if (id !== undefined) { id = id.toString(); } // ensure String id. - let key = ['relationships', related, 'data'].join('.'); + + // actual resource type of this relationship is found in related-proxy's meta. + let meta = this.constructor.metaForProperty(related); + let key = ['relationships', meta.relation, 'data'].join('.'); let data = this.get(key); - let type = pluralize(related); + let type = pluralize(meta.type); let identifier = { type: type, id: id }; let owner = (typeof getOwner === 'function') ? getOwner(this) : this.container; let resource = owner.lookup(`service:${type}`).cacheLookup(id); @@ -207,7 +210,7 @@ const Resource = Ember.Object.extend(ResourceOperationsMixin, { } else { data = identifier; if (resource) { - this.set(`${related}.content`, resource); + this.set(`${meta.relation}.content`, resource); } } return this.set(key, data); @@ -463,9 +466,9 @@ function useComputedPropsMetaToSetupRelationships(owner, factory, instance) { let meta = factory.metaForProperty(prop); if (meta && meta.kind) { if (meta.kind === 'hasOne') { - setupRelationship.call(instance, prop); + setupRelationship.call(instance, meta.relation); } else if (meta.kind === 'hasMany') { - setupRelationship.call(instance, prop, Ember.A([])); + setupRelationship.call(instance, meta.relation, Ember.A([])); } } } catch (e) { diff --git a/fixtures/api/employees.json b/fixtures/api/employees.json new file mode 100644 index 0000000..23c075d --- /dev/null +++ b/fixtures/api/employees.json @@ -0,0 +1,48 @@ +{ + "data": [ + { + "type": "employees", + "id": "1", + "links": { + "self": "http://api.pixelhandler.com/api/v1/employees/1" + }, + "attributes": { + "name": "The Special" + }, + "relationships": { + "supervisor": { + "links": { + "self": "http://api.pixelhandler.com/api/v1/employees/1/relationships/supervisor", + "related": "http://api.pixelhandler.com/api/v1/employees/1/supervisor" + } + } + } + }, + { + "type": "supervisors", + "id": "2", + "links": { + "self": "http://api.pixelhandler.com/api/v1/supervisors/2" + }, + "attributes": { + "name": "The Boss" + }, + "relationships": { + "direct-reports": { + "links": { + "self": "http://api.pixelhandler.com/api/v1/supervisors/2/relationships/direct-reports", + "related": "http://api.pixelhandler.com/api/v1/supervisors/2/direct-reports" + } + } + } + } + ], + "meta": { + "page": { + "sort": "-date", + "total": 2, + "limit": "5", + "offset": "0" + } + } +} diff --git a/fixtures/api/employees/1.json b/fixtures/api/employees/1.json new file mode 100644 index 0000000..e55782c --- /dev/null +++ b/fixtures/api/employees/1.json @@ -0,0 +1,19 @@ +{ + "data": { + "type": "employees", + "id": "1", + "links": { + "self": "http://api.pixelhandler.com/api/v1/employees/1" + }, + "attributes": { + "name": "The Special" + }, + "relationships": { + "supervisor": { + "links": { + "related": "http://api.pixelhandler.com/api/v1/employees/1/supervisor" + } + } + } + } +} diff --git a/fixtures/api/supervisors/2.json b/fixtures/api/supervisors/2.json new file mode 100644 index 0000000..672730a --- /dev/null +++ b/fixtures/api/supervisors/2.json @@ -0,0 +1,20 @@ +{ + "data": { + "type": "supervisors", + "id": "2", + "links": { + "self": "http://api.pixelhandler.com/api/v1/supervisors/2" + }, + "attributes": { + "name": "The Boss" + }, + "relationships": { + "direct-reports": { + "links": { + "self": "http://api.pixelhandler.com/api/v1/supervisors/2/relationships/direct-reports", + "related": "http://api.pixelhandler.com/api/v1/supervisors/2/direct-reports" + } + } + } + } +} diff --git a/tests/dummy/app/models/employee.js b/tests/dummy/app/models/employee.js index a24bfee..90a724d 100644 --- a/tests/dummy/app/models/employee.js +++ b/tests/dummy/app/models/employee.js @@ -4,5 +4,5 @@ import { hasOne, hasMany } from 'ember-jsonapi-resources/models/resource'; export default PersonResource.extend({ type: 'employees', pictures: hasMany('pictures'), - supervisor: hasOne({resource: 'supervisor', type: 'employees'}) + supervisor: hasOne('supervisor') }); diff --git a/tests/dummy/app/models/supervisor.js b/tests/dummy/app/models/supervisor.js index 3291a60..b794f44 100644 --- a/tests/dummy/app/models/supervisor.js +++ b/tests/dummy/app/models/supervisor.js @@ -3,5 +3,5 @@ import { hasMany } from 'ember-jsonapi-resources/models/resource'; export default EmployeeResource.extend({ type: 'supervisors', - directReports: hasMany('employees') + directReports: hasMany({resource: 'direct-reports', type: 'employees'}) }); diff --git a/tests/unit/adapters/application-test.js b/tests/unit/adapters/application-test.js index 63c1852..079cbc8 100644 --- a/tests/unit/adapters/application-test.js +++ b/tests/unit/adapters/application-test.js @@ -7,6 +7,8 @@ import { setup, teardown, mockServices } from 'dummy/tests/helpers/resources'; import postMock from 'fixtures/api/posts/1'; import postsMock from 'fixtures/api/posts'; import authorMock from 'fixtures/api/authors/1'; +import employeeMock from 'fixtures/api/employees/1'; +import supervisorMock from 'fixtures/api/supervisors/2'; let sandbox; @@ -224,51 +226,27 @@ test('#findRelated', function(assert) { test('#findRelated is called with optional type for the resource', function (assert) { assert.expect(4); const done = assert.async(); - let supervisor = this.container.lookup('model:supervisor').create({ - type: 'supervisors', - id: '1000000', - attributes: { - name: 'The Boss', - }, - relationships: { - employees: { - links: { - related: 'http://api.pixelhandler.com/api/v1/supervisors/1000000/employees' - } - } - } - }); - let EmployeeAdapter = Adapter.extend({type: 'employees', url: '/employees'}); - this.registry.register('service:supervisors', EmployeeAdapter.extend({ - cacheLookup: function() { - return supervisor; - } + let supervisor = this.container.lookup('model:supervisor').create(supervisorMock.data); + let employee = this.container.lookup('model:employee').create(employeeMock.data); + + let SupervisorAdapter = Adapter.extend({type: 'supervisors', url: '/supervisors'}); + SupervisorAdapter.reopenClass({isServiceFactory: true}); + let EmployeeAdapter = Adapter.extend({type: 'employees', url: '/employees'}); + EmployeeAdapter.reopenClass({isServiceFactory: true}); + + this.registry.register('service:employees', EmployeeAdapter.extend({ + cacheLookup: function () { return employee; } })); - EmployeeAdapter.reopenClass({ isServiceFactory: true }); - this.registry.register('service:employees', EmployeeAdapter.extend()); - let service = this.container.lookup('service:employees'); - let stub = sandbox.stub(service, 'findRelated', function () { - return RSVP.Promise.resolve(supervisor); - }); - let employee = this.container.lookup('model:employee').create({ - type: 'employees', - id: '1000001', - attributes: { - name: 'The Special', - }, - relationships: { - supervisor: { - links: { - related: 'http://api.pixelhandler.com/api/v1/employees/1000001/supervisor' - } - } - } + let employeeService = this.container.lookup('service:employees'); + let stub = sandbox.stub(employeeService, 'findRelated', function () { + return RSVP.Promise.resolve(null); }); - let url = employee.get('relationships.supervisor.links.related'); - employee.get('supervisor').then(() => { + + let url = supervisor.get('relationships.direct-reports.links.related'); + supervisor.get('directReports').then(() => { assert.ok(stub.calledOnce, 'employees service findRelated method called once'); - assert.equal(stub.firstCall.args[0].resource, 'supervisor', 'findRelated called with supervisor resource'); + assert.equal(stub.firstCall.args[0].resource, 'direct-reports', 'findRelated called with "direct-reports" resource'); assert.equal(stub.firstCall.args[0].type, 'employees', 'findRelated called with employees type'); assert.equal(stub.firstCall.args[1], url, 'findRelated called with url, ' + url); done(); @@ -404,6 +382,30 @@ test('#createRelationship (to-one)', function(assert) { }); }); +test('#createRelationship uses optional resource type', function (assert) { + assert.expect(2); + const done = assert.async(); + + mockServices.call(this); + let adapter = this.subject({type: 'supervisors', url: '/supervisors'}); + sandbox.stub(adapter, 'fetch', function () { return RSVP.Promise.resolve(null); }); + let resource = this.container.lookup('model:supervisor').create(supervisorMock.data); + let promise = adapter.createRelationship(resource, 'directReports', '1'); + + assert.ok(typeof promise.then === 'function', 'returns a thenable'); + promise.then(() => { + let jsonBody = JSON.stringify({data: [{type: 'employees', id: '1'}]}); + assert.ok( + adapter.fetch.calledWith( + supervisorMock.data.relationships['direct-reports'].links.self, + {method: 'POST', body: jsonBody} + ), + '#fetch called with url and options with data' + ); + done(); + }); +}); + test('#deleteRelationship (to-many)', function(assert) { assert.expect(2); const done = assert.async(); @@ -452,6 +454,30 @@ test('#deleteRelationship (to-one)', function(assert) { }); }); +test('#deleteRelationship uses optional resource type', function (assert) { + assert.expect(2); + const done = assert.async(); + + mockServices.call(this); + let adapter = this.subject({type: 'supervisors', url: '/supervisors'}); + sandbox.stub(adapter, 'fetch', function () { return RSVP.Promise.resolve(null); }); + let resource = this.container.lookup('model:supervisor').create(supervisorMock.data); + let promise = adapter.deleteRelationship(resource, 'directReports', '1'); + + assert.ok(typeof promise.then === 'function', 'returns a thenable'); + promise.then(() => { + let jsonBody = JSON.stringify({data: [{type: 'employees', id: '1'}]}); + assert.ok( + adapter.fetch.calledWith( + supervisorMock.data.relationships['direct-reports'].links.self, + {method: 'DELETE', body: jsonBody} + ), + '#fetch called with url and options with data' + ); + done(); + }); +}); + test('#patchRelationship (to-many)', function(assert) { assert.expect(2); const done = assert.async(); @@ -502,6 +528,32 @@ test('#patchRelationship (to-one)', function(assert) { }); }); +test('#patchRelationship uses optional resource type', function (assert) { + assert.expect(2); + const done = assert.async(); + + mockServices.call(this); + let adapter = this.subject({type: 'supervisors', url: '/supervisors'}); + sandbox.stub(adapter, 'fetch', function () { return RSVP.Promise.resolve(null); }); + let resource = this.container.lookup('model:supervisor').create(supervisorMock.data); + resource.addRelationship('directReports', '1'); + let promise = adapter.patchRelationship(resource, 'directReports'); + + assert.ok(typeof promise.then === 'function', 'returns a thenable'); + promise.then(() => { + let jsonBody = JSON.stringify({data: [{type: 'employees', id: '1'}]}); + assert.ok( + adapter.fetch.calledWith( + supervisorMock.data.relationships['direct-reports'].links.self, + {method: 'PATCH', body: jsonBody} + ), + '#fetch called with url and options with data' + ); + done(); + }); +}); + + // Even though create- and deleteRelationship both use _payloadForRelationship, // which does the casting of id to String, we test them seperately to ensure this // is always tested, even when internals change. diff --git a/tests/unit/models/resource-test.js b/tests/unit/models/resource-test.js index d6ba535..3477b62 100644 --- a/tests/unit/models/resource-test.js +++ b/tests/unit/models/resource-test.js @@ -41,6 +41,17 @@ test('in creating instances, ids are cast to string', function (assert) { assert.strictEqual(post.get('id'), id.toString(), 'new instance id cast to string'); }); +test('in creating instances, optional resource is used to set up relationships', function (assert) { + assert.expect(2); + + let supervisor = this.container.lookup('model:supervisor').create(); + let meta = supervisor.constructor.metaForProperty('directReports'); + let relationships = supervisor.get('relationships'); + + assert.ok(!relationships.hasOwnProperty('directReports'), 'camelCased relation directReports does not exist'); + assert.ok(relationships.hasOwnProperty(meta.relation), 'relation "direct-reports" exists as defined as optional resource'); +}); + test('#toString method', function(assert) { let resource = this.subject(); let stringified = resource.toString(); From ece1bf0802406363b0eaa901ec5e50f54cc91665 Mon Sep 17 00:00:00 2001 From: Aaron Heesakkers Date: Thu, 1 Sep 2016 11:08:49 +0200 Subject: [PATCH 2/2] uses _payloadFor in patchRelationship as well. Cleanup. --- addon/adapters/application.js | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/addon/adapters/application.js b/addon/adapters/application.js index bbabe6f..f1b096d 100644 --- a/addon/adapters/application.js +++ b/addon/adapters/application.js @@ -123,7 +123,6 @@ export default Ember.Object.extend(FetchMixin, Evented, { findRelated(resource, url) { let type = resource; if (typeof type === 'object') { - resource = resource.resource; type = resource.type; } // use resource's service if in container, otherwise use this service to fetch @@ -142,18 +141,25 @@ export default Ember.Object.extend(FetchMixin, Evented, { @return {Promise} */ createResource(resource) { - let url = this.get('url'); - const json = this.serializer.serialize(resource); - return this.fetch(url, { + return this.fetch(this.get('url'), { method: 'POST', - body: JSON.stringify(json) + body: JSON.stringify(this.serializer.serialize(resource)) }).then(function(resp) { if (resource.toString().match('JSONAPIResource') === null) { return resp; } else { - resource.set('id', resp.get('id') ); - let json = resp.getProperties('attributes', 'relationships', 'links', 'meta', 'type', 'isNew', 'id'); - resource.didUpdateResource(json); + resource.set('id', resp.get('id')); + resource.didUpdateResource( + resp.getProperties( + 'attributes', + 'relationships', + 'links', + 'meta', + 'type', + 'isNew', + 'id' + ) + ); this.cacheUpdate({ data: resource }); return resource; } @@ -261,12 +267,9 @@ export default Ember.Object.extend(FetchMixin, Evented, { @return {Promise} */ patchRelationship(resource, relationship) { - let meta = resource.constructor.metaForProperty(relationship); return this.fetch(this._urlForRelationship(resource, relationship), { method: 'PATCH', - body: JSON.stringify({ - data: resource.get(['relationships', meta.relation, 'data'].join('.')) - }) + body: JSON.stringify(this._payloadForRelationship(resource, relationship)) }); }, @@ -309,7 +312,7 @@ export default Ember.Object.extend(FetchMixin, Evented, { */ _urlForRelationship(resource, relationship) { let meta = resource.constructor.metaForProperty(relationship); - let url = resource.get(['relationships', meta.relation, 'links', 'self'].join('.')); + let url = resource.get(['relationships', meta.relation, 'links', 'self'].join('.')); return url || [this.get('url'), resource.get('id'), 'relationships', relationship].join('/'); }, @@ -318,13 +321,14 @@ export default Ember.Object.extend(FetchMixin, Evented, { @private @param {Resource} resource instance, has URLs via it's relationships property @param {String} relationship name (plural) to find the url from the resource instance - @param {String} id the id for the related resource + @param {String} id the id for the related resource or undefined current relationship data @return {Object} payload */ _payloadForRelationship(resource, relationship, id) { // actual resource type of this relationship is found in related-proxy's meta. let meta = resource.constructor.metaForProperty(relationship); let data = resource.get(['relationships', meta.relation, 'data'].join('.')); + if (id === undefined) { return {data: data}; } let resourceObject = { type: pluralize(meta.type), id: id.toString() }; return { data: (Array.isArray(data)) ? [resourceObject] : resourceObject }; },