refactor and add comments

apps/app/src/features/page-bulk-export/server/service/page-bulk-export-job-cron.ts

@@ -11,6 +11,9 @@ import { pageBulkExportService } from './page-bulk-export';
 
 const logger = loggerFactory('growi:service:cron');
 
+/**
+ * Manages cronjob which deletes unnecessary bulk export jobs
+ */
 class PageBulkExportJobCronService extends CronService {
 
   crowi: any;
@@ -22,10 +25,13 @@ class PageBulkExportJobCronService extends CronService {
 
   override async executeJob(): Promise<void> {
     await this.deleteExpiredExportJobs();
-    await this.deleteDownloadExpiredJobs();
+    await this.deleteDownloadExpiredExportJobs();
     await this.deleteFailedExportJobs();
   }
 
+  /**
+   * Delete bulk export jobs which are on-going and has passed the limit time for execution
+   */
   async deleteExpiredExportJobs() {
     const exportJobExpirationSeconds = configManager.getConfig('crowi', 'app:bulkExportJobExpirationSeconds');
     const expiredExportJobs = await PageBulkExportJob.find({
@@ -45,7 +51,10 @@ class PageBulkExportJobCronService extends CronService {
     }
   }
 
-  async deleteDownloadExpiredJobs() {
+  /**
+   * Delete bulk export jobs which have completed but the due time for downloading has passed
+   */
+  async deleteDownloadExpiredExportJobs() {
     const downloadExpirationSeconds = configManager.getConfig('crowi', 'app:bulkExportDownloadExpirationSeconds');
     const downloadExpiredExportJobs = await PageBulkExportJob.find({
       status: PageBulkExportJobStatus.completed,
@@ -63,6 +72,9 @@ class PageBulkExportJobCronService extends CronService {
     }
   }
 
+  /**
+   * Delete bulk export jobs which have failed
+   */
   async deleteFailedExportJobs() {
     const failedExportJobs = await PageBulkExportJob.find({ status: PageBulkExportJobStatus.failed });
     for (const failedExportJob of failedExportJobs) {

apps/app/src/features/page-bulk-export/server/service/page-bulk-export.ts

@@ -140,6 +140,7 @@ class PageBulkExportService {
    * Notify the user of the export result, and cleanup the resources used in the export process
    * @param succeeded whether the export was successful
    * @param pageBulkExportJob the page bulk export job
+   * @param activityParameters parameters to record user activity
    */
   private async notifyExportResultAndCleanUp(
       succeeded: boolean,
@@ -353,7 +354,7 @@ class PageBulkExportService {
     return `${this.tmpOutputRootDir}/${pageBulkExportJob._id}`;
   }
 
-  private async notifyExportResult(
+  async notifyExportResult(
       pageBulkExportJob: PageBulkExportJobDocument, action: SupportedActionType, activityParameters?: ActivityParameters,
   ) {
     const activity = await this.crowi.activityService.createActivity({

apps/app/src/features/questionnaire/server/service/questionnaire-cron.integ.ts

@@ -140,6 +140,8 @@ describe('QuestionnaireCronService', () => {
 
   beforeAll(async() => {
     await configManager.loadConfigs();
+    await configManager.updateConfigsInTheSameNamespace('crowi', { 'app:questionnaireCronMaxHoursUntilRequest': 0 });
+
     await User.create({
       name: 'Example for Questionnaire Service Test',
       username: 'questionnaire cron test user',

apps/app/src/features/questionnaire/server/service/questionnaire-cron.ts

@@ -16,7 +16,7 @@ const axios = require('axios').default;
 axiosRetry(axios, { retries: 3 });
 
 /**
- * manage cronjob which
+ * Manages cronjob which
  *  1. fetches QuestionnaireOrders from questionnaire server
  *  2. updates QuestionnaireOrder collection to contain only the ones that exist in the fetched list and is not finished (doesn't have to be started)
  *  3. changes QuestionnaireAnswerStatuses which are 'skipped' to 'not_answered'
@@ -27,13 +27,13 @@ class QuestionnaireCronService extends CronService {
   sleep = (msec: number): Promise<void> => new Promise(resolve => setTimeout(resolve, msec));
 
   override startCron(cronSchedule: string): void {
-    const maxHoursUntilRequest = configManager.getConfig('crowi', 'app:questionnaireCronMaxHoursUntilRequest');
-    const maxSecondsUntilRequest = maxHoursUntilRequest * 60 * 60;
-
-    super.startCron(cronSchedule, this.getPreExecute(maxSecondsUntilRequest));
+    super.startCron(cronSchedule);
   }
 
   override async executeJob(): Promise<void> {
+    // sleep for a random amount to scatter request time from GROWI apps to questionnaire server
+    await this.sleepBeforeJob();
+
     const questionnaireServerOrigin = configManager.getConfig('crowi', 'app:questionnaireServerOrigin');
 
     const fetchQuestionnaireOrders = async(): Promise<IQuestionnaireOrder[]> => {
@@ -81,12 +81,12 @@ class QuestionnaireCronService extends CronService {
     await changeSkippedAnswerStatusToNotAnswered();
   }
 
-  private getPreExecute(maxSecondsUntilRequest: number): () => Promise<void> {
-    return async() => {
-      // sleep for a random amount to scatter request time from GROWI apps to questionnaire server
-      const secToSleep = getRandomIntInRange(0, maxSecondsUntilRequest);
-      await this.sleep(secToSleep * 1000);
-    };
+  private async sleepBeforeJob() {
+    const maxHoursUntilRequest = configManager.getConfig('crowi', 'app:questionnaireCronMaxHoursUntilRequest');
+    const maxSecondsUntilRequest = maxHoursUntilRequest * 60 * 60;
+
+    const secToSleep = getRandomIntInRange(0, maxSecondsUntilRequest);
+    await this.sleep(secToSleep * 1000);
   }
 
 }

apps/app/src/server/service/cron.ts

@@ -5,27 +5,44 @@ import loggerFactory from '~/utils/logger';
 
 const logger = loggerFactory('growi:service:cron');
 
+/**
+ * Base class for services that manage a cronjob
+ */
 abstract class CronService {
 
+  // The current cronjob to manage
   cronJob: ScheduledTask;
 
-  startCron(cronSchedule: string, preExecute?: () => void): void {
+  /**
+   * Create and start a new cronjob
+   * @param cronSchedule e.g. '0 1 * * *'
+   */
+  startCron(cronSchedule: string): void {
     this.cronJob?.stop();
-    this.cronJob = this.generateCronJob(cronSchedule, preExecute);
+    this.cronJob = this.generateCronJob(cronSchedule);
     this.cronJob.start();
   }
 
+  /**
+   * Stop the current cronjob
+   */
   stopCron(): void {
     this.cronJob.stop();
   }
 
+  /**
+   * Execute the job. Define the job process in the subclass.
+   */
   abstract executeJob(): Promise<void>;
 
-  protected generateCronJob(cronSchedule: string, preExecute?: () => void): ScheduledTask {
+  /**
+   * Create a new cronjob
+   * @param cronSchedule e.g. '0 1 * * *'
+   */
+  protected generateCronJob(cronSchedule: string): ScheduledTask {
     return nodeCron.schedule(cronSchedule, async() => {
-      await preExecute?.();
       try {
-        this.executeJob();
+        await this.executeJob();
       }
       catch (e) {
         logger.error(e);