Skip to content

Commit 6f0c354

Browse files
svidgeniartemiev
svidgen
and
iartemiev
authored
fix(@aws-amplify/datastore): strictly define null vs undefined behavior on models (#10915)
* migrate start of undef-null related work from selective-sync-fk fix branch * stash * fixed * cleanup * fixed comment * fixed sqlite adapter tests; clarified normalize() comment * added testing and fix for readonly fields appearing in mutation inputs * additional testing * removing .only from test * updated docstring * bump the geo bundle size, assuming a dep increased in size * revert unnecessary type change * checkpoint * Revert "checkpoint" This reverts commit a8f326a. * extra validation of copyOf behavior Co-authored-by: Ivan Artemiev <[email protected]> --------- Co-authored-by: Ivan Artemiev <[email protected]>
1 parent de19ce4 commit 6f0c354

File tree

11 files changed

+677
-90
lines changed

11 files changed

+677
-90
lines changed

‎packages/datastore-storage-adapter/__tests__/SQLiteUtils.test.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,14 +201,22 @@ describe('SQLiteUtils tests', () => {
201201
});
202202

203203
const expected = [
204-
'INSERT INTO "Model" ("field1", "dateCreated", "id", "_version", "_lastChangedAt", "_deleted") VALUES (?, ?, ?, ?, ?, ?)',
204+
'INSERT INTO "Model" ("field1", "dateCreated", "id", "_version", "_lastChangedAt", "_deleted", "optionalField1", "emails", "ips", "metadata", "createdAt", "updatedAt") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)',
205205
[
206206
model.field1,
207207
model.dateCreated,
208208
model.id,
209+
// meta-data fields are not user-defined fields and therefore not
210+
// part of normalization today. they are `undefined` by default.
209211
undefined,
210212
undefined,
211213
undefined,
214+
null,
215+
null,
216+
null,
217+
null,
218+
null,
219+
null,
212220
],
213221
];
214222

@@ -224,13 +232,21 @@ describe('SQLiteUtils tests', () => {
224232
});
225233

226234
const expected = [
227-
`UPDATE "Model" SET "field1"=?, "dateCreated"=?, "_version"=?, "_lastChangedAt"=?, "_deleted"=? WHERE id=?`,
235+
'UPDATE "Model" SET "field1"=?, "dateCreated"=?, "_version"=?, "_lastChangedAt"=?, "_deleted"=?, "optionalField1"=?, "emails"=?, "ips"=?, "metadata"=?, "createdAt"=?, "updatedAt"=? WHERE id=?',
228236
[
229237
model.field1,
230238
model.dateCreated,
239+
// meta-data fields are not user-defined fields and therefore not
240+
// part of normalization today. they are `undefined` by default.
231241
undefined,
232242
undefined,
233243
undefined,
244+
null,
245+
null,
246+
null,
247+
null,
248+
null,
249+
null,
234250
model.id,
235251
],
236252
];

‎packages/datastore/__tests__/DataStore.ts

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2136,7 +2136,7 @@ describe('DataStore tests', () => {
21362136
optionalField1: undefined,
21372137
});
21382138

2139-
expect(model1.optionalField1).toBeUndefined();
2139+
expect(model1.optionalField1).toBeNull();
21402140
});
21412141

