Backmerge: #5949 - Delete of micromolecules bonds works wrong (or doe…

…sn't work) (#6124) * #5949 - Delete of micromolecules bonds works wrong (or doesn't work) - added invertAfterAllOperations method to atom and bonds operations to allow renderers rely on final state of model before rendering - added deleting of atoms and bonds from molecules struct to synchronize molecules and macromolecules modes - reworked bonds/atoms deletion logic - moved post execution logic from renderer to command
epam · Dec 11, 2024 · 8862787 · 8862787
1 parent dc4cfdc
commit 8862787
Show file tree

Hide file tree

Showing 10 changed files with 265 additions and 53 deletions.
diff --git a/packages/ketcher-core/src/application/editor/EditorHistory.ts b/packages/ketcher-core/src/application/editor/EditorHistory.ts
@@ -77,7 +77,6 @@ export class EditorHistory {
 
     const lastCommand = this.historyStack[this.historyPointer];
     lastCommand.execute(this.editor.renderersContainer);
-    lastCommand.executeAfterAllOperations(this.editor.renderersContainer);
     this.historyPointer++;
   }
 

diff --git a/packages/ketcher-core/src/application/editor/MacromoleculesConverter.ts b/packages/ketcher-core/src/application/editor/MacromoleculesConverter.ts
@@ -477,7 +477,7 @@ export class MacromoleculesConverter {
           );
         });
 
-        monomer.monomerItem.struct.bonds.forEach((bond) => {
+        monomer.monomerItem.struct.bonds.forEach((bond, bondId) => {
           const firstAtom = atomsMap.get(bond.begin);
           const secondAtom = atomsMap.get(bond.end);
 
@@ -491,6 +491,7 @@ export class MacromoleculesConverter {
               secondAtom,
               bond.type,
               bond.stereo,
+              bondId,
             ),
           );
         });

diff --git a/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts b/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts
@@ -19,43 +19,142 @@
 import { RenderersManager } from 'application/render/renderers/RenderersManager';
 import { Operation } from 'domain/entities/Operation';
 import { Atom } from 'domain/entities/CoreAtom';
+import {
+  Bond as MicromoleculesBond,
+  Atom as MicromoleculesAtom,
+} from 'domain/entities';
+import { KetcherLogger } from 'utilities';
 
+interface BondWithIdInMicromolecules {
+  bondId: number;
+  bond: MicromoleculesBond;
+}
+
+interface DeletedMoleculeStructItems {
+  atomInMoleculeStruct: MicromoleculesAtom;
+  bondsInMoleculeStruct: BondWithIdInMicromolecules[];
+}
+
+function addAtomToMoleculeStruct(
+  atom: Atom,
+  atomInMoleculeStruct: MicromoleculesAtom,
+  bondsInMoleculeStruct: BondWithIdInMicromolecules[] = [],
+) {
+  const moleculeStruct = atom.monomer.monomerItem.struct;
+
+  moleculeStruct.atoms.set(atom.atomIdInMicroMode, atomInMoleculeStruct);
+
+  bondsInMoleculeStruct.forEach(({ bondId, bond }) => {
+    moleculeStruct.bonds.set(bondId, bond);
+  });
+}
+
+function deleteAtomFromMoleculeStruct(atom: Atom) {
+  const moleculeStruct = atom.monomer.monomerItem.struct;
+  const atomInMoleculeStruct = moleculeStruct.atoms.get(atom.atomIdInMicroMode);
+
+  if (!atomInMoleculeStruct) {
+    KetcherLogger.warn('Atom is not found in molecule struct during deletion');
+
+    return;
+  }
+
+  const bondsInMoleculeStruct = moleculeStruct.bonds.filter((_, bond) => {
+    return (
+      bond.begin === atom.atomIdInMicroMode ||
+      bond.end === atom.atomIdInMicroMode
+    );
+  });
+
+  moleculeStruct.atoms.delete(atom.atomIdInMicroMode);
+
+  bondsInMoleculeStruct.forEach((_, bondId) => {
+    moleculeStruct.bonds.delete(bondId);
+  });
+
+  return {
+    atomInMoleculeStruct,
+    bondsInMoleculeStruct: [...bondsInMoleculeStruct.entries()].map(
+      ([bondId, bond]) => {
+        return { bondId, bond };
+      },
+    ),
+  };
+}
 export class AtomAddOperation implements Operation {
   public atom: Atom;
+  private deletedMoleculeStructItems?: DeletedMoleculeStructItems;
+  public priority = 2;
+
   constructor(
     public addAtomChangeModel: (atom?: Atom) => Atom,
-    public deleteAtomChangeModel: () => void,
+    public deleteAtomChangeModel: (atom: Atom) => void,
   ) {
     this.atom = this.addAtomChangeModel();
   }
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.atom = this.addAtomChangeModel(this.atom);
-    renderersManager.addAtom(this.atom);
+
+    if (this.deletedMoleculeStructItems) {
+      addAtomToMoleculeStruct(
+        this.atom,
+        this.deletedMoleculeStructItems.atomInMoleculeStruct,
+        this.deletedMoleculeStructItems.bondsInMoleculeStruct,
+      );
+    }
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     if (this.atom) {
-      this.deleteAtomChangeModel();
-      renderersManager.deleteAtom(this.atom);
+      this.deleteAtomChangeModel(this.atom);
     }
+
+    this.deletedMoleculeStructItems = deleteAtomFromMoleculeStruct(this.atom);
+  }
+
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.addAtom(this.atom);
+  }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteAtom(this.atom);
   }
 }
 
 export class AtomDeleteOperation implements Operation {
+  private deletedMoleculeStructItems?: DeletedMoleculeStructItems;
+  public priority = 2;
+
   constructor(
     public atom: Atom,
     public deleteAtomChangeModel: () => void,
     public addAtomChangeModel: (atom?: Atom) => Atom,
   ) {}
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.deleteAtomChangeModel();
-    renderersManager.deleteAtom(this.atom);
+
+    this.deletedMoleculeStructItems = deleteAtomFromMoleculeStruct(this.atom);
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     this.addAtomChangeModel(this.atom);
+
+    if (this.deletedMoleculeStructItems) {
+      addAtomToMoleculeStruct(
+        this.atom,
+        this.deletedMoleculeStructItems.atomInMoleculeStruct,
+        this.deletedMoleculeStructItems.bondsInMoleculeStruct,
+      );
+    }
+  }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
     renderersManager.addAtom(this.atom);
   }
