Skip to content

Commit

Permalink
Complete refactor of job configuration as NestJS modules
Browse files Browse the repository at this point in the history
- Developing new actions is documented in README.md for developers
- Move everything related to job config to src/config/job-config
- Major refactor of parsing from jobConfig.yaml to JobConfig objects
- Create module, interface, and factory for each JobAction
- Add handlebar-utils to help consistently applying templates to jobs
  and Dtos.
  - In test contexts the handlebars helpers may not be registered, so
    avoid using custom helpers or register them in the test
- Add options to LogJobAction. This is also good for templating tests
- Change MailService.sendMail to match nest's MailerService.sendMail
- Accept empty jobConfigurationFile as no jobs
- CaslModule now depends indirectly on the MailerModule. Thus all
  controllers need to either mock CaslAbilityFactory or add a mock
  MailerModule before importing CaslModule.
- Most tests disable jobConfig except for those that directly test it.
  These use test/config/jobconfig.yaml
  • Loading branch information
sbliven committed Dec 6, 2024
1 parent f63ed78 commit 4649326
Show file tree
Hide file tree
Showing 65 changed files with 1,157 additions and 790 deletions.
4 changes: 4 additions & 0 deletions src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import { EventEmitterModule } from "@nestjs/event-emitter";
import { AdminModule } from "./admin/admin.module";
import { HealthModule } from "./health/health.module";
import { LoggerModule } from "./loggers/logger.module";
import { JobConfigModule } from "./config/job-config/jobconfig.module";
import { DefaultJobActionFactories } from "./config/job-config/actions/defaultjobactions.module";

@Module({
imports: [
Expand All @@ -49,6 +51,8 @@ import { LoggerModule } from "./loggers/logger.module";
ConfigModule.forRoot({
load: [configuration],
}),
DefaultJobActionFactories,
JobConfigModule,
LoggerModule,
DatablocksModule,
DatasetsModule,
Expand Down
4 changes: 2 additions & 2 deletions src/auth/auth.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { OidcConfig } from "src/config/configuration";
import { BuildOpenIdClient, OidcStrategy } from "./strategies/oidc.strategy";
import { accessGroupServiceFactory } from "./access-group-provider/access-group-service-factory";
import { AccessGroupService } from "./access-group-provider/access-group.service";
import { CaslAbilityFactory } from "src/casl/casl-ability.factory";
import { CaslModule } from "src/casl/casl.module";

const OidcStrategyFactory = {
provide: "OidcStrategy",
Expand Down Expand Up @@ -56,14 +56,14 @@ const OidcStrategyFactory = {
property: "user",
session: false,
}),
CaslModule,
UsersModule,
],
providers: [
AuthService,
JwtStrategy,
LdapStrategy,
LocalStrategy,
CaslAbilityFactory,
OidcStrategyFactory,
accessGroupServiceFactory,
],
Expand Down
19 changes: 18 additions & 1 deletion src/casl/casl-ability.factory.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
import { MailerModule, MailerService } from "@nestjs-modules/mailer";
import { CaslAbilityFactory } from "./casl-ability.factory";
import { Test, TestingModule } from "@nestjs/testing";
import { CaslModule } from "./casl.module";

class MailerServiceMock {}

describe("CaslAbilityFactory", () => {
let casl: CaslAbilityFactory;
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
imports: [MailerModule.forRoot(), CaslModule],
})
.overrideProvider(MailerService)
.useClass(MailerServiceMock)
.compile();

casl = module.get<CaslAbilityFactory>(CaslAbilityFactory);
});

it("should be defined", () => {
expect(new CaslAbilityFactory()).toBeDefined();
expect(casl).toBeDefined();
});
});
15 changes: 10 additions & 5 deletions src/casl/casl-ability.factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ import { UserSettings } from "src/users/schemas/user-settings.schema";
import { User } from "src/users/schemas/user.schema";
import { Action } from "./action.enum";
import configuration from "src/config/configuration";
import { JobConfigService } from "src/config/job-config/jobconfig.service";
import {
CreateJobAuth,
StatusUpdateJobAuth,
} from "src/jobs/types/jobs-auth.enum";

import { JobConfig } from "src/jobs/config/jobconfig";
import { JobConfig } from "src/config/job-config/jobconfig.interface";

