Skip to content

Commit c3cf338

Browse files
anserwaseemanserwaseem
committed
refactor(env): simplify env sync module
- Remove unused assetFileNames param from readProjectEnvFiles - Consolidate config DTO builders via shared helpers - Extract missing-env-file push warnings into envSync
1 parent 470b0c0 commit c3cf338

5 files changed

Lines changed: 90 additions & 59 deletions

File tree

‎src/commands/pull.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ export async function pullCommand(options: PullOptions = {}): Promise<void> {
268268
localFiles,
269269
manifestExisting,
270270
enabledByProp,
271-
localEnv: await readProjectEnvFiles(projectRoot, localFiles.assetFiles ?? []),
271+
localEnv: await readProjectEnvFiles(projectRoot),
272272
});
273273

274274
if (plan.allArtifactsMatch && plan.manifestMatch) {

‎src/commands/push.ts‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,9 @@ import { computePushPlan, type PushSummary, type PushCounts } from '../core/sync
2424
import { buildAndWriteManifest } from '../core/manifest.js';
2525
import {
2626
buildEnvPushDiff,
27-
cloudHasNonAssetConfig,
28-
cloudHasSecrets,
2927
mergeConfigDtoForPush,
3028
readProjectEnvFiles,
29+
warnIfMissingEnvFilesForPush,
3130
} from '../core/envSync.js';
3231
import { ui } from '../core/ui.js';
3332

@@ -343,7 +342,7 @@ export async function pushCommand(options: PushOptions = {}): Promise<void> {
343342
});
344343
bundle = plan.bundle;
345344

