From 3147d5c649fe686ab56c74f9c5c474fe9452967b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 4 Sep 2020 19:44:21 -0700 Subject: [PATCH] Sort keys when converting objects to arrays for stable model fragments --- ui/app/serializers/allocation.js | 16 +++-- ui/app/serializers/application.js | 14 ++-- ui/app/serializers/job-summary.js | 18 ++--- ui/app/serializers/plugin.js | 12 ++-- ui/tests/unit/serializers/allocation-test.js | 72 +++++++++++++++++++ ui/tests/unit/serializers/application-test.js | 2 +- 6 files changed, 107 insertions(+), 27 deletions(-) diff --git a/ui/app/serializers/allocation.js b/ui/app/serializers/allocation.js index 3e7be2e79895..15cccb6e3771 100644 --- a/ui/app/serializers/allocation.js +++ b/ui/app/serializers/allocation.js @@ -24,13 +24,15 @@ export default class AllocationSerializer extends ApplicationSerializer { // Transform the map-based TaskStates object into an array-based // TaskState fragment list const states = hash.TaskStates || {}; - hash.TaskStates = Object.keys(states).map(key => { - const state = states[key] || {}; - const summary = { Name: key }; - Object.keys(state).forEach(stateKey => (summary[stateKey] = state[stateKey])); - summary.Resources = hash.TaskResources && hash.TaskResources[key]; - return summary; - }); + hash.TaskStates = Object.keys(states) + .sort() + .map(key => { + const state = states[key] || {}; + const summary = { Name: key }; + Object.keys(state).forEach(stateKey => (summary[stateKey] = state[stateKey])); + summary.Resources = hash.TaskResources && hash.TaskResources[key]; + return summary; + }); hash.JobVersion = hash.JobVersion != null ? hash.JobVersion : get(hash, 'Job.Version'); diff --git a/ui/app/serializers/application.js b/ui/app/serializers/application.js index c5332140605a..32ddd21edc77 100644 --- a/ui/app/serializers/application.js +++ b/ui/app/serializers/application.js @@ -105,14 +105,16 @@ export default class Application extends JSONSerializer { const map = hash[apiKey] || {}; - hash[uiKey] = Object.keys(map).map(mapKey => { - const propertiesForKey = map[mapKey] || {}; - const convertedMap = { Name: mapKey }; + hash[uiKey] = Object.keys(map) + .sort() + .map(mapKey => { + const propertiesForKey = map[mapKey] || {}; + const convertedMap = { Name: mapKey }; - assign(convertedMap, propertiesForKey); + assign(convertedMap, propertiesForKey); - return convertedMap; - }); + return convertedMap; + }); }); } diff --git a/ui/app/serializers/job-summary.js b/ui/app/serializers/job-summary.js index 50b35a07281c..bf65f6c3847d 100644 --- a/ui/app/serializers/job-summary.js +++ b/ui/app/serializers/job-summary.js @@ -11,16 +11,18 @@ export default class JobSummary extends ApplicationSerializer { // TaskGroupSummary fragment list const fullSummary = hash.Summary || {}; - hash.TaskGroupSummaries = Object.keys(fullSummary).map(key => { - const allocStats = fullSummary[key] || {}; - const summary = { Name: key }; + hash.TaskGroupSummaries = Object.keys(fullSummary) + .sort() + .map(key => { + const allocStats = fullSummary[key] || {}; + const summary = { Name: key }; - Object.keys(allocStats).forEach( - allocKey => (summary[`${allocKey}Allocs`] = allocStats[allocKey]) - ); + Object.keys(allocStats).forEach( + allocKey => (summary[`${allocKey}Allocs`] = allocStats[allocKey]) + ); - return summary; - }); + return summary; + }); // Lift the children stats out of the Children object const childrenStats = get(hash, 'Children'); diff --git a/ui/app/serializers/plugin.js b/ui/app/serializers/plugin.js index 1eed76606e1d..d113551e38ea 100644 --- a/ui/app/serializers/plugin.js +++ b/ui/app/serializers/plugin.js @@ -6,11 +6,13 @@ import ApplicationSerializer from './application'; // excessive copies of the originals which would otherwise just // be garbage collected. const unmap = (hash, propKey) => - Object.keys(hash).map(key => { - const record = hash[key]; - record[propKey] = key; - return record; - }); + Object.keys(hash) + .sort() + .map(key => { + const record = hash[key]; + record[propKey] = key; + return record; + }); export default class Plugin extends ApplicationSerializer { normalize(typeHash, hash) { diff --git a/ui/tests/unit/serializers/allocation-test.js b/ui/tests/unit/serializers/allocation-test.js index 7a76c208c5b2..80d58bbd6b77 100644 --- a/ui/tests/unit/serializers/allocation-test.js +++ b/ui/tests/unit/serializers/allocation-test.js @@ -303,6 +303,78 @@ module('Unit | Serializer | Allocation', function(hooks) { }, }, }, + + { + name: 'TaskStates are sorted for stable fragments', + in: { + ID: 'test-allocation', + JobID: 'test-summary', + Name: 'test-summary[1]', + Namespace: 'test-namespace', + TaskGroup: 'test-group', + CreateTime: +sampleDate * 1000000, + ModifyTime: +sampleDate * 1000000, + TaskStates: { + xyz: { + State: 'running', + Failed: false, + }, + abc: { + State: 'running', + Failed: false, + }, + }, + }, + out: { + data: { + id: 'test-allocation', + type: 'allocation', + attributes: { + taskGroupName: 'test-group', + name: 'test-summary[1]', + modifyTime: sampleDate, + createTime: sampleDate, + states: [ + { + name: 'abc', + state: 'running', + failed: false, + }, + { + name: 'xyz', + state: 'running', + failed: false, + }, + ], + wasPreempted: false, + allocationTaskGroup: null, + }, + relationships: { + followUpEvaluation: { + data: null, + }, + nextAllocation: { + data: null, + }, + previousAllocation: { + data: null, + }, + preemptedAllocations: { + data: [], + }, + preemptedByAllocation: { + data: null, + }, + job: { + data: { + id: '["test-summary","test-namespace"]', + type: 'job', + }, + }, + }, + }, + }, + }, ]; normalizationTestCases.forEach(testCase => { diff --git a/ui/tests/unit/serializers/application-test.js b/ui/tests/unit/serializers/application-test.js index 0990ece9ec5a..4476577d6570 100644 --- a/ui/tests/unit/serializers/application-test.js +++ b/ui/tests/unit/serializers/application-test.js @@ -68,8 +68,8 @@ module('Unit | Serializer | Application', function(hooks) { ID: 'test-test', Things: [1, 2, 3], ArrayableMap: { - a: { Order: 1 }, b: { Order: 2 }, + a: { Order: 1 }, 'c.d': { Order: 3 }, }, OriginalNameArrayableMap: {