Skip to content
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

Add structs read support #1047

Merged
merged 10 commits into from
Dec 16, 2024
Merged
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
12 changes: 12 additions & 0 deletions .changeset/quiet-bears-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@osdk/foundry-sdk-generator": patch
"@osdk/generator-converters": patch
"@osdk/cli.cmd.typescript": patch
"@osdk/shared.test": patch
"@osdk/generator": patch
"@osdk/client": patch
"@osdk/maker": patch
"@osdk/api": patch
---

Add support for reading struct properties.
12 changes: 8 additions & 4 deletions etc/api.report.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -694,10 +694,11 @@ export type OsdkObjectLinksObject<O extends ObjectTypeDefinition> = ObjectTypeLi
// Warning: (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
// Warning: (ae-forgotten-export) The symbol "MaybeArray" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "Converted" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "GetClientPropertyValueFromWire" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "MaybeNullable" needs to be exported by the entry point index.d.ts
//
// @public (undocumented)
export type OsdkObjectPropertyType<T extends ObjectMetadata.Property, STRICTLY_ENFORCE_NULLABLE extends boolean = true> = STRICTLY_ENFORCE_NULLABLE extends false ? MaybeArray<T, Converted<PropertyValueWireToClient[T["type"]]>> | undefined : MaybeNullable<T, MaybeArray<T, Converted<PropertyValueWireToClient[T["type"]]>>>;
export type OsdkObjectPropertyType<T extends ObjectMetadata.Property, STRICTLY_ENFORCE_NULLABLE extends boolean = true> = STRICTLY_ENFORCE_NULLABLE extends false ? MaybeArray<T, Converted<GetClientPropertyValueFromWire<T["type"]>>> | undefined : MaybeNullable<T, MaybeArray<T, Converted<GetClientPropertyValueFromWire<T["type"]>>>>;

