Skip to content

Commit 86fe43e

Browse files
jeanp413jeanp413
committed
More feedback
1 parent cc9ecec commit 86fe43e

File tree

5 files changed

+50
-25
lines changed

5 files changed

+50
-25
lines changed

‎components/gitpod-db/src/project-db.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ export interface ProjectDB extends TransactionalDB<ProjectDB> {
2727
projectId: string,
2828
envVar: ProjectEnvVarWithValue,
2929
): Promise<ProjectEnvVar | undefined>;
30-
setProjectEnvironmentVariable(projectId: string, envVar: ProjectEnvVarWithValue): Promise<void>;
30+
addProjectEnvironmentVariable(projectId: string, envVar: ProjectEnvVarWithValue): Promise<void>;
31+
updateProjectEnvironmentVariable(projectId: string, envVar: Required<ProjectEnvVarWithValue>): Promise<void>;
3132
getProjectEnvironmentVariables(projectId: string): Promise<ProjectEnvVar[]>;
3233
getProjectEnvironmentVariableById(variableId: string): Promise<ProjectEnvVar | undefined>;
3334
deleteProjectEnvironmentVariable(variableId: string): Promise<void>;

‎components/gitpod-db/src/typeorm/project-db-impl.ts

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -168,24 +168,8 @@ export class ProjectDBImpl extends TransactionalDBImpl<ProjectDB> implements Pro
168168
return envVarRepo.findOne({ projectId, name: envVar.name, deleted: false });
169169
}
170170

171-
public async setProjectEnvironmentVariable(projectId: string, envVar: ProjectEnvVarWithValue): Promise<void> {
172-
if (!envVar.name) {
173-
throw new Error("Variable name cannot be empty");
174-
}
175-
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(envVar.name)) {
176-
throw new Error(
177-
"Please choose a variable name containing only letters, numbers, or _, and which doesn't start with a number",
178-
);
179-
}
171+
public async addProjectEnvironmentVariable(projectId: string, envVar: ProjectEnvVarWithValue): Promise<void> {
180172
const envVarRepo = await this.getProjectEnvVarRepo();
181-
const envVarWithValue = await envVarRepo.findOne({ projectId, name: envVar.name, deleted: false });
182-
if (envVarWithValue) {
183-
await envVarRepo.update(
184-
{ id: envVarWithValue.id, projectId: envVarWithValue.projectId },
185-
{ value: envVar.value, censored: envVar.censored },
186-
);
187-
return;
188-
}
189173
await envVarRepo.save({
190174
id: uuidv4(),
191175
projectId,
@@ -197,6 +181,14 @@ export class ProjectDBImpl extends TransactionalDBImpl<ProjectDB> implements Pro
197181
});
198182
}
199183

184+
public async updateProjectEnvironmentVariable(
185+
projectId: string,
186+
envVar: Required<ProjectEnvVarWithValue>,
187+
): Promise<void> {
188+
const envVarRepo = await this.getProjectEnvVarRepo();
189+
await envVarRepo.update({ id: envVar.id, projectId }, { value: envVar.value, censored: envVar.censored });
190+
}
191+
200192
public async getProjectEnvironmentVariables(projectId: string): Promise<ProjectEnvVar[]> {
201193
const envVarRepo = await this.getProjectEnvVarRepo();
202194
const envVarsWithValue = await envVarRepo.find({ projectId, deleted: false });

‎components/gitpod-db/src/typeorm/user-db-impl.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,17 +409,28 @@ export class TypeORMUserDBImpl extends TransactionalDBImpl<UserDB> implements Us
409409
});
410410
}
411411

412-
public async setEnvVar(userId: string, envVar: UserEnvVarValue): Promise<void> {
412+
public async addEnvVar(userId: string, envVar: UserEnvVarValue): Promise<void> {
413413
const repo = await this.getUserEnvVarRepo();
414414
await repo.save({
415-
id: envVar.id || uuidv4(),
415+
id: uuidv4(),
416416
userId,
417417
name: envVar.name,
418418
repositoryPattern: envVar.repositoryPattern,
419419
value: envVar.value,
420420
});
421421
}
422422

423+
public async updateEnvVar(userId: string, envVar: Required<UserEnvVarValue>): Promise<void> {
424+
const repo = await this.getUserEnvVarRepo();
425+
await repo.update(
426+
{
427+
id: envVar.id,
428+
userId: userId,
429+
},
430+
{ name: envVar.name, repositoryPattern: envVar.repositoryPattern, value: envVar.value },
431+
);
432+
}
433+
423434
public async getEnvVars(userId: string): Promise<UserEnvVar[]> {
424435
const dbrepo = await this.getUserEnvVarRepo();
425436
const allVars = await dbrepo.find({ where: { userId } });

‎components/gitpod-db/src/user-db.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ export interface UserDB extends OAuthUserRepository, OAuthTokenRepository, Trans
114114
findUsersByEmail(email: string): Promise<User[]>;
115115

116116
findEnvVar(userId: string, envVar: UserEnvVarValue): Promise<UserEnvVar | undefined>;
117-
setEnvVar(userId: string, envVar: UserEnvVarValue): Promise<void>;
117+
addEnvVar(userId: string, envVar: UserEnvVarValue): Promise<void>;
118+
updateEnvVar(userId: string, envVar: UserEnvVarValue): Promise<void>;
118119
deleteEnvVar(envVar: UserEnvVar): Promise<void>;
119120
getEnvVars(userId: string): Promise<UserEnvVar[]>;
120121

‎components/server/src/user/env-var-service.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export class EnvVarService {
9494
this.analytics.track({ event: "envvar-set", userId });
9595

9696
// Ensure id is not set so a new variable is created
97-
await this.userDB.setEnvVar(userId, { ...variable, id: undefined });
97+
await this.userDB.addEnvVar(userId, variable);
9898
}
9999

100100
async updateUserEnvVar(
@@ -122,7 +122,7 @@ export class EnvVarService {
122122
this.analytics.track({ event: "envvar-set", userId });
123123

124124
// overwrite existing variable id rather than introduce a duplicate
125-
await this.userDB.setEnvVar(userId, { ...variable, id: existingVar.id });
125+
await this.userDB.updateEnvVar(userId, { ...variable, id: existingVar.id });
126126
}
127127

128128
async deleteUserEnvVar(
@@ -180,23 +180,43 @@ export class EnvVarService {
180180
async addProjectEnvVar(requestorId: string, projectId: string, envVar: ProjectEnvVarWithValue): Promise<void> {
181181
await this.auth.checkPermissionOnProject(requestorId, "write_env_var", projectId);
182182

183+
if (!envVar.name) {
184+
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Variable name cannot be empty");
185+
}
186+
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(envVar.name)) {
187+
throw new ApplicationError(
188+
ErrorCodes.BAD_REQUEST,
189+
"Please choose a variable name containing only letters, numbers, or _, and which doesn't start with a number",
190+
);
191+
}
192+
183193
const existingVar = await this.projectDB.findProjectEnvironmentVariable(projectId, envVar);
184194
if (existingVar) {
185195
throw new ApplicationError(ErrorCodes.BAD_REQUEST, `Project env var ${envVar.name} already exists`);
186196
}
187197

188-
return this.projectDB.setProjectEnvironmentVariable(projectId, envVar);
198+
return this.projectDB.addProjectEnvironmentVariable(projectId, envVar);
189199
}
190200

191201
async updateProjectEnvVar(requestorId: string, projectId: string, envVar: ProjectEnvVarWithValue): Promise<void> {
192202
await this.auth.checkPermissionOnProject(requestorId, "write_env_var", projectId);
193203

204+
if (!envVar.name) {
205+
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Variable name cannot be empty");
206+
}
207+
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(envVar.name)) {
208+
throw new ApplicationError(
209+
ErrorCodes.BAD_REQUEST,
210+
"Please choose a variable name containing only letters, numbers, or _, and which doesn't start with a number",
211+
);
212+
}
213+
194214
const existingVar = await this.projectDB.findProjectEnvironmentVariable(projectId, envVar);
195215
if (!existingVar) {
196216
throw new ApplicationError(ErrorCodes.BAD_REQUEST, `Project env var ${envVar.name} does not exists`);
197217
}
198218

199-
return this.projectDB.setProjectEnvironmentVariable(projectId, envVar);
219+
return this.projectDB.updateProjectEnvironmentVariable(projectId, { ...envVar, id: existingVar.id! });
200220
}
201221

202222
async deleteProjectEnvVar(requestorId: string, variableId: string): Promise<void> {

0 commit comments

Comments
 (0)