From cc820c5d5e176a8d71594d612af75e1c94b9bf02 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 3 Sep 2024 09:53:29 -0300 Subject: [PATCH 1/3] Fix mixed DB token env vars (#11894) * Fix mixed DB token env vars * Test env combinations * Fix linter issues * Fix linter issues --------- Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com> --- .changeset/empty-spoons-kiss.md | 5 + .../db/src/core/cli/commands/execute/index.ts | 4 +- .../db/src/core/cli/commands/push/index.ts | 16 ++- .../db/src/core/cli/commands/shell/index.ts | 5 +- .../db/src/core/cli/commands/verify/index.ts | 10 +- packages/db/src/core/cli/migration-queries.ts | 11 +- packages/db/src/core/integration/index.ts | 26 ++-- packages/db/src/core/utils.ts | 18 ++- packages/db/test/basics.test.js | 5 +- packages/db/test/static-remote.test.js | 3 +- packages/db/test/test-utils.js | 12 ++ packages/db/test/unit/remote-info.test.js | 117 ++++++++++++++++++ 12 files changed, 196 insertions(+), 36 deletions(-) create mode 100644 .changeset/empty-spoons-kiss.md create mode 100644 packages/db/test/unit/remote-info.test.js diff --git a/.changeset/empty-spoons-kiss.md b/.changeset/empty-spoons-kiss.md new file mode 100644 index 0000000000..8bb114ef6f --- /dev/null +++ b/.changeset/empty-spoons-kiss.md @@ -0,0 +1,5 @@ +--- +'@astrojs/db': patch +--- + +Fixes mixed environment variable for app token when using DB commands with libSQL remote. diff --git a/packages/db/src/core/cli/commands/execute/index.ts b/packages/db/src/core/cli/commands/execute/index.ts index c6c11cdbb6..9a5a1b8e2f 100644 --- a/packages/db/src/core/cli/commands/execute/index.ts +++ b/packages/db/src/core/cli/commands/execute/index.ts @@ -1,5 +1,4 @@ import { existsSync } from 'node:fs'; -import { getManagedAppTokenOrExit } from '@astrojs/studio'; import { LibsqlError } from '@libsql/client'; import type { AstroConfig } from 'astro'; import { green } from 'kleur/colors'; @@ -16,6 +15,7 @@ import { } from '../../../integration/vite-plugin-db.js'; import { bundleFile, importBundledFile } from '../../../load-file.js'; import type { DBConfig } from '../../../types.js'; +import { getManagedRemoteToken } from '../../../utils.js'; export async function cmd({ astroConfig, @@ -40,7 +40,7 @@ export async function cmd({ let virtualModContents: string; if (flags.remote) { - const appToken = await getManagedAppTokenOrExit(flags.token); + const appToken = await getManagedRemoteToken(flags.token); virtualModContents = getStudioVirtualModContents({ tables: dbConfig.tables ?? {}, appToken: appToken.token, diff --git a/packages/db/src/core/cli/commands/push/index.ts b/packages/db/src/core/cli/commands/push/index.ts index 0efa7ebcbf..0b7b32bf94 100644 --- a/packages/db/src/core/cli/commands/push/index.ts +++ b/packages/db/src/core/cli/commands/push/index.ts @@ -1,4 +1,3 @@ -import { getManagedAppTokenOrExit } from '@astrojs/studio'; import type { AstroConfig } from 'astro'; import { sql } from 'drizzle-orm'; import prompts from 'prompts'; @@ -7,7 +6,7 @@ import { createRemoteDatabaseClient } from '../../../../runtime/index.js'; import { safeFetch } from '../../../../runtime/utils.js'; import { MIGRATION_VERSION } from '../../../consts.js'; import type { DBConfig, DBSnapshot } from '../../../types.js'; -import { type Result, getRemoteDatabaseInfo } from '../../../utils.js'; +import { type RemoteDatabaseInfo, type Result, getManagedRemoteToken, getRemoteDatabaseInfo } from '../../../utils.js'; import { createCurrentSnapshot, createEmptySnapshot, @@ -26,8 +25,12 @@ export async function cmd({ }) { const isDryRun = flags.dryRun; const isForceReset = flags.forceReset; - const appToken = await getManagedAppTokenOrExit(flags.token); - const productionSnapshot = await getProductionCurrentSnapshot({ appToken: appToken.token }); + const dbInfo = getRemoteDatabaseInfo(); + const appToken = await getManagedRemoteToken(flags.token, dbInfo); + const productionSnapshot = await getProductionCurrentSnapshot({ + dbInfo, + appToken: appToken.token, + }); const currentSnapshot = createCurrentSnapshot(dbConfig); const isFromScratch = !productionSnapshot; const { queries: migrationQueries, confirmations } = await getMigrationQueries({ @@ -68,6 +71,7 @@ export async function cmd({ console.log(`Pushing database schema updates...`); await pushSchema({ statements: migrationQueries, + dbInfo, appToken: appToken.token, isDryRun, currentSnapshot: currentSnapshot, @@ -80,11 +84,13 @@ export async function cmd({ async function pushSchema({ statements, + dbInfo, appToken, isDryRun, currentSnapshot, }: { statements: string[]; + dbInfo: RemoteDatabaseInfo; appToken: string; isDryRun: boolean; currentSnapshot: DBSnapshot; @@ -99,8 +105,6 @@ async function pushSchema({ return new Response(null, { status: 200 }); } - const dbInfo = getRemoteDatabaseInfo(); - return dbInfo.type === 'studio' ? pushToStudio(requestBody, appToken, dbInfo.url) : pushToDb(requestBody, appToken, dbInfo.url); diff --git a/packages/db/src/core/cli/commands/shell/index.ts b/packages/db/src/core/cli/commands/shell/index.ts index 0b5c2c2f06..8237fdc1be 100644 --- a/packages/db/src/core/cli/commands/shell/index.ts +++ b/packages/db/src/core/cli/commands/shell/index.ts @@ -1,4 +1,3 @@ -import { getManagedAppTokenOrExit } from '@astrojs/studio'; import type { AstroConfig } from 'astro'; import { sql } from 'drizzle-orm'; import type { Arguments } from 'yargs-parser'; @@ -10,7 +9,7 @@ import { normalizeDatabaseUrl } from '../../../../runtime/index.js'; import { DB_PATH } from '../../../consts.js'; import { SHELL_QUERY_MISSING_ERROR } from '../../../errors.js'; import type { DBConfigInput } from '../../../types.js'; -import { getAstroEnv, getRemoteDatabaseInfo } from '../../../utils.js'; +import { getAstroEnv, getManagedRemoteToken, getRemoteDatabaseInfo } from '../../../utils.js'; export async function cmd({ flags, @@ -27,7 +26,7 @@ export async function cmd({ } const dbInfo = getRemoteDatabaseInfo(); if (flags.remote) { - const appToken = await getManagedAppTokenOrExit(flags.token); + const appToken = await getManagedRemoteToken(flags.token, dbInfo); const db = createRemoteDatabaseClient({ dbType: dbInfo.type, remoteUrl: dbInfo.url, diff --git a/packages/db/src/core/cli/commands/verify/index.ts b/packages/db/src/core/cli/commands/verify/index.ts index 950fb66156..d535fba1fe 100644 --- a/packages/db/src/core/cli/commands/verify/index.ts +++ b/packages/db/src/core/cli/commands/verify/index.ts @@ -1,4 +1,3 @@ -import { getManagedAppTokenOrExit } from '@astrojs/studio'; import type { AstroConfig } from 'astro'; import type { Arguments } from 'yargs-parser'; import type { DBConfig } from '../../../types.js'; @@ -9,6 +8,7 @@ import { getMigrationQueries, getProductionCurrentSnapshot, } from '../../migration-queries.js'; +import { getManagedRemoteToken, getRemoteDatabaseInfo } from '../../../utils.js'; export async function cmd({ dbConfig, @@ -19,8 +19,12 @@ export async function cmd({ flags: Arguments; }) { const isJson = flags.json; - const appToken = await getManagedAppTokenOrExit(flags.token); - const productionSnapshot = await getProductionCurrentSnapshot({ appToken: appToken.token }); + const dbInfo = getRemoteDatabaseInfo(); + const appToken = await getManagedRemoteToken(flags.token, dbInfo); + const productionSnapshot = await getProductionCurrentSnapshot({ + dbInfo, + appToken: appToken.token, + }); const currentSnapshot = createCurrentSnapshot(dbConfig); const { queries: migrationQueries, confirmations } = await getMigrationQueries({ oldSnapshot: productionSnapshot || createEmptySnapshot(), diff --git a/packages/db/src/core/cli/migration-queries.ts b/packages/db/src/core/cli/migration-queries.ts index 5c4a23557d..6f0e0a5e26 100644 --- a/packages/db/src/core/cli/migration-queries.ts +++ b/packages/db/src/core/cli/migration-queries.ts @@ -35,7 +35,7 @@ import type { ResolvedIndexes, TextColumn, } from '../types.js'; -import { type Result, getRemoteDatabaseInfo } from '../utils.js'; +import type { RemoteDatabaseInfo, Result } from '../utils.js'; const sqlite = new SQLiteAsyncDialect(); const genTempTableName = customAlphabet('abcdefghijklmnopqrstuvwxyz', 10); @@ -425,13 +425,12 @@ function hasRuntimeDefault(column: DBColumn): column is DBColumnWithDefault { } export function getProductionCurrentSnapshot(options: { + dbInfo: RemoteDatabaseInfo; appToken: string; }): Promise { - const dbInfo = getRemoteDatabaseInfo(); - - return dbInfo.type === 'studio' - ? getStudioCurrentSnapshot(options.appToken, dbInfo.url) - : getDbCurrentSnapshot(options.appToken, dbInfo.url); + return options.dbInfo.type === 'studio' + ? getStudioCurrentSnapshot(options.appToken, options.dbInfo.url) + : getDbCurrentSnapshot(options.appToken, options.dbInfo.url); } async function getDbCurrentSnapshot( diff --git a/packages/db/src/core/integration/index.ts b/packages/db/src/core/integration/index.ts index 263fa250a1..1b1eeec8e1 100644 --- a/packages/db/src/core/integration/index.ts +++ b/packages/db/src/core/integration/index.ts @@ -2,7 +2,7 @@ import { existsSync } from 'node:fs'; import { mkdir, writeFile } from 'node:fs/promises'; import { dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { type ManagedAppToken, getManagedAppTokenOrExit } from '@astrojs/studio'; +import type { ManagedAppToken } from '@astrojs/studio'; import { LibsqlError } from '@libsql/client'; import type { AstroConfig, AstroIntegration } from 'astro'; import { blue, yellow } from 'kleur/colors'; @@ -20,7 +20,7 @@ import { CONFIG_FILE_NAMES, DB_PATH } from '../consts.js'; import { EXEC_DEFAULT_EXPORT_ERROR, EXEC_ERROR } from '../errors.js'; import { resolveDbConfig } from '../load-file.js'; import { SEED_DEV_FILE_NAME } from '../queries.js'; -import { type VitePlugin, getDbDirectoryUrl } from '../utils.js'; +import { type VitePlugin, getDbDirectoryUrl, getManagedRemoteToken } from '../utils.js'; import { fileURLIntegration } from './file-url.js'; import { getDtsContent } from './typegen.js'; import { @@ -32,7 +32,7 @@ import { } from './vite-plugin-db.js'; function astroDBIntegration(): AstroIntegration { - let connectToStudio = false; + let connectToRemote = false; let configFileDependencies: string[] = []; let root: URL; let appToken: ManagedAppToken | undefined; @@ -72,12 +72,12 @@ function astroDBIntegration(): AstroIntegration { let dbPlugin: VitePlugin | undefined = undefined; const args = parseArgs(process.argv.slice(3)); - connectToStudio = process.env.ASTRO_INTERNAL_TEST_REMOTE || args['remote']; + connectToRemote = process.env.ASTRO_INTERNAL_TEST_REMOTE || args['remote']; - if (connectToStudio) { - appToken = await getManagedAppTokenOrExit(); + if (connectToRemote) { + appToken = await getManagedRemoteToken(); dbPlugin = vitePluginDb({ - connectToStudio, + connectToStudio: connectToRemote, appToken: appToken.token, tables, root: config.root, @@ -116,7 +116,7 @@ function astroDBIntegration(): AstroIntegration { configFileDependencies = dependencies; const localDbUrl = new URL(DB_PATH, config.root); - if (!connectToStudio && !existsSync(localDbUrl)) { + if (!connectToRemote && !existsSync(localDbUrl)) { await mkdir(dirname(fileURLToPath(localDbUrl)), { recursive: true }); await writeFile(localDbUrl, ''); } @@ -143,9 +143,9 @@ function astroDBIntegration(): AstroIntegration { // Wait for dev server log before showing "connected". setTimeout(() => { logger.info( - connectToStudio ? 'Connected to remote database.' : 'New local database created.', + connectToRemote ? 'Connected to remote database.' : 'New local database created.', ); - if (connectToStudio) return; + if (connectToRemote) return; const localSeedPaths = SEED_DEV_FILE_NAME.map( (name) => new URL(name, getDbDirectoryUrl(root)), @@ -160,7 +160,7 @@ function astroDBIntegration(): AstroIntegration { }, 'astro:build:start': async ({ logger }) => { if ( - !connectToStudio && + !connectToRemote && !databaseFileEnvDefined() && (output === 'server' || output === 'hybrid') ) { @@ -170,7 +170,7 @@ function astroDBIntegration(): AstroIntegration { throw new AstroDbError(message, hint); } - logger.info('database: ' + (connectToStudio ? yellow('remote') : blue('local database.'))); + logger.info('database: ' + (connectToRemote ? yellow('remote') : blue('local database.'))); }, 'astro:build:setup': async ({ vite }) => { tempViteServer = await getTempViteServer({ viteConfig: vite }); @@ -178,7 +178,7 @@ function astroDBIntegration(): AstroIntegration { await executeSeedFile({ fileUrl, viteServer: tempViteServer! }); }; }, - 'astro:build:done': async ({}) => { + 'astro:build:done': async ({ }) => { await appToken?.destroy(); await tempViteServer?.close(); }, diff --git a/packages/db/src/core/utils.ts b/packages/db/src/core/utils.ts index cf3e375354..53732c5db3 100644 --- a/packages/db/src/core/utils.ts +++ b/packages/db/src/core/utils.ts @@ -1,4 +1,4 @@ -import { getAstroStudioEnv } from '@astrojs/studio'; +import { getAstroStudioEnv, getManagedAppTokenOrExit, type ManagedAppToken } from '@astrojs/studio'; import type { AstroConfig, AstroIntegration } from 'astro'; import { loadEnv } from 'vite'; import './types.js'; @@ -37,6 +37,22 @@ export function getRemoteDatabaseInfo(): RemoteDatabaseInfo { }; } +export function getManagedRemoteToken(token?: string, dbInfo?: RemoteDatabaseInfo): Promise { + dbInfo ??= getRemoteDatabaseInfo(); + + if (dbInfo.type === 'studio') { + return getManagedAppTokenOrExit(token) + } + + const astroEnv = getAstroEnv(); + + return Promise.resolve({ + token: token ?? astroEnv.ASTRO_DB_APP_TOKEN, + renew: () => Promise.resolve(), + destroy: () => Promise.resolve(), + }); +} + export function getDbDirectoryUrl(root: URL | string) { return new URL('db/', root); } diff --git a/packages/db/test/basics.test.js b/packages/db/test/basics.test.js index c85880bef6..8622740eac 100644 --- a/packages/db/test/basics.test.js +++ b/packages/db/test/basics.test.js @@ -3,7 +3,7 @@ import { after, before, describe, it } from 'node:test'; import { load as cheerioLoad } from 'cheerio'; import testAdapter from '../../astro/test/test-adapter.js'; import { loadFixture } from '../../astro/test/test-utils.js'; -import { setupRemoteDbServer } from './test-utils.js'; +import { clearEnvironment, setupRemoteDbServer } from './test-utils.js'; describe('astro:db', () => { let fixture; @@ -19,6 +19,7 @@ describe('astro:db', () => { let devServer; before(async () => { + clearEnvironment(); devServer = await fixture.startDevServer(); }); @@ -98,6 +99,7 @@ describe('astro:db', () => { let remoteDbServer; before(async () => { + clearEnvironment(); remoteDbServer = await setupRemoteDbServer(fixture.config); devServer = await fixture.startDevServer(); }); @@ -178,6 +180,7 @@ describe('astro:db', () => { let remoteDbServer; before(async () => { + clearEnvironment(); process.env.ASTRO_STUDIO_APP_TOKEN = 'some token'; remoteDbServer = await setupRemoteDbServer(fixture.config); await fixture.build(); diff --git a/packages/db/test/static-remote.test.js b/packages/db/test/static-remote.test.js index ddfe735e55..bbe71539c0 100644 --- a/packages/db/test/static-remote.test.js +++ b/packages/db/test/static-remote.test.js @@ -2,7 +2,7 @@ import assert from 'node:assert/strict'; import { after, before, describe, it } from 'node:test'; import { load as cheerioLoad } from 'cheerio'; import { loadFixture } from '../../astro/test/test-utils.js'; -import { setupRemoteDbServer } from './test-utils.js'; +import { clearEnvironment, setupRemoteDbServer } from './test-utils.js'; describe('astro:db', () => { let fixture; @@ -44,6 +44,7 @@ describe('astro:db', () => { let remoteDbServer; before(async () => { + clearEnvironment(); process.env.ASTRO_DB_REMOTE_URL = `memory:`; await fixture.build(); }); diff --git a/packages/db/test/test-utils.js b/packages/db/test/test-utils.js index 3ad5d3c142..bf66cdb1ce 100644 --- a/packages/db/test/test-utils.js +++ b/packages/db/test/test-utils.js @@ -64,6 +64,18 @@ export async function setupRemoteDbServer(astroConfig) { }; } +/** + * Clears the environment variables related to Astro DB and Astro Studio. + */ +export function clearEnvironment() { + const keys = Array.from(Object.keys(process.env)); + for (const key of keys) { + if (key.startsWith('ASTRO_DB_') || key.startsWith('ASTRO_STUDIO_')) { + delete process.env[key]; + } + } +} + function createRemoteDbServer() { const dbClient = createClient({ url: ':memory:', diff --git a/packages/db/test/unit/remote-info.test.js b/packages/db/test/unit/remote-info.test.js new file mode 100644 index 0000000000..909feddd7c --- /dev/null +++ b/packages/db/test/unit/remote-info.test.js @@ -0,0 +1,117 @@ +import test, { after, beforeEach, describe } from "node:test"; +import assert from "node:assert"; +import { getRemoteDatabaseInfo, getManagedRemoteToken } from '../../dist/core/utils.js'; +import { clearEnvironment } from "../test-utils.js"; + +describe('RemoteDatabaseInfo', () => { + beforeEach(() => { + clearEnvironment(); + }); + + test('default remote info', () => { + const dbInfo = getRemoteDatabaseInfo(); + + assert.deepEqual(dbInfo, { + type: 'studio', + url: 'https://db.services.astro.build', + }); + }); + + test('configured Astro Studio remote', () => { + process.env.ASTRO_STUDIO_REMOTE_DB_URL = 'https://studio.astro.build'; + const dbInfo = getRemoteDatabaseInfo(); + + assert.deepEqual(dbInfo, { + type: 'studio', + url: 'https://studio.astro.build', + }); + }); + + test('configured libSQL remote', () => { + process.env.ASTRO_DB_REMOTE_URL = 'libsql://libsql.self.hosted'; + const dbInfo = getRemoteDatabaseInfo(); + + assert.deepEqual(dbInfo, { + type: 'libsql', + url: 'libsql://libsql.self.hosted', + }); + }); + + test('configured both libSQL and Studio remote', () => { + process.env.ASTRO_DB_REMOTE_URL = 'libsql://libsql.self.hosted'; + process.env.ASTRO_STUDIO_REMOTE_DB_URL = 'https://studio.astro.build'; + const dbInfo = getRemoteDatabaseInfo(); + + assert.deepEqual(dbInfo, { + type: 'studio', + url: 'https://studio.astro.build', + }); + }); +}); + +describe('RemoteManagedToken', () => { + // Avoid conflicts with other tests + beforeEach(() => { + clearEnvironment(); + process.env.ASTRO_STUDIO_APP_TOKEN = 'studio token' + process.env.ASTRO_DB_APP_TOKEN = 'db token' + }); + after(() => { clearEnvironment(); }); + + test('given token for default remote', async () => { + const { token } = await getManagedRemoteToken('given token'); + assert.equal(token, 'given token'); + }); + + test('token for default remote', async () => { + const { token } = await getManagedRemoteToken(); + + assert.equal(token, 'studio token'); + }); + + test('given token for configured Astro Studio remote', async () => { + process.env.ASTRO_STUDIO_REMOTE_DB_URL = 'https://studio.astro.build'; + const { token } = await getManagedRemoteToken('given token'); + assert.equal(token, 'given token'); + }); + + test('token for configured Astro Studio remote', async () => { + process.env.ASTRO_STUDIO_REMOTE_DB_URL = 'https://studio.astro.build'; + const { token } = await getManagedRemoteToken(); + + assert.equal(token, 'studio token'); + }); + + test('given token for configured libSQL remote', async () => { + process.env.ASTRO_DB_REMOTE_URL = 'libsql://libsql.self.hosted'; + const { token } = await getManagedRemoteToken('given token'); + assert.equal(token, 'given token'); + }); + + test('token for configured libSQL remote', async () => { + process.env.ASTRO_DB_REMOTE_URL = 'libsql://libsql.self.hosted'; + const { token } = await getManagedRemoteToken(); + + assert.equal(token, 'db token'); + }); + + test('token for given Astro Studio remote', async () => { + process.env.ASTRO_DB_REMOTE_URL = 'libsql://libsql.self.hosted'; + const { token } = await getManagedRemoteToken(undefined, { + type: 'studio', + url: 'https://studio.astro.build', + }); + + assert.equal(token, 'studio token'); + }); + + test('token for given libSQL remote', async () => { + process.env.ASTRO_STUDIO_REMOTE_URL = 'libsql://libsql.self.hosted'; + const { token } = await getManagedRemoteToken(undefined, { + type: 'libsql', + url: 'libsql://libsql.self.hosted', + }); + + assert.equal(token, 'db token'); + }); +}); From 5705200398b07d99badafb977a8401c4129a536c Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 3 Sep 2024 12:54:20 +0000 Subject: [PATCH 2/3] [ci] format --- packages/db/src/core/cli/commands/push/index.ts | 7 ++++++- .../db/src/core/cli/commands/verify/index.ts | 2 +- packages/db/src/core/integration/index.ts | 2 +- packages/db/src/core/utils.ts | 9 ++++++--- packages/db/test/unit/remote-info.test.js | 16 +++++++++------- 5 files changed, 23 insertions(+), 13 deletions(-) diff --git a/packages/db/src/core/cli/commands/push/index.ts b/packages/db/src/core/cli/commands/push/index.ts index 0b7b32bf94..590d4f06ee 100644 --- a/packages/db/src/core/cli/commands/push/index.ts +++ b/packages/db/src/core/cli/commands/push/index.ts @@ -6,7 +6,12 @@ import { createRemoteDatabaseClient } from '../../../../runtime/index.js'; import { safeFetch } from '../../../../runtime/utils.js'; import { MIGRATION_VERSION } from '../../../consts.js'; import type { DBConfig, DBSnapshot } from '../../../types.js'; -import { type RemoteDatabaseInfo, type Result, getManagedRemoteToken, getRemoteDatabaseInfo } from '../../../utils.js'; +import { + type RemoteDatabaseInfo, + type Result, + getManagedRemoteToken, + getRemoteDatabaseInfo, +} from '../../../utils.js'; import { createCurrentSnapshot, createEmptySnapshot, diff --git a/packages/db/src/core/cli/commands/verify/index.ts b/packages/db/src/core/cli/commands/verify/index.ts index d535fba1fe..35f489a803 100644 --- a/packages/db/src/core/cli/commands/verify/index.ts +++ b/packages/db/src/core/cli/commands/verify/index.ts @@ -1,6 +1,7 @@ import type { AstroConfig } from 'astro'; import type { Arguments } from 'yargs-parser'; import type { DBConfig } from '../../../types.js'; +import { getManagedRemoteToken, getRemoteDatabaseInfo } from '../../../utils.js'; import { createCurrentSnapshot, createEmptySnapshot, @@ -8,7 +9,6 @@ import { getMigrationQueries, getProductionCurrentSnapshot, } from '../../migration-queries.js'; -import { getManagedRemoteToken, getRemoteDatabaseInfo } from '../../../utils.js'; export async function cmd({ dbConfig, diff --git a/packages/db/src/core/integration/index.ts b/packages/db/src/core/integration/index.ts index 1b1eeec8e1..2a5824c35b 100644 --- a/packages/db/src/core/integration/index.ts +++ b/packages/db/src/core/integration/index.ts @@ -178,7 +178,7 @@ function astroDBIntegration(): AstroIntegration { await executeSeedFile({ fileUrl, viteServer: tempViteServer! }); }; }, - 'astro:build:done': async ({ }) => { + 'astro:build:done': async ({}) => { await appToken?.destroy(); await tempViteServer?.close(); }, diff --git a/packages/db/src/core/utils.ts b/packages/db/src/core/utils.ts index 53732c5db3..b246997e24 100644 --- a/packages/db/src/core/utils.ts +++ b/packages/db/src/core/utils.ts @@ -1,4 +1,4 @@ -import { getAstroStudioEnv, getManagedAppTokenOrExit, type ManagedAppToken } from '@astrojs/studio'; +import { type ManagedAppToken, getAstroStudioEnv, getManagedAppTokenOrExit } from '@astrojs/studio'; import type { AstroConfig, AstroIntegration } from 'astro'; import { loadEnv } from 'vite'; import './types.js'; @@ -37,11 +37,14 @@ export function getRemoteDatabaseInfo(): RemoteDatabaseInfo { }; } -export function getManagedRemoteToken(token?: string, dbInfo?: RemoteDatabaseInfo): Promise { +export function getManagedRemoteToken( + token?: string, + dbInfo?: RemoteDatabaseInfo, +): Promise { dbInfo ??= getRemoteDatabaseInfo(); if (dbInfo.type === 'studio') { - return getManagedAppTokenOrExit(token) + return getManagedAppTokenOrExit(token); } const astroEnv = getAstroEnv(); diff --git a/packages/db/test/unit/remote-info.test.js b/packages/db/test/unit/remote-info.test.js index 909feddd7c..2c58f28b7b 100644 --- a/packages/db/test/unit/remote-info.test.js +++ b/packages/db/test/unit/remote-info.test.js @@ -1,7 +1,7 @@ -import test, { after, beforeEach, describe } from "node:test"; -import assert from "node:assert"; -import { getRemoteDatabaseInfo, getManagedRemoteToken } from '../../dist/core/utils.js'; -import { clearEnvironment } from "../test-utils.js"; +import assert from 'node:assert'; +import test, { after, beforeEach, describe } from 'node:test'; +import { getManagedRemoteToken, getRemoteDatabaseInfo } from '../../dist/core/utils.js'; +import { clearEnvironment } from '../test-utils.js'; describe('RemoteDatabaseInfo', () => { beforeEach(() => { @@ -53,10 +53,12 @@ describe('RemoteManagedToken', () => { // Avoid conflicts with other tests beforeEach(() => { clearEnvironment(); - process.env.ASTRO_STUDIO_APP_TOKEN = 'studio token' - process.env.ASTRO_DB_APP_TOKEN = 'db token' + process.env.ASTRO_STUDIO_APP_TOKEN = 'studio token'; + process.env.ASTRO_DB_APP_TOKEN = 'db token'; + }); + after(() => { + clearEnvironment(); }); - after(() => { clearEnvironment(); }); test('given token for default remote', async () => { const { token } = await getManagedRemoteToken('given token'); From d63bc50d9940c1107e0fee7687e5c332549a0eff Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 3 Sep 2024 14:25:45 +0100 Subject: [PATCH 3/3] fix: keep data store outside of node_modules during dev (#11902) * fix: don't keep data store in node_modules during dev * Lint * Fix test * Wait for data store * Use helper for data store file * Fix data store file helper * Lint * Handle case where Vite already knows about save --- .changeset/forty-spies-train.md | 5 +++ packages/astro/src/content/content-layer.ts | 11 ++++- packages/astro/src/content/data-store.ts | 3 ++ .../vite-plugin-content-virtual-mod.ts | 41 +++++++++++-------- packages/astro/src/core/build/index.ts | 7 ++-- packages/astro/src/core/dev/dev.ts | 5 +-- packages/astro/src/core/sync/index.ts | 16 ++++---- packages/astro/test/content-layer.test.js | 10 +++-- packages/astro/test/test-utils.js | 22 ++++++++++ 9 files changed, 84 insertions(+), 36 deletions(-) create mode 100644 .changeset/forty-spies-train.md diff --git a/.changeset/forty-spies-train.md b/.changeset/forty-spies-train.md new file mode 100644 index 0000000000..5df78b648f --- /dev/null +++ b/.changeset/forty-spies-train.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes case where content layer did not update during clean dev builds on Linux and Windows diff --git a/packages/astro/src/content/content-layer.ts b/packages/astro/src/content/content-layer.ts index e14eed43b7..c07d5dd55f 100644 --- a/packages/astro/src/content/content-layer.ts +++ b/packages/astro/src/content/content-layer.ts @@ -223,7 +223,7 @@ export class ContentLayer { if (!existsSync(this.#settings.config.cacheDir)) { await fs.mkdir(this.#settings.config.cacheDir, { recursive: true }); } - const cacheFile = new URL(DATA_STORE_FILE, this.#settings.config.cacheDir); + const cacheFile = getDataStoreFile(this.#settings); await this.#store.writeToDisk(cacheFile); if (!existsSync(this.#settings.dotAstroDir)) { await fs.mkdir(this.#settings.dotAstroDir, { recursive: true }); @@ -283,6 +283,15 @@ export async function simpleLoader( context.store.set({ id: raw.id, data: item }); } } +/** + * Get the path to the data store file. + * During development, this is in the `.astro` directory so that the Vite watcher can see it. + * In production, it's in the cache directory so that it's preserved between builds. + */ +export function getDataStoreFile(settings: AstroSettings, isDev?: boolean) { + isDev ??= process?.env.NODE_ENV === 'development'; + return new URL(DATA_STORE_FILE, isDev ? settings.dotAstroDir : settings.config.cacheDir); +} function contentLayerSingleton() { let instance: ContentLayer | null = null; diff --git a/packages/astro/src/content/data-store.ts b/packages/astro/src/content/data-store.ts index fbf31d0f13..21d59363c0 100644 --- a/packages/astro/src/content/data-store.ts +++ b/packages/astro/src/content/data-store.ts @@ -91,6 +91,9 @@ export class DataStore { try { // @ts-expect-error - this is a virtual module const data = await import('astro:data-layer-content'); + if (data.default instanceof Map) { + return DataStore.fromMap(data.default); + } const map = devalue.unflatten(data.default); return DataStore.fromMap(map); } catch {} diff --git a/packages/astro/src/content/vite-plugin-content-virtual-mod.ts b/packages/astro/src/content/vite-plugin-content-virtual-mod.ts index 64e5d98ee4..6ffa37a4e1 100644 --- a/packages/astro/src/content/vite-plugin-content-virtual-mod.ts +++ b/packages/astro/src/content/vite-plugin-content-virtual-mod.ts @@ -20,7 +20,6 @@ import { CONTENT_FLAG, CONTENT_RENDER_FLAG, DATA_FLAG, - DATA_STORE_FILE, DATA_STORE_VIRTUAL_ID, MODULES_IMPORTS_FILE, MODULES_MJS_ID, @@ -29,6 +28,7 @@ import { RESOLVED_VIRTUAL_MODULE_ID, VIRTUAL_MODULE_ID, } from './consts.js'; +import { getDataStoreFile } from './content-layer.js'; import { type ContentLookupMap, getContentEntryIdAndSlug, @@ -54,12 +54,13 @@ export function astroContentVirtualModPlugin({ }: AstroContentVirtualModPluginParams): Plugin { let IS_DEV = false; const IS_SERVER = isServerLikeOutput(settings.config); - const dataStoreFile = new URL(DATA_STORE_FILE, settings.config.cacheDir); + let dataStoreFile: URL; return { name: 'astro-content-virtual-mod-plugin', enforce: 'pre', configResolved(config) { IS_DEV = config.mode === 'development'; + dataStoreFile = getDataStoreFile(settings, IS_DEV); }, async resolveId(id) { if (id === VIRTUAL_MODULE_ID) { @@ -180,25 +181,31 @@ export function astroContentVirtualModPlugin({ configureServer(server) { const dataStorePath = fileURLToPath(dataStoreFile); - // Watch for changes to the data store file - if (Array.isArray(server.watcher.options.ignored)) { - // The data store file is in node_modules, so is ignored by default, - // so we need to un-ignore it. - server.watcher.options.ignored.push(`!${dataStorePath}`); - } + server.watcher.add(dataStorePath); + function invalidateDataStore() { + const module = server.moduleGraph.getModuleById(RESOLVED_DATA_STORE_VIRTUAL_ID); + if (module) { + server.moduleGraph.invalidateModule(module); + } + server.ws.send({ + type: 'full-reload', + path: '*', + }); + } + + // If the datastore file changes, invalidate the virtual module + + server.watcher.on('add', (addedPath) => { + if (addedPath === dataStorePath) { + invalidateDataStore(); + } + }); + server.watcher.on('change', (changedPath) => { - // If the datastore file changes, invalidate the virtual module if (changedPath === dataStorePath) { - const module = server.moduleGraph.getModuleById(RESOLVED_DATA_STORE_VIRTUAL_ID); - if (module) { - server.moduleGraph.invalidateModule(module); - } - server.ws.send({ - type: 'full-reload', - path: '*', - }); + invalidateDataStore(); } }); }, diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index 72df05b891..70d2401284 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -61,6 +61,9 @@ export default async function build( const logger = createNodeLogger(inlineConfig); const { userConfig, astroConfig } = await resolveConfig(inlineConfig, 'build'); telemetry.record(eventCliSession('build', userConfig)); + + const settings = await createSettings(astroConfig, fileURLToPath(astroConfig.root)); + if (inlineConfig.force) { if (astroConfig.experimental.contentCollectionCache) { const contentCacheDir = new URL('./content/', astroConfig.cacheDir); @@ -70,11 +73,9 @@ export default async function build( logger.warn('content', 'content cache cleared (force)'); } } - await clearContentLayerCache({ astroConfig, logger, fs }); + await clearContentLayerCache({ settings, logger, fs }); } - const settings = await createSettings(astroConfig, fileURLToPath(astroConfig.root)); - const builder = new AstroBuilder(settings, { ...options, logger, diff --git a/packages/astro/src/core/dev/dev.ts b/packages/astro/src/core/dev/dev.ts index 12b6d3b55a..d99d00d49d 100644 --- a/packages/astro/src/core/dev/dev.ts +++ b/packages/astro/src/core/dev/dev.ts @@ -6,8 +6,7 @@ import { green } from 'kleur/colors'; import { gt, major, minor, patch } from 'semver'; import type * as vite from 'vite'; import type { AstroInlineConfig } from '../../@types/astro.js'; -import { DATA_STORE_FILE } from '../../content/consts.js'; -import { globalContentLayer } from '../../content/content-layer.js'; +import { getDataStoreFile, globalContentLayer } from '../../content/content-layer.js'; import { attachContentServerListeners } from '../../content/index.js'; import { MutableDataStore } from '../../content/mutable-data-store.js'; import { globalContentConfigObserver } from '../../content/utils.js'; @@ -108,7 +107,7 @@ export default async function dev(inlineConfig: AstroInlineConfig): Promise { if (force) { - await clearContentLayerCache({ astroConfig: settings.config, logger, fs }); + await clearContentLayerCache({ settings, logger, fs }); } const timerStart = performance.now(); @@ -106,7 +106,7 @@ export async function syncInternal({ settings.timer.start('Sync content layer'); let store: MutableDataStore | undefined; try { - const dataStoreFile = new URL(DATA_STORE_FILE, settings.config.cacheDir); + const dataStoreFile = getDataStoreFile(settings); if (existsSync(dataStoreFile)) { store = await MutableDataStore.fromFile(dataStoreFile); } diff --git a/packages/astro/test/content-layer.test.js b/packages/astro/test/content-layer.test.js index 0590e7e590..c853a3be5b 100644 --- a/packages/astro/test/content-layer.test.js +++ b/packages/astro/test/content-layer.test.js @@ -196,7 +196,11 @@ describe('Content Layer', () => { let devServer; let json; before(async () => { - devServer = await fixture.startDevServer(); + devServer = await fixture.startDevServer({ force: true }); + // Vite may not have noticed the saved data store yet. Wait a little just in case. + await fixture.onNextDataStoreChange(1000).catch(() => { + // Ignore timeout, because it may have saved before we get here. + }) const rawJsonResponse = await fixture.fetch('/collections.json'); const rawJson = await rawJsonResponse.text(); json = devalue.parse(rawJson); @@ -286,9 +290,7 @@ describe('Content Layer', () => { return JSON.stringify(data, null, 2); }); - // Writes are debounced to 500ms - await new Promise((r) => setTimeout(r, 700)); - + await fixture.onNextDataStoreChange(); const updatedJsonResponse = await fixture.fetch('/collections.json'); const updated = devalue.parse(await updatedJsonResponse.text()); assert.ok(updated.fileLoader[0].data.temperament.includes('Bouncy')); diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js index 95edeebd26..68fab03b04 100644 --- a/packages/astro/test/test-utils.js +++ b/packages/astro/test/test-utils.js @@ -48,6 +48,7 @@ process.env.ASTRO_TELEMETRY_DISABLED = true; * @property {() => Promise} loadTestAdapterApp * @property {() => Promise<(req: NodeRequest, res: NodeResponse) => void>} loadNodeAdapterHandler * @property {() => Promise} onNextChange + * @property {(timeout?: number) => Promise} onNextDataStoreChange * @property {typeof check} check * @property {typeof sync} sync * @property {AstroConfig} config @@ -183,6 +184,27 @@ export async function loadFixture(inlineConfig) { config.server.port = devServer.address.port; // update port return devServer; }, + onNextDataStoreChange: (timeout = 5000) => { + if (!devServer) { + return Promise.reject(new Error('No dev server running')); + } + + const dataStoreFile = path.join(root, '.astro', 'data-store.json'); + + return new Promise((resolve, reject) => { + const changeHandler = (fileName) => { + if (fileName === dataStoreFile) { + devServer.watcher.removeListener('change', changeHandler); + resolve(); + } + }; + devServer.watcher.on('change', changeHandler); + setTimeout(() => { + devServer.watcher.removeListener('change', changeHandler); + reject(new Error('Data store did not update within timeout')); + }, timeout); + }); + }, config, resolveUrl, fetch: async (url, init) => {