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-8403 Code improvements #43

Merged
merged 69 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 53 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
38ba503
Add callback to stop the worker service if the client is destroyed.
CeEv Dec 4, 2024
81c2de8
seperate job code
CeEv Dec 4, 2024
898d7ff
Move wait into loop and make it configurable.
CeEv Dec 4, 2024
b94bccd
Outsource handling to private methods.
CeEv Dec 5, 2024
1fc05d4
Add todo
CeEv Dec 5, 2024
98a4d5b
Add comments
CeEv Dec 5, 2024
01e1f49
BC-8403 - WIP: refactor Redis and YRedis modules
SevenWaysDP Dec 6, 2024
07203c3
Rename api to yRedisClient to match with our implicit naming guidlines.
CeEv Dec 9, 2024
23a89e7
Cleanup worker start calling.
CeEv Dec 9, 2024
8b0bf5c
Simplify test setup
CeEv Dec 9, 2024
cbdd9ac
Rename User to YRedisUser.
CeEv Dec 9, 2024
3cedcae
Outsource YRedisUser in additonal file.
CeEv Dec 9, 2024
b760ae3
Extract code example as show case to seperate y-redis logic from uws …
CeEv Dec 9, 2024
a6adeaa
Update y-redis.client.ts
CeEv Dec 9, 2024
cf82a85
Refactor y-redis service structure and add YRedisService implementation
SevenWaysDP Dec 9, 2024
6ba75bb
fixup! Refactor y-redis service structure and add YRedisService imple…
SevenWaysDP Dec 9, 2024
c96f60e
Outsource some small methods.
CeEv Dec 9, 2024
535f2c0
Merge branch 'BC-8403-code-improvments' of https://github.com/hpi-sch…
CeEv Dec 9, 2024
35e0666
Add RedisUserFactory
CeEv Dec 9, 2024
dc0575b
replace primitives
CeEv Dec 9, 2024
f7c01d2
Add todo
CeEv Dec 9, 2024
bf396e0
Refactor logger methods and update YRedisClient to use Logger for war…
SevenWaysDP Dec 10, 2024
37bc8b6
Switch from array type to single and use [] instead
CeEv Dec 10, 2024
e85a9aa
Merge branch 'BC-8403-code-improvments' of https://github.com/hpi-sch…
CeEv Dec 10, 2024
039f805
Fix import path for WorkerService in tldraw-worker.app.ts
SevenWaysDP Dec 10, 2024
9cd9a40
Code style changes
CeEv Dec 10, 2024
0fdefde
Merge branch 'BC-8403-code-improvments' of https://github.com/hpi-sch…
CeEv Dec 10, 2024
0ceffca
Move auth error logging to gateway.
CeEv Dec 10, 2024
f3dc7c2
wrap subscriber behind y-redis-service interface
CeEv Dec 10, 2024
24e414d
Change data flow to avoid double execution and import from helpers.
CeEv Dec 10, 2024
f57083d
Code style and add try catch for proceed data.
CeEv Dec 10, 2024
ba15d2a
Fix for tests
CeEv Dec 10, 2024
f54b098
Refactor y-redis subscriber service and update environment variables
SevenWaysDP Dec 11, 2024
a2f6205
add tests for y-redis.service
SevenWaysDP Dec 11, 2024
52f555a
Implement OnModuleInit in YRedisClient to create Redis group on initi…
SevenWaysDP Dec 11, 2024
4c3103f
temp
SevenWaysDP Dec 11, 2024
c53f425
revert .env.test
SevenWaysDP Dec 11, 2024
a576ea5
fixup! revert .env.test
SevenWaysDP Dec 11, 2024
dc5d908
Refactor YRedisModule to support server and worker configurations
SevenWaysDP Dec 11, 2024
2bb2a07
Add worker test cases
bischofmax Dec 11, 2024
a92b775
Merge branch 'BC-8403-code-improvments' of github.com:hpi-schul-cloud…
bischofmax Dec 11, 2024
7979661
Refactor YRedisDoc interface and implementation; introduce YRedisDocF…
SevenWaysDP Dec 12, 2024
a88ddb6
Enhance SubscriberService to wait if no streams are available; refact…
SevenWaysDP Dec 12, 2024
032ce7b
TESTS: Update websocket configuration to use port 3399; refactor test…
SevenWaysDP Dec 12, 2024
d451606
code review
SevenWaysDP Dec 12, 2024
d20fa3f
Merge remote-tracking branch 'origin/main' into BC-8403-code-improvments
SevenWaysDP Dec 12, 2024
6df6b03
Update TLDRAW_WEBSOCKET_URL to use port 3399 in Tldraw-Config API tests
SevenWaysDP Dec 12, 2024
91ab698
Minor naming fixes
bischofmax Dec 12, 2024
8e96c8a
Merge branch 'BC-8403-code-improvments' of github.com:hpi-schul-cloud…
bischofmax Dec 12, 2024
5664f91
Code review
SevenWaysDP Dec 12, 2024
0b2c66f
Rename deleteStorageReferencesIfExist
bischofmax Dec 12, 2024
715b14e
Rename helper methods in worker
bischofmax Dec 12, 2024
3e98e71
Merge branch 'BC-8403-code-improvments' of github.com:hpi-schul-cloud…
bischofmax Dec 12, 2024
a821b40
Improve authorization handling in WebsocketGateway by sending error r…
SevenWaysDP Dec 13, 2024
0743c46
revert renaming
SevenWaysDP Dec 13, 2024
54c469c
delete unused file
SevenWaysDP Dec 13, 2024
cbf5f6c
code review
SevenWaysDP Dec 13, 2024
0f4eee0
Refactor Redis service to factory pattern and update imports accordingly
SevenWaysDP Dec 13, 2024
6d5bb3c
code review
SevenWaysDP Dec 13, 2024
2bbfa1c
code review
SevenWaysDP Dec 13, 2024
e086f6b
Refactor StreamMessageReply interface to use Buffer for message prope…
SevenWaysDP Dec 13, 2024
e322f84
Refactor YRedisClient and WorkerService to remove destroyedCallback a…
SevenWaysDP Dec 13, 2024
be25869
Refactor logger usage to replace log method with info for improved cl…
SevenWaysDP Dec 13, 2024
f20d5cd
code review
SevenWaysDP Dec 13, 2024
a737787
Fix import path for YRedisDocProps to use correct filename
SevenWaysDP Dec 13, 2024
0d6ba28
add tests
SevenWaysDP Dec 13, 2024
7456a2c
Rename y-redis testing files to redis for consistency and update impo…
SevenWaysDP Dec 13, 2024
83186b2
Refactor RedisFactory tests to use consistent naming and improve clarity
SevenWaysDP Dec 13, 2024
68a2670
Refactor Redis testing factories for consistency and clarity
SevenWaysDP Dec 16, 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: 2 additions & 1 deletion .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ S3_SECRET_KEY=miniouser
S3_SSL=false

