ui: Adds @NullValue attr decorator (#9587)

There are many places in the API where we receive a property set to
`null` which can then lead to defensive code deeper in the app in order
to guard for this type of thing when usually we are expecting an array
or for the property to be undefined using omitempty on the backend.

Previously we had two places where we would deal with this in the
serializer using our 'remove-null' util (KV and Intentions).

This new decorator lets you declaritively define this type of data using
a decorator @NullValue([]) (which would replce a null value with [].

@NullValue in turn uses a more generic @replace helper, which we
currently don't need but would let you replace any value with another,
not just a null value.

An additional benefit here is that the guard/replacement is executed
lazily when we get the property instead of serializing all the values
when they come in via the API. On super large datasets, where we only
visualize part of the dataset (say in our scroll panes), this feels like
a good improvement on the previous approach.
This commit is contained in:
John Cowen 2021-01-27 10:41:24 +00:00 committed by GitHub
parent 623aab5880
commit 299ecff703
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 33 additions and 73 deletions

View File

@ -0,0 +1,22 @@
/**
* Simple replacing decorator, with the primary usecase for avoiding null API
* errors by decorating model attributes: @replace(null, []) @attr() Tags;
*/
const replace = (find, replace) => (target, propertyKey, desc) => {
return {
get: function() {
const value = desc.get.apply(this, arguments);
if (value === find) {
return replace;
}
return value;
},
set: function() {
return desc.set.apply(this, arguments);
},
};
};
export const nullValue = function(val) {
return replace(null, val);
};
export default replace;

View File

@ -1,6 +1,7 @@
import Model, { attr } from '@ember-data/model';
import { computed } from '@ember/object';
import { fragmentArray } from 'ember-data-model-fragments/attributes';
import { nullValue } from 'consul-ui/decorators/replace';
export const PRIMARY_KEY = 'uid';
export const SLUG_KEY = 'ID';
@ -17,7 +18,7 @@ export default class Intention extends Model {
@attr('string', { defaultValue: () => '*' }) DestinationName;
@attr('number') Precedence;
@attr('string', { defaultValue: () => 'consul' }) SourceType;
@attr('string') Action;
@nullValue(undefined) @attr('string') Action;
@attr('string') LegacyID;
@attr('boolean', { defaultValue: () => true }) Legacy;
@attr('number') SyncTime;

View File

@ -1,6 +1,7 @@
import Model, { attr } from '@ember-data/model';
import { computed } from '@ember/object';
import isFolder from 'consul-ui/utils/isFolder';
import { nullValue } from 'consul-ui/decorators/replace';
export const PRIMARY_KEY = 'uid';
// not really a slug as it contains slashes but all intents and purposes its
@ -15,12 +16,7 @@ export default class Kv extends Model {
@attr('string') Namespace;
@attr('number') LockIndex;
@attr('number') Flags;
// TODO: Consider defaulting all strings to '' because `typeof null !==
// 'string'` look into what other transformers do with `null` also
// preferably removeNull would be done in this layer also as if a property
// is `null` default Values don't kick in, which also explains `Tags`
// elsewhere
@attr('string') Value; //, {defaultValue: function() {return '';}}
@nullValue(undefined) @attr('string') Value;
@attr('number') CreateIndex;
@attr('number') ModifyIndex;
@attr('string') Session;

View File

@ -2,6 +2,7 @@ import Model, { attr } from '@ember-data/model';
import { computed } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import { fragment } from 'ember-data-model-fragments/attributes';
import { nullValue } from 'consul-ui/decorators/replace';
export const PRIMARY_KEY = 'uid';
export const SLUG_KEY = 'Name';
@ -21,7 +22,6 @@ export const Collection = class Collection {
return [...new Set(sources)].filter(Boolean).sort();
}
};
export default class Service extends Model {
@attr('string') uid;
@attr('string') Name;
@ -38,12 +38,13 @@ export default class Service extends Model {
@attr('number') SyncTime;
@attr('number') CreateIndex;
@attr('number') ModifyIndex;
@attr({ defaultValue: () => [] }) Tags;
@nullValue([]) @attr({ defaultValue: () => [] }) Tags;
@attr() Nodes; // array
@attr() Proxy; // Service
@attr() ExternalSources; // array
@fragment('gateway-config') GatewayConfig;
@nullValue([]) @attr() ExternalSources; // array
@attr() Meta; // {}
@attr() meta; // {}

View File

@ -2,7 +2,6 @@ import Serializer from './application';
import { inject as service } from '@ember/service';
import { get } from '@ember/object';
import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/intention';
import removeNull from 'consul-ui/utils/remove-null';
export default class IntentionSerializer extends Serializer {
@service('encoder') encoder;
@ -33,7 +32,7 @@ export default class IntentionSerializer extends Serializer {
respond((headers, body) => {
return cb(
headers,
body.map(item => this.ensureID(removeNull(item)))
body.map(item => this.ensureID(item))
);
}),
query
@ -44,7 +43,7 @@ export default class IntentionSerializer extends Serializer {
return super.respondForQueryRecord(
cb =>
respond((headers, body) => {
body = this.ensureID(removeNull(body));
body = this.ensureID(body);
return cb(headers, body);
}),
query

View File

@ -3,7 +3,6 @@ import { inject as service } from '@ember/service';
import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/kv';
import { NSPACE_KEY } from 'consul-ui/models/nspace';
import { NSPACE_QUERY_PARAM as API_NSPACE_KEY } from 'consul-ui/adapters/application';
import removeNull from 'consul-ui/utils/remove-null';
export default class KvSerializer extends Serializer {
@service('atob') decoder;
@ -19,7 +18,7 @@ export default class KvSerializer extends Serializer {
respondForQueryRecord(respond, query) {
return super.respondForQueryRecord(
cb => respond((headers, body) => cb(headers, removeNull(body[0]))),
cb => respond((headers, body) => cb(headers, body[0])),
query
);
}

View File

@ -1,9 +0,0 @@
export default function(obj) {
// non-recursive for the moment
return Object.keys(obj).reduce(function(prev, item, i, arr) {
if (obj[item] !== null) {
prev[item] = obj[item];
}
return prev;
}, {});
}

View File

@ -1,49 +0,0 @@
import removeNull from 'consul-ui/utils/remove-null';
import { skip } from 'qunit';
import { module, test } from 'qunit';
module('Unit | Utility | remove null', function() {
test('it removes null valued properties shallowly', function(assert) {
[
{
test: {
Value: null,
},
expected: {},
},
{
test: {
Key: 'keyname',
Value: null,
},
expected: {
Key: 'keyname',
},
},
{
test: {
Key: 'keyname',
Value: '',
},
expected: {
Key: 'keyname',
Value: '',
},
},
{
test: {
Key: 'keyname',
Value: false,
},
expected: {
Key: 'keyname',
Value: false,
},
},
].forEach(function(item) {
const actual = removeNull(item.test);
assert.deepEqual(actual, item.expected);
});
});
skip('it removes null valued properties deeply');
});