Skip to content

Commit

Permalink
fix: find by Date object in sqlite driver (#7538)
Browse files Browse the repository at this point in the history
* fix: find by Date object in sqlite driver

In sqlite, Date objects are persisted as UtcDatetimeString.
But a Date object parameter was escaped with .toISOString(), making such queries impossible.
This commit aligns both transforms.
This bug does *not* apply to better-sql where you can only bind numbers, strings, bigints, buffers,
and null.
This is breaking for when the user inserted their dates manually as ISO and relied on this old
maltransformation, after this their find()s by Date won't work anymore.

BREAKING CHANGE: Change Date serialization in selects
Closes: #2286

* add failing test

* fix: find by Date object in sqlite driver (with query builder)

Also consider query builder parameter escaping

* test: add test for 3426

Co-authored-by: James Ward <james@notjam.es>
  • Loading branch information
phil294 and imnotjames committed Feb 16, 2022
1 parent c3ea632 commit 942fee3
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/driver/sqlite-abstract/AbstractSqliteDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,10 @@ export abstract class AbstractSqliteDriver implements Driver {
return nativeParameters[key] === true ? 1 : 0;
}

if (nativeParameters[key] instanceof Date) {
return DateUtils.mixedDateToUtcDatetimeString(nativeParameters[key]);
}

return nativeParameters[key];
});

Expand All @@ -397,6 +401,11 @@ export abstract class AbstractSqliteDriver implements Driver {

}

if (value instanceof Date) {
escapedParameters.push(DateUtils.mixedDateToUtcDatetimeString(value));
return this.createParameter(key, escapedParameters.length - 1);
}

escapedParameters.push(value);
return this.createParameter(key, escapedParameters.length - 1);
}); // todo: make replace only in value statements, otherwise problems
Expand Down
12 changes: 12 additions & 0 deletions test/github-issues/2286/entity/Example.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {Entity} from "../../../../src/decorator/entity/Entity";
import {PrimaryColumn} from "../../../../src/decorator/columns/PrimaryColumn";
import {Column} from "../../../../src/decorator/columns/Column";

@Entity()
export class Example {
@PrimaryColumn()
id: Date;

@Column()
text: string
}
12 changes: 12 additions & 0 deletions test/github-issues/2286/entity/Post.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {Entity} from "../../../../src/decorator/entity/Entity";
import {PrimaryColumn} from "../../../../src/decorator/columns/PrimaryColumn";
import {Column} from "../../../../src/decorator/columns/Column";

@Entity()
export class Post {
@PrimaryColumn("int")
id: number;

@Column()
dateTimeColumn: Date;
}
80 changes: 80 additions & 0 deletions test/github-issues/2286/issue-2286.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import "reflect-metadata";
import { createTestingConnections, closeTestingConnections, reloadTestingDatabases } from "../../utils/test-utils";
import { Connection } from "../../../src/connection/Connection";
import { expect } from "chai";
import { Post } from "./entity/Post";
import { Example } from "./entity/Example";
import { Between } from "../../../src";

describe("github issues > #2286 find operators like MoreThan and LessThan doesn't work properly for date fields", () => {

let connections: Connection[];
before(async () => connections = await createTestingConnections({
entities: [ Post, Example ],
schemaCreate: true,
dropSchema: true,
/* Test not eligible for better-sql where binding Dates is impossible */
enabledDrivers: ["sqlite"]
}));
beforeEach(() => reloadTestingDatabases(connections));

after(() => closeTestingConnections(connections));

it("should find a record by its datetime value with find options", () => Promise.all(connections.map(async connection => {
const start = new Date("2000-01-01");
const end = new Date("2001-01-01");
const middle = new Date("2000-06-30");
const post = new Post();
post.dateTimeColumn = middle;

await connection.manager.save(post);

const postByDateEquals = await connection.manager.findOne(Post, {
dateTimeColumn: middle
});
expect(postByDateEquals).to.not.be.undefined;

const postByDateBetween = await connection.manager.findOne(Post, {
dateTimeColumn: Between(start, end)
});
expect(postByDateBetween).to.not.be.undefined;
})));

it("should find a record by its datetime value with query builder", () => Promise.all(connections.map(async connection => {
const now = new Date();
const post = new Post();
post.dateTimeColumn = now;

await connection.manager.save(post);

const postByDateEquals = await connection.manager.getRepository(Post)
.createQueryBuilder("post")
.where("post.dateTimeColumn = :now", { now })
.getOne();
expect(postByDateEquals).to.not.be.undefined;
})));

it("should save, update, and load with a date PK", () => Promise.all(connections.map(async connection => {
const start = new Date("2000-01-01");
const middle = new Date("2000-06-30");
const end = new Date("2001-01-01");

await connection.manager.save(Example, { id: start, text: "start" });
await connection.manager.save(Example, { id: middle, text: "middle" });
await connection.manager.save(Example, { id: end, text: "end" });

const repo = connection.manager.getRepository(Example);

let example = await repo.findOneOrFail({ id: middle });

expect(example.text).to.be.equal("middle");

example.text = "in between";

await repo.save(example);

example = await repo.findOneOrFail({ id: middle });

expect(example.text).to.be.equal("in between");
})));
});

0 comments on commit 942fee3

Please sign in to comment.