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

Add ability to add custom metadata to session table #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jeremija
Copy link

@jeremija jeremija commented Jan 9, 2019

Hi there,

Since there is an ISession interface and it is possible to add custom columns to the Session entity, I have extracted the repository.save part into a separate protected function to allow for easy overrides.

For example, if we wanted to add a userId column to the session table to be able to fetch all sessions for a specific user:

class MySessionStore extends TypeormStore {
  protected saveSession(session: ISession, sessionData: any) {
    (session as any).userId = sessionData.userId;
    return super.saveSession(session, sessionData);
  }
}

Let me know what you think!

@nykula
Copy link
Contributor

nykula commented Jan 9, 2019

Hello, thanks for your proposal. Will the following alternative suffice?

diff --git a/src/app/TypeormStore/TypeormStore.ts b/src/app/TypeormStore/TypeormStore.ts
index 86cffa1..0351f8c 100644
--- a/src/app/TypeormStore/TypeormStore.ts
+++ b/src/app/TypeormStore/TypeormStore.ts
@@ -102,2 +98,3 @@ export class TypeormStore extends Store {
       .then(() => this.repository.save({
+        ...sess,
         expiredAt: Date.now() + ttl * 1000,

Extending the repository like usual would be straightforward and type-safe then.

@Entity()
class Session implements ISession {
  @Index() @Column("bigint") public expiredAt = Date.now();
  @PrimaryColumn("varchar", { length: 255 }) public id = "";
  @Column("text") public json = "";
  @Column("varchar", { length: 255 }) public userId = "";
}

@EntityRepository(Session)
class SessionRepository extends Repository<Session> {
  /** Fetches all sessions for a specific user. */
  public findByUser(userId: string) {
    return this.findOne({ userId });
  }

  // `save(session)` accepts your Session instance with `userId`, no need for `as any`
}

class Api {
  public express = Express().use(
    ExpressSession({
      // Note, getCustomRepository in place of getRepository.
      store: new TypeormStore().connect(getCustomRepository(SessionRepository)),
      secret: "my secret"
    })
  );
}

If you go with this, please add yourself to contributors in package.json and the copyright header, modify the existing test so the views field has a type check, and document the behavior in README.

@yagoferrer
Copy link
Contributor

Hello! @nykula I was wondering if you have merged this code, or if you need help to do that.
Thanks,
Yago

@nykula
Copy link
Contributor

nykula commented Mar 3, 2020

@yagoferrer Haven't merged yet. Could you please try adding the change like in my comment above to your fork, and add some simple cases for checking it to TypeormStore.test.ts?

@araggohnxd
Copy link

are we any closer to getting this feature implemented?

@jeremija
Copy link
Author

I've personally stopped using NodeJS for backend work years ago so I no longer have any interest in pursuing this.

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

Successfully merging this pull request may close these issues.

None yet

4 participants