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

@foal/typeorm fetchUser class add the ObjectID type in parameters and returns #548

Closed
Fredy22 opened this issue Sep 23, 2019 · 5 comments
Closed

Comments

@Fredy22
Copy link

Fredy22 commented Sep 23, 2019

The ObjectID type is missing in the fetchUser class, for the mangodb database with typeorm

* @export
* @param {(Class<{ id: number|string|ObjectID }>)} userEntityClass - The entity class.
* @returns {((id: number|string|ObjectID) => Promise<any>)} The returned function expecting an id.
*/
export function fetchUser(userEntityClass: Class<{ id: number|string|ObjectID }>): (id: >number|string|ObjectID) => Promise<any> {
 return (id: number|string|ObjectID) => getRepository(userEntityClass).findOne({ id });
}
@LoicPoullain
Copy link
Member

LoicPoullain commented Oct 9, 2019

Sorry for the late reply. This is definitely an issue with current version 1 of FoalTS.

fetchUser function cannot add the type ObjectId in its definition because it is part of @foal/core. This package cannot rely on an ODM/ORM or a specific database such as mongodb.

The choice I made to define the id type as a number or a string definitely leads to issues when using MongoDB with TypeORM. When using @JWTRequired or @TokenRequired, the id object is converted to a string and is passed to the fetchUser as is. This is not a problem when using Mongoose, because the library accepts both string and ObjectID for document IDs. But in the case of TypeORM, AFAIK, the library does not accept strings for document IDs. And re-instanciating the id with new ObjectID(str) cannot work because the constructor expects the string to be encoded in hexadecimal and not in base64 which the framework uses.

This is definitely annoying because, in the state, I think we cannot use both the authentication system and TypeORM with MongoDB.

In the next version of FoalTS (version 2), the id will be of type any so that we do not have this design issue. However, it cannot be changed in version 1 because it could make some existing applications to break. 😕

@LoicPoullain LoicPoullain moved this from Backlog to To Do in Issue tracking Oct 9, 2019
@Fredy22
Copy link
Author

Fredy22 commented Oct 9, 2019

No problem. I suspected that testing mongodb with typeorm (still experimental) would not be easy :)
I think the easiest thing for me is to go through mongoose.

@LoicPoullain
Copy link
Member

@Fredy22 I finally found a way to solve this issue without adding breaking changes. The framework will export a new fetchMongoDBUser from @foal/typeorm for this.

The feature is on its way and will be released with v1.9

@Fredy22
Copy link
Author

Fredy22 commented May 7, 2020

Hello @LoicPoullain , ok thanks i test this :D

@LoicPoullain LoicPoullain mentioned this issue May 8, 2020
12 tasks
@LoicPoullain
Copy link
Member

The PR has been merged so I'm closing this issue. Version 1.9 should be released at the beginning of June.

In the meantime, you can copy paste this code in your application if you need it.

Issue tracking automation moved this from Work In Progress to Done / Closed This Release May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Issue tracking
  
Done / Closed This Release
Development

No branches or pull requests

2 participants