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

Docs: boost MongoDB README and tutorial files #4059

Closed
1 task
totolef opened this issue Nov 7, 2019 · 24 comments · Fixed by loopbackio/loopback-connector-mongodb#576
Closed
1 task

Docs: boost MongoDB README and tutorial files #4059

totolef opened this issue Nov 7, 2019 · 24 comments · Fixed by loopbackio/loopback-connector-mongodb#576
Labels
db:MongoDB Topics specific to MongoDB Docs good first issue
Milestone

Comments

@totolef
Copy link

totolef commented Nov 7, 2019

Steps to reproduce

I just follow the steps described in the following article : Inclusion of related models .
The only changed I made was to use mongodb instead of mySQL, and when I do

GET http://localhost:3000/customers?filter[include][][relation]=orders

I don't get the same result as in the article.

Current Behavior

I currently get :
[ { id: 1, name: 'Thor', }, { id: 2, name: 'Captain', }, ];

Expected Behavior

In the article :
[ { id: 1, name: 'Thor', orders: [ {id: 1, desc: 'Mjolnir', customerId: 1}, {id: 3, desc: 'Rocket Raccoon', customerId: 1}, ], }, { id: 2, name: 'Captain', orders: [{id: 2, desc: 'Shield', customerId: 2}], }, ];

Additional information

The article say :

It might not work well with ObjectId of MongoDB. Related GH issue: Spike: robust handling of ObjectID type for MongoDB

But in the issue, they speak about the type of the id, and actually, the id returned by Mongodb is a string.

I'm not an expert at all, so I might be missing something.
Would be interested if you have a workaround.

Acceptance Criteria

@totolef totolef added the bug label Nov 7, 2019
@dhmlau
Copy link
Member

dhmlau commented Nov 8, 2019

@agnes512, could you please take a look? Thanks.

@agnes512
Copy link
Contributor

@totolef MongoDB uses ObjectId as its id type. And ObjectID is represented as a string when converted to JSON. So we defines the id type with type string in LB4, so for your id property, it should be defined as this way instead of type number:

@model()
export class Customer extends Entity {
  @property({
    type: string,
    id: true,
    generated: true // when you create an instance, the id is generated by Mongo itself
  })
  id: string;
  ...

Unfortunately I don't think the controller( REST APIs) work well with inclusion when using MondoDB :(

For example, I have a Customer:

{ "_id" : ObjectId("5dc8abb25771d13b0a262a94"), "name" : "Thor" }

when create an Order with Thor's id through the controller endpoint POST /orders:

{customerId: '5dc8abb25771d13b0a262a94', desc: `Hammer`} 

the customerId I pass in to the POST /orders will become a string instead of ObjectId.

I guess that's why it couldn't find the correct instances if we create the orders with controllers.
This is the ObjectId issue described in that spike.

With MongoDB, the inclusion only works at the repository level. e.g:

      const thor = await customerRepository.create({name: 'Thor'});
      const thorOrder = await orderRepo.create({
        customerId: thor.id,
        description: "Hammer",
      });
      const result = await customerRepo.find({
        include: [{relation: 'orders'}],
      });

@agnes512
Copy link
Contributor

closing as it's the issue described in #3456. Feel free to continue the discussion over there.

@bajtos
Copy link
Member

bajtos commented Nov 15, 2019

the customerId I pass in to the POST /orders will become a string instead of ObjectId.

To make relations work with MongoDB, the foreign key property must be defined as follows:

@property({
  type: 'string',
  mongodb: {dataType: 'ObjectID'},
})
customerId: string

This will tell our MongoDB connector to convert string values supplied via REST API to ObjectID values used by the database.

I am re-opening this issue, we should improve our documentation to make it more clear what's needed to make relations & inclusion resolvers work with MongoDB.

@agnes512
Copy link
Contributor

To make relations work with MongoDB, the foreign key property must be defined as follows:

@property({
  type: 'string',
  mongodb: {dataType: 'ObjectID'},
})
customerId: string

I did, my id property:

  @property({
    type: 'string',
    id: true,
    generated: true,
    mongodb: {dataType: 'ObjectID'},
  })
  id?: string;

and customerId

  @property({
    type: 'string',
    mongodb: {dataType: 'ObjectID'},
  })
  customerId?: string;

and I even tried this in the POST /orders endpoint to create instances at repo level,

