fix: filters and limits in GET/jobs to get consistent results (#1866)

* fix filters and limits in GET to get consistent results

* update swaggerUI descriptions

Author: despadam

src/common/utils.ts

@@ -969,7 +969,7 @@ export const datasetsFullQueryDescriptionFields =
   </pre>';
 
 export const jobsFullQueryExampleFields =
-  '{"ownerGroup":["group1"], "statusCode": "jobCreated"}';
+  '{"ownerGroup": "group1", "statusCode": "jobCreated"}';
 
 export const jobsFullQueryDescriptionFields =
   '<pre>\n  \

src/jobs/jobs.controller.ts

@@ -37,6 +37,7 @@ import {
   ApiConsumes,
   ApiOperation,
   ApiQuery,
+  ApiParam,
   ApiResponse,
   ApiTags,
 } from "@nestjs/swagger";
@@ -48,6 +49,7 @@ import { JWTUser } from "src/auth/interfaces/jwt-user.interface";
 import { AccessGroupsType } from "src/config/configuration";
 import { Logger } from "@nestjs/common";
 import { UsersService } from "src/users/users.service";
+import { FullFacetResponse } from "src/common/types";
 import {
   filterDescriptionSimplified,
   filterExampleSimplified,
@@ -710,7 +712,7 @@ export class JobsController {
     description: "It creates a new job.",
   })
   @ApiBody({
-    description: "Input fields for the job to be created",
+    description: "Input fields for the job to be created.",
     required: true,
     type: CreateJobDtoV3,
   })
@@ -831,6 +833,11 @@ export class JobsController {
     summary: "It updates an existing job.",
     description: "It updates an existing job.",
   })