type Subjects =
| string
Expand Down Expand Up @@ -60,6 +61,10 @@ export type AppAbility = MongoAbility<PossibleAbilities, Conditions>;

@Injectable()
export class CaslAbilityFactory {
constructor(private jobConfigService: JobConfigService) {
//Logger.log(`Creating CaslAbilityFactory with ${jobConfigService ? Object.keys(jobConfigService.allJobConfigs).length : 0} job types`);
}

private endpointAccessors: {
[endpoint: string]: (user: JWTUser) => AppAbility;
} = {
Expand Down Expand Up @@ -356,7 +361,7 @@ export class CaslAbilityFactory {

// job creation
if (
configuration().jobConfiguration.some(
Object.values(this.jobConfigService.allJobConfigs).some(
(j) => j.create.auth == CreateJobAuth.All,
)
) {
Expand All @@ -366,7 +371,7 @@ export class CaslAbilityFactory {
}
cannot(Action.JobRead, JobClass);
if (
configuration().jobConfiguration.some(
Object.values(this.jobConfigService.allJobConfigs).some(
(j) => j.statusUpdate.auth == StatusUpdateJobAuth.All,
)
) {
Expand Down Expand Up @@ -425,7 +430,7 @@ export class CaslAbilityFactory {
can(Action.JobRead, JobClass);

if (
configuration().jobConfiguration.some(
Object.values(this.jobConfigService.allJobConfigs).some(
(j) =>
j.create.auth &&
jobCreateEndPointAuthorizationValues.includes(
Expand All @@ -448,7 +453,7 @@ export class CaslAbilityFactory {
can(Action.JobStatusUpdate, JobClass);
} else {
if (
configuration().jobConfiguration.some(
Object.values(this.jobConfigService.allJobConfigs).some(
(j) =>
j.statusUpdate.auth &&
jobUpdateEndPointAuthorizationValues.includes(
Expand Down
3 changes: 2 additions & 1 deletion src/casl/casl.module.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Module } from "@nestjs/common";
import { CaslAbilityFactory } from "./casl-ability.factory";

import { JobConfigModule } from "src/config/job-config/jobconfig.module";
@Module({
imports: [JobConfigModule],
providers: [CaslAbilityFactory],
exports: [CaslAbilityFactory],
})
Expand Down
10 changes: 10 additions & 0 deletions src/common/handlebars-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,13 @@ export const urlencode = (context: string): string => {
export const base64enc = (context: string): string => {
return btoa(context);
};

export const handlebarsHelpers = {
unwrapJSON: unwrapJSON,
keyToWord: formatCamelCase,
eq: (a: unknown, b: unknown) => a === b,
jsonify: jsonify,
job_v3: job_v3,
urlencode: urlencode,
base64enc: base64enc,
};
31 changes: 14 additions & 17 deletions src/common/mail.service.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,25 @@
import { MailerService } from "@nestjs-modules/mailer";
import { ISendMailOptions, MailerService } from "@nestjs-modules/mailer";
import { Injectable, Logger } from "@nestjs/common";
import { SentMessageInfo } from "nodemailer";

/**
* Service for sending emails using nodemailer.
*
* Use this rather than MailerService directly to allow configuration in AppModule
*/
@Injectable()
export class MailService {
constructor(private readonly mailerService: MailerService) {}

async sendMail(
to: string,
cc: string,
subject: string,
mailText: string | null,
html: string | null = null,
) {
async sendMail(options: ISendMailOptions): Promise<SentMessageInfo> {
try {
Logger.log("Sending email to: " + to, "Utils.sendMail");
await this.mailerService.sendMail({
to,
...(cc && { cc }),
...(subject && { subject }),
...(html && { html }),
...(mailText && { mailText }),
});
Logger.log("Sending email to: " + options.to, "Utils.sendMail");
await this.mailerService.sendMail(options);
} catch (error) {
Logger.error("Failed sending email to: " + to, "MailService.sendMail");
Logger.error(
"Failed sending email to: " + options.to,
"MailService.sendMail",
);
Logger.error(error, "MailService.sendMail");
}
}
Expand Down
34 changes: 2 additions & 32 deletions src/config/configuration.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,7 @@
import {
loadJobConfig,
registerCreateAction,
registerStatusUpdateAction,
} from "../jobs/config/jobconfig";
import { LogJobAction } from "../jobs/actions/logaction";
import { EmailJobAction } from "../jobs/actions/emailaction";
import { URLAction } from "src/jobs/actions/urlaction";
import { RabbitMQJobAction } from "src/jobs/actions/rabbitmqaction";
import * as fs from "fs";
import { merge } from "lodash";
import localconfiguration from "./localconfiguration";
import { boolean } from "mathjs";
import { ValidateAction } from "src/jobs/actions/validateaction";
import { DEFAULT_PROPOSAL_TYPE } from "src/proposals/schemas/proposal.schema";

const configuration = () => {
Expand Down Expand Up @@ -46,7 +36,7 @@ const configuration = () => {
process.env.OIDC_USERINFO_MAPPING_FIELD_USERNAME || ("" as string);

const jobConfigurationFile =
process.env.JOB_CONFIGURATION_FILE || ("jobConfig.yaml" as string);
process.env.JOB_CONFIGURATION_FILE || ("" as string);

const defaultLogger = {
type: "DefaultLogger",
Expand Down Expand Up @@ -75,9 +65,6 @@ const configuration = () => {
}
});

registerDefaultActions();
const job_configs = loadJobConfig(jobConfigurationFile);

// NOTE: Add the default proposal type here
Object.assign(jsonConfigMap.proposalTypes, {
DefaultProposal: DEFAULT_PROPOSAL_TYPE,
Expand All @@ -89,7 +76,7 @@ const configuration = () => {
api: "3",
},
swaggerPath: process.env.SWAGGER_PATH || "explorer",
jobConfiguration: job_configs,
jobConfigurationFile: jobConfigurationFile,
loggerConfigs: jsonConfigMap.loggers || [defaultLogger],
adminGroups: adminGroups.split(",").map((v) => v.trim()) ?? [],
deleteGroups: deleteGroups.split(",").map((v) => v.trim()) ?? [],
Expand Down Expand Up @@ -232,23 +219,6 @@ const configuration = () => {
return merge(config, localconfiguration);
};

/**
* Registers built-in JobActions. Should be called exactly once.
*/
export function registerDefaultActions() {
// Create
registerCreateAction(LogJobAction);
registerCreateAction(EmailJobAction);
registerCreateAction(URLAction);
registerCreateAction(RabbitMQJobAction);
registerCreateAction(ValidateAction);
// Status Update
registerStatusUpdateAction(LogJobAction);
registerStatusUpdateAction(EmailJobAction);
registerStatusUpdateAction(RabbitMQJobAction);
registerStatusUpdateAction(ValidateAction);
}

export type OidcConfig = ReturnType<typeof configuration>["oidc"];

export default configuration;
33 changes: 33 additions & 0 deletions src/config/job-config/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Job Configuration

## Background

Jobs are SciCat's main way of interacting with external systems. Thus the actions which
should be performed when a job is created or updated (with POST or PATCH requests; see
[job.controller.ts](../../jobs/job.controller.ts)) tend to be facility specific. To
facilitate this, the allowed jobs are configured in a YAML or JSON file. An example file
is available at [jobConfig.example.yaml](../../../jobConfig.example.yaml). The location
of the job config file is configurable in with the `JOB_CONFIGURATION_FILE` environment
variable.

The file is parsed when the application starts and converted to a `JobConfigService`
instance. Job types are arbitrary and facility-specific, but `archive` and `retrieve`
are traditional for interacting with the data storage system. Authorization can be
configured for each job type for *create* and *update* requests, and then a list of
actions are provided which should run after the request.

## Implementing an Action

Implementing an Action requires four (short) files:
1. `action.ts` contains a class implementing `JobAction`. It's constructor can take any arguments, but the existing actions take an `options` argument mirroring the expected yaml config. It does not need to be `@Injectable()`, since it is constructed by the factory.
2. `action.interface.ts` can contain additional types, e.g. the definition of the expected `options` and a type guard for casting to the options.
3. `action.factory.ts` implements `JobActionFactory`. The factory is provided by NestJS as a singleton, so it must be `@Injectable()`. This means that dependencies can be injected into the factory. It has a `create(options)` method, which constructs the action itself by combining the options from the yaml file with any dependencies injected by NestJS.
4. `action.module.ts` is an NestJS module that provides the factory.

The lists of known factories are provided to Nest with the `CREATE_JOB_ACTION_FACTORIES` and `UPDATE_JOB_ACTION_FACTORIES` symbols. The top-level AppModule imports built-in actions from `DefaultJobActionFactories`. Core actions should be added to this list. Plugins can use the NestJS dependency injection system to extend the lists if needed.

## Accessing the jobConfig

Parsing `jobConfig.yaml` is handled by the JobConfigService. NestJS injects the lists of factories (and the file path) during construction. When parsing reaches a new action, the correct factory is used to create an instance of the JobAction with the current list of options.

Code which needs the configuration for a particular job type (eg jobs.controller.ts) injects the `JobConfigService` and can then call `jobConfigService.get(jobType)`.
90 changes: 90 additions & 0 deletions src/config/job-config/actions/defaultjobactions.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { Module } from "@nestjs/common";
import { LogJobActionFactory } from "./logaction/logaction.factory";
import { LogJobActionModule } from "./logaction/logaction.module";
import { EmailJobActionFactory } from "./emailaction/emailaction.factory";
import { EmailJobActionModule } from "./emailaction/emailaction.module";
import { actionType as logActionType } from "./logaction/logaction.interface";
import { actionType as emailActionType } from "./emailaction/emailaction.interface";
import { ValidateJobActionModule } from "./validateaction/validateaction.module";
import { actionType as validateActionType } from "./validateaction/validateaction.interface";
import { ValidateJobActionFactory } from "./validateaction/validateaction.factory";
import { URLJobActionModule } from "./urlaction/urlaction.module";
import { URLJobActionFactory } from "./urlaction/urlaction.factory";
import { actionType as urlActionType } from "./urlaction/urlaction.interface";
import { RabbitMQJobActionModule } from "./rabbitmqaction/rabbitmqaction.module";
import { actionType as rabbitmqActionType } from "./rabbitmqaction/rabbitmqaction.interface";
import { RabbitMQJobActionFactory } from "./rabbitmqaction/rabbitmqaction.factory";
import {
CREATE_JOB_ACTION_FACTORIES,
UPDATE_JOB_ACTION_FACTORIES,
} from "../jobconfig.interface";

/**
* Provide a list of built-in job action factories.
*
* CREATE_JOB_ACTION_FACTORIES and UPDATE_JOB_ACTION_FACTORIES be extended (eg by a
* plugin) with additional factories.
*/
@Module({
imports: [
EmailJobActionModule,
LogJobActionModule,
ValidateJobActionModule,
URLJobActionModule,
RabbitMQJobActionModule,
],
providers: [
{
provide: CREATE_JOB_ACTION_FACTORIES,
useFactory: (
logJobActionFactory,
emailJobActionFactory,
validateJobActionFactory,
urlJobActionFactory,
rabbitMQJobActionFactory,
) => {
return {
[logActionType]: logJobActionFactory,
[emailActionType]: emailJobActionFactory,
[validateActionType]: validateJobActionFactory,
[urlActionType]: urlJobActionFactory,
[rabbitmqActionType]: rabbitMQJobActionFactory,
};
},
inject: [
LogJobActionFactory,
EmailJobActionFactory,
ValidateJobActionFactory,
URLJobActionFactory,
RabbitMQJobActionFactory,
],
},
{
provide: UPDATE_JOB_ACTION_FACTORIES,
useFactory: (
logJobActionFactory,
emailJobActionFactory,
validateJobActionFactory,
urlJobActionFactory,
rabbitMQJobActionFactory,
) => {
return {
[logActionType]: logJobActionFactory,
[emailActionType]: emailJobActionFactory,
[validateActionType]: validateJobActionFactory,
[urlActionType]: urlJobActionFactory,
[rabbitmqActionType]: rabbitMQJobActionFactory,
};
},
inject: [
LogJobActionFactory,
EmailJobActionFactory,
ValidateJobActionFactory,
URLJobActionFactory,
RabbitMQJobActionFactory,
],
},
],
exports: [CREATE_JOB_ACTION_FACTORIES, UPDATE_JOB_ACTION_FACTORIES],
})
export class DefaultJobActionFactories {}
Loading

0 comments on commit 4649326

Please sign in to comment.