[2/n]: Rest API -> TwentyORM migration POST rest/* (#9986)

# This PR

- Addressing #3644 
- Migrates the `POST rest/*` endpoint to use TwentyORM directly
- Adds integration tests
- Refactors common login in the v2 service file
- Refactors test utility files
This commit is contained in:
P A C · 先生
2025-02-04 13:36:52 +02:00
committed by GitHub
parent 40f43a4076
commit a5e27aa751
6 changed files with 195 additions and 64 deletions

View File

@ -15,10 +15,10 @@ import { Request, Response } from 'express';
import { RestApiCoreServiceV2 } from 'src/engine/api/rest/core/rest-api-core-v2.service'; import { RestApiCoreServiceV2 } from 'src/engine/api/rest/core/rest-api-core-v2.service';
import { RestApiCoreService } from 'src/engine/api/rest/core/rest-api-core.service'; import { RestApiCoreService } from 'src/engine/api/rest/core/rest-api-core.service';
import { RestApiExceptionFilter } from 'src/engine/api/rest/rest-api-exception.filter';
import { cleanGraphQLResponse } from 'src/engine/api/rest/utils/clean-graphql-response.utils'; import { cleanGraphQLResponse } from 'src/engine/api/rest/utils/clean-graphql-response.utils';
import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard'; import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard';
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
import { RestApiExceptionFilter } from 'src/engine/api/rest/rest-api-exception.filter';
@Controller('rest/*') @Controller('rest/*')
@UseGuards(JwtAuthGuard, WorkspaceAuthGuard) @UseGuards(JwtAuthGuard, WorkspaceAuthGuard)
@ -53,10 +53,11 @@ export class RestApiCoreController {
} }
@Post() @Post()
@UseFilters(RestApiExceptionFilter)
async handleApiPost(@Req() request: Request, @Res() res: Response) { async handleApiPost(@Req() request: Request, @Res() res: Response) {
const result = await this.restApiCoreService.createOne(request); const result = await this.restApiCoreServiceV2.createOne(request);
res.status(201).send(cleanGraphQLResponse(result.data.data)); res.status(201).send(result);
} }
@Patch() @Patch()

View File

@ -15,39 +15,16 @@ export class RestApiCoreServiceV2 {
) {} ) {}
async delete(request: Request) { async delete(request: Request) {
const { workspace } = request; const { id: recordId } = parseCorePath(request);
const { object: parsedObject, id: recordId } = parseCorePath(request);
const objectMetadata = await this.coreQueryBuilderFactory.getObjectMetadata(
request,
parsedObject,
);
if (!objectMetadata) {
throw new BadRequestException('Object metadata not found');
}
if (!recordId) { if (!recordId) {
throw new BadRequestException('Record ID not found'); throw new BadRequestException('Record ID not found');
} }
const objectMetadataNameSingular = const { objectMetadataNameSingular, repository } =
objectMetadata.objectMetadataItem.nameSingular; await this.getRepositoryAndMetadataOrFail(request);
if (!workspace?.id) {
throw new BadRequestException('Workspace not found');
}
const repository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
workspace.id,
objectMetadataNameSingular,
);
const recordToDelete = await repository.findOneOrFail({ const recordToDelete = await repository.findOneOrFail({
where: { where: { id: recordId },
id: recordId,
},
}); });
await repository.delete(recordId); await repository.delete(recordId);
@ -57,6 +34,20 @@ export class RestApiCoreServiceV2 {
}); });
} }
async createOne(request: Request) {
const { body } = request;
const { objectMetadataNameSingular, repository } =
await this.getRepositoryAndMetadataOrFail(request);
const createdRecord = await repository.save(body);
return this.formatResult(
'create',
objectMetadataNameSingular,
createdRecord,
);
}
private formatResult<T>( private formatResult<T>(
operation: 'delete' | 'create' | 'update' | 'find', operation: 'delete' | 'create' | 'update' | 'find',
objectNameSingular: string, objectNameSingular: string,
@ -70,4 +61,32 @@ export class RestApiCoreServiceV2 {
return result; return result;
} }
private async getRepositoryAndMetadataOrFail(request: Request) {
const { workspace } = request;
const { object: parsedObject } = parseCorePath(request);
const objectMetadata = await this.coreQueryBuilderFactory.getObjectMetadata(
request,
parsedObject,
);
if (!objectMetadata) {
throw new BadRequestException('Object metadata not found');
}
if (!workspace?.id) {
throw new BadRequestException('Workspace not found');
}
const objectMetadataNameSingular =
objectMetadata.objectMetadataItem.nameSingular;
const repository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
workspace.id,
objectMetadataNameSingular,
);
return { objectMetadataNameSingular, repository };
}
} }

View File

@ -51,7 +51,7 @@ export class HttpExceptionHandlerService {
return response.status(statusCode).send({ return response.status(statusCode).send({
statusCode, statusCode,
error: exception.code || 'Bad Request', error: exception.name || 'Bad Request',
messages: [exception?.message], messages: [exception?.message],
}); });
}; };

View File

@ -0,0 +1,95 @@
import {
FAKE_PERSON_ID,
PERSON_2_ID,
} from 'test/integration/constants/mock-person-ids.constants';
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
import { generateRecordName } from 'test/integration/utils/generate-record-name';
describe('Core REST API Create One endpoint', () => {
afterAll(async () => {
await makeRestAPIRequest({
method: 'delete',
path: `/people/${PERSON_2_ID}`,
}).expect(200);
});
it('2.a. should create a new person', async () => {
const personCity = generateRecordName(PERSON_2_ID);
const requestBody = {
id: PERSON_2_ID,
city: personCity,
};
const response = await makeRestAPIRequest({
method: 'post',
path: `/people`,
body: requestBody,
});
const createdPerson = response.body.data.createPerson;
expect(createdPerson.id).toBe(PERSON_2_ID);
expect(createdPerson.city).toBe(personCity);
});
it('2.b. should return a BadRequestException when trying to create a person with an existing ID', async () => {
const personCity = generateRecordName(PERSON_2_ID);
const requestBody = {
id: PERSON_2_ID,
city: personCity,
};
await makeRestAPIRequest({
method: 'post',
path: `/people`,
body: requestBody,
})
.expect(400)
.expect((res) => {
expect(res.body.messages[0]).toContain(
`duplicate key value violates unique constraint`,
);
expect(res.body.error).toBe('QueryFailedError');
});
});
it('2.c. should return an UnauthorizedException when no token is provided', async () => {
await makeRestAPIRequest({
method: 'post',
path: `/people`,
headers: { authorization: '' },
body: { id: FAKE_PERSON_ID, city: 'FakeCity' },
})
.expect(401)
.expect((res) => {
expect(res.body.error).toBe('UNAUTHENTICATED');
});
});
it('2.d. should return an UnauthorizedException when an invalid token is provided', async () => {
await makeRestAPIRequest({
method: 'post',
path: `/people`,
body: { id: FAKE_PERSON_ID, city: 'FakeCity' },
headers: { authorization: 'Bearer invalid-token' },
})
.expect(401)
.expect((res) => {
expect(res.body.error).toBe('UNAUTHENTICATED');
});
});
it('2.e. should return an UnauthorizedException when an expired token is provided', async () => {
await makeRestAPIRequest({
method: 'post',
path: `/people`,
body: { id: FAKE_PERSON_ID, city: 'FakeCity' },
headers: { authorization: `Bearer ${EXPIRED_ACCESS_TOKEN}` },
})
.expect(401)
.expect((res) => {
expect(res.body.error).toBe('UNAUTHENTICATED');
expect(res.body.messages[0]).toBe('Token has expired.');
});
});
});

View File

@ -3,7 +3,6 @@ import {
PERSON_1_ID, PERSON_1_ID,
} from 'test/integration/constants/mock-person-ids.constants'; } from 'test/integration/constants/mock-person-ids.constants';
import { PERSON_GQL_FIELDS } from 'test/integration/constants/person-gql-fields.constants'; import { PERSON_GQL_FIELDS } from 'test/integration/constants/person-gql-fields.constants';
import { createManyOperationFactory } from 'test/integration/graphql/utils/create-many-operation-factory.util';
import { findOneOperationFactory } from 'test/integration/graphql/utils/find-one-operation-factory.util'; import { findOneOperationFactory } from 'test/integration/graphql/utils/find-one-operation-factory.util';
import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util';
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util'; import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
@ -15,21 +14,15 @@ describe('Core REST API Delete One endpoint', () => {
beforeAll(async () => { beforeAll(async () => {
const personCity1 = generateRecordName(PERSON_1_ID); const personCity1 = generateRecordName(PERSON_1_ID);
// TODO: move this creation to REST API when the POST method is migrated const response = await makeRestAPIRequest({
const graphqlOperation = createManyOperationFactory({ method: 'post',
objectMetadataSingularName: 'person', path: '/people',
objectMetadataPluralName: 'people', body: {
gqlFields: PERSON_GQL_FIELDS, id: PERSON_1_ID,
data: [ city: personCity1,
{ },
id: PERSON_1_ID,
city: personCity1,
},
],
}); });
const response = await makeGraphqlAPIRequest(graphqlOperation);
people = response.body.data.createPeople; people = response.body.data.createPeople;
expect(people.length).toBe(1); expect(people.length).toBe(1);
expect(people[0].id).toBe(PERSON_1_ID); expect(people[0].id).toBe(PERSON_1_ID);
@ -55,16 +48,19 @@ describe('Core REST API Delete One endpoint', () => {
}); });
it('1a. should delete one person', async () => { it('1a. should delete one person', async () => {
const response = await makeRestAPIRequest( const response = await makeRestAPIRequest({
'delete', method: 'delete',
`/people/${PERSON_1_ID}`, path: `/people/${PERSON_1_ID}`,
); });
expect(response.body.data.deletePerson.id).toBe(PERSON_1_ID); expect(response.body.data.deletePerson.id).toBe(PERSON_1_ID);
}); });
it('1.b. should return a BadRequestException when trying to delete a non-existing person', async () => { it('1.b. should return a BadRequestException when trying to delete a non-existing person', async () => {
await makeRestAPIRequest('delete', `/people/${FAKE_PERSON_ID}`) await makeRestAPIRequest({
method: 'delete',
path: `/people/${FAKE_PERSON_ID}`,
})
.expect(400) .expect(400)
.expect((res) => { .expect((res) => {
expect(res.body.messages[0]).toContain( expect(res.body.messages[0]).toContain(
@ -75,8 +71,12 @@ describe('Core REST API Delete One endpoint', () => {
}); });
it('1.c. should return an UnauthorizedException when no token is provided', async () => { it('1.c. should return an UnauthorizedException when no token is provided', async () => {
await makeRestAPIRequest('delete', `/people/${PERSON_1_ID}`, { await makeRestAPIRequest({
authorization: '', method: 'delete',
path: `/people/${PERSON_1_ID}`,
headers: {
authorization: '',
},
}) })
.expect(401) .expect(401)
.expect((res) => { .expect((res) => {
@ -85,8 +85,12 @@ describe('Core REST API Delete One endpoint', () => {
}); });
it('1.d. should return an UnauthorizedException when an invalid token is provided', async () => { it('1.d. should return an UnauthorizedException when an invalid token is provided', async () => {
await makeRestAPIRequest('delete', `/people/${PERSON_1_ID}`, { await makeRestAPIRequest({
authorization: 'Bearer invalid-token', method: 'delete',
path: `/people/${PERSON_1_ID}`,
headers: {
authorization: 'Bearer invalid-token',
},
}) })
.expect(401) .expect(401)
.expect((res) => { .expect((res) => {
@ -95,8 +99,12 @@ describe('Core REST API Delete One endpoint', () => {
}); });
it('1.e. should return an UnauthorizedException when an expired token is provided', async () => { it('1.e. should return an UnauthorizedException when an expired token is provided', async () => {
await makeRestAPIRequest('delete', `/people/${PERSON_1_ID}`, { await makeRestAPIRequest({
authorization: `Bearer ${EXPIRED_ACCESS_TOKEN}`, method: 'delete',
path: `/people/${PERSON_1_ID}`,
headers: {
authorization: `Bearer ${EXPIRED_ACCESS_TOKEN}`,
},
}) })
.expect(401) .expect(401)
.expect((res) => { .expect((res) => {

View File

@ -4,15 +4,23 @@ import request from 'supertest';
export type RestAPIRequestMethod = 'get' | 'post' | 'put' | 'patch' | 'delete'; export type RestAPIRequestMethod = 'get' | 'post' | 'put' | 'patch' | 'delete';
export const makeRestAPIRequest = ( interface RestAPIRequestParams {
method: RestAPIRequestMethod, method: RestAPIRequestMethod;
path: string, path: string;
headers: IncomingHttpHeaders = {}, headers?: IncomingHttpHeaders;
) => { body?: any;
}
export const makeRestAPIRequest = ({
method,
path,
headers = {},
body,
}: RestAPIRequestParams) => {
const client = request(`http://localhost:${APP_PORT}`); const client = request(`http://localhost:${APP_PORT}`);
return client[method]('/rest' + path) return client[method](`/rest${path}`)
.set('Authorization', `Bearer ${ACCESS_TOKEN}`) .set('Authorization', `Bearer ${ACCESS_TOKEN}`)
.set({ ...headers }) .set(headers)
.send(); .send(body ? JSON.stringify(body) : undefined);
}; };