Skip to content

Commit

Permalink
feat: remove dbmSchema, only validate bmSchema on any op
Browse files Browse the repository at this point in the history
  • Loading branch information
kirillgroshkov committed Jan 24, 2024
1 parent 39abc28 commit 840bb03
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 100 deletions.
3 changes: 1 addition & 2 deletions scripts/cannon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { expressFunctionFactory, runCannon } from '@naturalcycles/bench-lib'
import { _omit } from '@naturalcycles/js-lib'
import { getValidationResult, stringId, runScript } from '@naturalcycles/nodejs-lib'
import { CommonDao, InMemoryDB } from '../src'
import { createTestItemsBM, testItemBMSchema, testItemDBMSchema, TEST_TABLE } from '../src/testing'
import { createTestItemsBM, testItemBMSchema, TEST_TABLE } from '../src/testing'

runScript(async () => {
await runCannon(
Expand All @@ -35,7 +35,6 @@ const db = new InMemoryDB()
const dao = new CommonDao({
table: TEST_TABLE,
db,
dbmSchema: testItemDBMSchema,
bmSchema: testItemBMSchema,
})

Expand Down
23 changes: 0 additions & 23 deletions src/commondao/common.dao.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,6 @@ export interface CommonDaoHooks<BM extends BaseDBEntity, DBM extends BaseDBEntit
*/
beforeCreate: (bm: Partial<BM>) => Partial<BM>

/**
* Called when loading things "as DBM" and validation is not skipped.
* When loading things as BM/TM - other hooks get involved instead:
* - beforeDBMToBM
* - beforeBMToTM
*
* TODO: maybe rename those to `validateAs(model)`
* as it only validates "final state", not intermediate
*/
beforeDBMValidate: (dbm: Partial<DBM>) => Partial<DBM>

beforeDBMToBM: (dbm: DBM) => Partial<BM> | Promise<Partial<BM>>
beforeBMToDBM: (bm: BM) => Partial<DBM> | Promise<Partial<DBM>>

Expand Down Expand Up @@ -136,7 +125,6 @@ export interface CommonDaoCfg<BM extends BaseDBEntity, DBM extends BaseDBEntity
/**
* Joi, AjvSchema or ZodSchema is supported.
*/
dbmSchema?: ObjectSchema<DBM> | AjvSchema<DBM> | ZodSchema<DBM>
bmSchema?: ObjectSchema<BM> | AjvSchema<BM> | ZodSchema<BM>

excludeFromIndexes?: (keyof DBM)[]
Expand Down Expand Up @@ -201,17 +189,6 @@ export interface CommonDaoCfg<BM extends BaseDBEntity, DBM extends BaseDBEntity
*/
useUpdatedProperty?: boolean

/**
* Default is false.
* If true - will run `_filterNullishValues` inside `validateAndConvert` function
* (instead of `_filterUndefinedValues`).
* This was the old db-lib behavior.
* This option allows to keep backwards-compatible behavior.
*
* @deprecated
*/
filterNullishValues?: boolean

/**
* Defaults to false.
* If true - run patch operations (patch, patchById) in a Transaction.
Expand Down
10 changes: 3 additions & 7 deletions src/commondao/common.dao.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@ import { DBLibError } from '../cnst'
import {
createTestItemsBM,
testItemBMSchema,
testItemDBMSchema,
TEST_TABLE,
TestItemBM,
TestItemDBM,
testItemBMJsonSchema,
testItemDBMJsonSchema,
} from '../testing'
import { CommonDao } from './common.dao'
import { CommonDaoCfg, CommonDaoLogLevel, CommonDaoSaveBatchOptions } from './common.dao.model'
Expand All @@ -37,7 +35,6 @@ const db = new InMemoryDB()
const daoCfg: CommonDaoCfg<TestItemBM, TestItemDBM> = {
table: TEST_TABLE,
db,
dbmSchema: testItemDBMSchema,
bmSchema: testItemBMSchema,
// logStarted: true,
logLevel: CommonDaoLogLevel.OPERATIONS,
Expand Down Expand Up @@ -375,7 +372,6 @@ test('ajvSchema', async () => {
table: TEST_TABLE,
db,
bmSchema: AjvSchema.create(testItemBMJsonSchema),
dbmSchema: AjvSchema.create(testItemDBMJsonSchema),
})

const items = createTestItemsBM(3)
Expand All @@ -396,9 +392,9 @@ test('ajvSchema', async () => {
)
expect(err).toBeInstanceOf(AjvValidationError)
expect(err).toMatchInlineSnapshot(`
[AjvValidationError: TEST_TABLEDBM.id123/k1 must be string
Input: { id: 'id123', k1: 5, created: 1529539200, updated: 1529539200 }]
`)
[AjvValidationError: TEST_TABLE.id123/k1 must be string
Input: { id: 'id123', k1: 5, created: 1529539200, updated: 1529539200 }]
`)

console.log((err as any).data)
})
Expand Down
63 changes: 27 additions & 36 deletions src/commondao/common.dao.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Transform } from 'node:stream'
import {
_assert,
_deepJsonEquals,
_filterNullishValues,
_filterUndefinedValues,
_isTruthy,
_objectAssignExact,
Expand Down Expand Up @@ -43,13 +42,7 @@ import {
writableVoid,
} from '@naturalcycles/nodejs-lib'
import { DBLibError } from '../cnst'
import {
CommonDBTransactionOptions,
DBModelType,
DBPatch,
DBTransaction,
RunQueryResult,
} from '../db.model'
import { CommonDBTransactionOptions, DBPatch, DBTransaction, RunQueryResult } from '../db.model'
import { DBQuery, RunnableDBQuery } from '../query/dbQuery'
import {
CommonDaoCfg,
Expand Down Expand Up @@ -91,9 +84,6 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {
hooks: {
parseNaturalId: () => ({}),
beforeCreate: bm => bm as BM,
beforeDBMValidate: dbm => dbm,
beforeDBMToBM: dbm => dbm as any,
beforeBMToDBM: bm => bm as any,
anonymize: dbm => dbm,
onValidationError: err => err,
...cfg.hooks,
Expand All @@ -112,7 +102,7 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {
const bm = this.cfg.hooks!.beforeCreate!(part)
// First assignIdCreatedUpdated, then validate!
this.assignIdCreatedUpdated(bm, opt)
return this.validateAndConvert(bm, this.cfg.bmSchema, DBModelType.BM, opt)
return this.validateAndConvert(bm, this.cfg.bmSchema, opt)
}

// GET
Expand Down Expand Up @@ -639,7 +629,7 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {
const idWasGenerated = !bm.id && this.cfg.generateId
this.assignIdCreatedUpdated(bm, opt) // mutates
_typeCast<BM>(bm)
let dbm = await this.bmToDBM(bm, opt)
let dbm = await this.bmToDBM(bm, opt) // validates BM

if (this.cfg.hooks!.beforeSave) {
dbm = (await this.cfg.hooks!.beforeSave(dbm))!
Expand Down Expand Up @@ -1113,11 +1103,16 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {
}

// DBM > BM
const bm = await this.cfg.hooks!.beforeDBMToBM!(dbm)
let bm: Partial<BM>
if (this.cfg.hooks!.beforeDBMToBM) {
bm = await this.cfg.hooks!.beforeDBMToBM(dbm)
} else {
bm = dbm as any
}

// Validate/convert BM

return this.validateAndConvert(bm, this.cfg.bmSchema, DBModelType.BM, opt)
return this.validateAndConvert(bm, this.cfg.bmSchema, opt)
}

async dbmsToBM(dbms: DBM[], opt: CommonDaoOptions = {}): Promise<BM[]> {
Expand All @@ -1133,20 +1128,23 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {
async bmToDBM(bm?: BM, opt?: CommonDaoOptions): Promise<DBM | undefined> {
if (bm === undefined) return

// optimization: no need to run the BM validation, since DBM will be validated anyway
// Validate/convert BM
// bm gets assigned to the new reference
// bm = this.validateAndConvert(bm, this.cfg.bmSchema, DBModelType.BM, opt)

// should not do it on load, but only on save!
// this.assignIdCreatedUpdated(bm, opt)

// bm gets assigned to the new reference
bm = this.validateAndConvert(bm, this.cfg.bmSchema, opt)

// BM > DBM
const dbm = { ...(await this.cfg.hooks!.beforeBMToDBM!(bm)) }
let dbm: DBM
if (this.cfg.hooks!.beforeBMToDBM) {
dbm = { ...((await this.cfg.hooks!.beforeBMToDBM(bm!)) as DBM) }
} else {
dbm = bm as any
}

// Validate/convert DBM

return this.validateAndConvert(dbm, this.cfg.dbmSchema, DBModelType.DBM, opt)
// return this.validateAndConvert(dbm, this.cfg.dbmSchema, DBModelType.DBM, opt)
return dbm
}

async bmsToDBM(bms: BM[], opt: CommonDaoOptions = {}): Promise<DBM[]> {
Expand All @@ -1164,12 +1162,15 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {

dbm = { ...dbm, ...this.cfg.hooks!.parseNaturalId!(dbm.id) }

// todo: is this the right place?
// todo: is anyToDBM even needed?
if (opt.anonymize) {
dbm = this.cfg.hooks!.anonymize!(dbm)
}

// Validate/convert DBM
return this.validateAndConvert(dbm, this.cfg.dbmSchema, DBModelType.DBM, opt)
// return this.validateAndConvert(dbm, this.cfg.dbmSchema, DBModelType.DBM, opt)
return dbm
}

anyToDBMs(entities: DBM[], opt: CommonDaoOptions = {}): DBM[] {
Expand All @@ -1185,7 +1186,6 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {
validateAndConvert<T>(
obj: Partial<T>,
schema: ObjectSchema<T> | AjvSchema<T> | ZodSchema<T> | undefined,
modelType?: DBModelType,
opt: CommonDaoOptions = {},
): any {
// Kirill 2021-10-18: I realized that there's little reason to keep removing null values
Expand All @@ -1197,16 +1197,7 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {
// obj = _filterNullishValues(obj as any)
// We still filter `undefined` values here, because `beforeDBMToBM` can return undefined values
// and they can be annoying with snapshot tests
if (this.cfg.filterNullishValues) {
obj = _filterNullishValues(obj)
} else {
obj = _filterUndefinedValues(obj)
}

// Pre-validation hooks
if (modelType === DBModelType.DBM) {
obj = this.cfg.hooks!.beforeDBMValidate!(obj as any) as T
}
obj = _filterUndefinedValues(obj)

// Return as is if no schema is passed or if `skipConversion` is set
if (!schema || opt.skipConversion) {
Expand All @@ -1215,7 +1206,7 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM> {

// This will Convert and Validate
const table = opt.table || this.cfg.table
const objectName = table + (modelType || '')
const objectName = table

let error: JoiValidationError | AjvValidationError | ZodValidationError<T> | undefined
let convertedValue: any
Expand Down
6 changes: 2 additions & 4 deletions src/testing/daoTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import { CommonDBImplementationQuirks, expectMatch } from './dbTest'
import {
createTestItemsBM,
testItemBMSchema,
testItemDBMSchema,
TEST_TABLE,
createTestItemBM,
testItemDBMJsonSchema,
testItemBMJsonSchema,
} from './test.model'
import { TestItemBM } from '.'

Expand All @@ -20,7 +19,6 @@ export function runCommonDaoTest(db: CommonDB, quirks: CommonDBImplementationQui
const dao = new CommonDao({
table: TEST_TABLE,
db,
dbmSchema: testItemDBMSchema,
bmSchema: testItemBMSchema,
logStarted: true,
logLevel: CommonDaoLogLevel.DATA_FULL,
Expand All @@ -43,7 +41,7 @@ export function runCommonDaoTest(db: CommonDB, quirks: CommonDBImplementationQui
// CREATE TABLE, DROP
if (support.createTable) {
test('createTable, dropIfExists=true', async () => {
await dao.createTable(testItemDBMJsonSchema, { dropIfExists: true })
await dao.createTable(testItemBMJsonSchema, { dropIfExists: true })
})
}

Expand Down
4 changes: 2 additions & 2 deletions src/testing/dbTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import {
createTestItemDBM,
createTestItemsDBM,
TEST_TABLE,
testItemBMJsonSchema,
TestItemDBM,
testItemDBMJsonSchema,
} from './test.model'
import { deepFreeze } from './test.util'

Expand Down Expand Up @@ -42,7 +42,7 @@ export function runCommonDBTest(db: CommonDB, quirks: CommonDBImplementationQuir
// CREATE TABLE, DROP
if (support.createTable) {
test('createTable, dropIfExists=true', async () => {
await db.createTable(TEST_TABLE, testItemDBMJsonSchema, { dropIfExists: true })
await db.createTable(TEST_TABLE, testItemBMJsonSchema, { dropIfExists: true })
})
}

Expand Down
4 changes: 0 additions & 4 deletions src/testing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import {
testItemBMJsonSchema,
testItemBMSchema,
TestItemDBM,
testItemDBMJsonSchema,
testItemDBMSchema,
TestItemTM,
testItemTMSchema,
TEST_TABLE,
Expand All @@ -25,11 +23,9 @@ export {
createTestItemBM,
createTestItemsDBM,
createTestItemsBM,
testItemDBMSchema,
testItemBMSchema,
testItemTMSchema,
testItemBMJsonSchema,
testItemDBMJsonSchema,
runCommonDBTest,
runCommonDaoTest,
runCommonKeyValueDBTest,
Expand Down
22 changes: 0 additions & 22 deletions src/testing/test.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,6 @@ export const testItemBMSchema = objectSchema<TestItemBM>({
b1: binarySchema.optional(),
}).concat(baseDBEntitySchema as any)

export const testItemDBMSchema = objectSchema<TestItemDBM>({
k1: stringSchema,
k2: stringSchema.allow(null).optional(),
k3: numberSchema.optional(),
even: booleanSchema.optional(),
b1: binarySchema.optional(),
}).concat(baseDBEntitySchema as any)

export const testItemTMSchema = objectSchema<TestItemTM>({
k1: stringSchema,
even: booleanSchema.optional(),
Expand All @@ -63,20 +55,6 @@ export const testItemBMJsonSchema: JsonSchemaObject<TestItemBM> = jsonSchema
.baseDBEntity()
.build()

export const testItemDBMJsonSchema: JsonSchemaObject<TestItemDBM> = jsonSchema
.rootObject<TestItemDBM>({
// todo: figure out how to not copy-paste these 3 fields
id: jsonSchema.string(),
created: jsonSchema.unixTimestamp(),
updated: jsonSchema.unixTimestamp(),
k1: jsonSchema.string(),
k2: jsonSchema.string().optional(),
k3: jsonSchema.number().optional(),
even: jsonSchema.boolean().optional(),
b1: jsonSchema.buffer().optional(),
})
.build()

export function createTestItemDBM(num = 1): TestItemDBM {
return {
id: `id${num}`,
Expand Down

0 comments on commit 840bb03

Please sign in to comment.