fix: Use ObjectFetcher to retrieve objects (#234)

- In Enterprise environment, there may be multiple connections. We should not be accessing the connection directly from the plugin for fetching objects, instead use the ObjectFetcher registered by the app - Pass in the object metadata to fetch the object - Needs deephaven/web-client-ui#1753
deephaven · Feb 5, 2024 · 728be7b · 728be7b
1 parent 39cf7db
commit 728be7b
Show file tree

Hide file tree

Showing 10 changed files with 437 additions and 467 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/plugins/ui/src/js/package.json b/plugins/ui/src/js/package.json
@@ -41,19 +41,19 @@
   },
   "dependencies": {
     "@adobe/react-spectrum": "^3.29.0",
-    "@deephaven/chart": "^0.60.0",
-    "@deephaven/components": "^0.60.0",
-    "@deephaven/dashboard": "^0.60.0",
-    "@deephaven/dashboard-core-plugins": "^0.60.0",
-    "@deephaven/icons": "^0.60.0",
-    "@deephaven/iris-grid": "^0.60.0",
-    "@deephaven/jsapi-bootstrap": "^0.60.0",
-    "@deephaven/jsapi-components": "^0.60.0",
-    "@deephaven/jsapi-types": "^0.60.0",
-    "@deephaven/log": "^0.60.0",
-    "@deephaven/plugin": "^0.60.0",
-    "@deephaven/react-hooks": "^0.60.0",
-    "@deephaven/utils": "^0.60.0",
+    "@deephaven/chart": "0.62.0",
+    "@deephaven/components": "0.62.0",
+    "@deephaven/dashboard": "0.62.0",
+    "@deephaven/dashboard-core-plugins": "0.62.0",
+    "@deephaven/icons": "0.62.0",
+    "@deephaven/iris-grid": "0.62.0",
+    "@deephaven/jsapi-bootstrap": "0.62.0",
+    "@deephaven/jsapi-components": "0.62.0",
+    "@deephaven/jsapi-types": "0.62.0",
+    "@deephaven/log": "0.62.0",
+    "@deephaven/plugin": "0.62.0",
+    "@deephaven/react-hooks": "0.62.0",
+    "@deephaven/utils": "0.62.0",
     "@fortawesome/react-fontawesome": "^0.2.0",
     "@react-types/shared": "^3.22.0",
     "json-rpc-2.0": "^1.6.0",

diff --git a/plugins/ui/src/js/src/DashboardPlugin.tsx b/plugins/ui/src/js/src/DashboardPlugin.tsx
@@ -7,12 +7,15 @@ import {
   useListener,
   useDashboardPluginData,
   emitCreateDashboard,
+  WidgetDescriptor,
+  PanelOpenEventDetail,
 } from '@deephaven/dashboard';
 import Log from '@deephaven/log';
-import { useConnection } from '@deephaven/jsapi-components';
-import { DeferredApiBootstrap } from '@deephaven/jsapi-bootstrap';
+import {
+  DeferredApiBootstrap,
+  useObjectFetcher,
+} from '@deephaven/jsapi-bootstrap';
 import { Widget } from '@deephaven/jsapi-types';
-import type { VariableDefinition } from '@deephaven/jsapi-types';
 import { ErrorBoundary } from '@deephaven/components';
 import styles from './styles.scss?inline';
 import { WidgetWrapper } from './WidgetTypes';
@@ -31,20 +34,21 @@ interface DashboardPluginData {
   type: string;
   title: string;
   id: string;
-  metadata: Record<string, unknown>;
+  widget: WidgetDescriptor;
 }
 
 export function DashboardPlugin({
   id,
   layout,
   registerComponent,
 }: DashboardPluginComponentProps): JSX.Element | null {
-  const connection = useConnection();
   const [pluginData] = useDashboardPluginData(
     id,
     DASHBOARD_ELEMENT
   ) as unknown as [DashboardPluginData];
 
+  const objectFetcher = useObjectFetcher();
+
   // Keep track of the widgets we've got opened.
   const [widgetMap, setWidgetMap] = useState<
     ReadonlyMap<string, WidgetWrapper>
@@ -53,31 +57,20 @@ export function DashboardPlugin({
   const handleWidgetOpen = useCallback(
     ({
       fetch,
-      metadata,
-      panelId: widgetId = shortid.generate(),
+      widgetId = shortid.generate(),
       widget,
     }: {
       fetch: () => Promise<Widget>;
-      metadata: Record<string, unknown>;
-      panelId?: string;
-      widget: VariableDefinition;
+      widgetId: string;
+      widget: WidgetDescriptor;
     }) => {
-      log.info('Opening widget with ID', widgetId, metadata);
+      log.info('Opening widget with ID', widgetId, widget);
       setWidgetMap(prevWidgetMap => {
         const newWidgetMap = new Map<string, WidgetWrapper>(prevWidgetMap);
-        // We need to create a new definition object, otherwise the layout will think it's already open
-        // Can't use a spread operator because the widget definition uses property accessors
-        const definition = {
-          type: widget.type,
-          title: widget.title,
-          id: widget.id,
-          name: widget.name,
-        };
         newWidgetMap.set(widgetId, {
-          definition,
           fetch,
           id: widgetId,
-          metadata,
+          widget,
         });
         return newWidgetMap;
       });
@@ -86,14 +79,8 @@ export function DashboardPlugin({
   );
 
   const handleDashboardOpen = useCallback(
-    ({
-      widget,
-      metadata,
-    }: {
-      widget: VariableDefinition;
-      metadata: Record<string, unknown>;
-    }) => {
-      const { id: dashboardId, type, title = 'Untitled' } = widget;
+    ({ widget }: { widget: WidgetDescriptor }) => {
+      const { id: dashboardId, type, name: title = 'Untitled' } = widget;
       if (dashboardId == null) {
         log.error("Can't open dashboard without an ID", widget);
         return;
@@ -106,7 +93,7 @@ export function DashboardPlugin({
           type,
           title,
           id: dashboardId,
-          metadata,
+          widget,
         } satisfies DashboardPluginData,
       });
     },
@@ -118,30 +105,25 @@ export function DashboardPlugin({
       fetch,
       panelId: widgetId = shortid.generate(),
       widget,
-      metadata = {},
-    }: {
-      fetch: () => Promise<Widget>;
-      panelId?: string;
-      widget: VariableDefinition;
-      metadata: Record<string, unknown>;
-    }) => {
+    }: PanelOpenEventDetail<Widget>) => {
       const { type } = widget;
 
       switch (type) {
         case NAME_ELEMENT: {
-          handleWidgetOpen({ fetch, panelId: widgetId, widget, metadata });
+          const widgetFetch = fetch ?? (() => objectFetcher(widget));
+          handleWidgetOpen({ fetch: widgetFetch, widgetId, widget });
           break;
         }
         case DASHBOARD_ELEMENT: {
-          handleDashboardOpen({ widget, metadata });
+          handleDashboardOpen({ widget });
           break;
         }
         default: {
           log.error('Unknown widget type', type);
         }
       }
     },
-    [handleDashboardOpen, handleWidgetOpen]
+    [handleDashboardOpen, handleWidgetOpen, objectFetcher]
   );
 
   useEffect(
@@ -157,17 +139,16 @@ export function DashboardPlugin({
         // We need to create a new definition object, otherwise the layout will think it's already open
         // Can't use a spread operator because the widget definition uses property accessors
 
+        const { widget } = pluginData;
         newWidgetMap.set(id, {
-          definition: pluginData,
-          fetch: () =>
-            connection.getObject(pluginData) as unknown as Promise<Widget>,
+          fetch: () => objectFetcher(widget),
           id,
-          metadata: {},
+          widget,
         });
         return newWidgetMap;
       });
     },
-    [connection, pluginData, id]
+    [objectFetcher, pluginData, id]
   );
 
   const handlePanelClose = useCallback((panelId: string) => {
@@ -206,7 +187,7 @@ export function DashboardPlugin({
     () =>
       [...widgetMap.entries()].map(([widgetId, widget]) => (
         <ErrorBoundary key={widgetId}>
-          <DeferredApiBootstrap options={widget.metadata}>
+          <DeferredApiBootstrap widget={widget.widget}>
             <WidgetHandler widget={widget} onClose={handleWidgetClose} />
           </DeferredApiBootstrap>
         </ErrorBoundary>

diff --git a/plugins/ui/src/js/src/DocumentHandler.test.tsx b/plugins/ui/src/js/src/DocumentHandler.test.tsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import { WidgetDefinition } from '@deephaven/dashboard';
+import { WidgetDescriptor } from '@deephaven/dashboard';
 import { TestUtils } from '@deephaven/utils';
 import { render } from '@testing-library/react';
 import DocumentHandler, { DocumentHandlerProps } from './DocumentHandler';
@@ -40,11 +40,11 @@ function makeDocument(children: React.ReactNode = []): React.ReactNode {
 
 function makeDocumentHandler({
   children = makeDocument(),
-  definition = TestUtils.createMockProxy<WidgetDefinition>({}),
+  widget = TestUtils.createMockProxy<WidgetDescriptor>({}),
   onClose = jest.fn(),
 }: Partial<DocumentHandlerProps> = {}) {
   return (
-    <DocumentHandler definition={definition} onClose={onClose}>
+    <DocumentHandler widget={widget} onClose={onClose}>
       {children}
     </DocumentHandler>
   );

diff --git a/plugins/ui/src/js/src/DocumentHandler.tsx b/plugins/ui/src/js/src/DocumentHandler.tsx
@@ -1,5 +1,5 @@
 import React, { useCallback, useMemo, useRef } from 'react';
-import { WidgetDefinition } from '@deephaven/dashboard';
+import { WidgetDescriptor } from '@deephaven/dashboard';
 import Log from '@deephaven/log';
 import { ReactPanelManagerContext } from './ReactPanelManager';
 import { getRootChildren } from './DocumentUtils';
@@ -8,7 +8,7 @@ const log = Log.module('@deephaven/js-plugin-ui/DocumentHandler');
 
 export type DocumentHandlerProps = React.PropsWithChildren<{
   /** Definition of the widget used to create this document. Used for titling panels if necessary. */
-  definition: WidgetDefinition;
+  widget: WidgetDescriptor;
 
   /** Triggered when all panels opened from this document have closed */
   onClose?: () => void;
@@ -20,20 +20,16 @@ export type DocumentHandlerProps = React.PropsWithChildren<{
  * or all non-panels, which will automatically be wrapped in one panel.
  * Responsible for opening any panels or dashboards specified in the document.
  */
-function DocumentHandler({
-  children,
-  definition,
-  onClose,
-}: DocumentHandlerProps) {
-  log.debug('Rendering document', definition);
+function DocumentHandler({ children, widget, onClose }: DocumentHandlerProps) {
+  log.debug('Rendering document', widget);
   const panelOpenCountRef = useRef(0);
 
   const metadata = useMemo(
     () => ({
-      name: definition.title ?? definition.name ?? 'Unknown',
-      type: definition.type,
+      name: widget.name ?? 'Unknown',
+      type: widget.type,
     }),
-    [definition]
+    [widget]
   );
 
   const handleOpen = useCallback(() => {
@@ -63,7 +59,7 @@ function DocumentHandler({
 
   return (
     <ReactPanelManagerContext.Provider value={panelManager}>
-      {getRootChildren(children, definition)}
+      {getRootChildren(children, widget)}
     </ReactPanelManagerContext.Provider>
   );
 }

diff --git a/plugins/ui/src/js/src/DocumentUtils.tsx b/plugins/ui/src/js/src/DocumentUtils.tsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import { WidgetDefinition } from '@deephaven/dashboard';
+import { WidgetDescriptor } from '@deephaven/dashboard';
 import ReactPanel from './ReactPanel';
 import { MixedPanelsError, NoChildrenError } from './errors';
 import Dashboard from './layout/Dashboard';
@@ -11,12 +11,12 @@ import Dashboard from './layout/Dashboard';
  *
  *
  * @param children Root children of the document.
- * @param definition Definition of the widget used to create this document. Used for titling panels if necessary.
+ * @param widget Descriptor of the widget used to create this document. Used for titling panels if necessary.
  * @returns The children, wrapped in a panel if necessary.
  */
 export function getRootChildren(
   children: React.ReactNode,
-  definition: WidgetDefinition
+  widget: WidgetDescriptor
 ): React.ReactNode {
   if (children == null) {
     return null;
@@ -48,10 +48,7 @@ export function getRootChildren(
   if (nonLayoutCount === childrenArray.length) {
     // Just wrap it in a panel
     return (
-      <ReactPanel
-        key="root"
-        title={definition.title ?? definition.id ?? definition.type}
-      >
+      <ReactPanel key="root" title={widget.name ?? widget.id ?? widget.type}>
         {children}
       </ReactPanel>
     );

diff --git a/plugins/ui/src/js/src/WidgetHandler.test.tsx b/plugins/ui/src/js/src/WidgetHandler.test.tsx
@@ -6,7 +6,7 @@ import { DocumentHandlerProps } from './DocumentHandler';
 import {
   makeDocumentUpdatedJsonRpcString,
   makeWidget,
-  makeWidgetDefinition,
+  makeWidgetDescriptor,
   makeWidgetWrapper,
 } from './WidgetTestUtils';
 
@@ -45,30 +45,30 @@ it('updates the document when event is received', async () => {
     fetchResolve = resolve;
   });
   const fetch = jest.fn(() => fetchPromise);
-  const definition = makeWidgetDefinition();
+  const widget = makeWidgetDescriptor();
   const cleanup = jest.fn();
   const mockAddEventListener = jest.fn(() => cleanup);
   const initialDocument = { foo: 'bar' };
-  const widget = makeWidget({
+  const widgetObject = makeWidget({
     addEventListener: mockAddEventListener,
     getDataAsString: jest.fn(() =>
       makeDocumentUpdatedJsonRpcString(initialDocument)
     ),
   });
-  const wrapper = makeWidgetWrapper({ definition, fetch });
+  const wrapper = makeWidgetWrapper({ widget, fetch });
   const { unmount } = render(makeWidgetHandler({ widget: wrapper }));
   expect(fetch).toHaveBeenCalledTimes(1);
   expect(mockAddEventListener).not.toHaveBeenCalled();
   expect(mockDocumentHandler).not.toHaveBeenCalled();
   await act(async () => {
-    fetchResolve!(widget);
+    fetchResolve!(widgetObject);
     await fetchPromise;
   });
 
   expect(mockAddEventListener).toHaveBeenCalledTimes(1);
   expect(mockDocumentHandler).toHaveBeenCalledWith(
     expect.objectContaining({
-      definition,
+      widget,
       children: initialDocument,
     })
   );
@@ -90,7 +90,7 @@ it('updates the document when event is received', async () => {
   });
   expect(mockDocumentHandler).toHaveBeenCalledWith(
     expect.objectContaining({
-      definition,
+      widget,
       children: updatedDocument,
     })
   );

diff --git a/plugins/ui/src/js/src/WidgetHandler.tsx b/plugins/ui/src/js/src/WidgetHandler.tsx
@@ -213,7 +213,7 @@ function WidgetHandler({ onClose, widget: wrapper }: WidgetHandlerProps) {
 
   useEffect(
     function loadWidget() {
-      log.debug('loadWidget', wrapper.id, wrapper.definition);
+      log.debug('loadWidget', wrapper.id, wrapper.widget);
       let isCancelled = false;
       async function loadWidgetInternal() {
         const newWidget = await wrapper.fetch();
@@ -224,7 +224,7 @@ function WidgetHandler({ onClose, widget: wrapper }: WidgetHandlerProps) {
           });
           return;
         }
-        log.debug('newWidget', wrapper.id, wrapper.definition, newWidget);
+        log.debug('newWidget', wrapper.id, wrapper.widget, newWidget);
         setWidget(newWidget);
       }
       loadWidgetInternal();
@@ -243,10 +243,7 @@ function WidgetHandler({ onClose, widget: wrapper }: WidgetHandlerProps) {
   return useMemo(
     () =>
       document != null ? (
-        <DocumentHandler
-          definition={wrapper.definition}
-          onClose={handleDocumentClose}
-        >
+        <DocumentHandler widget={wrapper.widget} onClose={handleDocumentClose}>
           {document}
         </DocumentHandler>
       ) : null,