From 65b21fe9048b670ee39b700901630a1cde16b516 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 10:16:37 +0100 Subject: [PATCH 1/9] perf(assignments-service): Add extra indexes on Assignee.solution and Evaluation.solution --- services/apps/assignments/src/assignee/assignee.schema.ts | 2 +- services/apps/assignments/src/evaluation/evaluation.schema.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/apps/assignments/src/assignee/assignee.schema.ts b/services/apps/assignments/src/assignee/assignee.schema.ts index 4e7be9380..bd1e50b79 100644 --- a/services/apps/assignments/src/assignee/assignee.schema.ts +++ b/services/apps/assignments/src/assignee/assignee.schema.ts @@ -35,7 +35,7 @@ export class Assignee { @IsMongoId() assignment: string; - @Prop() + @Prop({index: 1}) @ApiProperty() @IsMongoId() solution: string; diff --git a/services/apps/assignments/src/evaluation/evaluation.schema.ts b/services/apps/assignments/src/evaluation/evaluation.schema.ts index b5177a66b..76a7e6873 100644 --- a/services/apps/assignments/src/evaluation/evaluation.schema.ts +++ b/services/apps/assignments/src/evaluation/evaluation.schema.ts @@ -98,7 +98,7 @@ export class Evaluation { @IsMongoId() assignment: string; - @Prop() + @Prop({index: 1}) @ApiProperty() @IsMongoId() solution: string; From 2e4ec089eaead4491229959fbf3d2c6c1e8ac5b4 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 10:41:45 +0100 Subject: [PATCH 2/9] refactor(assignments-service): Make Assignee.assignment and Assignee.solution ObjectId --- .../src/assignee/assignee.controller.ts | 25 ++++++++++--------- .../src/assignee/assignee.handler.ts | 6 ++++- .../src/assignee/assignee.schema.ts | 17 ++++++------- .../assignments/src/course/course.service.ts | 13 +++++----- .../src/solution/solution.service.ts | 8 +++--- 5 files changed, 35 insertions(+), 34 deletions(-) diff --git a/services/apps/assignments/src/assignee/assignee.controller.ts b/services/apps/assignments/src/assignee/assignee.controller.ts index e2f328562..fbd2d8711 100644 --- a/services/apps/assignments/src/assignee/assignee.controller.ts +++ b/services/apps/assignments/src/assignee/assignee.controller.ts @@ -1,10 +1,11 @@ -import {NotFound} from '@mean-stream/nestx'; +import {NotFound, ObjectIdPipe} from '@mean-stream/nestx'; import {Body, Controller, Delete, Get, Param, Patch, Put} from '@nestjs/common'; import {ApiOkResponse, ApiOperation, ApiTags} from '@nestjs/swagger'; import {AssignmentAuth} from '../assignment/assignment-auth.decorator'; import {BulkUpdateAssigneeDto, PatchAssigneeDto, UpdateAssigneeDto} from './assignee.dto'; import {Assignee} from './assignee.schema'; import {AssigneeService} from './assignee.service'; +import {Types} from "mongoose"; const forbiddenResponse = 'Not owner of assignment, or invalid Assignment-Token'; @@ -20,7 +21,7 @@ export class AssigneeController { @AssignmentAuth({forbiddenResponse}) @ApiOkResponse({type: [Assignee]}) async findAll( - @Param('assignment') assignment: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, ): Promise { return this.assigneeService.findAll({assignment}, {sort: {assignee: 1}}); } @@ -30,7 +31,7 @@ export class AssigneeController { @AssignmentAuth({forbiddenResponse}) @ApiOkResponse({isArray: true}) async findUnique( - @Param('assignment') assignment: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, @Param('field') field: string, ): Promise { return this.assigneeService.model.distinct(field, {assignment}).exec(); @@ -40,7 +41,7 @@ export class AssigneeController { @AssignmentAuth({forbiddenResponse}) @ApiOkResponse({type: Assignee}) async updateMany( - @Param('assignment') assignment: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, @Body() dtos: BulkUpdateAssigneeDto[], ): Promise { return Promise.all(dtos.map(dto => this.assigneeService.upsert({assignment, solution: dto.solution}, dto))); @@ -51,8 +52,8 @@ export class AssigneeController { @AssignmentAuth({forbiddenResponse}) @ApiOkResponse({type: Assignee}) async findOne( - @Param('assignment') assignment: string, - @Param('solution') solution: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, + @Param('solution', ObjectIdPipe) solution: Types.ObjectId, ) { return this.assigneeService.findOne({assignment, solution}); } @@ -61,8 +62,8 @@ export class AssigneeController { @AssignmentAuth({forbiddenResponse}) @ApiOkResponse({type: Assignee}) async update( - @Param('assignment') assignment: string, - @Param('solution') solution: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, + @Param('solution', ObjectIdPipe) solution: Types.ObjectId, @Body() dto: UpdateAssigneeDto, ) { return this.assigneeService.upsert({assignment, solution}, dto); @@ -72,8 +73,8 @@ export class AssigneeController { @AssignmentAuth({forbiddenResponse}) @ApiOkResponse({type: Assignee}) async patch( - @Param('assignment') assignment: string, - @Param('solution') solution: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, + @Param('solution', ObjectIdPipe) solution: Types.ObjectId, @Body() dto: PatchAssigneeDto, ) { return this.assigneeService.updateOne({assignment, solution}, dto); @@ -84,8 +85,8 @@ export class AssigneeController { @AssignmentAuth({forbiddenResponse}) @ApiOkResponse({type: Assignee}) async remove( - @Param('assignment') assignment: string, - @Param('solution') solution: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, + @Param('solution', ObjectIdPipe) solution: Types.ObjectId, ) { return this.assigneeService.deleteOne({assignment, solution}); } diff --git a/services/apps/assignments/src/assignee/assignee.handler.ts b/services/apps/assignments/src/assignee/assignee.handler.ts index 5fcc8db23..e47eba4b2 100644 --- a/services/apps/assignments/src/assignee/assignee.handler.ts +++ b/services/apps/assignments/src/assignee/assignee.handler.ts @@ -2,6 +2,7 @@ import {Injectable} from '@nestjs/common'; import {OnEvent} from '@nestjs/event-emitter'; import {SolutionDocument} from '../solution/solution.schema'; import {AssigneeService} from './assignee.service'; +import {Types} from "mongoose"; @Injectable() export class AssigneeHandler { @@ -12,6 +13,9 @@ export class AssigneeHandler { @OnEvent('assignments.*.solutions.*.deleted') async onSolutionDeleted(solution: SolutionDocument) { - await this.assigneeService.deleteOne({assignment: solution.assignment, solution: solution.id}); + await this.assigneeService.deleteOne({ + assignment: new Types.ObjectId(solution.assignment), + solution: solution._id, + }); } } diff --git a/services/apps/assignments/src/assignee/assignee.schema.ts b/services/apps/assignments/src/assignee/assignee.schema.ts index bd1e50b79..241b03ae2 100644 --- a/services/apps/assignments/src/assignee/assignee.schema.ts +++ b/services/apps/assignments/src/assignee/assignee.schema.ts @@ -1,8 +1,9 @@ import {Prop, Schema, SchemaFactory} from '@nestjs/mongoose'; import {ApiProperty, ApiPropertyOptional} from '@nestjs/swagger'; -import {IsIn, IsMongoId, IsNotEmpty, IsOptional, IsPositive, IsString, ValidateNested} from 'class-validator'; -import {Document} from 'mongoose'; +import {IsIn, IsNotEmpty, IsOptional, IsPositive, IsString, ValidateNested} from 'class-validator'; +import {Document, Types} from 'mongoose'; import {Type} from "class-transformer"; +import {Ref} from "@mean-stream/nestx"; const OPTIONS = [1, 2, 3, 4]; @@ -30,15 +31,11 @@ export class Feedback { @Schema({id: false, _id: false}) export class Assignee { - @Prop({index: 1}) - @ApiProperty() - @IsMongoId() - assignment: string; + @Ref('Assignment', {index: 1}) + assignment: Types.ObjectId; - @Prop({index: 1}) - @ApiProperty() - @IsMongoId() - solution: string; + @Ref('Solution', {index: 1}) + solution: Types.ObjectId; @Prop() @ApiProperty() diff --git a/services/apps/assignments/src/course/course.service.ts b/services/apps/assignments/src/course/course.service.ts index 9409e1420..6bfb6dd83 100644 --- a/services/apps/assignments/src/course/course.service.ts +++ b/services/apps/assignments/src/course/course.service.ts @@ -35,12 +35,11 @@ export class CourseService extends MongooseRepository { const students = new Map(); const solutions = await this.solutionService.model.aggregate([ - {$match: {assignment: {$in: courseAssignmentsWhereUserIsMember}}}, - {$addFields: {id: {$toString: '$_id'}}}, + {$match: {assignment: {$in: courseAssignmentsWhereUserIsMember.map(o => o.toString())}}}, { $lookup: { from: 'assignees', - localField: 'id', + localField: '_id', foreignField: 'solution', as: '_assignees', }, @@ -101,12 +100,12 @@ export class CourseService extends MongooseRepository { return Array.from(new Set(students.values())); } - private async getCourseAssignmentsWhereUserIsMember(course: CourseDocument, user: string) { + private async getCourseAssignmentsWhereUserIsMember(course: CourseDocument, user: string): Promise { const userMembers = await this.memberService.findAll({ parent: {$in: course.assignments.map(a => new Types.ObjectId(a))}, user, }); - return userMembers.map(m => m.parent.toString()); + return userMembers.map(m => m.parent); } async getAssignees(id: Types.ObjectId, user: string): Promise { @@ -124,7 +123,7 @@ export class CourseService extends MongooseRepository { for await (const aggregateElement of this.assigneeService.model.aggregate<{ _id: { assignee: string, - assignment: string, + assignment: Types.ObjectId, }, solutions: number, duration: number, @@ -159,7 +158,7 @@ export class CourseService extends MongooseRepository { }; assigneeMap.set(assignee, courseAssignee); } - const index = course.assignments.indexOf(assignment); + const index = course.assignments.indexOf(assignment.toString()); courseAssignee.assignments[index] = rest; courseAssignee.solutions += rest.solutions; courseAssignee.duration += rest.duration; diff --git a/services/apps/assignments/src/solution/solution.service.ts b/services/apps/assignments/src/solution/solution.service.ts index 56fc5f098..42ff4ce54 100644 --- a/services/apps/assignments/src/solution/solution.service.ts +++ b/services/apps/assignments/src/solution/solution.service.ts @@ -48,17 +48,17 @@ export class SolutionService { { $match: preFilter, }, - { - $addFields: {id: {$toString: '$_id'}}, - }, { $lookup: { from: 'assignees', - localField: 'id', + localField: '_id', foreignField: 'solution', as: '_assignee', }, }, + { + $addFields: {id: {$toString: '$_id'}}, + }, { $lookup: { from: 'evaluations', From e2fa563845ab30a52e82d10c9c11ee9ee9386b45 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 10:47:41 +0100 Subject: [PATCH 3/9] refactor(assignments-service): Add solution sort constants --- .../apps/assignments/src/course/course.service.ts | 9 +++------ .../apps/assignments/src/solution/solution.schema.ts | 11 +++++++++++ .../apps/assignments/src/solution/solution.service.ts | 10 +--------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/services/apps/assignments/src/course/course.service.ts b/services/apps/assignments/src/course/course.service.ts index 6bfb6dd83..716d4e137 100644 --- a/services/apps/assignments/src/course/course.service.ts +++ b/services/apps/assignments/src/course/course.service.ts @@ -2,7 +2,7 @@ import {EventRepository, EventService, MongooseRepository} from '@mean-stream/ne import {Injectable} from '@nestjs/common'; import {InjectModel} from '@nestjs/mongoose'; import {Model, Types} from 'mongoose'; -import {AuthorInfo} from '../solution/solution.schema'; +import {AuthorInfo, SOLUTION_COLLATION, SOLUTION_SORT} from '../solution/solution.schema'; import {SolutionService} from '../solution/solution.service'; import {CourseAssignee, CourseStudent} from './course.dto'; import {Course, CourseDocument} from './course.schema'; @@ -55,12 +55,9 @@ export class CourseService extends MongooseRepository { feedback: 1, }, }, - {$sort: {'author.name': 1, 'author.github': 1}}, + {$sort: SOLUTION_SORT}, ], { - collation: { - locale: 'en', - caseFirst: 'off', - }, + collation: SOLUTION_COLLATION, }); const keys: (keyof AuthorInfo)[] = ['studentId', 'email', 'github', 'name']; diff --git a/services/apps/assignments/src/solution/solution.schema.ts b/services/apps/assignments/src/solution/solution.schema.ts index 2cc4bdac1..e2eca417d 100644 --- a/services/apps/assignments/src/solution/solution.schema.ts +++ b/services/apps/assignments/src/solution/solution.schema.ts @@ -141,3 +141,14 @@ export const SolutionSchema = SchemaFactory.createForClass(Solution) .index({assignment: 1, 'author.github': 1}) .index({assignment: 1, 'timestamp': 1}) ; + +export const SOLUTION_SORT = { + 'author.name': 1, + 'author.github': 1, + timestamp: 1, +} as const; + +export const SOLUTION_COLLATION = { + locale: 'en', + caseFirst: 'off', +} as const; diff --git a/services/apps/assignments/src/solution/solution.service.ts b/services/apps/assignments/src/solution/solution.service.ts index 42ff4ce54..3decfc2e5 100644 --- a/services/apps/assignments/src/solution/solution.service.ts +++ b/services/apps/assignments/src/solution/solution.service.ts @@ -5,15 +5,7 @@ import {InjectModel} from '@nestjs/mongoose'; import {FilterQuery, Model, UpdateQuery} from 'mongoose'; import {generateToken} from '../utils'; import {BatchUpdateSolutionDto, CreateSolutionDto, RichSolutionDto, UpdateSolutionDto} from './solution.dto'; -import {Solution, SolutionDocument} from './solution.schema'; - -const SOLUTION_SORT = { - 'author.name': 1, - 'author.github': 1, - timestamp: 1, -} as const; - -const SOLUTION_COLLATION = {locale: 'en', caseFirst: 'off'}; +import {Solution, SOLUTION_COLLATION, SOLUTION_SORT, SolutionDocument} from './solution.schema'; @Injectable() export class SolutionService { From ce454a1c7756a3b70cdc512d6182d71b90609bf3 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 11:06:27 +0100 Subject: [PATCH 4/9] fix(assignments-service): Correctly typed Evaluation.codeSearch.origin --- .../assignments/src/evaluation/evaluation.schema.ts | 11 +++++------ .../assignments/src/evaluation/evaluation.service.ts | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/services/apps/assignments/src/evaluation/evaluation.schema.ts b/services/apps/assignments/src/evaluation/evaluation.schema.ts index 76a7e6873..afcbf6dd2 100644 --- a/services/apps/assignments/src/evaluation/evaluation.schema.ts +++ b/services/apps/assignments/src/evaluation/evaluation.schema.ts @@ -14,7 +14,8 @@ import { Min, ValidateNested, } from 'class-validator'; -import {Document} from 'mongoose'; +import {Document, Types} from 'mongoose'; +import {OptionalRef} from "@mean-stream/nestx"; export class Location { @Prop() @@ -66,11 +67,9 @@ export class Snippet { } export class CodeSearchInfo { - @Prop() @ApiPropertyOptional({description: 'Only in GET responses'}) - @IsOptional() - @IsMongoId() - origin?: string; + @OptionalRef('Evaluation') + origin?: Types.ObjectId; @ApiPropertyOptional({description: 'Only in POST response'}) @IsOptional() @@ -91,7 +90,7 @@ export class CodeSearchInfo { @Schema({timestamps: true}) export class Evaluation { @ApiProperty() - _id: string; + _id: Types.ObjectId; @Prop() @ApiProperty() diff --git a/services/apps/assignments/src/evaluation/evaluation.service.ts b/services/apps/assignments/src/evaluation/evaluation.service.ts index c68c58129..0d402c32d 100644 --- a/services/apps/assignments/src/evaluation/evaluation.service.ts +++ b/services/apps/assignments/src/evaluation/evaluation.service.ts @@ -1,7 +1,7 @@ import {EventService} from '@mean-stream/nestx'; import {Injectable} from '@nestjs/common'; import {InjectModel} from '@nestjs/mongoose'; -import {FilterQuery, Model, UpdateQuery} from 'mongoose'; +import {FilterQuery, Model, Types, UpdateQuery} from 'mongoose'; import {AssignmentService} from '../assignment/assignment.service'; import {SearchService} from '../search/search.service'; import {CreateEvaluationDto, RemarkDto, UpdateEvaluationDto} from './evaluation.dto'; @@ -156,7 +156,7 @@ export class EvaluationService { ; } - private async codeSearchCreate(assignment: string, origin: any, dto: CreateEvaluationDto): Promise { + private async codeSearchCreate(assignment: string, origin: Types.ObjectId, dto: CreateEvaluationDto): Promise { const solutions = await this.codeSearch(assignment, dto.task, dto.snippets); const result = await this.model.bulkWrite(solutions.filter(s => s[1]).map(([solution, snippets]) => { const newEvaluation: UpdateQuery = { @@ -181,7 +181,7 @@ export class EvaluationService { return {created: result.upsertedCount}; } - private async codeSearchUpdate(assignment: string, task: string, origin: any, dto: UpdateEvaluationDto): Promise> { + private async codeSearchUpdate(assignment: string, task: string, origin: Types.ObjectId, dto: UpdateEvaluationDto): Promise> { const solutions = await this.codeSearch(assignment, task, dto.snippets!); let deleted = 0; let updated = 0; From 1aaa6c5b7b1c5be96ea6abdf2b92db9a68f4ad57 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 11:21:42 +0100 Subject: [PATCH 5/9] refactor(assignments-service): Make Evaluation.assignment and Evaluation.solution ObjectId --- .../src/assignment/assignment.service.ts | 4 ++-- .../src/evaluation/evaluation.controller.ts | 22 +++++++++---------- .../src/evaluation/evaluation.handler.ts | 6 ++++- .../src/evaluation/evaluation.schema.ts | 14 +++++------- .../src/evaluation/evaluation.service.ts | 16 +++++++------- .../src/solution/solution.service.ts | 6 +---- .../src/statistics/statistics.service.ts | 11 +++++----- 7 files changed, 38 insertions(+), 41 deletions(-) diff --git a/services/apps/assignments/src/assignment/assignment.service.ts b/services/apps/assignments/src/assignment/assignment.service.ts index e8f5ddb2d..a62544642 100644 --- a/services/apps/assignments/src/assignment/assignment.service.ts +++ b/services/apps/assignments/src/assignment/assignment.service.ts @@ -2,7 +2,7 @@ import {EventService} from '@mean-stream/nestx'; import {UserToken} from '@app/keycloak-auth'; import {Injectable} from '@nestjs/common'; import {InjectModel} from '@nestjs/mongoose'; -import {FilterQuery, Model, UpdateQuery} from 'mongoose'; +import {FilterQuery, Model, Types, UpdateQuery} from 'mongoose'; import {generateToken} from '../utils'; import {CreateAssignmentDto, ReadAssignmentDto, ReadTaskDto, UpdateAssignmentDto} from './assignment.dto'; import {Assignment, AssignmentDocument, Task} from './assignment.schema'; @@ -48,7 +48,7 @@ export class AssignmentService { }).exec(); } - async findOne(id: string): Promise { + async findOne(id: string | Types.ObjectId): Promise { return this.model.findById(id).exec(); } diff --git a/services/apps/assignments/src/evaluation/evaluation.controller.ts b/services/apps/assignments/src/evaluation/evaluation.controller.ts index 8a8e9e39d..9bb4ad3bf 100644 --- a/services/apps/assignments/src/evaluation/evaluation.controller.ts +++ b/services/apps/assignments/src/evaluation/evaluation.controller.ts @@ -1,5 +1,5 @@ import {AuthUser, UserToken} from '@app/keycloak-auth'; -import {NotFound} from '@mean-stream/nestx'; +import {NotFound, ObjectIdPipe} from '@mean-stream/nestx'; import {Body, Controller, Delete, Get, Headers, MessageEvent, Param, Patch, Post, Query, Sse} from '@nestjs/common'; import {ApiCreatedResponse, ApiOkResponse, ApiOperation, ApiTags} from '@nestjs/swagger'; import {FilterQuery, Types} from 'mongoose'; @@ -22,7 +22,7 @@ export class EvaluationController { ) { } - private toQuery(assignment: string, solution?: string, params: FilterEvaluationParams = {}): FilterQuery { + private toQuery(assignment: Types.ObjectId, solution?: Types.ObjectId, params: FilterEvaluationParams = {}): FilterQuery { const query: FilterQuery = {assignment}; solution && (query.solution = solution); params.file && (query['snippets.file'] = params.file); @@ -38,7 +38,7 @@ export class EvaluationController { @AssignmentAuth({forbiddenResponse: forbiddenAssignmentResponse}) @ApiOkResponse({type: [Evaluation]}) async findByAssignment( - @Param('assignment') assignment: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, @Query() params?: FilterEvaluationParams, ): Promise { return this.evaluationService.findAll(this.toQuery(assignment, undefined, params)); @@ -49,7 +49,7 @@ export class EvaluationController { @AssignmentAuth({forbiddenResponse: forbiddenAssignmentResponse}) @ApiOkResponse({isArray: true}) async findUnique( - @Param('assignment') assignment: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, @Param('field') field: string, @Query() params?: FilterEvaluationParams, ): Promise { @@ -61,7 +61,7 @@ export class EvaluationController { @AssignmentAuth({forbiddenResponse: forbiddenAssignmentResponse}) @ApiOkResponse({type: [RemarkDto]}) async findRemarks( - @Param('assignment') assignment: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, @Query() params?: FilterEvaluationParams, ): Promise { return this.evaluationService.findRemarks(this.toQuery(assignment, undefined, params)); @@ -71,8 +71,8 @@ export class EvaluationController { @AssignmentAuth({forbiddenResponse: forbiddenAssignmentResponse}) @ApiCreatedResponse({type: Evaluation}) async create( - @Param('assignment') assignment: string, - @Param('solution') solution: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, + @Param('solution', ObjectIdPipe) solution: Types.ObjectId, @Body() dto: CreateEvaluationDto, @AuthUser() user?: UserToken, ): Promise { @@ -83,8 +83,8 @@ export class EvaluationController { @SolutionAuth({forbiddenResponse}) @ApiOkResponse({type: [Evaluation]}) async findAll( - @Param('assignment') assignment: string, - @Param('solution') solution: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, + @Param('solution', ObjectIdPipe) solution: Types.ObjectId, @Query() params?: FilterEvaluationParams, ): Promise { return this.evaluationService.findAll(this.toQuery(assignment, solution, params)); @@ -94,8 +94,8 @@ export class EvaluationController { @SolutionAuth({forbiddenResponse}) @ApiOkResponse({type: Evaluation}) stream( - @Param('assignment') assignment: string, - @Param('solution') solution: string, + @Param('assignment', ObjectIdPipe) assignment: Types.ObjectId, + @Param('solution', ObjectIdPipe) solution: Types.ObjectId, @AuthUser() user?: UserToken, @Headers('assignment-token') assignmentToken?: string, @Headers('solution-token') solutionToken?: string, diff --git a/services/apps/assignments/src/evaluation/evaluation.handler.ts b/services/apps/assignments/src/evaluation/evaluation.handler.ts index e1b8ce414..9947524c0 100644 --- a/services/apps/assignments/src/evaluation/evaluation.handler.ts +++ b/services/apps/assignments/src/evaluation/evaluation.handler.ts @@ -2,6 +2,7 @@ import {Injectable} from '@nestjs/common'; import {OnEvent} from '@nestjs/event-emitter'; import {SolutionDocument} from '../solution/solution.schema'; import {EvaluationService} from './evaluation.service'; +import {Types} from "mongoose"; @Injectable() export class EvaluationHandler { @@ -12,6 +13,9 @@ export class EvaluationHandler { @OnEvent('assignments.*.solutions.*.deleted') async onSolutionDeleted(solution: SolutionDocument) { - await this.evaluationService.removeAll({assignment: solution.assignment, solution: solution.id}); + await this.evaluationService.removeAll({ + assignment: new Types.ObjectId(solution.assignment), + solution: solution._id, + }); } } diff --git a/services/apps/assignments/src/evaluation/evaluation.schema.ts b/services/apps/assignments/src/evaluation/evaluation.schema.ts index afcbf6dd2..6e249a0f5 100644 --- a/services/apps/assignments/src/evaluation/evaluation.schema.ts +++ b/services/apps/assignments/src/evaluation/evaluation.schema.ts @@ -15,7 +15,7 @@ import { ValidateNested, } from 'class-validator'; import {Document, Types} from 'mongoose'; -import {OptionalRef} from "@mean-stream/nestx"; +import {OptionalRef, Ref} from "@mean-stream/nestx"; export class Location { @Prop() @@ -92,15 +92,11 @@ export class Evaluation { @ApiProperty() _id: Types.ObjectId; - @Prop() - @ApiProperty() - @IsMongoId() - assignment: string; + @Ref('Assignment') + assignment: Types.ObjectId; - @Prop({index: 1}) - @ApiProperty() - @IsMongoId() - solution: string; + @Ref('Solution', {index: 1}) + solution: Types.ObjectId; @Prop() @ApiProperty() diff --git a/services/apps/assignments/src/evaluation/evaluation.service.ts b/services/apps/assignments/src/evaluation/evaluation.service.ts index 0d402c32d..4e21237f2 100644 --- a/services/apps/assignments/src/evaluation/evaluation.service.ts +++ b/services/apps/assignments/src/evaluation/evaluation.service.ts @@ -21,12 +21,12 @@ export class EvaluationService { this.eventService.emit(`assignments.${evaluation.assignment}.solutions.${evaluation.solution}.evaluations.${evaluation.id}.${event}`, evaluation); } - subscribe(assignment: string, solution: string, evaluation: string, event: string, user?: string) { + subscribe(assignment: Types.ObjectId, solution: Types.ObjectId, evaluation: Types.ObjectId | '*', event: string, user?: string) { // TODO only emit to users that have access to the assignment or solution return this.eventService.subscribe(`assignments.${assignment}.solutions.${solution}.evaluations.${evaluation}.${event}`, user); } - async create(assignment: string, solution: string, dto: CreateEvaluationDto, createdBy?: string): Promise { + async create(assignment: Types.ObjectId, solution: Types.ObjectId, dto: CreateEvaluationDto, createdBy?: string): Promise { const {codeSearch, ...rest} = dto; const evaluation = await this.model.findOneAndUpdate({ assignment, @@ -121,11 +121,11 @@ export class EvaluationService { return evaluations; } - private async codeSearch(assignmentId: string, taskId: string, snippets: Snippet[]): Promise<[string, Snippet[] | undefined][]> { + private async codeSearch(assignmentId: Types.ObjectId, taskId: string, snippets: Snippet[]): Promise<[Types.ObjectId, Snippet[] | undefined][]> { const assignment = await this.assignmentService.findOne(assignmentId); const task = assignment && this.assignmentService.findTask(assignment.tasks, taskId); const resultsBySnippet = await Promise.all(snippets.map(async snippet => { - const results = await this.searchService.find(assignmentId, { + const results = await this.searchService.find(assignmentId.toString(), { q: snippet.pattern || snippet.code, glob: task?.glob, wildcard: '***', @@ -149,14 +149,14 @@ export class EvaluationService { .map(solution => { if (solutionMatches[solution] !== snippets.length) { // remove solutions where any original snippet was not found - return [solution, undefined]; + return [new Types.ObjectId(solution), undefined]; } - return [solution, solutionSnippets[solution]]; + return [new Types.ObjectId(solution), solutionSnippets[solution]]; }) ; } - private async codeSearchCreate(assignment: string, origin: Types.ObjectId, dto: CreateEvaluationDto): Promise { + private async codeSearchCreate(assignment: Types.ObjectId, origin: Types.ObjectId, dto: CreateEvaluationDto): Promise { const solutions = await this.codeSearch(assignment, dto.task, dto.snippets); const result = await this.model.bulkWrite(solutions.filter(s => s[1]).map(([solution, snippets]) => { const newEvaluation: UpdateQuery = { @@ -181,7 +181,7 @@ export class EvaluationService { return {created: result.upsertedCount}; } - private async codeSearchUpdate(assignment: string, task: string, origin: Types.ObjectId, dto: UpdateEvaluationDto): Promise> { + private async codeSearchUpdate(assignment: Types.ObjectId, task: string, origin: Types.ObjectId, dto: UpdateEvaluationDto): Promise> { const solutions = await this.codeSearch(assignment, task, dto.snippets!); let deleted = 0; let updated = 0; diff --git a/services/apps/assignments/src/solution/solution.service.ts b/services/apps/assignments/src/solution/solution.service.ts index 3decfc2e5..739e65d07 100644 --- a/services/apps/assignments/src/solution/solution.service.ts +++ b/services/apps/assignments/src/solution/solution.service.ts @@ -48,13 +48,10 @@ export class SolutionService { as: '_assignee', }, }, - { - $addFields: {id: {$toString: '$_id'}}, - }, { $lookup: { from: 'evaluations', - localField: 'id', + localField: '_id', foreignField: 'solution', as: '_evaluations', pipeline: [ @@ -111,7 +108,6 @@ export class SolutionService { }, { $project: { - id: 0, _evaluations: 0, _assignee: 0, }, diff --git a/services/apps/assignments/src/statistics/statistics.service.ts b/services/apps/assignments/src/statistics/statistics.service.ts index 2a3d79d31..03feece67 100644 --- a/services/apps/assignments/src/statistics/statistics.service.ts +++ b/services/apps/assignments/src/statistics/statistics.service.ts @@ -11,6 +11,7 @@ import { TaskStatistics, TimeStatistics } from './statistics.dto'; +import {Types} from "mongoose"; const outlierDuration = 60; @@ -59,10 +60,10 @@ export class StatisticsService { solutions, , ] = await Promise.all([ - this.timeStatistics(assignment, taskStats, tasks), + this.timeStatistics(assignmentDoc._id, taskStats, tasks), this.countComments(assignment), this.solutionStatistics(assignmentDoc), - this.fillEvaluationStatistics(assignment, taskStats, tasks, evaluations, weightedEvaluations), + this.fillEvaluationStatistics(assignmentDoc._id, taskStats, tasks, evaluations, weightedEvaluations), ]); // needs to happen after timeStatistics and fillEvaluationStatistics @@ -80,7 +81,7 @@ export class StatisticsService { }; } - private async fillEvaluationStatistics(assignment: string, taskStats: Map, tasks: Map, evaluationStatistics: EvaluationStatistics, weightedEvaluationStatistics: EvaluationStatistics) { + private async fillEvaluationStatistics(assignment: Types.ObjectId, taskStats: Map, tasks: Map, evaluationStatistics: EvaluationStatistics, weightedEvaluationStatistics: EvaluationStatistics) { for await (const { codeSearch, points, @@ -115,7 +116,7 @@ export class StatisticsService { } } - private async timeStatistics(assignment: string, taskStats: Map, tasks: Map): Promise { + private async timeStatistics(assignment: Types.ObjectId, taskStats: Map, tasks: Map): Promise { let eventCount = 0; let totalTime = 0; let weightedTime = 0; @@ -182,7 +183,7 @@ export class StatisticsService { } passed++; } - const evaluated = (await this.evaluationService.findUnique('solution', {assignment: assignment.id})).length; + const evaluated = (await this.evaluationService.findUnique('solution', {assignment: assignment._id})).length; return { total, evaluated, From d812418d2029161250cc69951d363bb0bffdb808 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 11:50:25 +0100 Subject: [PATCH 6/9] refactor(assignments-service): Use Nestx Doc --- services/apps/assignments/src/assignee/assignee.schema.ts | 6 +++--- .../apps/assignments/src/assignee/assignee.service.ts | 2 +- .../apps/assignments/src/assignment/assignment.schema.ts | 5 +++-- .../apps/assignments/src/assignment/assignment.service.ts | 2 +- .../apps/assignments/src/classroom/classroom.service.ts | 4 ++-- services/apps/assignments/src/comment/comment.schema.ts | 4 ++-- services/apps/assignments/src/course/course.schema.ts | 5 +++-- .../apps/assignments/src/evaluation/evaluation.schema.ts | 7 +++---- services/apps/assignments/src/solution/solution.schema.ts | 8 +++++--- 9 files changed, 23 insertions(+), 20 deletions(-) diff --git a/services/apps/assignments/src/assignee/assignee.schema.ts b/services/apps/assignments/src/assignee/assignee.schema.ts index 241b03ae2..3999b94f8 100644 --- a/services/apps/assignments/src/assignee/assignee.schema.ts +++ b/services/apps/assignments/src/assignee/assignee.schema.ts @@ -1,9 +1,9 @@ import {Prop, Schema, SchemaFactory} from '@nestjs/mongoose'; import {ApiProperty, ApiPropertyOptional} from '@nestjs/swagger'; import {IsIn, IsNotEmpty, IsOptional, IsPositive, IsString, ValidateNested} from 'class-validator'; -import {Document, Types} from 'mongoose'; +import {Types} from 'mongoose'; import {Type} from "class-transformer"; -import {Ref} from "@mean-stream/nestx"; +import {Doc, Ref} from "@mean-stream/nestx"; const OPTIONS = [1, 2, 3, 4]; @@ -57,7 +57,7 @@ export class Assignee { feedback?: Feedback; } -export type AssigneeDocument = Assignee & Document; +export type AssigneeDocument = Doc; export const AssigneeSchema = SchemaFactory.createForClass(Assignee) .index({assignment: 1, solution: 1}, {unique: true}); diff --git a/services/apps/assignments/src/assignee/assignee.service.ts b/services/apps/assignments/src/assignee/assignee.service.ts index f03f7f7e8..5910600d0 100644 --- a/services/apps/assignments/src/assignee/assignee.service.ts +++ b/services/apps/assignments/src/assignee/assignee.service.ts @@ -8,7 +8,7 @@ import {Assignee, AssigneeDocument} from './assignee.schema'; @Injectable() export class AssigneeService extends MongooseRepository { constructor( - @InjectModel(Assignee.name) public model: Model, + @InjectModel(Assignee.name) public model: Model, private eventService: EventService, ) { super(model); diff --git a/services/apps/assignments/src/assignment/assignment.schema.ts b/services/apps/assignments/src/assignment/assignment.schema.ts index 707dd003f..27fbfa927 100644 --- a/services/apps/assignments/src/assignment/assignment.schema.ts +++ b/services/apps/assignments/src/assignment/assignment.schema.ts @@ -16,8 +16,9 @@ import { IsUrl, ValidateNested, } from 'class-validator'; -import {Document, Types} from 'mongoose'; +import {Types} from 'mongoose'; import {MOSS_LANGUAGES} from "../search/search.constants"; +import {Doc} from "@mean-stream/nestx"; @Schema({id: false, _id: false}) export class Task { @@ -192,6 +193,6 @@ export class Assignment { tasks: Task[]; } -export type AssignmentDocument = Assignment & Document; +export type AssignmentDocument = Doc; export const AssignmentSchema = SchemaFactory.createForClass(Assignment); diff --git a/services/apps/assignments/src/assignment/assignment.service.ts b/services/apps/assignments/src/assignment/assignment.service.ts index a62544642..2ba6f4d65 100644 --- a/services/apps/assignments/src/assignment/assignment.service.ts +++ b/services/apps/assignments/src/assignment/assignment.service.ts @@ -68,7 +68,7 @@ export class AssignmentService { }; } - async update(id: string, dto: UpdateAssignmentDto | UpdateQuery): Promise { + async update(id: string | Types.ObjectId, dto: UpdateAssignmentDto | UpdateQuery): Promise { const {token, classroom, ...rest} = dto; const update: UpdateQuery = rest; if (token) { diff --git a/services/apps/assignments/src/classroom/classroom.service.ts b/services/apps/assignments/src/classroom/classroom.service.ts index 64bef76b4..b70782c2d 100644 --- a/services/apps/assignments/src/classroom/classroom.service.ts +++ b/services/apps/assignments/src/classroom/classroom.service.ts @@ -150,7 +150,7 @@ export class ClassroomService { const upsertedId = solutions.upsertedIds[i]; if (commit && upsertedId) { const zip = await this.getRepoZip(assignment, this.getGithubName(repo, assignment), commit); - return zip ? this.fileService.importZipEntries(zip, assignment._id, upsertedId, commit) : []; + return zip ? this.fileService.importZipEntries(zip, assignment._id.toString(), upsertedId, commit) : []; } else { return []; } @@ -239,7 +239,7 @@ export class ClassroomService { private createImportSolution(assignment: AssignmentDocument, repo: RepositoryInfo, commit: string | undefined): ImportSolution { return { - assignment: assignment._id, + assignment: assignment._id.toString(), author: { name: '', email: '', diff --git a/services/apps/assignments/src/comment/comment.schema.ts b/services/apps/assignments/src/comment/comment.schema.ts index 5b0e3751a..819797f73 100644 --- a/services/apps/assignments/src/comment/comment.schema.ts +++ b/services/apps/assignments/src/comment/comment.schema.ts @@ -1,7 +1,7 @@ import {Prop, Schema, SchemaFactory} from '@nestjs/mongoose'; import {ApiProperty} from '@nestjs/swagger'; import {IsBoolean, IsDateString, IsEmail, IsMongoId, IsNotEmpty, IsString} from 'class-validator'; -import {Document} from 'mongoose'; +import {Doc} from "@mean-stream/nestx"; @Schema() export class Comment { @@ -47,7 +47,7 @@ export class Comment { distinguished: boolean; } -export type CommentDocument = Comment & Document; +export type CommentDocument = Doc; export const CommentSchema = SchemaFactory.createForClass(Comment) .index({assignment: 1, solution: 1}) diff --git a/services/apps/assignments/src/course/course.schema.ts b/services/apps/assignments/src/course/course.schema.ts index 24a99c066..3380896ee 100644 --- a/services/apps/assignments/src/course/course.schema.ts +++ b/services/apps/assignments/src/course/course.schema.ts @@ -1,7 +1,8 @@ import {Prop, Schema, SchemaFactory} from '@nestjs/mongoose'; import {ApiProperty, ApiPropertyOptional} from '@nestjs/swagger'; import {IsArray, IsMongoId, IsNotEmpty, IsOptional, IsString, IsUUID} from 'class-validator'; -import {Document, Types} from 'mongoose'; +import {Types} from 'mongoose'; +import {Doc} from "@mean-stream/nestx"; @Schema() export class Course { @@ -32,6 +33,6 @@ export class Course { assignments: string[]; } -export type CourseDocument = Course & Document; +export type CourseDocument = Doc; export const CourseSchema = SchemaFactory.createForClass(Course); diff --git a/services/apps/assignments/src/evaluation/evaluation.schema.ts b/services/apps/assignments/src/evaluation/evaluation.schema.ts index 6e249a0f5..da659e773 100644 --- a/services/apps/assignments/src/evaluation/evaluation.schema.ts +++ b/services/apps/assignments/src/evaluation/evaluation.schema.ts @@ -4,7 +4,6 @@ import {Type} from 'class-transformer'; import { IsAlphanumeric, IsInt, - IsMongoId, IsNotEmpty, IsNumber, IsOptional, @@ -14,8 +13,8 @@ import { Min, ValidateNested, } from 'class-validator'; -import {Document, Types} from 'mongoose'; -import {OptionalRef, Ref} from "@mean-stream/nestx"; +import {Types} from 'mongoose'; +import {Doc, OptionalRef, Ref} from "@mean-stream/nestx"; export class Location { @Prop() @@ -152,7 +151,7 @@ export class Evaluation { codeSearch?: CodeSearchInfo; } -export type EvaluationDocument = Evaluation & Document; +export type EvaluationDocument = Doc; export const EvaluationSchema = SchemaFactory.createForClass(Evaluation) .index({assignment: 1, solution: 1}) diff --git a/services/apps/assignments/src/solution/solution.schema.ts b/services/apps/assignments/src/solution/solution.schema.ts index e2eca417d..182e93535 100644 --- a/services/apps/assignments/src/solution/solution.schema.ts +++ b/services/apps/assignments/src/solution/solution.schema.ts @@ -5,7 +5,8 @@ import { IsBoolean, IsDateString, IsEmail, - IsHash, IsIn, + IsHash, + IsIn, IsMongoId, IsNumber, IsOptional, @@ -13,7 +14,8 @@ import { IsUUID, ValidateNested, } from 'class-validator'; -import {Document, Types} from 'mongoose'; +import {Types} from 'mongoose'; +import {Doc} from "@mean-stream/nestx"; export class Consent { @Prop() @@ -134,7 +136,7 @@ export class Solution { points?: number; } -export type SolutionDocument = Solution & Document; +export type SolutionDocument = Doc; export const SolutionSchema = SchemaFactory.createForClass(Solution) .index({assignment: 1, 'author.name': 1}) From e123894c4937673487a21299ce01ce593dc26356 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 11:54:35 +0100 Subject: [PATCH 7/9] refactor(assignments-service): Course students Solution _id --- services/apps/assignments/src/course/course.dto.ts | 4 +--- services/apps/assignments/src/course/course.service.ts | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/services/apps/assignments/src/course/course.dto.ts b/services/apps/assignments/src/course/course.dto.ts index e3552728e..53f40364c 100644 --- a/services/apps/assignments/src/course/course.dto.ts +++ b/services/apps/assignments/src/course/course.dto.ts @@ -12,11 +12,9 @@ export class UpdateCourseDto extends PartialType(CreateCourseDto) { } export class SolutionSummary extends PickType(Solution, [ + '_id', 'points', ] as const) { - @ApiProperty() - _id: string; - @ApiPropertyOptional() assignee?: string; } diff --git a/services/apps/assignments/src/course/course.service.ts b/services/apps/assignments/src/course/course.service.ts index 716d4e137..00c640ed2 100644 --- a/services/apps/assignments/src/course/course.service.ts +++ b/services/apps/assignments/src/course/course.service.ts @@ -86,7 +86,7 @@ export class CourseService extends MongooseRepository { const index = course.assignments.indexOf(assignment); student.solutions[index] = { - _id: _id.toString(), + _id, points, assignee, }; From 5d0ec5fe182252c55819d4c88d947ee965129bd6 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 12:04:34 +0100 Subject: [PATCH 8/9] perf(assignments-service): Optimize solution sort index --- .../apps/assignments/src/solution/solution.schema.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/services/apps/assignments/src/solution/solution.schema.ts b/services/apps/assignments/src/solution/solution.schema.ts index 182e93535..2313b8344 100644 --- a/services/apps/assignments/src/solution/solution.schema.ts +++ b/services/apps/assignments/src/solution/solution.schema.ts @@ -138,12 +138,6 @@ export class Solution { export type SolutionDocument = Doc; -export const SolutionSchema = SchemaFactory.createForClass(Solution) - .index({assignment: 1, 'author.name': 1}) - .index({assignment: 1, 'author.github': 1}) - .index({assignment: 1, 'timestamp': 1}) -; - export const SOLUTION_SORT = { 'author.name': 1, 'author.github': 1, @@ -154,3 +148,7 @@ export const SOLUTION_COLLATION = { locale: 'en', caseFirst: 'off', } as const; + +export const SolutionSchema = SchemaFactory.createForClass(Solution) + .index({assignment: 1, ...SOLUTION_SORT}, {collation: SOLUTION_COLLATION}) +; From c51514b9520122923c02fc38d275f378d368e378 Mon Sep 17 00:00:00 2001 From: Adrian Kunz Date: Wed, 15 Nov 2023 12:15:17 +0100 Subject: [PATCH 9/9] perf(assignments-service): Better assignment sort index --- .../assignments/src/assignment/assignment.schema.ts | 13 ++++++++++++- .../src/assignment/assignment.service.ts | 8 ++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/services/apps/assignments/src/assignment/assignment.schema.ts b/services/apps/assignments/src/assignment/assignment.schema.ts index 27fbfa927..343b424c8 100644 --- a/services/apps/assignments/src/assignment/assignment.schema.ts +++ b/services/apps/assignments/src/assignment/assignment.schema.ts @@ -195,4 +195,15 @@ export class Assignment { export type AssignmentDocument = Doc; -export const AssignmentSchema = SchemaFactory.createForClass(Assignment); +export const ASSIGNMENT_SORT = { + title: 1, +} as const; + +export const ASSIGNMENT_COLLATION = { + locale: 'en', + numericOrdering: true, +} as const; + +export const AssignmentSchema = SchemaFactory.createForClass(Assignment) + .index(ASSIGNMENT_SORT, {collation: ASSIGNMENT_COLLATION}) +; diff --git a/services/apps/assignments/src/assignment/assignment.service.ts b/services/apps/assignments/src/assignment/assignment.service.ts index 2ba6f4d65..adda46584 100644 --- a/services/apps/assignments/src/assignment/assignment.service.ts +++ b/services/apps/assignments/src/assignment/assignment.service.ts @@ -5,7 +5,7 @@ import {InjectModel} from '@nestjs/mongoose'; import {FilterQuery, Model, Types, UpdateQuery} from 'mongoose'; import {generateToken} from '../utils'; import {CreateAssignmentDto, ReadAssignmentDto, ReadTaskDto, UpdateAssignmentDto} from './assignment.dto'; -import {Assignment, AssignmentDocument, Task} from './assignment.schema'; +import {Assignment, ASSIGNMENT_COLLATION, ASSIGNMENT_SORT, AssignmentDocument, Task} from './assignment.schema'; import {MemberService} from "@app/member"; @Injectable() @@ -42,9 +42,9 @@ export class AssignmentService { } async findAll(where: FilterQuery = {}): Promise { - return this.model.find(where).sort({title: 1}).collation({ - locale: 'en', - numericOrdering: true, + return this.model.find(where, undefined,{ + sort: ASSIGNMENT_SORT, + collation: ASSIGNMENT_COLLATION, }).exec(); }