Belongs to issue

[stelescuraul]

Say we have the following models :

Users:

  @HasOne(() => Detail, 'userId')
  public detail: Detail;

Details:

  @BelongsTo(() => User, 'userId')
  public user: User;

The constrain in the database is:

CONSTRAINT "Detail_pkey" PRIMARY KEY (id),
  CONSTRAINT "Detail_userId_fkey" FOREIGN KEY ("userId")
      REFERENCES public."User" (id) MATCH SIMPLE
      ON UPDATE CASCADE ON DELETE SET NULL

That means that the same user can have multiple 'Details'

Easiest way to fix, is to support the constrain 'Unique' (maybe as an annotation) or have belongsToOne type of relation ?

@RobinBuschmann If i missed something, can you point me in the right direction on how to force the relation of HasOne and BelongsTo to only accept a one-to-one ?

[stelescuraul]

I have fixed this issue by splitting it like this and using the unique property on column. Took me a while to find:

  @AllowNull(false)
  @ForeignKey(() => User)
  @Column({
    unique: true,
  })
  public userId: number;

  @BelongsTo(() => User)
  public user: User;

But now I am facing another issue. When trying to drop and sync the models, or simply using them, because of minification, I have to declare modelName and tableName in @Table decorator, BUT ts is throwing an error about the function being void. If i don't, the drop and sync functions will try to run DROP TABLE (e) (e = minified table name). Using tableName and modelName I can avoid this issue.

severity: 'Error'
message: 'Unable to resolve signature of class decorator when called as an expression.
  Cannot invoke an expression whose type lacks a call signature. Type 'void' has no compatible call signatures.'

It comes from Table.d.ts:

export declare function Table(target: any): void;

changing it to return Function fixed the issue but i am not sure about other implications this might have.

[RobinBuschmann]

Hey, thanks for contributing.

As you already mentioned, this is the way to go:

 @AllowNull(false)
 @ForeignKey(() => User)
 @Column({
    unique: true,
 })
 public userId: number;

 @BelongsTo(() => User)
 public user: User;

Regarding your minification issue: It is not common practice to minify server-side code. Since there is no real benefit from it (see here and here). Sure, there CAN be a performance improvement - see here -, but when considering the 600 character limit it is not really necessary. Or have I overseen something? :)

Anyway, if you want to make it work with minification, you have to add modelName and tableName as you already did. To avoid the TypeScript error, you need to tell the TypeScript compiler, that your passed options object is of type DefineOptions<any>, since modelName is not part of the interface:

@Table({
  tableName: 'Detail',
  modelName: 'Detail'
} as DefineOptions<any>)
export class Detail extends Model<Detail> {
 // ...
}

For the next version I will add modelName to the interface, so that you can omit the as DefineOptions<any> part.

If you're happy with this answer, feel free to close this issue :)

[stelescuraul]

@RobinBuschmann Thanks for answering :)

Regarding the unique and AllowNull, maybe we could add that example to the documentation if it makes sense ? Someone else might not pay attention (like I did) and might spend some time finding it out :) Maybe add the example at @Table decorator right next to the timestamp example ?

Regarding the minification: To give you an overview of why I decided to minify the code, I do it through webpack with Uglify plugin. The reason behind it is that the code will be deployed to raspberry pies that will be handed to clients and although you can still read the code doing reverse engineering, it is a bit discouraging. But the main reason is the performance gain (like you pointed out) since the raspberry pi will be controlling multiple micro services.

Anyway, with or without minification some people might find another use for modelName property as well.

I have tried yesterday doing this:

const definition: DefineOptions<any> = {
  modelName: 'Detail',
  tableName: 'Detail',
};

but Typescript is complaining that the property modelName does not exist in type DefineOptions<any> but with your example it does work just fine. That is my error and I still don`t understand what is the exact difference between the 2 examples but that means I have to read more about Typescript :)

[RobinBuschmann]

@stelescuraul Since this is a sequelize issue I will not add this to the docs. I would like to keep only sequelize-typescript related stuff in the docs. As you can see here someone else faced the same problem with pure sequelize. Probably we can do some research on why sequelize doesn't implicitly add a unique to the foreign key. Probably it is a bug in sequelize and should be reported.

In respect of minification: I do understand. Well that's fair enough.

I'm just wondering that your assignment doesn't through an error. When I do this

const definition: DefineOptions<any> = {
  modelName: 'Detail',
  tableName: 'Detail',
};

I'm already getting an error:
TS2322:Type '{ tableName: string; modelName: string; }' is not assignable to type 'DefineOptions<any>'. Object literal may only specify known properties, and 'modelName' does not exist in type 'DefineOptions<any>'.

[stelescuraul]

@RobinBuschmann Well, for a one-to-one relationship it would make sense to add a unique constraint. Here, you would use belongsTo but lets take the following example:
One person has many houses, and x houses belong to the same user. In this scenario you might have a table for person and one for house. When you make the reference it is one-to-many but each house belongs to one person. So you would still use belongsTo but in this case the foreign key (userId lets call it) would not be unique. So I believe that the approach with unique property is alright and it gives you enough freedom to design and work with the models properly.

With this in mind, what i suggested regarding the docs, is to specify in sequelize-typescript how that relation should look like when it is defined (as an example). I have been looking for a decorator of type @Unique which does not exist. I am not sure it is completely relevant for sequelize-typescript but it might make it easier for the users to see an example.

Regarding the DefineOptions, that is exactly what I am getting too. I don't understand what is the difference between const definition: DefineOptions<any> and {...} as DefineOptions<any> and why one works and the other doesn't (I am relatively new to Typescript). Anyway, your suggestion worked so i`m happy with it. Thanks for the help :)

