Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 53 additions & 1 deletion src/origdatablocks/origdatablocks-public.v4.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import {
IOrigDatablockFields,
IOrigDatablockFiltersV4,
} from "./interfaces/origdatablocks.interface";
import { FullFacetFilters, FullFacetResponse } from "src/common/types";
import {
FullFacetFilters,
FullFacetResponse,
CountApiResponse,
} from "src/common/types";
import { getSwaggerOrigDatablockFilterContent } from "./types/origdatablock-filter-content";
import {
OrigDatablockLookupKeysEnum,
Expand Down Expand Up @@ -237,4 +241,52 @@ export class OrigDatablocksPublicV4Controller {

return origdatablock;
}

// GET /origdatablocks/public/files/count
@AllowAny()
@Get("/files/count")
@ApiOperation({
summary: "It returns the count of public files",
description:
"It returns the total number of public files across all origdatablocks matching the provided filter.",
})
@ApiQuery({
name: "filter",
description:
"Database filters to apply when retrieving count of public files",
required: false,
type: String,
content: getSwaggerOrigDatablockFilterContent({
where: true,
include: false,
fields: false,
limits: false,
}),
})
@ApiResponse({
status: HttpStatus.OK,
type: CountApiResponse,
description:
"Return the number of public files in the following format: { count: integer }",
})
async countFilesPublic(
@Query(
"filter",
new FilterValidationPipe(
ALLOWED_ORIGDATABLOCK_KEYS,
ALLOWED_ORIGDATABLOCK_FILTER_KEYS,
{
where: true,
include: false,
fields: false,
limits: false,
},
),
)
queryFilter?: string,
) {
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
const parsedFilter = JSON.parse(queryFilter ?? "{}");

return this.origDatablocksService.countFiles(parsedFilter);
}
}
42 changes: 42 additions & 0 deletions src/origdatablocks/origdatablocks.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,48 @@ export class OrigDatablocksService {
return { count };
}

async countFiles(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider simplifying the new countFiles aggregation pipeline by counting list sizes directly and cleaning up the fields projection handling.

You can simplify countFiles while keeping behavior intact.

1. Remove $unwind/$count in favor of $project + $group

You don’t need to explode dataFileList just to count its elements. Project its size and sum:

async countFiles(
  filter: FilterQuery<OrigDatablockDocument>,
): Promise<CountApiResponse> {
  const whereFilter: FilterQuery<OrigDatablockDocument> = filter.where ?? {};
  const fieldsProjection = filter.fields ?? [];

  const pipeline: PipelineStage[] = [{ $match: whereFilter }];
  this.addLookupFields(pipeline, filter.include);

  if (!isEmpty(fieldsProjection)) {
    const projection = parsePipelineProjection(fieldsProjection);
    pipeline.push({ $project: projection });
  }

  // keep dataset constraint but simplify the stages
  pipeline.push({
    $lookup: {
      from: "Dataset",
      as: "dataset_temp",
      let: { datasetId: "$datasetId" },
      pipeline: [{ $match: { $expr: { $eq: ["$pid", "$$datasetId"] } } }],
    },
  });

  // only keep origdatablocks for which a dataset exists
  pipeline.push({
    $match: {
      $expr: { $gt: [{ $size: "$dataset_temp" }, 0] },
    },
  });

  pipeline.push({ $unset: "dataset_temp" });

  // count files by summing sizes of dataFileList
  pipeline.push(
    {
      $project: {
        fileCount: { $size: "$dataFileList" },
      },
    },
    {
      $group: {
        _id: null,
        count: { $sum: "$fileCount" },
      },
    },
  );

  const [result] = await this.origDatablockModel
    .aggregate<{ count: number }>(pipeline)
    .exec();

  return { count: result?.count ?? 0 };
}

This keeps the dataset-existence constraint but removes $addFields, $unwind, and $count, making the intent “sum of list sizes” explicit.

2. Fix fieldsProjection typing/initialization

Right now fieldsProjection is typed as string[] but initialized with {} if filter.fields is an object (or falsy). Ensure its type/value align:

type FieldsInput = string[] | undefined; // adjust to whatever your filter type is

const fieldsProjection: string[] =
  Array.isArray(filter.fields) ? filter.fields : [];

Or, if filter.fields can already be guaranteed as string[], drop the {} default entirely:

const fieldsProjection: string[] = filter.fields ?? [];

This removes the implicit type mismatch and makes the projection logic easier to follow.

filter: FilterQuery<OrigDatablockDocument>,
): Promise<CountApiResponse> {
const whereFilter: FilterQuery<OrigDatablockDocument> = filter.where ?? {};
const fieldsProjection: string[] = filter.fields ?? [];

const pipeline: PipelineStage[] = [{ $match: whereFilter }];
this.addLookupFields(pipeline, filter.include);

if (!isEmpty(fieldsProjection)) {
const projection = parsePipelineProjection(fieldsProjection);
pipeline.push({ $project: projection });
}

pipeline.push({
$lookup: {
from: "Dataset",
as: "dataset_temp",
let: { datasetId: "$datasetId" },
pipeline: [{ $match: { $expr: { $eq: ["$pid", "$$datasetId"] } } }],
},
});

pipeline.push({
$addFields: {
datasetExist: { $gt: [{ $size: "$dataset_temp" }, 0] },
},
});

pipeline.push({ $unset: "dataset_temp" });

pipeline.push({ $unwind: "$dataFileList" });

pipeline.push({ $count: "count" });

const [result] = await this.origDatablockModel
.aggregate<{ count: number }>(pipeline)
.exec();

return { count: result?.count ?? 0 };
}

