Skip to content

Commit

Permalink
use validation with transformer for mooclet policy params
Browse files Browse the repository at this point in the history
  • Loading branch information
danoswaltCL committed Nov 7, 2024
1 parent caf1c0a commit be1c720
Show file tree
Hide file tree
Showing 13 changed files with 2,798 additions and 4,767 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
Req,
QueryParams,
Params,
UseBefore,
} from 'routing-controllers';
import { Experiment } from '../models/Experiment';
import { ExperimentNotFoundError } from '../errors/ExperimentNotFoundError';
Expand All @@ -26,10 +27,10 @@ import { ExperimentDTO, ExperimentFile, ValidatedExperimentError } from '../DTO/
import { ExperimentIds } from './validators/ExperimentIdsValidator';
import { MoocletExperimentService } from '../services/MoocletExperimentService';
import { env } from '../../env';
import { UnprocessableEntityException } from '@nestjs/common';
import { MoocletExperimentDTO } from '../DTO/MoocletExperimentDTO';
import { NotFoundException } from '@nestjs/common/exceptions';
import { ExperimentIdValidator } from '../DTO/ExperimentDTO';
import { ValidateMoocletPolicyParametersMiddleware } from '../middlewares/ValidateMoocletPolicyParameters';

interface ExperimentPaginationInfo extends PaginationResponse {
nodes: Experiment[];
Expand Down Expand Up @@ -958,28 +959,23 @@ export class ExperimentController {
*/

@Post()
@UseBefore(ValidateMoocletPolicyParametersMiddleware)
public create(
@Body({ validate: true }) experiment: ExperimentDTO | MoocletExperimentDTO,
@CurrentUser() currentUser: UserDTO,
@Req() request: AppRequest
): Promise<ExperimentDTO | MoocletExperimentDTO> {
request.logger.child({ user: currentUser });

// Manually check if the experiment is a MoocletExperimentDTO
if ('moocletPolicyParameters' in experiment) {
if (!env.mooclets.enabled) {
throw new UnprocessableEntityException(
'Failed to create Experiment: moocletPolicyParameters was provided but mooclets are not enabled.'
);
}
return this.moocletExperimentService.syncCreate({
experimentDTO: experiment,
currentUser,
logger: request.logger,
});
}

return this.experimentService.create(experiment as ExperimentDTO, currentUser, request.logger);
return this.experimentService.create(experiment, currentUser, request.logger);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Request, Response, NextFunction } from 'express';
import { BadRequestError, ExpressMiddlewareInterface, Middleware } from 'routing-controllers';
import { env } from '../../env';
import { UnprocessableEntityException } from '@nestjs/common';
import { validateMoocletPolicyParameters } from '../validators/MoocletPolicyParametersFactory';

@Middleware({ type: 'before' })
export class ValidateMoocletPolicyParametersMiddleware implements ExpressMiddlewareInterface {
public async use(req: Request, res: Response, next: NextFunction): Promise<void> {
const experiment = req.body;

if (!env.mooclets.enabled && 'moocletPolicyParameters' in experiment) {
throw new UnprocessableEntityException(
'Failed to create Experiment: moocletPolicyParameters was provided but mooclets are not enabled on backend.'
);
}

if (env.mooclets.enabled) {
try {
const policyParameters = await validateMoocletPolicyParameters(
experiment.assignmentAlgorithm,
experiment.moocletPolicyParameters
);
req.body.moocletPolicyParameters = policyParameters;
} catch (error) {
throw new BadRequestError(`Validation failed: ${error}`);
}
}

next();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { plainToInstance, Type, Transform } from 'class-transformer';
import { validate } from 'class-validator';
import { ASSIGNMENT_ALGORITHM, MOOCLET_POLICY_SCHEMA_MAP } from 'upgrade_types';
export class MoocletPolicyParametersDTO {
@Transform(({ value: policyParameters, obj: experiment }) => {
const policyParameterSchema = MOOCLET_POLICY_SCHEMA_MAP[experiment.assignmentAlgorithm];
if (!policyParameterSchema) {
throw new Error('Invalid assignment algorithm: ' + experiment.assignmentAlgorithm);
}
return plainToInstance(policyParameterSchema, policyParameters);
})
@Type(() => Object)
moocletPolicyParameters: any;
}

export async function validateMoocletPolicyParameters(assignmentAlgorithm: ASSIGNMENT_ALGORITHM, moocletPolicyParameters: any) {
const moocletPolicyParametersDTO = plainToInstance(MoocletPolicyParametersDTO, { assignmentAlgorithm, moocletPolicyParameters });
const errors = await validate(moocletPolicyParametersDTO.moocletPolicyParameters);

if (errors.length > 0) {
throw new Error(`Policy Parameter validation failed: ${errors}`);
}

return moocletPolicyParametersDTO.moocletPolicyParameters;
}
3 changes: 1 addition & 2 deletions backend/packages/Upgrade/src/app.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import 'reflect-metadata';
import { UpgradeLogger } from './lib/logger/UpgradeLogger';
import { env } from './env';

if (env.useNewRelic) {
require('newrelic/index');
}

import 'reflect-metadata';

import { bootstrapMicroframework } from 'microframework';
import { expressLoader } from './loaders/expressLoader';
import { winstonLoader } from './loaders/winstonLoader';
Expand Down
Loading

0 comments on commit be1c720

Please sign in to comment.