Skip to content
This repository was archived by the owner on Dec 12, 2023. It is now read-only.

Add resave option #47

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ Here's what the full _default_ module configuration looks like:
// Sessions aren't pinned to the user's IP address
ipPinning: false,
// Expiration of the sessions are not reset to the original expiryInSeconds on every request
rolling: false
rolling: false,
// Sessions are saved to the store, even if they were never modified during the request
resave: true
},
api: {
// The API is enabled
Expand Down
7 changes: 3 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"argon2": "^0.30.2",
"dayjs": "^1.11.6",
"defu": "^6.1.0",
"fast-deep-equal": "^3.1.3",
"h3": "^1.0.1",
"unstorage": "^1.0.1"
},
Expand Down
3 changes: 2 additions & 1 deletion src/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ const defaults: FilledModuleOptions = {
},
domain: false,
ipPinning: false as boolean|SessionIpPinningOptions,
rolling: false
rolling: false,
resave: true
},
api: {
isEnabled: true,
Expand Down
13 changes: 10 additions & 3 deletions src/runtime/server/middleware/session/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { deleteCookie, eventHandler, H3Event, parseCookies, setCookie } from 'h3'
import { nanoid } from 'nanoid'
import dayjs from 'dayjs'
import equal from 'fast-deep-equal'
import { SameSiteOptions, Session, SessionOptions } from '../../../../types'
import { dropStorageSession, getStorageSession, setStorageSession } from './storage'
import { processSessionIp, getHashedIpAddress } from './ipPinning'
Expand Down Expand Up @@ -147,17 +148,23 @@ const ensureSession = async (event: H3Event) => {
}

export default eventHandler(async (event: H3Event) => {
const sessionOptions = useRuntimeConfig().session.session as SessionOptions

// 1. Ensure that a session is present by either loading or creating one
await ensureSession(event)
const session = await ensureSession(event)
// 2. Save current state of the session
const source = { ...session }

// 2. Setup a hook that saves any changed made to the session by the subsequent endpoints & middlewares
// 3. Setup a hook that saves any changed made to the session by the subsequent endpoints & middlewares
event.res.on('finish', async () => {
// Session id may not exist if session was deleted
const session = await getSession(event)
if (!session) {
return
}

await setStorageSession(session.id, event.context.session)
if (sessionOptions.resave || !equal(event.context.session, source)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conflicts with #36, as the updated createdAt would not be saved.
So this should be updated when #36 is merged.

Suggested change
if (sessionOptions.resave || !equal(event.context.session, source)) {
if (sessionOptions.resave || sessionOptions.rolling || !equal(event.context.session, source)) {

Copy link
Contributor Author

@interpretor interpretor Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is not straightforward, I would suggest to merge only #48 instead of this and #41.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @interpretor, here you suggest to merge #48 and #41 (and abandon current one #47), while in #48 (comment) you suggest to merge #36, #41 and #47.

Can you please bring your reasoning on those changes and suggest from which one should I start a review? Perhaps #36 or #41?

Copy link
Contributor Author

@interpretor interpretor Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @valiafetisov, I think you misunderstood me. Here I suggest to merge #48, as it is based on #36, #41 and #47. #36 is already merged, and I don't recommend to merge #41 or #47 individually. Instead, I recommend to merge only #48 as it does handle the options from #41 and #47 better.

So it would be best to start the review from #48.

await setStorageSession(session.id, event.context.session)
}
})
})
9 changes: 8 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,14 @@ export interface SessionOptions {
* @example true
* @type boolean
*/
rolling: boolean
rolling: boolean,
/**
* Forces the session to be saved back to the session store, even if the session was never modified during the request.
* @default true
* @example false
* @type boolean
*/
resave: boolean
}

export interface ApiOptions {
Expand Down