Skip to content

Commit

Permalink
Don't recreate the same ViewConfig if it has not actually changed i…
Browse files Browse the repository at this point in the history
…n the UI

Signed-off-by: Andrew Stein <[email protected]>
  • Loading branch information
texodus committed Dec 30, 2024
1 parent 94666e4 commit b9dddd2
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 12 deletions.
20 changes: 20 additions & 0 deletions rust/perspective-client/src/rust/config/view_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,4 +256,24 @@ impl ViewConfig {
|| self.filter.iter().any(|x| x.column() == name)
|| self.columns.contains(&Some(name))
}

/// `ViewConfig` carries additional metadata in the form of `None` columns
/// which are filtered befor ebeing passed to the engine, but whose position
/// is a placeholder for Viewer functionality. `is_equivalent` tests
/// equivalency from the perspective of the engine.
pub fn is_equivalent(&self, other: &Self) -> bool {
let _self = self.clone();
let _self = ViewConfig {
columns: _self.columns.into_iter().filter(|x| x.is_some()).collect(),
.._self
};

let _other = other.clone();
let _other = ViewConfig {
columns: _other.columns.into_iter().filter(|x| x.is_some()).collect(),
..other.clone()
};

_self == _other
}
}
34 changes: 22 additions & 12 deletions rust/perspective-viewer/src/rust/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ impl Session {
let view = self.0.borrow_mut().view_sub.take();
self.borrow_mut().view_sub = None;
self.borrow_mut().config.reset(reset_expressions);

view.delete()
}

Expand Down Expand Up @@ -418,13 +419,7 @@ impl Session {
/// Update the config, setting the `columns` property to the plugin defaults
/// if provided.
pub fn update_view_config(&self, config_update: ViewConfigUpdate) {
if self.borrow().old_config.is_none() {
let config = self.borrow().config.clone();
self.borrow_mut().old_config = Some(config);
}

if self.borrow_mut().config.apply_update(config_update) {
ApiFuture::spawn(self.borrow_mut().view_sub.take().delete());
self.0.borrow_mut().is_clean = false;
self.view_config_changed.emit(());
}
Expand All @@ -442,26 +437,33 @@ impl Session {
/// In order to create a new view in this session, the session must first be
/// validated to create a `ValidSession<'_>` guard.
pub async fn validate(&self) -> Result<ValidSession<'_>, JsValue> {
let old = self.borrow_mut().old_config.take();
let is_diff = match old.as_ref() {
Some(old) => !old.is_equivalent(&self.borrow().config),
None => true,
};

if let Err(err) = self.validate_view_config().await {
web_sys::console::error_3(
&"Invalid config:".into(),
&err.clone().into(),
&JsValue::from_serde_ext(&self.borrow().config).unwrap(),
);

let old = self.borrow_mut().old_config.take();
if let Some(config) = old {
self.borrow_mut().config = config;
} else {
self.reset(true).await?;
}

self.0.view_created.emit(());
Err(err)?;
return Err(err)?;
} else {
let old_config = Some(self.borrow().config.clone());
self.borrow_mut().old_config = old_config;
}

self.borrow_mut().old_config = None;
Ok(ValidSession(self))
Ok(ValidSession(self, is_diff))
}

async fn flat_view(&self, flat: bool) -> ApiResult<View> {
Expand Down Expand Up @@ -571,14 +573,22 @@ impl Session {
}

/// A newtype wrapper which only provides `create_view()`
pub struct ValidSession<'a>(&'a Session);
pub struct ValidSession<'a>(&'a Session, bool);

impl<'a> ValidSession<'a> {
/// Set a new `View` (derived from this `Session`'s `Table`), and create the
/// `update()` subscription, consuming this `ValidSession<'_>` and returning
/// the original `&Session`.
pub async fn create_view(&self) -> Result<&'a Session, ApiError> {
if !self.0.reset_clean() && !self.0.borrow().is_paused {
if !self.1 {
let config = self.0.borrow().config.clone();
if let Some(sub) = &mut self.0.borrow_mut().view_sub.as_mut() {
sub.update_view_config(Rc::new(config));
return Ok(self.0);
}
}

let table = self
.0
.borrow()
Expand All @@ -602,7 +612,7 @@ impl<'a> ValidSession<'a> {
};

let view = self.0.borrow_mut().view_sub.take();
view.delete().await?;
ApiFuture::spawn(view.delete());
self.0.borrow_mut().view_sub = Some(sub);
}

Expand Down
7 changes: 7 additions & 0 deletions rust/perspective-viewer/src/rust/session/view_subscription.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ impl ViewSubscription {
Self { data }
}

/// It is possible to re-use a `ViewSubscription` without a costly
/// resubscribe under certain conditions, which still need an updated
/// `ViewConfig`.
pub fn update_view_config(&mut self, config: Rc<ViewConfig>) {
self.data.config = config
}

/// Getter for the underlying `View()`.
pub const fn get_view(&self) -> &View {
&self.data.view
Expand Down

0 comments on commit b9dddd2

Please sign in to comment.