Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mysql): time field should give Date #512

Merged
merged 1 commit into from
Feb 15, 2022
Merged

fix(mysql): time field should give Date #512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions plugins/database/mysql/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ declare module 'koishi' {

const logger = new Logger('mysql')

const DEFAULT_DATE = new Date('1970-01-01')

export type TableType = keyof Tables

function getIntegerType(length = 11) {
Expand Down Expand Up @@ -128,6 +130,15 @@ class MysqlDatabase extends Database {
} else if (meta?.type === 'list') {
const source = field.string()
return source ? source.split(',') : []
} else if (meta?.type === 'time') {
const source = field.string()
if (!source) return meta.initial
const time = new Date(DEFAULT_DATE)
const [h, m, s] = source.split(':')
time.setHours(parseInt(h))
time.setMinutes(parseInt(m))
time.setSeconds(parseInt(s))
return time
}

if (field.type === 'BIT') {
Expand Down
36 changes: 33 additions & 3 deletions plugins/database/tests/src/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ interface Foo {
text?: string
value?: number
list?: number[]
timestamp?: Date
date?: Date
time?: Date
regex?: string
}

Expand All @@ -22,7 +24,9 @@ function QueryOperators(app: App) {
text: 'string',
value: 'integer',
list: 'list',
date: 'timestamp',
timestamp: 'timestamp',
date: 'date',
time: 'time',
regex: 'string',
}, {
autoInc: true,
Expand All @@ -33,7 +37,12 @@ namespace QueryOperators {
export const comparison = function Comparison(app: App) {
before(async () => {
await app.database.remove('temp1', {})
await app.database.create('temp1', { text: 'awesome foo', date: new Date('2000-01-01') })
await app.database.create('temp1', {
text: 'awesome foo',
timestamp: new Date('2000-01-01'),
date: new Date('2020-01-01'),
time: new Date('2020-01-01 12:00:00'),
})
await app.database.create('temp1', { text: 'awesome bar' })
await app.database.create('temp1', { text: 'awesome baz' })
})
Expand Down Expand Up @@ -64,6 +73,16 @@ namespace QueryOperators {
})).eventually.to.have.length(2).with.nested.property('0.text').equal('awesome foo')
})

it('timestamp comparisons', async () => {
await expect(app.database.get('temp1', {
timestamp: { $gt: new Date('1999-01-01') },
})).eventually.to.have.length(1).with.nested.property('0.text').equal('awesome foo')

await expect(app.database.get('temp1', {
timestamp: { $lte: new Date('1999-01-01') },
})).eventually.to.have.length(0)
})

it('date comparisons', async () => {
await expect(app.database.get('temp1', {
date: { $gt: new Date('1999-01-01') },
Expand All @@ -74,13 +93,24 @@ namespace QueryOperators {
})).eventually.to.have.length(0)
})

it('time comparisons', async () => {
await expect(app.database.get('temp1', {
// date should not matter
time: { $gt: new Date('2022-01-01 11:00:00') },
})).eventually.to.have.length(1).with.nested.property('0.text').equal('awesome foo')

await expect(app.database.get('temp1', {
time: { $lte: new Date('2022-01-01 11:00:00') },
})).eventually.to.have.length(0)
})

it('shorthand syntax', async () => {
await expect(app.database.get('temp1', {
id: 2,
})).eventually.to.have.length(1).with.nested.property('0.text').equal('awesome bar')

await expect(app.database.get('temp1', {
date: new Date('2000-01-01'),
timestamp: new Date('2000-01-01'),
})).eventually.to.have.length(1).with.nested.property('0.text').equal('awesome foo')
})
}
Expand Down
70 changes: 52 additions & 18 deletions plugins/database/tests/src/update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { App, omit, Tables } from 'koishi'
import { App, Model, omit, Tables } from 'koishi'
import { expect } from 'chai'
export const DEFAULT_DATE = new Date('1970-01-01')

interface Bar {
id?: number
text?: string
num?: number
list?: string[]
timestamp?: Date
date?: Date
time?: Date
meta?: {
a?: string
b?: number
Expand All @@ -32,7 +35,9 @@ function OrmOperations(app: App) {
text: 'string',
num: 'integer',
list: 'list',
date: 'timestamp',
timestamp: 'timestamp',
date: 'date',
time: 'time',
meta: { type: 'json' },
}, {
autoInc: true,
Expand All @@ -47,6 +52,30 @@ function OrmOperations(app: App) {
})
}

function normalizeDate<T extends Tables[keyof Tables]>(row: T, model: Model.Config<T>): T {
const normalized = { ...row }
for (const k in row) {
if (!row[k]) continue
if (model.fields[k]?.type === 'time') {
const raw: Date = row[k] as any,
h = raw.getHours(),
m = raw.getMinutes(),
s = raw.getSeconds()
const date = new Date(DEFAULT_DATE)
date.setHours(h, m, s)
normalized[k] = date as any
}
}
return normalized
}

function expectShapeNormalized<T extends Tables[keyof Tables]>(t1: T[], t2: T[], model: Model.Config<T>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没看懂……

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

首先我假设 to have shape 等价于 deep equal（确实应该问一句的

然后这个 time 时间字段 koishi 定义的对应的 js 类型是 Date，那么在比较的时候，如果一个 time 类型的实际值和期望值日期不同但时间相同，理应是符合期望的但是使用 to have shape 就会导致判断为不符合。

所以这里的解决方法是都包一层，在比较之前，把所有的 time 类型字段的实际值和期望值都修改日期为某个特定日期（也就是这里 normalize 的意思）。这样只要修改前 time 字段的时间完全相等并且其他类型的值完全相等，normalize 后的两个值也就会完全相等了。

其实类似的处理应该也对 date 类型字段做一遍，因为它只在乎日期而不在乎时间，不过这里偷懒没写（

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是不是可以考虑去修改 .shape() 的行为呢

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我感觉不太行。要修改shape的话 还需要把 model.config（或者根据它重新组装相关的信息，那样的话都是要包一层还不如现在这样呢） 传进去（不然不知道哪些部分是 care 日期的/哪些是 care 时间的/哪些是需要完全相等的），我觉得会比较乱。

expect(t1.length).to.equal(t2.length)
t1.forEach((_, i) => {
expect(normalizeDate(t1[i], model)).to.have.shape(normalizeDate(t2[i], model))
})
}

namespace OrmOperations {
const merge = <T>(a: T, b: Partial<T>): T => ({ ...a, ...b })

Expand All @@ -57,8 +86,10 @@ namespace OrmOperations {
{ id: 2, text: 'pku' },
{ id: 3, num: 1989 },
{ id: 4, list: ['1', '1', '4'] },
{ id: 5, date: magicBorn },
{ id: 5, timestamp: magicBorn },
{ id: 6, meta: { a: 'foo', b: 233 } },
{ id: 7, date: magicBorn },
{ id: 8, time: new Date('2020-01-01 12:00:00') },
]

const bazTable: Baz[] = [
Expand All @@ -79,16 +110,17 @@ namespace OrmOperations {

export const create = function Create(app: App) {
it('auto increment primary key', async () => {
const model = app.model.config['temp2']
const table = barTable.map(bar => merge(app.model.create('temp2'), bar))
for (const index in barTable) {
const bar = await app.database.create('temp2', omit(barTable[index], ['id']))
barTable[index].id = bar.id
expect(bar).to.have.shape(table[index])
}
for (const obj of table) {
await expect(app.database.get('temp2', { id: obj.id })).eventually.shape([obj])
expectShapeNormalized(await app.database.get('temp2', { id: obj.id }), [obj], model)
}
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, model)
})

it('specify primary key', async () => {
Expand Down Expand Up @@ -127,9 +159,10 @@ namespace OrmOperations {
}

export const set = function Set(app: App) {
const modelTemp2 = app.model.config['temp2']
it('basic support', async () => {
const table = await setup(app, 'temp2', barTable)
const data = table.find(bar => bar.date)
const data = table.find(bar => bar.timestamp)
data.text = 'thu'
const magicIds = table.slice(0, 2).map((data) => {
data.text = 'thu'
Expand All @@ -138,10 +171,10 @@ namespace OrmOperations {
await expect(app.database.set('temp2', {
$or: [
{ id: magicIds },
{ date: magicBorn },
{ timestamp: magicBorn },
],
}, { text: 'thu' })).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})

it('using expressions', async () => {
Expand All @@ -151,7 +184,7 @@ namespace OrmOperations {
await expect(app.database.set('temp2', [table[1].id, table[2].id, 9], {
num: { $multiply: [2, { $: 'id' }] },
})).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})

it('using object literals', async () => {
Expand All @@ -162,7 +195,7 @@ namespace OrmOperations {
await expect(app.database.set('temp2', [data1.id, data2.id, 9], {
meta: { b: 114514 },
})).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})

it('nested property', async () => {
Expand All @@ -174,11 +207,12 @@ namespace OrmOperations {
await expect(app.database.set('temp2', [data1.id, data2.id, 9], {
'meta.a': { $concat: [{ $ifNull: [{ $: 'meta.a' }, ''] }, 'bar'] },
})).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})
}

export const upsert = function Upsert(app: App) {
const modelTemp2 = app.model.config['temp2']
it('update existing records', async () => {
const table = await setup(app, 'temp2', barTable)
const data = [
Expand All @@ -190,18 +224,18 @@ namespace OrmOperations {
table[index] = merge(table[index], update)
})
await expect(app.database.upsert('temp2', data)).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})

it('insert new records', async () => {
const table = await setup(app, 'temp2', barTable)
const data = [
{ id: table[5].id + 1, text: 'wmlake' },
{ id: table[5].id + 2, text: 'bytower' },
{ id: table[table.length - 1].id + 1, text: 'wmlake' },
{ id: table[table.length - 1].id + 2, text: 'bytower' },
]
table.push(...data.map(bar => merge(app.model.create('temp2'), bar)))
await expect(app.database.upsert('temp2', data)).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})

it('using expressions', async () => {
Expand All @@ -218,7 +252,7 @@ namespace OrmOperations {
{ id: 3, num: { $add: [3, { $: 'num' }] } },
{ id: 9, num: 999 },
])).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})

it('using object literals', async () => {
Expand All @@ -235,7 +269,7 @@ namespace OrmOperations {
{ id: 6, meta: { b: 514 } },
{ id: 9, meta: { b: 114514 } },
])).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})

it('nested property', async () => {
Expand All @@ -252,7 +286,7 @@ namespace OrmOperations {
{ id: 6, 'meta.b': 666 },
{ id: 9, 'meta.b': 999 },
])).eventually.fulfilled
await expect(app.database.get('temp2', {})).eventually.shape(table)
expectShapeNormalized(await app.database.get('temp2', {}), table, modelTemp2)
})
}

Expand Down