346-
const localEnv = await readProjectEnvFiles(root, data.assetFiles ?? []);
345+
const localEnv = await readProjectEnvFiles(root);
347346
const assetFileNames = data.assetFiles ?? [];
348347
const envPushDiff = buildEnvPushDiff(
349348
localEnv,
@@ -352,16 +351,12 @@ export async function pushCommand(options: PushOptions = {}): Promise<void> {
352351
);
353352
const { configChanged: envConfigChanged, secretsChanged: envSecretsChanged } = envPushDiff;
354353

355-
if (!localEnv.envConfigPresent && cloudHasNonAssetConfig(cloudApp.config, assetFileNames)) {
356-
ui.warn(
357-
'.env.config is missing locally. Run `ensemble pull` to restore env vars from cloud. Config env push skipped.'
358-
);
359-
}
360-
if (!localEnv.envSecretsPresent && cloudHasSecrets(cloudApp.secrets)) {
361-
ui.warn(
362-
'.env.secrets is missing locally. Run `ensemble pull` to restore secrets from cloud. Secrets env push skipped.'
363-
);
364-
}
354+
warnIfMissingEnvFilesForPush(
355+
localEnv,
356+
{ config: cloudApp.config, secrets: cloudApp.secrets },
357+
assetFileNames,
358+
(message) => ui.warn(message)
359+
);
365360
const localConfigDto = envPushDiff.local.config;
366361
const localSecretsDto = envPushDiff.local.secrets;
367362
const pushConfigDto =

‎src/commands/release.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ export async function releaseCreateCommand(options: ReleaseCreateOptions = {}):
110110
const appName = (appConfig.name as string | undefined) ?? 'App';
111111
const appHome = appConfig.appHome as string | undefined;
112112
const localFiles = await collectAppFiles(root);
113-
const localEnv = await readProjectEnvFiles(root, localFiles.assetFiles ?? []);
113+
const localEnv = await readProjectEnvFiles(root);
114114
const localConfig = buildConfigDtoForReleaseSnapshot(localEnv.envConfig);
115115
const localApp = buildDocumentsFromParsed(localFiles, appId, appName, appHome, undefined);
116116
const snapshot: CloudApp = {

‎src/core/envSync.ts‎

Lines changed: 56 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -46,53 +46,62 @@ export function collectAssetEnvKeys(assetFileNames: string[] = []): Set<string>
4646
return new Set(['assets', ...assetFileNames.map(deriveAssetEnvKey)]);
4747
}
4848

49-
export function stripAssetKeysFromConfigDto(
49+
function filterConfigEnvVariables(
5050
config: ConfigDTO | undefined,
51-
assetFileNames: string[] = []
52-
): ConfigDTO | undefined {
53-
const assetKeys = collectAssetEnvKeys(assetFileNames);
54-
const envVariables: Record<string, string> = {};
51+
includeKey: (key: string) => boolean
52+
): Record<string, string> {
5553
const vars = config?.envVariables;
56-
if (!vars || typeof vars !== 'object') return undefined;
54+
if (!vars || typeof vars !== 'object') return {};
55+
const envVariables: Record<string, string> = {};
5756
for (const [key, value] of Object.entries(vars)) {
58-
if (assetKeys.has(key) || value === undefined || value === null) continue;
57+
if (!includeKey(key) || value === undefined || value === null) continue;
5958
envVariables[key] = String(value);
6059
}
61-
return Object.keys(envVariables).length > 0 ? { envVariables } : undefined;
60+
return envVariables;
6261
}
6362

64-
export function buildConfigDtoFromEnvConfigFile(
63+
function buildConfigDtoFromEntries(
6564
entries: EnvEntry[],
66-
assetFileNames: string[] = []
65+
options?: { excludeKeys?: Set<string> }
6766
): ConfigDTO | undefined {
68-
const assetKeys = collectAssetEnvKeys(assetFileNames);
6967
const envVariables: Record<string, string> = {};
7068
for (const entry of entries) {
71-
if (assetKeys.has(entry.key)) continue;
69+
if (options?.excludeKeys?.has(entry.key)) continue;
7270
envVariables[entry.key] = entry.value;
7371
}
7472
return Object.keys(envVariables).length > 0 ? { envVariables } : undefined;
7573
}
7674

75+
export function stripAssetKeysFromConfigDto(
76+
config: ConfigDTO | undefined,
77+
assetFileNames: string[] = []
78+
): ConfigDTO | undefined {
79+
const assetKeys = collectAssetEnvKeys(assetFileNames);
80+
const envVariables = filterConfigEnvVariables(config, (key) => !assetKeys.has(key));
81+
return Object.keys(envVariables).length > 0 ? { envVariables } : undefined;
82+
}
83+
84+
export function buildConfigDtoFromEnvConfigFile(
85+
entries: EnvEntry[],
86+
assetFileNames: string[] = []
87+
): ConfigDTO | undefined {
88+
return buildConfigDtoFromEntries(entries, {
89+
excludeKeys: collectAssetEnvKeys(assetFileNames),
90+
});
91+
}
92+
7793
export function mergeConfigDtoForPush(
7894
localNonAssetConfig: ConfigDTO | undefined,
7995
cloudConfig: ConfigDTO | undefined,
8096
assetFileNames: string[] = []
8197
): ConfigDTO {
8298
const assetKeys = collectAssetEnvKeys(assetFileNames);
83-
const merged: Record<string, string> = {};
84-
85-
for (const [key, value] of Object.entries(cloudConfig?.envVariables ?? {})) {
86-
if (assetKeys.has(key) && value !== undefined && value !== null) {
87-
merged[key] = String(value);
88-
}
89-
}
90-
91-
for (const [key, value] of Object.entries(localNonAssetConfig?.envVariables ?? {})) {
92-
merged[key] = String(value);
93-
}
94-
95-
return { envVariables: merged };
99+
return {
100+
envVariables: {
101+
...filterConfigEnvVariables(cloudConfig, (key) => assetKeys.has(key)),
102+
...(localNonAssetConfig?.envVariables ?? {}),
103+
},
104+
};
96105
}
97106

98107
export function buildSecretsDtoFromEnvSecretsFile(entries: EnvEntry[]): SecretDTO | undefined {
@@ -104,11 +113,25 @@ export function buildSecretsDtoFromEnvSecretsFile(entries: EnvEntry[]): SecretDT
104113
return { secrets };
105114
}
106115

107-
export async function readProjectEnvFiles(
108-
projectRoot: string,
109-
assetFileNames: string[] = []
110-
): Promise<LocalEnvFiles> {
111-
void assetFileNames;
116+
export function warnIfMissingEnvFilesForPush(
117+
localEnv: LocalEnvFiles,
118+
cloudEnv: CloudEnvState,
119+
assetFileNames: string[] = [],
120+
warn: (message: string) => void
121+
): void {
122+
if (!localEnv.envConfigPresent && cloudHasNonAssetConfig(cloudEnv.config, assetFileNames)) {
123+
warn(
124+
'.env.config is missing locally. Run `ensemble pull` to restore env vars from cloud. Config env push skipped.'
125+
);
126+
}
127+
if (!localEnv.envSecretsPresent && cloudHasSecrets(cloudEnv.secrets)) {
128+
warn(
129+
'.env.secrets is missing locally. Run `ensemble pull` to restore secrets from cloud. Secrets env push skipped.'
130+
);
131+
}
132+
}
133+
134+
export async function readProjectEnvFiles(projectRoot: string): Promise<LocalEnvFiles> {
112135
const [envConfigPresent, envSecretsPresent] = await Promise.all([
113136
envFileExists(projectRoot, '.env.config'),
114137
envFileExists(projectRoot, '.env.secrets'),
@@ -245,12 +268,7 @@ export function buildEnvPushDiff(
245268
}
246269

247270
export function buildConfigDtoForReleaseSnapshot(entries: EnvEntry[]): ConfigDTO | undefined {
248-
if (entries.length === 0) return undefined;
249-
const envVariables: Record<string, string> = {};
250-
for (const entry of entries) {
251-
envVariables[entry.key] = entry.value;
252-
}
253-
return { envVariables };
271+
return buildConfigDtoFromEntries(entries);
254272
}
255273

256274
/** restores `.env.config` from a release snapshot; secrets are never included in releases */
@@ -286,12 +304,8 @@ async function upsertCloudAssetConfigEntries(
286304
assetFileNames: string[] = []
287305
): Promise<void> {
288306
const assetKeys = collectAssetEnvKeys(assetFileNames);
289-
const vars = cloudConfig?.envVariables ?? {};
290-
const entries: EnvEntry[] = [];
291-
for (const [key, value] of Object.entries(vars)) {
292-
if (!assetKeys.has(key) || value === undefined || value === null) continue;
293-
entries.push({ key, value: String(value) });
294-
}
307+
const envVariables = filterConfigEnvVariables(cloudConfig, (key) => assetKeys.has(key));
308+
const entries = Object.entries(envVariables).map(([key, value]) => ({ key, value }));
295309
if (entries.length > 0) {
296310
await upsertEnvFile(projectRoot, '.env.config', entries);
297311
}

‎tests/core/envSync.test.ts‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
mergeConfigDtoForPush,
1818
readProjectEnvFiles,
1919
secretsDtoToEnvEntries,
20+
warnIfMissingEnvFilesForPush,
2021
} from '../../src/core/envSync.js';
2122
import type { ConfigDTO, SecretDTO } from '../../src/core/dto.js';
2223

@@ -88,7 +89,7 @@ describe('envSync', () => {
8889
);
8990
await fs.writeFile(path.join(tmpDir, '.env.secrets'), 'S1=local-secret\n', 'utf8');
9091

91-
const local = await readProjectEnvFiles(tmpDir, ['logo.png']);
92+
const local = await readProjectEnvFiles(tmpDir);
9293
expect(buildConfigDtoFromEnvConfigFile(local.envConfig, ['logo.png'])).toEqual({
9394
envVariables: { API_URL: 'https://local.example.com' },
9495
});
@@ -105,7 +106,7 @@ describe('envSync', () => {
105106
);
106107
await fs.writeFile(path.join(tmpDir, '.env.secrets'), 'S1=local-secret\n', 'utf8');
107108

108-
const local = await readProjectEnvFiles(tmpDir, []);
109+
const local = await readProjectEnvFiles(tmpDir);
109110
expect(
110111
envConfigEntriesMatchCloud(local.envConfig, {
111112
envVariables: { API_URL: 'https://cloud.example.com' },
@@ -293,6 +294,27 @@ describe('envSync', () => {
293294
).toBe(false);
294295
});
295296

297+
it('warns when env files are missing locally but cloud has values', () => {
298+
const warnings: string[] = [];
299+
warnIfMissingEnvFilesForPush(
300+
{
301+
envConfig: [],
302+
envSecrets: [],
303+
envConfigPresent: false,
304+
envSecretsPresent: false,
305+
},
306+
{
307+
config: { envVariables: { E1: 'EV1' } },
308+
secrets: { secrets: { S1: 'SK1' } },
309+
},
310+
[],
311+
(message) => warnings.push(message)
312+
);
313+
expect(warnings).toHaveLength(2);
314+
expect(warnings[0]).toContain('.env.config is missing');
315+
expect(warnings[1]).toContain('.env.secrets is missing');
316+
});
317+
296318
it('does not treat missing env files as empty local deletes on push', () => {
297319
const diff = buildEnvPushDiff(
298320
{

0 commit comments

Comments
 (0)