-
Notifications
You must be signed in to change notification settings - Fork 61
[bindings] Expose Engine.clone() to JS and add explicit Engine.prepare() warm-up API across bindings
#722
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
base: main
Are you sure you want to change the base?
[bindings] Expose Engine.clone() to JS and add explicit Engine.prepare() warm-up API across bindings
#722
Changes from 2 commits
839933c
72515f6
730e6de
c65e844
196b6d6
4cc82e2
639bfe3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,6 +138,14 @@ impl Engine { | |
| self.engine.set_rego_v0(enable) | ||
| } | ||
|
|
||
| /// Clone this engine. | ||
| /// | ||
| /// Useful for creating per-request engines after loading policy/data once. | ||
| #[wasm_bindgen(js_name = "clone")] | ||
| pub fn cloneEngine(&self) -> Engine { | ||
| Clone::clone(self) | ||
| } | ||
|
|
||
| /// Add a policy | ||
| /// | ||
| /// The policy is parsed into AST. | ||
|
|
@@ -158,6 +166,14 @@ impl Engine { | |
| self.engine.add_data(data).map_err(error_to_jsvalue) | ||
| } | ||
|
|
||
| /// Prepare the engine for evaluation. | ||
| /// | ||
| /// This initializes internal evaluation structures so a cloned engine can | ||
| /// evaluate without requiring an initial "dummy" evaluation. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to remark that if we don't call this, the first eval on the engine will pay this cost. Also about adding policies after preparation.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added explicit prepare semantics in wasm docs: first eval pays this one-time setup if |
||
| pub fn prepare(&mut self) -> Result<(), JsValue> { | ||
| self.engine.prepare().map_err(error_to_jsvalue) | ||
| } | ||
|
|
||
| /// Get the list of packages defined by loaded policies. | ||
| /// | ||
| /// See https://docs.rs/regorus/latest/regorus/struct.Engine.html#method.get_packages | ||
|
|
@@ -487,6 +503,9 @@ mod tests { | |
| )?; | ||
| assert_eq!(pkg, "data.test"); | ||
|
|
||
| // Prepare before first evaluation. | ||
| engine.prepare()?; | ||
|
|
||
| let results = engine.evalQuery("data".to_string())?; | ||
| let r = regorus::Value::from_json_str(&results).map_err(error_to_jsvalue)?; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -505,6 +505,34 @@ impl Engine { | |
| self.add_data(Value::from_json_str(data_json)?) | ||
| } | ||
|
|
||
| /// Prepare the engine for evaluation without executing a query or rule. | ||
| /// | ||
| /// This parses and initializes internal evaluation data structures so that | ||
| /// subsequent evaluations (or cloned engines) can run without paying the | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make the documentation more clear about why this is needed, what ahappens if not called, what happens if policies added after this etc.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Expanded core |
||
| /// one-time preparation cost during the first evaluation call. | ||
| /// | ||
| /// ``` | ||
| /// # use regorus::*; | ||
| /// # fn main() -> anyhow::Result<()> { | ||
| /// let mut engine = Engine::new(); | ||
| /// engine.add_policy("test.rego".to_string(), r#" | ||
| /// package test | ||
| /// import rego.v1 | ||
| /// allow if input.user == "alice" | ||
| /// "#.to_string())?; | ||
| /// | ||
| /// engine.prepare()?; | ||
| /// let mut cloned = engine.clone(); | ||
| /// | ||
| /// cloned.set_input_json(r#"{"user":"alice"}"#)?; | ||
| /// assert_eq!(cloned.eval_rule("data.test.allow".to_string())?, Value::from(true)); | ||
| /// # Ok(()) | ||
| /// # } | ||
| /// ``` | ||
| pub fn prepare(&mut self) -> Result<()> { | ||
| self.prepare_for_eval(false, false) | ||
| } | ||
|
Comment on lines
+545
to
+547
|
||
|
|
||
| /// Set whether builtins should raise errors strictly or not. | ||
| /// | ||
| /// Regorus differs from OPA in that by default builtins will | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remarks on the performance of this. Tahe a deep look. It is intended to avoid deep colies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the clone docs to clarify performance intent: clone avoids reparsing/reloading immutable policy structures, while mutable evaluation state is copied for isolation. Commit:
730e6de.