Read-only mode for controllers (#6259)

Closes #6181

- Added a read-only flag for the project model (now controlled by a temporary shortcut).
- Renaming the project, editing the code, connecting and disconnecting nodes, and collapsing, and navigating through collapsed nodes are forbidden and will result in the error message in the console.
- Some of the above actions produce undesired effects on the IDE, which will be fixed later. This PR is focused on restricting actual AST modifications.
- Moving nodes (and updating metadata in general) no longer causes reevaluation


https://user-images.githubusercontent.com/6566674/231616408-4f334bb7-1985-43ba-9953-4c0998338a9b.mp4

app/gui/controller/engine-protocol/src/language_server.rs

@@ -113,7 +113,7 @@ trait API {
     /// have permission to edit the resources for which edits are sent. This failure may be partial,
     /// in that some edits are applied and others are not.
     #[MethodInput=ApplyTextFileEditInput, rpc_name="text/applyEdit"]
-    fn apply_text_file_edit(&self, edit: FileEdit) -> ();
+    fn apply_text_file_edit(&self, edit: FileEdit, execute: bool) -> ();
 
     /// Create a new execution context. Return capabilities executionContext/canModify and
     /// executionContext/receivesUpdates containing freshly created ContextId

app/gui/controller/engine-protocol/src/language_server/tests.rs

@@ -547,7 +547,7 @@ fn test_execution_context() {
     let path = main.clone();
     let edit = FileEdit { path, edits, old_version, new_version };
     test_request(
-        |client| client.apply_text_file_edit(&edit),
+        |client| client.apply_text_file_edit(&edit, &true),
         "text/applyEdit",
         json!({
             "edit" : {
@@ -572,7 +572,8 @@ fn test_execution_context() {
                 ],
                 "oldVersion" : "d3ee9b1ba1990fecfd794d2f30e0207aaa7be5d37d463073096d86f8",
                 "newVersion" : "6a33e22f20f16642697e8bd549ff7b759252ad56c05a1b0acc31dc69"
-            }
+            },
+            "execute": true
         }),
         unit_json.clone(),
         (),

app/gui/src/controller/graph/executed.rs

@@ -44,6 +44,10 @@ pub struct NotEvaluatedYet(double_representation::node::Id);
 #[fail(display = "The node {} does not resolve to a method call.", _0)]
 pub struct NoResolvedMethod(double_representation::node::Id);
 
+#[allow(missing_docs)]
+#[derive(Debug, Fail, Clone, Copy)]
+#[fail(display = "Operation is not permitted in read only mode")]
+pub struct ReadOnly;
 
 
 // ====================
@@ -216,19 +220,25 @@ impl Handle {
     /// This will cause pushing a new stack frame to the execution context and changing the graph
     /// controller to point to a new definition.
     ///
-    /// Fails if method graph cannot be created (see `graph_for_method` documentation).
+    /// ### Errors
+    /// - Fails if method graph cannot be created (see `graph_for_method` documentation).
+    /// - Fails if the project is in read-only mode.
     pub async fn enter_method_pointer(&self, local_call: &LocalCall) -> FallibleResult {
-        debug!("Entering node {}.", local_call.call);
-        let method_ptr = &local_call.definition;
-        let graph = controller::Graph::new_method(&self.project, method_ptr);
-        let graph = graph.await?;
-        self.execution_ctx.push(local_call.clone()).await?;
-        debug!("Replacing graph with {graph:?}.");
-        self.graph.replace(graph);
-        debug!("Sending graph invalidation signal.");
-        self.notifier.publish(Notification::EnteredNode(local_call.clone())).await;
-
-        Ok(())
+        if self.project.read_only() {
+            Err(ReadOnly.into())
+        } else {
+            debug!("Entering node {}.", local_call.call);
+            let method_ptr = &local_call.definition;
+            let graph = controller::Graph::new_method(&self.project, method_ptr);
+            let graph = graph.await?;
+            self.execution_ctx.push(local_call.clone()).await?;
+            debug!("Replacing graph with {graph:?}.");
+            self.graph.replace(graph);
+            debug!("Sending graph invalidation signal.");
+            self.notifier.publish(Notification::EnteredNode(local_call.clone())).await;
+
+            Ok(())
+        }
     }
 
     /// Attempts to get the computed value of the specified node.
@@ -246,15 +256,21 @@ impl Handle {
 
     /// Leave the current node. Reverse of `enter_node`.
     ///
-    /// Fails if this execution context is already at the stack's root or if the parent graph
+    /// ### Errors
+    /// - Fails if this execution context is already at the stack's root or if the parent graph
     /// cannot be retrieved.
+    /// - Fails if the project is in read-only mode.
     pub async fn exit_node(&self) -> FallibleResult {
-        let frame = self.execution_ctx.pop().await?;
-        let method = self.execution_ctx.current_method();
-        let graph = controller::Graph::new_method(&self.project, &method).await?;
-        self.graph.replace(graph);
-        self.notifier.publish(Notification::SteppedOutOfNode(frame.call)).await;
-        Ok(())
+        if self.project.read_only() {
+            Err(ReadOnly.into())
+        } else {
+            let frame = self.execution_ctx.pop().await?;
+            let method = self.execution_ctx.current_method();
+            let graph = controller::Graph::new_method(&self.project, &method).await?;
+            self.graph.replace(graph);
+            self.notifier.publish(Notification::SteppedOutOfNode(frame.call)).await;
+            Ok(())
+        }
     }
 
     /// Interrupt the program execution.
@@ -264,9 +280,16 @@ impl Handle {
     }
 
     /// Restart the program execution.
+    ///
+    /// ### Errors
+    /// - Fails if the project is in read-only mode.
     pub async fn restart(&self) -> FallibleResult {
-        self.execution_ctx.restart().await?;
-        Ok(())
+        if self.project.read_only() {
+            Err(ReadOnly.into())
+        } else {
+            self.execution_ctx.restart().await?;
+            Ok(())
+        }
     }
 
     /// Get the current call stack frames.
@@ -308,15 +331,29 @@ impl Handle {
     }
 
     /// Create connection in graph.
+    ///
+    /// ### Errors
+    /// - Fails if the project is in read-only mode.
     pub fn connect(&self, connection: &Connection) -> FallibleResult {
-        self.graph.borrow().connect(connection, self)
+        if self.project.read_only() {
+            Err(ReadOnly.into())
+        } else {
+            self.graph.borrow().connect(connection, self)
+        }
     }
 
     /// Remove the connections from the graph. Returns an updated edge destination endpoint for
     /// disconnected edge, in case it is still used as destination-only edge. When `None` is
     /// returned, no update is necessary.
+    ///
+    /// ### Errors
+    /// - Fails if the project is in read-only mode.
     pub fn disconnect(&self, connection: &Connection) -> FallibleResult<Option<span_tree::Crumbs>> {
-        self.graph.borrow().disconnect(connection, self)
+        if self.project.read_only() {
+            Err(ReadOnly.into())
+        } else {
+            self.graph.borrow().disconnect(connection, self)
+        }
     }
 }
 
@@ -368,6 +405,8 @@ pub mod tests {
     use crate::model::execution_context::ExpressionId;
     use crate::test;
 
+    use crate::test::mock::Fixture;
+    use controller::graph::SpanTree;
     use engine_protocol::language_server::types::test::value_update_with_type;
     use wasm_bindgen_test::wasm_bindgen_test;
     use wasm_bindgen_test::wasm_bindgen_test_configure;
@@ -454,4 +493,95 @@ pub mod tests {
 
         notifications.expect_pending();
     }
+
+    // Test that moving nodes is possible in read-only mode.
+    #[wasm_bindgen_test]
+    fn read_only_mode_does_not_restrict_moving_nodes() {
+        use model::module::Position;
+
+        let fixture = crate::test::mock::Unified::new().fixture();
+        let Fixture { executed_graph, graph, .. } = fixture;
+
+        let nodes = executed_graph.graph().nodes().unwrap();
+        let node = &nodes[0];
+
+        let pos1 = Position::new(500.0, 250.0);
+        let pos2 = Position::new(300.0, 150.0);
+
+        graph.set_node_position(node.id(), pos1).unwrap();
+        assert_eq!(graph.node(node.id()).unwrap().position(), Some(pos1));
+        graph.set_node_position(node.id(), pos2).unwrap();
+        assert_eq!(graph.node(node.id()).unwrap().position(), Some(pos2));
+    }
+
+    // Test that certain actions are forbidden in read-only mode.
+    #[wasm_bindgen_test]
+    fn read_only_mode() {
+        fn run(code: &str, f: impl FnOnce(&Handle)) {
+            let mut data = crate::test::mock::Unified::new();
+            data.set_code(code);
+            let fixture = data.fixture();
+            fixture.read_only.set(true);
+            let Fixture { executed_graph, .. } = fixture;
+            f(&executed_graph);
+        }
+
+
+        // === Editing the node. ===
+
+        let default_code = r#"
+main =
+    foo = 2 * 2
+"#;
+        run(default_code, |executed| {
+            let nodes = executed.graph().nodes().unwrap();
+            let node = &nodes[0];
+            assert!(executed.graph().set_expression(node.info.id(), "5 * 20").is_err());
+        });
+
+
+        // === Collapsing nodes. ===
+
+        let code = r#"
+main =
+    foo = 2
+    bar = foo + 6
+    baz = 2 + foo + bar
+    caz = baz / 2 * baz
+"#;
+        run(code, |executed| {
+            let nodes = executed.graph().nodes().unwrap();
+            // Collapse two middle nodes.
+            let nodes_range = vec![nodes[1].id(), nodes[2].id()];
+            assert!(executed.graph().collapse(nodes_range, "extracted").is_err());
+        });
+
+
+        // === Connecting nodes. ===
+
+        let code = r#"
+main =
+    2 + 2
+    5 * 5
+"#;
+        run(code, |executed| {
+            let nodes = executed.graph().nodes().unwrap();
+            let sum_node = &nodes[0];
+            let product_node = &nodes[1];
+
+            assert_eq!(sum_node.expression().to_string(), "2 + 2");
+            assert_eq!(product_node.expression().to_string(), "5 * 5");
+
+            let context = &span_tree::generate::context::Empty;
+            let sum_tree = SpanTree::<()>::new(&sum_node.expression(), context).unwrap();
+            let sum_input =
+                sum_tree.root_ref().leaf_iter().find(|n| n.is_argument()).unwrap().crumbs;
+            let connection = Connection {
+                source:      controller::graph::Endpoint::new(product_node.id(), []),
+                destination: controller::graph::Endpoint::new(sum_node.id(), sum_input),
+            };
+
+            assert!(executed.connect(&connection).is_err());
+        });
+    }
 }

app/gui/src/controller/module.rs

@@ -173,7 +173,7 @@ impl Handle {
     ) -> FallibleResult<Self> {
         let ast = parser.parse(code.to_string(), id_map).try_into()?;
         let metadata = default();
-        let model = Rc::new(model::module::Plain::new(path, ast, metadata, repository));
+        let model = Rc::new(model::module::Plain::new(path, ast, metadata, repository, default()));
         Ok(Handle { model, language_server, parser })
     }
 

app/gui/src/controller/searcher.rs

@@ -97,6 +97,7 @@ pub struct CannotCommitExpression {
 }
 
 
+
 // =====================
 // === Notifications ===
 // =====================

app/gui/src/model/module.rs

@@ -744,7 +744,9 @@ pub mod test {
             repository: Rc<model::undo_redo::Repository>,
         ) -> Module {
             let ast = parser.parse_module(&self.code, self.id_map.clone()).unwrap();
-            let module = Plain::new(self.path.clone(), ast, self.metadata.clone(), repository);
+            let path = self.path.clone();
+            let metadata = self.metadata.clone();
+            let module = Plain::new(path, ast, metadata, repository, default());
             Rc::new(module)
         }
     }

app/gui/src/model/module/plain.rs

@@ -26,6 +26,17 @@ use std::collections::hash_map::Entry;
 
 
 
+// ==============
+// === Errors ===
+// ==============
+
+#[allow(missing_docs)]
+#[derive(Debug, Clone, Copy, Fail)]
+#[fail(display = "Attempt to edit a read-only module")]
+pub struct EditInReadOnly;
+
+
+
 // ==============
 // === Module ===
 // ==============
@@ -41,6 +52,7 @@ pub struct Module {
     content:       RefCell<Content>,
     notifications: notification::Publisher<Notification>,
     repository:    Rc<model::undo_redo::Repository>,
+    read_only:     Rc<Cell<bool>>,
 }
 
 impl Module {
@@ -50,36 +62,44 @@ impl Module {
         ast: ast::known::Module,
         metadata: Metadata,
         repository: Rc<model::undo_redo::Repository>,
+        read_only: Rc<Cell<bool>>,
     ) -> Self {
         Module {
             content: RefCell::new(ParsedSourceFile { ast, metadata }),
             notifications: default(),
             path,
             repository,
+            read_only,
         }
     }
 
     /// Replace the module's content with the new value and emit notification of given kind.
     ///
-    /// Fails if the `new_content` is so broken that it cannot be serialized to text. In such case
+    /// ### Errors
+    /// - Fails if the `new_content` is so broken that it cannot be serialized to text. In such case
     /// the module's state is guaranteed to remain unmodified and the notification will not be
     /// emitted.
+    /// - Fails if the module is read-only. Metadata-only changes are allowed in read-only mode.
     #[profile(Debug)]
     fn set_content(&self, new_content: Content, kind: NotificationKind) -> FallibleResult {
         if new_content == *self.content.borrow() {
             debug!("Ignoring spurious update.");
             return Ok(());
         }
-        trace!("Updating module's content: {kind:?}. New content:\n{new_content}");
-        let transaction = self.repository.transaction("Setting module's content");
-        transaction.fill_content(self.id(), self.content.borrow().clone());
-
-        // We want the line below to fail before changing state.
-        let new_file = new_content.serialize()?;
-        let notification = Notification::new(new_file, kind);
-        self.content.replace(new_content);
-        self.notifications.notify(notification);
-        Ok(())
+        if self.read_only.get() && kind != NotificationKind::MetadataChanged {
+            Err(EditInReadOnly.into())
+        } else {
+            trace!("Updating module's content: {kind:?}. New content:\n{new_content}");
+            let transaction = self.repository.transaction("Setting module's content");
+            transaction.fill_content(self.id(), self.content.borrow().clone());
+
+            // We want the line below to fail before changing state.
+            let new_file = new_content.serialize()?;
+            let notification = Notification::new(new_file, kind);
+            self.content.replace(new_content);
+            self.notifications.notify(notification);
+            Ok(())
+        }
     }
 
     /// Use `f` to update the module's content.