// @public (undocumented)
export interface PageResult<T> {
Expand Down Expand Up @@ -852,6 +853,9 @@ export interface SelectArg<Q extends ObjectOrInterfaceDefinition, L extends Prop
// @public (undocumented)
export type SelectArgToKeys<Q extends ObjectOrInterfaceDefinition, A extends SelectArg<Q, any, any>> = A extends SelectArg<Q, never> ? PropertyKeys<Q> : A["$select"] extends readonly string[] ? A["$select"][number] : PropertyKeys<Q>;

// @public (undocumented)
export type SimpleWirePropertyTypes = "string" | "datetime" | "double" | "boolean" | "integer" | "timestamp" | "short" | "long" | "float" | "decimal" | "byte" | "marking" | "numericTimeseries" | "stringTimeseries" | "sensorTimeseries" | "attachment" | "geopoint" | "geoshape" | "geotimeSeriesReference";

// @public (undocumented)
export interface SingleLinkAccessor<T extends ObjectTypeDefinition> {
fetchOne: <const A extends SelectArg<T, PropertyKeys<T>, boolean>>(options?: A) => Promise<A extends FetchPageArgs<T, infer L, infer R, any, infer S> ? Osdk.Instance<T, ExtractOptions<R, S>, L> : Osdk.Instance<T>>;
Expand Down Expand Up @@ -945,11 +949,11 @@ export type TimeSeriesQuery = {
export type TwoDimensionalQueryAggregationDefinition = AggregationKeyDataType<"date" | "double" | "timestamp">;

// Warning: (ae-forgotten-export) The symbol "AGG_FOR_TYPE" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "PropertyValueClientToWire" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "GetWirePropertyValueFromClient" needs to be exported by the entry point index.d.ts
//
// @public (undocumented)
export type ValidAggregationKeys<Q extends ObjectOrInterfaceDefinition> = keyof ({
[KK in AggregatableKeys<Q> as `${KK & string}:${AGG_FOR_TYPE<PropertyValueClientToWire[CompileTimeMetadata<Q>["properties"][KK]["type"]]>}`]?: any;
[KK in AggregatableKeys<Q> as `${KK & string}:${AGG_FOR_TYPE<GetWirePropertyValueFromClient<CompileTimeMetadata<Q>["properties"][KK]["type"]>>}`]?: any;
} & {
$count?: any;
});
Expand All @@ -973,7 +977,7 @@ export type WhereClause<T extends ObjectOrInterfaceDefinition> = OrWhereClause<T
});

// @public (undocumented)
export type WirePropertyTypes = "string" | "datetime" | "double" | "boolean" | "integer" | "timestamp" | "short" | "long" | "float" | "decimal" | "byte" | "marking" | "numericTimeseries" | "stringTimeseries" | "sensorTimeseries" | "attachment" | "geopoint" | "geoshape" | "geotimeSeriesReference";
export type WirePropertyTypes = SimpleWirePropertyTypes | Record<string, SimpleWirePropertyTypes>;

// Warnings were encountered during analysis:
//
Expand Down
17 changes: 12 additions & 5 deletions packages/api/src/Definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
* limitations under the License.
*/

import type { PropertyValueWireToClient } from "./mapping/PropertyValueMapping.js";
import type {
GetClientPropertyValueFromWire,
PropertyValueWireToClient,
} from "./mapping/PropertyValueMapping.js";
import type { ObjectMetadata } from "./ontology/ObjectTypeDefinition.js";

type MaybeArray<T extends { multiplicity?: boolean | undefined }, U> =
Expand All @@ -34,12 +37,16 @@ type Converted<T> = T extends Array<any> ? T[1] : T;
export type OsdkObjectPropertyType<
T extends ObjectMetadata.Property,
STRICTLY_ENFORCE_NULLABLE extends boolean = true,
> = STRICTLY_ENFORCE_NULLABLE extends false
? MaybeArray<T, Converted<PropertyValueWireToClient[T["type"]]>> | undefined
> = STRICTLY_ENFORCE_NULLABLE extends false ?
| MaybeArray<T, Converted<GetClientPropertyValueFromWire<T["type"]>>>
| undefined
: MaybeNullable<
T,
MaybeArray<T, Converted<PropertyValueWireToClient[T["type"]]>>
MaybeArray<T, Converted<GetClientPropertyValueFromWire<T["type"]>>>
>;

export type OsdkObjectRawPropertyType<T extends ObjectMetadata.Property> =
MaybeNullable<T, MaybeArray<T, Raw<PropertyValueWireToClient[T["type"]]>>>;
MaybeNullable<
T,
MaybeArray<T, Raw<GetClientPropertyValueFromWire<T["type"]>>>
>;
9 changes: 6 additions & 3 deletions packages/api/src/aggregate/AggregatableKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
* limitations under the License.
*/

import type { PropertyValueClientToWire } from "../mapping/PropertyValueMapping.js";
import type {
GetWirePropertyValueFromClient,
PropertyValueClientToWire,
} from "../mapping/PropertyValueMapping.js";
import type {
ObjectOrInterfaceDefinition,
PropertyKeys,
Expand All @@ -40,9 +43,9 @@ export type ValidAggregationKeys<
& {
[
KK in AggregatableKeys<Q> as `${KK & string}:${AGG_FOR_TYPE<
PropertyValueClientToWire[
GetWirePropertyValueFromClient<
CompileTimeMetadata<Q>["properties"][KK]["type"]
]
>
>}`
]?: any;
}
Expand Down
5 changes: 4 additions & 1 deletion packages/api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ export type {
ThreeDimensionalQueryAggregationDefinition,
TwoDimensionalQueryAggregationDefinition,
} from "./ontology/QueryDefinition.js";
export type { WirePropertyTypes } from "./ontology/WirePropertyTypes.js";
export type {
SimpleWirePropertyTypes,
WirePropertyTypes,
} from "./ontology/WirePropertyTypes.js";
export type { OsdkBase, PrimaryKeyType } from "./OsdkBase.js";
export type { OsdkObject } from "./OsdkObject.js";
export type { ConvertProps, Osdk } from "./OsdkObjectFrom.js";
Expand Down
17 changes: 17 additions & 0 deletions packages/api/src/mapping/PropertyValueMapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ export interface PropertyValueWireToClient {
geotimeSeriesReference: GeotimeSeriesProperty<GeoJSON.Point>;
}

export type GetClientPropertyValueFromWire<
T extends
| keyof PropertyValueWireToClient
| Record<string, keyof PropertyValueWireToClient>,
> = T extends keyof PropertyValueWireToClient ? PropertyValueWireToClient[T]
: T extends Record<string, keyof PropertyValueWireToClient>
? { [K in keyof T]: PropertyValueWireToClient[T[K]] }
: never;

/**
* Map from the PropertyDefinition type to the typescript type that we accept
*/
Expand All @@ -71,3 +80,11 @@ export interface PropertyValueClientToWire {
sensorTimeseries: TimeSeriesProperty<string | number>;
geotimeSeriesReference: GeotimeSeriesProperty<GeoJSON.Point>;
}
export type GetWirePropertyValueFromClient<
T extends
| keyof PropertyValueClientToWire
| Record<string, keyof PropertyValueClientToWire>,
> = T extends keyof PropertyValueClientToWire ? PropertyValueClientToWire[T]
: T extends Record<string, keyof PropertyValueClientToWire>
? { [K in keyof T]: PropertyValueClientToWire[T[K]] }
: never;
34 changes: 34 additions & 0 deletions packages/api/src/object/ObjectDefinitions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,38 @@ describe("Object definitions", () => {
Attachment[]
>();
});
it("correctly maps struct types", () => {
const structType = {
type: {
integerField: "integer",
floatField: "float",
attachment: "attachment",
},
} as const;

const structTypeArray = {
type: {
integerField: "integer",
floatField: "float",
attachment: "attachment",
},
multiplicity: true,
} as const;
expectTypeOf<OsdkObjectPropertyType<typeof structType>>().toEqualTypeOf<
{
readonly integerField: number;
readonly floatField: number;
readonly attachment: Attachment;
}
>();

expectTypeOf<OsdkObjectPropertyType<typeof structTypeArray>>()
.toEqualTypeOf<
{
readonly integerField: number;
readonly floatField: number;
readonly attachment: Attachment;
}[]
>();
});
});
4 changes: 4 additions & 0 deletions packages/api/src/ontology/WirePropertyTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
*/

export type WirePropertyTypes =
Copy link
Member

Choose a reason for hiding this comment

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

Should this be WireAndStructPropertyTypes? That way we don't break WirePropertyTypes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Talked a bit on slack, just to clarify here as well, the reason you think this is a break is because now instead of just a list of strings, its SimpleWirePropertyTypes and the record type? Because technically this boils down to the same thing and is just an additive change, but if someone is using this type, could be confusing since now it referencs something called SimpleWirePropertyTypes?

| SimpleWirePropertyTypes
| Record<string, SimpleWirePropertyTypes>;

export type SimpleWirePropertyTypes =
| "string"
| "datetime"
| "double"
Expand Down
4 changes: 2 additions & 2 deletions packages/cli.cmd.typescript/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"@arethetypeswrong/cli": "^0.15.2",
"@osdk/cli.common": "workspace:~",
"@osdk/generator": "workspace:~",
"@osdk/internal.foundry.core": "2.6.0-beta.1",
"@osdk/internal.foundry.ontologiesv2": "2.6.0-beta.1",
"@osdk/internal.foundry.core": "2.8.0",
"@osdk/internal.foundry.ontologiesv2": "2.8.0",
"@osdk/shared.client.impl": "workspace:~",
"consola": "^3.2.3",
"fast-deep-equal": "^3.1.3",
Expand Down
4 changes: 2 additions & 2 deletions packages/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
"@osdk/api": "workspace:~",
"@osdk/client.unstable": "workspace:*",
"@osdk/generator-converters": "workspace:*",
"@osdk/internal.foundry.core": "2.6.0-beta.1",
"@osdk/internal.foundry.ontologiesv2": "2.6.0-beta.1",
"@osdk/internal.foundry.core": "2.8.0",
"@osdk/internal.foundry.ontologiesv2": "2.8.0",
"@osdk/shared.client": "^1.0.1",
"@osdk/shared.client.impl": "workspace:~",
"@osdk/shared.client2": "^1.0.0",
Expand Down
32 changes: 17 additions & 15 deletions packages/client/src/object/aggregate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
import type {
AggregateOpts,
AggregateOptsThatErrorsAndDisallowsOrderingWithMultipleGroupBy,
CompileTimeMetadata,
GroupByClause,
ObjectOrInterfaceDefinition,
PropertyKeys,
ValidAggregationKeys,
} from "@osdk/api";
import type { Employee } from "@osdk/client.test.ontology";
Expand All @@ -37,6 +40,8 @@ import {
type Mock,
vi,
} from "vitest";
import type { Client } from "../Client.js";
import { createClient } from "../createClient.js";
import { createMinimalClient } from "../createMinimalClient.js";
import type { MinimalClient } from "../MinimalClientContext.js";
import { aggregate } from "./aggregate.js";
Expand All @@ -50,6 +55,7 @@ const metadata = {

let mockFetch: Mock;
let clientCtx: MinimalClient;
let client: Client;

beforeAll(() => {
mockFetch = vi.fn();
Expand All @@ -63,10 +69,18 @@ beforeAll(() => {
clientCtx = createMinimalClient(
metadata,
"https://host.com",
async () => "",
async () => "myAccessToken",
{},
mockFetch,
);

client = createClient(
"https://host.com",
metadata.ontologyRid,
async () => "",
undefined,
mockFetch,
);
});

const aggregationResponse: AggregateObjectsResponseV2 = {
Expand Down Expand Up @@ -171,13 +185,7 @@ describe("aggregate", () => {
>
>(false); // subSelect should hide unused keys

const grouped = await aggregate(
clientCtx,
objectTypeWithAllPropertyTypes,
{
type: "base",
objectType: "ToDo",
},
const grouped = await client(objectTypeWithAllPropertyTypes).aggregate(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason type tests were having issues when using the aggregate function directly, but were all good when using the user exposed aggregate function. This should be fine since the user facing stuff works, but just flagging

Copy link
Member

Choose a reason for hiding this comment

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

hmm, that seems off. what kind of issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, the aggregate function was not capturing clause types correctly and everything was still in generic form (e.g. like it wasn't getting the concrete information passed in $select: {myProp:asc}

{
$select: {
"id:approximateDistinct": "unordered",
Expand Down Expand Up @@ -509,13 +517,7 @@ describe("aggregate", () => {
});

it("prohibits ordered select with multiple groupBy", async () => {
aggregate(
clientCtx,
Todo,
{
type: "base",
objectType: "ToDo",
},
client(Todo).aggregate(
{
$select: {
// @ts-expect-error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export function createOsdkObject<
for (const propKey of Object.keys(rawObj)) {
if (
propKey in objectDef.properties
&& typeof (objectDef.properties[propKey].type) === "string"
&& specialPropertyTypes.has(objectDef.properties[propKey].type)
) {
rawObj[propKey] = createSpecialProperty(
Expand All @@ -107,7 +108,10 @@ function createSpecialProperty(
const rawValue = rawObject[p as any];
const propDef = objectDef.properties[p as any];
if (process.env.NODE_ENV !== "production") {
invariant(propDef != null && specialPropertyTypes.has(propDef.type));
invariant(
propDef != null && typeof propDef.type === "string"
&& specialPropertyTypes.has(propDef.type),
);
}
{
{
Expand Down
1 change: 1 addition & 0 deletions packages/client/src/object/object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ describe("OsdkObject", () => {
it("objects are enumerable in an sdk", async () => {
const objects = Object.keys($Objects);
expect(objects.sort()).toStrictEqual([
"BgaoNflPlayer",
"Employee",
"ObjectWithTimestampPrimaryKey",
"Office",
Expand Down
52 changes: 52 additions & 0 deletions packages/client/src/objectSet/ObjectSet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { isOk, WhereClause } from "@osdk/api";
import {
$ontologyRid,
BarInterface,
BgaoNflPlayer,
Employee,
FooInterface,
Office,
Expand Down Expand Up @@ -207,6 +208,57 @@ describe("ObjectSet", () => {
expect(employee.$primaryKey).toBe(stubData.employee1.employeeId);
});

it("check struct parsing", async () => {
const player = await client(BgaoNflPlayer).fetchOne(
"tkelce",
);
expectTypeOf<typeof player>().toMatchTypeOf<
Osdk<BgaoNflPlayer, PropertyKeys<BgaoNflPlayer>>
>;
expectTypeOf<typeof player.address>().toMatchTypeOf<
{
addressLine1: string | undefined;
addressLine2: string | undefined;
city: string | undefined;
state: string | undefined;
zipCode: number | undefined;
} | undefined
>;

const address1 = player.address!.addressLine1;
expectTypeOf<typeof address1>().toMatchTypeOf<
string | undefined
>;
expect(address1).toEqual("15 Muppets Lane");

const address2 = player.address?.addressLine2;
expectTypeOf<typeof address2>().toMatchTypeOf<
string | undefined
>;
expect(address2).toEqual("Resort No 4");

const city = player.address?.city;
expectTypeOf<typeof city>().toMatchTypeOf<
string | undefined
>;
expect(city).toEqual("Memphis");

const state = player.address?.state;
expectTypeOf<typeof state>().toMatchTypeOf<
string | undefined
>;
expect(state).toEqual("TN");

const zipCode = player.address?.zipCode;
expectTypeOf<typeof zipCode>().toMatchTypeOf<
number | undefined
>;
expect(zipCode).toEqual(11100);

expect(player.$primaryKey).toEqual(stubData.travisPlayer.__primaryKey);
expect(player.address).toEqual(stubData.travisPlayer.address);
});

it("allows fetching by PK from a base object set - fetchOneWithErrors", async () => {
const employeeResult = await client(Employee)
.fetchOneWithErrors(
Expand Down
Loading
Loading