+
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteAtom(this.atom);
+  }
 }
diff --git a/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts b/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts
@@ -19,43 +19,91 @@
 import { RenderersManager } from 'application/render/renderers/RenderersManager';
 import { Operation } from 'domain/entities/Operation';
 import { Bond } from 'domain/entities/CoreBond';
+import { Bond as MicromoleculesBond } from 'domain/entities/bond';
+
+function addBondToMoleculeStruct(
+  bond: Bond,
+  bondInMoleculeStruct: MicromoleculesBond,
+) {
+  const moleculeStruct = bond.firstAtom.monomer.monomerItem.struct;
+
+  moleculeStruct.bonds.set(bond.bondIdInMicroMode, bondInMoleculeStruct);
+}
+
+function deleteBondFromMoleculeStruct(bond: Bond) {
+  const moleculeStruct = bond.firstAtom.monomer.monomerItem.struct;
+  const bondInMoleculeStruct = moleculeStruct.bonds.get(bond.bondIdInMicroMode);
+
+  moleculeStruct.bonds.delete(bond.bondIdInMicroMode);
+
+  return bondInMoleculeStruct;
+}
 
 export class BondAddOperation implements Operation {
   public bond: Bond;
+  private bondInMoleculeStruct?: MicromoleculesBond;
+  public priority = 1;
   constructor(
     public addBondChangeModel: (bond?: Bond) => Bond,
-    public deleteBondChangeModel: (bond?: Bond) => void,
+    public deleteBondChangeModel: (bond: Bond) => void,
   ) {
     this.bond = this.addBondChangeModel();
   }
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.bond = this.addBondChangeModel(this.bond);
-    renderersManager.addBond(this.bond);
+
+    if (this.bondInMoleculeStruct) {
+      addBondToMoleculeStruct(this.bond, this.bondInMoleculeStruct);
+    }
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     if (this.bond) {
       this.deleteBondChangeModel(this.bond);
-      renderersManager.deleteBond(this.bond);
     }
+
+    this.bondInMoleculeStruct = deleteBondFromMoleculeStruct(this.bond);
+  }
+
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.addBond(this.bond);
+  }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteBond(this.bond);
   }
 }
 
 export class BondDeleteOperation implements Operation {
+  private bondInMoleculeStruct?: MicromoleculesBond;
+  public priority = 1;
+
   constructor(
     public bond: Bond,
-    public deleteBondChangeModel: (bond?: Bond) => void,
-    public addBondChangeModel: (bond?: Bond) => Bond,
+    public deleteBondChangeModel: (bond: Bond) => void,
+    public addBondChangeModel: (bond: Bond) => Bond,
   ) {}
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.deleteBondChangeModel(this.bond);
-    renderersManager.deleteBond(this.bond);
+
+    this.bondInMoleculeStruct = deleteBondFromMoleculeStruct(this.bond);
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     this.addBondChangeModel(this.bond);
+
+    if (this.bondInMoleculeStruct) {
+      addBondToMoleculeStruct(this.bond, this.bondInMoleculeStruct);
+    }
+  }
+
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteBond(this.bond);
+  }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
     renderersManager.addBond(this.bond);
   }
 }