FEATURE_TLDRAW_ENABLED=true
TLDRAW_WEBSOCKET_URL=ws://localhost:3345
TLDRAW_WEBSOCKET_URL=ws://localhost:3399
TLDRAW_WEBSOCKET_PORT=3399

X_API_ALLOWED_KEYS=randomString
10 changes: 10 additions & 0 deletions src/apps/tldraw-worker.app.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import { NestFactory } from '@nestjs/core';
import { WorkerModule } from '../modules/worker/worker.module.js';
import { WorkerService } from '../modules/worker/worker.service.js';

async function bootstrap(): Promise<void> {
const nestApp = await NestFactory.createApplicationContext(WorkerModule);

await nestApp.init();
const workerService = await nestApp.resolve(WorkerService);

try {
workerService.start();
} catch (error) {
console.error(error);
workerService.stop();
process.exit(1);
}
}
bootstrap();
1 change: 0 additions & 1 deletion src/infra/authorization/authorization.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ export class AuthorizationService {

private createErrorResponsePayload(code: number, reason: string): ResponsePayload {
const response = ResponsePayloadBuilder.buildWithError(code, reason);
this.logger.log(`Error: ${code} - ${reason}`);

return response;
}
Expand Down
2 changes: 1 addition & 1 deletion src/infra/logger/logger.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('Logger', () => {
describe('WHEN warn logging', () => {
it('should call winstonLogger.warning', () => {
const error = new Error('custom error');
service.warn(error.message, error.stack);
service.warning(error.message, error.stack);
SevenWaysDP marked this conversation as resolved.
Show resolved Hide resolved
expect(winstonLogger.warning).toHaveBeenCalled();
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/infra/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class Logger {
this.logger.info(this.createMessage(message, context));
}

public warn(message: unknown, context?: string): void {
public warning(message: unknown, context?: string): void {
SevenWaysDP marked this conversation as resolved.
Show resolved Hide resolved
this.logger.warning(this.createMessage(message, context));
}

Expand Down
4 changes: 2 additions & 2 deletions src/infra/metrics/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export * from './metrics.module.js';
export * from './metrics.service.js';
export { MetricsModule } from './metrics.module.js';
export { MetricsService } from './metrics.service.js';
4 changes: 2 additions & 2 deletions src/infra/redis/interfaces/redis-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ export interface RedisAdapter {
exists(stream: string): Promise<number>;
createGroup(): Promise<void>;
quit(): Promise<void>;
readStreams(streams: StreamNameClockPair[]): Promise<StreamMessagesReply>;
readMessagesFromStream(streamName: string): Promise<StreamMessagesReply>;
readStreams(streams: StreamNameClockPair[]): Promise<StreamMessagesReply[]>;
readMessagesFromStream(streamName: string): Promise<StreamMessagesReply[]>;
reclaimTasks(consumerName: string, redisTaskDebounce: number, tryClaimCount: number): Promise<XAutoClaimResponse>;
getDeletedDocEntries(): Promise<StreamMessageReply[]>;
deleteDeleteDocEntry(id: string): Promise<number>;
Expand Down
7 changes: 1 addition & 6 deletions src/infra/redis/interfaces/stream-message-reply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@ export interface StreamMessageReply {
message: Record<keyof Message, RedisKey>;
}

export interface StreamMessagesSingleReply {
export interface StreamMessagesReply {
name: string;
messages: StreamMessageReply[] | null;
}

export type StreamMessagesReply = {
name: string;
messages: StreamMessageReply[] | null;
}[];
4 changes: 2 additions & 2 deletions src/infra/redis/ioredis.adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class IoRedisAdapter implements RedisAdapter {
await this.redis.quit();
}

public async readStreams(streams: StreamNameClockPair[]): Promise<StreamMessagesReply> {
public async readStreams(streams: StreamNameClockPair[]): Promise<StreamMessagesReply[]> {
const reads = await this.redis.xreadBuffer(
'COUNT',
1000,
Expand All @@ -104,7 +104,7 @@ export class IoRedisAdapter implements RedisAdapter {
return streamReplyRes;
}

public async readMessagesFromStream(streamName: string): Promise<StreamMessagesReply> {
public async readMessagesFromStream(streamName: string): Promise<StreamMessagesReply[]> {
const reads = await this.redis.xreadBuffer('STREAMS', streamName, '0');

const streamReplyRes = mapToStreamMessagesReply(reads);
Expand Down
2 changes: 1 addition & 1 deletion src/infra/redis/mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function mapToStreamMessagesReplies(messages: XItems | unknown): StreamMe
return result;
}

export function mapToStreamMessagesReply(streamReply: XReadBufferReply | unknown): StreamMessagesReply {
export function mapToStreamMessagesReply(streamReply: XReadBufferReply | unknown): StreamMessagesReply[] {
if (streamReply === null) {
return [];
}
Expand Down
34 changes: 26 additions & 8 deletions src/infra/redis/redis.module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
import { Module } from '@nestjs/common';
import { DynamicModule, Module } from '@nestjs/common';
import { ConfigurationModule } from '../configuration/configuration.module.js';
import { Logger } from '../logger/logger.js';
import { LoggerModule } from '../logger/logger.module.js';
import { RedisAdapter } from './interfaces/redis-adapter.js';
import { RedisConfig } from './redis.config.js';
import { RedisService } from './redis.service.js';
import { RedisFactory } from './redis.service.js';

@Module({
imports: [LoggerModule, ConfigurationModule.register(RedisConfig)],
providers: [RedisService],
exports: [RedisService],
})
export class RedisModule {}
@Module({})
export class RedisModule {
public static registerFor(token: string): DynamicModule {
return {
module: RedisModule,
imports: [LoggerModule, ConfigurationModule.register(RedisConfig)],
providers: [
{
provide: token,
useFactory: async (config: RedisConfig, logger: Logger): Promise<RedisAdapter> => {
const redisFactory = new RedisFactory(config, logger);
const redisAdapter = await redisFactory.createRedisInstance();

return redisAdapter;
},
inject: [RedisConfig, Logger],
},
],
exports: [token],
};
}
}
6 changes: 3 additions & 3 deletions src/infra/redis/redis.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as util from 'util';
import { Logger } from '../logger/index.js';
import { IoRedisAdapter } from './ioredis.adapter.js';
import { RedisConfig } from './redis.config.js';
import { RedisService } from './redis.service.js';
import { RedisFactory } from './redis.service.js';

jest.mock('ioredis', () => {
return {
Expand Down Expand Up @@ -50,7 +50,7 @@ describe('Redis Service', () => {
const constructorSpy = jest.spyOn(Redis.prototype, 'constructor');

const logger = createMock<Logger>();
const service = new RedisService(config, logger);
const service = new RedisFactory(config, logger);

const expectedProps = {
sentinels: [
Expand Down Expand Up @@ -105,7 +105,7 @@ describe('Redis Service', () => {
const constructorSpy = jest.spyOn(Redis.prototype, 'constructor');

const logger = createMock<Logger>();
const service = new RedisService(config, logger);
const service = new RedisFactory(config, logger);

const expectedProps = redisUrl;

Expand Down
6 changes: 2 additions & 4 deletions src/infra/redis/redis.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Injectable } from '@nestjs/common';
import * as dns from 'dns';
import { Redis } from 'ioredis';
import * as util from 'util';
Expand All @@ -7,13 +6,12 @@ import { RedisAdapter } from './interfaces/index.js';
import { IoRedisAdapter } from './ioredis.adapter.js';
import { RedisConfig } from './redis.config.js';

@Injectable()
export class RedisService {
export class RedisFactory {
public constructor(
private readonly config: RedisConfig,
private readonly logger: Logger,
) {
this.logger.setContext(RedisService.name);
this.logger.setContext(RedisFactory.name);
}
SevenWaysDP marked this conversation as resolved.
Show resolved Hide resolved

public async createRedisInstance(): Promise<RedisAdapter> {
Expand Down
2 changes: 1 addition & 1 deletion src/infra/storage/storage.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Injectable, OnModuleInit } from '@nestjs/common';
import { Client } from 'minio';
import { randomUUID } from 'crypto';
import { Client } from 'minio';
import { Stream } from 'stream';
import * as Y from 'yjs';
import { Logger } from '../logger/index.js';
Expand Down
Loading
Loading