sequelize
Creation of model instance with wrong types possible
Versions
Tested on both
- sequelize: 5.22
- sequelize-typescript: 1.1.0
- typescript: 4.2.3 and
- sequelize: ^6.5.0
- sequelize-typescript: 2.1.0
- typescript: 4.2.3
Issue type
- [ x? ] bug report
- [ x ] feature request
Actual behavior
I have a simple repository (updated the 'simple' example from this repo). Added some stricter typing to the columns. I typed User.name as DataType.STRING(128). After syncing the database I add a new instance to user by calling User.create({name: true}). This passes without any problems and creates a row in the User table with User.name = true.
Expected behavior
I would expect the data column to validate that the inserted data is actually of that type. I know SQLite does not care, but would expect either sequelize or sequelize-typescript to check this.
Related code
See this repo I created with what I described. User instance is created in server.ts.
Hi, @rdvhoorn There are additional attributes since v6:
// types expected for entry creation
interface UserCreateAttributes {
name: string;
}
// and update
interface UserAttributes extends UserCreateAttributes {
id: number;
}
@Table
export default class User extends Model<UserAttributes,UserCreateAttributes>
// optionally add `implements` to prevent types disagrements,
// however this might be problematic if you're using getters/setters w/ different types
implements UserAttributes {
@PrimaryKey
@Column
id: number;
@Column
name: string;
}
// and now expect error here
User.create({name: true});
// or here
new User({name: true});
And this does throw errors correctly? It looks really good! But I don't really understand the difference? In the sense of, why would this solution work? Do the interfaces cause type strictness for some reason? I would not see why @Column(DataType.STRING) would result in less type strictness than an interface?
interface is for typescript typehinting (ide/compilation), @Column(DataType.STRING) is for database engine (runtime). interface itself is in general not present in the final code. However sequelize-typescript can infer DataType from property type (class properties) according to code bellow, but that's about it when comes to the type dependency between these two.
https://github.com/RobinBuschmann/sequelize-typescript/blob/f3ee651d19285751b50ebed7122bb8e0bc27c755/src/sequelize/data-type/data-type-service.ts#L16-L34
Edit:
To be clear, you should probably still add DataType.STRING(128) if you want limit field length and you're using something other than sqlite. Or to be explicit about what type database should be using.
Note that numeric arguments in parentheses that following the type name (ex: "VARCHAR(255)") are ignored by SQLite - SQLite does not impose any length restrictions (other than the large global SQLITE_MAX_LENGTH limit) on the length of strings, BLOBs or numeric values. https://www.sqlite.org/datatype3.html#affinity_name_examples
Currently, i am using SQLite in my projects. Still had to check whether the numbers actually did something :P
However, if indeed the interfaces do nothing at runtime, but only at compile time, then it wouldn't fix my issue that I can create an instance with wrong types at runtime? Say for example I have a express route and get an updated instances. If I go to update that without checking types, then it will simply be fine right? I would love for it to then return an error that typings do not match, because that would make most sense to me.
Currently, as I have not found a better solution, I'm using validators to check typings at runtime. Drawback is that they only really work for simple types and doing that with attributes which can be null is annoying
If data is received from the user (or from anywhere outside app), you should check data before doing anything. I like to use fastify instead of express, which allows checking almost everything via json-schema and has better typescript support. Also you can make sure that output has a correct format. Bellow very basic examples to show how to approach this.
However, if indeed the interfaces do nothing at runtime, [...] I can create an instance with wrong types at runtime?
If you're keeping types what they actually are and not fixing problems with any, then code won't even compile if you try to assign wrong type. You may even want to enable strict rule in tsconfig and add eslint with typescript-eslint for more pedantic experience.
You can also make custom sequelize validators, try to modify validation/is.ts for starters.
And mentioned fastify example:
api-schemas.ts
// optional for code-completion
import type { JSONSchema7 as Schema } from "json-schema";
export interface paramId {
id: number;
}
export const paramIdSchema = {
type: "object",
properties: {
id: { type: "integer" },
},
required: ["id"],
} as Schema;
export interface getManyRequest {
offset?: number;
limit?: number;
sort?: string;
sortDir?: "asc" | "desc";
filter?: string;
}
export const getManyRequestSchema = {
type: "object",
properties: {
offset: { type: "number", minimum: 0, multipleOf: 1, default: 0 },
limit: {
type: "number",
minimum: 0,
maximum: 500,
multipleOf: 1,
default: 25,
},
sort: { type: "string" },
sortDir: { type: "string", enum: ["asc", "desc"] },
filter: { type: "string" },
},
} as Schema;
export interface getManyResponse {
offset: number;
length: number;
count: number;
data: unknown[];
}
export const getManyResponseSchema = {
type: "object",
properties: {
offset: { type: "number" },
length: { type: "number" },
count: { type: "number" },
data: { type: "array" },
},
required: ["offset", "length", "count", "data"],
} as Schema;
api.ts
import {
getManyRequest,
getManyRequestSchema,
getManyResponse,
getManyResponseSchema,
paramId,
paramIdSchema,
} from "./api-schemas";
const fastify = Fastify({
ignoreTrailingSlash: true,
//...
ajv: {
customOptions: {
// check options at: https://ajv.js.org/options.html#usage
$data: true,
useDefaults: true,
removeAdditional: true,
// make sure that request have correct types or fail if not possible
coerceTypes: "array",
},
},
});
//...
// fastify documentation: https://www.fastify.io/docs/latest/Validation-and-Serialization/
fastify.get<{ Querystring: getManyRequest }>(
"/posts",
{
schema: {
querystring: getManyRequestSchema,
response: {
200: getManyResponseSchema,
},
},
},
async (req, res) => {
// following consts have validated and known types now
const {
filter = "",
limit = 10,
offset = 0,
sort = "createdAt",
sortDir = "desc",
} = req.query;
// handle request
}
);
fastify.get<{ Params: paramId }>(
"/posts/:id",
{
schema: {
params: paramIdSchema,
},
},
async (req, res) => {
// id is now number
const id = req.params.id;
// handle request
}
);
Hi,
thanks for he insight KapitanOczywisty using the interfaces. But like rdvhoorn I also do not for sure understand why the field definition in the class itself are not enough to constraint the class.create() function. I can imagine to make such a thing to work would require some reflection based compilation ...
Seems that also the automatic fields createdAt, updatedAt, need to be placed into the UserAttributes interface, if they are addressed in the code.
One problem is that the User class cannot be extended anymore:
export default class AconcreteUser extends User<interfaceA, interfaceB>
does not work anymore.
I see there is no option to use the shortcut way introduced in the sequelize typescript section:
import { Table, Column, PrimaryKey, Model } from "sequelize-typescript";
import { InferAttributes, InferCreationAttributes } from "sequelize";
@Table
export default class User extends Model<InferAttributes<User>, InferCreationAttributes<User>>
{
@PrimaryKey
@Column
declare id: number;
@Column
declare name: string;
}
User.create({name: "should be ok"});
// but expect error here
new User({name: `true});
This shows complicated type mismatch errors. Nevertheless this "InferAttribute" approach is too complex for me anyway.