21422142
test('Optional field can be initialized with null', () => {
@@ -2179,7 +2179,7 @@ describe('DataStore tests', () => {
21792179
expect(model1.id).toBe(model2.id);
21802180

21812181
expect(model1.optionalField1).toBe('something-else');
2182-
expect(model2.optionalField1).toBeUndefined();
2182+
expect(model2.optionalField1).toBeNull();
21832183
});
21842184

21852185
test('Optional field can be set to null inside copyOf', () => {
@@ -2199,7 +2199,7 @@ describe('DataStore tests', () => {
21992199
// ID should be kept the same
22002200
expect(model1.id).toBe(model2.id);
22012201

2202-
expect(model1.optionalField1).toBeUndefined();
2202+
expect(model1.optionalField1).toBeNull();
22032203
expect(model2.optionalField1).toBeNull();
22042204
});
22052205

@@ -2699,15 +2699,61 @@ describe('DataStore tests', () => {
26992699
);
27002700
});
27012701

2702-
test('valid model with nulls', () => {
2703-
expect(() => {
2704-
new Model({
2705-
field1: 'someField',
2706-
dateCreated: new Date().toISOString(),
2707-
emails: null,
2708-
ips: null,
2709-
});
2710-
}).not.toThrow();
2702+
test('valid model with null optional fields', () => {
2703+
const m = new Model({
2704+
field1: 'someField',
2705+
dateCreated: new Date().toISOString(),
2706+
optionalField1: null,
2707+
});
2708+
expect(m.optionalField1).toBe(null);
2709+
});
2710+
2711+
test('valid model with `undefined` optional fields', () => {
2712+
const m = new Model({
2713+
field1: 'someField',
2714+
dateCreated: new Date().toISOString(),
2715+
optionalField1: undefined,
2716+
});
2717+
expect(m.optionalField1).toBe(null);
2718+
});
2719+
2720+
test('valid model with omitted optional fields', () => {
2721+
const m = new Model({
2722+
field1: 'someField',
2723+
dateCreated: new Date().toISOString(),
2724+
/**
2725+
* Omitting this:
2726+
*
2727+
* optionalField: undefined
2728+
*/
2729+
});
2730+
expect(m.optionalField1).toBe(null);
2731+
});
2732+
2733+
test('copyOf() setting optional field to null', () => {
2734+
const emailsVal = ['[email protected]'];
2735+
const original = new Model({
2736+
field1: 'someField',
2737+
dateCreated: new Date().toISOString(),
2738+
optionalField1: 'defined value',
2739+
emails: emailsVal,
2740+
});
2741+
const copied = Model.copyOf(original, d => (d.optionalField1 = null));
2742+
expect(copied.optionalField1).toBe(null);
2743+
expect(copied.emails).toEqual(emailsVal);
2744+
});
2745+
2746+
test('copyOf() setting optional field to undefined', () => {
2747+
const original = new Model({
2748+
field1: 'someField',
2749+
dateCreated: new Date().toISOString(),
2750+
optionalField1: 'defined value',
2751+
});
2752+
const copied = Model.copyOf(
2753+
original,
2754+
d => (d.optionalField1 = undefined)
2755+
);
2756+
expect(copied.optionalField1).toBe(null);
27112757
});
27122758

27132759
test('pass null to non nullable array field', () => {
@@ -3683,7 +3729,7 @@ describe('DataStore tests', () => {
36833729
dateCreated: new Date().toISOString(),
36843730
});
36853731

3686-
expect(model1.description).toBeUndefined();
3732+
expect(model1.description).toBeNull();
36873733
});
36883734

36893735
test('Optional field can be initialized with null', () => {
@@ -3721,7 +3767,7 @@ describe('DataStore tests', () => {
37213767
expect(model1.postId).toBe(model2.postId);
37223768

37233769
expect(model1.description).toBe('something-else');
3724-
expect(model2.description).toBeUndefined();
3770+
expect(model2.description).toBeNull();
37253771
});
37263772

37273773
test('Optional field can be set to null inside copyOf', () => {
@@ -3742,7 +3788,7 @@ describe('DataStore tests', () => {
37423788
// postId should be kept the same
37433789
expect(model1.postId).toBe(model2.postId);
37443790

3745-
expect(model1.description).toBeUndefined();
3791+
expect(model1.description).toBeNull();
37463792
expect(model2.description).toBeNull();
37473793
});
37483794

‎packages/datastore/__tests__/Merger.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ describe('Merger', () => {
4444
{
4545
id: modelId,
4646
field1: 'Create',
47+
dateCreated: new Date().toISOString(),
4748
optionalField1: null,
4849
_version: 1,
4950
_lastChangedAt: 1619627611860,
@@ -52,6 +53,7 @@ describe('Merger', () => {
5253
{
5354
id: modelId,
5455
field1: 'Create',
56+
dateCreated: new Date().toISOString(),
5557
optionalField1: null,
5658
_version: 2,
5759
_lastChangedAt: 1619627619017,
@@ -75,6 +77,7 @@ describe('Merger', () => {
7577
{
7678
id: modelId,
7779
field1: 'Create',
80+
dateCreated: new Date().toISOString(),
7881
optionalField1: null,
7982
_version: 1,
8083
_lastChangedAt: 1619627611860,
@@ -83,6 +86,7 @@ describe('Merger', () => {
8386
{
8487
id: modelId,
8588
field1: 'Update',
89+
dateCreated: new Date().toISOString(),
8690
optionalField1: null,
8791
_version: 2,
8892
_lastChangedAt: 1619627619017,
@@ -91,6 +95,7 @@ describe('Merger', () => {
9195
{
9296
id: modelId,
9397
field1: 'Another Update',
98+
dateCreated: new Date().toISOString(),
9499
optionalField1: 'Optional',
95100
_version: 2,
96101
_lastChangedAt: 1619627621329,
@@ -115,6 +120,7 @@ describe('Merger', () => {
115120
{
116121
id: modelId,
117122
field1: 'Create',
123+
dateCreated: new Date().toISOString(),
118124
optionalField1: null,
119125
_version: 1,
120126
_lastChangedAt: 1619627611860,
@@ -123,6 +129,7 @@ describe('Merger', () => {
123129
{
124130
id: modelId,
125131
field1: 'Create',
132+
dateCreated: new Date().toISOString(),
126133
optionalField1: null,
127134
_version: 2,
128135
_lastChangedAt: 1619627619017,
@@ -131,6 +138,7 @@ describe('Merger', () => {
131138
{
132139
id: modelId,
133140
field1: 'New Create with the same id',
141+
dateCreated: new Date().toISOString(),
134142
optionalField1: null,
135143
_version: 1,
136144
_lastChangedAt: 1619627621329,
@@ -178,6 +186,7 @@ describe('Merger', () => {
178186
{
179187
postId: customPk,
180188
title: 'Create1',
189+
dateCreated: new Date().toISOString(),
181190
description: null,
182191
_version: 1,
183192
_lastChangedAt: 1619627611860,
@@ -186,6 +195,7 @@ describe('Merger', () => {
186195
{
187196
postId: customPk,
188197
title: 'Create1',
198+
dateCreated: new Date().toISOString(),
189199
description: null,
190200
_version: 2,
191201
_lastChangedAt: 1619627619017,
@@ -214,6 +224,7 @@ describe('Merger', () => {
214224
{
215225
postId: customPk,
216226
title: 'Create1',
227+
dateCreated: new Date().toISOString(),
217228
description: null,
218229
_version: 1,
219230
_lastChangedAt: 1619627611860,
@@ -222,6 +233,7 @@ describe('Merger', () => {
222233
{
223234
postId: customPk,
224235
title: 'Update1',
236+
dateCreated: new Date().toISOString(),
225237
description: null,
226238
_version: 2,
227239
_lastChangedAt: 1619627619017,
@@ -230,6 +242,7 @@ describe('Merger', () => {
230242
{
231243
postId: customPk,
232244
title: 'Another Update1',
245+
dateCreated: new Date().toISOString(),
233246
description: 'Optional1',
234247
_version: 2,
235248
_lastChangedAt: 1619627621329,
@@ -259,6 +272,7 @@ describe('Merger', () => {
259272
{
260273
postId: customPk,
261274
title: 'Create1',
275+
dateCreated: new Date().toISOString(),
262276
description: null,
263277
_version: 1,
264278
_lastChangedAt: 1619627611860,
@@ -267,6 +281,7 @@ describe('Merger', () => {
267281
{
268282
postId: customPk,
269283
title: 'Create1',
284+
dateCreated: new Date().toISOString(),
270285
description: null,
271286
_version: 2,
272287
_lastChangedAt: 1619627619017,
@@ -275,6 +290,7 @@ describe('Merger', () => {
275290
{
276291
postId: customPk,
277292
title: 'New Create with the same custom pk',
293+
dateCreated: new Date().toISOString(),
278294
description: null,
279295
_version: 1,
280296
_lastChangedAt: 1619627621329,

0 commit comments

Comments
 (0)