From df52c111e73e66c13f78bb7228924737403afa96 Mon Sep 17 00:00:00 2001
From: Steve Reis <stevereis93@gmail.com>
Date: Thu, 2 Jun 2022 16:08:13 +0200
Subject: [PATCH] fix: User automatically extended in the request
---
.../connectors/exareme/main.connector.ts | 13 +-
.../connectors/exareme/transformations.ts | 4 +
api/src/engine/engine.interfaces.ts | 2 +-
.../users/interceptors/users.interceptor.ts | 30 +++
api/src/users/users.module.ts | 11 +-
api/src/users/users.resolver.spec.ts | 175 ++++++++----------
api/src/users/users.resolver.ts | 59 +++---
api/src/users/users.service.spec.ts | 97 +++++++---
api/src/users/users.service.ts | 26 ++-
9 files changed, 242 insertions(+), 175 deletions(-)
create mode 100644 api/src/users/interceptors/users.interceptor.ts
diff --git a/api/src/engine/connectors/exareme/main.connector.ts b/api/src/engine/connectors/exareme/main.connector.ts
index 9420070..e0bde63 100644
--- a/api/src/engine/connectors/exareme/main.connector.ts
+++ b/api/src/engine/connectors/exareme/main.connector.ts
@@ -22,12 +22,11 @@ import {
Experiment,
PartialExperiment,
} from 'src/engine/models/experiment/experiment.model';
-import { ExperimentCreateInput } from 'src/experiments/models/input/experiment-create.input';
-import { ExperimentEditInput } from 'src/experiments/models/input/experiment-edit.input';
import { ListExperiments } from 'src/engine/models/experiment/list-experiments.model';
import { Group } from 'src/engine/models/group.model';
import { Variable } from 'src/engine/models/variable.model';
-import { UpdateUserInput } from 'src/users/inputs/update-user.input';
+import { ExperimentCreateInput } from 'src/experiments/models/input/experiment-create.input';
+import { ExperimentEditInput } from 'src/experiments/models/input/experiment-edit.input';
import { User } from 'src/users/models/user.model';
import { transformToUser } from '../datashield/transformations';
import {
@@ -41,6 +40,7 @@ import { ExperimentData } from './interfaces/experiment/experiment.interface';
import { ExperimentsData } from './interfaces/experiment/experiments.interface';
import { Hierarchy } from './interfaces/hierarchy.interface';
import { Pathology } from './interfaces/pathology.interface';
+import { dataToUser } from './transformations';
import transformToAlgorithms from './transformations/algorithms';
type Headers = Record<string, string>;
@@ -194,15 +194,16 @@ export default class ExaremeService implements IEngineService {
}
}
- async updateUser(request: Request): Promise<UpdateUserInput | undefined> {
+ async updateUser(request: Request): Promise<User> {
const path = this.options.baseurl + 'activeUser/agreeNDA';
- await firstValueFrom(
+
+ const result = await firstValueFrom(
this.post<string>(request, path, {
agreeNDA: true,
}),
);
- return undefined; //we don't want to manage data locally
+ return dataToUser.evaluate(result);
}
getPassthrough(
diff --git a/api/src/engine/connectors/exareme/transformations.ts b/api/src/engine/connectors/exareme/transformations.ts
index 4a91590..ede7dcd 100644
--- a/api/src/engine/connectors/exareme/transformations.ts
+++ b/api/src/engine/connectors/exareme/transformations.ts
@@ -166,6 +166,10 @@ export const dataToHeatmap = jsonata(`
)
`);
+export const dataToUser = jsonata(`
+$ ~> |$|{'id': subjectId}, ['subjectId']|
+`);
+
dataToHeatmap.registerFunction(
'toMat',
(a) => {
diff --git a/api/src/engine/engine.interfaces.ts b/api/src/engine/engine.interfaces.ts
index d9ef0e6..08fb861 100644
--- a/api/src/engine/engine.interfaces.ts
+++ b/api/src/engine/engine.interfaces.ts
@@ -120,7 +120,7 @@ export interface IEngineService {
req?: Request,
userId?: string,
data?: UpdateUserInput,
- ): Promise<UpdateUserInput | undefined>;
+ ): Promise<User | undefined>;
/**
* Perform a logout on the current logged in user
diff --git a/api/src/users/interceptors/users.interceptor.ts b/api/src/users/interceptors/users.interceptor.ts
new file mode 100644
index 0000000..e41804c
--- /dev/null
+++ b/api/src/users/interceptors/users.interceptor.ts
@@ -0,0 +1,30 @@
+import {
+ CallHandler,
+ ExecutionContext,
+ Injectable,
+ NestInterceptor,
+} from '@nestjs/common';
+import { GqlExecutionContext } from '@nestjs/graphql';
+import { Observable } from 'rxjs';
+import { User } from '../models/user.model';
+import { UsersService } from '../users.service';
+
+@Injectable()
+export class UsersInterceptor implements NestInterceptor {
+ constructor(private readonly usersService: UsersService) {}
+
+ async intercept(
+ context: ExecutionContext,
+ next: CallHandler,
+ ): Promise<Observable<any>> {
+ const ctx = GqlExecutionContext.create(context);
+ const req = ctx.getContext().req ?? ctx.switchToHttp().getRequest();
+
+ const user: User = req.user;
+ if (user && user.id) {
+ await this.usersService.extendedUser(user);
+ }
+
+ return next.handle();
+ }
+}
diff --git a/api/src/users/users.module.ts b/api/src/users/users.module.ts
index 86e30c7..b537487 100644
--- a/api/src/users/users.module.ts
+++ b/api/src/users/users.module.ts
@@ -1,11 +1,20 @@
import { Module } from '@nestjs/common';
+import { APP_INTERCEPTOR } from '@nestjs/core';
import { TypeOrmModule } from '@nestjs/typeorm';
+import { UsersInterceptor } from './interceptors/users.interceptor';
import { User } from './models/user.model';
import { UsersResolver } from './users.resolver';
import { UsersService } from './users.service';
@Module({
imports: [TypeOrmModule.forFeature([User])],
- providers: [UsersResolver, UsersService],
+ providers: [
+ UsersResolver,
+ UsersService,
+ {
+ provide: APP_INTERCEPTOR,
+ useClass: UsersInterceptor,
+ },
+ ],
})
export class UsersModule {}
diff --git a/api/src/users/users.resolver.spec.ts b/api/src/users/users.resolver.spec.ts
index ad7ccb8..ad93fc2 100644
--- a/api/src/users/users.resolver.spec.ts
+++ b/api/src/users/users.resolver.spec.ts
@@ -1,18 +1,29 @@
import { getMockReq } from '@jest-mock/express';
-import { NotFoundException } from '@nestjs/common';
+import { InternalServerErrorException } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
-import { MockFunctionMetadata, ModuleMocker } from 'jest-mock';
import { ENGINE_SERVICE } from '../engine/engine.constants';
+import { IEngineService } from '../engine/engine.interfaces';
import { UpdateUserInput } from './inputs/update-user.input';
import { User } from './models/user.model';
import { UsersResolver } from './users.resolver';
-import { InternalUser, UsersService } from './users.service';
+import { UsersService } from './users.service';
-const moduleMocker = new ModuleMocker(global);
+type MockEngineService = Partial<Record<keyof IEngineService, jest.Mock>>;
+type MockUsersService = Partial<Record<keyof UsersService, jest.Mock>>;
+
+const createEngineService = (): MockEngineService => ({
+ updateUser: jest.fn(),
+});
+
+const createUsersService = (): MockUsersService => ({
+ update: jest.fn(),
+});
describe('UsersResolver', () => {
let resolver: UsersResolver;
const req = getMockReq();
+ let engineService: MockEngineService;
+ let usersService: MockUsersService;
const user: User = {
id: 'guest',
@@ -20,117 +31,83 @@ describe('UsersResolver', () => {
fullname: 'This is la Peste',
};
- const updateData: UpdateUserInput = {
- agreeNDA: true,
- };
-
- const internUser: InternalUser = {
- id: 'guest',
- agreeNDA: false,
- };
-
- const internUserWrong: InternalUser = {
- id: 'guest1',
- agreeNDA: false,
- };
-
- const findOne = jest
- .fn()
- .mockResolvedValueOnce(internUserWrong)
- .mockResolvedValueOnce(internUserWrong)
- .mockImplementationOnce(() => {
- throw new NotFoundException();
- })
- .mockResolvedValue(internUser);
-
- const getActiveUser = jest
- .fn()
- .mockResolvedValueOnce(user)
- .mockResolvedValueOnce({})
- .mockResolvedValue(user);
-
- const engineService = {
- getActiveUser,
- updateUser: jest
- .fn()
- .mockReturnValue(undefined)
- .mockResolvedValue(undefined),
- };
-
- const updateService = jest.fn().mockResolvedValue({ ...user, ...internUser });
-
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
- providers: [UsersResolver],
- })
- .useMocker((token) => {
- if (token == UsersService) {
- return {
- findOne,
- update: updateService,
- };
- }
- if (token == ENGINE_SERVICE) {
- return engineService;
- }
- if (typeof token === 'function') {
- const mockMetadata = moduleMocker.getMetadata(
- token,
- ) as MockFunctionMetadata<any, any>;
- const Mock = moduleMocker.generateFromMetadata(mockMetadata);
- return new Mock();
- }
- })
- .compile();
+ providers: [
+ UsersResolver,
+ { provide: UsersService, useValue: createUsersService() },
+ {
+ provide: ENGINE_SERVICE,
+ useValue: createEngineService(),
+ },
+ ],
+ }).compile();
resolver = module.get<UsersResolver>(UsersResolver);
+ engineService = module.get<MockEngineService>(ENGINE_SERVICE);
+ usersService = module.get<UsersService>(
+ UsersService,
+ ) as unknown as MockUsersService;
});
- it('Get user with different id from engine and database', async () => {
- expect(await resolver.getUser(req, user)).toStrictEqual({
- ...user,
+ describe('getUser', () => {
+ it('Get simple user', async () => {
+ const excpectedUser: User = {
+ id: 'guest',
+ username: 'guest',
+ fullname: 'This is la Peste',
+ };
+ const result = await resolver.getUser(excpectedUser);
+ expect(result).toStrictEqual(excpectedUser);
});
- });
- it('Get user incomplete merge', async () => {
- expect(resolver.getUser(req, user)).rejects.toThrowError();
+ it('Undefined user should throw an InternalServerErrorException', async () => {
+ try {
+ await resolver.getUser(undefined);
+ } catch (err) {
+ expect(err).toBeInstanceOf(InternalServerErrorException);
+ }
+ });
});
- it('Get user not found in db', async () => {
- expect(await resolver.getUser(req, user)).toStrictEqual(user);
- });
+ describe('updateUser', () => {
+ it('Update user from engine ', async () => {
+ const updateData: UpdateUserInput = {
+ agreeNDA: true,
+ };
+ const expectedUser = {
+ ...user,
+ ...updateData,
+ };
+
+ engineService.updateUser.mockReturnValue(expectedUser);
+ const result = await resolver.updateUser(req, updateData, user);
- it('Get user in engine and database (merge)', async () => {
- expect(await resolver.getUser(req, user)).toStrictEqual({
- ...user,
- ...internUser,
+ expect(result).toStrictEqual(expectedUser);
});
- });
- it('Undefined user should not throw exception', async () => {
- expect(await resolver.getUser(req, undefined)).toBeTruthy();
- });
+ it('Update user from database', async () => {
+ const updateData: UpdateUserInput = {
+ agreeNDA: true,
+ };
+ const expectedUser = {
+ ...user,
+ ...updateData,
+ };
- it('Update user from engine ', async () => {
- engineService.updateUser.mockClear();
- updateService.mockClear();
- await resolver.updateUser(req, updateData, user);
- expect(engineService.updateUser.mock.calls.length > 0);
- expect(updateService.mock.calls.length === 0);
- });
+ engineService.updateUser = undefined;
+ usersService.update.mockReturnValue(expectedUser);
+ const result = await resolver.updateUser(req, updateData, user);
- it('Update user from database', async () => {
- engineService.updateUser = jest
- .fn()
- .mockReturnValue(undefined)
- .mockResolvedValue(undefined);
- expect(await resolver.updateUser(req, updateData, user)).toStrictEqual({
- ...user,
- ...internUser,
+ expect(result).toStrictEqual(expectedUser);
});
- });
- it('Undefined user should not throw exception', async () => {
- expect(await resolver.updateUser(req, updateData, user)).toBeTruthy();
+ it('Undefined user should throw an exception', async () => {
+ try {
+ await resolver.updateUser(req, {});
+ } catch (err) {
+ expect(err).toBeInstanceOf(InternalServerErrorException);
+ }
+ });
});
});
diff --git a/api/src/users/users.resolver.ts b/api/src/users/users.resolver.ts
index 494f842..5d54d88 100644
--- a/api/src/users/users.resolver.ts
+++ b/api/src/users/users.resolver.ts
@@ -27,39 +27,15 @@ export class UsersResolver {
@Query(() => User, { name: 'user' })
/**
- * It returns the user from the engine, if it exists. Same from the internal database
- * merge internal object over engine one to have a final user.
- * @param {Request} request - Request
- * @param {User} reqUser - The user that is currently logged in.
- * @returns A user object.
+ * Return the user object
+ * @param {User} user - User - This is the user object that is passed in from the decorator.
+ * @returns The user object
*/
- async getUser(@GQLRequest() request: Request, @CurrentUser() reqUser: User) {
- const user: Partial<User> = {};
+ async getUser(@CurrentUser() user: User) {
+ if (!user || !user.id || !user.username)
+ throw new InternalServerErrorException('User cannot be retrieve');
- if (this.engineService.getActiveUser) {
- const engineUser = await this.engineService.getActiveUser(request);
- if (engineUser) Object.assign(user, engineUser);
- }
-
- // Checking if the user exists in the internal database. If it does, it will assign the user to the `user` object.
- try {
- const internalUser = reqUser
- ? await this.usersService.findOne(reqUser.id)
- : undefined;
-
- if (internalUser && (!user.id || internalUser.id === user.id)) {
- Object.assign(user, internalUser);
- }
- } catch (e) {
- this.logger.verbose(e);
- }
-
- if (!user.id || !user.username)
- throw new InternalServerErrorException(
- 'The user cannot be construct from the engine',
- );
-
- return user as User;
+ return user;
}
/**
@@ -75,18 +51,27 @@ export class UsersResolver {
@Args('updateUserInput') updateUserInput: UpdateUserInput,
@CurrentUser() user?: User,
) {
- let updateData: UpdateUserInput | undefined = updateUserInput;
+ if (!user || !user.id || !user.username)
+ throw new InternalServerErrorException('User cannot be retrieve');
+
+ let updatedInfo: Partial<User>;
+
if (this.engineService.updateUser) {
- updateData = await this.engineService.updateUser(
+ updatedInfo = await this.engineService.updateUser(
request,
user?.id,
- updateData,
+ updateUserInput,
+ );
+ } else {
+ const internalUser = await this.usersService.update(
+ user.id,
+ updateUserInput,
);
+ if (internalUser) Object.assign(user, internalUser);
}
- if (updateData && Object.keys(updateData).length > 0)
- await this.usersService.update(user.id, updateData);
+ if (updatedInfo) Object.assign(user, updatedInfo);
- return this.getUser(request, user);
+ return user;
}
}
diff --git a/api/src/users/users.service.spec.ts b/api/src/users/users.service.spec.ts
index f75bffe..aaad4fb 100644
--- a/api/src/users/users.service.spec.ts
+++ b/api/src/users/users.service.spec.ts
@@ -1,14 +1,21 @@
+import { NotFoundException } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
-import { MockFunctionMetadata, ModuleMocker } from 'jest-mock';
+import { Repository } from 'typeorm';
import { UpdateUserInput } from './inputs/update-user.input';
import { User } from './models/user.model';
import { UsersService } from './users.service';
-const moduleMocker = new ModuleMocker(global);
+type MockRepository<T = any> = Partial<Record<keyof Repository<T>, jest.Mock>>;
+
+const createMockRepository = <T = any>(): MockRepository<T> => ({
+ findOne: jest.fn(),
+ save: jest.fn(),
+});
describe('UsersService', () => {
let service: UsersService;
+ let usersRepository: MockRepository;
const user: User = {
id: 'guest',
username: 'guest',
@@ -21,40 +28,70 @@ describe('UsersService', () => {
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
- providers: [UsersService],
- })
- .useMocker((token) => {
- if (token === getRepositoryToken(User)) {
- return {
- findOne: jest
- .fn()
- .mockResolvedValue(user)
- .mockResolvedValueOnce(undefined),
- save: jest.fn().mockResolvedValue({ ...user, ...updateData }), //todo
- };
- }
- if (typeof token === 'function') {
- const mockMetadata = moduleMocker.getMetadata(
- token,
- ) as MockFunctionMetadata<any, any>;
- const Mock = moduleMocker.generateFromMetadata(mockMetadata);
- return new Mock();
- }
- })
- .compile();
+ providers: [
+ UsersService,
+ {
+ provide: getRepositoryToken(User),
+ useValue: createMockRepository<User>(),
+ },
+ ],
+ }).compile();
service = module.get<UsersService>(UsersService);
+ usersRepository = module.get<MockRepository>(getRepositoryToken(User));
});
- it('getUser', async () => {
- expect(service.findOne('IdThatDoesNotExist')).rejects.toThrow();
- expect(await service.findOne('idThatExist')).toBe(user);
+ describe('getUser', () => {
+ describe('when user exist', () => {
+ it('Should return a user', async () => {
+ usersRepository.findOne.mockReturnValue(user);
+ const result = await service.findOne('idThatExist');
+
+ expect(result).toStrictEqual(user);
+ });
+ });
+
+ describe('otherwise', () => {
+ it('Should return a NotFoundException', async () => {
+ usersRepository.findOne.mockReturnValue(undefined);
+
+ try {
+ await service.findOne('IdThatDoesNotExist');
+ } catch (err) {
+ expect(err).toBeInstanceOf(NotFoundException);
+ }
+ });
+ });
});
- it('updateUser', async () => {
- expect(await service.update('idThatExist', updateData)).toStrictEqual({
- ...user,
- ...updateData,
+ describe('updateUser', () => {
+ it('should return an updated user', async () => {
+ const expectedUser = { ...user, ...updateData };
+ usersRepository.save.mockResolvedValue(expectedUser);
+
+ const result = await service.update('idThatExist', updateData);
+
+ expect(result).toStrictEqual(expectedUser);
+ });
+ });
+
+ describe('extendedUser', () => {
+ it('should return an extended user', async () => {
+ const localUser: User = {
+ id: 'dummyId',
+ username: 'dummyUsername',
+ };
+
+ const expectedUser = {
+ ...localUser,
+ agreeNDA: true,
+ };
+
+ usersRepository.findOne.mockReturnValue(expectedUser);
+
+ await service.extendedUser(localUser);
+
+ expect(localUser).toStrictEqual(expectedUser);
});
});
});
diff --git a/api/src/users/users.service.ts b/api/src/users/users.service.ts
index bd8105d..bcf4317 100644
--- a/api/src/users/users.service.ts
+++ b/api/src/users/users.service.ts
@@ -1,4 +1,4 @@
-import { Injectable, NotFoundException } from '@nestjs/common';
+import { Injectable, Logger, NotFoundException } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { UpdateUserInput } from './inputs/update-user.input';
@@ -13,6 +13,8 @@ export class UsersService {
private readonly userRepository: Repository<InternalUser>,
) {}
+ private readonly logger = new Logger(UsersService.name);
+
/**
* Get a user by id
* @param {string} id - The id of the user to be retrieved.
@@ -40,4 +42,26 @@ export class UsersService {
return this.userRepository.save(updateData);
}
+
+ /**
+ * It takes a user object, checks if it has an id, and if it does, it tries to find the user in the
+ * database and then merges the database user with the user object
+ * @param {User} user - User - The user object that is being extended.
+ */
+ async extendedUser(user: User) {
+ if (!user || !user.id) {
+ return;
+ }
+
+ try {
+ const dbUser = await this.findOne(user.id);
+
+ Object.assign(user, dbUser);
+ } catch (err) {
+ if (err instanceof NotFoundException)
+ this.logger.debug(
+ `Extension of ${user.id} aborted, no user found in database`,
+ );
+ }
+ }
}
--
GitLab