From 04f96c58b3eb492949380c40f0e5fee3aca0666f Mon Sep 17 00:00:00 2001 From: Sebastijan K <58827427+sebastijankuzner@users.noreply.github.com> Date: Mon, 11 Apr 2022 13:38:11 +0200 Subject: [PATCH] fix(core-api): sorting with multiple orderBy values (#4636) --- .../core-api/controllers/controller.test.ts | 7 ++-- .../core-api/controllers/delegates.test.ts | 4 +-- .../unit/core-api/controllers/locks.test.ts | 2 +- .../unit/core-api/controllers/wallets.test.ts | 32 ++++++++--------- .../core-api/src/controllers/controller.ts | 7 +--- packages/core-api/src/resources-new/block.ts | 2 +- .../core-api/src/resources-new/transaction.ts | 2 +- packages/core-api/src/schemas.ts | 36 +++++++++---------- 8 files changed, 45 insertions(+), 47 deletions(-) diff --git a/__tests__/unit/core-api/controllers/controller.test.ts b/__tests__/unit/core-api/controllers/controller.test.ts index e334850278..58a41b6779 100644 --- a/__tests__/unit/core-api/controllers/controller.test.ts +++ b/__tests__/unit/core-api/controllers/controller.test.ts @@ -1,6 +1,5 @@ import "jest-extended"; -import Joi from "joi"; import { Controller } from "@packages/core-api/src/controllers/controller"; import { Resource } from "@packages/core-api/src/interfaces"; import { Application, Container } from "@packages/core-kernel"; @@ -8,6 +7,7 @@ import { Identifiers } from "@packages/core-kernel/src/ioc"; import { Transactions as MagistrateTransactions } from "@packages/core-magistrate-crypto"; import { TransactionHandlerRegistry } from "@packages/core-transactions/src/handlers/handler-registry"; import { Transactions } from "@packages/crypto"; +import Joi from "joi"; import { initApp } from "../__support__"; @@ -168,7 +168,10 @@ describe("Controller", () => { it("should parse order", async () => { const request = { query: { - orderBy: "test:desc,test2:asc", + orderBy: [ + { direction: "desc", property: "test" }, + { direction: "asc", property: "test2" }, + ], }, }; diff --git a/__tests__/unit/core-api/controllers/delegates.test.ts b/__tests__/unit/core-api/controllers/delegates.test.ts index fca2e28c13..87fc229687 100644 --- a/__tests__/unit/core-api/controllers/delegates.test.ts +++ b/__tests__/unit/core-api/controllers/delegates.test.ts @@ -284,7 +284,7 @@ describe("DelegatesController", () => { query: { page: 1, limit: 100, - orderBy: "height:desc", + orderBy: [{ direction: "desc", property: "height" }], transform: false, }, }, @@ -337,7 +337,7 @@ describe("DelegatesController", () => { query: { page: 1, limit: 100, - orderBy: "height:desc", + orderBy: [{ direction: "desc", property: "height" }], transform: true, }, }, diff --git a/__tests__/unit/core-api/controllers/locks.test.ts b/__tests__/unit/core-api/controllers/locks.test.ts index e9805aaf3c..75c7fcedd7 100644 --- a/__tests__/unit/core-api/controllers/locks.test.ts +++ b/__tests__/unit/core-api/controllers/locks.test.ts @@ -143,7 +143,7 @@ describe("LocksController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], }, payload: { ids: [lock1Id, lock2Id], diff --git a/__tests__/unit/core-api/controllers/wallets.test.ts b/__tests__/unit/core-api/controllers/wallets.test.ts index a47c4d2ad2..a31e7fd5b0 100644 --- a/__tests__/unit/core-api/controllers/wallets.test.ts +++ b/__tests__/unit/core-api/controllers/wallets.test.ts @@ -252,7 +252,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, type: Enums.TransactionType.MultiPayment, }, @@ -272,7 +272,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, }, [{ direction: "desc", property: "amount" }], @@ -316,7 +316,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, type: Enums.TransactionType.MultiPayment, }, @@ -336,7 +336,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, }, [{ direction: "desc", property: "amount" }], @@ -398,7 +398,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, type: Enums.TransactionType.MultiPayment, }, @@ -418,7 +418,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, }, [{ direction: "desc", property: "amount" }], @@ -462,7 +462,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, type: Enums.TransactionType.MultiPayment, }, @@ -482,7 +482,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, }, [{ direction: "desc", property: "amount" }], @@ -565,7 +565,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, type: Enums.TransactionType.MultiPayment, }, @@ -585,7 +585,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, }, [{ direction: "desc", property: "amount" }], @@ -629,7 +629,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, type: Enums.TransactionType.MultiPayment, }, @@ -649,7 +649,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, }, [{ direction: "desc", property: "amount" }], @@ -711,7 +711,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, }, }, @@ -731,7 +731,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: false, }, [{ direction: "desc", property: "amount" }], @@ -775,7 +775,7 @@ describe("WalletsController", () => { query: { page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, }, }, @@ -795,7 +795,7 @@ describe("WalletsController", () => { // not filtered out from criteria page: 1, limit: 100, - orderBy: "amount:desc", + orderBy: [{ direction: "desc", property: "amount" }], transform: true, }, [{ direction: "desc", property: "amount" }], diff --git a/packages/core-api/src/controllers/controller.ts b/packages/core-api/src/controllers/controller.ts index ceddc9cf12..f0eadf7c8a 100644 --- a/packages/core-api/src/controllers/controller.ts +++ b/packages/core-api/src/controllers/controller.ts @@ -56,12 +56,7 @@ export class Controller { return []; } - const orderBy = Array.isArray(request.query.orderBy) ? request.query.orderBy : request.query.orderBy.split(","); - - return orderBy.map((s: string) => ({ - property: s.split(":")[0], - direction: s.split(":")[1] === "desc" ? "desc" : "asc", - })); + return request.query.orderBy; } protected getListingOptions(): Contracts.Search.Options { diff --git a/packages/core-api/src/resources-new/block.ts b/packages/core-api/src/resources-new/block.ts index c37195db13..90fff6433a 100644 --- a/packages/core-api/src/resources-new/block.ts +++ b/packages/core-api/src/resources-new/block.ts @@ -32,4 +32,4 @@ export const blockCriteriaSchemaObject = { }; export const blockParamSchema = Joi.alternatives(blockIdSchema, blockHeightSchema); -export const blockSortingSchema = Schemas.createSortingSchema(Schemas.blockCriteriaSchemas, [], false); +export const blockSortingSchema = Schemas.createSortingSchema(Schemas.blockCriteriaSchemas); diff --git a/packages/core-api/src/resources-new/transaction.ts b/packages/core-api/src/resources-new/transaction.ts index 1f4893cd78..b545ab118d 100644 --- a/packages/core-api/src/resources-new/transaction.ts +++ b/packages/core-api/src/resources-new/transaction.ts @@ -18,4 +18,4 @@ export const transactionCriteriaSchemaObject = { }; export const transactionParamSchema = transactionIdSchema; -export const transactionSortingSchema = Schemas.createSortingSchema(Schemas.transactionCriteriaSchemas, [], false); +export const transactionSortingSchema = Schemas.createSortingSchema(Schemas.transactionCriteriaSchemas); diff --git a/packages/core-api/src/schemas.ts b/packages/core-api/src/schemas.ts index d2b1cc84e9..40add91b29 100644 --- a/packages/core-api/src/schemas.ts +++ b/packages/core-api/src/schemas.ts @@ -44,11 +44,7 @@ export const createRangeCriteriaSchema = (item: Joi.Schema): Joi.Schema => { // Sorting -export const createSortingSchema = ( - schemaObject: SchemaObject, - wildcardPaths: string[] = [], - transform: boolean = true, -): Joi.ObjectSchema => { +export const createSortingSchema = (schemaObject: SchemaObject, wildcardPaths: string[] = []): Joi.ObjectSchema => { const getObjectPaths = (object: SchemaObject): string[] => { return Object.entries(object) .map(([key, value]) => { @@ -66,8 +62,17 @@ export const createSortingSchema = ( const sorting: Contracts.Search.Sorting = []; - for (const item of value.split(",")) { + let items: string[] = []; + + if (Array.isArray(value)) { + items = value; + } else if (value) { + items = [value]; + } + + for (const item of items) { const pair = item.split(":"); + const property = String(pair[0]); const direction = pair.length === 1 ? "asc" : pair[1]; @@ -83,21 +88,13 @@ export const createSortingSchema = ( }); } - if (transform) { - sorting.push({ property, direction: direction as "asc" | "desc" }); - } else { - sorting.push(value); - } + sorting.push({ property, direction: direction as "asc" | "desc" }); } return sorting; }); - if (transform) { - return Joi.object({ orderBy: orderBy.default([]) }); - } else { - return Joi.object({ orderBy }); - } + return Joi.object({ orderBy: orderBy.default([]) }); }; // Pagination @@ -162,8 +159,11 @@ export const orderBy = Joi.alternatives().try( Joi.array().items(Joi.string().regex(/^[a-z._]{1,40}:(asc|desc)$/i)), ); -export const blocksOrderBy = orderBy.default("height:desc"); -export const transactionsOrderBy = orderBy.default(["timestamp:desc", "sequence:desc"]); +export const blocksOrderBy = orderBy.default({ property: "height", direction: "desc" }); +export const transactionsOrderBy = orderBy.default([ + { property: "timestamp", direction: "desc" }, + { property: "sequence", direction: "desc" }, +]); const equalCriteria = (value: any) => value; const numericCriteria = (value: any) =>