Feat/improve error management in core module (#8933)
## Summary This Pull Request introduces a custom validator for checking forbidden words in workspaces and refines how exceptions are handled within the workspace module. - Introduced `ForbiddenWords` custom class validator for validating forbidden words against specific fields in `UpdateWorkspaceInput`. - Added `EnvironmentService` usage in `WorkspaceService` to check default subdomains. - New file `workspaceGraphqlApiExceptionHandler` to handle GraphQL API exceptions with specific error mappings. - Expanded `WorkspaceExceptionCode` with `SUBDOMAIN_ALREADY_TAKEN`. - Added new unit tests for validating forbidden words and exception handler behavior.
This commit is contained in:
@ -2,6 +2,8 @@ import { Field, InputType } from '@nestjs/graphql';
|
|||||||
|
|
||||||
import { IsBoolean, IsOptional, IsString, Matches } from 'class-validator';
|
import { IsBoolean, IsOptional, IsString, Matches } from 'class-validator';
|
||||||
|
|
||||||
|
import { ForbiddenWords } from 'src/engine/utils/custom-class-validator/ForbiddenWords';
|
||||||
|
|
||||||
@InputType()
|
@InputType()
|
||||||
export class UpdateWorkspaceInput {
|
export class UpdateWorkspaceInput {
|
||||||
@Field({ nullable: true })
|
@Field({ nullable: true })
|
||||||
@ -13,6 +15,7 @@ export class UpdateWorkspaceInput {
|
|||||||
@IsString()
|
@IsString()
|
||||||
@IsOptional()
|
@IsOptional()
|
||||||
@Matches(/^[a-z0-9][a-z0-9-]{1,28}[a-z0-9]$/)
|
@Matches(/^[a-z0-9][a-z0-9-]{1,28}[a-z0-9]$/)
|
||||||
|
@ForbiddenWords(['demo'])
|
||||||
subdomain?: string;
|
subdomain?: string;
|
||||||
|
|
||||||
@Field({ nullable: true })
|
@Field({ nullable: true })
|
||||||
|
|||||||
@ -23,7 +23,7 @@ import {
|
|||||||
WorkspaceExceptionCode,
|
WorkspaceExceptionCode,
|
||||||
} from 'src/engine/core-modules/workspace/workspace.exception';
|
} from 'src/engine/core-modules/workspace/workspace.exception';
|
||||||
import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.validate';
|
import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.validate';
|
||||||
import { ConflictError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
// eslint-disable-next-line @nx/workspace-inject-workspace-repository
|
// eslint-disable-next-line @nx/workspace-inject-workspace-repository
|
||||||
@ -39,6 +39,7 @@ export class WorkspaceService extends TypeOrmQueryService<Workspace> {
|
|||||||
private readonly featureFlagService: FeatureFlagService,
|
private readonly featureFlagService: FeatureFlagService,
|
||||||
private readonly billingSubscriptionService: BillingSubscriptionService,
|
private readonly billingSubscriptionService: BillingSubscriptionService,
|
||||||
private readonly userWorkspaceService: UserWorkspaceService,
|
private readonly userWorkspaceService: UserWorkspaceService,
|
||||||
|
private readonly environmentService: EnvironmentService,
|
||||||
) {
|
) {
|
||||||
super(workspaceRepository);
|
super(workspaceRepository);
|
||||||
}
|
}
|
||||||
@ -61,8 +62,14 @@ export class WorkspaceService extends TypeOrmQueryService<Workspace> {
|
|||||||
payload.subdomain,
|
payload.subdomain,
|
||||||
);
|
);
|
||||||
|
|
||||||
if (!subdomainAvailable) {
|
if (
|
||||||
throw new ConflictError('Subdomain already taken');
|
!subdomainAvailable ||
|
||||||
|
this.environmentService.get('DEFAULT_SUBDOMAIN') === payload.subdomain
|
||||||
|
) {
|
||||||
|
throw new WorkspaceException(
|
||||||
|
'Subdomain already taken',
|
||||||
|
WorkspaceExceptionCode.SUBDOMAIN_ALREADY_TAKEN,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -0,0 +1,63 @@
|
|||||||
|
import {
|
||||||
|
ConflictError,
|
||||||
|
InternalServerError,
|
||||||
|
NotFoundError,
|
||||||
|
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||||
|
import {
|
||||||
|
WorkspaceException,
|
||||||
|
WorkspaceExceptionCode,
|
||||||
|
} from 'src/engine/core-modules/workspace/workspace.exception';
|
||||||
|
|
||||||
|
import { workspaceGraphqlApiExceptionHandler } from './workspaceGraphqlApiExceptionHandler';
|
||||||
|
|
||||||
|
describe('workspaceGraphqlApiExceptionHandler', () => {
|
||||||
|
it('should throw NotFoundError when WorkspaceExceptionCode is SUBDOMAIN_NOT_FOUND', () => {
|
||||||
|
const error = new WorkspaceException(
|
||||||
|
'Subdomain not found',
|
||||||
|
WorkspaceExceptionCode.SUBDOMAIN_NOT_FOUND,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(() => workspaceGraphqlApiExceptionHandler(error)).toThrow(
|
||||||
|
NotFoundError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should throw NotFoundError when WorkspaceExceptionCode is WORKSPACE_NOT_FOUND', () => {
|
||||||
|
const error = new WorkspaceException(
|
||||||
|
'Workspace not found',
|
||||||
|
WorkspaceExceptionCode.WORKSPACE_NOT_FOUND,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(() => workspaceGraphqlApiExceptionHandler(error)).toThrow(
|
||||||
|
NotFoundError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should throw ConflictError when WorkspaceExceptionCode is SUBDOMAIN_ALREADY_TAKEN', () => {
|
||||||
|
const error = new WorkspaceException(
|
||||||
|
'Subdomain already taken',
|
||||||
|
WorkspaceExceptionCode.SUBDOMAIN_ALREADY_TAKEN,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(() => workspaceGraphqlApiExceptionHandler(error)).toThrow(
|
||||||
|
ConflictError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should throw InternalServerError for unknown WorkspaceExceptionCode', () => {
|
||||||
|
// @ts-expect-error - should never happen but it allow to test the default case
|
||||||
|
const error = new WorkspaceException('Unknown error', 'UNKNOWN_CODE');
|
||||||
|
|
||||||
|
expect(() => workspaceGraphqlApiExceptionHandler(error)).toThrow(
|
||||||
|
InternalServerError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should throw the original error if it is not a WorkspaceException', () => {
|
||||||
|
const genericError = new Error('Generic error');
|
||||||
|
|
||||||
|
expect(() => workspaceGraphqlApiExceptionHandler(genericError)).toThrow(
|
||||||
|
genericError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -0,0 +1,25 @@
|
|||||||
|
import {
|
||||||
|
ConflictError,
|
||||||
|
InternalServerError,
|
||||||
|
NotFoundError,
|
||||||
|
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||||
|
import {
|
||||||
|
WorkspaceException,
|
||||||
|
WorkspaceExceptionCode,
|
||||||
|
} from 'src/engine/core-modules/workspace/workspace.exception';
|
||||||
|
|
||||||
|
export const workspaceGraphqlApiExceptionHandler = (error: Error) => {
|
||||||
|
if (error instanceof WorkspaceException) {
|
||||||
|
switch (error.code) {
|
||||||
|
case WorkspaceExceptionCode.SUBDOMAIN_NOT_FOUND:
|
||||||
|
case WorkspaceExceptionCode.WORKSPACE_NOT_FOUND:
|
||||||
|
throw new NotFoundError(error.message);
|
||||||
|
case WorkspaceExceptionCode.SUBDOMAIN_ALREADY_TAKEN:
|
||||||
|
throw new ConflictError(error.message);
|
||||||
|
default:
|
||||||
|
throw new InternalServerError(error.message);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
throw error;
|
||||||
|
};
|
||||||
@ -8,5 +8,6 @@ export class WorkspaceException extends CustomException {
|
|||||||
|
|
||||||
export enum WorkspaceExceptionCode {
|
export enum WorkspaceExceptionCode {
|
||||||
SUBDOMAIN_NOT_FOUND = 'SUBDOMAIN_NOT_FOUND',
|
SUBDOMAIN_NOT_FOUND = 'SUBDOMAIN_NOT_FOUND',
|
||||||
|
SUBDOMAIN_ALREADY_TAKEN = 'SUBDOMAIN_ALREADY_TAKEN',
|
||||||
WORKSPACE_NOT_FOUND = 'WORKSPACE_NOT_FOUND',
|
WORKSPACE_NOT_FOUND = 'WORKSPACE_NOT_FOUND',
|
||||||
}
|
}
|
||||||
|
|||||||
@ -40,6 +40,7 @@ import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.
|
|||||||
import { LoginTokenService } from 'src/engine/core-modules/auth/token/services/login-token.service';
|
import { LoginTokenService } from 'src/engine/core-modules/auth/token/services/login-token.service';
|
||||||
import { DomainManagerService } from 'src/engine/core-modules/domain-manager/service/domain-manager.service';
|
import { DomainManagerService } from 'src/engine/core-modules/domain-manager/service/domain-manager.service';
|
||||||
import { getAuthProvidersByWorkspace } from 'src/engine/core-modules/workspace/utils/getAuthProvidersByWorkspace';
|
import { getAuthProvidersByWorkspace } from 'src/engine/core-modules/workspace/utils/getAuthProvidersByWorkspace';
|
||||||
|
import { workspaceGraphqlApiExceptionHandler } from 'src/engine/core-modules/workspace/utils/workspaceGraphqlApiExceptionHandler';
|
||||||
|
|
||||||
import { Workspace } from './workspace.entity';
|
import { Workspace } from './workspace.entity';
|
||||||
|
|
||||||
@ -91,10 +92,14 @@ export class WorkspaceResolver {
|
|||||||
@Args('data') data: UpdateWorkspaceInput,
|
@Args('data') data: UpdateWorkspaceInput,
|
||||||
@AuthWorkspace() workspace: Workspace,
|
@AuthWorkspace() workspace: Workspace,
|
||||||
) {
|
) {
|
||||||
return this.workspaceService.updateWorkspaceById({
|
try {
|
||||||
...data,
|
return this.workspaceService.updateWorkspaceById({
|
||||||
id: workspace.id,
|
...data,
|
||||||
});
|
id: workspace.id,
|
||||||
|
});
|
||||||
|
} catch (error) {
|
||||||
|
workspaceGraphqlApiExceptionHandler(error);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Mutation(() => String)
|
@Mutation(() => String)
|
||||||
|
|||||||
@ -0,0 +1,38 @@
|
|||||||
|
import { validate } from 'class-validator';
|
||||||
|
|
||||||
|
import { ForbiddenWords } from 'src/engine/utils/custom-class-validator/ForbiddenWords';
|
||||||
|
|
||||||
|
describe('ForbiddenWordsConstraint', () => {
|
||||||
|
test('should throw error when word is forbidden', async () => {
|
||||||
|
class Test {
|
||||||
|
@ForbiddenWords(['forbidden', 'restricted'])
|
||||||
|
subdomain: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
const instance = new Test();
|
||||||
|
|
||||||
|
instance.subdomain = 'forbidden';
|
||||||
|
|
||||||
|
const errors = await validate(instance);
|
||||||
|
|
||||||
|
expect(errors.length).toBeGreaterThan(0);
|
||||||
|
expect(errors[0].constraints).toEqual({
|
||||||
|
ForbiddenWordsConstraint: 'forbidden, restricted are not allowed',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should pass validation word is not in the list', async () => {
|
||||||
|
class Test {
|
||||||
|
@ForbiddenWords(['forbidden', 'restricted'])
|
||||||
|
subdomain: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
const instance = new Test();
|
||||||
|
|
||||||
|
instance.subdomain = 'valid';
|
||||||
|
|
||||||
|
const errors = await validate(instance);
|
||||||
|
|
||||||
|
expect(errors.length).toEqual(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -0,0 +1,39 @@
|
|||||||
|
import {
|
||||||
|
registerDecorator,
|
||||||
|
ValidationArguments,
|
||||||
|
ValidationOptions,
|
||||||
|
ValidatorConstraint,
|
||||||
|
ValidatorConstraintInterface,
|
||||||
|
} from 'class-validator';
|
||||||
|
|
||||||
|
@ValidatorConstraint({ async: false })
|
||||||
|
export class ForbiddenWordsConstraint implements ValidatorConstraintInterface {
|
||||||
|
private forbiddenWords: Set<string>;
|
||||||
|
|
||||||
|
constructor() {}
|
||||||
|
|
||||||
|
validate(value: string, validationArguments: ValidationArguments) {
|
||||||
|
this.forbiddenWords = new Set(validationArguments.constraints[0]);
|
||||||
|
|
||||||
|
return !this.forbiddenWords.has(value);
|
||||||
|
}
|
||||||
|
|
||||||
|
defaultMessage() {
|
||||||
|
return `${Array.from(this.forbiddenWords).join(', ')} are not allowed`;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export function ForbiddenWords(
|
||||||
|
forbiddenWords: string[],
|
||||||
|
validationOptions?: ValidationOptions,
|
||||||
|
) {
|
||||||
|
return function (object: object, propertyName: string) {
|
||||||
|
registerDecorator({
|
||||||
|
target: object.constructor,
|
||||||
|
propertyName: propertyName,
|
||||||
|
options: validationOptions,
|
||||||
|
constraints: [forbiddenWords],
|
||||||
|
validator: ForbiddenWordsConstraint,
|
||||||
|
});
|
||||||
|
};
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user