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

Help needed with relationMappings and eager loading #1350

Closed
mrsurname opened this issue May 28, 2019 · 6 comments
Closed

Help needed with relationMappings and eager loading #1350

mrsurname opened this issue May 28, 2019 · 6 comments

Comments

@mrsurname
Copy link

mrsurname commented May 28, 2019

How to reproduce

Very simplified tables

CREATE TABLE ideas (
    id BIGSERIAL NOT NULL primary key,
    text VARCHAR(1024)
);

CREATE TABLE cmt (
    id BIGSERIAL NOT NULL primary key,
    text VARCHAR(1024),
    idea BIGINT NOT NULL
)

Models

import { Model } from 'objection';

export default class Ideas extends Model {
  readonly id!: number;
  static tableName = 'ideas';
}
import { Model, RelationMappings } from 'objection';

export default class Cmt extends Model {
  static tableName = 'cmt';


  static modelPaths = [__dirname];

  static relationMappings: RelationMappings = {
    ideas: {
      relation: Model.BelongsToOneRelation,
      modelClass: 'ideas',
      join: {
        from: 'cmt.idea',
        to: 'ideas.id'
      }
    },
  };
}

A request somewhere

const withIdeas = await Cmt.query()
    .eager('ideas');

Response is:

Error: Cmt.relationMappings.ideas: join.from must have format TableName.columnName. For example "SomeTable.id" or in case of composite key ["SomeTable.a", "SomeTable.b"].

    at BelongsToOneRelation.createError (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/relations/Relation.js:232:14)
    at Object.createError (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/relations/Relation.js:46:32)
    at createRelationProperty (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/relations/Relation.js:346:17)
    at createJoinProperties (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/relations/Relation.js:302:20)
    at BelongsToOneRelation.setMapping (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/relations/Relation.js:54:11)
    at Object.keys.reduce (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/model/Model.js:735:29)
    at Array.reduce (<anonymous>)
    at getRelations (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/model/Model.js:731:46)
    at cachedGet (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/model/Model.js:679:61)
    at Function.getRelations (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/model/Model.js:465:12)
    at WhereInEagerOperation.onAdd (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/queryBuilder/operations/eager/WhereInEagerOperation.js:36:34)
    at QueryBuilder.addOperation (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/queryBuilder/QueryBuilderOperationSupport.js:234:33)
    at addEagerFetchOperation (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/queryBuilder/QueryBuilder.js:1259:13)
    at beforeExecute (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/queryBuilder/QueryBuilder.js:1138:5)
    at Bluebird.try (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/queryBuilder/QueryBuilder.js:568:31)
    at tryCatcher (/home/vladimir/Projects/ucom/ucom.backend/node_modules/bluebird/js/release/util.js:16:23)
    at Function.Promise.attempt.Promise.try (/home/vladimir/Projects/ucom/ucom.backend/node_modules/bluebird/js/release/method.js:39:29)
    at QueryBuilder.execute (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/queryBuilder/QueryBuilder.js:568:24)
    at QueryBuilder.then (/home/vladimir/Projects/ucom/ucom.backend/node_modules/objection/lib/queryBuilder/QueryBuilder.js:473:26)
    at process._tickCallback (internal/process/next_tick.js:68:7)

The error is happened here - empty array is returned
https://github.com/Vincit/objection.js/blob/master/lib/relations/RelationProperty.js#L205

A dirty fix:

https://github.com/Vincit/objection.js/blob/master/lib/utils/objectUtils.js#L64
change the last line

from
return Array.from(map.values());

to
return Array.from(map);


Please, help. What I'm doing wrong? :(

@koskimas
Copy link
Collaborator

Yes it does.

@Vincit Vincit locked as spam and limited conversation to collaborators May 28, 2019
@Vincit Vincit unlocked this conversation May 28, 2019
@koskimas
Copy link
Collaborator

Oh, there was a question at the bottom. I thought this was just one of those "your code that has 3600 tests doesn't work!#"!#!"#!"!!" issues.

Is the name of the ideas model file ideas.js (after compilation to js)?

@koskimas koskimas changed the title Eager loading does not work with the PostgreSQL Help needed with relationMappings and eager loading May 28, 2019
@koskimas koskimas reopened this May 28, 2019
@mrsurname
Copy link
Author

mrsurname commented May 28, 2019

Thank you a lot for the quick response, and I'm sorry about not to be obvious enough.

After searching a lot, I finally found a 3rd party dependency, which breaks eager relation. It is not a problem related to PostgreSQL.

Please observe a test project to investigate it.

https://github.com/vladimirice/objection-postgresql

If I add this dependency, an error occurs.
https://github.com/vladimirice/objection-postgresql/blob/master/example.js#L8

I use this dependency for one of my projects. A project above is just a sample project to reproduce an issue.

Maybe It is not a problem of objectionjs. Maybe a dependency 'node-dependency-injection' breaks something or use some tricks. But maybe not. I'll be pleased if you can take a look at what is happening.

Maybe this issue can help you to improve something in objection.js. Thanks!

UPD:
node -v v10.15.2
npm -v 6.9.1-next.0
PostgreSQL 10.6

@zazoomauro
Copy link

zazoomauro commented May 28, 2019

The conflict happens when require('collections')
https://github.com/montagejs/collections

Nothing related to node-dependency-injection

@mrsurname
Copy link
Author

@zazoomauro thanks a lot for the investigation. Yes, the problem is related to collections

@koskimas I investigate this issue at least four hour. Is it possible to somehow avoid objectionjs errors related to collections usage? If some other project users a dependency with collections it cannot use objectionjs or using collections can suddenly break a lot of objectionjs features.

I think it is a good proposal to aviod such kind of vulnerability

Thanks a lot for you work

@koskimas
Copy link
Collaborator

koskimas commented May 29, 2019

Is it possible to somehow avoid objectionjs errors related to collections usage?

It's possible, but I won't do that. No library should ever change the built-in methods like Array.find. You should drop whatever library that does this or put your efforts towards those libraries dropping these evil practices. It's insanely dangerous (and quite frankly stupid) to override the built-ins. Especially if they break the standards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants