Sonar => Remove code smells

ExpressAPI/src/express/API.ts

@@ -21,7 +21,7 @@ import DojoCliVersionHelper           from '../helpers/DojoCliVersionHelper';
 
 class API implements WorkerTask {
     private readonly backend: Express;
-    private server: http.Server | undefined;
+    private server!: http.Server;
 
     constructor() {
         this.backend = express();
@@ -89,7 +89,7 @@ class API implements WorkerTask {
             const {
                       port,
                       address
-                  } = this.server!.address() as AddressInfo;
+                  } = this.server.address() as AddressInfo;
             logger.info(`Server started on http://${ address }:${ port }`);
         });
     }

ExpressAPI/src/helpers/GlobalHelper.ts

@@ -35,6 +35,24 @@ class GlobalHelper {
                     throw error;
                 }
             }
+
+            return undefined;
+        };
+    }
+
+    isRepoNameAlreadyTaken(errorDescription: unknown) {
+        return errorDescription instanceof Object && 'name' in errorDescription && errorDescription.name instanceof Array && errorDescription.name.length > 0 && errorDescription.name[0] === 'has already been taken';
+    }
+
+    addRepoMember(repositoryId: number) {
+        return async (memberId: number): Promise<Gitlab.MemberSchema | false> => {
+            try {
+                return await GitlabManager.addRepositoryMember(repositoryId, memberId, Gitlab.AccessLevel.DEVELOPER);
+            } catch ( error ) {
+                logger.error('Add member error');
+                logger.error(JSON.stringify(error));
+                return false;
+            }
         };
     }
 }

ExpressAPI/src/managers/GitlabManager.ts

@@ -51,11 +51,11 @@ class GitlabManager extends SharedGitlabManager {
         }
     }
 
-    createRepository(name: string, description: string, visibility: 'public' | 'internal' | 'private', initializeWithReadme: boolean, namespace: number, sharedRunnersEnabled: boolean, wikiEnabled: boolean, import_url: string): Promise<ProjectSchema> {
+    createRepository(name: string, description: string, visibility: 'public' | 'internal' | 'private', initializeWithReadme: boolean, namespace: number, sharedRunnersEnabled: boolean, wikiEnabled: boolean, importUrl: string): Promise<ProjectSchema> {
         return this.api.Projects.create({
                                             name                : name,
                                             description         : description,
-                                            importUrl           : import_url,
+                                            importUrl           : importUrl,
                                             initializeWithReadme: initializeWithReadme,
                                             namespaceId         : namespace,
                                             sharedRunnersEnabled: sharedRunnersEnabled,

ExpressAPI/src/routes/AssignmentRoutes.ts

@@ -96,15 +96,18 @@ class AssignmentRoutes implements RoutesManager {
             if ( error instanceof GitbeakerRequestError ) {
                 if ( error.cause?.description ) {
                     const description = error.cause.description as unknown;
-                    if ( description instanceof Object && 'name' in description && description.name instanceof Array && description.name.length > 0 && description.name[0] === 'has already been taken' ) {
-                        return req.session.sendResponse(res, StatusCodes.CONFLICT, {}, `Repository name has already been taken`, DojoStatusCode.ASSIGNMENT_NAME_CONFLICT);
+                    if ( GlobalHelper.isRepoNameAlreadyTaken(description) ) {
+                        req.session.sendResponse(res, StatusCodes.CONFLICT, {}, `Repository name has already been taken`, DojoStatusCode.ASSIGNMENT_NAME_CONFLICT);
+                        return;
                     }
                 }
 
-                return req.session.sendResponse(res, error.cause?.response.status ?? StatusCodes.INTERNAL_SERVER_ERROR);
+                req.session.sendResponse(res, error.cause?.response.status ?? StatusCodes.INTERNAL_SERVER_ERROR);
+                return;
             }
 
-            return req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR);
+            req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR);
+            return;
         }
 
         await new Promise(resolve => setTimeout(resolve, Config.gitlab.repository.timeoutAfterCreation));
@@ -117,15 +120,7 @@ class AssignmentRoutes implements RoutesManager {
             await repoCreationFnExec(() => GitlabManager.deleteFile(repository.id, '.gitlab-ci.yml', 'Remove .gitlab-ci.yml'));
             await repoCreationFnExec(() => GitlabManager.createFile(repository.id, '.gitlab-ci.yml', fs.readFileSync(path.join(__dirname, '../../assets/assignment_gitlab_ci.yml'), 'base64'), 'Add .gitlab-ci.yml (DO NOT MODIFY THIS FILE)'), 'CI/CD file creation error');
 
-            await repoCreationFnExec(() => Promise.all(params.members.map(member => member.id).map(async (memberId: number): Promise<Gitlab.MemberSchema | false> => {
-                try {
-                    return await GitlabManager.addRepositoryMember(repository.id, memberId, Gitlab.AccessLevel.DEVELOPER);
-                } catch ( error ) {
-                    logger.error('Add member error');
-                    logger.error(JSON.stringify(error));
-                    return false;
-                }
-            })), 'Add repository members error');
+            await repoCreationFnExec(() => Promise.all(params.members.map(member => member.id).map(GlobalHelper.addRepoMember(repository.id))), 'Add repository members error');
 
             const assignment: Assignment = await repoCreationFnExec(() => db.assignment.create({
                                                                                                    data: {
@@ -151,7 +146,7 @@ class AssignmentRoutes implements RoutesManager {
                                                                                                    }
                                                                                                }), 'Database error') as Assignment;
 
-            return req.session.sendResponse(res, StatusCodes.OK, assignment);
+            req.session.sendResponse(res, StatusCodes.OK, assignment);
         } catch ( error ) {
             /* Empty */
         }

ExpressAPI/src/routes/ExerciseRoutes.ts

@@ -109,16 +109,14 @@ class ExerciseRoutes implements RoutesManager {
                 logger.error('Repo creation error');
                 logger.error(JSON.stringify(error));
 
-                if ( error instanceof GitbeakerRequestError ) {
-                    if ( error.cause?.description ) {
+                if ( error instanceof GitbeakerRequestError && error.cause?.description ) {
                     const description = error.cause.description as unknown;
-                        if ( description instanceof Object && 'name' in description && description.name instanceof Array && description.name.length > 0 && description.name[0] === 'has already been taken' ) {
+                    if ( GlobalHelper.isRepoNameAlreadyTaken(description) ) {
                         suffix++;
                     } else {
                         req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, {}, 'Unknown gitlab error while forking repository', DojoStatusCode.EXERCISE_CREATION_GITLAB_ERROR);
                         return undefined;
                     }
-                    }
                 } else {
                     req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, {}, 'Unknown error while forking repository', DojoStatusCode.EXERCISE_CREATION_INTERNAL_ERROR);
                     return undefined;
@@ -126,12 +124,6 @@ class ExerciseRoutes implements RoutesManager {
             }
         } while ( suffix < Config.exercise.maxSameName );
 
-        if ( suffix >= Config.exercise.maxSameName ) {
-            logger.error('Max exercise with same name reached');
-            req.session.sendResponse(res, StatusCodes.INSUFFICIENT_SPACE_ON_RESOURCE, undefined, 'Max exercise per assignment reached', DojoStatusCode.MAX_EXERCISE_PER_ASSIGNMENT_REACHED);
-            return undefined;
-        }
-
         return repository;
     }
 
@@ -172,15 +164,7 @@ class ExerciseRoutes implements RoutesManager {
 
             await repoCreationFnExec(() => GitlabManager.updateFile(repository.id, '.gitlab-ci.yml', fs.readFileSync(path.join(__dirname, '../../assets/exercise_gitlab_ci.yml'), 'base64'), 'Add .gitlab-ci.yml (DO NOT MODIFY THIS FILE)'), 'CI/CD file update error');
 
-            await repoCreationFnExec(async () => Promise.all([ ...new Set([ ...assignment.staff.map(user => user.id), ...params.members.map(member => member.id) ]) ].map(async (memberId: number): Promise<Gitlab.MemberSchema | false> => {
-                try {
-                    return await GitlabManager.addRepositoryMember(repository.id, memberId, Gitlab.AccessLevel.DEVELOPER);
-                } catch ( error ) {
-                    logger.error('Add member error');
-                    logger.error(JSON.stringify(error));
-                    return false;
-                }
-            })), 'Add repository members error');
+            await repoCreationFnExec(async () => Promise.all([ ...new Set([ ...assignment.staff, ...params.members ].map(member => member.id)) ].map(GlobalHelper.addRepoMember(repository.id))), 'Add repository members error');
 
             const exercise: Exercise = await repoCreationFnExec(() => db.exercise.create({
                                                                                              data: {
@@ -209,7 +193,8 @@ class ExerciseRoutes implements RoutesManager {
                                                                                              }
                                                                                          })) as Exercise;
 
-            return req.session.sendResponse(res, StatusCodes.OK, exercise);
+            req.session.sendResponse(res, StatusCodes.OK, exercise);
+            return;
         } catch ( error ) {
             /* Empty */
         }