diff --git a/packages/ketcher-core/src/application/editor/operations/drawingEntity/index.ts b/packages/ketcher-core/src/application/editor/operations/drawingEntity/index.ts
@@ -36,15 +36,8 @@ export class DrawingEntityMoveOperation implements Operation {
       : this.moveDrawingEntityChangeModel();
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     this.invertMoveDrawingEntityChangeModel();
-
-    if (this.drawingEntity instanceof BaseBond) {
-      renderersManager.redrawDrawingEntity(this.drawingEntity);
-    } else {
-      renderersManager.moveDrawingEntity(this.drawingEntity);
-    }
-
     this.wasInverted = true;
   }
 
@@ -59,6 +52,14 @@ export class DrawingEntityMoveOperation implements Operation {
       renderersManager.moveDrawingEntity(this.drawingEntity);
     }
   }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
+    if (this.drawingEntity instanceof BaseBond) {
+      renderersManager.redrawDrawingEntity(this.drawingEntity);
+    } else {
+      renderersManager.moveDrawingEntity(this.drawingEntity);
+    }
+  }
 }
 
 export class DrawingEntityRedrawOperation implements Operation {

diff --git a/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts b/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts
@@ -305,14 +305,16 @@ export class RenderersManager {
     monomer.renderer?.updateAttachmentPoints();
   }
 
-  public update(modelChanges?: Command) {
+  public reinitializeViewModel() {
     const editor = CoreEditor.provideEditorInstance();
     const viewModel = editor.viewModel;
 
-    modelChanges?.execute(this);
     viewModel.initialize([...editor.drawingEntitiesManager.bonds.values()]);
-    modelChanges?.executeAfterAllOperations(this);
+  }
 
+  public update(modelChanges?: Command) {
+    this.reinitializeViewModel();
+    modelChanges?.execute(this);
     this.runPostRenderMethods();
     notifyRenderComplete();
   }

diff --git a/packages/ketcher-core/src/domain/entities/Command.ts b/packages/ketcher-core/src/domain/entities/Command.ts
@@ -33,24 +33,42 @@ export class Command {
     }
 
     operations.forEach((operation) => operation.invert(renderersManagers));
+    renderersManagers.reinitializeViewModel();
+    this.invertAfterAllOperations(renderersManagers, operations);
     renderersManagers.runPostRenderMethods();
   }
 
   public execute(renderersManagers: RenderersManager) {
     this.operations.forEach((operation) =>
       operation.execute(renderersManagers),
     );
+    renderersManagers.reinitializeViewModel();
+    this.executeAfterAllOperations(renderersManagers);
     renderersManagers.runPostRenderMethods();
   }
 
-  public executeAfterAllOperations(renderersManagers: RenderersManager) {
-    this.operations.forEach((operation) => {
+  public executeAfterAllOperations(
+    renderersManagers: RenderersManager,
+    operations = this.operations,
+  ) {
+    operations.forEach((operation) => {
       if (operation.executeAfterAllOperations) {
         operation.executeAfterAllOperations(renderersManagers);
       }
     });
   }
 
+  public invertAfterAllOperations(
+    renderersManagers: RenderersManager,
+    operations = this.operations,
+  ) {
+    operations.forEach((operation) => {
+      if (operation.invertAfterAllOperations) {
+        operation.invertAfterAllOperations(renderersManagers);
+      }
+    });
+  }
+
   public clear() {
     this.operations = [];
   }

diff --git a/packages/ketcher-core/src/domain/entities/CoreBond.ts b/packages/ketcher-core/src/domain/entities/CoreBond.ts
@@ -11,6 +11,7 @@ export class Bond extends DrawingEntity {
   constructor(
     public firstAtom: Atom,
     public secondAtom: Atom,
+    public bondIdInMicroMode,
     public type = 1,
     public stereo = 0,
   ) {