Deprecate SnapshotConnection.openRemote and make SnapshotConnection.openFile IPC only
#7171
Conversation
|
Please add to your TODO list: Don't break display-test-app. See usage here. |
| @RpcOperation.setRoutingProps(unknownIModelId) | ||
| public async openFile(_filePath: string, _opts?: SnapshotOpenOptions): Promise<IModelConnectionProps> { return this.forward(arguments); } | ||
|
|
||
| /** | ||
| * @deprecated in 4.10. Use [[ConnectionConnection.openRemote]]. |
There was a problem hiding this comment.
Do you mean CheckpointConnection?
| @@ -487,7 +487,7 @@ export class IModelHost { | |||
| [ | |||
| IModelReadRpcImpl, | |||
| IModelTileRpcImpl, | |||
| SnapshotIModelRpcImpl, | |||
| SnapshotIModelRpcImpl, // eslint-disable-line deprecation/deprecation | |||
| WipRpcImpl, | |||
There was a problem hiding this comment.
Didnt realize we actually used this WipRpc...
not in this PR's context but we should review its usage and look at killing it off
There was a problem hiding this comment.
WipRpcInterface has no usage in core and shouldn't be used outside core as it's internal.
| @@ -224,6 +224,15 @@ class IpcAppHandler extends IpcHandler implements IpcAppFunctions { | |||
| public async openStandalone(filePath: string, openMode: OpenMode, opts?: StandaloneOpenOptions): Promise<IModelConnectionProps> { | |||
| return StandaloneDb.openFile(filePath, openMode, opts).getConnectionProps(); | |||
| } | |||
| public async openSnapshot(filePath: string, opts?: SnapshotOpenOptions | undefined): Promise<IModelConnectionProps> { | |||
| let resolvedFileName: string | undefined = filePath; | |||
| if (IModelHost.snapshotFileNameResolver) { | |||
There was a problem hiding this comment.
i dont think anyone is actually using this fileNameResolver, id like to try and not carry fwd junk with us
There was a problem hiding this comment.
IModelHost.snapshotFileNameResolver is public and, if we start ignoring it, it would be a breaking change.
We could deprecate it now and stop using it in 5.0.
Public API changes
SnapshotConnection.openRemoteAPI.SnapshotConnection.openFileonly available in IPC apps.Motivation
SnapshotConnectionrelies onSnapshotIModelRpcInterfacefor it's communication with the backend. With deprecation and eventual replacement of RPC, all RPC interfaces owned by core need to reviewed and rewritten as needed. While some of the other RPC interfaces (e.g.IModelReadRpcInterface) will need to be converted to the new architecture, I believe,SnapshotIModelRpcInterfacecan be moved to IPC as it doesn't make sense to have Web implementation for it.Details
SnapshotConnection/SnapshotIModelRpcInterfacehas two main methods:openFile(filePath: string, ...)- opening iModel using file path only makes sense if backend is on the same machine. This is the case in Electron and mobile in which IPC can be used. For that reason newIpcApp.appFunctionIpc.openSnapshotwas added.openRemote(key: string, ...)- as far as I can tell, this was intended to be used in Web Viewer. Web Viewer and other iTwin.js web applications should be usingCheckpointConnection.openRemote(...). Since GPB doesn't have implementation registered forSnapshotIModelRpcInterface, this is not even usable in the web without custom backend.TODO
SnapshotConnection.openFilein full-stack-tests/core as we will want to keep running those tests in Chrome.SnapshotConnection.openFilein full-stack-tests/presentation or support IPC there.