Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BC-5424 - persistent storage for tldraw #4685

Merged
merged 34 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a40ee1b
add env vars related to tldraw asset upload
davwas Jan 9, 2024
4190ed4
fix resolved user school id
davwas Jan 9, 2024
16665ea
fix package lock json
davwas Jan 10, 2024
9844450
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 10, 2024
5ae793e
fix tldraw configs
davwas Jan 11, 2024
a52712c
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 11, 2024
5971daf
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 12, 2024
2727511
fix formatting
davwas Jan 12, 2024
9fe419a
Add recursive deletion of files for drawing entity
blazejpass Jan 15, 2024
d6f7cf4
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 18, 2024
b1649b2
Change roles permission for users to upload images for students in tl…
blazejpass Jan 22, 2024
6bd3dc6
Fix lint
blazejpass Jan 22, 2024
d112e77
Merge remote-tracking branch 'origin/BC-5424-tldraw-persistent-storag…
blazejpass Jan 22, 2024
31de79e
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 22, 2024
f5d4d63
Add test for drawing authorizable in reference loader
blazejpass Jan 22, 2024
5a79db3
Merge remote-tracking branch 'origin/BC-5424-tldraw-persistent-storag…
blazejpass Jan 22, 2024
1c61198
fix user permissions for tldraw assets
davwas Jan 22, 2024
937d407
test tldraw db url env var
davwas Jan 22, 2024
9906ee8
test tldraw db url env var 2
davwas Jan 22, 2024
eccb43b
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 23, 2024
11ad9f3
Merge branch 'main' into BC-5424-tldraw-persistent-storage
CeEv Jan 24, 2024
d10a186
update packages
davwas Jan 24, 2024
107de35
hacky temporary solution proposal
davwas Jan 26, 2024
4fd2c1d
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 26, 2024
4c36cbf
fix test config
davwas Jan 26, 2024
e681fb7
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 29, 2024
9d0745e
fix conflicts
davwas Jan 29, 2024
cf34896
fix schema
davwas Jan 29, 2024
156e6b3
fix comments
davwas Jan 29, 2024
80a6900
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 29, 2024
57c51d5
fix import
davwas Jan 29, 2024
465124d
fix tests
davwas Jan 29, 2024
31fd8cc
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 29, 2024
0d24bb5
Merge branch 'main' into BC-5424-tldraw-persistent-storage
davwas Jan 30, 2024
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
3 changes: 1 addition & 2 deletions apps/server/src/config/database.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ interface GlobalConstants {
DB_URL: string;
DB_PASSWORD?: string;
DB_USERNAME?: string;
TLDRAW_DB_URL: string;
}

const usedGlobals: GlobalConstants = globals;

