From 267e8e39aadb2136c85e615909a654808d57399d Mon Sep 17 00:00:00 2001 From: Michael Shanks Date: Mon, 5 Oct 2020 17:28:23 +0100 Subject: [PATCH] Coerse record fields, to be a bit more tolerant of data input --- .../builder/src/constants/backend/index.js | 10 +- packages/server/src/api/controllers/record.js | 19 ++-- .../src/api/routes/tests/couchTestUtils.js | 4 +- .../src/api/routes/tests/record.spec.js | 91 ++++++++++++++++++- 4 files changed, 108 insertions(+), 16 deletions(-) diff --git a/packages/builder/src/constants/backend/index.js b/packages/builder/src/constants/backend/index.js index 45de862a98..8cf9f12230 100644 --- a/packages/builder/src/constants/backend/index.js +++ b/packages/builder/src/constants/backend/index.js @@ -6,7 +6,7 @@ export const FIELDS = { constraints: { type: "string", length: {}, - presence: { allowEmpty: true }, + presence: false, }, }, NUMBER: { @@ -15,7 +15,7 @@ export const FIELDS = { type: "number", constraints: { type: "number", - presence: { allowEmpty: true }, + presence: false, numericality: { greaterThanOrEqualTo: "", lessThanOrEqualTo: "" }, }, }, @@ -25,7 +25,7 @@ export const FIELDS = { type: "boolean", constraints: { type: "boolean", - presence: { allowEmpty: true }, + presence: false, }, }, // OPTIONS: { @@ -44,7 +44,7 @@ export const FIELDS = { constraints: { type: "string", length: {}, - presence: { allowEmpty: true }, + presence: false, datetime: { latest: "", earliest: "", @@ -57,7 +57,7 @@ export const FIELDS = { type: "attachment", constraints: { type: "array", - presence: { allowEmpty: true }, + presence: false, }, }, // LINKED_FIELDS: { diff --git a/packages/server/src/api/controllers/record.js b/packages/server/src/api/controllers/record.js index 508a2210b6..586bb81074 100644 --- a/packages/server/src/api/controllers/record.js +++ b/packages/server/src/api/controllers/record.js @@ -33,7 +33,7 @@ exports.patch = async function(ctx) { const model = await db.get(record.modelId) const patchfields = ctx.request.body - coersceRecordValues(record, model) + coerceRecordValues(record, model) for (let key in patchfields) { if (!model.schema[key]) continue @@ -73,7 +73,7 @@ exports.save = async function(ctx) { const model = await db.get(record.modelId) - coersceRecordValues(record, model) + coerceRecordValues(record, model) const validateResult = await validate({ record, @@ -223,12 +223,12 @@ async function validate({ instanceId, modelId, record, model }) { return { valid: Object.keys(errors).length === 0, errors } } -function coersceRecordValues(record, model) { +function coerceRecordValues(record, model) { for (let [key, value] of Object.entries(record)) { const field = model.schema[key] if (!field) continue const mapping = Object.prototype.hasOwnProperty.call( - TYPE_TRANSFORM_MAP, + TYPE_TRANSFORM_MAP[field.type], value ) ? TYPE_TRANSFORM_MAP[field.type][value] @@ -243,10 +243,11 @@ const TYPE_TRANSFORM_MAP = { "": "", [null]: "", [undefined]: undefined, - parse: s => s.toString(), + parse: s => s, }, number: { "": null, + [null]: null, [undefined]: undefined, parse: n => parseFloat(n), }, @@ -254,19 +255,21 @@ const TYPE_TRANSFORM_MAP = { "": null, [undefined]: undefined, [null]: null, - parse: d => new Date(d).getTime(), + parse: d => d, }, - attachments: { + attachment: { + "": [], [null]: [], [undefined]: undefined, parse: a => a, }, boolean: { + "": null, [null]: null, [undefined]: undefined, parse: b => { - if (b === "false") return false if (b === "true") return true + if (b === "false") return false return b }, }, diff --git a/packages/server/src/api/routes/tests/couchTestUtils.js b/packages/server/src/api/routes/tests/couchTestUtils.js index a22a2a427f..56462837dd 100644 --- a/packages/server/src/api/routes/tests/couchTestUtils.js +++ b/packages/server/src/api/routes/tests/couchTestUtils.js @@ -46,13 +46,13 @@ exports.createModel = async (request, appId, instanceId, model) => { key: "name", schema: { name: { - type: "text", + type: "string", constraints: { type: "string", }, }, description: { - type: "text", + type: "string", constraints: { type: "string", }, diff --git a/packages/server/src/api/routes/tests/record.spec.js b/packages/server/src/api/routes/tests/record.spec.js index 1944499e0f..1b0381a924 100644 --- a/packages/server/src/api/routes/tests/record.spec.js +++ b/packages/server/src/api/routes/tests/record.spec.js @@ -38,7 +38,7 @@ describe("/records", () => { const createRecord = async r => await request - .post(`/api/${model._id}/records`) + .post(`/api/${r ? r.modelId : record.modelId}/records`) .send(r || record) .set(defaultHeaders(app._id, instance._id)) .expect('Content-Type', /json/) @@ -152,6 +152,95 @@ describe("/records", () => { .expect('Content-Type', /json/) .expect(404) }) + + it("record values are coerced", async () => { + const str = {type:"string", constraints: { type: "string", presence: false }} + const attachment = {type:"attachment", constraints: { type: "array", presence: false }} + const bool = {type:"boolean", constraints: { type: "boolean", presence: false }} + const number = {type:"number", constraints: { type: "number", presence: false }} + const datetime = {type:"datetime", constraints: { type: "string", presence: false, datetime: {earliest:"", latest: ""} }} + + model = await createModel(request, app._id, instance._id, { + name: "TestModel2", + type: "model", + key: "name", + schema: { + name: str, + stringUndefined: str, + stringNull: str, + stringString: str, + numberEmptyString: number, + numberNull: number, + numberUndefined: number, + numberString: number, + datetimeEmptyString: datetime, + datetimeNull: datetime, + datetimeUndefined: datetime, + datetimeString: datetime, + datetimeDate: datetime, + boolNull: bool, + boolEmpty: bool, + boolUndefined: bool, + boolString: bool, + boolBool: bool, + attachmentNull : attachment, + attachmentUndefined : attachment, + attachmentEmpty : attachment, + }, + }) + + record = { + name: "Test Record", + stringUndefined: undefined, + stringNull: null, + stringString: "i am a string", + numberEmptyString: "", + numberNull: null, + numberUndefined: undefined, + numberString: "123", + numberNumber: 123, + datetimeEmptyString: "", + datetimeNull: null, + datetimeUndefined: undefined, + datetimeString: "1984-04-20T00:00:00.000Z", + datetimeDate: new Date("1984-04-20"), + boolNull: null, + boolEmpty: "", + boolUndefined: undefined, + boolString: "true", + boolBool: true, + modelId: model._id, + attachmentNull : null, + attachmentUndefined : undefined, + attachmentEmpty : "", + } + + const id = (await createRecord(record)).body._id + + const saved = (await loadRecord(id)).body + + expect(saved.stringUndefined).toBe(undefined) + expect(saved.stringNull).toBe("") + expect(saved.stringString).toBe("i am a string") + expect(saved.numberEmptyString).toBe(null) + expect(saved.numberNull).toBe(null) + expect(saved.numberUndefined).toBe(undefined) + expect(saved.numberString).toBe(123) + expect(saved.numberNumber).toBe(123) + expect(saved.datetimeEmptyString).toBe(null) + expect(saved.datetimeNull).toBe(null) + expect(saved.datetimeUndefined).toBe(undefined) + expect(saved.datetimeString).toBe(new Date(record.datetimeString).toISOString()) + expect(saved.datetimeDate).toBe(record.datetimeDate.toISOString()) + expect(saved.boolNull).toBe(null) + expect(saved.boolEmpty).toBe(null) + expect(saved.boolUndefined).toBe(undefined) + expect(saved.boolString).toBe(true) + expect(saved.boolBool).toBe(true) + expect(saved.attachmentNull).toEqual([]) + expect(saved.attachmentUndefined).toBe(undefined) + expect(saved.attachmentEmpty).toEqual([]) + }) }) describe("patch", () => {