Skip to content
Merged
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
ca7185d
BC-7561 - a bit of cleanup
virgilchiriac Jan 15, 2025
dba8d83
BC-7561 - improve logic for bulk deletion
virgilchiriac Jan 20, 2025
a364574
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 20, 2025
ee088b5
fix api tets and change sorting
virgilchiriac Jan 20, 2025
e3b952c
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 20, 2025
fc5ce93
Fix some tests
virgilchiriac Jan 21, 2025
da3d694
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 21, 2025
3772528
split pending and failed into a separate job
virgilchiriac Jan 22, 2025
42127e2
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 22, 2025
7b8eccf
implement type casting for command flags
virgilchiriac Jan 22, 2025
9745671
fix linter / remove tests
virgilchiriac Jan 22, 2025
26444f0
unset db debug mode
virgilchiriac Jan 22, 2025
d10659a
undo flag in console
virgilchiriac Jan 22, 2025
e8ab582
fix test
virgilchiriac Jan 23, 2025
eb96b3f
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 23, 2025
8b8e624
fix tests
virgilchiriac Jan 23, 2025
3cc6551
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 23, 2025
86bfdcf
add cronjob
virgilchiriac Jan 23, 2025
51ea68e
fix test
virgilchiriac Jan 23, 2025
3114233
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 23, 2025
ec23c1d
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 24, 2025
e80920a
improve test cov
virgilchiriac Jan 24, 2025
2ec2d7d
Merge branch 'BC-7561-batch-delete-fix' of https://github.com/hpi-sch…
virgilchiriac Jan 24, 2025
a42c73b
fixes from revviews
virgilchiriac Jan 24, 2025
e216fc9
fix query
virgilchiriac Jan 24, 2025
92dc292
hacky type casting for console args
virgilchiriac Jan 27, 2025
e73ff3a
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 27, 2025
d070847
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 27, 2025
88906bd
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 27, 2025
a7e13c7
undo package.json command
virgilchiriac Jan 27, 2025
3d25423
Merge branch 'BC-7561-batch-delete-fix' of https://github.com/hpi-sch…
virgilchiriac Jan 27, 2025
c40c290
throttle down max number of processing requests in batch
virgilchiriac Jan 28, 2025
e6e8051
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 28, 2025
6adc016
fix max concurency / limit and some tests
virgilchiriac Jan 29, 2025
90dc237
Merge branch 'BC-7561-batch-delete-fix' of https://github.com/hpi-sch…
virgilchiriac Jan 29, 2025
39a1d67
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Jan 29, 2025
b2a607b
optimization
virgilchiriac Jan 29, 2025
b6d183a
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Feb 3, 2025
490e35b
Merge branch 'BC-7561-batch-delete-fix' of https://github.com/hpi-sch…
virgilchiriac Feb 3, 2025
4ebe5f8
fix test
virgilchiriac Feb 4, 2025
240b55e
fix pending count query
virgilchiriac Feb 5, 2025
bc08140
improve tests readability
virgilchiriac Feb 5, 2025
634705f
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Feb 5, 2025
cc1dd23
Merge branch 'main' into BC-7561-batch-delete-fix
virgilchiriac Feb 5, 2025
caba67f
fix linter
virgilchiriac Feb 5, 2025
1e62200
Merge branch 'BC-7561-batch-delete-fix' of https://github.com/hpi-sch…
virgilchiriac Feb 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
throttle down max number of processing requests in batch
avoid loading node by checking how many requests
are already currently in progress
  • Loading branch information
