Merge pull request #758 from ICRAR/eagle-1335

EAGLE-1335: New steps in checkGraph to check correctness of the graphConfigs

src/GraphConfig.ts

@@ -2,7 +2,6 @@ import * as ko from "knockout";
 
 import { Branded } from "./main";
 import { Eagle } from "./Eagle";
-import { EagleConfig } from "./EagleConfig";
 import { Errors } from "./Errors";
 import { Field } from "./Field";
 import { LogicalGraph } from "./LogicalGraph";
@@ -116,11 +115,29 @@ export class GraphConfig {
         return null;
     }
 
+    removeNode = (node: GraphConfigNode): void => {
+        for (let i = this.nodes().length - 1; i >= 0 ; i--){
+            if (this.nodes()[i].getId() === node.getId()){
+                this.nodes.splice(i, 1);
+                break;
+            }
+        }
+    }
+
     removeField = (field: Field): void => {
-        const node = Eagle.getInstance().logicalGraph().findNodeById(field.getNodeId());
-        this.findNodeById(node.getId()).removeFieldById(field.getId());
+        // get reference to the GraphConfigNode containing the field
+        const graphConfigNode: GraphConfigNode = this.findNodeById(field.getNodeId());
+
+        // remove the field
+        graphConfigNode.removeFieldById(field.getId());
+
+        // we check if removing the GraphConfigField means that the GraphConfigNode now has zero fields
+        if (graphConfigNode.getFields().length === 0){
+            this.removeNode(graphConfigNode);
+        }
 
-        // TODO: do we need to check if removing the field means that the node now has zero fields?
+        // re-check graph
+        Eagle.getInstance().checkGraph();
     }
 
     addValue = (nodeId: NodeId, fieldId: FieldId, value: string) => {
@@ -365,7 +382,7 @@ export class GraphConfigField {
         return this;
     }
 
-    getId = (): string => {
+    getId = (): FieldId => {
         return this.id();
     }
 

src/LogicalGraph.ts

@@ -906,13 +906,14 @@ export class LogicalGraph {
         const eagle = Eagle.getInstance()
         const graph = eagle.logicalGraph()
 
-        // check that all node, edge, field ids are unique
-        // {
+        // clear old issues
+        graph.issues([]);
+
+        // check that all node, edge, field, and config ids are unique
         const ids : string[] = [];
 
         // loop over graph nodes
         for (const node of graph.getNodes()){
-            //check for unique ids
             if (ids.includes(node.getId())){
                 const issue: Errors.Issue = Errors.ShowFix(
                     "Node (" + node.getName() + ") does not have a unique id",
@@ -921,10 +922,10 @@ export class LogicalGraph {
                     "Assign node a new id"
                 );
                 graph.issues.push({issue : issue, validity : Errors.Validity.Error})
-                // errorsWarnings.errors.push(issue);
             }
             ids.push(node.getId());
 
+            // loop over fields within graphs
             for (const field of node.getFields()){
                 if (ids.includes(field.getId())){
                     const issue: Errors.Issue = Errors.ShowFix(
@@ -934,7 +935,6 @@ export class LogicalGraph {
                         "Assign field a new id"
                     );
                     graph.issues.push({issue : issue, validity : Errors.Validity.Error})
-                    // errorsWarnings.errors.push(issue);
                 }
                 ids.push(field.getId());
             }
@@ -950,10 +950,78 @@ export class LogicalGraph {
                     "Assign edge a new id"
                 );
                 graph.issues.push({issue : issue, validity : Errors.Validity.Error})
-                // errorsWarnings.errors.push(issue);
             }
             ids.push(edge.getId());
         }
-        // }
+
+        // loop over the graph configs
+        for (const graphConfig of graph.getGraphConfigs()){
+            if (ids.includes(graphConfig.getId())){
+                const issue: Errors.Issue = Errors.ShowFix(
+                    "Graph Config (" + graphConfig.getId() + ") does not have a unique id",
+                    function(){Utils.showGraphConfig(eagle, graphConfig.getId())},
+                    function(){graphConfig.setId(Utils.generateGraphConfigId())},
+                    "Assign graph config a new id"
+                );
+                graph.issues.push({issue : issue, validity : Errors.Validity.Error})
+            }
+
+            ids.push(graphConfig.getId());
+        }
+
+        // check that active graph config id actually refers to a graph config in the graphConfigs dict
+        if (graph.activeGraphConfigId() !== undefined && graph.activeGraphConfigId() !== ""){
+            if (graph.getActiveGraphConfig() === null){
+                const issue: Errors.Issue = Errors.Fix(
+                    "Active Graph Config Id (" + graph.activeGraphConfigId() + ") does not match a known graph config",
+                    function(){
+                        // if there are no graph config, set active id to undefined
+                        // otherwise, just set the active id to the id of the first graph config in the list
+                        if (graph.getGraphConfigs().length === 0){
+                            graph.setActiveGraphConfig(undefined);
+                        } else {
+                            graph.setActiveGraphConfig(graph.getGraphConfigs()[0].getId());
+                        }
+                    },
+                    "Make first graph config active, or set undefined if no graph configs present"
+                );
+                graph.issues.push({issue : issue, validity : Errors.Validity.Error})
+            }
+        }
+
+        // check that all fields, in all nodes, in all graph configs are actually present in the graph
+        for (const graphConfig of graph.getGraphConfigs()){
+            for (const graphConfigNode of graphConfig.getNodes()){
+                // check that node exists in graph
+                const graphNode: Node = graph.findNodeByIdQuiet(graphConfigNode.getId());
+
+                if (graphNode === null){
+                    const issue: Errors.Issue = Errors.Fix(
+                        "Node (" + graphConfigNode.getId() +") in graph config (" + graphConfig.getName() + ") is not present in Logical Graph",
+                        function(){
+                            graphConfig.removeNode(graphConfigNode);
+                        },
+                        "Delete node from graph config"
+                    );
+                    graph.issues.push({issue : issue, validity : Errors.Validity.Error});
+                    break;
+                }
+
+                for (const graphConfigField of graphConfigNode.getFields()){
+                    const graphField: Field = graphNode.findFieldById(graphConfigField.getId());
+
+                    if (graphField === null){
+                        const issue: Errors.Issue = Errors.Fix(
+                            "Field (" + graphConfigField.getId() + ") in graph config (" + graphConfig.getName() + ", " + graphNode.getName() + ") is not present in Logical Graph",
+                            function(){
+                                graphConfigNode.removeFieldById(graphConfigField.getId());
+                            },
+                            "Delete field from node in graph config"
+                        );
+                        graph.issues.push({issue: issue, validity: Errors.Validity.Error});
+                    }
+                }
+            }
+        }
     }
 }

src/ParameterTable.ts

@@ -1,5 +1,6 @@
 import * as ko from "knockout";
 
+import { Daliuge } from "./Daliuge";
 import { Eagle } from './Eagle';
 import { Edge } from "./Edge";
 import { Field } from './Field';
@@ -170,18 +171,22 @@ export class ParameterTable {
                 }
 
                 for (const node of config.getNodes()){
-                    const lgNode = lg.findNodeById(node.getId());
-
-                    if (lgNode === null){
-                        console.warn("ParameterTable.getTableFields(): Could not find node", node.getId());
-                        continue;
-                    }
-
                     for (const field of node.getFields()){
+                        const lgNode = lg.findNodeByIdQuiet(node.getId());
+
+                        if (lgNode === null){
+                            const dummyField: Field = new Field(field.getId(), "<Missing Node:" + node.getId() +">", field.getValue(), "?", field.getComment(), true, Daliuge.DataType.Unknown, false, [], false, Daliuge.FieldType.Unknown, Daliuge.FieldUsage.NoPort);
+                            dummyField.setNodeId(node.getId());
+                            displayedFields.push(dummyField);
+                            continue;
+                        }
+
                         const lgField = lgNode.findFieldById(field.getId());
 
                         if (lgField === null){
-                            console.warn("ParameterTable.getTableFields(): Could not find field", field.getId(), "on node", lgNode.getName());
+                            const dummyField: Field = new Field(field.getId(), "<Missing Field: " + field.getId() + ">", field.getValue(), "?", field.getComment(), true, Daliuge.DataType.Unknown, false, [], false, Daliuge.FieldType.Unknown, Daliuge.FieldUsage.NoPort);
+                            dummyField.setNodeId(node.getId());
+                            displayedFields.push(dummyField);
                             continue;
                         }
 
@@ -199,6 +204,12 @@ export class ParameterTable {
     // TODO: move to Eagle.ts?
     //       doesn't seem to depend on any ParameterTable state, only Eagle state
     static getNodeLockedState = (field:Field) : boolean => {
+        // this handles a special case where EAGLE is displaying the "Graph Configuration Attributes Table"
+        // all the field names shown in that table should be locked (readonly)
+        if (Setting.find(Setting.BOTTOM_WINDOW_MODE).value() === Eagle.BottomWindowMode.GraphConfigAttributesTable){
+            return true;
+        }
+
         const eagle: Eagle = Eagle.getInstance();
         if(Eagle.selectedLocation() === Eagle.FileType.Palette){
             if(eagle.selectedNode() === null){

src/Utils.ts

@@ -45,6 +45,7 @@ import { Repository } from './Repository';
 import { Setting } from './Setting';
 import { UiModeSystem } from "./UiModes";
 import { ParameterTable } from "./ParameterTable";
+import { GraphConfigurationsTable } from "./GraphConfigurationsTable";
 
 export class Utils {
     // Allowed file extensions
@@ -1446,6 +1447,8 @@ export class Utils {
     static checkGraph(eagle: Eagle): void {
         const graph: LogicalGraph = eagle.logicalGraph();
 
+        LogicalGraph.isValid();
+
         // check all nodes are valid
         for (const node of graph.getNodes()){
             Node.isValid(node, Eagle.FileType.Graph);
@@ -2160,6 +2163,18 @@ export class Utils {
         },100)
     }
 
+    static showGraphConfig(eagle: Eagle, graphConfigId: GraphConfig.Id){
+        // open the graph configs table
+        GraphConfigurationsTable.openModal();
+
+        const graphConfig: GraphConfig = eagle.logicalGraph().getGraphConfigById(graphConfigId);
+
+        // highlight the name of the graph config
+        setTimeout(() => {
+            $('#tableRow_' + graphConfig.getName()).focus().select()
+        }, 100);
+    }
+
     // only update result if it is worse that current result
     static worstEdgeError(errorsWarnings: Errors.ErrorsWarnings) : Errors.Validity {
         if (errorsWarnings === null){

src/bindingHandlers/eagleTooltip.ts

@@ -47,6 +47,11 @@ ko.bindingHandlers.eagleTooltip = {
             let result = ''
             let size = '300px'
 
+            // abort if the input html is undefined
+            if (typeof html === 'undefined'){
+                return;
+            }
+
             //html can be either a string or a Object with a content string and size (in pixels)
             if(html.content != undefined){
                 size = html.size

templates/parameter_table.html

@@ -178,7 +178,7 @@ <h5 class="bottomWindowHeader">Fields Table: </h5>
                                             <!-- /ko -->
                                             <!-- ko if: Setting.findValue(Setting.BOTTOM_WINDOW_MODE) === Eagle.BottomWindowMode.GraphConfigAttributesTable -->
                                                 <td class='columnCell column_NodeName'>
-                                                    <input class="tableParameter" type="string" data-bind="value: $root.logicalGraph().findNodeById(nodeId()).getName(), disabled: true, eagleTooltip: $root.logicalGraph().findNodeById(nodeId()).getDescription()">
+                                                    <input class="tableParameter" type="string" data-bind="value: $root.logicalGraph().findNodeByIdQuiet(nodeId())?.getName(), disabled: true, eagleTooltip: $root.logicalGraph().findNodeByIdQuiet(nodeId())?.getDescription()">
                                                 </td>
                                             <!-- /ko -->
                                             <!-- ko if: ParameterTable.getActiveColumnVisibility().displayText() -->