From ade635f5f95513234a09be98554b3f4a77e34dd4 Mon Sep 17 00:00:00 2001 From: Zao Soula Date: Mon, 18 Nov 2024 10:19:37 +0100 Subject: [PATCH 01/16] feat: add meta to all relations (#1042) --- src/orm/relations/belongs_to/index.ts | 3 +++ src/orm/relations/has_many/index.ts | 3 +++ src/orm/relations/has_many_through/index.ts | 3 +++ src/orm/relations/has_one/index.ts | 3 +++ src/orm/relations/many_to_many/index.ts | 3 +++ src/types/relations.ts | 3 +++ 6 files changed, 18 insertions(+) diff --git a/src/orm/relations/belongs_to/index.ts b/src/orm/relations/belongs_to/index.ts index 3bf70bf3..9505154a 100644 --- a/src/orm/relations/belongs_to/index.ts +++ b/src/orm/relations/belongs_to/index.ts @@ -59,6 +59,8 @@ export class BelongsTo implements BelongsToRelationContract LucidModel, @@ -68,6 +70,7 @@ export class BelongsTo implements BelongsToRelationContract */ onQueryHook + declare meta?: any + constructor( public relationName: string, public relatedModel: () => LucidModel, @@ -70,6 +72,7 @@ export class HasMany implements HasManyRelationContract this.serializeAs = this.options.serializeAs === undefined ? this.relationName : this.options.serializeAs this.onQueryHook = this.options.onQuery + this.meta = this.options.meta } /** diff --git a/src/orm/relations/has_many_through/index.ts b/src/orm/relations/has_many_through/index.ts index d71510c7..67a9cdd8 100644 --- a/src/orm/relations/has_many_through/index.ts +++ b/src/orm/relations/has_many_through/index.ts @@ -59,6 +59,8 @@ export class HasManyThrough implements HasManyThroughRelationContract LucidModel, @@ -75,6 +77,7 @@ export class HasManyThrough implements HasManyThroughRelationContract { */ onQueryHook + declare meta?: any + constructor( public relationName: string, public relatedModel: () => LucidModel, @@ -57,6 +59,7 @@ export class HasOne implements HasOneRelationContract { this.onQueryHook = this.options.onQuery this.serializeAs = this.options.serializeAs === undefined ? this.relationName : this.options.serializeAs + this.meta = this.options.meta } /** diff --git a/src/orm/relations/many_to_many/index.ts b/src/orm/relations/many_to_many/index.ts index d1481f12..5a42939f 100644 --- a/src/orm/relations/many_to_many/index.ts +++ b/src/orm/relations/many_to_many/index.ts @@ -62,6 +62,8 @@ export class ManyToMany implements ManyToManyRelationContract LucidModel + meta?: any } /** From 65790bfa0c66902b88de60ccf5f965548a143310 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Wed, 27 Nov 2024 12:57:56 +0530 Subject: [PATCH 02/16] fix: use correct property name for reading local primary key value FIXES: #1069 --- src/orm/base_model/index.ts | 8 +++---- test/orm/base_model.spec.ts | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/orm/base_model/index.ts b/src/orm/base_model/index.ts index 93affd18..fc540599 100644 --- a/src/orm/base_model/index.ts +++ b/src/orm/base_model/index.ts @@ -2106,10 +2106,8 @@ class BaseModelImpl implements LucidRow { client: QueryClientContract ): any { const modelConstructor = this.constructor as typeof BaseModel - const primaryKeyColumn = modelConstructor.$keys.attributesToColumns.get( - modelConstructor.primaryKey, - modelConstructor.primaryKey - ) + const primaryKey = modelConstructor.primaryKey + const primaryKeyColumn = modelConstructor.$keys.attributesToColumns.get(primaryKey, primaryKey) /** * Returning insert query for the inserts @@ -2126,7 +2124,7 @@ class BaseModelImpl implements LucidRow { * updating primary key itself */ const primaryKeyValue = modelConstructor.selfAssignPrimaryKey - ? this.$original[primaryKeyColumn] + ? this.$original[primaryKey] : this.$primaryKeyValue /** diff --git a/test/orm/base_model.spec.ts b/test/orm/base_model.spec.ts index 1feddbbb..a35709dd 100644 --- a/test/orm/base_model.spec.ts +++ b/test/orm/base_model.spec.ts @@ -1402,6 +1402,49 @@ test.group('Base Model | persist', (group) => { assert.lengthOf(users, 1) assert.equal(users[0].id.toLowerCase(), newUuid) }) + + test('use custom name for the local primary key', async ({ fs, assert }) => { + const app = new AppFactory().create(fs.baseUrl, () => {}) + await app.init() + const db = await getDb() + const adapter = ormAdapter(db) + + const BaseModel = getBaseModel(adapter) + + class User extends BaseModel { + static table = 'uuid_users' + static selfAssignPrimaryKey = true + + @column({ isPrimary: true, columnName: 'id' }) + declare userId: string + + @column() + declare username: string + + @column() + declare createdAt: string + + @column({ columnName: 'updated_at' }) + declare updatedAt: string + } + + User.boot() + + const uuid = '2da96a33-57a0-4752-9d56-0e2485d4d2a4' + + const user = new User() + user.userId = uuid + user.username = 'virk' + await user.save() + + const newUuid = '4da96a33-57a0-4752-9d56-0e2485d4d2a1' + user.userId = newUuid + + await user.save() + const users = await User.all() + assert.lengthOf(users, 1) + assert.equal(users[0].userId.toLowerCase(), newUuid) + }) }) test.group('Self assign primary key', () => { From 9bf48b6f7ed222022ff9407d275a49494bb73271 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Wed, 27 Nov 2024 13:02:34 +0530 Subject: [PATCH 03/16] style: fix linting issue --- commands/db_seed.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/db_seed.ts b/commands/db_seed.ts index a6b71ee3..f4496938 100644 --- a/commands/db_seed.ts +++ b/commands/db_seed.ts @@ -23,7 +23,7 @@ export default class DbSeed extends BaseCommand { startApp: true, } - private declare seeder: SeedsRunner + declare private seeder: SeedsRunner /** * Track if one or more seeders have failed From 4e8f23268e5db694ca7d98301d371db6005dc667 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Thu, 28 Nov 2024 09:02:13 +0100 Subject: [PATCH 04/16] chore(errors): improve message for missing attribute error --- src/errors.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/errors.ts b/src/errors.ts index 2ccb60f9..b4cc6b93 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -22,7 +22,7 @@ export const E_UNMANAGED_DB_CONNECTION = createError<[string]>( ) export const E_MISSING_MODEL_ATTRIBUTE = createError<[string, string, string]>( - '"%s" expects "%s" to exist on "%s" model, but is missing', + 'Relation "%s" expects "%s" to exist on "%s" model, but is missing. Did you forget to define the column?', 'E_MISSING_MODEL_ATTRIBUTE', 500 ) From 0ff68f23f33970a6ca553516a31d31e65b066cb4 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Thu, 28 Nov 2024 09:12:19 +0100 Subject: [PATCH 05/16] tests(errors): improve message for missing attribute error --- test/orm/model_belongs_to.spec.ts | 7 +++++-- test/orm/model_has_many.spec.ts | 7 +++++-- test/orm/model_has_many_through.spec.ts | 11 +++++++---- test/orm/model_has_one.spec.ts | 7 +++++-- test/orm/model_many_to_many.spec.ts | 10 ++++++++-- 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/test/orm/model_belongs_to.spec.ts b/test/orm/model_belongs_to.spec.ts index 41b30a20..4b547fbf 100644 --- a/test/orm/model_belongs_to.spec.ts +++ b/test/orm/model_belongs_to.spec.ts @@ -53,7 +53,10 @@ test.group('Model | BelongsTo | Options', (group) => { Profile.boot() Profile.$getRelation('user')!.boot() } catch ({ message }) { - assert.equal(message, '"Profile.user" expects "id" to exist on "User" model, but is missing') + assert.equal( + message, + 'Relation "Profile.user" expects "id" to exist on "User" model, but is missing. Did you forget to define the column?' + ) } }) @@ -84,7 +87,7 @@ test.group('Model | BelongsTo | Options', (group) => { } catch ({ message }) { assert.equal( message, - '"Profile.user" expects "userId" to exist on "Profile" model, but is missing' + 'Relation "Profile.user" expects "userId" to exist on "Profile" model, but is missing. Did you forget to define the column?' ) } }) diff --git a/test/orm/model_has_many.spec.ts b/test/orm/model_has_many.spec.ts index b602a980..86835702 100644 --- a/test/orm/model_has_many.spec.ts +++ b/test/orm/model_has_many.spec.ts @@ -55,7 +55,10 @@ test.group('Model | HasMany | Options', (group) => { User.boot() User.$getRelation('posts')!.boot() } catch ({ message }) { - assert.equal(message, '"User.posts" expects "id" to exist on "User" model, but is missing') + assert.equal( + message, + 'Relation "User.posts" expects "id" to exist on "User" model, but is missing. Did you forget to define the column?' + ) } }) @@ -85,7 +88,7 @@ test.group('Model | HasMany | Options', (group) => { } catch ({ message }) { assert.equal( message, - '"User.posts" expects "userId" to exist on "Post" model, but is missing' + 'Relation "User.posts" expects "userId" to exist on "Post" model, but is missing. Did you forget to define the column?' ) } }) diff --git a/test/orm/model_has_many_through.spec.ts b/test/orm/model_has_many_through.spec.ts index 0f6841a8..f99f57ee 100644 --- a/test/orm/model_has_many_through.spec.ts +++ b/test/orm/model_has_many_through.spec.ts @@ -58,7 +58,7 @@ test.group('Model | Has Many Through | Options', (group) => { } catch ({ message }) { assert.equal( message, - '"Country.posts" expects "id" to exist on "Country" model, but is missing' + 'Relation "Country.posts" expects "id" to exist on "Country" model, but is missing. Did you forget to define the column?' ) } }) @@ -92,7 +92,7 @@ test.group('Model | Has Many Through | Options', (group) => { } catch ({ message }) { assert.equal( message, - '"Country.posts" expects "countryId" to exist on "User" model, but is missing' + 'Relation "Country.posts" expects "countryId" to exist on "User" model, but is missing. Did you forget to define the column?' ) } }) @@ -127,7 +127,10 @@ test.group('Model | Has Many Through | Options', (group) => { Country.boot() Country.$getRelation('posts')!.boot() } catch ({ message }) { - assert.equal(message, '"Country.posts" expects "id" to exist on "User" model, but is missing') + assert.equal( + message, + 'Relation "Country.posts" expects "id" to exist on "User" model, but is missing. Did you forget to define the column?' + ) } }) @@ -166,7 +169,7 @@ test.group('Model | Has Many Through | Options', (group) => { } catch ({ message }) { assert.equal( message, - '"Country.posts" expects "userId" to exist on "Post" model, but is missing' + 'Relation "Country.posts" expects "userId" to exist on "Post" model, but is missing. Did you forget to define the column?' ) } }) diff --git a/test/orm/model_has_one.spec.ts b/test/orm/model_has_one.spec.ts index 5f2120c1..72aace33 100644 --- a/test/orm/model_has_one.spec.ts +++ b/test/orm/model_has_one.spec.ts @@ -53,7 +53,10 @@ test.group('Model | HasOne | Options', (group) => { User.boot() User.$getRelation('profile')!.boot() } catch ({ message }) { - assert.equal(message, '"User.profile" expects "id" to exist on "User" model, but is missing') + assert.equal( + message, + 'Relation "User.profile" expects "id" to exist on "User" model, but is missing. Did you forget to define the column?' + ) } }) @@ -83,7 +86,7 @@ test.group('Model | HasOne | Options', (group) => { } catch ({ message }) { assert.equal( message, - '"User.profile" expects "userId" to exist on "Profile" model, but is missing' + 'Relation "User.profile" expects "userId" to exist on "Profile" model, but is missing. Did you forget to define the column?' ) } }) diff --git a/test/orm/model_many_to_many.spec.ts b/test/orm/model_many_to_many.spec.ts index c1444f57..42bae9f3 100644 --- a/test/orm/model_many_to_many.spec.ts +++ b/test/orm/model_many_to_many.spec.ts @@ -57,7 +57,10 @@ test.group('Model | ManyToMany | Options', (group) => { User.boot() User.$getRelation('skills')!.boot() } catch ({ message }) { - assert.equal(message, '"User.skills" expects "id" to exist on "User" model, but is missing') + assert.equal( + message, + 'Relation "User.skills" expects "id" to exist on "User" model, but is missing. Did you forget to define the column?' + ) } }) @@ -141,7 +144,10 @@ test.group('Model | ManyToMany | Options', (group) => { User.boot() User.$getRelation('skills')!.boot() } catch ({ message }) { - assert.equal(message, '"User.skills" expects "id" to exist on "Skill" model, but is missing') + assert.equal( + message, + 'Relation "User.skills" expects "id" to exist on "Skill" model, but is missing. Did you forget to define the column?' + ) } }) From 35821eb95189b8a868f37f9c9e444dccf22d2ed6 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Wed, 4 Dec 2024 07:21:52 +0530 Subject: [PATCH 06/16] chore: update dependencies --- package.json | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 82e865a9..d1bb1e34 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ }, "devDependencies": { "@adonisjs/assembler": "^7.8.2", - "@adonisjs/core": "^6.14.1", + "@adonisjs/core": "^6.15.2", "@adonisjs/eslint-config": "^2.0.0-beta.7", "@adonisjs/prettier-config": "^1.4.0", "@adonisjs/tsconfig": "^1.4.0", @@ -69,36 +69,36 @@ "@japa/runner": "^3.1.4", "@libsql/sqlite3": "^0.3.1", "@release-it/conventional-changelog": "^9.0.3", - "@swc/core": "^1.9.2", + "@swc/core": "^1.9.3", "@types/chance": "^1.1.6", "@types/luxon": "^3.4.2", - "@types/node": "^22.9.0", + "@types/node": "^22.10.1", "@types/pretty-hrtime": "^1.0.3", "@types/qs": "^6.9.17", - "@vinejs/vine": "^2.1.0", - "better-sqlite3": "^11.5.0", + "@vinejs/vine": "^3.0.0", + "better-sqlite3": "^11.6.0", "c8": "^10.1.2", "chance": "^1.1.12", "copyfiles": "^2.4.1", "cross-env": "^7.0.3", "del-cli": "^6.0.0", - "dotenv": "^16.4.5", - "eslint": "^9.14.0", + "dotenv": "^16.4.7", + "eslint": "^9.16.0", "fs-extra": "^11.2.0", "luxon": "^3.5.0", - "mysql2": "^3.11.4", + "mysql2": "^3.11.5", "pg": "^8.13.1", - "prettier": "^3.3.3", + "prettier": "^3.4.1", "reflect-metadata": "^0.2.2", "release-it": "^17.10.0", "sqlite3": "^5.1.7", "tedious": "^18.6.1", "ts-node": "^10.9.2", - "typescript": "^5.6.3" + "typescript": "^5.7.2" }, "dependencies": { "@adonisjs/presets": "^2.6.3", - "@faker-js/faker": "^9.2.0", + "@faker-js/faker": "^9.3.0", "@poppinss/hooks": "^7.2.4", "@poppinss/macroable": "^1.0.3", "@poppinss/utils": "^6.8.3", @@ -108,19 +108,23 @@ "knex": "^3.1.0", "knex-dynamic-connection": "^3.2.0", "pretty-hrtime": "^1.0.3", - "qs": "^6.13.0", + "qs": "^6.13.1", "slash": "^5.1.0", "tarn": "^3.0.2" }, "peerDependencies": { "@adonisjs/assembler": "^7.7.0", "@adonisjs/core": "^6.10.1", + "@vinejs/vine": "^2.0.0 || ^3.0.0", "luxon": "^3.4.4" }, "peerDependenciesMeta": { "@adonisjs/assembler": { "optional": true }, + "@vinejs/vine": { + "optional": true + }, "luxon": { "optional": true } From 4a2adcd696430d06db5bda103635c2ef6dc74c44 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Wed, 4 Dec 2024 07:31:21 +0530 Subject: [PATCH 07/16] feat: add support for creating transaction client from model --- src/orm/base_model/index.ts | 20 +++++++++++++++++- src/types/model.ts | 15 +++++++++++++- test/orm/base_model.spec.ts | 41 +++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/orm/base_model/index.ts b/src/orm/base_model/index.ts index fc540599..6244fe5c 100644 --- a/src/orm/base_model/index.ts +++ b/src/orm/base_model/index.ts @@ -11,7 +11,11 @@ import { DateTime } from 'luxon' import Hooks from '@poppinss/hooks' import lodash from '@poppinss/utils/lodash' import { Exception, defineStaticProperty } from '@poppinss/utils' -import { QueryClientContract, TransactionClientContract } from '../../types/database.js' +import { + IsolationLevels, + QueryClientContract, + TransactionClientContract, +} from '../../types/database.js' import { LucidRow, @@ -241,6 +245,20 @@ class BaseModelImpl implements LucidRow { return this.$adapter.query(this, options) } + /** + * Returns the model query instance for the given model + */ + static async transaction( + options?: ModelAdapterOptions & { isolationLevel?: IsolationLevels } + ): Promise { + const client = this.$adapter.modelConstructorClient(this, options) + if (client.isTransaction) { + return client as TransactionClientContract + } + + return client.transaction() + } + /** * Create a model instance from the adapter result. The result value must * be a valid object, otherwise `null` is returned. diff --git a/src/types/model.ts b/src/types/model.ts index 7e698d17..db2d9459 100644 --- a/src/types/model.ts +++ b/src/types/model.ts @@ -9,7 +9,12 @@ import { DateTime } from 'luxon' import type Hooks from '@poppinss/hooks' -import { DialectContract, QueryClientContract, TransactionClientContract } from './database.js' +import { + DialectContract, + IsolationLevels, + QueryClientContract, + TransactionClientContract, +} from './database.js' import { Update, @@ -1143,6 +1148,14 @@ export interface LucidModel { options?: ModelAdapterOptions ): ModelQueryBuilderContract + /** + * Returns transaction client from the model. It is same as + * calling "db.transaction" + */ + transaction( + options?: ModelAdapterOptions & { isolationLevel?: IsolationLevels } + ): Promise + /** * Truncate model table */ diff --git a/test/orm/base_model.spec.ts b/test/orm/base_model.spec.ts index a35709dd..6082709e 100644 --- a/test/orm/base_model.spec.ts +++ b/test/orm/base_model.spec.ts @@ -8277,3 +8277,44 @@ test.group('Base Model | lockForUpdate', (group) => { ) }) }) + +test.group('Base Model | transaction', (group) => { + group.setup(async () => { + await setup() + }) + + group.teardown(async () => { + await cleanupTables() + }) + + group.each.teardown(async () => { + await resetTables() + }) + + test('create transaction client using model', async ({ fs, assert }) => { + const app = new AppFactory().create(fs.baseUrl, () => {}) + await app.init() + const db = getDb() + const adapter = ormAdapter(db) + + const BaseModel = getBaseModel(adapter) + + class User extends BaseModel { + @column({ isPrimary: true }) + declare id: number + + @column() + declare username: string + + @column() + declare email: string + } + + const client = await User.transaction() + await client.insertQuery().table('users').insert({ username: 'virk' }) + await client.rollback() + const user = await User.find(1) + + assert.isNull(user) + }) +}) From 151f862891d8b997e5432597a659837f3e559a15 Mon Sep 17 00:00:00 2001 From: Andrej Adamcik Date: Wed, 4 Dec 2024 06:10:44 +0100 Subject: [PATCH 08/16] feat: add isDirty method (#1068) --- src/orm/base_model/index.ts | 13 +++++++++++++ src/types/model.ts | 2 ++ test/orm/base_model.spec.ts | 29 +++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/orm/base_model/index.ts b/src/orm/base_model/index.ts index 6244fe5c..beb472bc 100644 --- a/src/orm/base_model/index.ts +++ b/src/orm/base_model/index.ts @@ -1784,6 +1784,19 @@ class BaseModelImpl implements LucidRow { return this } + /** + * Returns whether any of the fields have been modified + */ + isDirty(fields?: any): boolean { + const keys = Array.isArray(fields) ? fields : fields ? [fields] : [] + + if (keys.length === 0) { + return this.$isDirty + } + + return keys.some((key) => key in this.$dirty) + } + /** * Enable force update even when no attributes * are dirty diff --git a/src/types/model.ts b/src/types/model.ts index db2d9459..e178a945 100644 --- a/src/types/model.ts +++ b/src/types/model.ts @@ -618,6 +618,8 @@ export interface LucidRow { fill(value: Partial>, allowExtraProperties?: boolean): this merge(value: Partial>, allowExtraProperties?: boolean): this + isDirty(fields?: keyof ModelAttributes | (keyof ModelAttributes)[]): boolean + /** * Enable force update even when no attributes * are dirty diff --git a/test/orm/base_model.spec.ts b/test/orm/base_model.spec.ts index 6082709e..630a10a2 100644 --- a/test/orm/base_model.spec.ts +++ b/test/orm/base_model.spec.ts @@ -813,6 +813,35 @@ test.group('Base Model | dirty', (group) => { user.location.isDirty = true assert.deepEqual(user.$dirty, { location: { state: 'goa', country: 'India', isDirty: true } }) }) + + test('isDirty returns whether field is dirty', async ({ fs, assert }) => { + const app = new AppFactory().create(fs.baseUrl, () => {}) + await app.init() + const db = getDb() + const adapter = ormAdapter(db) + + const BaseModel = getBaseModel(adapter) + + class User extends BaseModel { + @column() + declare username: string + + @column() + declare email: string + } + + const user = new User() + + assert.isFalse(user.isDirty()) + assert.isFalse(user.isDirty('username')) + + user.username = 'virk' + + assert.isTrue(user.isDirty()) + assert.isTrue(user.isDirty('username')) + assert.isFalse(user.isDirty('email')) + assert.isTrue(user.isDirty(['username', 'email'])) + }) }) test.group('Base Model | persist', (group) => { From 62a1ba5beee7d1b0ff5bad85b35420c3834e8724 Mon Sep 17 00:00:00 2001 From: Xavier Stouder Date: Wed, 4 Dec 2024 07:06:17 +0100 Subject: [PATCH 09/16] refactor: improvements to the unique rule (#1012) --- providers/database_provider.ts | 15 ++++++++++ src/bindings/vinejs.ts | 50 +++++++++++++++++++++++++++------- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/providers/database_provider.ts b/providers/database_provider.ts index cb06a8f4..2c8f555e 100644 --- a/providers/database_provider.ts +++ b/providers/database_provider.ts @@ -16,6 +16,7 @@ import { QueryClient } from '../src/query_client/index.js' import { BaseModel } from '../src/orm/base_model/index.js' import { DatabaseTestUtils } from '../src/test_utils/database.js' import type { DatabaseConfig, DbQueryEventNode } from '../src/types/database.js' +import { DatabaseQueryBuilderContract } from '../src/types/querybuilder.js' /** * Extending AdonisJS types @@ -49,6 +50,20 @@ declare module '@vinejs/vine' { */ unique(callback: (db: Database, value: string, field: FieldContext) => Promise): this + /** + * Ensure the value is unique inside the database by table and column name. + * Optionally, you can define a filter to narrow down the query. + */ + unique(options: { + table: string + column?: string + filter?: ( + db: DatabaseQueryBuilderContract, + value: unknown, + field: FieldContext + ) => Promise + }): this + /** * Ensure the value is exists inside the database by self * executing a query. diff --git a/src/bindings/vinejs.ts b/src/bindings/vinejs.ts index a048350e..87f84c50 100644 --- a/src/bindings/vinejs.ts +++ b/src/bindings/vinejs.ts @@ -9,24 +9,54 @@ import vine, { VineNumber, VineString } from '@vinejs/vine' import type { Database } from '../database/main.js' +import type { FieldContext } from '@vinejs/vine/types' +import { DatabaseQueryBuilderContract } from '../types/querybuilder.js' /** * Defines the "unique" and "exists" validation rules with * VineJS. */ export function defineValidationRules(db: Database) { - const uniqueRule = vine.createRule[0]>( - async (value, checker, field) => { - if (!field.isValid) { - return + const uniqueRule = vine.createRule< + | ((db: Database, value: string, field: FieldContext) => Promise) + | { + table: string + column?: string + filter?: ( + db: DatabaseQueryBuilderContract, + value: unknown, + field: FieldContext + ) => Promise } + >(async (value, checkerOrOptions, field) => { + if (!field.isValid) { + return + } - const isUnique = await checker(db, value as string, field) + if (typeof checkerOrOptions === 'function') { + const isUnique = await checkerOrOptions(db, value as string, field) if (!isUnique) { field.report('The {{ field }} has already been taken', 'database.unique', field) } + return } - ) + + if (typeof value !== 'string') { + return + } + + if (typeof field.name !== 'string') { + return + } + + const { table, column = field.name, filter } = checkerOrOptions + const baseQuery = db.from(table).select(column).where(column, value) + await filter?.(baseQuery, value, field) + const row = await baseQuery.first() + if (row) { + field.report('The {{ field }} has already been taken', 'database.unique', field) + } + }) const existsRule = vine.createRule[0]>( async (value, checker, field) => { @@ -41,14 +71,14 @@ export function defineValidationRules(db: Database) { } ) - VineString.macro('unique', function (this: VineString, checker) { - return this.use(uniqueRule(checker)) + VineString.macro('unique', function (this: VineString, checkerOrOptions) { + return this.use(uniqueRule(checkerOrOptions)) }) VineString.macro('exists', function (this: VineString, checker) { return this.use(existsRule(checker)) }) - VineNumber.macro('unique', function (this: VineNumber, checker) { - return this.use(uniqueRule(checker)) + VineNumber.macro('unique', function (this: VineNumber, checkerOrOptions) { + return this.use(uniqueRule(checkerOrOptions)) }) VineNumber.macro('exists', function (this: VineNumber, checker) { return this.use(existsRule(checker)) From 761f8231c56891c1f4682cf9a92637060e83346d Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Wed, 4 Dec 2024 17:22:46 +0530 Subject: [PATCH 10/16] feat: allow unique and exists validations to perform case insensitive search --- providers/database_provider.ts | 36 ++- src/bindings/vinejs.ts | 149 ++++++++---- src/types/vine.ts | 65 +++++ test/bindings/vinejs.spec.ts | 432 +++++++++++++++++++++++++++++++++ test/database_provider.spec.ts | 9 + 5 files changed, 622 insertions(+), 69 deletions(-) create mode 100644 src/types/vine.ts create mode 100644 test/bindings/vinejs.spec.ts diff --git a/providers/database_provider.ts b/providers/database_provider.ts index 2c8f555e..76a8113e 100644 --- a/providers/database_provider.ts +++ b/providers/database_provider.ts @@ -1,13 +1,12 @@ /* * @adonisjs/lucid * - * (c) Harminder Virk + * (c) AdonisJS * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ -import type { FieldContext } from '@vinejs/vine/types' import type { ApplicationService } from '@adonisjs/core/types' import { Database } from '../src/database/main.js' @@ -16,7 +15,7 @@ import { QueryClient } from '../src/query_client/index.js' import { BaseModel } from '../src/orm/base_model/index.js' import { DatabaseTestUtils } from '../src/test_utils/database.js' import type { DatabaseConfig, DbQueryEventNode } from '../src/types/database.js' -import { DatabaseQueryBuilderContract } from '../src/types/querybuilder.js' +import { VineDbSearchCallback, VineDbSearchOptions } from '../src/types/vine.js' /** * Extending AdonisJS types @@ -40,7 +39,13 @@ declare module '@adonisjs/core/test_utils' { * Extending VineJS schema types */ declare module '@vinejs/vine' { - interface VineLucidBindings { + interface VineLucidBindings { + /** + * Ensure the value is unique inside the database by table and column name. + * Optionally, you can define a filter to narrow down the query. + */ + unique(options: VineDbSearchOptions): this + /** * Ensure the value is unique inside the database by self * executing a query. @@ -48,35 +53,26 @@ declare module '@vinejs/vine' { * - The callback must return "true", if the value is unique (does not exist). * - The callback must return "false", if the value is not unique (already exists). */ - unique(callback: (db: Database, value: string, field: FieldContext) => Promise): this + unique(callback: VineDbSearchCallback): this /** - * Ensure the value is unique inside the database by table and column name. + * Ensure the value exists inside the database by table and column name. * Optionally, you can define a filter to narrow down the query. */ - unique(options: { - table: string - column?: string - filter?: ( - db: DatabaseQueryBuilderContract, - value: unknown, - field: FieldContext - ) => Promise - }): this + exists(options: VineDbSearchOptions): this /** - * Ensure the value is exists inside the database by self + * Ensure the value exists inside the database by self * executing a query. * * - The callback must return "false", if the value exists. * - The callback must return "true", if the value does not exist. */ - exists(callback: (db: Database, value: string, field: FieldContext) => Promise): this + exists(callback: VineDbSearchCallback): this } - interface VineNumber extends VineLucidBindings {} - - interface VineString extends VineLucidBindings {} + interface VineNumber extends VineLucidBindings {} + interface VineString extends VineLucidBindings {} } /** diff --git a/src/bindings/vinejs.ts b/src/bindings/vinejs.ts index 87f84c50..45c0c6dc 100644 --- a/src/bindings/vinejs.ts +++ b/src/bindings/vinejs.ts @@ -7,80 +7,131 @@ * file that was distributed with this source code. */ -import vine, { VineNumber, VineString } from '@vinejs/vine' import type { Database } from '../database/main.js' -import type { FieldContext } from '@vinejs/vine/types' -import { DatabaseQueryBuilderContract } from '../types/querybuilder.js' +import vine, { VineNumber, VineString } from '@vinejs/vine' +import { VineDbSearchCallback, VineDbSearchOptions } from '../types/vine.js' + +/** + * Default validation messages used by the unique and the + * exists rules + */ +export const messages = { + 'database.unique': 'The {{ field }} has already been taken', + 'database.exists': 'The selected {{ field }} is invalid', +} /** * Defines the "unique" and "exists" validation rules with * VineJS. */ export function defineValidationRules(db: Database) { - const uniqueRule = vine.createRule< - | ((db: Database, value: string, field: FieldContext) => Promise) - | { - table: string - column?: string - filter?: ( - db: DatabaseQueryBuilderContract, - value: unknown, - field: FieldContext - ) => Promise + const uniqueRule = vine.createRule( + async (value, callbackOrOptions, field) => { + if (!field.isValid) { + return } - >(async (value, checkerOrOptions, field) => { - if (!field.isValid) { - return - } - if (typeof checkerOrOptions === 'function') { - const isUnique = await checkerOrOptions(db, value as string, field) - if (!isUnique) { - field.report('The {{ field }} has already been taken', 'database.unique', field) + /** + * Rely on the callback to execute the query and return value + * a boolean. + * + * True means value is unique + * False means value is not unique + */ + if (typeof callbackOrOptions === 'function') { + const isUnique = await callbackOrOptions(db, value as string, field) + if (!isUnique) { + field.report(messages['database.unique'], 'database.unique', field) + } + return } - return - } - if (typeof value !== 'string') { - return - } + const { table, column, filter, connection, caseInsensitive } = callbackOrOptions + const query = db.connection(connection).from(table).select(column) - if (typeof field.name !== 'string') { - return - } + /** + * Apply where clause respecting the caseInsensitive flag. + */ + if (caseInsensitive) { + query.whereRaw(`lower(${column}) = ?`, [db.raw(`lower(?)`, [value])]) + } else { + query.where(column, value as string) + } + + /** + * Apply user filter + */ + await filter?.(query, value as string, field) - const { table, column = field.name, filter } = checkerOrOptions - const baseQuery = db.from(table).select(column).where(column, value) - await filter?.(baseQuery, value, field) - const row = await baseQuery.first() - if (row) { - field.report('The {{ field }} has already been taken', 'database.unique', field) + /** + * Fetch the first row from the database + */ + const row = await query.first() + if (row) { + field.report(messages['database.unique'], 'database.unique', field) + } } - }) + ) - const existsRule = vine.createRule[0]>( - async (value, checker, field) => { + const existsRule = vine.createRule( + async (value, callbackOrOptions, field) => { if (!field.isValid) { return } - const exists = await checker(db, value as string, field) - if (!exists) { - field.report('The selected {{ field }} is invalid', 'database.exists', field) + /** + * Rely on the callback to execute the query and return value + * a boolean. + * + * True means value exists + * False means value does not exist + */ + if (typeof callbackOrOptions === 'function') { + const exists = await callbackOrOptions(db, value as string, field) + if (!exists) { + field.report(messages['database.exists'], 'database.exists', field) + } + return + } + + const { table, column, filter, connection, caseInsensitive } = callbackOrOptions + const query = db.connection(connection).from(table).select(column) + + /** + * Apply where clause respecting the caseInsensitive flag. + */ + if (caseInsensitive) { + query.whereRaw(`lower(${column}) = ?`, [db.raw(`lower(?)`, [value])]) + } else { + query.where(column, value as string) + } + + /** + * Apply user filter + */ + await filter?.(query, value as string, field) + + /** + * Fetch the first row from the database + */ + const row = await query.first() + if (!row) { + field.report(messages['database.exists'], 'database.exists', field) } } ) - VineString.macro('unique', function (this: VineString, checkerOrOptions) { - return this.use(uniqueRule(checkerOrOptions)) + VineString.macro('unique', function (this: VineString, callbackOrOptions) { + return this.use(uniqueRule(callbackOrOptions)) }) - VineString.macro('exists', function (this: VineString, checker) { - return this.use(existsRule(checker)) + VineString.macro('exists', function (this: VineString, callbackOrOptions) { + return this.use(existsRule(callbackOrOptions)) }) - VineNumber.macro('unique', function (this: VineNumber, checkerOrOptions) { - return this.use(uniqueRule(checkerOrOptions)) + + VineNumber.macro('unique', function (this: VineNumber, callbackOrOptions) { + return this.use(uniqueRule(callbackOrOptions)) }) - VineNumber.macro('exists', function (this: VineNumber, checker) { - return this.use(existsRule(checker)) + VineNumber.macro('exists', function (this: VineNumber, callbackOrOptions) { + return this.use(existsRule(callbackOrOptions)) }) } diff --git a/src/types/vine.ts b/src/types/vine.ts new file mode 100644 index 00000000..3edf6644 --- /dev/null +++ b/src/types/vine.ts @@ -0,0 +1,65 @@ +/* + * @adonisjs/lucid + * + * (c) AdonisJS + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +import type { FieldContext } from '@vinejs/vine/types' +import type { Database } from '../database/main.js' +import type { DatabaseQueryBuilderContract } from './querybuilder.js' + +/** + * Options for the unique and the exists validations + */ +export type VineDbSearchOptions = { + /** + * Database table for the query + */ + table: string + + /** + * The column against which to search the value + */ + column: string + + /** + * Specify a custom connection for the query + */ + connection?: string + + /** + * Enable to perform a case insensitive search on the column. The + * current value and the existing value in the database will be + * lowercased using the "lower" function + * + * https://www.sqlite.org/lang_corefunc.html#lower + * https://docs.aws.amazon.com/redshift/latest/dg/r_LOWER.html + * https://dev.mysql.com/doc/refman/8.0/en/string-functions.html#function_lower + * https://www.postgresql.org/docs/9.1/functions-string.html + * https://docs.microsoft.com/en-us/sql/t-sql/functions/lower-transact-sql?view=sql-server-ver15 + * https://coderwall.com/p/6yhsuq/improve-case-insensitive-queries-in-postgres-using-smarter-indexes + */ + caseInsensitive?: boolean + + /** + * Apply a custom filter to the query builder + */ + filter?: ( + db: DatabaseQueryBuilderContract, + value: string, + field: FieldContext + ) => void | Promise +} + +/** + * Callback to self execute the query for the unique and the + * exists validations + */ +export type VineDbSearchCallback = ( + db: Database, + value: ValueType, + field: FieldContext +) => Promise diff --git a/test/bindings/vinejs.spec.ts b/test/bindings/vinejs.spec.ts new file mode 100644 index 00000000..4d81c0f7 --- /dev/null +++ b/test/bindings/vinejs.spec.ts @@ -0,0 +1,432 @@ +/* + * @adonisjs/lucid + * + * (c) AdonisJS + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +import vine from '@vinejs/vine' +import { test } from '@japa/runner' +import { Database } from '../../src/database/main.js' +import { defineValidationRules } from '../../src/bindings/vinejs.js' +import { getConfig, setup, cleanup, logger, createEmitter } from '../../test-helpers/index.js' + +let db: Database + +test.group('VineJS | unique', (group) => { + group.setup(async () => { + const config = { + connection: 'primary', + connections: { primary: getConfig() }, + } + + db = new Database(config, logger, createEmitter()) + defineValidationRules(db) + }) + + group.teardown(async () => { + await db.manager.closeAll() + }) + + group.each.setup(async () => { + await setup() + return () => cleanup() + }) + + test('fail when value is already in use', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + email: 'foo@bar.com', + }) + + const validator = vine.compile( + vine.object({ + email: vine.string().unique({ + table: 'users', + column: 'email', + }), + }) + ) + + try { + await validator.validate({ + email: 'foo@bar.com', + }) + } catch (error) { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The email has already been taken', + rule: 'database.unique', + }, + ]) + } + }) + + test('perform case-insensitive search', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + username: 'FOO', + email: 'foo@BAR.com', + }) + + const validator = vine.compile( + vine.object({ + username: vine.string().unique({ + table: 'users', + column: 'username', + }), + email: vine.string().unique({ + table: 'users', + column: 'email', + caseInsensitive: true, + }), + }) + ) + + try { + await validator.validate({ + /** + * Username validation will pass because of case + * mismatch + */ + username: 'foo', + /** + * Email validation will fail regardless of the + * case mismatch + */ + email: 'foo@bar.com', + }) + } catch (error) { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The email has already been taken', + rule: 'database.unique', + }, + ]) + } + }) + + test('perform case-insensitive search (all fields)', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + username: 'FOO', + email: 'foo@BAR.com', + }) + + const validator = vine.compile( + vine.object({ + username: vine.string().unique({ + table: 'users', + column: 'username', + caseInsensitive: true, + }), + email: vine.string().unique({ + table: 'users', + column: 'email', + caseInsensitive: true, + }), + }) + ) + + try { + await validator.validate({ + /** + * Username validation will fail regardless of the + * case mismatch + */ + username: 'foo', + /** + * Email validation will fail regardless of the + * case mismatch + */ + email: 'foo@bar.com', + }) + } catch (error) { + assert.deepEqual(error.messages, [ + { + field: 'username', + message: 'The username has already been taken', + rule: 'database.unique', + }, + { + field: 'email', + message: 'The email has already been taken', + rule: 'database.unique', + }, + ]) + } + }) + + test('apply additional filters', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + email: 'foo@bar.com', + }) + + const validator = vine.compile( + vine.object({ + email: vine.string().unique({ + table: 'users', + column: 'email', + filter(query) { + query.whereNotNull('country_id') + }, + }), + }) + ) + + assert.deepEqual( + await validator.validate({ + email: 'foo@bar.com', + }), + { + email: 'foo@bar.com', + } + ) + }) + + test('fail when callback returns false', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + email: 'foo@bar.com', + }) + + const validator = vine.compile( + vine.object({ + email: vine.string().unique(async ($db, value) => { + const row = await $db.from('users').where('email', value).first() + return row ? false : true + }), + }) + ) + + try { + await validator.validate({ + email: 'foo@bar.com', + }) + } catch (error) { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The email has already been taken', + rule: 'database.unique', + }, + ]) + } + }) + + test('pass when callback returns true', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + email: 'foo@bar.com', + }) + + const validator = vine.compile( + vine.object({ + email: vine.string().unique(async ($db, value) => { + const row = await $db + .from('users') + .where('email', value) + .whereNotNull('country_id') + .first() + return row ? false : true + }), + }) + ) + + assert.deepEqual( + await validator.validate({ + email: 'foo@bar.com', + }), + { + email: 'foo@bar.com', + } + ) + }) +}) + +test.group('VineJS | exists', (group) => { + group.setup(async () => { + const config = { + connection: 'primary', + connections: { primary: getConfig() }, + } + + db = new Database(config, logger, createEmitter()) + defineValidationRules(db) + }) + + group.teardown(async () => { + await db.manager.closeAll() + }) + + group.each.setup(async () => { + await setup() + return () => cleanup() + }) + + test('fail when value does not exists', async ({ assert }) => { + assert.plan(1) + + const validator = vine.compile( + vine.object({ + email: vine.string().exists({ + table: 'users', + column: 'email', + }), + }) + ) + + try { + await validator.validate({ + email: 'foo@bar.com', + }) + } catch (error) { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The selected email is invalid', + rule: 'database.exists', + }, + ]) + } + }) + + test('perform case-insensitive search', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + username: 'FOO', + email: 'foo@BAR.com', + }) + + const validator = vine.compile( + vine.object({ + username: vine.string().exists({ + table: 'users', + column: 'username', + }), + email: vine.string().exists({ + table: 'users', + column: 'email', + caseInsensitive: true, + }), + }) + ) + + try { + await validator.validate({ + /** + * Username validation will fail because of case + * mismatch + */ + username: 'foo', + /** + * Email validation will pass regardless of the + * case mismatch + */ + email: 'foo@bar.com', + }) + } catch (error) { + assert.deepEqual(error.messages, [ + { + field: 'username', + message: 'The selected username is invalid', + rule: 'database.exists', + }, + ]) + } + }) + + test('apply additional filters', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + email: 'foo@bar.com', + }) + + const validator = vine.compile( + vine.object({ + email: vine.string().exists({ + table: 'users', + column: 'email', + filter(query) { + query.whereNull('country_id') + }, + }), + }) + ) + + assert.deepEqual( + await validator.validate({ + email: 'foo@bar.com', + }), + { + email: 'foo@bar.com', + } + ) + }) + + test('fail when callback returns false', async ({ assert }) => { + assert.plan(1) + + const validator = vine.compile( + vine.object({ + email: vine.string().exists(async ($db, value) => { + const row = await $db.from('users').where('email', value).first() + return !!row + }), + }) + ) + + try { + await validator.validate({ + email: 'foo@bar.com', + }) + } catch (error) { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The selected email is invalid', + rule: 'database.exists', + }, + ]) + } + }) + + test('pass when callback returns true', async ({ assert }) => { + assert.plan(1) + + await db.table('users').insert({ + email: 'foo@bar.com', + }) + + const validator = vine.compile( + vine.object({ + email: vine.string().exists(async ($db, value) => { + const row = await $db.from('users').where('email', value).first() + return !!row + }), + }) + ) + + assert.deepEqual( + await validator.validate({ + email: 'foo@bar.com', + }), + { + email: 'foo@bar.com', + } + ) + }) +}) diff --git a/test/database_provider.spec.ts b/test/database_provider.spec.ts index 6c254687..47c78380 100644 --- a/test/database_provider.spec.ts +++ b/test/database_provider.spec.ts @@ -1,3 +1,12 @@ +/* + * @adonisjs/lucid + * + * (c) AdonisJS + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + import { test } from '@japa/runner' import { IgnitorFactory } from '@adonisjs/core/factories' From c13678283fb757e2ec79671ad9154357458d026c Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Wed, 4 Dec 2024 17:45:47 +0530 Subject: [PATCH 11/16] test: fix failing tests --- providers/database_provider.ts | 4 +- src/bindings/vinejs.ts | 181 +++++++++++++++++---------------- src/types/vine.ts | 4 +- test/bindings/vinejs.spec.ts | 51 +++++++--- 4 files changed, 136 insertions(+), 104 deletions(-) diff --git a/providers/database_provider.ts b/providers/database_provider.ts index 76a8113e..cf003399 100644 --- a/providers/database_provider.ts +++ b/providers/database_provider.ts @@ -44,7 +44,7 @@ declare module '@vinejs/vine' { * Ensure the value is unique inside the database by table and column name. * Optionally, you can define a filter to narrow down the query. */ - unique(options: VineDbSearchOptions): this + unique(options: VineDbSearchOptions): this /** * Ensure the value is unique inside the database by self @@ -59,7 +59,7 @@ declare module '@vinejs/vine' { * Ensure the value exists inside the database by table and column name. * Optionally, you can define a filter to narrow down the query. */ - exists(options: VineDbSearchOptions): this + exists(options: VineDbSearchOptions): this /** * Ensure the value exists inside the database by self diff --git a/src/bindings/vinejs.ts b/src/bindings/vinejs.ts index 45c0c6dc..ebdc90fa 100644 --- a/src/bindings/vinejs.ts +++ b/src/bindings/vinejs.ts @@ -25,113 +25,122 @@ export const messages = { * VineJS. */ export function defineValidationRules(db: Database) { - const uniqueRule = vine.createRule( - async (value, callbackOrOptions, field) => { - if (!field.isValid) { - return - } + function uniqueRule() { + return vine.createRule | VineDbSearchOptions>( + async (value, callbackOrOptions, field) => { + if (!field.isValid) { + return + } - /** - * Rely on the callback to execute the query and return value - * a boolean. - * - * True means value is unique - * False means value is not unique - */ - if (typeof callbackOrOptions === 'function') { - const isUnique = await callbackOrOptions(db, value as string, field) - if (!isUnique) { - field.report(messages['database.unique'], 'database.unique', field) + /** + * Rely on the callback to execute the query and return value + * a boolean. + * + * True means value is unique + * False means value is not unique + */ + if (typeof callbackOrOptions === 'function') { + const isUnique = await callbackOrOptions(db, value as ValueType, field) + if (!isUnique) { + field.report(messages['database.unique'], 'database.unique', field) + } + return } - return - } - const { table, column, filter, connection, caseInsensitive } = callbackOrOptions - const query = db.connection(connection).from(table).select(column) + const { table, column, filter, connection, caseInsensitive } = callbackOrOptions + const query = db.connection(connection).from(table).select(column) - /** - * Apply where clause respecting the caseInsensitive flag. - */ - if (caseInsensitive) { - query.whereRaw(`lower(${column}) = ?`, [db.raw(`lower(?)`, [value])]) - } else { - query.where(column, value as string) - } + /** + * Apply where clause respecting the caseInsensitive flag. + */ + if (caseInsensitive) { + query.whereRaw(`lower(${column}) = ?`, [db.raw(`lower(?)`, [value])]) + } else { + query.where(column, value as string | number) + } - /** - * Apply user filter - */ - await filter?.(query, value as string, field) - - /** - * Fetch the first row from the database - */ - const row = await query.first() - if (row) { - field.report(messages['database.unique'], 'database.unique', field) - } - } - ) + /** + * Apply user filter + */ + await filter?.(query, value as ValueType, field) - const existsRule = vine.createRule( - async (value, callbackOrOptions, field) => { - if (!field.isValid) { - return + /** + * Fetch the first row from the database + */ + const row = await query.first() + if (row) { + field.report(messages['database.unique'], 'database.unique', field) + } } + ) + } + + function existsRule() { + return vine.createRule | VineDbSearchOptions>( + async (value, callbackOrOptions, field) => { + if (!field.isValid) { + return + } - /** - * Rely on the callback to execute the query and return value - * a boolean. - * - * True means value exists - * False means value does not exist - */ - if (typeof callbackOrOptions === 'function') { - const exists = await callbackOrOptions(db, value as string, field) - if (!exists) { - field.report(messages['database.exists'], 'database.exists', field) + /** + * Rely on the callback to execute the query and return value + * a boolean. + * + * True means value exists + * False means value does not exist + */ + if (typeof callbackOrOptions === 'function') { + const exists = await callbackOrOptions(db, value as ValueType, field) + if (!exists) { + field.report(messages['database.exists'], 'database.exists', field) + } + return } - return - } - const { table, column, filter, connection, caseInsensitive } = callbackOrOptions - const query = db.connection(connection).from(table).select(column) + const { table, column, filter, connection, caseInsensitive } = callbackOrOptions + const query = db.connection(connection).from(table).select(column) - /** - * Apply where clause respecting the caseInsensitive flag. - */ - if (caseInsensitive) { - query.whereRaw(`lower(${column}) = ?`, [db.raw(`lower(?)`, [value])]) - } else { - query.where(column, value as string) - } + /** + * Apply where clause respecting the caseInsensitive flag. + */ + if (caseInsensitive) { + query.whereRaw(`lower(${column}) = ?`, [db.raw(`lower(?)`, [value])]) + } else { + query.where(column, value as string | number) + } - /** - * Apply user filter - */ - await filter?.(query, value as string, field) - - /** - * Fetch the first row from the database - */ - const row = await query.first() - if (!row) { - field.report(messages['database.exists'], 'database.exists', field) + /** + * Apply user filter + */ + await filter?.(query, value as ValueType, field) + + /** + * Fetch the first row from the database + */ + const row = await query.first() + if (!row) { + field.report(messages['database.exists'], 'database.exists', field) + } } - } - ) + ) + } + + const uniqueRuleForString = uniqueRule() + const uniqueRuleForNumber = uniqueRule() + const existsRuleForString = existsRule() + const existsRuleForNumber = existsRule() VineString.macro('unique', function (this: VineString, callbackOrOptions) { - return this.use(uniqueRule(callbackOrOptions)) + return this.use(uniqueRuleForString(callbackOrOptions)) }) VineString.macro('exists', function (this: VineString, callbackOrOptions) { - return this.use(existsRule(callbackOrOptions)) + return this.use(existsRuleForString(callbackOrOptions)) }) VineNumber.macro('unique', function (this: VineNumber, callbackOrOptions) { - return this.use(uniqueRule(callbackOrOptions)) + return this.use(uniqueRuleForNumber(callbackOrOptions)) }) VineNumber.macro('exists', function (this: VineNumber, callbackOrOptions) { - return this.use(existsRule(callbackOrOptions)) + return this.use(existsRuleForNumber(callbackOrOptions)) }) } diff --git a/src/types/vine.ts b/src/types/vine.ts index 3edf6644..a51b54d7 100644 --- a/src/types/vine.ts +++ b/src/types/vine.ts @@ -14,7 +14,7 @@ import type { DatabaseQueryBuilderContract } from './querybuilder.js' /** * Options for the unique and the exists validations */ -export type VineDbSearchOptions = { +export type VineDbSearchOptions = { /** * Database table for the query */ @@ -49,7 +49,7 @@ export type VineDbSearchOptions = { */ filter?: ( db: DatabaseQueryBuilderContract, - value: string, + value: ValueType, field: FieldContext ) => void | Promise } diff --git a/test/bindings/vinejs.spec.ts b/test/bindings/vinejs.spec.ts index 4d81c0f7..592a35e1 100644 --- a/test/bindings/vinejs.spec.ts +++ b/test/bindings/vinejs.spec.ts @@ -14,6 +14,9 @@ import { defineValidationRules } from '../../src/bindings/vinejs.js' import { getConfig, setup, cleanup, logger, createEmitter } from '../../test-helpers/index.js' let db: Database +const dialectPerformsCaseSensitiveSearch = ['mysql', 'mysql_legacy', 'mssql'].includes( + process.env.DB! +) test.group('VineJS | unique', (group) => { group.setup(async () => { @@ -102,13 +105,23 @@ test.group('VineJS | unique', (group) => { email: 'foo@bar.com', }) } catch (error) { - assert.deepEqual(error.messages, [ - { - field: 'email', - message: 'The email has already been taken', - rule: 'database.unique', - }, - ]) + if (dialectPerformsCaseSensitiveSearch) { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The email has already been taken', + rule: 'database.unique', + }, + ]) + } else { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The email has already been taken', + rule: 'database.unique', + }, + ]) + } } }) @@ -338,13 +351,23 @@ test.group('VineJS | exists', (group) => { email: 'foo@bar.com', }) } catch (error) { - assert.deepEqual(error.messages, [ - { - field: 'username', - message: 'The selected username is invalid', - rule: 'database.exists', - }, - ]) + if (dialectPerformsCaseSensitiveSearch) { + assert.deepEqual(error.messages, [ + { + field: 'email', + message: 'The email has already been taken', + rule: 'database.unique', + }, + ]) + } else { + assert.deepEqual(error.messages, [ + { + field: 'username', + message: 'The selected username is invalid', + rule: 'database.exists', + }, + ]) + } } }) From 13c42e715c795f937aa1f182438dced54ede7176 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Wed, 4 Dec 2024 17:58:50 +0530 Subject: [PATCH 12/16] test: fix failing tests --- test/bindings/vinejs.spec.ts | 54 +++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/test/bindings/vinejs.spec.ts b/test/bindings/vinejs.spec.ts index 592a35e1..348510ac 100644 --- a/test/bindings/vinejs.spec.ts +++ b/test/bindings/vinejs.spec.ts @@ -107,6 +107,11 @@ test.group('VineJS | unique', (group) => { } catch (error) { if (dialectPerformsCaseSensitiveSearch) { assert.deepEqual(error.messages, [ + { + field: 'username', + message: 'The username has already been taken', + rule: 'database.unique', + }, { field: 'email', message: 'The email has already been taken', @@ -337,29 +342,32 @@ test.group('VineJS | exists', (group) => { }) ) - try { - await validator.validate({ - /** - * Username validation will fail because of case - * mismatch - */ - username: 'foo', - /** - * Email validation will pass regardless of the - * case mismatch - */ - email: 'foo@bar.com', - }) - } catch (error) { - if (dialectPerformsCaseSensitiveSearch) { - assert.deepEqual(error.messages, [ - { - field: 'email', - message: 'The email has already been taken', - rule: 'database.unique', - }, - ]) - } else { + if (dialectPerformsCaseSensitiveSearch) { + assert.deepEqual( + await validator.validate({ + username: 'foo', + email: 'foo@bar.com', + }), + { + username: 'foo', + email: 'foo@bar.com', + } + ) + } else { + try { + await validator.validate({ + /** + * Username validation will fail because of case + * mismatch + */ + username: 'foo', + /** + * Email validation will pass regardless of the + * case mismatch + */ + email: 'foo@bar.com', + }) + } catch (error) { assert.deepEqual(error.messages, [ { field: 'username', From ac3b40067e7860cb02e0bf3c7b1bd852836b7f67 Mon Sep 17 00:00:00 2001 From: thetutlage Date: Wed, 4 Dec 2024 12:56:05 +0000 Subject: [PATCH 13/16] chore(release): 21.5.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d1bb1e34..ffb40dca 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@adonisjs/lucid", "description": "SQL ORM built on top of Active Record pattern", - "version": "21.4.0", + "version": "21.5.0", "engines": { "node": ">=18.16.0" }, From 1afdd81d432be0f803d7cd2dde47d261074dd83c Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Mon, 9 Dec 2024 12:23:16 +0530 Subject: [PATCH 14/16] fix: Model.transaction always create a transaction and do not re-use existing one --- package.json | 8 ++++---- src/orm/base_model/index.ts | 25 +++++++++++++++++++------ src/types/model.ts | 5 ++--- test/orm/base_model.spec.ts | 26 ++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index ffb40dca..6ba62c3a 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ }, "devDependencies": { "@adonisjs/assembler": "^7.8.2", - "@adonisjs/core": "^6.15.2", + "@adonisjs/core": "^6.16.0", "@adonisjs/eslint-config": "^2.0.0-beta.7", "@adonisjs/prettier-config": "^1.4.0", "@adonisjs/tsconfig": "^1.4.0", @@ -69,14 +69,14 @@ "@japa/runner": "^3.1.4", "@libsql/sqlite3": "^0.3.1", "@release-it/conventional-changelog": "^9.0.3", - "@swc/core": "^1.9.3", + "@swc/core": "^1.10.0", "@types/chance": "^1.1.6", "@types/luxon": "^3.4.2", "@types/node": "^22.10.1", "@types/pretty-hrtime": "^1.0.3", "@types/qs": "^6.9.17", "@vinejs/vine": "^3.0.0", - "better-sqlite3": "^11.6.0", + "better-sqlite3": "^11.7.0", "c8": "^10.1.2", "chance": "^1.1.12", "copyfiles": "^2.4.1", @@ -88,7 +88,7 @@ "luxon": "^3.5.0", "mysql2": "^3.11.5", "pg": "^8.13.1", - "prettier": "^3.4.1", + "prettier": "^3.4.2", "reflect-metadata": "^0.2.2", "release-it": "^17.10.0", "sqlite3": "^5.1.7", diff --git a/src/orm/base_model/index.ts b/src/orm/base_model/index.ts index beb472bc..f1a1cdbc 100644 --- a/src/orm/base_model/index.ts +++ b/src/orm/base_model/index.ts @@ -248,15 +248,28 @@ class BaseModelImpl implements LucidRow { /** * Returns the model query instance for the given model */ - static async transaction( + static transaction( + callback: (trx: TransactionClientContract) => Promise, options?: ModelAdapterOptions & { isolationLevel?: IsolationLevels } - ): Promise { - const client = this.$adapter.modelConstructorClient(this, options) - if (client.isTransaction) { - return client as TransactionClientContract + ): Promise + static transaction( + options?: ModelAdapterOptions & { + isolationLevel?: IsolationLevels + } + ): Promise + static async transaction( + callbackOrOptions?: + | ((trx: TransactionClientContract) => Promise) + | (ModelAdapterOptions & { isolationLevel?: IsolationLevels }), + options?: ModelAdapterOptions & { isolationLevel?: IsolationLevels } + ): Promise { + if (typeof callbackOrOptions === 'function') { + const client = this.$adapter.modelConstructorClient(this, options) + return client.transaction(callbackOrOptions, options) } - return client.transaction() + const client = this.$adapter.modelConstructorClient(this, callbackOrOptions) + return client.transaction(callbackOrOptions) } /** diff --git a/src/types/model.ts b/src/types/model.ts index e178a945..a191d37f 100644 --- a/src/types/model.ts +++ b/src/types/model.ts @@ -14,6 +14,7 @@ import { IsolationLevels, QueryClientContract, TransactionClientContract, + TransactionFn, } from './database.js' import { @@ -1154,9 +1155,7 @@ export interface LucidModel { * Returns transaction client from the model. It is same as * calling "db.transaction" */ - transaction( - options?: ModelAdapterOptions & { isolationLevel?: IsolationLevels } - ): Promise + transaction: TransactionFn /** * Truncate model table diff --git a/test/orm/base_model.spec.ts b/test/orm/base_model.spec.ts index 630a10a2..9668da8c 100644 --- a/test/orm/base_model.spec.ts +++ b/test/orm/base_model.spec.ts @@ -8346,4 +8346,30 @@ test.group('Base Model | transaction', (group) => { assert.isNull(user) }) + + test('auto manage transaction when callback is provided', async ({ fs, assert }) => { + const app = new AppFactory().create(fs.baseUrl, () => {}) + await app.init() + const db = getDb() + const adapter = ormAdapter(db) + + const BaseModel = getBaseModel(adapter) + + class User extends BaseModel { + @column({ isPrimary: true }) + declare id: number + + @column() + declare username: string + + @column() + declare email: string + } + + const [userId] = await User.transaction(async (trx) => { + return trx.insertQuery().table('users').insert({ username: 'virk' }).returning('id') + }) + const user = await User.find(userId) + assert.equal(user!.username, 'virk') + }) }) From 102b9cb5b7e025baaf76b10321e61affc56e5feb Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Mon, 9 Dec 2024 12:28:00 +0530 Subject: [PATCH 15/16] test: fix breaking tests --- src/types/model.ts | 3 +-- test/orm/base_model.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/types/model.ts b/src/types/model.ts index a191d37f..0b41283f 100644 --- a/src/types/model.ts +++ b/src/types/model.ts @@ -10,11 +10,10 @@ import { DateTime } from 'luxon' import type Hooks from '@poppinss/hooks' import { + TransactionFn, DialectContract, - IsolationLevels, QueryClientContract, TransactionClientContract, - TransactionFn, } from './database.js' import { diff --git a/test/orm/base_model.spec.ts b/test/orm/base_model.spec.ts index 9668da8c..3216fc53 100644 --- a/test/orm/base_model.spec.ts +++ b/test/orm/base_model.spec.ts @@ -8366,10 +8366,10 @@ test.group('Base Model | transaction', (group) => { declare email: string } - const [userId] = await User.transaction(async (trx) => { + await User.transaction(async (trx) => { return trx.insertQuery().table('users').insert({ username: 'virk' }).returning('id') }) - const user = await User.find(userId) + const user = await User.find(1) assert.equal(user!.username, 'virk') }) }) From ca67d8d4b5c8cd036e8396b47c1eaffb4c71ec04 Mon Sep 17 00:00:00 2001 From: thetutlage Date: Mon, 9 Dec 2024 07:09:25 +0000 Subject: [PATCH 16/16] chore(release): 21.5.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6ba62c3a..dbc5373e 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@adonisjs/lucid", "description": "SQL ORM built on top of Active Record pattern", - "version": "21.5.0", + "version": "21.5.1", "engines": { "node": ">=18.16.0" },