  @post('/orders', {
...
  ): Promise<Order> {
    const cus = await this.customerRepository.create({name: 'TEST'});
    console.log(typeof cus.id ); // object
    return this.orderRepository.create({
      desc: 'test customerId',
      customerId: cus.id,
    });
  }

But in MongoDB it still shows that the customerId I just created is a string :(

@agnes512
Copy link
Contributor

> db.Order.find();
{ "_id" : ObjectId("5dcb3765f62dc495e4882643"), "description" : "Thor's Mjolnir", 
"customerId" : ObjectId("5dcb3765f62dc495e4882642") } // created through our repository-tests
{ "_id" : ObjectId("5dc8a9a202605439e3aebdba"), "desc" : "test customerId", 
"customerId" : "5dc8a99402605439e3aebdb9" }   // created through the end point above

when traverse related models with debugger, the find method in dao.js only returns the first order instance.

@bajtos
Copy link
Member

bajtos commented Nov 15, 2019

@agnes512 thank you for verifying. Looks like my understanding of mongodb.dataType was not correct then.

Could you please share the app you are using to test this? I would like to see if I can spot any easy fix.

@bajtos bajtos self-assigned this Nov 15, 2019
@agnes512
Copy link
Contributor

@bajtos sure, thanks! sorry I couldn't figure it out myself 😅

The app: https://github.com/agnes512/lb4Mongo/tree/master/mon
( POST /orders in order.controller.ts in modified)

And I use rpeository-tests in loopback-next a lot to compare the differences ( it bothers me that it works in our tests but not real apps)

I mainly use tests in has-many-inclusion-resolver file to create insatnces:

comment out

    //beforeEach(async () => {
      //await customerRepo.deleteAll();
      //await orderRepo.deleteAll();
    //});

and it.only... the second test.

These should be able to create instances in the same database repository_tests.

@bajtos
Copy link
Member

bajtos commented Nov 21, 2019

Here are my findings:

  • Your application uses loopback-connector-mongodb version 4.x which does not support dataType: 'ObjectID' yet. This feature was implemented in the version 5.0.0.
  • When I upgrade the connector to v5 and try to create an order, the server crashes.

Request body for POST /orders:

{
  "desc": "a pencil",
  "customerId": "5dd6acd8242760334f6aef63"
}

Error printed to the console:

Error: 5dd6ade040647f35d746e7a4 is not an ObjectID string
    at coerceToObjectId (/Users/bajtos/src/loopback/lb4mongo/mon/node_modules/loopback-connector-mongodb/lib/mongodb.js:1944:13)
    at MongoDB.coerceId (/Users/bajtos/src/loopback/lb4mongo/mon/node_modules/loopback-connector-mongodb/lib/mongodb.js:560:15)
    at /Users/bajtos/src/loopback/lb4mongo/mon/node_modules/loopback-connector-mongodb/lib/mongodb.js:599:20
    at /Users/bajtos/src/loopback/lb4mongo/mon/node_modules/loopback-datasource-juggler/lib/observer.js:269:22
    at doNotify (/Users/bajtos/src/loopback/lb4mongo/mon/node_modules/loopback-datasource-juggler/lib/observer.js:157:49)
   (...)

The value 5dd6ade040647f35d746e7a4 is the same as returned by GET /customer.

I am using MongoDB server version 3.4.10.

/cc @hacksparrow

@bajtos
Copy link
Member

bajtos commented Nov 21, 2019

I managed to track down the problem, will open a pull request.

@agnes512
Copy link
Contributor

agnes512 commented Nov 21, 2019

@bajtos I got the same error too yesterday when I was testing out dataType: ObjectId ( in loopback-next folder, so the modules should be latest) with replacement operations. I think I solved it by only decorating the customerId with mongodb: {dataType: 'ObjectID'} ( not the id property). Based on the test cases in mongo id.test.js, creation works for string-like id. But I don't see if it works for id as ObjectId.

@bajtos
Copy link
Member

bajtos commented Nov 21, 2019

With my fix in place (see loopbackio/loopback-connector-mongodb#553), the records in the database are created with the correct type.

> db.Order.find()
{ "_id" : ObjectId("5dd6acee242760334f6aef65"), "desc" : "test customerId", "customerId" : "5dd6acee242760334f6aef64" }

Now I need to verify that inclusion resolvers work too, unfortunately I have run out of time today.

@bajtos
Copy link
Member

bajtos commented Nov 21, 2019

Weird. When creating new orders via POST /orders endpoint, it looks like my user-supplied customerId is replaced with a unique auto-generated object id.

@bajtos bajtos changed the title Inclusion resolver with MongoDD Inclusion resolver with MongoDB Nov 26, 2019
@bajtos bajtos added the db:MongoDB Topics specific to MongoDB label Nov 26, 2019
@SidharthRaveendran
Copy link

I'm unable to get relations to work with MongoDB as well, is there any workaround to get it running at the moment?

This is my repository following the Todo and TodoList tutorial
https://github.com/SidharthRaveendran/loopback_relationship

@dougal83
Copy link
Contributor

dougal83 commented Dec 24, 2019

I'm unable to get relations to work with MongoDB as well, is there any workaround to get it running at the moment?

This is my repository following the Todo and TodoList tutorial
https://github.com/SidharthRaveendran/loopback_relationship

I'm not seeing todo-list-controller.ts in your project. Perhaps taking another look at the tutorial or compare your code to the prebuilt example might be a good next step. To create the example locally use lb4 example todo-list. Happy to take a look if you're having the same MongoDb issue with the generated example code.

@SidharthRaveendran
Copy link

Sorry, missed to push that part of the code, I've updated the repo but still have the issue. Could you tell me how to fix it?

@agnes512
Copy link
Contributor

@SidharthRaveendran Hi, what kind of errors are you getting? Are they related to ObjectId?

@SidharthRaveendran
Copy link

SidharthRaveendran commented Dec 24, 2019

Hi, @agnes512 I'm new to loopback framework, so I'm not sure if it's related to ObjectId or not, but I'm guessing it's related to the inclusion resolver.

This is the error i get.

Unhandled error in GET /todo-lists/5e01c04bc798688d01f91ab5/todos: 500 TypeError: Cannot read property 'type' of undefined
    at Object.resolveBelongsToMetadata (/app/loopback_relations/node_modules/@loopback/repository/src/relations/belongs-to/belongs-to.helpers.ts:27:21)
    at Object.createBelongsToAccessor (/app/loopback_relations/node_modules/@loopback/repository/src/relations/belongs-to/belongs-to-accessor.ts:46:16)
    at TodoRepository.createBelongsToAccessorFor (/app/loopback_relations/node_modules/@loopback/repository/src/repositories/legacy-juggler-bridge.ts:292:12)
    at new TodoRepository (/app/loopback_relations/src/repositories/todo.repository.ts:19:26)
    at value_promise_1.transformValueOrPromise.args (/app/loopback_relations/node_modules/@loopback/context/src/resolver.ts:73:14)
    at Object.transformValueOrPromise (/app/loopback_relations/node_modules/@loopback/context/src/value-promise.ts:270:12)
    at Object.instantiateClass (/app/loopback_relations/node_modules/@loopback/context/src/resolver.ts:66:35)
    at _setValueGetter (/app/loopback_relations/node_modules/@loopback/context/src/binding.ts:533:29)
    at Binding._getValue (/app/loopback_relations/node_modules/@loopback/context/src/binding.ts:410:14)
    at resolution_session_1.ResolutionSession.runWithBinding.s (/app/loopback_relations/node_modules/@loopback/context/src/binding.ts:305:23)
    at value_promise_1.tryWithFinally (/app/loopback_relations/node_modules/@loopback/context/src/resolution-session.ts:114:13)
    at Object.tryWithFinally (/app/loopback_relations/node_modules/@loopback/context/src/value-promise.ts:196:14)
    at Function.runWithBinding (/app/loopback_relations/node_modules/@loopback/context/src/resolution-session.ts:113:12)
    at Binding.getValue (/app/loopback_relations/node_modules/@loopback/context/src/binding.ts:302:40)
    at RequestContext.getValueOrPromise (/app/loopback_relations/node_modules/@loopback/context/src/context.ts:956:32)
    at RequestContext.get (/app/loopback_relations/node_modules/@loopback/context/src/context.ts:766:17)

@dougal83
Copy link
Contributor

@SidharthRaveendran I've just created https://github.com/dougal83/loopback4-example-todo-list-mongodb so you have a reference to work toward. Unfortunately as discussed in this issue the include functionality doesn't appear to be working.

@SidharthRaveendran
Copy link

@dougal83 the include functionality seems to be working in your example! thank you

Screenshot 2019-12-25 at 8 41 47 AM

@SidharthRaveendran
Copy link

SidharthRaveendran commented Dec 26, 2019

@dougal83 But I still can't seem to figure out what part of your code fixed this issue :(

@dougal83
Copy link
Contributor

@dougal83 But I still can't seem to figure out what part of your code fixed this issue :(

@SidharthRaveendran I bumped the version of loopback-connector-mongodb to 5.x.x. I suspect that could be the reason, but oddly enough when I run my example the inclusion doesn't work.

Probably best to wait for @bajtos who understands the issue.

@bajtos
Copy link
Member

bajtos commented Mar 6, 2020

I am afraid I don't have bandwidth to look into this issue anytime soon 😢

@bajtos bajtos removed their assignment Mar 9, 2020
@deepakrkris deepakrkris added the p1 label Apr 16, 2020
@agnes512
Copy link
Contributor

I just checked thoroughly:

  • Need to make sure the package version is loopback-connector-mongodb: "^5.2.1"

  • If dataType: 'ObjectID' is not set (no ObjectID coercion to ids):
    CRUD can be operated with non-objectid-like string or objectid-like string ids. Inclusion works as well

  • If dataType: 'ObjectID' is set. To make sure relations work as expected, all pk, fk have set dataType: 'ObjectID. e.g:

@property({
    type: 'string',
    id: true,
    mongodb: {dataType: 'ObjectID'}
  })
  id: string;
...
  @belongsTo(() => TodoList, {}, {
    mongodb: {dataType: 'ObjectID'}
  })
  todoListId: string;  // fk

These should be documented in mongo connector README and LB3 & LB4 mongodb connector tutorials.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
db:MongoDB Topics specific to MongoDB Docs good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants