-
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
Interfaces for Subscribe #1018
Merged
Merged
Interfaces for Subscribe #1018
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@osdk/client": patch | ||
--- | ||
|
||
Allows interfaces to be used with subscribe |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,10 @@ import WebSocket from "isomorphic-ws"; | |
import invariant from "tiny-invariant"; | ||
import type { Logger } from "../Logger.js"; | ||
import type { ClientCacheKey, MinimalClient } from "../MinimalClientContext.js"; | ||
import { convertWireToOsdkObjects } from "../object/convertWireToOsdkObjects.js"; | ||
import { | ||
convertWireToOsdkObjects, | ||
convertWireToOsdkObjects2, | ||
} from "../object/convertWireToOsdkObjects.js"; | ||
|
||
const MINIMUM_RECONNECT_DELAY_MS = 5 * 1000; | ||
|
||
|
@@ -65,7 +68,8 @@ interface Subscription< | |
> { | ||
listener: Required<ObjectSetListener<Q, P>>; | ||
objectSet: ObjectSet; | ||
primaryKeyPropertyName: string; | ||
interfaceApiName?: string; | ||
primaryKeyPropertyName?: string; | ||
requestedProperties: Array<P>; | ||
requestedReferenceProperties: Array<P>; | ||
subscriptionId: string; | ||
|
@@ -173,31 +177,49 @@ export class ObjectSetListenerWebsocket { | |
globalThis.crypto ??= (await import("node:crypto")).webcrypto as any; | ||
} | ||
|
||
const objDef = await this.#client.ontologyProvider.getObjectDefinition( | ||
objectType.apiName, | ||
); | ||
const objDef = objectType.type === "object" | ||
? await this.#client.ontologyProvider.getObjectDefinition( | ||
objectType.apiName, | ||
) | ||
: await this.#client.ontologyProvider.getInterfaceDefinition( | ||
objectType.apiName, | ||
); | ||
|
||
if (properties.length === 0) { | ||
properties = Object.keys(objDef.properties) as Array<P>; | ||
} | ||
let objectProperties: Array<P> = []; | ||
let referenceProperties: Array<P> = []; | ||
|
||
const objectProperties = properties.filter((p) => | ||
objDef.properties[p].type !== "geotimeSeriesReference" | ||
); | ||
const referenceProperties = properties.filter((p) => | ||
objDef.properties[p].type === "geotimeSeriesReference" | ||
); | ||
if (objectType.type === "object") { | ||
if (properties.length === 0) { | ||
properties = Object.keys(objDef.properties) as Array<P>; | ||
} | ||
|
||
objectProperties = properties.filter((p) => | ||
objDef.properties[p].type !== "geotimeSeriesReference" | ||
); | ||
|
||
referenceProperties = properties.filter((p) => | ||
objDef.properties[p].type === "geotimeSeriesReference" | ||
); | ||
} else { | ||
objectProperties = []; | ||
referenceProperties = properties; | ||
} | ||
|
||
const sub: Subscription<Q, P> = { | ||
listener: fillOutListener<Q, P>(listener), | ||
objectSet, | ||
primaryKeyPropertyName: objDef.primaryKeyApiName, | ||
primaryKeyPropertyName: objDef.type === "interface" | ||
? undefined | ||
: objDef.primaryKeyApiName, | ||
requestedProperties: objectProperties, | ||
requestedReferenceProperties: referenceProperties, | ||
status: "preparing", | ||
// Since we don't have a real subscription id yet but we need to keep | ||
// track of this reference, we can just use a random uuid. | ||
subscriptionId: `TMP-${crypto.randomUUID()}`, | ||
interfaceApiName: objDef.type === "object" | ||
? undefined | ||
: objDef.apiName, | ||
}; | ||
|
||
this.#subscriptions.set(sub.subscriptionId, sub); | ||
|
@@ -273,7 +295,12 @@ export class ObjectSetListenerWebsocket { | |
const subscribe: ObjectSetStreamSubscribeRequests = { | ||
id, | ||
requests: readySubs.map<ObjectSetStreamSubscribeRequest>(( | ||
{ objectSet, requestedProperties, requestedReferenceProperties }, | ||
{ | ||
objectSet, | ||
requestedProperties, | ||
requestedReferenceProperties, | ||
interfaceApiName, | ||
}, | ||
) => { | ||
return { | ||
objectSet: objectSet, | ||
|
@@ -432,15 +459,17 @@ export class ObjectSetListenerWebsocket { | |
); | ||
const osdkObjectsWithReferenceUpdates = await Promise.all( | ||
referenceUpdates.map(async (o) => { | ||
const osdkObjectArray = await convertWireToOsdkObjects( | ||
const osdkObjectArray = await this.#client.objectFactory( | ||
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 using the old interface API's since the our new implementations require an interface to object type properties map. I could manually create this, but I think it would be best to talk through this @ssanjay1 and just unblock for right now |
||
this.#client, | ||
[{ | ||
__apiName: o.objectType, | ||
__primaryKey: o.primaryKey[sub.primaryKeyPropertyName], | ||
__primaryKey: sub.primaryKeyPropertyName != null | ||
? o.primaryKey[sub.primaryKeyPropertyName] | ||
: undefined, | ||
...o.primaryKey, | ||
[o.property]: o.value, | ||
}], | ||
undefined, | ||
sub.interfaceApiName, | ||
) as Array<Osdk.Instance<any, never, any>>; | ||
const singleOsdkObject = osdkObjectArray[0] ?? undefined; | ||
return singleOsdkObject != null | ||
|
@@ -465,11 +494,10 @@ export class ObjectSetListenerWebsocket { | |
for (const key of keysToDelete) { | ||
delete o.object[key]; | ||
} | ||
|
||
const osdkObjectArray = await this.#client.objectFactory( | ||
this.#client, | ||
[o.object], | ||
undefined, | ||
sub.interfaceApiName, | ||
) as Array<Osdk.Instance<any, never, any>>; | ||
const singleOsdkObject = osdkObjectArray[0] ?? undefined; | ||
return singleOsdkObject != null | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletions
1
packages/e2e.generated.catchall/src/generatedNoCheck/ontology/interfaces.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
export { FooInterface } from './interfaces/FooInterface.js'; | ||
export { InterfaceNoProps } from './interfaces/InterfaceNoProps.js'; | ||
export { OsdkTestInterface } from './interfaces/OsdkTestInterface.js'; |
63 changes: 63 additions & 0 deletions
63
...ages/e2e.generated.catchall/src/generatedNoCheck/ontology/interfaces/OsdkTestInterface.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
import type { PropertyDef as $PropertyDef } from '@osdk/client'; | ||
import { $osdkMetadata } from '../../OntologyMetadata.js'; | ||
|
||
import type { | ||
InterfaceDefinition as $InterfaceDefinition, | ||
ObjectSet as $ObjectSet, | ||
Osdk as $Osdk, | ||
PropertyValueWireToClient as $PropType, | ||
} from '@osdk/client'; | ||
|
||
export type OsdkObjectLinks$OsdkTestInterface = {}; | ||
|
||
export namespace OsdkTestInterface { | ||
export type PropertyKeys = 'objectDescription'; | ||
|
||
export interface Props { | ||
readonly objectDescription: $PropType['string'] | undefined; | ||
} | ||
export type StrictProps = Props; | ||
|
||
export interface ObjectSet extends $ObjectSet<OsdkTestInterface, OsdkTestInterface.ObjectSet> {} | ||
|
||
export type OsdkInstance< | ||
OPTIONS extends never | '$rid' = never, | ||
K extends keyof OsdkTestInterface.Props = keyof OsdkTestInterface.Props, | ||
> = $Osdk.Instance<OsdkTestInterface, OPTIONS, K>; | ||
|
||
/** @deprecated use OsdkInstance */ | ||
export type OsdkObject< | ||
OPTIONS extends never | '$rid' = never, | ||
K extends keyof OsdkTestInterface.Props = keyof OsdkTestInterface.Props, | ||
> = OsdkInstance<OPTIONS, K>; | ||
} | ||
|
||
export interface OsdkTestInterface extends $InterfaceDefinition { | ||
osdkMetadata: typeof $osdkMetadata; | ||
type: 'interface'; | ||
apiName: 'OsdkTestInterface'; | ||
__DefinitionMetadata?: { | ||
objectSet: OsdkTestInterface.ObjectSet; | ||
props: OsdkTestInterface.Props; | ||
linksType: OsdkObjectLinks$OsdkTestInterface; | ||
strictProps: OsdkTestInterface.StrictProps; | ||
apiName: 'OsdkTestInterface'; | ||
description: 'OsdkTestInterface'; | ||
displayName: 'OsdkTestInterface'; | ||
links: {}; | ||
properties: { | ||
/** | ||
* (no ontology metadata) | ||
*/ | ||
objectDescription: $PropertyDef<'string', 'nullable', 'single'>; | ||
}; | ||
rid: 'ri.ontology.main.interface.06c534fd-4f68-44d9-b268-72729a47eaab'; | ||
type: 'interface'; | ||
}; | ||
} | ||
|
||
export const OsdkTestInterface: OsdkTestInterface = { | ||
type: 'interface', | ||
apiName: 'OsdkTestInterface', | ||
osdkMetadata: $osdkMetadata, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
OSW assumes the names we provide them are the API names for the actual object type implementations instead of the SPT names. Therefore the current API I decided on was assuming if you pass an interface in that you are only specifying reference properties. This does require any casting what is passed in, and I think it's a larger conversation if the TS OSDK supports specifying anything beyond an SPT.
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.
There are ongoing discussions to allow OSW to take in the SPT names instead of the object type property API names here. I'll also open up the discussion as to how we want to allow requesting reference properties in the finalized API