-
Notifications
You must be signed in to change notification settings - Fork 20
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
a2f331d
bec53ca
d8fa066
ecb0522
abc0b2d
831f890
a270994
7a8847f
2659b3b
1167fae
330dac1
eb99bab
c5686e0
624b662
2812538
c2c675f
95829f8
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: { | ||
nihalbhatnagar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[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