fix(dataset): ensure that fields required for the lookups are included

src/datasets/datasets.service.ts

@@ -1,4 +1,10 @@
-import { Inject, Injectable, NotFoundException, Scope } from "@nestjs/common";
+import {
+  BadRequestException,
+  Inject,
+  Injectable,
+  NotFoundException,
+  Scope,
+} from "@nestjs/common";
 import { ConfigService } from "@nestjs/config";
 import { REQUEST } from "@nestjs/core";
 import { InjectModel } from "@nestjs/mongoose";
@@ -77,6 +83,7 @@ export class DatasetsService {
       this.extractRelationsAndScopes(datasetLookupFields);
 
     const scopes = relationsAndScopes.scopes;
+    const addedRelations: string[] = [];
     for (const field of relationsAndScopes.relations) {
       const fieldValue = structuredClone(DATASET_LOOKUP_FIELDS[field]);
       if (!fieldValue) continue;
@@ -97,7 +104,7 @@ export class DatasetsService {
         includePipeline.push({ $limit: scope.limits.limit });
       if (scope?.limits?.sort) {
         const sort = parsePipelineSort(scope.limits.sort);
-        pipeline.push({ $sort: sort });
+        includePipeline.push({ $sort: sort });
       }
 
       if (includePipeline.length > 0)
@@ -106,7 +113,9 @@ export class DatasetsService {
         ).concat(includePipeline);
 
       pipeline.push(fieldValue);
+      addedRelations.push(field);
     }
+    return addedRelations;
   }
 
   private extractRelationsAndScopes(
@@ -183,15 +192,21 @@ export class DatasetsService {
     filter: IDatasetFiltersV4<DatasetDocument, IDatasetFields>,
   ): Promise<PartialOutputDatasetDto[]> {
     const whereFilter: FilterQuery<DatasetDocument> = filter.where ?? {};
-    const fieldsProjection = (filter.fields ?? []) as string[];
+    let fieldsProjection = (filter.fields ?? []) as string[];
     const limits = filter.limits ?? {
       limit: 10,
       skip: 0,
       sort: { createdAt: "desc" },
     };
 
     const pipeline: PipelineStage[] = [{ $match: whereFilter }];
-    this.addLookupFields(pipeline, filter.include);
+    const addedRelations = this.addLookupFields(pipeline, filter.include);
+
+    if (Array.isArray(fieldsProjection) && fieldsProjection.length > 0) {
+      fieldsProjection = Array.from(
+        new Set([...fieldsProjection, ...addedRelations]),
+      );
+    }
 
     if (!isEmpty(fieldsProjection)) {
       const projection = parsePipelineProjection(fieldsProjection);
@@ -206,12 +221,18 @@ export class DatasetsService {
     pipeline.push({ $skip: limits.skip || 0 });
 
     pipeline.push({ $limit: limits.limit || 10 });
+    try {
+      const data = await this.datasetModel
+        .aggregate<PartialOutputDatasetDto>(pipeline)
+        .exec();
 
-    const data = await this.datasetModel
-      .aggregate<PartialOutputDatasetDto>(pipeline)
-      .exec();
-
-    return data;
+      return data;
+    } catch (error) {
+      if (error instanceof Error) {
+        throw new BadRequestException(error.message);
+      }
+      throw new BadRequestException("An unknown error occurred");
+    }
   }
 
   async fullquery(
@@ -312,13 +333,22 @@ export class DatasetsService {
     filter: FilterQuery<DatasetDocument>,
   ): Promise<OutputDatasetDto | null> {
     const whereFilter: FilterQuery<DatasetDocument> = filter.where ?? {};
-    const fieldsProjection: string[] = filter.fields ?? {};
+    let fieldsProjection: string[] = filter.fields ?? {};
+
     const limits: QueryOptions<DatasetDocument> = filter.limits ?? {
       skip: 0,
       sort: { createdAt: "desc" },
     };
 
     const pipeline: PipelineStage[] = [{ $match: whereFilter }];
+    const addedRelations = this.addLookupFields(pipeline, filter.include);
+
+    if (Array.isArray(fieldsProjection)) {
+      fieldsProjection = Array.from(
+        new Set([...fieldsProjection, ...addedRelations]),
+      );
+    }
+
     if (!isEmpty(fieldsProjection)) {
       const projection = parsePipelineProjection(fieldsProjection);
       pipeline.push({ $project: projection });
@@ -331,13 +361,18 @@ export class DatasetsService {
 
     pipeline.push({ $skip: limits.skip || 0 });
 
-    this.addLookupFields(pipeline, filter.include);
-
-    const [data] = await this.datasetModel
-      .aggregate<OutputDatasetDto | undefined>(pipeline)
-      .exec();
+    try {
+      const [data] = await this.datasetModel
+        .aggregate<OutputDatasetDto | undefined>(pipeline)
+        .exec();
 
-    return data || null;
+      return data || null;
+    } catch (error) {
+      if (error instanceof Error) {
+        throw new BadRequestException(error.message);
+      }
+      throw new BadRequestException("An unknown error occurred");
+    }
   }
 
   async count(

src/datasets/datasets.v4.controller.ts

@@ -422,7 +422,8 @@ export class DatasetsV4Controller {
   })
   @ApiQuery({
     name: "filter",
-    description: "Database filters to apply when retrieving datasets",
+    description:
+      "Database filters to apply when retrieving datasets <br> ⚠️ Do not include both a parent field (e.g. 'proposals') and its subfields (e.g. 'proposals.type') in the same request, as this will cause a path collision error. ",
     required: false,
     type: String,
     content: getSwaggerDatasetFilterContent(),
@@ -593,7 +594,7 @@ export class DatasetsV4Controller {
   @ApiOperation({
     summary: "It returns the first dataset found.",
     description:
-      "It returns the first dataset of the ones that matches the filter provided. The list returned can be modified by providing a filter.",
+      "It returns the first dataset of the ones that matches the filter provided. The list returned can be modified by providing a filter.<br> ⚠️ Do not include both a parent field (e.g. 'proposals') and its subfields (e.g. 'proposals.type') in the same request, as this will cause a path collision error.",
   })
   @ApiQuery({
     name: "filter",

test/DatasetV4.js

@@ -612,7 +612,6 @@ describe("2500: Datasets v4 tests", () => {
         .then((res) => {
           res.body.should.be.a("array");
           const [firstDataset] = res.body;
-
           firstDataset.should.have.property("pid");
           firstDataset.should.have.property("instruments");
           firstDataset.should.have.property("proposals");
@@ -703,12 +702,12 @@ describe("2500: Datasets v4 tests", () => {
             dataset.should.have.property("datasetName");
             dataset.should.have.property("pid");
             dataset.should.not.have.property("description");
-            dataset.should.not.have.property("instruments");
-            dataset.should.not.have.property("proposals");
-            dataset.should.not.have.property("datablocks");
-            dataset.should.not.have.property("attachments");
-            dataset.should.not.have.property("origdatablocks");
-            dataset.should.not.have.property("samples");
+            dataset.should.have.property("instruments");
+            dataset.should.have.property("proposals");
+            dataset.should.have.property("datablocks");
+            dataset.should.have.property("attachments");
+            dataset.should.have.property("origdatablocks");
+            dataset.should.have.property("samples");
 
             dataset.datasetName.should.match(/Dataset/i);
           });
@@ -907,7 +906,33 @@ describe("2500: Datasets v4 tests", () => {
         });
     });
 
-    it("0303: should fetch dataset relation fields if provided in the filter", async () => {
+    it("0303: should fetch specific dataset fields only if fields is provided in the filter with relationships", async () => {
+      const filter = {
+        include: ["instruments", "proposals"],
+        fields: ["datasetName"],
+      };
+
+      return request(appUrl)
+        .get(`/api/v4/datasets/findOne`)
+        .query({ filter: JSON.stringify(filter) })
+        .auth(accessTokenAdminIngestor, { type: "bearer" })
+        .expect(TestData.SuccessfulGetStatusCode)
+        .expect("Content-Type", /json/)
+        .then((res) => {
+          res.body.should.be.a("object");
+
+          res.body.should.have.property("datasetName");
+
+          res.body.should.not.have.property("description");
+          res.body.should.not.have.property("pid");
+
+          res.body.should.have.property("instruments");
+          res.body.should.have.property("proposals");
+          res.body.should.not.have.property("datablocks");
+        });
+    });
+
+    it("0304: should fetch dataset relation fields if provided in the filter", async () => {
       const filter = {
         include: ["instruments", "proposals"],
       };
@@ -928,7 +953,7 @@ describe("2500: Datasets v4 tests", () => {
         });
     });
 
-    it("0304: should fetch all dataset relation fields if provided in the filter", async () => {
+    it("0305: should fetch all dataset relation fields if provided in the filter", async () => {
       const filter = {
         include: ["all"],
       };
@@ -952,7 +977,7 @@ describe("2500: Datasets v4 tests", () => {
         });
     });
 
-    it("0305: should be able to fetch the dataset providing where filter", async () => {
+    it("0306: should be able to fetch the dataset providing where filter", async () => {
       const filter = {
         where: {
           datasetName: {
@@ -974,7 +999,7 @@ describe("2500: Datasets v4 tests", () => {
         });
     });
 
-    it("0306: should be able to fetch a dataset providing all allowed filters together", async () => {
+    it("0307: should be able to fetch a dataset providing all allowed filters together", async () => {
       const filter = {
         where: {
           datasetName: {
@@ -1017,7 +1042,7 @@ describe("2500: Datasets v4 tests", () => {
         });
     });
 
-    it("0307: should not be able to provide filters that are not allowed", async () => {
+    it("0308: should not be able to provide filters that are not allowed", async () => {
       const filter = {
         customField: { datasetName: "test" },
       };
@@ -1029,6 +1054,30 @@ describe("2500: Datasets v4 tests", () => {
         .expect(TestData.BadRequestStatusCode)
         .expect("Content-Type", /json/);
     });
+
+    it("0309: should throw BadRequest when subfields within the embedded documents are used in the fields projection", async () => {
+      const filter = {
+        include: ["all"],
+        fields: [
+          "datasetName",
+          "attachments.thumbnail",
+          "attachments.relationships.targetType",
+          "origdatablocks.dataFileList.path",
+        ],
+      };
+
+      return request(appUrl)
+        .get(`/api/v4/datasets/findOne`)
+        .query({ filter: JSON.stringify(filter) })
+        .auth(accessTokenAdminIngestor, { type: "bearer" })
+        .expect(TestData.BadRequestStatusCode)
+        .expect("Content-Type", /json/)
+        .then((res) => {
+          res.body.should.be.a("object");
+          res.body.should.have.property("message");
+          res.body.message.should.match(/Invalid \$project :: caused by :: Path collision at origdatablocks/);
+        });
+    });
   });
 
   describe("Datasets v4 count tests", () => {

test/DatasetV4Public.js

@@ -269,12 +269,12 @@ describe("2600: Datasets v4 public endpoints tests", () => {
             dataset.should.have.property("datasetName");
             dataset.should.have.property("pid");
             dataset.should.not.have.property("description");
-            dataset.should.not.have.property("instruments");
-            dataset.should.not.have.property("proposals");
-            dataset.should.not.have.property("datablocks");
-            dataset.should.not.have.property("attachments");
-            dataset.should.not.have.property("origdatablocks");
-            dataset.should.not.have.property("samples");
+            dataset.should.have.property("instruments");
+            dataset.should.have.property("proposals");
+            dataset.should.have.property("datablocks");
+            dataset.should.have.property("attachments");
+            dataset.should.have.property("origdatablocks");
+            dataset.should.have.property("samples");
 
             dataset.datasetName.should.match(/Dataset/i);
           });