diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index d6d723cafa3..84b0c8fa9cf 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -240,6 +240,24 @@ tags: - configmap + - name: Data deletion trigger failed CronJob + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: data-deletion-trigger-failed-cronjob.yml.j2 + when: WITH_API_ADMIN is defined and WITH_API_ADMIN|bool + tags: + - cronjob + + - name: Data deletion trigger failed CronJob ConfigMap + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: data-deletion-trigger-failed-cronjob-configmap.yml.j2 + when: WITH_API_ADMIN is defined and WITH_API_ADMIN|bool + tags: + - configmap + - name: amqp files storage Deployment kubernetes.core.k8s: kubeconfig: ~/.kube/config diff --git a/ansible/roles/schulcloud-server-core/templates/data-deletion-trigger-failed-cronjob-configmap.yml.j2 b/ansible/roles/schulcloud-server-core/templates/data-deletion-trigger-failed-cronjob-configmap.yml.j2 new file mode 100644 index 00000000000..c20477519ed --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/data-deletion-trigger-failed-cronjob-configmap.yml.j2 @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + namespace: {{ NAMESPACE }} + name: data-deletion-trigger-failed-cronjob-configmap + labels: + app: data-deletion-trigger-failed-cronjob +data: + NODE_OPTIONS: "--max-old-space-size=3072" + NEST_LOG_LEVEL: "error" + EXIT_ON_ERROR: "true" + SC_DOMAIN: "{{ DOMAIN }}" + FEATURE_PROMETHEUS_METRICS_ENABLED: "true" + ETHERPAD__PAD_URI: "https://{{ DOMAIN }}/etherpad/p" + diff --git a/ansible/roles/schulcloud-server-core/templates/data-deletion-trigger-failed-cronjob.yml.j2 b/ansible/roles/schulcloud-server-core/templates/data-deletion-trigger-failed-cronjob.yml.j2 new file mode 100644 index 00000000000..fcf52dc09da --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/data-deletion-trigger-failed-cronjob.yml.j2 @@ -0,0 +1,111 @@ +apiVersion: batch/v1 +kind: CronJob +metadata: + namespace: {{ NAMESPACE }} + labels: + app: data-deletion-trigger-failed + app.kubernetes.io/part-of: schulcloud-verbund + app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }} + app.kubernetes.io/name: data-deletion-trigger-failed + app.kubernetes.io/component: data-deletion + app.kubernetes.io/managed-by: ansible + git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }} + git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }} + name: data-deletion-trigger-failed-cronjob +spec: + concurrencyPolicy: Forbid + schedule: "{{ SERVER_DATA_DELETION_TRIGGER_FAILED_CRONJOB_SCHEDULE|default("@daily", true) }}" + jobTemplate: + metadata: + labels: + app: data-deletion-trigger-failed + app.kubernetes.io/part-of: schulcloud-verbund + app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }} + app.kubernetes.io/name: data-deletion-trigger-failed + app.kubernetes.io/component: data-deletion + app.kubernetes.io/managed-by: ansible + git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }} + git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }} + spec: + template: + spec: + securityContext: + runAsUser: 1000 + runAsGroup: 1000 + fsGroup: 1000 + runAsNonRoot: true + containers: + - name: data-deletion-trigger-failed-cronjob + image: {{ SCHULCLOUD_SERVER_IMAGE }}:{{ SCHULCLOUD_SERVER_IMAGE_TAG }} + envFrom: + - configMapRef: + name: data-deletion-trigger-failed-cronjob-configmap + - secretRef: + name: admin-api-client-secret + - secretRef: + name: api-files-secret + command: ['/bin/sh', '-c'] + args: ['npm run nest:start:deletion-console -- execution trigger -f true'] + resources: + limits: + cpu: {{ API_CPU_LIMITS|default("2000m", true) }} + memory: {{ API_MEMORY_LIMITS|default("2Gi", true) }} + requests: + cpu: {{ API_CPU_REQUESTS|default("100m", true) }} + memory: {{ API_MEMORY_REQUESTS|default("150Mi", true) }} + restartPolicy: OnFailure +{% if AFFINITY_ENABLE is defined and AFFINITY_ENABLE|bool %} + affinity: + podAffinity: + preferredDuringSchedulingIgnoredDuringExecution: + - weight: 20 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: app.kubernetes.io/part-of + operator: In + values: + - schulcloud-verbund + topologyKey: "kubernetes.io/hostname" + namespaceSelector: {} + - weight: 10 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: git.repo + operator: In + values: + - {{ SCHULCLOUD_SERVER_REPO_NAME }} + topologyKey: "kubernetes.io/hostname" + namespaceSelector: {} + - weight: 10 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: git.branch + operator: In + values: + - {{ SCHULCLOUD_SERVER_BRANCH_NAME }} + topologyKey: "kubernetes.io/hostname" + namespaceSelector: {} + - weight: 10 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: app.kubernetes.io/version + operator: In + values: + - {{ SCHULCLOUD_SERVER_IMAGE_TAG }} + topologyKey: "kubernetes.io/hostname" + namespaceSelector: {} +{% endif %} + metadata: + labels: + app: data-deletion-trigger-failed + app.kubernetes.io/part-of: schulcloud-verbund + app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }} + app.kubernetes.io/name: data-deletion-trigger-failed + app.kubernetes.io/component: data-deletion + app.kubernetes.io/managed-by: ansible + git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }} + git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }} \ No newline at end of file diff --git a/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.spec.ts b/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.spec.ts index 21171adb405..0b4eb6cd53c 100644 --- a/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.spec.ts +++ b/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.spec.ts @@ -1,4 +1,3 @@ -import { TriggerDeletionExecutionOptions } from '../interface'; import { TriggerDeletionExecutionOptionsBuilder } from './trigger-deletion-execution-options.builder'; describe(TriggerDeletionExecutionOptionsBuilder.name, () => { @@ -6,18 +5,17 @@ describe(TriggerDeletionExecutionOptionsBuilder.name, () => { describe('when called with proper arguments', () => { const setup = () => { const limit = 1000; + const runFailed = false; - const expectedOutput: TriggerDeletionExecutionOptions = { limit }; - - return { limit, expectedOutput }; + return { limit, runFailed }; }; it('should return valid object with expected values', () => { - const { limit, expectedOutput } = setup(); + const { limit, runFailed } = setup(); - const output = TriggerDeletionExecutionOptionsBuilder.build(limit); + const output = TriggerDeletionExecutionOptionsBuilder.build(limit, runFailed); - expect(output).toStrictEqual(expectedOutput); + expect(output).toStrictEqual({ limit, runFailed }); }); }); }); diff --git a/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.ts b/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.ts index ed652006e9d..89644fb4887 100644 --- a/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.ts +++ b/apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.ts @@ -1,7 +1,7 @@ import { TriggerDeletionExecutionOptions } from '../interface'; export class TriggerDeletionExecutionOptionsBuilder { - static build(limit: number): TriggerDeletionExecutionOptions { - return { limit }; + static build(limit: number, runFailed: boolean): TriggerDeletionExecutionOptions { + return { limit, runFailed }; } } diff --git a/apps/server/src/modules/deletion-console/deletion-client/deletion.client.spec.ts b/apps/server/src/modules/deletion-console/deletion-client/deletion.client.spec.ts index 2a445e9bc4e..44136994f35 100644 --- a/apps/server/src/modules/deletion-console/deletion-client/deletion.client.spec.ts +++ b/apps/server/src/modules/deletion-console/deletion-client/deletion.client.spec.ts @@ -249,5 +249,28 @@ describe(DeletionClient.name, () => { await expect(client.executeDeletions()).rejects.toThrow(Error); }); }); + + describe('when runFailed is true', () => { + const setup = () => { + setupConfig(); + + const response: AxiosResponse = axiosResponseFactory.build({ + status: 204, + }); + + httpService.post.mockReturnValueOnce(of(response)); + + const fullUrl = 'http://api-admin:4030/admin/api/v1/deletionExecutions?runFailed=true'; + + return { fullUrl }; + }; + it('should call endpoint with runFailed param', async () => { + const { fullUrl } = setup(); + + await client.executeDeletions(undefined, true); + + expect(httpService.post).toHaveBeenCalledWith(fullUrl, null, expect.any(Object)); + }); + }); }); }); diff --git a/apps/server/src/modules/deletion-console/deletion-client/deletion.client.ts b/apps/server/src/modules/deletion-console/deletion-client/deletion.client.ts index f18fd181ade..c37fd77062d 100644 --- a/apps/server/src/modules/deletion-console/deletion-client/deletion.client.ts +++ b/apps/server/src/modules/deletion-console/deletion-client/deletion.client.ts @@ -26,9 +26,9 @@ export class DeletionClient { } } - public async executeDeletions(limit?: number): Promise { + public async executeDeletions(limit?: number, runFailed?: boolean): Promise { try { - const response = await this.postDeletionExecutionRequest(limit); + const response = await this.postDeletionExecutionRequest(limit, runFailed); this.checkResponseStatusCode(response, HttpStatusCode.NoContent); } catch (err) { throw this.createError(err, 'executeDeletions'); @@ -46,13 +46,25 @@ export class DeletionClient { return response; } - private async postDeletionExecutionRequest(limit?: number): Promise> { + private async postDeletionExecutionRequest(limit?: number, runFailed?: boolean): Promise> { const defaultHeaders = this.createDefaultHeaders(); - const headers = this.isLimitGeaterZero(limit) ? { ...defaultHeaders, params: { limit } } : defaultHeaders; + const baseUrl = this.configService.get('ADMIN_API_CLIENT_BASE_URL', { infer: true }); - const postDeletionExecutionsEndpoint = new URL('/admin/api/v1/deletionExecutions', baseUrl).toString(); + const endpoint = '/admin/api/v1/deletionExecutions'; + + const postDeletionExecutionsEndpoint = new URL(endpoint, baseUrl); + + const params = new URLSearchParams(postDeletionExecutionsEndpoint.search); + if (limit && this.isLimitGreaterZero(limit)) { + params.append('limit', limit.toString()); + } + if (runFailed) { + params.append('runFailed', runFailed.toString()); + } + + const fullUrl = `${postDeletionExecutionsEndpoint.toString()}?${params.toString()}`; - const request = this.httpService.post(postDeletionExecutionsEndpoint, null, headers); + const request = this.httpService.post(fullUrl, null, defaultHeaders); const response = await firstValueFrom(request); return response; @@ -87,7 +99,7 @@ export class DeletionClient { } } - private isLimitGeaterZero(limit?: number): boolean { + private isLimitGreaterZero(limit?: number): boolean { return typeof limit === 'number' && limit > 0; } diff --git a/apps/server/src/modules/deletion-console/deletion-execution.console.spec.ts b/apps/server/src/modules/deletion-console/deletion-execution.console.spec.ts index 833831c5b77..27eb1850c5e 100644 --- a/apps/server/src/modules/deletion-console/deletion-execution.console.spec.ts +++ b/apps/server/src/modules/deletion-console/deletion-execution.console.spec.ts @@ -6,6 +6,7 @@ import { DeletionExecutionConsole } from './deletion-execution.console'; import { DeletionExecutionTriggerResultBuilder, TriggerDeletionExecutionOptionsBuilder } from './builder'; import { DeletionExecutionUc } from './uc'; import { DeletionConsoleModule } from './deletion-console.app.module'; +import { TriggerDeletionExecutionOptions } from './interface'; describe(DeletionExecutionConsole.name, () => { let module: TestingModule; @@ -38,7 +39,7 @@ describe(DeletionExecutionConsole.name, () => { const setup = () => { const limit = 1000; - const options = TriggerDeletionExecutionOptionsBuilder.build(1000); + const options = TriggerDeletionExecutionOptionsBuilder.build(1000, false); return { limit, options }; }; @@ -50,13 +51,13 @@ describe(DeletionExecutionConsole.name, () => { await console.triggerDeletionExecution(options); - expect(spy).toBeCalledWith(limit); + expect(spy).toBeCalledWith(limit, false); }); }); describe(`when ${DeletionExecutionUc.name}'s triggerDeletionExecution() method doesn't throw an exception`, () => { const setup = () => { - const options = TriggerDeletionExecutionOptionsBuilder.build(1000); + const options = TriggerDeletionExecutionOptionsBuilder.build(1000, false); deletionExecutionUc.triggerDeletionExecution.mockResolvedValueOnce(undefined); const spy = jest.spyOn(DeletionExecutionTriggerResultBuilder, 'buildSuccess'); @@ -75,7 +76,7 @@ describe(DeletionExecutionConsole.name, () => { describe(`when ${DeletionExecutionUc.name}'s triggerDeletionExecution() method throws an exception`, () => { const setup = () => { - const options = TriggerDeletionExecutionOptionsBuilder.build(1000); + const options = TriggerDeletionExecutionOptionsBuilder.build(1000, false); const err = new Error('some error occurred...'); deletionExecutionUc.triggerDeletionExecution.mockRejectedValueOnce(err); @@ -92,5 +93,29 @@ describe(DeletionExecutionConsole.name, () => { expect(spy).toHaveBeenCalledWith(err); }); }); + + describe('when runFailed is given as string "true"', () => { + it('should convert runFailed to boolean', async () => { + const options = { limit: 1000, runFailed: 'true' } as unknown as TriggerDeletionExecutionOptions; + + const spy = jest.spyOn(deletionExecutionUc, 'triggerDeletionExecution').mockResolvedValueOnce(undefined); + + await console.triggerDeletionExecution(options); + + expect(spy).toHaveBeenCalledWith(1000, true); + }); + }); + + describe('when limit is a string "5"', () => { + it('should convert limit to number', async () => { + const options = { limit: '5', runFailed: false } as unknown as TriggerDeletionExecutionOptions; + + const spy = jest.spyOn(deletionExecutionUc, 'triggerDeletionExecution').mockResolvedValueOnce(undefined); + + await console.triggerDeletionExecution(options); + + expect(spy).toHaveBeenCalledWith(5, false); + }); + }); }); }); diff --git a/apps/server/src/modules/deletion-console/deletion-execution.console.ts b/apps/server/src/modules/deletion-console/deletion-execution.console.ts index fe704c9058e..fade3e3b872 100644 --- a/apps/server/src/modules/deletion-console/deletion-execution.console.ts +++ b/apps/server/src/modules/deletion-console/deletion-execution.console.ts @@ -14,20 +14,36 @@ export class DeletionExecutionConsole { options: [ { flags: '-l, --limit ', + // TODO implemnt fn with proper command testing + // fn: (value: string) => (value ? Number(value) : undefined), + description: 'Limit of the requested deletion executions that should be performed.', + required: false, + }, + { + flags: '-f, --runFailed ', + // TODO implemnt fn with proper command testing + // fn: (value: string) => /^(true|yes|1)$/i.test(value), description: 'Limit of the requested deletion executions that should be performed.', required: false, }, ], }) - async triggerDeletionExecution(options: TriggerDeletionExecutionOptions): Promise { + public async triggerDeletionExecution(options: TriggerDeletionExecutionOptions): Promise { // Try to trigger the deletion execution(s) via Deletion API client, // return successful status in case of a success, otherwise return // a result with a failure status and a proper error message. let result: DeletionExecutionTriggerResult; - try { - await this.deletionExecutionUc.triggerDeletionExecution(options.limit ? Number(options.limit) : undefined); + if (typeof options.limit === 'string') { + options.limit = Number(options.limit); + } + + if (typeof options.runFailed === 'string') { + options.runFailed = /^(true|yes|1)$/i.test(options.runFailed); + } + try { + await this.deletionExecutionUc.triggerDeletionExecution(options.limit, options.runFailed); result = DeletionExecutionTriggerResultBuilder.buildSuccess(); } catch (err) { result = DeletionExecutionTriggerResultBuilder.buildFailure(err as Error); diff --git a/apps/server/src/modules/deletion-console/interface/trigger-deletion-execution-options.interface.ts b/apps/server/src/modules/deletion-console/interface/trigger-deletion-execution-options.interface.ts index b17aafa1112..14d69b46c1f 100644 --- a/apps/server/src/modules/deletion-console/interface/trigger-deletion-execution-options.interface.ts +++ b/apps/server/src/modules/deletion-console/interface/trigger-deletion-execution-options.interface.ts @@ -1,3 +1,4 @@ export interface TriggerDeletionExecutionOptions { - limit: number; + limit: string | number; + runFailed: string | boolean; } diff --git a/apps/server/src/modules/deletion-console/uc/deletion-execution.uc.ts b/apps/server/src/modules/deletion-console/uc/deletion-execution.uc.ts index 66bcd597d16..4dcc0ed17ea 100644 --- a/apps/server/src/modules/deletion-console/uc/deletion-execution.uc.ts +++ b/apps/server/src/modules/deletion-console/uc/deletion-execution.uc.ts @@ -5,7 +5,7 @@ import { DeletionClient } from '../deletion-client'; export class DeletionExecutionUc { constructor(private readonly deletionClient: DeletionClient) {} - async triggerDeletionExecution(limit?: number): Promise { - await this.deletionClient.executeDeletions(limit); + async triggerDeletionExecution(limit?: number, runFailed?: boolean): Promise { + await this.deletionClient.executeDeletions(limit, runFailed); } } diff --git a/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.spec.ts b/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.spec.ts index b86277aca15..1078ec41c34 100644 --- a/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.spec.ts +++ b/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.spec.ts @@ -10,18 +10,18 @@ describe(DeletionRequestBodyPropsBuilder.name, () => { const setup = () => { const domain = DomainName.PSEUDONYMS; const refId = new ObjectId().toHexString(); - const deleteInMinutes = 1000; - return { domain, refId, deleteInMinutes }; + const deleteAfterMinutes = 1000; + return { domain, refId, deleteAfterMinutes }; }; it('should build deletionRequestBodyParams with all attributes', () => { - const { domain, refId, deleteInMinutes } = setup(); + const { domain, refId, deleteAfterMinutes } = setup(); - const result = DeletionRequestBodyPropsBuilder.build(domain, refId, deleteInMinutes); + const result = DeletionRequestBodyPropsBuilder.build(domain, refId, deleteAfterMinutes); // Assert expect(result.targetRef.domain).toEqual(domain); expect(result.targetRef.id).toEqual(refId); - expect(result.deleteInMinutes).toEqual(deleteInMinutes); + expect(result.deleteAfterMinutes).toEqual(deleteAfterMinutes); }); }); }); diff --git a/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.ts b/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.ts index 7295d8c657f..5f6bfb9be7d 100644 --- a/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.ts +++ b/apps/server/src/modules/deletion/api/builder/deletion-request-body-props.builder.ts @@ -3,10 +3,10 @@ import { DeletionRequestBodyProps } from '../controller/dto'; import { DomainName } from '../../domain/types'; export class DeletionRequestBodyPropsBuilder { - static build(domain: DomainName, id: EntityId, deleteInMinutes?: number): DeletionRequestBodyProps { + static build(domain: DomainName, id: EntityId, deleteAfterMinutes?: number): DeletionRequestBodyProps { const deletionRequestItem = { targetRef: { domain, id }, - deleteInMinutes, + deleteAfterMinutes, }; return deletionRequestItem; diff --git a/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts b/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts index 53d5fc2af4f..6b7324a95f4 100644 --- a/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts +++ b/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts @@ -85,17 +85,17 @@ describe(`deletionRequest create (api)`, () => { domain: DomainName.USER, id: '653e4833cc39e5907a1e18d2', }, - deleteInMinutes: 0, + deleteAfterMinutes: 0, }; - const defaultDeleteInMinutes = 43200; + const defaultDeleteAfterMinutes = 43200; const operationalTimeToleranceInSeconds = 30; return { deletionRequestToCreate, deletionRequestToImmediateRemoval, - defaultDeleteInMinutes, + defaultDeleteAfterMinutes, operationalTimeToleranceInSeconds, }; }; @@ -117,34 +117,30 @@ describe(`deletionRequest create (api)`, () => { expect(result.requestId).toBeDefined(); }); - describe('when the "delete in minutes" param has not been provided', () => { - it( - 'should set the "deletion planned at" date to the date after the default "delete in minutes" value ' + - '(43200 minutes which is 30 days), with some operational time tolerance', - async () => { - const { deletionRequestToCreate, defaultDeleteInMinutes, operationalTimeToleranceInSeconds } = setup(); + describe('when the "delete after minutes" param has not been provided', () => { + it('should set the "deleteAfter" date to the date after the default DELETION_DELETE_AFTER_MINUTES ', async () => { + const { deletionRequestToCreate, defaultDeleteAfterMinutes, operationalTimeToleranceInSeconds } = setup(); - const response = await testApiClient.post('', deletionRequestToCreate); + const response = await testApiClient.post('', deletionRequestToCreate); - const result = response.body as DeletionRequestResponse; - const createdDeletionRequestId = result.requestId; + const result = response.body as DeletionRequestResponse; + const createdDeletionRequestId = result.requestId; - const createdItem = await em.findOneOrFail(DeletionRequestEntity, createdDeletionRequestId); + const createdItem = await em.findOneOrFail(DeletionRequestEntity, createdDeletionRequestId); - const isDeletionPlannedAtDateCorrect = isDeletionPlannedWithinAcceptableRange( - createdItem.createdAt, - createdItem.deleteAfter, - defaultDeleteInMinutes, - operationalTimeToleranceInSeconds - ); + const isDeletionPlannedAtDateCorrect = isDeletionPlannedWithinAcceptableRange( + createdItem.createdAt, + createdItem.deleteAfter, + defaultDeleteAfterMinutes, + operationalTimeToleranceInSeconds + ); - expect(isDeletionPlannedAtDateCorrect).toEqual(true); - } - ); + expect(isDeletionPlannedAtDateCorrect).toEqual(true); + }); }); - describe('when the "delete in minutes" param has been set to 0', () => { - it('should set the "deletion planned at" date to now, with some operational time tolerance', async () => { + describe('when the "delete after minutes" param has been set to 0', () => { + it('should set the "deleteAfter" date to now', async () => { const { deletionRequestToImmediateRemoval, operationalTimeToleranceInSeconds } = setup(); const response = await testApiClient.post('', deletionRequestToImmediateRemoval); @@ -164,6 +160,32 @@ describe(`deletionRequest create (api)`, () => { expect(isDeletionPlannedAtDateCorrect).toEqual(true); }); }); + + describe('when the "delete after minutes" param has been set to > 0', () => { + it('should set the "deleteAfter" date to now plus "delete after minutes" ', async () => { + const { deletionRequestToCreate, operationalTimeToleranceInSeconds } = setup(); + + const deleteAfterMinutes = 120; + + deletionRequestToCreate.deleteAfterMinutes = deleteAfterMinutes; + + const response = await testApiClient.post('', deletionRequestToCreate); + + const result = response.body as DeletionRequestResponse; + const createdDeletionRequestId = result.requestId; + + const createdItem = await em.findOneOrFail(DeletionRequestEntity, createdDeletionRequestId); + + const isDeletionPlannedAtDateCorrect = isDeletionPlannedWithinAcceptableRange( + createdItem.createdAt, + createdItem.deleteAfter, + deleteAfterMinutes, + operationalTimeToleranceInSeconds + ); + + expect(isDeletionPlannedAtDateCorrect).toEqual(true); + }); + }); }); }); }); diff --git a/apps/server/src/modules/deletion/api/controller/deletion-executions.controller.ts b/apps/server/src/modules/deletion/api/controller/deletion-executions.controller.ts index 4fad5858ccf..4229edd76e8 100644 --- a/apps/server/src/modules/deletion/api/controller/deletion-executions.controller.ts +++ b/apps/server/src/modules/deletion/api/controller/deletion-executions.controller.ts @@ -19,6 +19,9 @@ export class DeletionExecutionsController { status: 204, }) async executeDeletions(@Query() deletionExecutionQuery: DeletionExecutionParams) { - return this.deletionRequestUc.executeDeletionRequests(deletionExecutionQuery.limit); + return this.deletionRequestUc.executeDeletionRequests( + deletionExecutionQuery.limit, + deletionExecutionQuery.runFailed + ); } } diff --git a/apps/server/src/modules/deletion/api/controller/dto/deletion-execution.params.ts b/apps/server/src/modules/deletion/api/controller/dto/deletion-execution.params.ts index 9ca4a18bd71..79e54651988 100644 --- a/apps/server/src/modules/deletion/api/controller/dto/deletion-execution.params.ts +++ b/apps/server/src/modules/deletion/api/controller/dto/deletion-execution.params.ts @@ -1,5 +1,5 @@ import { ApiPropertyOptional } from '@nestjs/swagger'; -import { IsInt, IsOptional, Min } from 'class-validator'; +import { IsBoolean, IsInt, IsOptional, Min } from 'class-validator'; export class DeletionExecutionParams { @IsInt() @@ -7,4 +7,8 @@ export class DeletionExecutionParams { @IsOptional() @ApiPropertyOptional({ description: 'Page limit, defaults to 100.', minimum: 1 }) limit?: number = 100; + + @IsBoolean() + @IsOptional() + runFailed?: boolean = false; } diff --git a/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts b/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts index 1c4b5b80460..418fe5400a6 100644 --- a/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts +++ b/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts @@ -14,24 +14,18 @@ describe(DeletionRequestBodyProps.name, () => { expect(deletionRequestBodyProps).toBeDefined(); }); - it('deleteInMinutes should be a number with default value 43200', () => { + it('deleteAfterMinutes should be optional', () => { const { deletionRequestBodyProps } = setup(); - expect(deletionRequestBodyProps.deleteInMinutes).toBeDefined(); - expect(typeof deletionRequestBodyProps.deleteInMinutes).toBe('number'); - expect(deletionRequestBodyProps.deleteInMinutes).toBe(43200); + deletionRequestBodyProps.deleteAfterMinutes = undefined; + expect(deletionRequestBodyProps.deleteAfterMinutes).toBeUndefined(); }); - it('deleteInMinutes should be optional', () => { + it('deleteAfterMinutes should be a number when provided', () => { const { deletionRequestBodyProps } = setup(); - deletionRequestBodyProps.deleteInMinutes = undefined; - expect(deletionRequestBodyProps.deleteInMinutes).toBeUndefined(); - }); - - it('deleteInMinutes should be a number when provided', () => { - const { deletionRequestBodyProps } = setup(); - const customDeleteInMinutes = 60; - deletionRequestBodyProps.deleteInMinutes = customDeleteInMinutes; - expect(deletionRequestBodyProps.deleteInMinutes).toBe(customDeleteInMinutes); + const customdeleteAfterMinutes = 60; + deletionRequestBodyProps.deleteAfterMinutes = customdeleteAfterMinutes; + expect(deletionRequestBodyProps.deleteAfterMinutes).toBe(customdeleteAfterMinutes); + expect(typeof deletionRequestBodyProps.deleteAfterMinutes).toBe('number'); }); }); }); diff --git a/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.ts b/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.ts index 5dc07f31d9b..99c0634c68b 100644 --- a/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.ts +++ b/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.ts @@ -2,7 +2,6 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { IsNumber, IsOptional, Min } from 'class-validator'; import { DeletionTargetRef } from '../../../domain/interface'; -const MINUTES_OF_30_DAYS = 30 * 24 * 60; export class DeletionRequestBodyProps { @ApiProperty({ required: true, @@ -17,5 +16,5 @@ export class DeletionRequestBodyProps { required: true, nullable: false, }) - deleteInMinutes?: number = MINUTES_OF_30_DAYS; + deleteAfterMinutes?: number; } diff --git a/apps/server/src/modules/deletion/api/uc/deletion-request.uc.spec.ts b/apps/server/src/modules/deletion/api/uc/deletion-request.uc.spec.ts index 2a08e643579..40c273c7389 100644 --- a/apps/server/src/modules/deletion/api/uc/deletion-request.uc.spec.ts +++ b/apps/server/src/modules/deletion/api/uc/deletion-request.uc.spec.ts @@ -1,10 +1,9 @@ import { LegacyLogger } from '@core/logger'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { MikroORM } from '@mikro-orm/core'; -import { ConfigModule } from '@nestjs/config'; +import { ConfigService } from '@nestjs/config'; import { EventBus } from '@nestjs/cqrs'; import { Test, TestingModule } from '@nestjs/testing'; -import { createConfigModuleOptions } from '@shared/common/config-module-options'; import { setupEntities } from '@testing/setup-entities'; import { ObjectId } from 'bson'; import { DomainDeletionReportBuilder, DomainOperationReportBuilder } from '../../domain/builder'; @@ -17,6 +16,7 @@ import { DeletionRequestLogResponseBuilder } from '../builder'; import { DeletionRequestBodyProps } from '../controller/dto'; import { DeletionLogStatisticBuilder, DeletionTargetRefBuilder } from '../controller/dto/builder'; import { DeletionRequestUc } from './deletion-request.uc'; +import { DeletionConfig } from '../../deletion.config'; describe(DeletionRequestUc.name, () => { let module: TestingModule; @@ -24,12 +24,12 @@ describe(DeletionRequestUc.name, () => { let deletionRequestService: DeepMocked; let deletionLogService: DeepMocked; let eventBus: DeepMocked; + let configService: DeepMocked>; beforeAll(async () => { const orm = await setupEntities(); module = await Test.createTestingModule({ - imports: [ConfigModule.forRoot(createConfigModuleOptions(deletionTestConfig))], providers: [ DeletionRequestUc, { @@ -54,6 +54,10 @@ describe(DeletionRequestUc.name, () => { provide: MikroORM, useValue: orm, }, + { + provide: ConfigService, + useValue: createMock(), + }, ], }).compile(); @@ -61,6 +65,9 @@ describe(DeletionRequestUc.name, () => { deletionRequestService = module.get(DeletionRequestService); deletionLogService = module.get(DeletionLogService); eventBus = module.get(EventBus); + configService = module.get(ConfigService); + + jest.useFakeTimers(); }); beforeEach(() => { @@ -70,30 +77,35 @@ describe(DeletionRequestUc.name, () => { describe('createDeletionRequest', () => { describe('when creating a deletionRequest', () => { const setup = () => { + const deleteAfterMinutes = 1; const deletionRequestToCreate: DeletionRequestBodyProps = { targetRef: { domain: DomainName.USER, id: new ObjectId().toHexString(), }, - deleteInMinutes: 1440, + deleteAfterMinutes, }; const deletionRequest = deletionRequestFactory.build(); + const deleteAfter = new Date(); + deleteAfter.setMinutes(deleteAfter.getMinutes() + deleteAfterMinutes); + return { deletionRequestToCreate, deletionRequest, + deleteAfter, }; }; it('should call the service to create the deletionRequest', async () => { - const { deletionRequestToCreate } = setup(); + const { deletionRequestToCreate, deleteAfter } = setup(); await uc.createDeletionRequest(deletionRequestToCreate); expect(deletionRequestService.createDeletionRequest).toHaveBeenCalledWith( deletionRequestToCreate.targetRef.id, deletionRequestToCreate.targetRef.domain, - deletionRequestToCreate.deleteInMinutes + deleteAfter ); }); @@ -147,30 +159,80 @@ describe(DeletionRequestUc.name, () => { describe('executeDeletionRequests', () => { describe('when deletionRequests to execute exists', () => { const setup = () => { + deletionRequestService.findInProgressCount.mockResolvedValue(1); + + configService.get.mockImplementation((key) => { + if (key === 'ADMIN_API__DELETION_DELETE_AFTER_MINUTES') { + return 1; + } + if (key === 'ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS') { + return 100; + } + if (key === 'ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS') { + return 1000; + } + if (key === 'ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS') { + return 2; + } + if (key === 'ADMIN_API__DELETION_DELAY_MILLISECONDS') { + return 100; + } + }); + const deletionRequest = deletionRequestFactory.buildWithId({ deleteAfter: new Date('2023-01-01') }); + deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequest]); + deletionRequestService.markDeletionRequestAsPending.mockResolvedValueOnce(true); return { deletionRequest }; }; - it('should call deletionRequestService.countPendingDeletionRequests', async () => { - deletionRequestService.countPendingDeletionRequests.mockResolvedValue(0); + it('should call deletionRequestService.findInProgressCount', async () => { + setup(); await uc.executeDeletionRequests(); - expect(deletionRequestService.countPendingDeletionRequests).toHaveBeenCalled(); + expect(deletionRequestService.findInProgressCount).toHaveBeenCalled(); }); - it('should call deletionRequestService.findAllItemsToExecute', async () => { - deletionRequestService.countPendingDeletionRequests.mockResolvedValue(0); + it('should call deletionRequestService.findAllItemsToExecute with correct limit', async () => { + setup(); await uc.executeDeletionRequests(); - expect(deletionRequestService.findAllItemsToExecute).toHaveBeenCalled(); + expect(deletionRequestService.findAllItemsToExecute).toHaveBeenCalledWith(1, undefined); + }); + + describe('when limit is given', () => { + it('should call deletionRequestService.findAllItemsToExecute with correct limit', async () => { + setup(); + + await uc.executeDeletionRequests(10); + + expect(deletionRequestService.findAllItemsToExecute).toHaveBeenCalledWith(9, undefined); + }); + }); + + describe('when limit is not given', () => { + it('should call deletionRequestService.findAllItemsToExecute with correct limit', async () => { + setup(); + + await uc.executeDeletionRequests(); + + expect(deletionRequestService.findAllItemsToExecute).toHaveBeenCalledWith(1, undefined); + }); + }); + + describe('when getFailed is true', () => { + it('should call deletionRequestService.findAllItemsToExecute with getFailed true', async () => { + setup(); + + await uc.executeDeletionRequests(undefined, true); + + expect(deletionRequestService.findAllItemsToExecute).toHaveBeenCalledWith(1, true); + }); }); it('should call deletionRequestService.markDeletionRequestAsPending to update status of deletionRequests', async () => { const { deletionRequest } = setup(); - deletionRequestService.countPendingDeletionRequests.mockResolvedValue(0); - deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequest]); await uc.executeDeletionRequests(); @@ -179,8 +241,6 @@ describe(DeletionRequestUc.name, () => { it('should call eventBus.publish', async () => { const { deletionRequest } = setup(); - deletionRequestService.countPendingDeletionRequests.mockResolvedValue(0); - deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequest]); await uc.executeDeletionRequests(); @@ -188,14 +248,35 @@ describe(DeletionRequestUc.name, () => { new UserDeletedEvent(deletionRequest.id, deletionRequest.targetRefId) ); }); + + /* + it('should work with a delay of 0', async () => { + const deletionRequest = deletionRequestFactory.buildWithId({ deleteAfter: new Date('2023-01-01') }); + configService.get.mockImplementation((key) => { + if (key === 'ADMIN_API__DELETION_DELAY_MILLISECONDS') { + return 0; + } + return deletionTestConfig()[key]; + }); + + deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequest]); + deletionRequestService.findInProgressCount.mockResolvedValueOnce(0); + + await uc.executeDeletionRequests(); + + expect(deletionRequestService.markDeletionRequestAsFailed).not.toHaveBeenCalled(); + }); + */ }); describe('when an error occurred', () => { const setup = () => { const deletionRequestToExecute = deletionRequestFactory.build({ deleteAfter: new Date('2023-01-01') }); - deletionRequestService.countPendingDeletionRequests.mockResolvedValue(0); + configService.get.mockImplementation((key) => deletionTestConfig()[key]); + deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequestToExecute]); + deletionRequestService.findInProgressCount.mockResolvedValueOnce(1); eventBus.publish.mockRejectedValueOnce(new Error()); return { diff --git a/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts b/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts index 68eb39e9c59..92444e035c4 100644 --- a/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts +++ b/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts @@ -17,7 +17,7 @@ import { DeletionConfig } from '../../deletion.config'; @Injectable() @EventsHandler(DataDeletedEvent) export class DeletionRequestUc implements IEventHandler { - config: string[]; + private config: string[]; constructor( private readonly deletionRequestService: DeletionRequestService, @@ -38,13 +38,13 @@ export class DeletionRequestUc implements IEventHandler { 'file', 'fileRecords', 'lessons', + 'news', 'pseudonyms', 'rocketChatUser', + 'submissions', 'task', 'teams', 'user', - 'submissions', - 'news', ]; } @@ -65,53 +65,48 @@ export class DeletionRequestUc implements IEventHandler { async createDeletionRequest(deletionRequest: DeletionRequestBodyProps): Promise { this.logger.debug({ action: 'createDeletionRequest', deletionRequest }); + const minutes = + deletionRequest.deleteAfterMinutes ?? this.configService.get('ADMIN_API__DELETION_DELETE_AFTER_MINUTES'); + const deleteAfter = new Date(); + deleteAfter.setMinutes(deleteAfter.getMinutes() + minutes); const result = await this.deletionRequestService.createDeletionRequest( deletionRequest.targetRef.id, deletionRequest.targetRef.domain, - deletionRequest.deleteInMinutes + deleteAfter ); return result; } - async executeDeletionRequests(limit?: number): Promise { + async executeDeletionRequests(limit?: number, getFailed?: boolean): Promise { this.logger.debug({ action: 'executeDeletionRequests', limit }); - const maxAmountOfDeletionRequestsDoConcurrently = this.configService.get( - 'ADMIN_API__MAX_CONCURRENT_DELETION_REQUESTS' - ); - const callsDelayMilliseconds = this.configService.get('ADMIN_API__DELETION_DELAY_MILLISECONDS'); - let tasks: DeletionRequest[] = []; + let deletionRequests: DeletionRequest[] = []; + const configLimit = this.configService.get('ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS'); do { - const numberOfDeletionRequestsWithStatusPending = - // eslint-disable-next-line no-await-in-loop - await this.deletionRequestService.countPendingDeletionRequests(); - const numberOfDeletionRequestsToProccess = - maxAmountOfDeletionRequestsDoConcurrently - numberOfDeletionRequestsWithStatusPending; - this.logger.debug({ - action: 'numberItemsWithStatusPending, amountWillingToTake', - numberOfDeletionRequestsWithStatusPending, - numberOfDeletionRequestsToProccess, - }); // eslint-disable-next-line no-await-in-loop - if (numberOfDeletionRequestsToProccess > 0) { - // eslint-disable-next-line no-await-in-loop - tasks = await this.deletionRequestService.findAllItemsToExecute(numberOfDeletionRequestsToProccess); + const inProgress = await this.deletionRequestService.findInProgressCount(); + + const max = limit ? limit - inProgress : configLimit - inProgress; + + // eslint-disable-next-line no-await-in-loop + deletionRequests = await this.deletionRequestService.findAllItemsToExecute(max, getFailed); + + if (max > 0) { + this.logger.debug({ action: 'processing deletion request', deletionRequests }); + // eslint-disable-next-line no-await-in-loop await Promise.all( - tasks.map(async (req) => { + deletionRequests.map(async (req) => { await this.executeDeletionRequest(req); }) ); - } - // short sleep mode to give time for deletion process to do their work - if (callsDelayMilliseconds && callsDelayMilliseconds > 0) { + // eslint-disable-next-line no-await-in-loop - await new Promise((resolve) => { - setTimeout(resolve, callsDelayMilliseconds); - }); + await this.delayForDeletion(); } - } while (tasks.length > 0); + } while (deletionRequests.length > 0); + this.logger.debug({ action: 'deletion process completed' }); } @@ -150,4 +145,15 @@ export class DeletionRequestUc implements IEventHandler { await this.deletionRequestService.markDeletionRequestAsFailed(deletionRequest.id); } } + + private async delayForDeletion() { + const delay = this.configService.get('ADMIN_API__DELETION_DELAY_MILLISECONDS'); + if (delay > 0) { + return new Promise((resolve) => { + setTimeout(resolve, delay); + }); + } + + return Promise.resolve(); + } } diff --git a/apps/server/src/modules/deletion/deletion.config.ts b/apps/server/src/modules/deletion/deletion.config.ts index 5a02567c00a..2cd761d7941 100644 --- a/apps/server/src/modules/deletion/deletion.config.ts +++ b/apps/server/src/modules/deletion/deletion.config.ts @@ -18,7 +18,9 @@ export interface DeletionConfig FilesConfig, RocketChatUserConfig, XApiKeyAuthGuardConfig { - ADMIN_API__MODIFICATION_THRESHOLD_MS: number; - ADMIN_API__MAX_CONCURRENT_DELETION_REQUESTS: number; + ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS: number; + ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS: number; ADMIN_API__DELETION_DELAY_MILLISECONDS: number; + ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS: number; + ADMIN_API__DELETION_DELETE_AFTER_MINUTES: number; } diff --git a/apps/server/src/modules/deletion/domain/service/deletion-request.service.spec.ts b/apps/server/src/modules/deletion/domain/service/deletion-request.service.spec.ts index 6f191ff5ec6..bae7a31adbe 100644 --- a/apps/server/src/modules/deletion/domain/service/deletion-request.service.spec.ts +++ b/apps/server/src/modules/deletion/domain/service/deletion-request.service.spec.ts @@ -1,5 +1,5 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { ConfigModule } from '@nestjs/config'; +import { ConfigModule, ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; import { createConfigModuleOptions } from '@shared/common/config-module-options'; import { setupEntities } from '@testing/setup-entities'; @@ -13,6 +13,7 @@ describe(DeletionRequestService.name, () => { let module: TestingModule; let service: DeletionRequestService; let deletionRequestRepo: DeepMocked; + let configService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -28,8 +29,12 @@ describe(DeletionRequestService.name, () => { service = module.get(DeletionRequestService); deletionRequestRepo = module.get(DeletionRequestRepo); + configService = module.get(ConfigService); await setupEntities(); + + jest.useFakeTimers(); + jest.setSystemTime(new Date()); }); beforeEach(() => { @@ -51,20 +56,23 @@ describe(DeletionRequestService.name, () => { const setup = () => { const targetRefId = '653e4833cc39e5907a1e18d2'; const targetRefDomain = DomainName.USER; + const deleteAfter = new Date(); + const minutes = 60; + deleteAfter.setMinutes(deleteAfter.getMinutes() + minutes); - return { targetRefId, targetRefDomain }; + return { targetRefId, targetRefDomain, deleteAfter }; }; it('should call deletionRequestRepo.create', async () => { - const { targetRefId, targetRefDomain } = setup(); + const { targetRefId, targetRefDomain, deleteAfter } = setup(); - await service.createDeletionRequest(targetRefId, targetRefDomain); + await service.createDeletionRequest(targetRefId, targetRefDomain, deleteAfter); expect(deletionRequestRepo.create).toHaveBeenCalledWith( expect.objectContaining({ id: expect.any(String), targetRefDomain, - deleteAfter: expect.any(Date), + deleteAfter, targetRefId, status: StatusModel.REGISTERED, }) @@ -105,30 +113,38 @@ describe(DeletionRequestService.name, () => { describe('findAllItemsToExecute', () => { describe('when finding all deletionRequests for execution', () => { const setup = () => { + const limit = 100; + const dateInPast = new Date(); - const threshold = 1000; - const limit = undefined; dateInPast.setDate(dateInPast.getDate() - 1); + + const thresholdOlderMs = configService.get('ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS') ?? 100; + const thresholdNewerMs = configService.get('ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS') ?? 1000; + + const olderThan = new Date(Date.now() - thresholdOlderMs); + const newerThan = new Date(Date.now() - thresholdNewerMs); + const deletionRequest1 = deletionRequestFactory.build({ deleteAfter: dateInPast }); const deletionRequest2 = deletionRequestFactory.build({ deleteAfter: dateInPast }); - deletionRequestRepo.findAllItemsToExecution.mockResolvedValue([deletionRequest1, deletionRequest2]); + deletionRequestRepo.findAllItems.mockResolvedValue([deletionRequest1, deletionRequest2]); const deletionRequests = [deletionRequest1, deletionRequest2]; - return { deletionRequests, limit, threshold }; + + return { deletionRequests, limit, olderThan, newerThan }; }; - it('should call deletionRequestRepo.findAllItemsByDeletionDate with required parameter', async () => { - const { limit, threshold } = setup(); + it('should call deletionRequestRepo.findAllItemsToExecution with required parameter', async () => { + const { limit } = setup(); - await service.findAllItemsToExecute(); + await service.findAllItemsToExecute(limit); - expect(deletionRequestRepo.findAllItemsToExecution).toBeCalledWith(threshold, limit); + expect(deletionRequestRepo.findAllItems).toBeCalledWith(limit); }); it('should return array of two deletionRequests to execute', async () => { - const { deletionRequests } = setup(); - const result = await service.findAllItemsToExecute(); + const { deletionRequests, limit } = setup(); + const result = await service.findAllItemsToExecute(limit); expect(result).toHaveLength(2); expect(result).toEqual(deletionRequests); @@ -136,33 +152,36 @@ describe(DeletionRequestService.name, () => { }); }); - describe('countPendingDeletionRequests', () => { - describe('when counting all deletionRequests with status pending', () => { + describe('findInProgressCount', () => { + describe('when finding in progress deletionRequests', () => { const setup = () => { - const deletionRequestWithStatusPending = deletionRequestFactory.buildListWithId(5, { - status: StatusModel.PENDING, - }); + const thresholdNewerMs = configService.get('ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS') ?? 1000; + const newerThan = new Date(Date.now() - thresholdNewerMs); + + const count = 2; - deletionRequestRepo.countPendingDeletionRequests.mockResolvedValue(deletionRequestWithStatusPending.length); + deletionRequestRepo.findInProgressCount.mockResolvedValue(count); - const numberDeletionRequestsWithStatusPending = deletionRequestWithStatusPending.length; - return { numberDeletionRequestsWithStatusPending }; + return { count, newerThan }; }; - it('should call deletionRequestRepo.countPendingDeletionRequests', async () => { - await service.countPendingDeletionRequests(); + it('should call deletionRequestRepo.findInProgressCount', async () => { + const { newerThan } = setup(); - expect(deletionRequestRepo.countPendingDeletionRequests).toBeCalled(); + await service.findInProgressCount(); + + expect(deletionRequestRepo.findInProgressCount).toBeCalledWith(newerThan); }); - it('should return number of deletionRequests with status pending', async () => { - const { numberDeletionRequestsWithStatusPending } = setup(); - const result = await service.countPendingDeletionRequests(); + it('should return count of in progress deletionRequests', async () => { + const { count } = setup(); + const result = await service.findInProgressCount(); - expect(result).toEqual(numberDeletionRequestsWithStatusPending); + expect(result).toEqual(count); }); }); }); + describe('update', () => { describe('when updating deletionRequest', () => { const setup = () => { diff --git a/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts b/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts index 7e72b076bcc..abea479f654 100644 --- a/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts +++ b/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts @@ -9,23 +9,30 @@ import { DeletionConfig } from '../../deletion.config'; @Injectable() export class DeletionRequestService { + private olderThan: Date; + + private newerThan: Date; + constructor( private readonly deletionRequestRepo: DeletionRequestRepo, private readonly configService: ConfigService - ) {} + ) { + const thresholdOlder = this.configService.get('ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS'); + this.olderThan = new Date(Date.now() - thresholdOlder); + + const thresholdNewer = this.configService.get('ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS'); + this.newerThan = new Date(Date.now() - thresholdNewer); + } async createDeletionRequest( targetRefId: EntityId, targetRefDomain: DomainName, - deleteInMinutes = 43200 + deleteAfter: Date ): Promise<{ requestId: EntityId; deletionPlannedAt: Date }> { - const dateOfDeletion = new Date(); - dateOfDeletion.setMinutes(dateOfDeletion.getMinutes() + deleteInMinutes); - const newDeletionRequest = new DeletionRequest({ id: new ObjectId().toHexString(), targetRefDomain, - deleteAfter: dateOfDeletion, + deleteAfter, targetRefId, status: StatusModel.REGISTERED, }); @@ -41,17 +48,17 @@ export class DeletionRequestService { return deletionRequest; } - async findAllItemsToExecute(limit?: number): Promise { - const threshold = this.configService.get('ADMIN_API__MODIFICATION_THRESHOLD_MS'); - const itemsToDelete: DeletionRequest[] = await this.deletionRequestRepo.findAllItemsToExecution(threshold, limit); + public async findAllItemsToExecute(limit: number, getFailed = false): Promise { + const deletionRequests = getFailed + ? await this.deletionRequestRepo.findAllFailedItems(limit, this.olderThan, this.newerThan) + : await this.deletionRequestRepo.findAllItems(limit); - return itemsToDelete; + return deletionRequests; } - async countPendingDeletionRequests(): Promise { - const numberItemsWithStatusPending: number = await this.deletionRequestRepo.countPendingDeletionRequests(); - - return numberItemsWithStatusPending; + public async findInProgressCount(): Promise { + const count = await this.deletionRequestRepo.findInProgressCount(this.newerThan); + return count; } async update(deletionRequestToUpdate: DeletionRequest): Promise { diff --git a/apps/server/src/modules/deletion/domain/testing/test-config.ts b/apps/server/src/modules/deletion/domain/testing/test-config.ts index 86e6c86798b..f0ca43c7b16 100644 --- a/apps/server/src/modules/deletion/domain/testing/test-config.ts +++ b/apps/server/src/modules/deletion/domain/testing/test-config.ts @@ -1,9 +1,15 @@ import { Configuration } from '@hpi-schul-cloud/commons'; const deletionConfig = { - ADMIN_API__MODIFICATION_THRESHOLD_MS: Configuration.get('ADMIN_API__MODIFICATION_THRESHOLD_MS') as number, - ADMIN_API__MAX_CONCURRENT_DELETION_REQUESTS: Configuration.get( - 'ADMIN_API__MAX_CONCURRENT_DELETION_REQUESTS' + ADMIN_API__DELETION_DELETE_AFTER_MINUTES: Configuration.get('ADMIN_API__DELETION_DELETE_AFTER_MINUTES') as number, + ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS: Configuration.get( + 'ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS' + ) as number, + ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS: Configuration.get( + 'ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS' + ) as number, + ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS: Configuration.get( + 'ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS' ) as number, ADMIN_API__DELETION_DELAY_MILLISECONDS: Configuration.get('ADMIN_API__DELETION_DELAY_MILLISECONDS') as number, }; @@ -12,8 +18,10 @@ const config = () => deletionConfig; export const deletionTestConfig = () => { const conf = config(); - conf.ADMIN_API__MODIFICATION_THRESHOLD_MS = 1000; - conf.ADMIN_API__MAX_CONCURRENT_DELETION_REQUESTS = 2; + conf.ADMIN_API__DELETION_DELETE_AFTER_MINUTES = 1; + conf.ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS = 100; + conf.ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS = 1000; + conf.ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS = 2; conf.ADMIN_API__DELETION_DELAY_MILLISECONDS = 100; return conf; }; diff --git a/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts b/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts index aa6b7755532..4021cc91a3e 100644 --- a/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts +++ b/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts @@ -102,18 +102,18 @@ describe(DeletionRequestRepo.name, () => { }); }); - describe('findAllItemsToExecution', () => { + describe('findAllItems', () => { describe('when there is no deletionRequest for execution', () => { const setup = () => { - const threshold = 1000; + const limit = 1000; return { - threshold, + limit, }; }; it('should return empty array', async () => { - const { threshold } = setup(); - const result = await repo.findAllItemsToExecution(threshold); + const { limit } = setup(); + const result = await repo.findAllItems(limit); expect(result).toEqual([]); }); @@ -121,9 +121,11 @@ describe(DeletionRequestRepo.name, () => { describe('when there are deletionRequests for execution', () => { const setup = async () => { - const threshold = 1000; + const limit = 1000; + const dateInFuture = new Date(); dateInFuture.setDate(dateInFuture.getDate() + 30); + const deletionRequestEntity1: DeletionRequestEntity = deletionRequestEntityFactory.build({ createdAt: new Date(2023, 7, 1), updatedAt: new Date(2023, 8, 2), @@ -140,14 +142,17 @@ describe(DeletionRequestRepo.name, () => { createdAt: new Date(2023, 8, 1), updatedAt: new Date(2023, 8, 1), deleteAfter: new Date(2023, 9, 1), + status: StatusModel.REGISTERED, }); const deletionRequestEntity4: DeletionRequestEntity = deletionRequestEntityFactory.build({ createdAt: new Date(2023, 9, 1), updatedAt: new Date(2023, 9, 1), deleteAfter: new Date(2023, 10, 1), + status: StatusModel.REGISTERED, }); const deletionRequestEntity5: DeletionRequestEntity = deletionRequestEntityFactory.build({ deleteAfter: dateInFuture, + status: StatusModel.REGISTERED, }); await em.persistAndFlush([ @@ -178,34 +183,21 @@ describe(DeletionRequestRepo.name, () => { createdAt: deletionRequestEntity3.createdAt, updatedAt: deletionRequestEntity3.updatedAt, }, - { - id: deletionRequestEntity2.id, - targetRefDomain: deletionRequestEntity2.targetRefDomain, - deleteAfter: deletionRequestEntity2.deleteAfter, - targetRefId: deletionRequestEntity2.targetRefId, - status: deletionRequestEntity2.status, - createdAt: deletionRequestEntity2.createdAt, - updatedAt: deletionRequestEntity2.updatedAt, - }, ]; - return { deletionRequestEntity1, deletionRequestEntity5, expectedArray, threshold }; + return { deletionRequestEntity1, deletionRequestEntity5, expectedArray, limit }; }; - it('should find deletionRequests with deleteAfter smaller then today and status with value registered or failed', async () => { - const { deletionRequestEntity1, deletionRequestEntity5, expectedArray, threshold } = await setup(); + it('should find deletionRequests with deleteAfter smaller then today and status with value registered', async () => { + const { deletionRequestEntity1, deletionRequestEntity5, expectedArray, limit } = await setup(); - const results = await repo.findAllItemsToExecution(threshold); + const results = await repo.findAllItems(limit); - expect(results.length).toEqual(3); + expect(results.length).toEqual(2); // Verify explicit fields. expect(results).toEqual( - expect.arrayContaining([ - expect.objectContaining(expectedArray[0]), - expect.objectContaining(expectedArray[1]), - expect.objectContaining(expectedArray[2]), - ]) + expect.arrayContaining([expect.objectContaining(expectedArray[0]), expect.objectContaining(expectedArray[1])]) ); const result1: DeletionRequest = await repo.findById(deletionRequestEntity1.id); @@ -218,9 +210,9 @@ describe(DeletionRequestRepo.name, () => { }); it('should find deletionRequests to execute with limit = 2', async () => { - const { expectedArray, threshold } = await setup(); + const { expectedArray } = await setup(); - const results = await repo.findAllItemsToExecution(threshold, 2); + const results = await repo.findAllItems(2); expect(results.length).toEqual(2); @@ -232,44 +224,114 @@ describe(DeletionRequestRepo.name, () => { }); }); - describe('countPendingDeletionRequests', () => { - describe('when there is no deletionRequest with status pending', () => { - it('should return zero', async () => { - const result = await repo.countPendingDeletionRequests(); - - expect(result).toEqual(0); + describe('findAllFailedItems', () => { + const setup = async () => { + const limit = 10; + const olderThan = new Date(2023, 11, 1); + const newerThan = new Date(2023, 8, 1); + + const deletionRequestEntity1: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 7, 1), + updatedAt: new Date(2023, 8, 2), + deleteAfter: new Date(2023, 8, 1), + status: StatusModel.PENDING, + }); + const deletionRequestEntity2: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 7, 1), + updatedAt: new Date(2023, 8, 2), + deleteAfter: new Date(2023, 8, 1), + status: StatusModel.FAILED, + }); + const deletionRequestEntity3: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 8, 1), + updatedAt: new Date(2023, 7, 1), + deleteAfter: new Date(2023, 9, 1), + status: StatusModel.FAILED, + }); + const deletionRequestEntity4: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 8, 1), + updatedAt: new Date(2023, 8, 1), + deleteAfter: new Date(2023, 9, 1), + status: StatusModel.REGISTERED, }); - }); - describe('when there are deletionRequests with status pending', () => { - const setup = async () => { - const deletionRequestWithStatusPending: DeletionRequestEntity[] = deletionRequestEntityFactory.buildListWithId( - 5, - { - status: StatusModel.PENDING, - } - ); + await em.persistAndFlush([ + deletionRequestEntity1, + deletionRequestEntity2, + deletionRequestEntity3, + deletionRequestEntity4, + ]); + em.clear(); - const deletionRequestWithStatusRegistered: DeletionRequestEntity[] = - deletionRequestEntityFactory.buildListWithId(2, { - status: StatusModel.REGISTERED, - }); + return { limit, olderThan, newerThan, deletionRequestEntity1, deletionRequestEntity2 }; + }; - const expectedNumberDeletionRequestWithStatusPending = deletionRequestWithStatusPending.length; + it('should throw an error if olderThan is less than newerThan', async () => { + const { limit } = await setup(); - await em.persistAndFlush([...deletionRequestWithStatusPending, ...deletionRequestWithStatusRegistered]); - em.clear(); + const olderThan = new Date(2023, 10, 1); + const newerThan = new Date(2023, 11, 1); - return { expectedNumberDeletionRequestWithStatusPending }; - }; + await expect(repo.findAllFailedItems(limit, olderThan, newerThan)).rejects.toThrow(); + }); - it('should count deletionRequests with status pending and return proper number', async () => { - const { expectedNumberDeletionRequestWithStatusPending } = await setup(); + it('should return failed deletion requests within the specified date range', async () => { + const { limit, olderThan, newerThan, deletionRequestEntity1, deletionRequestEntity2 } = await setup(); - const result = await repo.countPendingDeletionRequests(); + const results = await repo.findAllFailedItems(limit, olderThan, newerThan); - expect(result).toEqual(expectedNumberDeletionRequestWithStatusPending); + expect(results.length).toBe(2); + expect(results[0].id).toBe(deletionRequestEntity1.id); + expect(results[1].id).toBe(deletionRequestEntity2.id); + }); + }); + + describe('findInProgressCount', () => { + const setup = async () => { + const newerThan = new Date(2023, 8, 1); + + const deletionRequestEntity1: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 7, 1), + updatedAt: new Date(2023, 8, 2), + deleteAfter: new Date(2023, 8, 1), + status: StatusModel.PENDING, + }); + const deletionRequestEntity2: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 7, 1), + updatedAt: new Date(2023, 8, 2), + deleteAfter: new Date(2023, 8, 1), + status: StatusModel.FAILED, }); + const deletionRequestEntity3: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 8, 1), + updatedAt: new Date(2023, 7, 1), + deleteAfter: new Date(2023, 9, 1), + status: StatusModel.FAILED, + }); + const deletionRequestEntity4: DeletionRequestEntity = deletionRequestEntityFactory.build({ + createdAt: new Date(2023, 8, 1), + updatedAt: new Date(2023, 8, 1), + deleteAfter: new Date(2023, 9, 1), + status: StatusModel.REGISTERED, + }); + + await em.persistAndFlush([ + deletionRequestEntity1, + deletionRequestEntity2, + deletionRequestEntity3, + deletionRequestEntity4, + ]); + em.clear(); + + return { newerThan }; + }; + + it('should return the count of deletionRequests with status pending or failed', async () => { + const { newerThan } = await setup(); + + const result = await repo.findInProgressCount(newerThan); + + expect(result).toBe(2); }); }); diff --git a/apps/server/src/modules/deletion/repo/deletion-request.repo.ts b/apps/server/src/modules/deletion/repo/deletion-request.repo.ts index ed608270bd0..986129ecd1f 100644 --- a/apps/server/src/modules/deletion/repo/deletion-request.repo.ts +++ b/apps/server/src/modules/deletion/repo/deletion-request.repo.ts @@ -6,6 +6,7 @@ import { DeletionRequest } from '../domain/do'; import { DeletionRequestEntity } from './entity'; import { DeletionRequestMapper } from './mapper'; import { DeletionRequestScope } from './scope'; +import { StatusModel } from '../domain/types'; @Injectable() export class DeletionRequestRepo { @@ -31,13 +32,34 @@ export class DeletionRequestRepo { await this.em.flush(); } - async findAllItemsToExecution(threshold: number, limit?: number): Promise { - const currentDate = new Date(); - const modificationThreshold = new Date(Date.now() - threshold); - const scope = new DeletionRequestScope().byDeleteAfter(currentDate).byStatus(modificationThreshold); - const order = { createdAt: SortOrder.desc }; + async findAllItems(limit: number): Promise { + const scope = new DeletionRequestScope(); + scope.byDeleteAfter(new Date()); + scope.byStatusAndDate([StatusModel.REGISTERED]); - const [deletionRequestEntities] = await this.em.findAndCount(DeletionRequestEntity, scope.query, { + const order = { createdAt: SortOrder.asc }; + + const deletionRequestEntities = await this.em.find(DeletionRequestEntity, scope.query, { + limit, + orderBy: order, + }); + + const mapped: DeletionRequest[] = deletionRequestEntities.map((entity) => DeletionRequestMapper.mapToDO(entity)); + + return mapped; + } + + async findAllFailedItems(limit: number, olderThan: Date, newerThan: Date): Promise { + if (olderThan < newerThan) { + throw new Error('olderThan must be greater than newerThan'); + } + const scope = new DeletionRequestScope(); + scope.byDeleteAfter(new Date()); + scope.byStatusAndDate([StatusModel.FAILED, StatusModel.PENDING], olderThan, newerThan); + + const order = { createdAt: SortOrder.asc }; + + const deletionRequestEntities = await this.em.find(DeletionRequestEntity, scope.query, { limit, orderBy: order, }); @@ -47,12 +69,13 @@ export class DeletionRequestRepo { return mapped; } - async countPendingDeletionRequests(): Promise { - const scope = new DeletionRequestScope().byStatusPending(); + public async findInProgressCount(newerThan: Date): Promise { + const scope = new DeletionRequestScope(); + scope.byStatusAndDate([StatusModel.PENDING, StatusModel.FAILED], undefined, newerThan); - const numberItemsWithStatusPending: number = await this.em.count(DeletionRequestEntity, scope.query); + const count = await this.em.count(DeletionRequestEntity, scope.query); - return numberItemsWithStatusPending; + return count; } async update(deletionRequest: DeletionRequest): Promise { diff --git a/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts b/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts index 971b712cab7..27801cbbe89 100644 --- a/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts +++ b/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts @@ -1,7 +1,7 @@ import { BaseEntityWithTimestamps } from '@shared/domain/entity/base.entity'; import { EntityId } from '@shared/domain/types'; import { ObjectId } from 'bson'; -import { Entity, Property, Index } from '@mikro-orm/core'; +import { Entity, Property, Index, Unique } from '@mikro-orm/core'; import { DomainOperationReport, DomainDeletionReport } from '../../domain/interface'; import { DomainName } from '../../domain/types'; @@ -17,6 +17,7 @@ export interface DeletionLogEntityProps { } @Entity({ tableName: 'deletionlogs' }) +@Unique({ properties: ['domain', 'deletionRequestId'] }) // TODO check if this is correct, if we want to try to delete multiple times, it should not create a new log entry, but update it. Or first remove it export class DeletionLogEntity extends BaseEntityWithTimestamps { @Property() @Index() diff --git a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts index ae043621731..2966cbd62e8 100644 --- a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts +++ b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts @@ -32,40 +32,112 @@ describe(DeletionRequestScope.name, () => { }); }); - describe('byStatus', () => { + describe('byStatusAndDate', () => { const setup = () => { - const fifteenMinutesAgo = new Date(Date.now() - 15 * 60 * 1000); // 15 minutes ago - const expectedQuery = { - $or: [ - { status: StatusModel.FAILED }, + const newerThan = new Date(Date.now() - 15 * 60 * 1000); // 15 minutes ago + const olderThan = new Date(Date.now() - 5 * 60 * 1000); // 5 minutes ago + + const expectedQueryOlder = { + status: { + $in: [StatusModel.FAILED, StatusModel.PENDING], + }, + $and: [ + { + updatedAt: { + $lt: olderThan, + }, + }, + ], + }; + const expectedQueryNewer = { + status: { + $in: [StatusModel.FAILED, StatusModel.PENDING], + }, + $and: [ { - $and: [ - { status: [StatusModel.REGISTERED, StatusModel.PENDING] }, - { updatedAt: { $lt: fifteenMinutesAgo } }, - ], + updatedAt: { + $gte: newerThan, + }, }, ], }; + const expectedQueryOlderAndNewer = { + status: { + $in: [StatusModel.FAILED, StatusModel.PENDING], + }, + $and: [ + { + updatedAt: { + $lt: olderThan, + }, + }, + { + updatedAt: { + $gte: newerThan, + }, + }, + ], + }; + const expectedQueryNoDates = { + status: { + $in: [StatusModel.FAILED, StatusModel.PENDING], + }, + }; + return { - expectedQuery, - fifteenMinutesAgo, + expectedQueryOlder, + expectedQueryNewer, + expectedQueryOlderAndNewer, + expectedQueryNoDates, + olderThan, + newerThan, }; }; - describe('when fifteenMinutesAgo is set', () => { + describe('when olderThan is set', () => { it('should add query', () => { - const { expectedQuery, fifteenMinutesAgo } = setup(); + const { expectedQueryOlder, olderThan } = setup(); - const result = scope.byStatus(fifteenMinutesAgo); + const result = scope.byStatusAndDate([StatusModel.FAILED, StatusModel.PENDING], olderThan); expect(result).toBeInstanceOf(DeletionRequestScope); - expect(scope.query).toEqual(expectedQuery); + expect(scope.query).toEqual(expectedQueryOlder); + }); + }); + describe('when newerThan is set', () => { + it('should add query', () => { + const { expectedQueryNewer, newerThan } = setup(); + + const result = scope.byStatusAndDate([StatusModel.FAILED, StatusModel.PENDING], undefined, newerThan); + + expect(result).toBeInstanceOf(DeletionRequestScope); + expect(scope.query).toEqual(expectedQueryNewer); + }); + }); + describe('when olderThan and newerThan are set', () => { + it('should add query', () => { + const { expectedQueryOlderAndNewer, olderThan, newerThan } = setup(); + + const result = scope.byStatusAndDate([StatusModel.FAILED, StatusModel.PENDING], olderThan, newerThan); + + expect(result).toBeInstanceOf(DeletionRequestScope); + expect(scope.query).toEqual(expectedQueryOlderAndNewer); + }); + }); + describe('when olderThan and newerThan are not set', () => { + it('should add query', () => { + const { expectedQueryNoDates } = setup(); + + const result = scope.byStatusAndDate([StatusModel.FAILED, StatusModel.PENDING]); + + expect(result).toBeInstanceOf(DeletionRequestScope); + expect(scope.query).toEqual(expectedQueryNoDates); }); }); }); - describe('byStatusPending', () => { + describe('byStatusAndDate for Pending', () => { const setup = () => { - const expectedQuery = { status: [StatusModel.PENDING] }; + const expectedQuery = { status: { $in: [StatusModel.PENDING] } }; return { expectedQuery }; }; @@ -74,7 +146,7 @@ describe(DeletionRequestScope.name, () => { it('should add query', () => { const { expectedQuery } = setup(); - const result = scope.byStatusPending(); + const result = scope.byStatusAndDate([StatusModel.PENDING]); expect(result).toBeInstanceOf(DeletionRequestScope); expect(scope.query).toEqual(expectedQuery); diff --git a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts index 6e602f0690c..5c0d00f5415 100644 --- a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts +++ b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts @@ -1,4 +1,5 @@ import { Scope } from '@shared/repo/scope'; +import { FilterQuery } from '@mikro-orm/core'; import { DeletionRequestEntity } from '../entity'; import { StatusModel } from '../../domain/types'; @@ -9,22 +10,22 @@ export class DeletionRequestScope extends Scope { return this; } - byStatus(fifteenMinutesAgo: Date): this { - this.addQuery({ - $or: [ - { status: StatusModel.FAILED }, - { - $and: [{ status: [StatusModel.REGISTERED, StatusModel.PENDING] }, { updatedAt: { $lt: fifteenMinutesAgo } }], - }, - ], - }); + byStatusAndDate(status: StatusModel[], olderThan?: Date, newerThan?: Date): this { + const query: FilterQuery = { status: { $in: status } }; - return this; - } + const dateConditions: FilterQuery[] = []; + if (olderThan) { + dateConditions.push({ updatedAt: { $lt: olderThan } }); + } + if (newerThan) { + dateConditions.push({ updatedAt: { $gte: newerThan } }); + } - byStatusPending(): this { - this.addQuery({ status: [StatusModel.PENDING] }); + if (dateConditions.length > 0) { + query.$and = dateConditions; + } + this.addQuery(query); return this; } } diff --git a/apps/server/src/modules/server/admin-api-server.config.ts b/apps/server/src/modules/server/admin-api-server.config.ts index ff2b53cd7c0..d0dc54349ed 100644 --- a/apps/server/src/modules/server/admin-api-server.config.ts +++ b/apps/server/src/modules/server/admin-api-server.config.ts @@ -19,11 +19,17 @@ export interface AdminApiServerConfig } const config: AdminApiServerConfig = { - ADMIN_API__MODIFICATION_THRESHOLD_MS: Configuration.get('ADMIN_API__MODIFICATION_THRESHOLD_MS') as number, - ADMIN_API__MAX_CONCURRENT_DELETION_REQUESTS: Configuration.get( - 'ADMIN_API__MAX_CONCURRENT_DELETION_REQUESTS' + ADMIN_API__DELETION_DELETE_AFTER_MINUTES: Configuration.get('ADMIN_API__DELETION_DELETE_AFTER_MINUTES') as number, + ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS: Configuration.get( + 'ADMIN_API__DELETION_MODIFICATION_THRESHOLD_MS' + ) as number, + ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS: Configuration.get( + 'ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS' ) as number, ADMIN_API__DELETION_DELAY_MILLISECONDS: Configuration.get('ADMIN_API__DELETION_DELAY_MILLISECONDS') as number, + ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS: Configuration.get( + 'ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS' + ) as number, NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string, EXIT_ON_ERROR: Configuration.get('EXIT_ON_ERROR') as boolean, AVAILABLE_LANGUAGES: (Configuration.get('I18N__AVAILABLE_LANGUAGES') as string).split(',') as LanguageType[], diff --git a/apps/server/src/modules/user/interfaces/user-config.ts b/apps/server/src/modules/user/interfaces/user-config.ts index 8923e95fb1d..769f2e20af0 100644 --- a/apps/server/src/modules/user/interfaces/user-config.ts +++ b/apps/server/src/modules/user/interfaces/user-config.ts @@ -1,4 +1,5 @@ export interface UserConfig { AVAILABLE_LANGUAGES: string[]; TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION: string; + CALENDAR_SERVICE_ENABLED: boolean; } diff --git a/apps/server/src/modules/user/service/user.service.spec.ts b/apps/server/src/modules/user/service/user.service.spec.ts index ec59a7dc5fd..acbb232e94b 100644 --- a/apps/server/src/modules/user/service/user.service.spec.ts +++ b/apps/server/src/modules/user/service/user.service.spec.ts @@ -851,6 +851,8 @@ describe('UserService', () => { userRepo.findByIdOrNull.mockResolvedValueOnce(user); userRepo.deleteUser.mockResolvedValue(1); + config.get.mockReturnValue(true); + return { expectedResult, user, diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index 3ed16f65725..1a06ef69f59 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -250,9 +250,14 @@ export class UserService implements DeletionService, IEventHandler('CALENDAR_SERVICE_ENABLED')) { + const calendarEventsDeleted = await this.removeCalendarEvents(userId); + subdomainOperation.push(calendarEventsDeleted); + } const numberOfDeletedUsers = await this.userRepo.deleteUser(userId); @@ -263,7 +268,7 @@ export class UserService implements DeletionService, IEventHandler:],[:]" }, - "MODIFICATION_THRESHOLD_MS": { + "DELETION_DELETE_AFTER_MINUTES": { "type": "number", - "default": 60000, - "description": "threshold in milliseconds to get deletionRequest to delete" + "default": 43200, + "description": "threshold in minutes, after which a deletion request can be processed" }, - "MAX_CONCURRENT_DELETION_REQUESTS": { + "DELETION_MODIFICATION_THRESHOLD_MS": { "type": "number", - "default": 100, - "description": "max amount of deletion requests proccessed concurently" + "default": 300000, + "description": "threshold in milliseconds to try again to process a hanging Pending or Failed deletion requests" + }, + "DELETION_CONSIDER_FAILED_AFTER_MS" : { + "type": "number", + "default": 360000000, + "description": "threshold in milliseconds to stop trying to process Pending or Failed deletion requests" + }, + "DELETION_MAX_CONCURRENT_DELETION_REQUESTS": { + "type": "number", + "default": 20, + "description": "max amount of deletion requests to be processed simultaneously" }, "DELETION_DELAY_MILLISECONDS": { "type": "number", "default": 5000, - "description": "deley in milliseconds to give time modules to perform deletion proccess" + "description": "delay in milliseconds to sleep before processing another batch of deletion requests" } }, "default": {} diff --git a/package.json b/package.json index fadfb01bef5..7da8d6a9756 100644 --- a/package.json +++ b/package.json @@ -61,6 +61,7 @@ "nest:start:debug": "nest start server --debug --watch", "nest:start:prod": "node dist/apps/server/apps/server.app", "nest:start:admin-api-server": "nest start admin-api-server", + "nest:start:admin-api-server:dev": "nest start admin-api-server --watch", "nest:start:admin-api-server:debug": "nest start admin-api-server --debug --watch", "nest:start:admin-api-server:prod": "node dist/apps/server/apps/admin-api-server.app", "nest:start:board-collaboration": "nest start board-collaboration",