/** Database URL */
export const { DB_URL, DB_PASSWORD, DB_USERNAME, TLDRAW_DB_URL } = usedGlobals;
export const { DB_URL, DB_PASSWORD, DB_USERNAME } = usedGlobals;
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync {

async visitDrawingElementAsync(drawingElement: DrawingElement): Promise<void> {
await this.drawingElementAdapterService.deleteDrawingBinData(drawingElement.id);
await this.filesStorageClientAdapterService.deleteFilesOfParent(drawingElement.id);
CeEv marked this conversation as resolved.
Show resolved Hide resolved

this.deleteNode(drawingElement);
await this.visitChildrenAsync(drawingElement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
BoardExternalReferenceType,
BoardRoles,
ColumnBoard,
DrawingElement,
UserBoardRoles,
UserRoleEnum,
} from '@shared/domain/domainobject';
Expand Down Expand Up @@ -34,10 +35,12 @@ export class BoardDoAuthorizableService implements AuthorizationLoaderService {
const ids = [...ancestorIds, boardDo.id];
const rootId = ids[0];
const rootBoardDo = await this.boardDoRepo.findById(rootId, 1);
const isDrawingElement = boardDo instanceof DrawingElement;

if (rootBoardDo instanceof ColumnBoard) {
if (rootBoardDo.context?.type === BoardExternalReferenceType.Course) {
const course = await this.courseRepo.findById(rootBoardDo.context.id);
const users = this.mapCourseUsersToUsergroup(course);
const users = this.mapCourseUsersToUsergroup(course, isDrawingElement);
return new BoardDoAuthorizable({ users, id: boardDo.id });
}
} else {
Expand All @@ -47,7 +50,7 @@ export class BoardDoAuthorizableService implements AuthorizationLoaderService {
return new BoardDoAuthorizable({ users: [], id: boardDo.id });
}

private mapCourseUsersToUsergroup(course: Course): UserBoardRoles[] {
private mapCourseUsersToUsergroup(course: Course, isDrawingElement: boolean): UserBoardRoles[] {
const users = [
...course.getTeachersList().map((user) => {
return {
Expand All @@ -72,7 +75,7 @@ export class BoardDoAuthorizableService implements AuthorizationLoaderService {
userId: user.id,
firstName: user.firstName,
lastName: user.lastName,
roles: [BoardRoles.READER],
roles: isDrawingElement ? [BoardRoles.EDITOR] : [BoardRoles.READER],
Copy link
Contributor

@CeEv CeEv Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the default value of this boolean?
For me is this change unclear, can we speak about it?
With this line you create a exception for one element that invert the authorisation (logic/role), it look like the first step into the wrong direction.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default should be false and only set the student to editors when accessing a drawing element.

Currently all students are readers. In the submission case we actually overrule the permission in the UC layer.
As this is a temporary solution until we have the proper authZ set in place.

protected async checkSubmissionItemWritePermission(userId: EntityId, submissionItem: SubmissionItem) {

I would be better to set exceptions outside this function like how the submission is handling it to keep the core code clean for now.

Copy link
Contributor Author

@davwas davwas Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is false.

We want to store uploaded assets from tldraw in s3 storage - by default every authorized user is supposed to be able to upload to tldraw. There is already a logic in place to upload files to column board nodes. If requested board node is an instance of DrawingElement (which is Tldraw) we want to give student role access to upload.

The alternative I can think of is to create new authorizable service only for DrawingElements which would basically be a copy paste of board-do-authorizable.service with just this one line changed. We are open to any other suggestions.

userRoleEnum: UserRoleEnum.STUDENT,
};
}),
Expand Down
14 changes: 10 additions & 4 deletions apps/server/src/modules/tldraw/config.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
import { Configuration } from '@hpi-schul-cloud/commons';

export interface TldrawConfig {
TLDRAW_DB_URL: string;
NEST_LOG_LEVEL: string;
INCOMING_REQUEST_TIMEOUT: number;
TLDRAW_DB_COLLECTION_NAME: string;
TLDRAW_DB_FLUSH_SIZE: string;
TLDRAW_DB_MULTIPLE_COLLECTIONS: boolean;
CONNECTION_STRING: string;
FEATURE_TLDRAW_ENABLED: boolean;
TLDRAW_PING_TIMEOUT: number;
TLDRAW_GC_ENABLED: number;
TLDRAW_ASSETS_ENABLED: boolean;
TLDRAW_ASSETS_MAX_SIZE: number;
TLDRAW_ASSETS_ALLOWED_EXTENSIONS_LIST: string;
API_HOST: number;
}

const tldrawConnectionString: string = Configuration.get('TLDRAW_DB_URL') as string;
export const TLDRAW_DB_URL: string = Configuration.get('TLDRAW_DB_URL') as string;
export const TLDRAW_SOCKET_PORT = Configuration.get('TLDRAW__SOCKET_PORT') as number;

const tldrawConfig = {
TLDRAW_DB_URL,
NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string,
INCOMING_REQUEST_TIMEOUT: Configuration.get('INCOMING_REQUEST_TIMEOUT_API') as number,
TLDRAW_DB_COLLECTION_NAME: Configuration.get('TLDRAW__DB_COLLECTION_NAME') as string,
TLDRAW_DB_FLUSH_SIZE: Configuration.get('TLDRAW__DB_FLUSH_SIZE') as number,
TLDRAW_DB_MULTIPLE_COLLECTIONS: Configuration.get('TLDRAW__DB_MULTIPLE_COLLECTIONS') as boolean,
FEATURE_TLDRAW_ENABLED: Configuration.get('FEATURE_TLDRAW_ENABLED') as boolean,
CONNECTION_STRING: tldrawConnectionString,
TLDRAW_PING_TIMEOUT: Configuration.get('TLDRAW__PING_TIMEOUT') as number,
TLDRAW_GC_ENABLED: Configuration.get('TLDRAW__GC_ENABLED') as boolean,
TLDRAW_ASSETS_ENABLED: Configuration.get('TLDRAW__ASSETS_ENABLED') as boolean,
TLDRAW_ASSETS_MAX_SIZE: Configuration.get('TLDRAW__ASSETS_MAX_SIZE') as number,
TLDRAW_ASSETS_ALLOWED_EXTENSIONS_LIST: Configuration.get('TLDRAW__ASSETS_ALLOWED_EXTENSIONS_LIST') as string,
API_HOST: Configuration.get('API_HOST') as string,
};

export const SOCKET_PORT = Configuration.get('TLDRAW__SOCKET_PORT') as number;
export const config = () => tldrawConfig;
4 changes: 2 additions & 2 deletions apps/server/src/modules/tldraw/controller/tldraw.ws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { BadRequestException } from '@nestjs/common';
import { Logger } from '@src/core/logger';
import { AxiosError } from 'axios';
import { WebsocketCloseErrorLoggable } from '../loggable/websocket-close-error.loggable';
import { TldrawConfig, SOCKET_PORT } from '../config';
import { TldrawConfig, TLDRAW_SOCKET_PORT } from '../config';
import { WsCloseCodeEnum, WsCloseMessageEnum } from '../types';
import { TldrawWsService } from '../service';

@WebSocketGateway(SOCKET_PORT)
@WebSocketGateway(TLDRAW_SOCKET_PORT)
export class TldrawWs implements OnGatewayInit, OnGatewayConnection {
@WebSocketServer()
server!: Server;
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/modules/tldraw/repo/tldraw-board.repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class TldrawBoardRepo {
public mdb: MongodbPersistence;

constructor(public readonly configService: ConfigService<TldrawConfig, true>) {
this.connectionString = this.configService.get<string>('CONNECTION_STRING');
this.connectionString = this.configService.get<string>('TLDRAW_DB_URL');
this.collectionName = this.configService.get<string>('TLDRAW_DB_COLLECTION_NAME') ?? 'drawings';
this.flushSize = this.configService.get<number>('TLDRAW_DB_FLUSH_SIZE') ?? 400;
this.multipleCollections = this.configService.get<boolean>('TLDRAW_DB_MULTIPLE_COLLECTIONS');
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/modules/tldraw/tldraw.module.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Module, NotFoundException } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config';
import { createConfigModuleOptions, DB_PASSWORD, DB_USERNAME, TLDRAW_DB_URL } from '@src/config';
import { createConfigModuleOptions, DB_PASSWORD, DB_USERNAME } from '@src/config';
import { CoreModule } from '@src/core';
import { Logger } from '@src/core/logger';
import { MikroOrmModule, MikroOrmModuleSyncOptions } from '@mikro-orm/nestjs';
Expand All @@ -9,7 +9,7 @@ import { RabbitMQWrapperTestModule } from '@infra/rabbitmq';
import { Dictionary, IPrimaryKey } from '@mikro-orm/core';
import { AuthorizationModule } from '@modules/authorization';
import { TldrawDrawing } from './entities';
import { config } from './config';
import { config, TLDRAW_DB_URL } from './config';
import { TldrawService } from './service/tldraw.service';
import { TldrawBoardRepo } from './repo';
import { TldrawController } from './controller/tldraw.controller';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export class ResolvedUserMapper {
dto.lastName = user.lastName;
dto.createdAt = user.createdAt;
dto.updatedAt = user.updatedAt;
dto.schoolId = user.school.toString();
dto.schoolId = user.school.id;
dto.roles = roles.map((role) => {
return { name: role.name, id: role.id };
});
Expand Down
34 changes: 25 additions & 9 deletions config/default.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1430,10 +1430,15 @@
"API_KEY": ""
}
},
"FEATURE_TLDRAW_ENABLED": {
"type": "boolean",
"default": true,
"description": "Enables tldraw feature"
},
"TLDRAW": {
"type": "object",
"description": "Tldraw managing variables.",
"required": ["PING_TIMEOUT", "SOCKET_PORT","GC_ENABLED", "DB_COLLECTION_NAME", "DB_FLUSH_SIZE", "DB_MULTIPLE_COLLECTIONS"],
"description": "Configuration of tldraw related settings",
"required": ["PING_TIMEOUT", "SOCKET_PORT","GC_ENABLED", "DB_COLLECTION_NAME", "DB_FLUSH_SIZE", "DB_MULTIPLE_COLLECTIONS", "ASSETS_ENABLED", "ASSETS_MAX_SIZE", "ASSETS_ALLOWED_EXTENSIONS_LIST"],
"properties": {
"SOCKET_PORT": {
"type": "number",
Expand All @@ -1458,27 +1463,38 @@
"DB_MULTIPLE_COLLECTIONS": {
"type": "boolean",
"description": "DB collection allowing multiple collections for drawing"
},
"ASSETS_ENABLED": {
"type": "boolean",
"description": "Enables uploading assets to tldraw board"
},
"ASSETS_MAX_SIZE": {
"type": "integer",
"description": "Maximum asset size in bytes"
},
"ASSETS_ALLOWED_EXTENSIONS_LIST": {
"type": "string",
"description": "List with allowed assets extensions, comma separated, empty if all extensions should be allowed",
"examples": ["png,jpg,jpeg,svg,webp"]
}
},
"default": {
"SOCKET_PORT": 3345,
"PING_TIMEOUT": 10000,
"PING_TIMEOUT": 30000,
"GC_ENABLED": true,
"DB_COLLECTION_NAME": "drawings",
"DB_FLUSH_SIZE": 400,
"DB_MULTIPLE_COLLECTIONS": false
"DB_MULTIPLE_COLLECTIONS": false,
"ASSETS_ENABLED": true,
"ASSETS_MAX_SIZE": 25000000,
"ASSETS_ALLOWED_EXTENSIONS_LIST": ""
}
},
"TLDRAW_DB_URL": {
"type": "string",
"default": "mongodb://127.0.0.1:27017/tldraw",
"description": "DB connection url"
},
"FEATURE_TLDRAW_ENABLED": {
"type": "boolean",
"default": true,
"description": "Tldraw feature enabled"
},
"TLDRAW_URI": {
"type": "string",
"default": "http://localhost:3349",
Expand Down
5 changes: 0 additions & 5 deletions config/globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,12 @@ switch (NODE_ENV) {
}

let defaultDbUrl = null;
let defaultTldrawDbUrl = null;
switch (NODE_ENV) {
case ENVIRONMENTS.TEST:
defaultDbUrl = 'mongodb://127.0.0.1:27017/schulcloud-test';
break;
default:
defaultDbUrl = 'mongodb://127.0.0.1:27017/schulcloud';
defaultTldrawDbUrl = 'mongodb://127.0.0.1:27017/tldraw';
}

const globals = {
Expand Down Expand Up @@ -106,9 +104,6 @@ const globals = {

// calendar
CALENDAR_URI: process.env.CALENDAR_URI,

// tldraw
TLDRAW_DB_URL: process.env.TLDRAW_DB_URL || defaultTldrawDbUrl,
};

// validation /////////////////////////////////////////////////
Expand Down
5 changes: 4 additions & 1 deletion config/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
"GC_ENABLED": true,
"DB_COLLECTION_NAME": "drawings",
"DB_FLUSH_SIZE": 400,
"DB_MULTIPLE_COLLECTIONS": false
"DB_MULTIPLE_COLLECTIONS": false,
"ASSETS_ENABLED": true,
"ASSETS_MAX_SIZE": 25000000,
"ASSETS_ALLOWED_EXTENSIONS_LIST": ""
}
}
5 changes: 4 additions & 1 deletion src/services/config/publicAppConfigService.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ const exposedVars = [
'FEATURE_ENABLE_LDAP_SYNC_DURING_MIGRATION',
'FEATURE_CTL_CONTEXT_CONFIGURATION_ENABLED',
'FEATURE_SHOW_NEW_CLASS_VIEW_ENABLED',
'FEATURE_TLDRAW_ENABLED',
'FEATURE_CTL_TOOLS_COPY_ENABLED',
'FEATURE_TLDRAW_ENABLED',
'TLDRAW__ASSETS_ENABLED',
'TLDRAW__ASSETS_MAX_SIZE',
'TLDRAW__ASSETS_ALLOWED_EXTENSIONS_LIST',
];

/**
Expand Down
Loading