-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RDP's in TS OSDK #1035
base: main
Are you sure you want to change the base?
RDP's in TS OSDK #1035
Changes from 12 commits
246f893
50ca838
4e17a4e
01f5935
ebe19e4
4ea27dc
d2d0a0d
efbed3d
40ee625
3e535da
20835ea
76a3ee1
73c23fe
d605787
214dad7
73fbffa
9070c29
ac7f03f
2777334
a8546ca
0cd49f5
0a89f9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
"@osdk/shared.test": minor | ||
"@osdk/client": minor | ||
"@osdk/api": minor | ||
--- | ||
|
||
Adds support for runtime derived properties |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/* | ||
* Copyright 2024 Palantir Technologies, Inc. All rights reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import type { | ||
ObjectOrInterfaceDefinition, | ||
} from "../ontology/ObjectOrInterface.js"; | ||
import type { | ||
ObjectMetadata, | ||
PropertyDef, | ||
} from "../ontology/ObjectTypeDefinition.js"; | ||
import type { BaseDeriveObjectSet } from "./DeriveObjectSet.js"; | ||
|
||
export type DerivedPropertyDefinition< | ||
T extends ObjectMetadata.Property, | ||
> = { | ||
definitionId: unknown; | ||
type: T; | ||
}; | ||
|
||
export type DeriveClause< | ||
Q extends ObjectOrInterfaceDefinition, | ||
> = { | ||
[key: string]: ( | ||
baseObjectSet: BaseDeriveObjectSet<Q>, | ||
) => DerivedPropertyDefinition<ObjectMetadata.Property>; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
/* | ||
* Copyright 2024 Palantir Technologies, Inc. All rights reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import type { ValidAggregationKeys } from "../aggregate/AggregatableKeys.js"; | ||
import type { WhereClause } from "../aggregate/WhereClause.js"; | ||
import type { | ||
ObjectOrInterfaceDefinition, | ||
PropertyKeys, | ||
} from "../ontology/ObjectOrInterface.js"; | ||
import type { | ||
CompileTimeMetadata, | ||
PropertyDef, | ||
} from "../ontology/ObjectTypeDefinition.js"; | ||
import type { LinkedType, LinkNames } from "../util/LinkUtils.js"; | ||
import type { DerivedPropertyDefinition } from "./DeriveClause.js"; | ||
|
||
export interface BaseDeriveObjectSet<Q extends ObjectOrInterfaceDefinition> | ||
extends FilterableDeriveObjectSet<Q> | ||
{ | ||
readonly pivotTo: <L extends LinkNames<Q>>( | ||
type: L, | ||
) => NonNullable<CompileTimeMetadata<Q>["links"][L]["multiplicity"]> extends | ||
false ? SingleLinkDeriveObjectSet<LinkedType<Q, L>> | ||
: ManyLinkDeriveObjectSet<LinkedType<Q, L>>; | ||
} | ||
|
||
export interface DeriveObjectSet<Q extends ObjectOrInterfaceDefinition> | ||
extends | ||
BaseDeriveObjectSet<Q>, | ||
AggregatableDeriveObjectSet<Q>, | ||
SingleLinkDeriveObjectSet<Q> | ||
{} | ||
|
||
interface FilterableDeriveObjectSet< | ||
Q extends ObjectOrInterfaceDefinition, | ||
> { | ||
readonly where: ( | ||
clause: WhereClause<Q>, | ||
) => this; | ||
} | ||
|
||
type CollectAggregations = "collectSet" | "collectList"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parroting gateway names for now. This probably needs a review into |
||
|
||
type BaseDeriveAggregations = | ||
| "approximateDistinct" | ||
| "exactDistinct" | ||
| "approximatePercentile"; | ||
|
||
export type StringDeriveAggregateOption = | ||
| BaseDeriveAggregations | ||
| CollectAggregations; | ||
export type NumericDeriveAggregateOption = | ||
| "min" | ||
| "max" | ||
| "sum" | ||
| "avg" | ||
| BaseDeriveAggregations | ||
| CollectAggregations; | ||
|
||
interface AggregatableDeriveObjectSet< | ||
Q extends ObjectOrInterfaceDefinition, | ||
> extends FilterableDeriveObjectSet<Q> { | ||
readonly aggregate: < | ||
V extends ValidAggregationKeys< | ||
Q, | ||
StringDeriveAggregateOption, | ||
NumericDeriveAggregateOption | ||
>, | ||
>( | ||
aggregationSpecifier: V, | ||
opts?: V extends `${any}:${infer P}` | ||
? P extends CollectAggregations ? { limit: number } | ||
: P extends "approximatePercentile" ? { percentile: number } | ||
: never | ||
: never, | ||
) => DerivedPropertyDefinition< | ||
V extends `${infer N}:${infer P}` | ||
? P extends CollectAggregations ? PropertyDef< | ||
CompileTimeMetadata<Q>["properties"][N]["type"], | ||
"nullable", | ||
"array" | ||
> | ||
: P extends "approximateDistinct" | "exactDistinct" | "$count" | ||
? PropertyDef<"integer"> | ||
: PropertyDef<"double"> | ||
: never | ||
>; | ||
} | ||
|
||
interface SingleLinkDeriveObjectSet< | ||
Q extends ObjectOrInterfaceDefinition, | ||
> extends AggregatableDeriveObjectSet<Q>, BaseDeriveObjectSet<Q> { | ||
readonly selectProperty: <R extends PropertyKeys<Q>>( | ||
propertyName: R, | ||
) => DerivedPropertyDefinition<CompileTimeMetadata<Q>["properties"][R]>; | ||
} | ||
|
||
interface ManyLinkDeriveObjectSet< | ||
Q extends ObjectOrInterfaceDefinition, | ||
> extends AggregatableDeriveObjectSet<Q> { | ||
readonly pivotTo: <L extends LinkNames<Q>>( | ||
type: L, | ||
) => ManyLinkDeriveObjectSet<LinkedType<Q, L>>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,15 @@ export type { | |
OsdkObjectLinksObject, | ||
SingleLinkAccessor, | ||
} from "./definitions/LinkDefinitions.js"; | ||
export type { | ||
DeriveClause, | ||
DerivedPropertyDefinition, | ||
} from "./derivedProperties/DeriveClause.js"; | ||
export type { | ||
BaseDeriveObjectSet, | ||
DeriveObjectSet, | ||
} from "./derivedProperties/DeriveObjectSet.js"; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra new line causes grouping when organizing imports instead of perfect sorting. |
||
export { DurationMapping } from "./groupby/GroupByClause.js"; | ||
export type { | ||
AllGroupByValues, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,10 @@ import type { AggregateOpts } from "../aggregate/AggregateOpts.js"; | |
import type { AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy } from "../aggregate/AggregateOptsThatErrors.js"; | ||
import type { AggregationsResults } from "../aggregate/AggregationsResults.js"; | ||
import type { WhereClause } from "../aggregate/WhereClause.js"; | ||
import type { | ||
DeriveClause, | ||
DerivedPropertyDefinition, | ||
} from "../derivedProperties/DeriveClause.js"; | ||
import type { | ||
AsyncIterArgs, | ||
Augments, | ||
|
@@ -34,7 +38,9 @@ import type { | |
} from "../ontology/ObjectOrInterface.js"; | ||
import type { | ||
CompileTimeMetadata, | ||
ObjectMetadata, | ||
ObjectTypeDefinition, | ||
PropertyDef, | ||
} from "../ontology/ObjectTypeDefinition.js"; | ||
import type { PrimaryKeyType } from "../OsdkBase.js"; | ||
import type { ExtractOptions, Osdk } from "../OsdkObjectFrom.js"; | ||
|
@@ -254,4 +260,28 @@ export interface ObjectSet< | |
listener: ObjectSetListener<Q, P>, | ||
opts?: ObjectSetListenerOptions<Q, P>, | ||
) => { unsubscribe: () => void }; | ||
|
||
readonly withProperties: < | ||
D extends DeriveClause<Q>, | ||
>( | ||
clause: D, | ||
) => ObjectSet< | ||
DerivedPropertyExtendedObjectDefinition< | ||
Q, | ||
D | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm okay with it for now but this feels like a place where having the simpler types that are not based on functions produces more readable types? We have to do the extraction work anyway to create things like |
||
> | ||
>; | ||
} | ||
|
||
type DerivedPropertyExtendedObjectDefinition< | ||
K extends ObjectOrInterfaceDefinition, | ||
D extends DeriveClause<K>, | ||
> = { | ||
__DefinitionMetadata: { | ||
properties: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is all we need to propagate the types through all further object set operations. The FLUP here is to make the type of the whole object set extractable and definable so that a user can pass around object sets that are correctly typed with the additional properties and do not have to be inferred. |
||
[T in Extract<keyof D, string>]: D[T] extends | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd have to check but my gut is |
||
(baseObjectSet: any) => DerivedPropertyDefinition<infer P> ? P | ||
: never; | ||
}; | ||
}; | ||
} & K; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to keep all the agg options a bit simpler to parse though, another way to format this type could be keeping the
Agg_For_Type
and modifying it to take an extra boolean (that controls whether this is for derive or not, default to false) and have it branch to the right agg options that way, rather than having the responsibility on the user to pass in custom options if they want. That being said, if you feel like in the future we'll need to customize agg options even more, this does make it more flexible so up to youThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do keep it like this, lets remove
AGG_FOR_TYPE
if its not being used anywhereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yeah this does make sense to me, will refactor