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

refactor: ♻️ Create a ConfigModule that exposes a ConfigService which loads the appropriate .env file #28

Merged
merged 8 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
11 changes: 7 additions & 4 deletions .env.sample
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
GATEWAY_HOST=localhost
GATEWAY_HOST=http://localhost
GATEWAY_PORT=3333

USER_HOST=localhost
USER_HOST=http://localhost
USER_PORT=15001

TASK_HOST=localhost
TASK_PORT=15002
TASK_HOST=http://localhost
TASK_PORT=15002

DATABASE_USER=test
DATABASE_PASSWORD=test
30 changes: 24 additions & 6 deletions apps/gateway/src/app/app.module.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,38 @@
import { IntrospectAndCompose } from '@apollo/gateway';
import { gatewayConfig, userAppConfig } from '@libs/config';
import { ApolloGatewayDriver, ApolloGatewayDriverConfig } from '@nestjs/apollo';
import { Module } from '@nestjs/common';
import { ConfigModule, ConfigService } from '@nestjs/config';
import { GraphQLModule } from '@nestjs/graphql';
import { subgraphsConfig } from '@graphql-federation-workspace/applications-config';

import { AppController } from './app.controller';
import { AppService } from './app.service';

@Module({
imports: [
GraphQLModule.forRoot<ApolloGatewayDriverConfig>({
ConfigModule.forRoot({
isGlobal: true,
load: [gatewayConfig, userAppConfig],
}),
GraphQLModule.forRootAsync<ApolloGatewayDriverConfig>({
imports: [ConfigModule],
inject: [ConfigService],
driver: ApolloGatewayDriver,
gateway: {
supergraphSdl: new IntrospectAndCompose({
subgraphs: subgraphsConfig,
}),
useFactory: (configService: ConfigService) => {
const userAppConfig = configService.get('userApp');
return {
driver: ApolloGatewayDriver,
gateway: {
supergraphSdl: new IntrospectAndCompose({
subgraphs: [
{
name: userAppConfig.name,
url: `${userAppConfig.host}:${userAppConfig.port}/graphql`,
zhumeisongsong marked this conversation as resolved.
Show resolved Hide resolved
},
],
}),
},
};
},
}),
],
Expand Down
10 changes: 7 additions & 3 deletions apps/gateway/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@
*/

import { Logger } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { NestFactory } from '@nestjs/core';
import { gatewayConfig } from '@graphql-federation-workspace/applications-config';

import { AppModule } from './app/app.module';

async function bootstrap() {
const app = await NestFactory.create(AppModule);
const port = gatewayConfig.port;

const configService = app.get(ConfigService);
const config = configService.get('gateway');
const host = config.host;
const port = config.port;
zhumeisongsong marked this conversation as resolved.
Show resolved Hide resolved

await app.listen(port);
Logger.log(`🚀 Application is running on: ${gatewayConfig.host}:${port}`);
Logger.log(`🚀 Application is running on: ${host}:${port}`);
}

bootstrap();
10 changes: 9 additions & 1 deletion apps/users-application/src/app/app.module.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import { userAppConfig } from '@libs/config';
import { Module } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config';

import { AppController } from './app.controller';
import { AppService } from './app.service';
import { UsersModule } from '../users/users.module';

@Module({
imports: [UsersModule],
imports: [
ConfigModule.forRoot({
isGlobal: true,
load: [userAppConfig],
}),
UsersModule,
],
controllers: [AppController],
providers: [AppService],
})
Expand Down
10 changes: 7 additions & 3 deletions apps/users-application/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@
*/

import { Logger } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { NestFactory } from '@nestjs/core';
import { userSubGraph } from '@graphql-federation-workspace/applications-config';

import { AppModule } from './app/app.module';

async function bootstrap() {
const app = await NestFactory.create(AppModule);
const port = userSubGraph.port;

const configService = app.get(ConfigService);
const config = configService.get('userApp');
const host = config.host;
const port = config.port;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and type safety for configuration.

The current configuration retrieval lacks error handling and type safety. Consider these improvements:

  const configService = app.get(ConfigService);
- const config = configService.get('userApp');
- const host = config.host;
- const port = config.port;
+ const config = configService.get<{host: string; port: number}>('userApp');
+ if (!config) {
+   throw new Error('User application configuration is missing');
+ }
+ const { host, port } = config;
+ if (!host || !port) {
+   throw new Error('Host and port must be defined in configuration');
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const configService = app.get(ConfigService);
const config = configService.get('userApp');
const host = config.host;
const port = config.port;
const configService = app.get(ConfigService);
const config = configService.get<{host: string; port: number}>('userApp');
if (!config) {
throw new Error('User application configuration is missing');
}
const { host, port } = config;
if (!host || !port) {
throw new Error('Host and port must be defined in configuration');
}

💡 Codebase verification

Missing configuration keys in .env files.

  • The following configuration keys are used in main.ts but not found in any .env files:
    • userApp
    • gateway
🔗 Analysis chain

Verify configuration consistency across applications.

Since this is part of a larger ConfigModule implementation, ensure configuration structure is consistent across all applications.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check configuration structure consistency across apps

# Check for ConfigService usage in other apps
echo "Checking ConfigService usage patterns:"
rg -l "ConfigService" apps/

# Check environment variable structure in .env files
echo -e "\nChecking .env structure:"
fd -g ".env*" -x cat {} \; | grep -i "userApp"

# Check for similar configuration patterns in other main.ts files
echo -e "\nChecking main.ts files for configuration patterns:"
fd "main.ts" apps/ -x grep -l "ConfigService" {}

Length of output: 758


Script:

#!/bin/bash
# Description: Check configuration structure consistency across apps

# Check for ConfigService usage in other apps
echo "Checking ConfigService usage patterns:"
rg -l "ConfigService" apps/

# Check environment variable structure in .env files
echo -e "\nChecking .env structure:"
fd --hidden -g ".env*" -x cat {} \; | grep -i "userApp"

# Check for similar configuration patterns in other main.ts files
echo -e "\nChecking main.ts files for configuration patterns:"
fd "main.ts" apps/ -x grep -l "ConfigService" {}

Length of output: 559


Script:

#!/bin/bash
# Description: Verify configuration keys across applications

# List ConfigService configuration keys in each main.ts
echo "Extracting configuration keys from main.ts files:"
fd "main.ts" apps/ -x grep -H "configService.get" {} | awk -F"'" '{print $2}' > config_keys.txt

# Display the extracted configuration keys
echo -e "\nConfiguration keys found:"
cat config_keys.txt

# Check each application's .env file for the extracted configuration keys
echo -e "\nVerifying presence of configuration keys in .env files:"
while read -r key; do
  echo "Checking for $key in .env files:"
  fd --hidden -g ".env*" -x grep -H "$key" {} || echo "$key not found in any .env files."
done < config_keys.txt

Length of output: 889


await app.listen(port);
Logger.log(`🚀 Application is running on: ${userSubGraph.host}:${port}`);
Logger.log(`🚀 Application is running on: ${host}:${port}`);
}

bootstrap();
9 changes: 0 additions & 9 deletions libs/application-config/project.json

This file was deleted.

2 changes: 0 additions & 2 deletions libs/application-config/src/index.ts

This file was deleted.

73 changes: 0 additions & 73 deletions libs/application-config/src/lib/applications-config.spec.ts

This file was deleted.

33 changes: 0 additions & 33 deletions libs/application-config/src/lib/applications-config.ts

This file was deleted.

12 changes: 0 additions & 12 deletions libs/application-config/src/lib/subgraphs-config.spec.ts

This file was deleted.

10 changes: 0 additions & 10 deletions libs/application-config/src/lib/subgraphs-config.ts

This file was deleted.

File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
export default {
displayName: 'applications-config',
displayName: 'config',
preset: '../../jest.preset.js',
testEnvironment: 'node',
transform: {
'^.+\\.[tj]s$': ['ts-jest', { tsconfig: '<rootDir>/tsconfig.spec.json' }],
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../coverage/libs/application-config',
coverageDirectory: '../../coverage/libs/config',
zhumeisongsong marked this conversation as resolved.
Show resolved Hide resolved
};
9 changes: 9 additions & 0 deletions libs/config/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "config",
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "libs/config/src",
"projectType": "library",
"tags": [],
"// targets": "to see all targets run: nx show project config --web",
"targets": {}
}
2 changes: 2 additions & 0 deletions libs/config/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './lib/applications.config';
export * from './lib/database.config';
41 changes: 41 additions & 0 deletions libs/config/src/lib/applications.config.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { gatewayConfig, userAppConfig } from './applications.config';

describe('Config Tests', () => {
describe('gatewayConfig', () => {
it('should return default values when environment variables are not set', () => {
const config = gatewayConfig();
expect(config.host).toBe('http://localhost');
expect(config.port).toBe('3333');
});

it('should return environment values when environment variables are set', () => {
process.env['GATEWAY_HOST'] = 'http://gateway-host';
process.env['GATEWAY_PORT'] = '4000';
const config = gatewayConfig();
expect(config.host).toBe('http://gateway-host');
expect(config.port).toBe('4000');
delete process.env['GATEWAY_HOST'];
delete process.env['GATEWAY_PORT'];
});
});

describe('userAppConfig', () => {
it('should return default values when environment variables are not set', () => {
const config = userAppConfig();
expect(config.host).toBe('http://localhost');
expect(config.port).toBe('15001');
expect(config.name).toBe('user');
});

it('should return environment values when environment variables are set', () => {
process.env['USER_HOST'] = 'http://user-host';
process.env['USER_PORT'] = '5000';
const config = userAppConfig();
expect(config.host).toBe('http://user-host');
expect(config.port).toBe('5000');
expect(config.name).toBe('user');
delete process.env['USER_HOST'];
delete process.env['USER_PORT'];
});
});
});
20 changes: 20 additions & 0 deletions libs/config/src/lib/applications.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { registerAs } from '@nestjs/config';

const DEFAULT_HOST = 'http://localhost';

const DEFAULT_PORT = {
user: '15001',
task: '15002',
gateway: '3333',
};

export const gatewayConfig = registerAs('gateway', () => ({
host: process.env['GATEWAY_HOST'] ?? DEFAULT_HOST,
port: process.env['GATEWAY_PORT'] ?? DEFAULT_PORT.gateway,
}));
zhumeisongsong marked this conversation as resolved.
Show resolved Hide resolved

export const userAppConfig = registerAs('userApp', () => ({
host: process.env['USER_HOST'] ?? DEFAULT_HOST,
port: process.env['USER_PORT'] ?? DEFAULT_PORT.user,
name: 'user',
}));
zhumeisongsong marked this conversation as resolved.
Show resolved Hide resolved
zhumeisongsong marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading