From 02c65c567772a297dc25c3191597b60531fa9b73 Mon Sep 17 00:00:00 2001 From: Bryant Park Date: Tue, 18 Jun 2024 13:45:58 -0400 Subject: [PATCH] Add support for ordering on $select clauses in aggregations (#393) * Add OrderedAggregationClause with foo:bar properties * Update part of aggregate.test.ts Change the Exclude in the part to use keyof * Factor out ValidAggregationKeys * Update AggregationResultsWithoutGroups to handle new syntax - Replace AggregatableKeys | with ValidAggregationKeys * Move ValidAggregationKeys to AggregatableKeys.ts * Revert adding keyof to the Exclude in AOThatErrors * Add OrderedAggregateOpts to the union * Disallow ordered $select with multiple $groupBy in the type system This was done by wrapping the `AggregateOptionsThatErrors` in another `AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy` type. This also inadvertently fixed the broken aggregate.test.ts file. This makes no sense. Adding another layer somehow helped TypeScript correctly infer the original type again? * Update modernToLegacyAggregationClause to handle new format * Expect TS errors in legacyToModernSingleAggregationResult Fix later * Migrate AggregationsResults.test.ts to new syntax * Migrate more files to new syntax * Treat decimal and long as numeric properties instead of string properties * Remove long comment in aggregate.test.ts The issue the comment was describing was fixed in f1ca1e1c4bf367b3e986a26618eb3a95f5343091. * Test mock request body for aggregate without grouping * Add orderBy with and without groupBy tests * Allow ordered select with empty groupBy * Add test for type errors in the case of multiple groupBys - no ordering on select allowed * Don't export AggregateOptsThatErrors * Revert "Expect TS errors in legacyToModernSingleAggregationResult" This reverts commit 519fd7bdbf4c8919180268eaaa660ee1ef77cb22. * Restrict the types in legacyToModernSingleAggregationResult.ts as much as I can Leaving as is - it doesn't seem any worse than before this PR. * Rename k, v in modernToLegacyAggClause * Reorder fields in modernToLegacyAggClause to be consistent (micro-optimization) * Replace infer _ with string in AggResultsWOGroups * Fix broken tests after the order of fields was changed * Refactor AggregationResultsWithoutGroups with helper types * Remove AggregationCountResult type * Refactor test setup into a beforeEach * Add changeset * Use beforeAll instead of beforeEach in aggregate.test.ts * Remove outdated comment --- .changeset/chilly-singers-compare.md | 5 + .../fetchAggregationForEmployees.ts | 8 +- .../fetchAggregationForEmployeesGrouped.ts | 8 +- .../cli/src/runAggregationGroupByDatesTest.ts | 8 +- .../basic/cli/src/runAggregationsTest.ts | 25 +- examples-extra/basic/cli/src/typeChecks.ts | 10 +- packages/client.api/package.json | 1 + .../src/aggregate/AggregatableKeys.ts | 26 ++ .../client.api/src/aggregate/AggregateOpts.ts | 15 +- .../src/aggregate/AggregateOptsThatErrors.ts | 19 +- .../aggregate/AggregationResultsWithGroups.ts | 11 +- .../AggregationResultsWithoutGroups.ts | 35 +- .../src/aggregate/AggregationsClause.test.ts | 4 +- .../src/aggregate/AggregationsClause.ts | 29 +- .../src/aggregate/AggregationsResults.test.ts | 11 +- .../src/aggregate/AggregationsResults.ts | 22 +- packages/client.api/src/index.ts | 17 +- .../legacyToModernSingleAggregationResult.ts | 25 +- .../modernToLegacyAggregationClause.ts | 46 ++- packages/client/src/object/aggregate.test.ts | 332 ++++++++++++++---- packages/client/src/object/aggregate.ts | 8 +- packages/client/src/objectSet/ObjectSet.ts | 4 +- pnpm-lock.yaml | 3 + 23 files changed, 474 insertions(+), 198 deletions(-) create mode 100644 .changeset/chilly-singers-compare.md diff --git a/.changeset/chilly-singers-compare.md b/.changeset/chilly-singers-compare.md new file mode 100644 index 000000000..3bba8e21c --- /dev/null +++ b/.changeset/chilly-singers-compare.md @@ -0,0 +1,5 @@ +--- +"@osdk/client": minor +--- + +Rework $select syntax in aggregations to add support for ordering by metrics diff --git a/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployees.ts b/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployees.ts index 6ae56578a..3586c3ef3 100644 --- a/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployees.ts +++ b/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployees.ts @@ -25,9 +25,11 @@ export async function fetchAggregationForEmployees( ) { const result = await client(Employee).aggregate({ $select: { - locationCity: "approximateDistinct", - locationName: "approximateDistinct", - employeeNumber: ["avg", "max", "min"], + "locationCity:approximateDistinct": "unordered", + "locationName:approximateDistinct": "unordered", + "employeeNumber:avg": "unordered", + "employeeNumber:max": "unordered", + "employeeNumber:min": "unordered", }, $groupBy: undefined, }); diff --git a/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployeesGrouped.ts b/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployeesGrouped.ts index 767ae070a..22841be64 100644 --- a/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployeesGrouped.ts +++ b/examples-extra/basic/cli/src/legacy-examples/fetchAggregationForEmployeesGrouped.ts @@ -25,9 +25,11 @@ export async function fetchAggregationForEmployeesGrouped( ) { const result = await client(Employee).aggregate({ $select: { - locationCity: "approximateDistinct", - locationName: "approximateDistinct", - employeeNumber: ["avg", "max", "min"], + "locationCity:approximateDistinct": "unordered", + "locationName:approximateDistinct": "unordered", + "employeeNumber:avg": "unordered", + "employeeNumber:max": "unordered", + "employeeNumber:min": "unordered", }, $groupBy: { locationType: "exact", diff --git a/examples-extra/basic/cli/src/runAggregationGroupByDatesTest.ts b/examples-extra/basic/cli/src/runAggregationGroupByDatesTest.ts index 6c2df4a9f..705783f37 100644 --- a/examples-extra/basic/cli/src/runAggregationGroupByDatesTest.ts +++ b/examples-extra/basic/cli/src/runAggregationGroupByDatesTest.ts @@ -19,17 +19,17 @@ import { client } from "./client.js"; export async function runAggregationGroupByDatesTest() { const groupedTimestamps = await client(BuilderDeploymentState).aggregate({ - $select: { $count: true }, + $select: { $count: "unordered" }, $groupBy: { currentTimestamp: { $duration: [10, "seconds"] } }, }); const groupedDates = await client(BuilderDeploymentState).aggregate({ - $select: { $count: true }, + $select: { $count: "unordered" }, $groupBy: { date: { $duration: [10, "days"] } }, }); const rangedDates = await client(BuilderDeploymentState).aggregate({ - $select: { $count: true }, + $select: { $count: "unordered" }, $groupBy: { date: { $ranges: [["2008-03-01", "2009-11-05"], ["2015-10-01", "2018-11-05"]], @@ -38,7 +38,7 @@ export async function runAggregationGroupByDatesTest() { }); const rangedTimestamps = await client(BuilderDeploymentState).aggregate({ - $select: { $count: true }, + $select: { $count: "unordered" }, $groupBy: { currentTimestamp: { $ranges: [["2023-04-02T17:28:00Z", "2023-04-03T18:28:00Z"], [ diff --git a/examples-extra/basic/cli/src/runAggregationsTest.ts b/examples-extra/basic/cli/src/runAggregationsTest.ts index 21440236f..cf3a53890 100644 --- a/examples-extra/basic/cli/src/runAggregationsTest.ts +++ b/examples-extra/basic/cli/src/runAggregationsTest.ts @@ -29,7 +29,12 @@ export async function runAggregationsTest() { const testAggregateCountNoGroup = await client(BoundariesUsState) .aggregate({ - $select: { $count: true, latitude: ["min", "max", "avg"] }, + $select: { + $count: "unordered", + "latitude:max": "unordered", + "latitude:min": "unordered", + "latitude:avg": "unordered", + }, }); // Should be 51 because it includes DC @@ -41,7 +46,12 @@ export async function runAggregationsTest() { ); const testAggregateCountWithGroups = await client(BoundariesUsState) .aggregate({ - $select: { $count: true, latitude: ["min", "max", "avg"] }, + $select: { + $count: "unordered", + "latitude:max": "unordered", + "latitude:min": "unordered", + "latitude:avg": "unordered", + }, $groupBy: { usState: "exact", longitude: { @@ -52,7 +62,12 @@ export async function runAggregationsTest() { const testAggregateCountWithFixedGroups = await client(BoundariesUsState) .aggregate({ - $select: { $count: true, latitude: ["min", "max", "avg"] }, + $select: { + $count: "unordered", + "latitude:max": "unordered", + "latitude:min": "unordered", + "latitude:avg": "unordered", + }, $groupBy: { longitude: { $exactWithLimit: 40, @@ -62,7 +77,9 @@ export async function runAggregationsTest() { const testAggregateCountWithRangeGroups = await client(BoundariesUsState) .aggregate({ - $select: { $count: true }, + $select: { + $count: "unordered", + }, $groupBy: { latitude: { $ranges: [[34, 39], [ diff --git a/examples-extra/basic/cli/src/typeChecks.ts b/examples-extra/basic/cli/src/typeChecks.ts index c6cc5d399..a17a7c771 100644 --- a/examples-extra/basic/cli/src/typeChecks.ts +++ b/examples-extra/basic/cli/src/typeChecks.ts @@ -48,11 +48,11 @@ export async function typeChecks(client: Client) { const q = await client(Ontology.objects.ObjectTypeWithAllPropertyTypes) .aggregate({ $select: { - integer: "sum", - float: "sum", - decimal: "sum", - short: ["max"], - string: "approximateDistinct", + "integer:sum": "unordered", + "float:sum": "unordered", + "decimal:sum": "unordered", + "short:max": "unordered", + "string:approximateDistinct": "unordered", }, $groupBy: { string: "exact", diff --git a/packages/client.api/package.json b/packages/client.api/package.json index 19ec62875..2a0607a73 100644 --- a/packages/client.api/package.json +++ b/packages/client.api/package.json @@ -33,6 +33,7 @@ "@osdk/api": "workspace:^", "@osdk/internal.foundry": "workspace:^", "@types/geojson": "^7946.0.14", + "type-fest": "^4.18.2", "typescript": "^5.4.5" }, "publishConfig": { diff --git a/packages/client.api/src/aggregate/AggregatableKeys.ts b/packages/client.api/src/aggregate/AggregatableKeys.ts index 20c6af172..5374c74e3 100644 --- a/packages/client.api/src/aggregate/AggregatableKeys.ts +++ b/packages/client.api/src/aggregate/AggregatableKeys.ts @@ -15,6 +15,32 @@ */ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; +import type { PropertyValueClientToWire } from "@osdk/client.api"; + +export type StringAggregateOption = "approximateDistinct"; +export type NumericAggregateOption = + | "min" + | "max" + | "sum" + | "avg" + | "approximateDistinct"; + +type AGG_FOR_TYPE = number extends T ? NumericAggregateOption + : string extends T ? StringAggregateOption + : never; + +export type ValidAggregationKeys< + Q extends ObjectOrInterfaceDefinition, +> = keyof ( + & { + [ + KK in AggregatableKeys as `${KK & string}:${AGG_FOR_TYPE< + PropertyValueClientToWire[Q["properties"][KK]["type"]] + >}` + ]?: any; + } + & { $count?: any } +); export type AggregatableKeys< Q extends ObjectOrInterfaceDefinition, diff --git a/packages/client.api/src/aggregate/AggregateOpts.ts b/packages/client.api/src/aggregate/AggregateOpts.ts index accb2d0ee..1933fca6b 100644 --- a/packages/client.api/src/aggregate/AggregateOpts.ts +++ b/packages/client.api/src/aggregate/AggregateOpts.ts @@ -16,15 +16,16 @@ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; import type { GroupByClause } from "../groupby/GroupByClause.js"; -import type { AggregatableKeys } from "./AggregatableKeys.js"; -import type { AggregationClause } from "./AggregationsClause.js"; +import type { + OrderedAggregationClause, + UnorderedAggregationClause, +} from "./AggregationsClause.js"; import type { WhereClause } from "./WhereClause.js"; -export type AggregateOpts< - Q extends ObjectOrInterfaceDefinition, - KK extends AggregatableKeys = AggregatableKeys, -> = { - $select: AggregationClause; +export type AggregateOpts = { + $select: + | UnorderedAggregationClause + | OrderedAggregationClause; $where?: WhereClause; $groupBy?: GroupByClause; }; diff --git a/packages/client.api/src/aggregate/AggregateOptsThatErrors.ts b/packages/client.api/src/aggregate/AggregateOptsThatErrors.ts index 888995c53..dec431bb1 100644 --- a/packages/client.api/src/aggregate/AggregateOptsThatErrors.ts +++ b/packages/client.api/src/aggregate/AggregateOptsThatErrors.ts @@ -15,10 +15,27 @@ */ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; +import type { SingleKeyObject } from "type-fest"; import type { GroupByClause } from "../groupby/GroupByClause.js"; import type { AggregateOpts } from "./AggregateOpts.js"; +import type { UnorderedAggregationClause } from "./AggregationsClause.js"; -export type AggregateOptsThatErrors< +export type AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy< + Q extends ObjectOrInterfaceDefinition, + AO extends AggregateOpts, +> = SingleKeyObject extends never ? ( + AO["$select"] extends UnorderedAggregationClause + ? AggregateOptsThatErrors + : {} extends AO["$groupBy"] ? AggregateOptsThatErrors + : { + $groupBy: AO["$groupBy"]; + $select: UnorderedAggregationClause; + $where?: AO["$where"]; + } + ) + : AggregateOptsThatErrors; + +type AggregateOptsThatErrors< Q extends ObjectOrInterfaceDefinition, AO extends AggregateOpts, > = diff --git a/packages/client.api/src/aggregate/AggregationResultsWithGroups.ts b/packages/client.api/src/aggregate/AggregationResultsWithGroups.ts index 2a97412e2..a97284066 100644 --- a/packages/client.api/src/aggregate/AggregationResultsWithGroups.ts +++ b/packages/client.api/src/aggregate/AggregationResultsWithGroups.ts @@ -17,15 +17,15 @@ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; import type { OsdkObjectPropertyType } from "../Definitions.js"; import type { GroupByClause, GroupByRange } from "../groupby/GroupByClause.js"; +import type { AggregationResultsWithoutGroups } from "./AggregationResultsWithoutGroups.js"; import type { - AggregationCountResult, - AggregationResultsWithoutGroups, -} from "./AggregationResultsWithoutGroups.js"; -import type { AggregationClause } from "./AggregationsClause.js"; + OrderedAggregationClause, + UnorderedAggregationClause, +} from "./AggregationsClause.js"; export type AggregationResultsWithGroups< Q extends ObjectOrInterfaceDefinition, - A extends AggregationClause, + A extends UnorderedAggregationClause | OrderedAggregationClause, G extends GroupByClause | undefined, > = ( & { @@ -35,6 +35,5 @@ export type AggregationResultsWithGroups< : OsdkObjectPropertyType; }; } - & AggregationCountResult & AggregationResultsWithoutGroups )[]; diff --git a/packages/client.api/src/aggregate/AggregationResultsWithoutGroups.ts b/packages/client.api/src/aggregate/AggregationResultsWithoutGroups.ts index 0e9236171..16702b3ea 100644 --- a/packages/client.api/src/aggregate/AggregationResultsWithoutGroups.ts +++ b/packages/client.api/src/aggregate/AggregationResultsWithoutGroups.ts @@ -15,26 +15,29 @@ */ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; -import type { AggregationClause } from "@osdk/client.api"; +import type { + OrderedAggregationClause, + UnorderedAggregationClause, +} from "@osdk/client.api"; import type { OsdkObjectPropertyType } from "../Definitions.js"; -import type { StringArrayToUnion } from "../util/StringArrayToUnion.js"; -type SubselectKeys, P extends keyof AC> = - AC[P] extends readonly string[] | string ? P : never; +type ExtractPropName = T extends `${infer PropName}:${string}` + ? PropName + : T extends "$count" ? T + : never; + +type ExtractMetricNameForPropName = T extends + `${PropName}:${infer MetricName}` ? MetricName : never; export type AggregationResultsWithoutGroups< Q extends ObjectOrInterfaceDefinition, - AC extends AggregationClause, + AC extends UnorderedAggregationClause | OrderedAggregationClause, > = { - [P in keyof Q["properties"] as SubselectKeys]: AC[P] extends - readonly string[] | string ? { - [Z in StringArrayToUnion]: Z extends "approximateDistinct" ? number - : OsdkObjectPropertyType; - } - : never; + [PropName in ExtractPropName]: PropName extends "$count" + ? number + : { + [MetricName in ExtractMetricNameForPropName]: + MetricName extends "approximateDistinct" ? number + : OsdkObjectPropertyType; + }; }; - -export type AggregationCountResult< - Q extends ObjectOrInterfaceDefinition, - A extends AggregationClause, -> = "$count" extends keyof A ? { $count: number } : {}; diff --git a/packages/client.api/src/aggregate/AggregationsClause.test.ts b/packages/client.api/src/aggregate/AggregationsClause.test.ts index 36483a4de..f654a76fd 100644 --- a/packages/client.api/src/aggregate/AggregationsClause.test.ts +++ b/packages/client.api/src/aggregate/AggregationsClause.test.ts @@ -16,7 +16,7 @@ import { describe, it } from "vitest"; import type { AggregatableKeys } from "./AggregatableKeys.js"; -import type { AggregationClause } from "./AggregationsClause.js"; +import type { UnorderedAggregationClause } from "./AggregationsClause.js"; export type huh = AggregatableKeys< { @@ -43,7 +43,7 @@ export type huh = AggregatableKeys< }["objects"]["Todo"] >; -export type Q = AggregationClause< +export type Q = UnorderedAggregationClause< { metadata: any; objects: { diff --git a/packages/client.api/src/aggregate/AggregationsClause.ts b/packages/client.api/src/aggregate/AggregationsClause.ts index 85e73903d..7c90a869a 100644 --- a/packages/client.api/src/aggregate/AggregationsClause.ts +++ b/packages/client.api/src/aggregate/AggregationsClause.ts @@ -15,28 +15,11 @@ */ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; -import type { AggregatableKeys } from "@osdk/client.api"; +import type { ValidAggregationKeys } from "@osdk/client.api"; -type StringAggregateOption = "approximateDistinct"; -type NumericAggregateOption = - | "min" - | "max" - | "sum" - | "avg" - | "approximateDistinct"; +export type UnorderedAggregationClause = + { [AK in ValidAggregationKeys]?: "unordered" }; -type totalCountOption = { $count?: true }; - -export type AggregationClause< - Q extends ObjectOrInterfaceDefinition, - K extends AggregatableKeys = AggregatableKeys, -> = - & { - [P in K]?: Q["properties"][P]["type"] extends "string" - ? StringAggregateOption | StringAggregateOption[] - : Q["properties"][P]["type"] extends - "double" | "integer" | "float" | "decimal" | "byte" | "long" | "short" - ? NumericAggregateOption | NumericAggregateOption[] - : never; - } - & totalCountOption; +export type OrderedAggregationClause = { + [AK in ValidAggregationKeys]?: "unordered" | "asc" | "desc"; +}; diff --git a/packages/client.api/src/aggregate/AggregationsResults.test.ts b/packages/client.api/src/aggregate/AggregationsResults.test.ts index cde9727cd..fc56abeed 100644 --- a/packages/client.api/src/aggregate/AggregationsResults.test.ts +++ b/packages/client.api/src/aggregate/AggregationsResults.test.ts @@ -43,8 +43,8 @@ type T_AGG_RESULTS_TEST_1 = AggregationsResults< }["objects"]["Todo"], { $select: { - locationCity: "approximateDistinct"; - text: "approximateDistinct"; + "locationCity:approximateDistinct": "unordered"; + "text:approximateDistinct": "unordered"; }; $groupBy: { text: "exact"; @@ -76,9 +76,10 @@ type Q = AggregationResultsWithoutGroups< queries: {}; }["objects"]["Todo"], { - locationCity: "approximateDistinct"; - id: ["max", "sum"]; - text: "approximateDistinct"; + "locationCity:approximateDistinct": "unordered"; + "id:max": "unordered"; + "id:sum": "unordered"; + "text:approximateDistinct": "unordered"; } >; diff --git a/packages/client.api/src/aggregate/AggregationsResults.ts b/packages/client.api/src/aggregate/AggregationsResults.ts index 052f2458c..d7262c4c3 100644 --- a/packages/client.api/src/aggregate/AggregationsResults.ts +++ b/packages/client.api/src/aggregate/AggregationsResults.ts @@ -15,26 +15,22 @@ */ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; -import type { AggregatableKeys } from "./AggregatableKeys.js"; +import type { + AggregatableKeys, + ValidAggregationKeys, +} from "./AggregatableKeys.js"; import type { AggregateOpts } from "./AggregateOpts.js"; import type { AggregationResultsWithGroups } from "./AggregationResultsWithGroups.js"; -import type { - AggregationCountResult, - AggregationResultsWithoutGroups, -} from "./AggregationResultsWithoutGroups.js"; +import type { AggregationResultsWithoutGroups } from "./AggregationResultsWithoutGroups.js"; export type AggregationsResults< Q extends ObjectOrInterfaceDefinition, AO extends AggregateOpts, -> = Exclude | "$count"> extends never +> = Exclude> extends never ? unknown extends AO["$groupBy"] // groupBy is missing - ? - & AggregationResultsWithoutGroups - & AggregationCountResult + ? AggregationResultsWithoutGroups : Exclude extends never // groupBy is explicitly undefined - ? - & AggregationResultsWithoutGroups - & AggregationCountResult + ? AggregationResultsWithoutGroups : Exclude> extends never ? AggregationResultsWithGroups : `Sorry, the following are not valid groups for an aggregation: ${Exclude< @@ -43,5 +39,5 @@ export type AggregationsResults< >}` : `Sorry, the following are not valid selectors for an aggregation: ${Exclude< keyof AO["$select"] & string, - AggregatableKeys | "$count" + ValidAggregationKeys >}`; diff --git a/packages/client.api/src/index.ts b/packages/client.api/src/index.ts index cd11324eb..b4c86deed 100644 --- a/packages/client.api/src/index.ts +++ b/packages/client.api/src/index.ts @@ -25,15 +25,20 @@ export type { ApplyBatchActionOptions, OsdkActionParameters, } from "./actions/Actions.js"; -export type { AggregatableKeys } from "./aggregate/AggregatableKeys.js"; +export type { + AggregatableKeys, + NumericAggregateOption, + StringAggregateOption, + ValidAggregationKeys, +} from "./aggregate/AggregatableKeys.js"; export type { AggregateOpts } from "./aggregate/AggregateOpts.js"; -export type { AggregateOptsThatErrors } from "./aggregate/AggregateOptsThatErrors.js"; +export type { AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy } from "./aggregate/AggregateOptsThatErrors.js"; export type { AggregationResultsWithGroups } from "./aggregate/AggregationResultsWithGroups.js"; +export type { AggregationResultsWithoutGroups } from "./aggregate/AggregationResultsWithoutGroups.js"; export type { - AggregationCountResult, - AggregationResultsWithoutGroups, -} from "./aggregate/AggregationResultsWithoutGroups.js"; -export type { AggregationClause } from "./aggregate/AggregationsClause.js"; + OrderedAggregationClause, + UnorderedAggregationClause, +} from "./aggregate/AggregationsClause.js"; export type { AggregationsResults } from "./aggregate/AggregationsResults.js"; export type { AndWhereClause, diff --git a/packages/client/src/internal/conversions/legacyToModernSingleAggregationResult.ts b/packages/client/src/internal/conversions/legacyToModernSingleAggregationResult.ts index d769c58ba..768d66a76 100644 --- a/packages/client/src/internal/conversions/legacyToModernSingleAggregationResult.ts +++ b/packages/client/src/internal/conversions/legacyToModernSingleAggregationResult.ts @@ -14,21 +14,26 @@ * limitations under the License. */ +import type { ObjectOrInterfaceDefinition } from "@osdk/api"; import type { - AggregationClause, AggregationResultsWithoutGroups, + NumericAggregateOption, + OrderedAggregationClause, + StringAggregateOption, + UnorderedAggregationClause, } from "@osdk/client.api"; import type { AggregateObjectsResponseV2 } from "@osdk/internal.foundry"; import invariant from "tiny-invariant"; import type { ArrayElement } from "../../util/ArrayElement.js"; export function legacyToModernSingleAggregationResult< - AC extends AggregationClause, + Q extends ObjectOrInterfaceDefinition, + AC extends UnorderedAggregationClause | OrderedAggregationClause, >( entry: ArrayElement, -): AggregationResultsWithoutGroups { +): AggregationResultsWithoutGroups { return entry.metrics.reduce( - (accumulator, curValue) => { + (accumulator: AggregationResultsWithoutGroups, curValue) => { const parts = curValue.name.split("."); if (parts[0] === "count") { return accumulator; @@ -37,13 +42,17 @@ export function legacyToModernSingleAggregationResult< parts.length === 2, "assumed we were getting a `${key}.${type}`", ); - if (!(parts[0] in accumulator)) { - accumulator[parts[0]] = {}; + const property = parts[0] as keyof AggregationResultsWithoutGroups; + const metricType = parts[1] as + | StringAggregateOption + | NumericAggregateOption; + if (!(property in accumulator)) { + accumulator[property] = {} as any; // fixme? } - accumulator[parts[0]][parts[1]] = curValue.value; + (accumulator[property] as any)[metricType] = curValue.value; // fixme? return accumulator; }, - {} as AggregationResultsWithoutGroups, + {} as AggregationResultsWithoutGroups, ); } diff --git a/packages/client/src/internal/conversions/modernToLegacyAggregationClause.ts b/packages/client/src/internal/conversions/modernToLegacyAggregationClause.ts index 310d24b14..7cb2c7990 100644 --- a/packages/client/src/internal/conversions/modernToLegacyAggregationClause.ts +++ b/packages/client/src/internal/conversions/modernToLegacyAggregationClause.ts @@ -14,32 +14,42 @@ * limitations under the License. */ -import type { AggregationClause } from "@osdk/client.api"; +import type { + NumericAggregateOption, + OrderedAggregationClause, + StringAggregateOption, + UnorderedAggregationClause, +} from "@osdk/client.api"; import type { AggregationV2 } from "@osdk/internal.foundry"; +const directionFieldMap = (dir?: "asc" | "desc" | "unordered") => + dir === "asc" ? "ASC" : dir === "desc" ? "DESC" : undefined; + export function modernToLegacyAggregationClause< - AC extends AggregationClause, + AC extends UnorderedAggregationClause | OrderedAggregationClause, >(select: AC) { - return Object.entries(select).flatMap(([k, v]) => { - if (k === "$count") { - if (v) return { type: "count", name: "count" }; - return []; - } else if (Array.isArray(v)) { - return v.map((v2) => { + return Object.entries(select).flatMap( + ([propAndMetric, aggregationType]) => { + if (propAndMetric === "$count") { return { - type: v2, - name: `${k}.${v2}`, - field: k, + type: "count", + name: "count", + direction: directionFieldMap(aggregationType), }; - }); - } else { + } + + const colonPos = propAndMetric.lastIndexOf(":"); + const property = propAndMetric.slice(0, colonPos); + const metric = propAndMetric.slice(colonPos + 1); + return [ { - type: v as "min" | "max" | "sum" | "avg" | "approximateDistinct", // FIXME v has additional possible values - name: `${k}.${v}`, - field: k, + type: metric as StringAggregateOption | NumericAggregateOption, + name: `${property}.${metric}`, + direction: directionFieldMap(aggregationType), + field: property, }, ]; - } - }); + }, + ); } diff --git a/packages/client/src/object/aggregate.test.ts b/packages/client/src/object/aggregate.test.ts index d8035d8af..8f915fe29 100644 --- a/packages/client/src/object/aggregate.test.ts +++ b/packages/client/src/object/aggregate.test.ts @@ -17,14 +17,24 @@ import type { ObjectTypeDefinition, OntologyDefinition } from "@osdk/api"; import type { AggregateOpts, - AggregateOptsThatErrors, + AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy, GroupByClause, } from "@osdk/client.api"; import type { AggregateObjectsResponseV2 } from "@osdk/internal.foundry"; import type { TypeOf } from "ts-expect"; import { expectType } from "ts-expect"; -import { describe, expectTypeOf, it, type Mock, vi } from "vitest"; +import { + beforeAll, + beforeEach, + describe, + expect, + expectTypeOf, + it, + type Mock, + vi, +} from "vitest"; import { createMinimalClient } from "../createMinimalClient.js"; +import type { MinimalClient } from "../MinimalClientContext.js"; import { aggregate } from "./aggregate.js"; interface TodoDef extends ObjectTypeDefinition<"Todo"> { @@ -135,53 +145,58 @@ const mockOntology = { type mockOntology = typeof mockOntology; interface MockOntology extends mockOntology {} -describe("aggregate", () => { - it("works", async () => { - const mockFetch: Mock = vi.fn(); +let mockFetch: Mock; +let clientCtx: MinimalClient; + +beforeAll(() => { + mockFetch = vi.fn(); + + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + json: () => new Promise((resolve) => resolve(aggregationResponse)), + }); + + clientCtx = createMinimalClient( + mockOntology.metadata, + "https://host.com", + async () => "", + {}, + mockFetch, + ); +}); - const aggregationResponse: AggregateObjectsResponseV2 = { - accuracy: "APPROXIMATE", - data: [ +const aggregationResponse: AggregateObjectsResponseV2 = { + accuracy: "APPROXIMATE", + data: [ + { + group: { + text: "hello", + }, + metrics: [ { - group: { - text: "hello", - }, - metrics: [ - { - name: "text.approximateDistinct", - value: 1, - }, - { - name: "priority.avg", - value: 1, - }, - { - name: "id.max", - value: 1, - }, - { - name: "id.avg", - value: 1, - }, - ], + name: "text.approximateDistinct", + value: 1, + }, + { + name: "priority.avg", + value: 1, + }, + { + name: "id.max", + value: 1, + }, + { + name: "id.avg", + value: 1, }, ], - }; - - mockFetch.mockResolvedValue({ - ok: true, - status: 200, - json: () => new Promise((resolve) => resolve(aggregationResponse)), - }); - - const clientCtx = createMinimalClient( - mockOntology.metadata, - "https://host.com", - async () => "", - {}, - mockFetch, - ); + }, + ], +}; +describe("aggregate", () => { + it("works", async () => { const notGrouped = await aggregate( clientCtx, Todo, @@ -191,14 +206,38 @@ describe("aggregate", () => { }, { $select: { - text: "approximateDistinct", - priority: "avg", - id: ["max", "avg"], - $count: true, + "text:approximateDistinct": "unordered", + "priority:avg": "unordered", + "id:max": "unordered", + "id:avg": "unordered", + "$count": "unordered", }, }, ); + expect(mockFetch).toHaveBeenCalledWith( + "https://host.com/api/v2/ontologies/ri.a.b.c.d/objectSets/aggregate", + { + body: JSON.stringify({ + "objectSet": { "type": "base", "objectType": "ToDo" }, + "groupBy": [], + "aggregation": [ + { + "type": "approximateDistinct", + "name": "text.approximateDistinct", + "field": "text", + }, + { "type": "avg", "name": "priority.avg", "field": "priority" }, + { "type": "max", "name": "id.max", "field": "id" }, + { "type": "avg", "name": "id.avg", "field": "id" }, + { "type": "count", "name": "count" }, + ], + }), + method: "POST", + headers: expect.anything(), + }, + ); + expectType(notGrouped.text.approximateDistinct); expectType(notGrouped.priority.avg); expectType(notGrouped.id.max); @@ -222,8 +261,9 @@ describe("aggregate", () => { }, { $select: { - id: "approximateDistinct", - $count: true, + "id:approximateDistinct": "unordered", + "priority:max": "unordered", + "$count": "unordered", }, $groupBy: { text: "exact", @@ -258,10 +298,10 @@ describe("aggregate", () => { expectType(grouped[0].$group.boolean); expectType< - AggregateOptsThatErrors { }> >({ $select: { - id: "approximateDistinct", - $count: true, + "id:approximateDistinct": "unordered", + "$count": "unordered", }, $groupBy: { text: "exact", @@ -290,11 +330,11 @@ describe("aggregate", () => { }); expectType< - AggregateOptsThatErrors { id: "approximateDistinct", // @ts-expect-error wrongSelectKey: "don't work", - $count: true, + "$count": "unordered", }, $groupBy: { // @ts-expect-error @@ -330,9 +370,9 @@ describe("aggregate", () => { expectTypeOf< typeof aggregate { }, { $select: { - id: "approximateDistinct", + "id:approximateDistinct": "unordered", // @ts-expect-error - wrongSelectKey: "wrongKey", - $count: true, + "wrongSelectKey": "don't work", + "$count": "unordered", }, $groupBy: { text: "exact", @@ -400,6 +440,162 @@ describe("aggregate", () => { }); }); + it("works with $orderBy (no groups)", async () => { + const notGrouped = await aggregate( + clientCtx, + Todo, + { + type: "base", + objectType: "ToDo", + }, + { + $select: { + "text:approximateDistinct": "asc", + "priority:avg": "desc", + "id:max": "asc", + "id:avg": "unordered", + "$count": "unordered", + }, + }, + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://host.com/api/v2/ontologies/ri.a.b.c.d/objectSets/aggregate", + { + body: JSON.stringify({ + "objectSet": { "type": "base", "objectType": "ToDo" }, + "groupBy": [], + "aggregation": [ + { + "type": "approximateDistinct", + "name": "text.approximateDistinct", + direction: "ASC", + "field": "text", + }, + { + "type": "avg", + "name": "priority.avg", + direction: "DESC", + "field": "priority", + }, + { + "type": "max", + "name": "id.max", + direction: "ASC", + "field": "id", + }, + { "type": "avg", "name": "id.avg", "field": "id" }, + { "type": "count", "name": "count" }, + ], + }), + method: "POST", + headers: expect.anything(), + }, + ); + + expectType(notGrouped.text.approximateDistinct); + expectType(notGrouped.priority.avg); + expectType(notGrouped.id.max); + expectType(notGrouped.id.avg); + expectType(notGrouped.$count); + expectType< + TypeOf< + { + other: any; + }, + typeof notGrouped + > + >(false); // subselect should hide unused keys + }); + + it("works with $orderBy (1 group)", async () => { + const grouped = await aggregate( + clientCtx, + Todo, + { + type: "base", + objectType: "ToDo", + }, + { + $select: { + "id:max": "desc", + "text:approximateDistinct": "asc", + "id:avg": "unordered", + "$count": "unordered", + }, + $groupBy: { + priority: "exact", + }, + }, + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://host.com/api/v2/ontologies/ri.a.b.c.d/objectSets/aggregate", + { + body: JSON.stringify({ + "objectSet": { "type": "base", "objectType": "ToDo" }, + "groupBy": [{ "type": "exact", "field": "priority" }], + "aggregation": [ + { + "type": "max", + "name": "id.max", + direction: "DESC", + "field": "id", + }, + { + "type": "approximateDistinct", + "name": "text.approximateDistinct", + direction: "ASC", + "field": "text", + }, + { "type": "avg", "name": "id.avg", "field": "id" }, + { "type": "count", "name": "count" }, + ], + }), + method: "POST", + headers: expect.anything(), + }, + ); + + expectType(grouped[0].text.approximateDistinct); + expectType(grouped[0].id.max); + expectType(grouped[0].id.avg); + expectType(grouped[0].$count); + expectType< + TypeOf< + { + other: any; + }, + typeof grouped + > + >(false); // subselect should hide unused keys + }); + + it("prohibits ordered select with multiple groupBy", async () => { + aggregate( + clientCtx, + Todo, + { + type: "base", + objectType: "ToDo", + }, + { + $select: { + // @ts-expect-error + "id:max": "desc", + // @ts-expect-error + "text:approximateDistinct": "asc", + "id:avg": "unordered", + "$count": "unordered", + }, + $groupBy: { + priority: "exact", + timestamp: "exact", + }, + }, + ); + }); + it("works with where: todo", async () => { type f = AggregateOpts< { @@ -458,8 +654,8 @@ describe("aggregate", () => { }["objects"]["Todo"] > = { $select: { - locationCity: "approximateDistinct", - text: "approximateDistinct", + "locationCity:approximateDistinct": "unordered", + "text:approximateDistinct": "unordered", }, }; diff --git a/packages/client/src/object/aggregate.ts b/packages/client/src/object/aggregate.ts index 23f6de5af..e3d8f3c9c 100644 --- a/packages/client/src/object/aggregate.ts +++ b/packages/client/src/object/aggregate.ts @@ -17,7 +17,7 @@ import type { ObjectOrInterfaceDefinition } from "@osdk/api"; import type { AggregateOpts, - AggregateOptsThatErrors, + AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy, AggregationResultsWithGroups, AggregationsResults, } from "@osdk/client.api"; @@ -47,7 +47,7 @@ export async function aggregateOrThrow< type: "base", objectType: objectType["apiName"] as string, }, - req: AggregateOptsThatErrors, + req: AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy, ): Promise> { return aggregate(clientCtx, objectType, objectSet, req); } @@ -62,7 +62,7 @@ export async function aggregate< type: "base", objectType: objectType["apiName"] as string, }, - req: AggregateOptsThatErrors, + req: AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy, ): Promise> { const body: AggregateObjectsRequestV2 = { aggregation: modernToLegacyAggregationClause( @@ -96,7 +96,7 @@ export async function aggregate< return { ...aggregationToCountResult(result.data[0]), - ...legacyToModernSingleAggregationResult( + ...legacyToModernSingleAggregationResult( result.data[0], ), } as any; diff --git a/packages/client/src/objectSet/ObjectSet.ts b/packages/client/src/objectSet/ObjectSet.ts index 21e74d426..637634faf 100644 --- a/packages/client/src/objectSet/ObjectSet.ts +++ b/packages/client/src/objectSet/ObjectSet.ts @@ -22,7 +22,7 @@ import type { } from "@osdk/api"; import type { AggregateOpts, - AggregateOptsThatErrors, + AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy, AggregationsResults, BaseObjectSet, LinkedType, @@ -81,7 +81,7 @@ export interface ObjectSet extends MinimalObjectSet { aggregate: >( - req: AggregateOptsThatErrors, + req: AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy, ) => Promise>; where: ( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 58a75d117..4b1b60d81 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -792,6 +792,9 @@ importers: '@types/geojson': specifier: ^7946.0.14 version: 7946.0.14 + type-fest: + specifier: ^4.18.2 + version: 4.18.2 typescript: specifier: ^5.4.5 version: 5.4.5