edit: found these answers regarding the as and interface:
http://acdcjunior.github.io/typescript-cast-object-to-other-type-or-instanceof.html
http://stackoverflow.com/questions/23281962/how-do-i-cast-to-an-interface-an-object-may-implement

[RobinBuschmann]

@stelescuraul You mean, that the reason why belongsTo does not implicitly sets unique to the foreign key is, that belongsTo is also used for one-to-many relations and not for one-to-one only? Am I getting you right?
A @Unique decorator is a good idea. I will implement it for the next version.

Regarding TypeScripts structural typing:

• https://www.typescriptlang.org/docs/handbook/type-compatibility.html (docs)
• Allow stricter structural typing microsoft/TypeScript#391 (older issue)

[stelescuraul]

@RobinBuschmann Yes, that is what I mean. But from their documentation they have a belongsTo which says:

One-To-Many associations are connecting one source with multiple targets. The targets however are again connected to exactly one specific source.

and then a belongsToMany which says:

Belongs-To-Many associations are used to connect sources with multiple targets. Furthermore the targets can also have connections to multiple sources.

I have made a small test using the sequelize library with two models (User and Detail) with the following relation:

User.hasOne(Detail);
Detail.belongsTo(User);

I have taken two approaches to add details to the User instance:
If I do:

userInstace.createDetail({...});
userInstace.createDetail({...});

The resulting query is this:

Executing (default): DROP TABLE IF EXISTS "details";
Executing (default): DROP TABLE IF EXISTS "users";
Executing (default): INSERT INTO "users" ("id","name","createdAt","updatedAt") VALUES (DEFAULT,'Raul','2017-05-07 16:37:03.628 +00:00','2017-05-07 16:37:03.628 +00:00') RETURNING *;
Executing (default): INSERT INTO "details" ("id","summary","createdAt","updatedAt","userId") VALUES (DEFAULT,'Test','2017-05-07 16:37:03.638 +00:00','2017-05-07 16:37:03.638 +00:00',1) RETURNING *;
Executing (default): INSERT INTO "details" ("id","summary","createdAt","updatedAt","userId") VALUES (DEFAULT,'Should not be added','2017-05-07 16:37:03.644 +00:00','2017-05-07 16:37:03.644 +00:00',1) RETURNING *;

As you can see, two details instances are referencing the same user instance when they shouldn't. But if I first create the two details instances and try to add one at the time to a user, this happens:

usrInstance.setDetail(detailInstance1);
usrInstance.setDetail(detailInstance2);

Executing (default): DROP TABLE IF EXISTS "details";
Executing (default): DROP TABLE IF EXISTS "users";
Executing (default): INSERT INTO "users" ("id","name","createdAt","updatedAt") VALUES (DEFAULT,'Raul','2017-05-07 16:41:18.515 +00:00','2017-05-07 16:41:18.515 +00:00') RETURNING *;
Executing (default): INSERT INTO "details" ("id","summary","createdAt","updatedAt") VALUES (DEFAULT,'1','2017-05-07 16:41:18.540 +00:00','2017-05-07 16:41:18.540 +00:00') RETURNING *;
Executing (default): INSERT INTO "details" ("id","summary","createdAt","updatedAt") VALUES (DEFAULT,'2','2017-05-07 16:41:18.546 +00:00','2017-05-07 16:41:18.546 +00:00') RETURNING *;
Executing (default): SELECT "id", "summary", "createdAt", "updatedAt", "userId" FROM "details" AS "detail" WHERE "detail"."userId" = 1 LIMIT 1;
Executing (default): UPDATE "details" SET "userId"=1,"updatedAt"='2017-05-07 16:41:18.558 +00:00' WHERE "id" = 1
Executing (default): SELECT "id", "summary", "createdAt", "updatedAt", "userId" FROM "details" AS "detail" WHERE "detail"."userId" = 1 LIMIT 1;
Executing (default): UPDATE "details" SET "userId"=NULL,"updatedAt"='2017-05-07 16:41:18.565 +00:00' WHERE "id" = 1
Executing (default): UPDATE "details" SET "userId"=1,"updatedAt"='2017-05-07 16:41:18.576 +00:00' WHERE "id" = 2

In this case, sequelize will update the first detail instance property userId to NULL and set the second instance property userId to 1

So sequelize handles the creation and association differently (userInstance.createDetail vs userInstance.setDetail). I will be submitting a question to sequelize to check if that is the intentional behavior or if it is an actual bug.

However, it can be avoided by setting the unique to true.

Thanks for the Typescript references. Also, glad to contribute to this project, I would help you with the code as well but I am still learning Typescript and I don't feel confident just yet :)

Feel free to close this issue if you`d like.

[RobinBuschmann]

Thank you, I appreciate any help.

[RobinBuschmann]

@stelescuraul I've released a new version(0.3.0) with the issues you've discovered. thanks again