Improve shape metadata update logic (#385)

* Update shape metadata logic

* Add tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

packages/base/src/3dview/mainviewmodel.ts

@@ -2,7 +2,6 @@ import { IWorkerMessage } from '@jupytercad/occ-worker';
 import {
   IAnnotation,
   IDict,
-  IDisplayShape,
   IJcadObjectDocChange,
   IJCadWorker,
   IJCadWorkerRegistry,
@@ -106,8 +105,6 @@ export class MainViewModel implements IDisposable {
           }
         });
 
-        this._saveMeta(result);
-
         if (this._firstRender) {
           const postShapes = this._jcadModel.sharedModel
             .outputs as any as IDict<IPostResult>;
@@ -231,15 +228,6 @@ export class MainViewModel implements IDisposable {
     }
   }
 
-  private _saveMeta(payload: IDisplayShape['payload']['result']) {
-    if (!this._jcadModel) {
-      return;
-    }
-    Object.entries(payload).forEach(([objName, data]) => {
-      this._jcadModel.sharedModel.setShapeMeta(objName, data.meta);
-    });
-  }
-
   private async _onSharedObjectsChanged(
     _: IJupyterCadDoc,
     change: IJcadObjectDocChange

packages/base/src/commands.ts

@@ -1,5 +1,6 @@
 import {
   IDict,
+  IDryRunResponsePayload,
   IJCadContent,
   IJCadFormSchemaRegistry,
   IJCadObject,
@@ -38,6 +39,7 @@ import keybindings from './keybindings.json';
 import { JupyterCadPanel, JupyterCadWidget } from './widget';
 import { DocumentRegistry } from '@jupyterlab/docregistry';
 import { PathExt } from '@jupyterlab/coreutils';
+import { MainViewModel } from './3dview/mainviewmodel';
 import { handleRemoveObject } from './panelview';
 
 export function newName(type: string, model: IJupyterCadModel): string {
@@ -52,6 +54,23 @@ export function newName(type: string, model: IJupyterCadModel): string {
   return name;
 }
 
+export async function dryRunCheck(options: {
+  jcadContent: IJCadContent;
+  mainView: MainViewModel;
+  requestedOperator: string;
+}): Promise<IDryRunResponsePayload | null> {
+  const { jcadContent, mainView, requestedOperator } = options;
+  const dryRunResult = await mainView.dryRun(jcadContent);
+  if (dryRunResult.status === 'error') {
+    showErrorMessage(
+      `Failed to apply ${requestedOperator} operator`,
+      `The ${requestedOperator} tool was unable to create the desired shape due to invalid parameter values. The values you entered may not be compatible with the dimensions of your piece.`
+    );
+    return null;
+  }
+  return dryRunResult;
+}
+
 export function setVisible(
   sharedModel: IJupyterCadDoc,
   name: string,
@@ -208,18 +227,21 @@ export async function executeOperator(
     ...currentJcadContent,
     objects: [...currentJcadContent.objects, objectModel]
   };
-  const dryRunResult =
-    await current.content.currentViewModel.dryRun(updatedContent);
-  if (dryRunResult.status === 'error') {
-    showErrorMessage(
-      `Failed to create the ${name} operation`,
-      `The ${name} tool was unable to create the desired shape due to invalid parameter values. The values you entered may not be compatible with the dimensions of your piece.`
-    );
 
+  const dryRunResult = await dryRunCheck({
+    jcadContent: updatedContent,
+    mainView: current.content.currentViewModel,
+    requestedOperator: name
+  });
+  if (!dryRunResult) {
     return;
   }
-
   // Everything's good, we can apply the change to the shared model
+
+  const objMeta = dryRunResult.shapeMetadata?.[objectModel.name];
+  if (objMeta) {
+    objectModel.shapeMetadata = objMeta;
+  }
   sharedModel.transact(() => {
     transaction(sharedModel);
   });
@@ -1093,7 +1115,7 @@ namespace Private {
         title: value.title,
         sourceData: value.default(current.context.model),
         schema: FORM_SCHEMA[value.shape],
-        syncData: (props: IDict) => {
+        syncData: async (props: IDict) => {
           const { Name, ...parameters } = props;
           const objectModel: IJCadObject = {
             shape: value.shape as Parts,
@@ -1103,8 +1125,28 @@ namespace Private {
           };
 
           const sharedModel = current.context.model.sharedModel;
+
           if (sharedModel) {
             if (!sharedModel.objectExists(objectModel.name)) {
+              // Try a dry run with the update content to verify its feasibility
+              const currentJcadContent = current.context.model.getContent();
+              const updatedContent: IJCadContent = {
+                ...currentJcadContent,
+                objects: [...currentJcadContent.objects, objectModel]
+              };
+
+              const dryRunResult = await dryRunCheck({
+                jcadContent: updatedContent,
+                mainView: current.content.currentViewModel,
+                requestedOperator: value.shape
+              });
+              if (!dryRunResult) {
+                return;
+              }
+              const objMeta = dryRunResult.shapeMetadata?.[objectModel.name];
+              if (objMeta) {
+                objectModel.shapeMetadata = objMeta;
+              }
               sharedModel.addObject(objectModel);
             } else {
               showErrorMessage(

packages/base/src/panelview/objectproperties.tsx

@@ -143,11 +143,18 @@ class ObjectPropertiesReact extends React.Component<IProps, IStates> {
     }
 
     // Dry run was successful, ready to apply the update now
+    const meta: IDict = dryRunResult.shapeMetadata?.[objectName] ?? {};
     const obj = model.sharedModel.getObjectByName(objectName);
     if (obj) {
-      model.sharedModel.updateObjectByName(objectName, 'parameters', {
-        ...obj['parameters'],
-        ...properties
+      model.sharedModel.updateObjectByName(objectName, {
+        data: {
+          key: 'parameters',
+          value: {
+            ...obj['parameters'],
+            ...properties
+          }
+        },
+        meta
       });
     }
   }

packages/occ-worker/src/worker.ts

@@ -56,8 +56,9 @@ self.onmessage = async (event: MessageEvent): Promise<void> => {
       break;
     }
     case WorkerAction.DRY_RUN: {
+      let response: IDict = {};
       try {
-        WorkerHandler[WorkerAction.DRY_RUN](message.payload);
+        response = WorkerHandler[WorkerAction.DRY_RUN](message.payload);
       } catch (e) {
         let msg = '';
 
@@ -80,13 +81,17 @@ self.onmessage = async (event: MessageEvent): Promise<void> => {
         );
         return;
       }
-
+      const shapeMetadata: IDict = {};
+      Object.entries(response.result ?? {}).forEach(([objName, data]) => {
+        shapeMetadata[objName] = (data as any)?.['meta'];
+      });
       sendToMain(
         {
           action: MainAction.DRY_RUN_RESPONSE,
           payload: {
             id: message.payload.id,
-            status: 'ok'
+            status: 'ok',
+            shapeMetadata
           }
         },
         id

packages/schema/src/doc.ts

@@ -158,15 +158,18 @@ export class JupyterCadDoc
     });
   }
 
-  updateObjectByName(name: string, key: string, value: any): void {
+  updateObjectByName(
+    name: string,
+    payload: { data: { key: string; value: any }; meta?: IDict }
+  ): void {
     const obj = this._getObjectAsYMapByName(name);
     if (!obj) {
       return;
     }
+    const { key, value } = payload.data;
 
     this.transact(() => {
       // Special case for changing parameters, we may need to update dependencies
-      console.log('update ', key);
       if (key === 'parameters') {
         switch (obj.get('shape')) {
           case 'Part::Cut': {
@@ -190,6 +193,9 @@ export class JupyterCadDoc
       }
 
       obj.set(key, value);
+      if (payload.meta) {
+        obj.set('shapeMetadata', payload.meta);
+      }
     });
   }
 

packages/schema/src/interfaces.ts

@@ -95,7 +95,10 @@ export interface IJupyterCadDoc extends YDocument<IJupyterCadDocChange> {
   removeObjectByName(name: string): void;
   addObject(value: IJCadObject): void;
   addObjects(value: Array<IJCadObject>): void;
-  updateObjectByName(name: string, key: string, value: any): void;
+  updateObjectByName(
+    name: string,
+    payload: { data: { key: string; value: any }; meta?: IDict }
+  ): void;
   getDependants(name: string): string[];
 
   getOption(key: keyof IJCadOptions): IDict | undefined;
@@ -258,6 +261,7 @@ export interface IDryRunResponsePayload {
   id: string;
   status: 'ok' | 'error';
   message?: string;
+  shapeMetadata?: IDict;
 }
 
 export interface IDryRunResponse extends IMainMessageBase {

python/jupytercad_core/jupytercad_core/jcad_ydoc.py

@@ -31,6 +31,7 @@ def get(self) -> str:
         return json.dumps(
             dict(objects=objects, options=options, metadata=meta, outputs=outputs),
             indent=2,
+            sort_keys=True,
         )
 
     def set(self, value: str) -> None:

ui-tests/tests/ui.spec.ts

@@ -297,4 +297,64 @@ test.describe('UI Test', () => {
       }
     });
   });
+
+  test.describe('JCAD creation test', () => {
+    test.describe('Extension activation test', () => {
+      test('should create a new JCAD file', async ({ page, request }) => {
+        await page.goto();
+        await page
+          .getByLabel('notebook content')
+          .getByText('New JCAD File')
+          .click();
+
+        await page.getByTitle('New Box').getByRole('button').click();
+        await page.getByRole('button', { name: 'Submit' }).click();
+        await page.waitForTimeout(1000);
+        await page.getByText('Box 1').click();
+        await page.locator('#tab-key-1-6').click();
+
+        await page.waitForTimeout(1000);
+        const main = await page.locator('#jp-main-dock-panel');
+
+        if (main) {
+          expect(await main.screenshot()).toMatchSnapshot({
+            name: `JCAD-New.png`
+          });
+        }
+        await page.getByLabel('Length*').fill('0.5');
+        await page.getByRole('button', { name: 'Submit' }).click();
+        await page.waitForTimeout(500);
+        await page.getByLabel('Width*').fill('1.5');
+        await page.getByRole('button', { name: 'Submit' }).click();
+        await page.waitForTimeout(500);
+        await page.getByLabel('Height*').fill('2');
+        await page.getByRole('button', { name: 'Submit' }).click();
+        await page.waitForTimeout(500);
+        if (main) {
+          expect(await main.screenshot()).toMatchSnapshot({
+            name: `JCAD-Modified.png`
+          });
+        }
+
+        await page.getByTitle('Undo').getByRole('button').click();
+        await page.getByTitle('Undo').getByRole('button').click();
+        await page.getByTitle('Undo').getByRole('button').click();
+        await page.waitForTimeout(1000);
+        if (main) {
+          expect(await main.screenshot()).toMatchSnapshot({
+            name: `JCAD-Undo.png`
+          });
+        }
+        await page.getByTitle('Redo').getByRole('button').click();
+        await page.getByTitle('Redo').getByRole('button').click();
+        await page.getByTitle('Redo').getByRole('button').click();
+        await page.waitForTimeout(1000);
+        if (main) {
+          expect(await main.screenshot()).toMatchSnapshot({
+            name: `JCAD-Redo.png`
+          });
+        }
+      });
+    });
+  });
 });