async aggregateSizeAndFileCount(
datasetId: string,
): Promise<{ size: number; numberOfFiles: number }> {
Expand Down
55 changes: 55 additions & 0 deletions src/origdatablocks/origdatablocks.v4.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
IsValidResponse,
FullFacetFilters,
FullFacetResponse,
CountApiResponse,
} from "src/common/types";
import { getSwaggerOrigDatablockFilterContent } from "./types/origdatablock-filter-content";
import {
Expand Down Expand Up @@ -695,6 +696,60 @@ export class OrigDatablocksV4Controller {
);
}

// GET /origdatablocks/files/count
@UseGuards(PoliciesGuard)
@CheckPolicies("origdatablocks", (ability: AppAbility) =>
ability.can(Action.OrigdatablockRead, OrigDatablock),
)
@Get("/files/count")
@ApiOperation({
summary: "It returns the count of files",
description:
"It returns the total number of files across all origdatablocks matching the provided filter.",
})
@ApiQuery({
name: "filter",
description: "Database filters to apply when retrieving count for files",
required: false,
type: String,
content: getSwaggerOrigDatablockFilterContent({
where: true,
include: false,
fields: false,
limits: false,
}),
})
@ApiResponse({
status: HttpStatus.OK,
type: CountApiResponse,
description:
"Return the number of files in the following format: { count: integer }",
})
async countFiles(
@Req() request: Request,
@Query(
"filter",
new FilterValidationPipe(
ALLOWED_ORIGDATABLOCK_KEYS,
ALLOWED_ORIGDATABLOCK_FILTER_KEYS,
{
where: true,
include: false,
fields: false,
limits: false,
},
),
)
queryFilter?: string,
) {
const parsedFilter = JSON.parse(queryFilter ?? "{}");
Comment thread
abdimo101 marked this conversation as resolved.
const mergedFilter = this.addAccessBasedFilters(
request.user as JWTUser,
parsedFilter,
);

return this.origDatablocksService.countFiles(mergedFilter);
}
// GET /origdatablocks/:id
@UseGuards(PoliciesGuard)
@CheckPolicies("origdatablocks", (ability: AppAbility) =>
Expand Down
50 changes: 46 additions & 4 deletions test/OrigDatablockV4.js
Original file line number Diff line number Diff line change
Expand Up @@ -909,31 +909,73 @@ describe("2800: OrigDatablock v4 endpoint tests", () => {
});
});