virgilchiriac committed Jan 28, 2025
commit c40c290e24eb37cc625ec25d72b0b53a43952511
Original file line number Diff line number Diff line change
@@ -160,15 +160,23 @@ describe(DeletionRequestUc.name, () => {
const deletionRequest = deletionRequestFactory.buildWithId({ deleteAfter: new Date('2023-01-01') });

configService.get.mockImplementation((key) => deletionTestConfig()[key]);
deletionRequestService.findInProgressCount.mockResolvedValueOnce(1);

return { deletionRequest };
};
it('should call deletionRequestService.findInProgressCount', async () => {
setup();

await uc.executeDeletionRequests();

expect(deletionRequestService.findInProgressCount).toHaveBeenCalled();
});
it('should call deletionRequestService.findAllItemsToExecute', async () => {
setup();

await uc.executeDeletionRequests();

expect(deletionRequestService.findAllItemsToExecute).toHaveBeenCalled();
expect(deletionRequestService.findAllItemsToExecute).toHaveBeenCalledWith(1, undefined);
});

it('should call deletionRequestService.markDeletionRequestAsPending to update status of deletionRequests', async () => {
@@ -203,6 +211,7 @@ describe(DeletionRequestUc.name, () => {
});

deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequest]);
deletionRequestService.findInProgressCount.mockResolvedValueOnce(0);

await uc.executeDeletionRequests();

@@ -215,6 +224,7 @@ describe(DeletionRequestUc.name, () => {
const deletionRequestToExecute = deletionRequestFactory.build({ deleteAfter: new Date('2023-01-01') });

deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequestToExecute]);
deletionRequestService.findInProgressCount.mockResolvedValueOnce(1);
eventBus.publish.mockRejectedValueOnce(new Error());

return {
10 changes: 7 additions & 3 deletions apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts
Original file line number Diff line number Diff line change
@@ -81,14 +81,18 @@ export class DeletionRequestUc implements IEventHandler<DataDeletedEvent> {
async executeDeletionRequests(limit?: number, getFailed?: boolean): Promise<void> {
this.logger.debug({ action: 'executeDeletionRequests', limit });

limit = limit ?? this.configService.get<number>('ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS');
const inProgress = await this.deletionRequestService.findInProgressCount();

const max = limit
? limit - inProgress
: this.configService.get<number>('ADMIN_API__DELETION_MAX_CONCURRENT_DELETION_REQUESTS') - inProgress;

let deletionRequests: DeletionRequest[] = [];

if (limit > 0) {
if (max > 0) {
do {
// eslint-disable-next-line no-await-in-loop
deletionRequests = await this.deletionRequestService.findAllItemsToExecute(limit, getFailed);
deletionRequests = await this.deletionRequestService.findAllItemsToExecute(max, getFailed);

this.logger.debug({ action: 'processing deletion request', deletionRequests });

Original file line number Diff line number Diff line change
@@ -152,6 +152,36 @@ describe(DeletionRequestService.name, () => {
});
});

describe('findInProgressCount', () => {
describe('when finding in progress deletionRequests', () => {
const setup = () => {
const thresholdNewerMs = configService.get<number>('ADMIN_API__DELETION_CONSIDER_FAILED_AFTER_MS') ?? 1000;
const newerThan = new Date(Date.now() - thresholdNewerMs);

const count = 2;

deletionRequestRepo.findInProgressCount.mockResolvedValue(count);

return { count, newerThan };
};

it('should call deletionRequestRepo.findInProgressCount', async () => {
const { newerThan } = setup();

await service.findInProgressCount();

expect(deletionRequestRepo.findInProgressCount).toBeCalledWith(newerThan);
});

it('should return count of in progress deletionRequests', async () => {
const { count } = setup();
const result = await service.findInProgressCount();

expect(result).toEqual(count);
});
});
});

describe('update', () => {
describe('when updating deletionRequest', () => {
const setup = () => {
Original file line number Diff line number Diff line change
@@ -48,14 +48,19 @@ export class DeletionRequestService {
return deletionRequest;
}

async findAllItemsToExecute(limit: number, getFailed = false): Promise<DeletionRequest[]> {
public async findAllItemsToExecute(limit: number, getFailed = false): Promise<DeletionRequest[]> {
const deletionRequests = getFailed
? await this.deletionRequestRepo.findAllFailedItems(limit, this.olderThan, this.newerThan)
: await this.deletionRequestRepo.findAllItems(limit);

return deletionRequests;
}

public async findInProgressCount(): Promise<number> {
const count = await this.deletionRequestRepo.findInProgressCount(this.newerThan);
return count;
}

async update(deletionRequestToUpdate: DeletionRequest): Promise<void> {
await this.deletionRequestRepo.update(deletionRequestToUpdate);
}
Original file line number Diff line number Diff line change
@@ -286,6 +286,55 @@ describe(DeletionRequestRepo.name, () => {
});
});

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);
});
});

describe('update', () => {
describe('when updating deletionRequest', () => {
const setup = async () => {
Original file line number Diff line number Diff line change
@@ -69,6 +69,15 @@ export class DeletionRequestRepo {
return mapped;
}

public async findInProgressCount(newerThan: Date): Promise<number> {
const scope = new DeletionRequestScope();
scope.byStatusAndDate([StatusModel.PENDING, StatusModel.FAILED], undefined, newerThan);

const count = await this.em.count(DeletionRequestEntity, scope.query);

return count;
}

async update(deletionRequest: DeletionRequest): Promise<void> {
const deletionRequestEntity = DeletionRequestMapper.mapToEntity(deletionRequest);
const referencedEntity = this.em.getReference(DeletionRequestEntity, deletionRequestEntity.id);

Unchanged files with check annotations Beta

// application imports
import { LegacyLogger, Logger } from '@core/logger';
import { RedisIoAdapter } from '@infra/socketio';

Check warning on line 10 in apps/server/src/apps/board-collaboration.app.ts

GitHub Actions / nest_lint

'@infra/socketio' import is restricted from being used by a pattern. apps-modules may NOT import from apps, @infra, @shared, or migrations
import { BoardCollaborationModule } from '@modules/board/board-collaboration.app.module';
import { ExpressAdapter } from '@nestjs/platform-express';
import { SwaggerDocumentOptions } from '@nestjs/swagger';
import { DeletionConsoleModule } from '@modules/deletion-console/deletion-console.app.module';
import { BootstrapConsole } from 'nestjs-console';
async function run() {

Check warning on line 5 in apps/server/src/apps/deletion-console.app.ts

GitHub Actions / nest_lint

Missing return type on function
const bootstrap = new BootstrapConsole({
module: DeletionConsoleModule,
useDecorators: true,
// application imports
import { LegacyLogger } from '@core/logger';
import { FilesStorageApiModule } from '@modules/files-storage/files-storage-api.app.module';
import { API_VERSION_PATH } from '@modules/files-storage/files-storage.const';

Check warning on line 13 in apps/server/src/apps/files-storage.app.ts

GitHub Actions / nest_lint

'@modules/files-storage/files-storage.const' import is restricted from being used by a pattern. Do not deep import from a module
import { SwaggerDocumentOptions } from '@nestjs/swagger';
import { enableOpenApiDocs } from './helpers';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
/* istanbul ignore file */
/* eslint-disable no-console */
import { NestFactory } from '@nestjs/core';

Check warning on line 3 in apps/server/src/apps/h5p-editor.app.ts

GitHub Actions / nest_lint

Filename 'h5p-editor.app.ts' does not match kebabcase
import { ExpressAdapter } from '@nestjs/platform-express';
import express from 'express';
/* istanbul ignore file */
/* eslint-disable no-console */
import { LegacyLogger } from '@core/logger';

Check warning on line 3 in apps/server/src/apps/h5p-library-management.app.ts

GitHub Actions / nest_lint

Filename 'h5p-library-management.app.ts' does not match kebabcase
import { H5PLibraryManagementService } from '@modules/h5p-library-management';
import { H5PLibraryManagementModule } from '@modules/h5p-library-management/h5p-library-management.app.module';
import { NestFactory } from '@nestjs/core';
export class AppStartLoggable implements Loggable {
constructor(private readonly info: AppStartInfo) {}
getLogMessage(): LogMessage {

Check warning on line 13 in apps/server/src/apps/helpers/app-start-loggable.ts

GitHub Actions / nest_lint

Missing accessibility modifier on method definition getLogMessage
const data: LogMessageData = { appName: this.info.appName };
if (this.info.port !== undefined) {
export class PrometheusMetricsSetupStateLoggable implements Loggable {
constructor(private readonly state: PrometheusMetricsSetupState) {}
getLogMessage(): LogMessage {

Check warning on line 22 in apps/server/src/apps/helpers/metrics/prometheus-metrics.ts

GitHub Actions / nest_lint

Missing accessibility modifier on method definition getLogMessage
return {
message: 'Setting up Prometheus metrics...',
data: {
}
}
export const addPrometheusMetricsMiddlewaresIfEnabled = (logger: Logger, app: Express) => {

Check warning on line 32 in apps/server/src/apps/helpers/metrics/prometheus-metrics.ts

GitHub Actions / nest_lint

Missing return type on function
if (!PrometheusMetricsConfig.instance.isEnabled) {
logger.debug(
new PrometheusMetricsSetupStateLoggable(
);
};
export const createAndStartPrometheusMetricsAppIfEnabled = (logger: Logger) => {

Check warning on line 52 in apps/server/src/apps/helpers/metrics/prometheus-metrics.ts

GitHub Actions / nest_lint

Missing return type on function
if (!PrometheusMetricsConfig.instance.isEnabled) {
logger.debug(
new PrometheusMetricsSetupStateLoggable(PrometheusMetricsSetupState.FEATURE_DISABLED_APP_WILL_NOT_BE_CREATED)
this._collectMetricsRouteMetrics = Configuration.get('PROMETHEUS_METRICS_COLLECT_METRICS_ROUTE_METRICS') as boolean;
}
public static get instance() {

Check warning on line 44 in apps/server/src/apps/helpers/metrics/prometheus/config.ts

GitHub Actions / nest_lint

Public accessibility modifier on get property accessor instance
if (this._instance === undefined) {
this._instance = new this();
}
ENV NODE_ENV=production
ENV NO_COLOR="true"
CMD npm run start

Check warning on line 27 in Dockerfile

GitHub Actions / build_and_push

JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals

JSONArgsRecommended: JSON arguments recommended for CMD to prevent unintended behavior related to OS signals More info: https://docs.docker.com/go/dockerfile/rule/json-args-recommended/
ARG BASE_IMAGE
FROM $BASE_IMAGE

Check warning on line 2 in Dockerfile.filepreview

GitHub Actions / build_and_push

Default value for global ARG results in an empty or invalid base image name

InvalidDefaultArgInFrom: Default value for ARG $BASE_IMAGE results in empty or invalid base image name More info: https://docs.docker.com/go/dockerfile/rule/invalid-default-arg-in-from/
RUN apk add --no-cache imagemagick imagemagick-heic imagemagick-jpeg imagemagick-pdf imagemagick-raw imagemagick-svg imagemagick-tiff imagemagick-webp