Complete integration of in-memory cache, and connect with AuthCodeEntity #21
Conversation
| @@ -9,4 +9,7 @@ export class AuthSecretEntity extends BaseEntity { | |||
|
|
|||
| @Property() | |||
| encryptedClientSecret!: string | |||
|
|
|||
| @Property() | |||
| isVerified!: boolean | |||
There was a problem hiding this comment.
this will warrant another migration
| async getEntity(entityKey: string): Promise<RedisGetEntityResponse<RedisEntityType>>{ | ||
| return new Promise((resolve) => { | ||
| RedisClient().get(entityKey, (err, value) => { | ||
| if(err || value === null){ |
There was a problem hiding this comment.
I sound like a broken record, but you might want to run Prettier
There was a problem hiding this comment.
Finally configured it correctly - expect a very large formatting commit.
| return { | ||
| clientId, | ||
| userId, | ||
| authCodeString, | ||
| id | ||
| } |
There was a problem hiding this comment.
does this deliberately exclude the EntityKey?
There was a problem hiding this comment.
Yep, that's not relevant to the stored json.
| url: `${process.env.CACHE_URL}`, | ||
| password: `${process.env.CACHE_PASSWORD}` | ||
| }) | ||
| let redisClient: null | redis.RedisClient = null |
| async exists(clientId: string): Promise<Result<boolean, DBErrors>> { | ||
| const authSecretEntity = await this.authSecretEntityRepo.findOne({ | ||
| clientId: clientId, | ||
| }) | ||
| if(authSecretEntity !== null){ | ||
| return Result.ok(authSecretEntity !== null) | ||
| } else { | ||
| return Result.err(new DBError.AuthSecretNotFoundError(clientId)) | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm pretty sure this function isn't doing what's intended. If the repo can't find an authSecretEntity, shouldn't it return false instead of an error? Same comment also applies to Mock repo.
| @@ -1,5 +1,11 @@ | |||
| import { Router } from 'express' | |||
| import { Controllers } from '../../../../../setup/application' | |||
| import RateLimit from 'express-rate-limit' | |||
There was a problem hiding this comment.
Was going to include this in separate PR, but thought it would worth it to squeeze it into this once.
| "singleQuote": true, | ||
| "printWidth": 100 | ||
| } |
There was a problem hiding this comment.
nice fix, added to loolabs/waterpark#203
| @@ -10,13 +10,13 @@ export class MikroAuthSecretRepo implements AuthSecretRepo { | |||
| constructor(protected authSecretEntityRepo: EntityRepository<AuthSecretEntity>) {} | |||
|
|
|||
| async exists(clientId: string): Promise<Result<boolean, DBErrors>> { | |||
| const authSecretEntity = await this.authSecretEntityRepo.findOne({ | |||
| clientId: clientId, | |||
| const authSecretEntity = await this.authSecretEntityRepo.findOne({ | |||
There was a problem hiding this comment.
I remember there was a pretty hefty origin story behind DBErrors about how we'd handle errors from Mikro. Does that pattern require a try/catch here?
There was a problem hiding this comment.
Hmmm, Mikro doesn't seem to throw an error here. But in general, errors that could occur in db methods like this would have a DBError assigned.
| }) | ||
| if(authSecretEntity !== null){ | ||
| return Result.ok(authSecretEntity !== null) | ||
| if (authSecretEntity !== null) { |
There was a problem hiding this comment.
could alternatively write return Result.ok(authSecretEntity !== null)
| for (const authSecretEntity of this.authSecretEntities.values()) { | ||
| if (authSecretEntity.clientId === clientId) return Result.ok(true) | ||
| } | ||
| return Result.err(new DBError.AuthSecretNotFoundError(clientId)) | ||
| return Result.ok(false) |
There was a problem hiding this comment.
this is a mock repo so performance doesn't matter, but you could actually implement it in O(1) time with return Result.ok(this.authSecretEntities.has(clientId))
| if (user !== null) { | ||
| return Result.ok(true) | ||
| } else { | ||
| return Result.err(new DBError.UserNotFoundError(userEmail.value)) | ||
| return Result.ok(false) | ||
| } |
There was a problem hiding this comment.
return Result.ok(user !== null)
Closes #11 and #13.
This PR includes a complete
Redisentity integration into the auth repo, specifically to serve theAuthCodeEntity. TheAuthCodestill maintains theAggregateRootdomain events, but has it's own base repository which connects to the configuredRedisinstance. The repository makes use of theRedisset and get methods, optionally applying TTL to the given entity to expire it easily after some time has passed.The
Redisrepo caches objects by stringifying the corresponding typescript entity. By overriding thetoJSONmethod in any entity derived class, the repo controls what it saved whenJSON.stringifyis called with the entity object.