describe("OrigDatablocks v4 count tests", () => {
it("0600: should not be able to fetch the count of origdatablocks files if not logged in", async () => {
const filter = {
where: {
datasetId: {
$regex: datasetPid,
$options: "i",
},
},
};

return request(appUrl)
.get("/api/v4/origdatablocks/files/count")
.query({ filter: JSON.stringify(filter) })
.expect(TestData.AccessForbiddenStatusCode)
.expect("Content-Type", /json/);
});

it("0601: should be able to fetch the count of origdatablocks files", async () => {
const filter = {
where: {
datasetId: {
$regex: datasetPid,
$options: "i",
},
},
};

return request(appUrl)
.get("/api/v4/origdatablocks/files/count")
.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("count");
res.body.count.should.be.greaterThan(0);
});
});
});

describe("OrigDatablocks v4 delete tests", () => {
it("0600: should not be able to delete origdatablock if not logged in", () => {
it("0700: should not be able to delete origdatablock if not logged in", () => {
return request(appUrl)
.delete(`/api/v4/origdatablocks/${encodeURIComponent(origDatablockPid)}`)
.expect(TestData.AccessForbiddenStatusCode)
.expect("Content-Type", /json/);
});

it("0601: should not be able to delete origdatablock as owner", () => {
it("0701: should not be able to delete origdatablock as owner", () => {
return request(appUrl)
.delete(`/api/v4/origdatablocks/${encodeURIComponent(origDatablockPid)}`)
.auth(accessTokenUser1, { type: "bearer" })
.expect(TestData.AccessForbiddenStatusCode)
.expect("Content-Type", /json/);
});

it("0602: should not be able to delete origdatablock as adminIngestor", () => {
it("0702: should not be able to delete origdatablock as adminIngestor", () => {
return request(appUrl)
.delete(`/api/v4/origdatablocks/${encodeURIComponent(origDatablockPid)}`)
.auth(accessTokenAdminIngestor, { type: "bearer" })
.expect(TestData.AccessForbiddenStatusCode)
.expect("Content-Type", /json/);
});

it("0603: should be able to delete origdatablock as archivemanager", () => {
it("0703: should be able to delete origdatablock as archivemanager", () => {
return request(appUrl)
.delete(`/api/v4/origdatablocks/${encodeURIComponent(origDatablockPid)}`)
.auth(accessTokenArchiveManager, { type: "bearer" })
Expand Down
29 changes: 27 additions & 2 deletions test/OrigDatablockV4Public.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,33 @@ describe("2900: OrigDatablock v4 public endpoint tests", () => {
});
});

describe("OrigDatablocks v4 public count tests", () => {
it("0300: should be able to fetch the count of origdatablocks files providing where filter", async () => {
const filter = {
where: {
datasetId: {
$regex: datasetPid,
$options: "i",
},
},
};

return request(appUrl)
.get("/api/v4/origdatablocks/public/files/count")
.query({ filter: JSON.stringify(filter) })
.expect(TestData.SuccessfulGetStatusCode)
.expect("Content-Type", /json/)
.then((res) => {
res.body.should.be.a("object");
res.body.should.have.property("count");
res.body.count.should.be.a("number");
res.body.count.should.be.greaterThan(0);
});
});
});

describe("Cleanup after the tests", () => {
it("0300: delete all origdatablocks as archivemanager", async () => {
it("0400: delete all origdatablocks as archivemanager", async () => {
return await request(appUrl)
.get("/api/v4/origdatablocks")
.auth(accessTokenArchiveManager, { type: "bearer" })
Expand All @@ -385,7 +410,7 @@ describe("2900: OrigDatablock v4 public endpoint tests", () => {
});
});

it("0301: delete all datasets as archivemanager", async () => {
it("0401: delete all datasets as archivemanager", async () => {
return await request(appUrl)
.get("/api/v4/datasets")
.auth(accessTokenArchiveManager, { type: "bearer" })
Expand Down
Loading