+  @ApiParam({
+    name: "id",
+    description: "Id of the job to be modified.",
+    type: String,
+  })
   @ApiBody({
     description: "Fields for the job to be updated",
     required: true,
@@ -866,6 +873,11 @@ export class JobsController {
       "It updates an existing job. Set `content-type` to `application/merge-patch+json` if you would like to update nested objects. Warning! `application/merge-patch+json` doesn’t support updating a specific item in an array — the result will always replace the entire target if it’s not an object.",
   })
   @ApiConsumes("application/json", "application/merge-patch+json")
+  @ApiParam({
+    name: "id",
+    description: "Id of the job to be modified.",
+    type: String,
+  })
   @ApiBody({
     description: "Fields for the job to be updated",
     required: true,
@@ -896,7 +908,9 @@ export class JobsController {
         fields: JSON.parse(filters.fields ?? ("{}" as string)),
         limits: JSON.parse(filters.limits ?? ("{}" as string)),
       };
-      const jobsFound = await this.jobsService.fullquery(parsedFilters);
+      const jobsFound = await this.jobsService.findByFilters(
+        parsedFilters.fields,
+      );
       const jobsAccessible: JobClass[] = [];
 
       // for each job run a casl JobReadOwner on a jobInstance
@@ -921,7 +935,10 @@ export class JobsController {
           }
         }
       }
-      return jobsAccessible;
+      return this.jobsService.applyFilterLimits(
+        jobsAccessible,
+        parsedFilters.limits,
+      );
     } catch (e) {
       throw new HttpException(
         {
@@ -1029,33 +1046,11 @@ export class JobsController {
   ): Promise<Record<string, unknown>[]> {
     try {
       const fields: IJobFields = JSON.parse(filters.fields ?? ("{}" as string));
-      const queryFilters: IFilters<JobDocument, FilterQuery<JobDocument>> = {
-        fields: fields,
-        limits: JSON.parse("{}" as string),
-      };
-      const jobsFound = await this.jobsService.fullquery(queryFilters);
+      const jobsFound = await this.fullQueryJobs(request, filters);
       const jobIdsAccessible: string[] = [];
-
-      // for each job run a casl JobReadOwner on a jobInstance
       if (jobsFound != null) {
         for (const i in jobsFound) {
-          const jobConfiguration = this.getJobTypeConfiguration(
-            jobsFound[i].type,
-          );
-          const ability = this.caslAbilityFactory.jobsInstanceAccess(
-            request.user as JWTUser,
-            jobConfiguration,
-          );
-          // check if the user can get this job
-          const jobInstance = await this.generateJobInstanceForPermissions(
-            jobsFound[i],
-          );
-          const canRead =
-            ability.can(Action.JobReadAny, JobClass) ||
-            ability.can(Action.JobReadAccess, jobInstance);
-          if (canRead) {
-            jobIdsAccessible.push(jobsFound[i]._id);
-          }
+          jobIdsAccessible.push(jobsFound[i]._id);
         }
       }
       fields._id = { $in: jobIdsAccessible };
@@ -1092,21 +1087,25 @@ export class JobsController {
   @ApiQuery({
     name: "fields",
     description:
-      "Define the filter conditions by specifying the name of values of fields requested.",
+      "Define the filter conditions by specifying the values of fields requested.\n" +
+      jobsFullQueryDescriptionFields,
     required: false,
     type: String,
+    example: jobsFullQueryExampleFields,
   })
   @ApiQuery({
     name: "facets",
     description:
       "Define a list of field names, for which facet counts should be calculated.",
     required: false,
     type: String,
+    example: '["type","ownerGroup","statusCode"]',
   })
   @ApiResponse({
     status: HttpStatus.OK,
-    type: [Object],
-    description: "Return jobs requested.",
+    type: FullFacetResponse,
+    isArray: true,
+    description: "Return fullfacet response for jobs requested.",
   })
   async fullFacetV3(
     @Req() request: Request,
@@ -1132,21 +1131,25 @@ export class JobsController {
   @ApiQuery({
     name: "fields",
     description:
-      "Define the filter conditions by specifying the name of values of fields requested.",
+      "Define the filter conditions by specifying the values of fields requested.\n" +
+      jobsFullQueryDescriptionFields,
     required: false,
     type: String,
+    example: jobsFullQueryExampleFields,
   })
   @ApiQuery({
     name: "facets",
     description:
       "Define a list of field names, for which facet counts should be calculated.",
     required: false,
     type: String,
+    example: '["type","ownerGroup","statusCode"]',
   })
   @ApiResponse({
     status: HttpStatus.OK,
-    type: [Object],
-    description: "Return jobs requested.",
+    type: FullFacetResponse,
+    isArray: true,
+    description: "Return fullfacet response for jobs requested.",
   })
   async fullFacetV4(
     @Req() request: Request,
@@ -1202,6 +1205,11 @@ export class JobsController {
     summary: "It returns the requested job.",
     description: "It returns the requested job.",
   })
+  @ApiParam({
+    name: "id",
+    description: "Id of the job to be retrieved.",
+    type: String,
+  })
   @ApiResponse({
     status: HttpStatus.OK,
     type: OutputJobV3Dto,
@@ -1228,6 +1236,11 @@ export class JobsController {
     summary: "It returns the requested job.",
     description: "It returns the requested job.",
   })
+  @ApiParam({
+    name: "id",
+    description: "Id of the job to be retrieved.",
+    type: String,
+  })
   @ApiResponse({
     status: HttpStatus.OK,
     type: JobClass,
@@ -1260,7 +1273,9 @@ export class JobsController {
         throw { message: "Invalid filter syntax." };
       }
       // for each job run a casl JobReadOwner on a jobInstance
-      const jobsFound = await this.jobsService.findAll(parsedFilter);
+      const jobsFound = await this.jobsService.findByFilters(
+        parsedFilter.where,
+      );
       const jobsAccessible: JobClass[] = [];
 
       for (const i in jobsFound) {
@@ -1282,7 +1297,10 @@ export class JobsController {
           jobsAccessible.push(jobsFound[i]);
         }
       }
-      return jobsAccessible;
+      return this.jobsService.applyFilterLimits(
+        jobsAccessible,
+        parsedFilter.limits,
+      );
     } catch (e) {
       throw new HttpException(
         {
@@ -1376,6 +1394,11 @@ export class JobsController {
     summary: "It deletes the requested job.",
     description: "It deletes the requested job.",
   })
+  @ApiParam({
+    name: "id",
+    description: "Id of the job to be deleted.",
+    type: String,
+  })
   @ApiResponse({
     status: HttpStatus.OK,
     type: undefined,
@@ -1412,6 +1435,11 @@ export class JobsController {
     summary: "It deletes the requested job.",
     description: "It deletes the requested job.",
   })
+  @ApiParam({
+    name: "id",
+    description: "Id of the job to be deleted.",
+    type: String,
+  })
   @ApiResponse({
     status: HttpStatus.OK,
     type: undefined,

src/jobs/jobs.service.ts

@@ -10,7 +10,11 @@ import {
   UpdateQuery,
 } from "mongoose";
 import { JWTUser } from "src/auth/interfaces/jwt-user.interface";
-import { IFacets, IFilters } from "src/common/interfaces/common.interface";
+import {
+  IFacets,
+  IFilters,
+  ILimitsFilter,
+} from "src/common/interfaces/common.interface";
 import {
   addCreatedByFields,
   addUpdatedByField,
@@ -50,6 +54,43 @@ export class JobsService {
     return createdJob.save();
   }
 
+  async findByFilters(
+    fields: FilterQuery<JobDocument> | undefined,
+  ): Promise<JobClass[]> {
+    const filters: FilterQuery<JobDocument> =
+      createFullqueryFilter<JobDocument>(this.jobModel, "id", fields ?? {});
+    return this.jobModel.find(filters).exec();
+  }
+
+  applyFilterLimits(
+    jobs: JobClass[],
+    limits: ILimitsFilter | undefined,
+  ): JobClass[] {
+    const modifiers: QueryOptions = parseLimitFilters(limits);
+    if (modifiers.sort) {
+      jobs = jobs.sort((a, b) => {
+        for (const [key, order] of Object.entries(modifiers.sort) as [
+          keyof JobClass,
+          1 | -1,
+        ][]) {
+          const aValue = a[key];
+          const bValue = b[key];
+          if (aValue === undefined || bValue === undefined) continue;
+          if (aValue < bValue) return order === 1 ? -1 : 1;
+          if (aValue > bValue) return order === 1 ? 1 : -1;
+        }
+        return 0;
+      });
+    }
+    if (modifiers.skip) {
+      jobs = jobs.slice(modifiers.skip);
+    }
+    if (modifiers.limit) {
+      jobs = jobs.slice(0, modifiers.limit);
+    }
+    return jobs;
+  }
+
   async findAll(
     filter: IFilters<JobDocument, FilterQuery<JobDocument>>,
   ): Promise<JobClass[]> {