Presentation: Fix presentation IPC initialization (iTwin#2398)

common/changes/@itwin/presentation-backend/presentation-fix-ipc-initialization_2021-10-01-05-48.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@itwin/presentation-backend",
+      "comment": "Fix Presentation IPC handler not being registered until the first Presentation request is made.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@itwin/presentation-backend"
+}

presentation/backend/src/presentation-backend/Presentation.ts

@@ -6,11 +6,12 @@
  * @module Core
  */
 
+import { IModelHost, IpcHost } from "@itwin/core-backend";
 import { DisposeFunc, Logger } from "@itwin/core-bentley";
-import { IModelHost } from "@itwin/core-backend";
 import { RpcManager } from "@itwin/core-common";
 import { PresentationError, PresentationRpcInterface, PresentationStatus } from "@itwin/presentation-common";
 import { PresentationBackendLoggerCategory } from "./BackendLoggerCategory";
+import { PresentationIpcHandler } from "./PresentationIpcHandler";
 import { PresentationManager, PresentationManagerProps } from "./PresentationManager";
 import { PresentationRpcImpl } from "./PresentationRpcImpl";
 import { TemporaryStorage } from "./TemporaryStorage";
@@ -87,6 +88,7 @@ export class Presentation {
   private static _initProps: PresentationProps | undefined;
   private static _clientsStorage: TemporaryStorage<ClientStoreItem> | undefined;
   private static _requestTimeout: number | undefined;
+  private static _disposeIpcHandler: DisposeFunc | undefined;
   private static _shutdownListener: DisposeFunc | undefined;
   private static _manager: PresentationManager | undefined;
 
@@ -106,12 +108,9 @@ export class Presentation {
    * @param props Optional properties for [[PresentationManager]]
    */
   public static initialize(props?: PresentationProps): void {
-    try {
-      RpcManager.registerImpl(PresentationRpcInterface, PresentationRpcImpl);
-    } catch (_e) {
-      // note: RpcManager.registerImpl throws when called more than once with the same
-      // rpc interface. However, it doesn't provide any way to unregister a, interface so we end up
-      // using the one registered first. At least we can avoid an exception...
+    RpcManager.registerImpl(PresentationRpcInterface, PresentationRpcImpl);
+    if (IpcHost.isValid) {
+      this._disposeIpcHandler = PresentationIpcHandler.register();
     }
     this._initProps = props || {};
     this._shutdownListener = IModelHost.onBeforeShutdown.addListener(() => Presentation.terminate());
@@ -154,6 +153,10 @@ export class Presentation {
       this._manager.dispose();
       this._manager = undefined;
     }
+    RpcManager.unregisterImpl(PresentationRpcInterface);
+    if (this._disposeIpcHandler) {
+      this._disposeIpcHandler();
+    }
     this._initProps = undefined;
     if (this._requestTimeout)
       this._requestTimeout = undefined;

presentation/backend/src/presentation-backend/PresentationManager.ts

@@ -8,8 +8,8 @@
 
 import * as hash from "object-hash";
 import * as path from "path";
-import { Id64String } from "@itwin/core-bentley";
 import { BriefcaseDb, IModelDb, IModelJsNative, IpcHost } from "@itwin/core-backend";
+import { Id64String } from "@itwin/core-bentley";
 import { FormatProps, UnitSystemKey } from "@itwin/core-quantity";
 import {
   Content, ContentDescriptorRequestOptions, ContentFlags, ContentRequestOptions, ContentSourcesRequestOptions, DefaultContentDisplayTypes, Descriptor,
@@ -25,7 +25,6 @@ import {
   createDefaultNativePlatform, NativePlatformDefinition, NativePlatformRequestTypes, NativePresentationDefaultUnitFormats,
   NativePresentationUnitSystem,
 } from "./NativePlatform";
-import { PresentationIpcHandler } from "./PresentationIpcHandler";
 import { RulesetManager, RulesetManagerImpl } from "./RulesetManager";
 import { RulesetVariablesManager, RulesetVariablesManagerImpl } from "./RulesetVariablesManager";
 import { SelectionScopesHelper } from "./SelectionScopesHelper";
@@ -298,10 +297,8 @@ export class PresentationManager {
   private _props: PresentationManagerProps;
   private _nativePlatform?: NativePlatformDefinition;
   private _rulesets: RulesetManager;
-  private _isOneFrontendPerBackend: boolean;
   private _isDisposed: boolean;
   private _disposeIModelOpenedListener?: () => void;
-  private _disposeIpcHandler?: () => void;
   private _updatesTracker?: UpdatesTracker;
 
   /** Get / set active locale used for localizing presentation data */
@@ -349,16 +346,11 @@ export class PresentationManager {
     if (this._props.enableSchemasPreload)
       this._disposeIModelOpenedListener = BriefcaseDb.onOpened.addListener(this.onIModelOpened);
 
-    this._isOneFrontendPerBackend = IpcHost.isValid;
-
-    if (IpcHost.isValid) {
-      if (isChangeTrackingEnabled) {
-        this._updatesTracker = UpdatesTracker.create({
-          nativePlatformGetter: this.getNativePlatform,
-          pollInterval: props.updatesPollInterval!,
-        });
-      }
-      this._disposeIpcHandler = PresentationIpcHandler.register();
+    if (IpcHost.isValid && isChangeTrackingEnabled) {
+      this._updatesTracker = UpdatesTracker.create({
+        nativePlatformGetter: this.getNativePlatform,
+        pollInterval: props.updatesPollInterval!,
+      });
     }
   }
 
@@ -379,9 +371,6 @@ export class PresentationManager {
       this._updatesTracker = undefined;
     }
 
-    if (this._disposeIpcHandler)
-      this._disposeIpcHandler();
-
     this._isDisposed = true;
   }
 

presentation/backend/src/test/Presentation.test.ts

@@ -6,10 +6,11 @@ import { expect } from "chai";
 import * as faker from "faker";
 import * as sinon from "sinon";
 import * as moq from "typemoq";
-import { IModelHost } from "@itwin/core-backend";
+import { IModelHost, IpcHost } from "@itwin/core-backend";
 import { RpcManager } from "@itwin/core-common";
 import { PresentationError } from "@itwin/presentation-common";
 import { MultiManagerPresentationProps, Presentation } from "../presentation-backend/Presentation";
+import { PresentationIpcHandler } from "../presentation-backend/PresentationIpcHandler";
 import { PresentationManager } from "../presentation-backend/PresentationManager";
 import { TemporaryStorage } from "../presentation-backend/TemporaryStorage";
 
@@ -110,6 +111,23 @@ describe("Presentation", () => {
       expect(() => Presentation.getRequestTimeout()).to.throw(PresentationError);
     });
 
+    it("unregisters PresentationRpcInterface impl", () => {
+      Presentation.initialize();
+      const unregisterSpy = sinon.stub(RpcManager, "unregisterImpl");
+      Presentation.terminate();
+      expect(unregisterSpy).to.be.calledOnce;
+    });
+
+    it("unregisters PresentationIpcHandler if IpcHost is valid", () => {
+      sinon.stub(IpcHost, "isValid").get(() => true);
+      const unregisterSpy = sinon.spy();
+      sinon.stub(PresentationIpcHandler, "register").returns(unregisterSpy);
+      Presentation.initialize();
+      expect(unregisterSpy).to.not.be.called;
+      Presentation.terminate();
+      expect(unregisterSpy).to.be.calledOnce;
+    });
+
   });
 
 });

presentation/backend/src/test/PresentationManager.test.ts

@@ -8,8 +8,8 @@ import * as faker from "faker";
 import * as path from "path";
 import * as sinon from "sinon";
 import * as moq from "typemoq";
-import { DbResult, Id64String, using } from "@itwin/core-bentley";
 import { BriefcaseDb, ECSqlStatement, ECSqlValue, IModelDb, IModelHost, IpcHost } from "@itwin/core-backend";
+import { DbResult, Id64String, using } from "@itwin/core-bentley";
 import {
   ArrayTypeDescription, CategoryDescription, Content, ContentDescriptorRequestOptions, ContentFlags, ContentJSON, ContentRequestOptions,
   ContentSourcesRequestOptions, DefaultContentDisplayTypes, Descriptor, DescriptorJSON, DescriptorOverrides, DiagnosticsOptions, DiagnosticsScopeLogs,
@@ -31,7 +31,6 @@ import {
 } from "@itwin/presentation-common/lib/test/_helpers/random";
 import { PRESENTATION_BACKEND_ASSETS_ROOT, PRESENTATION_COMMON_ASSETS_ROOT } from "../presentation-backend/Constants";
 import { NativePlatformDefinition, NativePlatformRequestTypes, NativePresentationUnitSystem } from "../presentation-backend/NativePlatform";
-import { PresentationIpcHandler } from "../presentation-backend/PresentationIpcHandler";
 import {
   HierarchyCacheMode, HybridCacheConfig, PresentationManager, PresentationManagerMode, PresentationManagerProps,
 } from "../presentation-backend/PresentationManager";
@@ -354,8 +353,6 @@ describe("PresentationManager", () => {
 
       it("creates an `UpdateTracker` when in read-write mode, `updatesPollInterval` is specified and IPC host is available", () => {
         sinon.stub(IpcHost, "isValid").get(() => true);
-        sinon.stub(PresentationIpcHandler, "register").returns(() => { });
-
         const tracker = sinon.createStubInstance(UpdatesTracker) as unknown as UpdatesTracker;
         const stub = sinon.stub(UpdatesTracker, "create").returns(tracker);
         using(new PresentationManager({ addon: addon.object, mode: PresentationManagerMode.ReadWrite, updatesPollInterval: 123 }), (_) => {
@@ -548,16 +545,6 @@ describe("PresentationManager", () => {
       expect(() => manager.getNativePlatform()).to.throw(PresentationError);
     });
 
-    it("unregisters PresentationIpcHandler if IpcHost is valid", () => {
-      sinon.stub(IpcHost, "isValid").get(() => true);
-      const nativePlatformMock = moq.Mock.ofType<NativePlatformDefinition>();
-      const unregisterSpy = sinon.spy();
-      sinon.stub(PresentationIpcHandler, "register").returns(unregisterSpy);
-      const manager = new PresentationManager({ addon: nativePlatformMock.object });
-      manager.dispose();
-      expect(unregisterSpy).to.be.calledOnce;
-    });
-
   });
 
   describe("getRulesetId", () => {