Understanding middleware store calls

[callmetwan]

Hi! Thanks for building this library, it's real nifty!

I am vetting it for a project I'm working on and it seems like a good fit. I had a few questions about the inner workings of Middleware.ts.

https://github.com/jcs224/hono_sessions/blob/main/src/Middleware.ts#L103-L115

    if (c.get('session_key_rotation') === true && !(store instanceof CookieStore)) {
      await store.deleteSession(sid)
      sid = await nanoid(21)
      await store.createSession(sid, session.getCache())

      setCookie(c, sessionCookieName, encryptionKey ? await encrypt(encryptionKey, sid) : sid, cookieOptions)
    }

    store instanceof CookieStore ? await store.persistSessionData(c, session.getCache()) : await store.persistSessionData(sid, session.getCache())

    if (session.getCache()._delete) {
      store instanceof CookieStore ? await store.deleteSession(c) : await store.deleteSession(sid)
    }
  }

The conditional check to rotate the session key makes sense to me. What I'm struggling with is the next line immediately calls persistSessionData.

As it stands the session is created and then it is immediately updated, seemingly resulting in an unnecessary roundtrip to the data store. Is there

Next there is a check for deletion. Why do this at the end rather than start with that check and skip session rotation/persistence?

My thoughts as code:

if (session.getCache()._delete) {
      store instanceof CookieStore ? await store.deleteSession(c) : await store.deleteSession(sid)
    } else {
      if (c.get('session_key_rotation') === true && !(store instanceof CookieStore)) {
        await store.deleteSession(sid)
        sid = await nanoid(21)
        await store.createSession(sid, session.getCache())

        setCookie(c, sessionCookieName, encryptionKey ? await encrypt(encryptionKey, sid) : sid, cookieOptions)
      } else {
        store instanceof CookieStore ? await store.persistSessionData(c, session.getCache()) : await store.persistSessionData(sid, session.getCache())
      }
    }

Thanks for letting me muse!

[jcs224]

You're probably right about some things not being perfectly efficient. The reason being, some features got added over time on top of existing code, and I haven't bothered to evaluate the old code after the new code is written.

That said, this looks good and if you're willing, make the change and run the tests and make a PR 🙂 You can find instructions at the bottom of the README for running them locally.

[callmetwan]

Thanks for the reply. Happy to do that!

[callmetwan]

Pull request created!