Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read-only mode for controllers #6259

Merged
merged 12 commits into from
Apr 20, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" : {
Expand All @@ -572,7 +572,8 @@ fn test_execution_context() {
],
"oldVersion" : "d3ee9b1ba1990fecfd794d2f30e0207aaa7be5d37d463073096d86f8",
"newVersion" : "6a33e22f20f16642697e8bd549ff7b759252ad56c05a1b0acc31dc69"
}
},
"execute": true
}),
unit_json.clone(),
(),
Expand Down
176 changes: 153 additions & 23 deletions app/gui/src/controller/graph/executed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;


// ====================
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
});
}
}
2 changes: 1 addition & 1 deletion app/gui/src/controller/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
}

Expand Down
1 change: 1 addition & 0 deletions app/gui/src/controller/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub struct CannotCommitExpression {
}



// =====================
// === Notifications ===
// =====================
Expand Down
4 changes: 3 additions & 1 deletion app/gui/src/model/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
42 changes: 31 additions & 11 deletions app/gui/src/model/module/plain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ===
// ==============
Expand All @@ -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 {
Expand All @@ -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.
Expand Down
Loading