adonisjs / lucid

AdonisJS SQL ORM. Supports PostgreSQL, MySQL, MSSQL, Redshift, SQLite and many more
https://lucid.adonisjs.com/
MIT License
1.03k stars 189 forks source link

Inherit `belongsTo` relation form parent Model #921

Closed Melchyore closed 6 months ago

Melchyore commented 1 year ago

Package version

18.3.0

Node.js and npm version

Node.js v18.2.1 npm v8.19.2

Sample Code (to reproduce the issue)

(The full code is in the Bonus section)

Hello, this is a follow-up related to this discussion

Mixin:

import {
  BelongsTo,
  belongsTo,
  column,
  LucidModel,
} from "@ioc:Adonis/Lucid/Orm";
import { NormalizeConstructor } from "@ioc:Adonis/Core/Helpers";
import User from "./User";

export function ShareableModel<T extends NormalizeConstructor<LucidModel>>(
  superclass: T
) {
  class SModel extends superclass {
    @column({ isPrimary: true })
    public id: number;

    @column()
    public ownerId: number;

    @belongsTo(() => User)
    public owner: BelongsTo<typeof User>;
  }

  return SModel;
}

Model that extends the above mixin:

import { DateTime } from "luxon";
import { BaseModel, column } from "@ioc:Adonis/Lucid/Orm";
import { compose } from "@ioc:Adonis/Core/Helpers";
import { ShareableModel } from "./Shareable";

export default class Item extends compose(BaseModel, ShareableModel) {
  @column.dateTime({ autoCreate: true })
  public createdAt: DateTime;
}

When you run pnpm run dev you'll see the following error

 TypeError: Cannot read properties of undefined (reading 'primaryKey')

   ⁃ SnakeCaseNamingStrategy.relationLocalKey
     C:/**/reproduction/node_modules/.pnpm/@adonisjs+lucid@18.3.0_6c14cb9d2d5b8a4ecf677920732b141b/node_modules/@adonisjs/lucid/build/src/Orm/NamingStrategies/SnakeCase.js:40
   ⁃ BelongsTo.boot
     C:/**/reproduction/node_modules/.pnpm/@adonisjs+lucid@18.3.0_6c14cb9d2d5b8a4ecf677920732b141b/node_modules/@adonisjs/lucid/build/src/Orm/Relations/BelongsTo/index.js:144
   ⁃ anonymous
     C:/**/reproduction/node_modules/.pnpm/@adonisjs+lucid@18.3.0_6c14cb9d2d5b8a4ecf677920732b141b/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:513

If you remove the belongsTo relationship, it works. After some investigation, it seems that the decorator is receiving an undefined related model.

BONUS (a sample repo to reproduce the issue)

Took the code from here and created the following reproduction repo: https://github.com/Melchyore/lucid-error-reproduction

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Melchyore commented 1 year ago

Keep open

Melchyore commented 11 months ago

A little update, in BelongsTo class, this.relatedModel returns a function but this.relatedModel() returns undefined.

thetutlage commented 6 months ago

I did not tried the reproduction you shared so far, but I think this approach will have many other limitations as well.

For example, the default table name for the relationship will be s_models and not items. Also, it might break the DB -> Model hydration layer.

I hope, we had traits like PHP, then all this would have been a lot easier, whereas in case of inheritance, the applied decorators do not know anything about the child model and hence they compute values as per the current (aka parent) model.


Solution

The solution is quite dirty, or maybe not, it depends how much visual clutter can you tolerate.

As the first step, you will be better off if you add properties and relationships to the final model directly. For example, create a function as follows.

function MakeShareable<T extends typeof BaseModel>(superclass: T): T {
  superclass.boot()
  superclass.$addColumn('userId', {})
  superclass.$addRelation('user', 'belongsTo', () => User, {})

  return superclass
}

Then use it on the Item model as follows.

import { MakeShareable } from './some_path'

class Item extends compose(BaseModel, ShareableModel) {
  @column.dateTime({ autoCreate: true })
  public createdAt: DateTime;
}

export default MakeShareable(Item)

Now, this will take care of the runtime side of things. However, for types you will have to create an interface and export it alongside the function.

interface Shareable {
  userId: number
  user: BelongsTo<typeof User>
}

Here's the final code will look like.

import { MakeShareable, Shareable } from './some_path'

interface Item extends Shareable {}

class Item extends compose(BaseModel, ShareableModel) {
  @column.dateTime({ autoCreate: true })
  public createdAt: DateTime;
}

export default MakeShareable(Item)
thetutlage commented 6 months ago

Okay, scratch my last response. Because, I wrote some tests and it seems to be working fine, because we are cloning and copying the relationships on the child model.

The reason it is not working for you is because of circular imports. If you log the User model inside the belongsTo decorator, you will find it to be undefined.

    @belongsTo(() => {
      console.log(User)
      return User
    })
thetutlage commented 6 months ago

Alright, I have to run a few more tests to see how it goes with circular dependencies. Will post updates here

thetutlage commented 6 months ago

It should be all good in the next release, but that will be the major release targeting v6

Melchyore commented 6 months ago

It should be all good in the next release, but that will be the major release targeting v6

Many thanks!