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

feat(sessions): implement ttl and flash #12693

Merged
merged 8 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ export const AstroConfigSchema = z.object({
return val;
})
.optional(),
ttl: z.number().optional(),
})
.optional(),
svg: z
Expand Down
74 changes: 46 additions & 28 deletions packages/astro/src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ export const PERSIST_SYMBOL = Symbol();
const DEFAULT_COOKIE_NAME = 'astro-session';
const VALID_COOKIE_REGEX = /^[\w-]+$/;

interface SessionEntry {
data: any;
expires?: number | 'flash';
}

export class AstroSession<TDriver extends SessionDriverName = any> {
// The cookies object.
#cookies: AstroCookies;
Expand All @@ -32,19 +37,23 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
#cookieName: string;
// The unstorage object for the session driver.
#storage: Storage | undefined;
#data: Map<string, any> | undefined;
#data: Map<string, SessionEntry> | undefined;
// The session ID. A v4 UUID.
#sessionID: string | undefined;
// Sessions to destroy. Needed because we won't have the old session ID after it's destroyed locally.
#toDestroy = new Set<string>();
// Session keys to delete. Used for sparse data sets to avoid overwriting the deleted value.
// Session keys to delete. Used for partial data sets to avoid overwriting the deleted value.
#toDelete = new Set<string>();
// Whether the session is dirty and needs to be saved.
#dirty = false;
// Whether the session cookie has been set.
#cookieSet = false;
// Whether the session data is sparse and needs to be merged with the existing data.
#sparse = true;
// The local data is "partial" if it has not been loaded from storage yet and only
// contains values that have been set or deleted in-memory locally.
// We do this to avoid the need to block on loading data when it is only being set.
// When we load the data from storage, we need to merge it with the local partial data,
// preserving in-memory changes and deletions.
#partial = true;

constructor(
cookies: AstroCookies,
Expand All @@ -67,7 +76,7 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
* Gets a session value. Returns `undefined` if the session or value does not exist.
*/
async get<T = any>(key: string): Promise<T | undefined> {
return (await this.#ensureData()).get(key);
return (await this.#ensureData()).get(key)?.data;
}

/**
Expand All @@ -88,22 +97,22 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
* Gets all session values.
*/
async values() {
return (await this.#ensureData()).values();
return [...(await this.#ensureData()).values()].map((entry) => entry.data);
}

/**
* Gets all session entries.
*/
async entries() {
return (await this.#ensureData()).entries();
return [...(await this.#ensureData()).entries()].map(([key, entry]) => [key, entry.data]);
}

/**
* Deletes a session value.
*/
delete(key: string) {
this.#data?.delete(key);
if (this.#sparse) {
if (this.#partial) {
this.#toDelete.add(key);
}
this.#dirty = true;
Expand All @@ -113,7 +122,7 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
* Sets a session value. The session is created if it does not exist.
*/

set<T = any>(key: string, value: T) {
set<T = any>(key: string, value: T, { ttl }: { ttl?: number | 'flash' } = {}) {
if (!key) {
throw new AstroError({
...SessionStorageSaveError,
Expand All @@ -138,10 +147,19 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
this.#cookieSet = true;
}
this.#data ??= new Map();
this.#data.set(key, value);
const lifetime = ttl ?? this.#config.ttl;
const expires = typeof lifetime === 'number' ? Date.now() + lifetime * 1000 : lifetime;
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment that explains the 1000?

this.#data.set(key, {
data: value,
expires,
});
this.#dirty = true;
}

flash<T = any>(key: string, value: T) {
this.set(key, value, { ttl: 'flash' });
}

/**
* Destroys the session, clearing the cookie and storage if it exists.
*/
Expand Down Expand Up @@ -194,6 +212,7 @@ export class AstroSession<TDriver extends SessionDriverName = any> {

if (this.#dirty && this.#data) {
const data = await this.#ensureData();
this.#toDelete.forEach((key) => data.delete(key));
const key = this.#ensureSessionID();
let serialized;
try {
Expand Down Expand Up @@ -256,12 +275,12 @@ export class AstroSession<TDriver extends SessionDriverName = any> {

/**
* Attempts to load the session data from storage, or creates a new data object if none exists.
* If there is existing sparse data, it will be merged into the new data object.
* If there is existing partial data, it will be merged into the new data object.
*/

async #ensureData() {
const storage = await this.#ensureStorage();
if (this.#data && !this.#sparse) {
if (this.#data && !this.#partial) {
return this.#data;
}
this.#data ??= new Map();
Expand Down Expand Up @@ -289,26 +308,25 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
),
});
}
// The local data is "sparse" if it has not been loaded from storage yet. This means
// it only contains values that have been set or deleted in-memory locally.
// We do this to avoid the need to block on loading data when it is only being set.
// When we load the data from storage, we need to merge it with the local sparse data,
// preserving in-memory changes and deletions.

if (this.#sparse) {
// For sparse updates, only copy values from storage that:
// 1. Don't exist in memory (preserving in-memory changes)
// 2. Haven't been marked for deletion
for (const [key, value] of storedMap) {
if (!this.#data.has(key) && !this.#toDelete.has(key)) {
this.#data.set(key, value);

const now = Date.now();

// Only copy values from storage that:
// 1. Don't exist in memory (preserving in-memory changes)
// 2. Haven't been marked for deletion
// 3. Haven't expired
for (const [key, value] of storedMap) {
const expired = typeof value.expires === 'number' && value.expires < now;
if (!this.#data.has(key) && !this.#toDelete.has(key) && !expired) {
this.#data.set(key, value);
if (value?.expires === 'flash') {
this.#toDelete.add(key);
this.#dirty = true;
}
}
} else {
this.#data = storedMap;
}

this.#sparse = false;
this.#partial = false;
return this.#data;
} catch (err) {
await this.#destroySafe();
Expand Down
5 changes: 5 additions & 0 deletions packages/astro/src/types/public/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ interface CommonSessionConfig {
cookie?:
| string
| (Omit<AstroCookieSetOptions, 'httpOnly' | 'expires' | 'encode'> & { name?: string });

/**
* Default session duration in seconds. If not set, the session will be stored until deleted, or until the cookie expires.
*/
ttl?: number;
}

interface BuiltinSessionConfig<TDriver extends keyof BuiltinDriverOptions>
Expand Down
1 change: 1 addition & 0 deletions packages/astro/test/fixtures/sessions/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default defineConfig({
experimental: {
session: {
driver: 'fs',
ttl: 20,
},
},
});
4 changes: 4 additions & 0 deletions packages/astro/test/fixtures/sessions/src/pages/flash.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
Astro.session.flash('flash-value', `Flashed value at ${new Date().toISOString()}`);
return Astro.redirect('/');
Comment on lines +2 to +3
Copy link
Member

Choose a reason for hiding this comment

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

Can we add more cases, such as:

  • usage with Astro.rewrite
  • usage with the next("/some-route") (the other form of rewrite): see if the value changes when the next middleware function read from .flash
  • usage with multiple middleware functions e.g. sequence(fn1, fn2)

---
3 changes: 3 additions & 0 deletions packages/astro/test/fixtures/sessions/src/pages/index.astro
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
const value = await Astro.session.get('value');
const flash = await Astro.session.get('flash-value');
---
<html lang="en">
<head>
Expand All @@ -9,5 +10,7 @@ const value = await Astro.session.get('value');

<h1>Hi</h1>
<p>{value}</p>
<p>Flash: {flash}</p>
<p><a href="/flash">Set flash</a></p>
<a href="/cart" style="font-size: 36px">🛒</a>
</html>
47 changes: 43 additions & 4 deletions packages/astro/test/units/sessions/astro-session.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const stringify = (data) => JSON.parse(devalueStringify(data));
const defaultConfig = {
driver: 'memory',
cookie: 'test-session',
ttl: 60,
};

// Helper to create a new session instance with mocked dependencies
Expand Down Expand Up @@ -142,7 +143,7 @@ test('AstroSession - Data Persistence', async (t) => {

await t.test('should load data from storage', async () => {
const mockStorage = {
get: async () => stringify(new Map([['key', 'value']])),
get: async () => stringify(new Map([['key', { data: 'value' }]])),
setItem: async () => {},
};

Expand All @@ -151,8 +152,46 @@ test('AstroSession - Data Persistence', async (t) => {
const value = await session.get('key');
assert.equal(value, 'value');
});

await t.test('should remove expired session data', async () => {
const mockStorage = {
get: async () => stringify(new Map([['key', { data: 'value', expires: -1 }]])),
setItem: async () => {},
};

const session = createSession(defaultConfig, defaultMockCookies, mockStorage);

const value = await session.get('key');

assert.equal(value, undefined);
});


await t.test('should not write flash session data to storage a second time', async () => {
let storedData;
const mockStorage = {
get: async () => stringify(new Map([['key', { data: 'value', expires: "flash" }]])),
setItem: async (_key, value) => {
storedData = value;
},
};
const session = createSession(defaultConfig, defaultMockCookies, mockStorage);

const value = await session.get('key');

assert.equal(value, 'value');

await session[PERSIST_SYMBOL]();

const emptyMap = devalueStringify(new Map());
assert.equal(storedData, emptyMap);
})


});



test('AstroSession - Error Handling', async (t) => {
await t.test('should throw error when setting invalid data', async () => {
const session = createSession();
Expand Down Expand Up @@ -227,9 +266,9 @@ test('AstroSession - Sparse Data Operations', async (t) => {
await t.test('should handle multiple operations in sparse mode', async () => {
const existingData = stringify(
new Map([
['keep', 'original'],
['delete', 'remove'],
['update', 'old'],
['keep', { data: 'original' }],
['delete', { data: 'remove' }],
['update', { data: 'old' }],
]),
);

Expand Down
Loading