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.

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

merged 1 commit into from
Feb 15, 2022

Conversation

DDEle
Copy link
Contributor

@DDEle DDEle commented Feb 12, 2022

Note that the new test cases added introduce a failure for other database plugins like MongoDB and LevelDB (time comparisons)

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 时间的/哪些是需要完全相等的),我觉得会比较乱。

@shigma shigma merged commit 6129625 into master Feb 15, 2022
@shigma shigma deleted the mysql-fix branch February 15, 2022 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants