From afcfba0910a8e818331217eb45ca6c65e0544f4c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 23 Oct 2017 10:22:58 -0700 Subject: [PATCH] Treat namespace and job name as a composite primary key --- ui/app/adapters/job.js | 34 +++++++++++++++++---------- ui/app/models/job.js | 1 + ui/app/serializers/allocation.js | 12 +++++++++- ui/app/serializers/deployment.js | 8 +++++++ ui/app/serializers/job.js | 9 +++++-- ui/tests/unit/adapters/job-test.js | 30 +++++++++++++++++++---- ui/tests/unit/serializers/job-test.js | 32 +++++++++++++++++++++++-- 7 files changed, 105 insertions(+), 21 deletions(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index 5f193403a..4d18d1625 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -1,7 +1,7 @@ import Ember from 'ember'; import ApplicationAdapter from './application'; -const { RSVP, inject } = Ember; +const { RSVP, inject, assign } = Ember; export default ApplicationAdapter.extend({ system: inject.service(), @@ -9,12 +9,10 @@ export default ApplicationAdapter.extend({ shouldReloadAll: () => true, buildQuery() { - const namespace = this.get('system.activeNamespace'); + const namespace = this.get('system.activeNamespace.id'); - if (namespace) { - return { - namespace: namespace.get('name'), - }; + if (namespace && namespace !== 'default') { + return { namespace }; } }, @@ -22,7 +20,7 @@ export default ApplicationAdapter.extend({ const namespace = this.get('system.activeNamespace'); return this._super(...arguments).then(data => { data.forEach(job => { - job.Namespace = namespace && namespace.get('id'); + job.Namespace = namespace ? namespace.get('id') : 'default'; }); return data; }); @@ -31,11 +29,21 @@ export default ApplicationAdapter.extend({ findRecord(store, { modelName }, id, snapshot) { // To make a findRecord response reflect the findMany response, the JobSummary // from /summary needs to be stitched into the response. + + // URL is the form of /job/:name?namespace=:namespace with arbitrary additional query params + const [name, namespace] = JSON.parse(id); + const namespaceQuery = namespace && namespace !== 'default' ? { namespace } : {}; return RSVP.hash({ - job: this._super(...arguments), - summary: this.ajax(`${this.buildURL(modelName, id, snapshot, 'findRecord')}/summary`, 'GET', { - data: this.buildQuery(), + job: this.ajax(this.buildURL(modelName, name, snapshot, 'findRecord'), 'GET', { + data: assign(this.buildQuery() || {}, namespaceQuery), }), + summary: this.ajax( + `${this.buildURL(modelName, name, snapshot, 'findRecord')}/summary`, + 'GET', + { + data: assign(this.buildQuery() || {}, namespaceQuery), + } + ), }).then(({ job, summary }) => { job.JobSummary = summary; return job; @@ -52,7 +60,9 @@ export default ApplicationAdapter.extend({ }, fetchRawDefinition(job) { - const url = this.buildURL('job', job.get('id'), job, 'findRecord'); - return this.ajax(url, 'GET', { data: this.buildQuery() }); + const [name, namespace] = JSON.parse(job.get('id')); + const namespaceQuery = namespace && namespace !== 'default' ? { namespace } : {}; + const url = this.buildURL('job', name, job, 'findRecord'); + return this.ajax(url, 'GET', { data: assign(this.buildQuery() || {}, namespaceQuery) }); }, }); diff --git a/ui/app/models/job.js b/ui/app/models/job.js index 4e1e3a8e3..43c4b1225 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -10,6 +10,7 @@ const { computed } = Ember; export default Model.extend({ region: attr('string'), name: attr('string'), + plainId: attr('string'), type: attr('string'), priority: attr('number'), allAtOnce: attr('boolean'), diff --git a/ui/app/serializers/allocation.js b/ui/app/serializers/allocation.js index b8a52d809..32722fc50 100644 --- a/ui/app/serializers/allocation.js +++ b/ui/app/serializers/allocation.js @@ -1,9 +1,11 @@ import Ember from 'ember'; import ApplicationSerializer from './application'; -const { get } = Ember; +const { get, inject } = Ember; export default ApplicationSerializer.extend({ + system: inject.service(), + attrs: { taskGroupName: 'TaskGroup', states: 'TaskStates', @@ -22,6 +24,14 @@ export default ApplicationSerializer.extend({ hash.JobVersion = hash.JobVersion != null ? hash.JobVersion : get(hash, 'Job.Version'); + hash.PlainJobId = hash.JobID; + hash.Namespace = + hash.Namespace || + get(hash, 'Job.Namespace') || + this.get('system.activeNamespace.id') || + 'default'; + hash.JobID = JSON.stringify([hash.JobID, hash.Namespace]); + // TEMPORARY: https://github.com/emberjs/data/issues/5209 hash.OriginalJobId = hash.JobID; diff --git a/ui/app/serializers/deployment.js b/ui/app/serializers/deployment.js index 2cae8c7a9..04e5475a0 100644 --- a/ui/app/serializers/deployment.js +++ b/ui/app/serializers/deployment.js @@ -14,6 +14,14 @@ export default ApplicationSerializer.extend({ return assign({ Name: key }, deploymentStats); }); + hash.PlainJobId = hash.JobID; + hash.Namespace = + hash.Namespace || + get(hash, 'Job.Namespace') || + this.get('system.activeNamespace.id') || + 'default'; + hash.JobID = JSON.stringify([hash.JobID, hash.Namespace]); + return this._super(typeHash, hash); }, diff --git a/ui/app/serializers/job.js b/ui/app/serializers/job.js index 8854b15c8..0f36f7524 100644 --- a/ui/app/serializers/job.js +++ b/ui/app/serializers/job.js @@ -12,9 +12,13 @@ export default ApplicationSerializer.extend({ normalize(typeHash, hash) { hash.NamespaceID = hash.Namespace; + // ID is a composite of both the job ID and the namespace the job is in + hash.PlainId = hash.ID; + hash.ID = JSON.stringify([hash.ID, hash.NamespaceID || 'default']); + // Transform the map-based JobSummary object into an array-based // JobSummary fragment list - hash.TaskGroupSummaries = Object.keys(get(hash, 'JobSummary.Summary')).map(key => { + hash.TaskGroupSummaries = Object.keys(get(hash, 'JobSummary.Summary') || {}).map(key => { const allocStats = get(hash, `JobSummary.Summary.${key}`); const summary = { Name: key }; @@ -40,9 +44,10 @@ export default ApplicationSerializer.extend({ const namespace = !hash.NamespaceID || hash.NamespaceID === 'default' ? undefined : hash.NamespaceID; const { modelName } = modelClass; + const jobURL = this.store .adapterFor(modelName) - .buildURL(modelName, this.extractId(modelClass, hash), hash, 'findRecord'); + .buildURL(modelName, hash.PlainId, hash, 'findRecord'); return assign(this._super(...arguments), { allocations: { diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index 473b67232..433ac1f0d 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -10,6 +10,7 @@ moduleFor('adapter:job', 'Unit | Adapter | Job', { this.server = startMirage(); this.server.create('node'); this.server.create('job', { id: 'job-1' }); + this.server.create('job', { id: 'job-2', namespaceId: 'some-namespace' }); }, afterEach() { this.server.shutdown(); @@ -18,22 +19,43 @@ moduleFor('adapter:job', 'Unit | Adapter | Job', { test('The job summary is stitched into the job request', function(assert) { const { pretender } = this.server; - const jobId = 'job-1'; + const jobName = 'job-1'; + const jobNamespace = 'default'; + const jobId = JSON.stringify([jobName, jobNamespace]); this.subject().findRecord(null, { modelName: 'job' }, jobId); assert.deepEqual( pretender.handledRequests.mapBy('url'), - ['/v1/namespaces', `/v1/job/${jobId}`, `/v1/job/${jobId}/summary`], + ['/v1/namespaces', `/v1/job/${jobName}`, `/v1/job/${jobName}/summary`], 'The three requests made are /namespaces, /job/:id, and /job/:id/summary' ); }); +test('When the job has a namespace other than default, it is in the URL', function(assert) { + const { pretender } = this.server; + const jobName = 'job-2'; + const jobNamespace = 'some-namespace'; + const jobId = JSON.stringify([jobName, jobNamespace]); + + this.subject().findRecord(null, { modelName: 'job' }, jobId); + + assert.deepEqual( + pretender.handledRequests.mapBy('url'), + [ + '/v1/namespaces', + `/v1/job/${jobName}?namespace=${jobNamespace}`, + `/v1/job/${jobName}/summary?namespace=${jobNamespace}`, + ], + 'The three requests made are /namespaces, /job/:id?namespace=:namespace, and /job/:id/summary?namespace=:namespace' + ); +}); + test('When there is no token set in the token service, no x-nomad-token header is set', function( assert ) { const { pretender } = this.server; - const jobId = 'job-1'; + const jobId = JSON.stringify(['job-1', 'default']); this.subject().findRecord(null, { modelName: 'job' }, jobId); @@ -47,7 +69,7 @@ test('When a token is set in the token service, then x-nomad-token header is set assert ) { const { pretender } = this.server; - const jobId = 'job-1'; + const jobId = JSON.stringify(['job-1', 'default']); const secret = 'here is the secret'; this.subject().set('token.secret', secret); diff --git a/ui/tests/unit/serializers/job-test.js b/ui/tests/unit/serializers/job-test.js index 9e4a2e2ad..15bf2b0f2 100644 --- a/ui/tests/unit/serializers/job-test.js +++ b/ui/tests/unit/serializers/job-test.js @@ -51,10 +51,11 @@ test('The JobSummary object is transformed from a map to a list', function(asser JobModifyIndex: 7, }; - const normalized = this.subject().normalize(JobModel, original); + const { data } = this.subject().normalize(JobModel, original); - assert.deepEqual(normalized.data.attributes, { + assert.deepEqual(data.attributes, { name: 'example', + plainId: 'example', type: 'service', priority: 50, periodic: false, @@ -116,6 +117,7 @@ test('The children stats are lifted out of the JobSummary object', function(asse assert.deepEqual(normalized.data.attributes, { name: 'example', + plainId: 'example', type: 'service', priority: 50, periodic: false, @@ -130,3 +132,29 @@ test('The children stats are lifted out of the JobSummary object', function(asse modifyIndex: 9, }); }); + +test('`default` is used as the namespace in the job ID when there is no namespace in the payload', function( + assert +) { + const original = { + ID: 'example', + Name: 'example', + }; + + const { data } = this.subject().normalize(JobModel, original); + assert.equal(data.id, JSON.stringify([data.attributes.name, 'default'])); +}); + +test('The ID of the record is a composite of both the name and the namespace', function(assert) { + const original = { + ID: 'example', + Name: 'example', + Namespace: 'special-namespace', + }; + + const { data } = this.subject().normalize(JobModel, original); + assert.equal( + data.id, + JSON.stringify([data.attributes.name, data.relationships.namespace.data.id]) + ); +});