Merge pull request #385 from fujaba/fix/aggregation-performance

Fix Aggregation Performance

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<Assignee[]> {
     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<unknown[]> {
     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<Assignee[]> {
     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});
   }

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

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 {Types} from 'mongoose';
 import {Type} from "class-transformer";
+import {Doc, 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()
-  @ApiProperty()
-  @IsMongoId()
-  solution: string;
+  @Ref('Solution', {index: 1})
+  solution: Types.ObjectId;
 
   @Prop()
   @ApiProperty()
@@ -60,7 +57,7 @@ export class Assignee {
   feedback?: Feedback;
 }
 
-export type AssigneeDocument = Assignee & Document;
+export type AssigneeDocument = Doc<Assignee, never>;
 
 export const AssigneeSchema = SchemaFactory.createForClass(Assignee)
   .index({assignment: 1, solution: 1}, {unique: true});

services/apps/assignments/src/assignee/assignee.service.ts

@@ -8,7 +8,7 @@ import {Assignee, AssigneeDocument} from './assignee.schema';
 @Injectable()
 export class AssigneeService extends MongooseRepository<Assignee, never, AssigneeDocument> {
   constructor(
-    @InjectModel(Assignee.name) public model: Model<Assignee>,
+    @InjectModel(Assignee.name) public model: Model<Assignee, object, object, object, AssigneeDocument>,
     private eventService: EventService,
   ) {
     super(model);

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,17 @@ export class Assignment {
   tasks: Task[];
 }
 
-export type AssignmentDocument = Assignment & Document;
+export type AssignmentDocument = Doc<Assignment>;
 
-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})
+;

services/apps/assignments/src/assignment/assignment.service.ts

@@ -2,10 +2,10 @@ 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';
+import {Assignment, ASSIGNMENT_COLLATION, ASSIGNMENT_SORT, AssignmentDocument, Task} from './assignment.schema';
 import {MemberService} from "@app/member";
 
 @Injectable()
@@ -42,13 +42,13 @@ export class AssignmentService {
   }
 
   async findAll(where: FilterQuery<Assignment> = {}): Promise<AssignmentDocument[]> {
-    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();
   }
 
-  async findOne(id: string): Promise<AssignmentDocument | null> {
+  async findOne(id: string | Types.ObjectId): Promise<AssignmentDocument | null> {
     return this.model.findById(id).exec();
   }
 
@@ -68,7 +68,7 @@ export class AssignmentService {
     };
   }
 
-  async update(id: string, dto: UpdateAssignmentDto | UpdateQuery<Assignment>): Promise<Assignment | null> {
+  async update(id: string | Types.ObjectId, dto: UpdateAssignmentDto | UpdateQuery<Assignment>): Promise<Assignment | null> {
     const {token, classroom, ...rest} = dto;
     const update: UpdateQuery<Assignment> = rest;
     if (token) {

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: '',

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<Comment>;
 
 export const CommentSchema = SchemaFactory.createForClass(Comment)
   .index({assignment: 1, solution: 1})

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

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<Course>;
 
 export const CourseSchema = SchemaFactory.createForClass(Course);

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';
@@ -35,12 +35,11 @@ export class CourseService extends MongooseRepository<Course> {
 
     const students = new Map<string, CourseStudent>();
     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',
         },
@@ -56,12 +55,9 @@ export class CourseService extends MongooseRepository<Course> {
           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'];
@@ -90,7 +86,7 @@ export class CourseService extends MongooseRepository<Course> {
 
       const index = course.assignments.indexOf(assignment);
       student.solutions[index] = {
-        _id: _id.toString(),
+        _id,
         points,
         assignee,
       };
@@ -101,12 +97,12 @@ export class CourseService extends MongooseRepository<Course> {
     return Array.from(new Set(students.values()));
   }
 
-  private async getCourseAssignmentsWhereUserIsMember(course: CourseDocument, user: string) {
+  private async getCourseAssignmentsWhereUserIsMember(course: CourseDocument, user: string): Promise<Types.ObjectId[]> {
     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<CourseAssignee[]> {
@@ -124,7 +120,7 @@ export class CourseService extends MongooseRepository<Course> {
     for await (const aggregateElement of this.assigneeService.model.aggregate<{
       _id: {
         assignee: string,
-        assignment: string,
+        assignment: Types.ObjectId,
       },
       solutions: number,
       duration: number,
@@ -159,7 +155,7 @@ export class CourseService extends MongooseRepository<Course> {
         };
         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;