Add config versioning

packages/perspective/src/js/config/constants.js

@@ -34,6 +34,7 @@ export const CONFIG_ALIASES = {
 };
 
 export const CONFIG_VALID_KEYS = [
+    "version",
     "viewport",
     "group_by",
     "split_by",

rust/perspective-viewer/src/rust/config/view_config.rs

@@ -30,6 +30,9 @@ use crate::utils::*;
 #[derive(Clone, Debug, Deserialize, Default, PartialEq, Serialize)]
 #[serde(deny_unknown_fields)]
 pub struct ViewConfig {
+    #[serde(default)]
+    pub version: u32,
+
     #[serde(default)]
     pub group_by: Vec<String>,
 
@@ -108,6 +111,9 @@ impl ViewConfig {
 #[derive(Clone, Deserialize, Default, Serialize)]
 #[serde(deny_unknown_fields)]
 pub struct ViewConfigUpdate {
+    #[serde(default)]
+    pub version: Option<u32>,
+
     #[serde(default)]
     pub group_by: Option<Vec<String>>,
 

rust/perspective-viewer/src/rust/config/viewer_config.rs

@@ -49,6 +49,8 @@ impl FromStr for ViewerConfigEncoding {
 #[derive(Serialize, PartialEq)]
 #[serde(deny_unknown_fields)]
 pub struct ViewerConfig {
+    // any version change is by definition breaking, so no semver here
+    pub version: u32,
     pub plugin: String,
     pub plugin_config: Value,
     pub settings: bool,
@@ -62,6 +64,7 @@ pub struct ViewerConfig {
 // `#[serde(flatten)]` makes messagepack 2x as big as they can no longer be
 // struct fields, so make a tuple alternative for serialization in binary.
 type ViewerConfigBinarySerialFormat<'a> = (
+    u32,
     &'a String,
     &'a Value,
     bool,
@@ -71,6 +74,7 @@ type ViewerConfigBinarySerialFormat<'a> = (
 );
 
 type ViewerConfigBinaryDeserialFormat = (
+    VersionUpdate,
     PluginUpdate,
     Option<Value>,
     SettingsUpdate,
@@ -82,6 +86,7 @@ type ViewerConfigBinaryDeserialFormat = (
 impl ViewerConfig {
     fn token(&self) -> ViewerConfigBinarySerialFormat<'_> {
         (
+            self.version,
             &self.plugin,
             &self.plugin_config,
             self.settings,
@@ -125,6 +130,9 @@ impl ViewerConfig {
 #[derive(Clone, Deserialize)]
 // #[serde(deny_unknown_fields)]
 pub struct ViewerConfigUpdate {
+    #[serde(default)]
+    pub version: VersionUpdate,
+
     #[serde(default)]
     pub plugin: PluginUpdate,
 
@@ -146,9 +154,10 @@ pub struct ViewerConfigUpdate {
 
 impl ViewerConfigUpdate {
     fn from_token(
-        (plugin, plugin_config, settings, theme, title, view_config): ViewerConfigBinaryDeserialFormat,
+        (version, plugin, plugin_config, settings, theme, title, view_config): ViewerConfigBinaryDeserialFormat,
     ) -> ViewerConfigUpdate {
         ViewerConfigUpdate {
+            version,
             plugin,
             plugin_config,
             settings,
@@ -196,6 +205,7 @@ pub type PluginUpdate = OptionalUpdate<String>;
 pub type SettingsUpdate = OptionalUpdate<bool>;
 pub type ThemeUpdate = OptionalUpdate<String>;
 pub type TitleUpdate = OptionalUpdate<String>;
+pub type VersionUpdate = OptionalUpdate<u32>;
 
 /// Handles `{}` when included as a field with `#[serde(default)]`.
 impl<T: Clone> Default for OptionalUpdate<T> {

rust/perspective-viewer/src/rust/custom_elements/viewer.rs

@@ -314,6 +314,7 @@ impl PerspectiveViewerElement {
         clone!(self.session, self.renderer, self.root, self.presentation);
         ApiFuture::new(async move {
             let ViewerConfigUpdate {
+                version,
                 plugin,
                 plugin_config,
                 settings,
@@ -322,6 +323,13 @@ impl PerspectiveViewerElement {
                 mut view_config,
             } = ViewerConfigUpdate::decode(&update)?;
 
+            if let OptionalUpdate::Update(version) = version && version != session.api_version() {
+                web_sys::console::warn_1(&"The provided config version does not match the viewer's API version. \
+                    If this configuration fails, please migrate it and try again!".into());
+                // TODO: auto-migrate the configuration if possible
+                // migrate()
+            }
+
             if !session.has_table() {
                 if let OptionalUpdate::Update(x) = settings {
                     presentation.set_settings_attribute(x);

rust/perspective-viewer/src/rust/model/get_viewer_config.rs

@@ -44,6 +44,7 @@ pub trait GetViewerConfigModel: HasSession + HasRenderer + HasPresentation {
     fn get_viewer_config(&self) -> Pin<Box<dyn Future<Output = ApiResult<ViewerConfig>>>> {
         clone!(self.renderer(), self.session(), self.presentation());
         Box::pin(async move {
+            let version = session.api_version();
             let view_config = session.get_view_config().clone();
             let js_plugin = renderer.get_active_plugin()?;
             let settings = presentation.is_settings_open();
@@ -52,6 +53,7 @@ pub trait GetViewerConfigModel: HasSession + HasRenderer + HasPresentation {
             let theme = presentation.get_selected_theme_name().await;
             let title = presentation.get_title();
             Ok(ViewerConfig {
+                version,
                 plugin,
                 title,
                 plugin_config,

rust/perspective-viewer/src/rust/session.rs

@@ -95,6 +95,12 @@ pub type MetadataRef<'a> = std::cell::Ref<'a, SessionMetadata>;
 pub type MetadataMutRef<'a> = std::cell::RefMut<'a, SessionMetadata>;
 
 impl Session {
+    pub const API_VERSION: u32 = 0;
+
+    pub fn api_version(&self) -> u32 {
+        Self::API_VERSION
+    }
+
     pub fn metadata(&self) -> MetadataRef<'_> {
         std::cell::Ref::map(self.borrow(), |x| &x.metadata)
     }

rust/perspective-viewer/src/rust/session/replace_expression_update.rs

@@ -30,14 +30,14 @@ impl ViewConfig {
     ) -> ViewConfigUpdate {
         let expression = new_expr;
         let Self {
+            version,
             columns,
             expressions,
             group_by,
             split_by,
             sort,
             filter,
             aggregates,
-            ..
         } = self.clone();
 
         let expressions = expressions
@@ -116,6 +116,7 @@ impl ViewConfig {
             .collect::<Vec<_>>();
 
         ViewConfigUpdate {
+            version: Some(version),
             columns: Some(columns),
             aggregates: Some(aggregates),
             expressions: Some(expressions),

rust/perspective-viewer/src/ts/migrate.ts

@@ -111,6 +111,9 @@ function migrate_workspace(old, options) {
  * @returns
  */
 function migrate_viewer(old, omit_attributes, options) {
+    // if (old.version && old.version > 0) {
+    // TODO: Check for version and implement a migration chain.
+    // } else {
     return chain(
         old,
         [
@@ -126,9 +129,14 @@ function migrate_viewer(old, omit_attributes, options) {
             omit_attributes
                 ? migrate_attributes_workspace
                 : migrate_attributes_viewer,
+            (old) => {
+                old.version = 0;
+                return old;
+            },
         ].filter((x) => !!x),
         options
     );
+    // }
 }
 
 /**
@@ -357,6 +365,22 @@ function migrate_expressions(old, options) {
         }
     }
 
+    // check for strings, replace with objects
+    for (let i in old.expressions) {
+        if (typeof old.expressions[i] === "string") {
+            console.warn("Replacing deprecated string expression with object");
+            let old_expr = old.expressions[i];
+            let [whole_expr, name, expr] = old_expr.match(
+                /\/\/\s*([^\n]+)\n(.*)/
+            ) ?? [old_expr, null, null];
+            if (name && expr) {
+                old.expressions[i] = { name, expr };
+            } else {
+                old.expressions[i] = { name: whole_expr, expr: whole_expr };
+            }
+        }
+    }
+
     return old;
 }
 

rust/perspective-viewer/test/js/events.spec.ts

@@ -67,6 +67,7 @@ test.describe("Events", () => {
         });
 
         expect(config).toEqual({
+            version: 0,
             aggregates: {},
             split_by: [],
             columns: ["Profit", "Sales"],

rust/perspective-viewer/test/js/migrate_viewer.spec.js

@@ -46,14 +46,20 @@ const TESTS = [
             plugin_config: {},
         },
         {
+            version: 0,
             plugin: "Y Area",
             plugin_config: {},
             group_by: ["bucket(\"Order Date\", 'M')"],
             split_by: ["Ship Mode"],
             columns: ["Sales"],
             filter: [["Category", "==", "Office Supplies"]],
             sort: [],
-            expressions: ["bucket(\"Order Date\", 'M')"],
+            expressions: [
+                {
+                    name: "bucket(\"Order Date\", 'M')",
+                    expr: "bucket(\"Order Date\", 'M')",
+                },
+            ],
             aggregates: {},
             title: null,
         },
@@ -74,6 +80,7 @@ const TESTS = [
             plugin_config: { Sales: { color_mode: "gradient", gradient: 10 } },
         },
         {
+            version: 0,
             plugin: "Datagrid",
             plugin_config: {
                 columns: {
@@ -112,6 +119,7 @@ const TESTS = [
             aggregates: {},
         },
         {
+            version: 0,
             plugin: "Datagrid",
             plugin_config: {
                 columns: {
@@ -158,6 +166,7 @@ const TESTS = [
             aggregates: {},
         },
         {
+            version: 0,
             plugin: "Datagrid",
             plugin_config: {
                 columns: {
@@ -225,6 +234,7 @@ const TESTS = [
             split_by: [],
         },
         {
+            version: 0,
             plugin: "Datagrid",
             plugin_config: {
                 columns: {
@@ -267,6 +277,45 @@ const TESTS = [
             title: null,
         },
     ],
+    [
+        "Move expressions from string to object",
+        {
+            plugin: "Datagrid",
+            plugin_config: {
+                columns: {},
+                editable: true,
+                scroll_lock: false,
+            },
+            title: null,
+            group_by: [],
+            split_by: [],
+            columns: ["'hello'", "expr"],
+            filter: [],
+            sort: [],
+            expressions: ["// expr\n1+1", "'hello'"],
+            aggregates: {},
+        },
+        {
+            version: 0,
+            plugin: "Datagrid",
+            plugin_config: {
+                columns: {},
+                editable: true,
+                scroll_lock: false,
+            },
+            title: null,
+            group_by: [],
+            split_by: [],
+            columns: ["'hello'", "expr"],
+            filter: [],
+            sort: [],
+            expressions: [
+                { name: "expr", expr: "1+1" },
+                { name: "'hello'", expr: "'hello'" },
+            ],
+            aggregates: {},
+        },
+    ],
 ];
 
 test.beforeEach(async ({ page }) => {

rust/perspective-viewer/test/js/plugins.spec.js

@@ -34,6 +34,7 @@ test.describe("Plugin Priority Order", () => {
         });
 
         const expected = {
+            version: 0,
             aggregates: {},
             columns: [
                 "Row ID",

rust/perspective-viewer/test/js/save_restore.spec.js

@@ -51,6 +51,7 @@ test.describe("Save/Restore", async () => {
         });
 
         expect(config).toEqual({
+            version: 0,
             aggregates: {},
             split_by: [],
             columns: ["Profit", "Sales"],
@@ -85,6 +86,7 @@ test.describe("Save/Restore", async () => {
         });
 
         expect(config).toEqual({
+            version: 0,
             aggregates: {},
             split_by: [],
             columns: ["Profit", "Sales"],
@@ -106,6 +108,7 @@ test.describe("Save/Restore", async () => {
         });
 
         expect(config2).toEqual({
+            version: 0,
             aggregates: {},
             split_by: [],
             columns: [
@@ -147,6 +150,7 @@ test.describe("Save/Restore", async () => {
         }, config);
 
         expect(config3).toEqual({
+            version: 0,
             aggregates: {},
             split_by: [],
             columns: ["Profit", "Sales"],

rust/perspective-viewer/test/js/settings.spec.js

@@ -115,10 +115,13 @@ test.describe("Settings", () => {
                 //   "RuntimeError::unreachable",
             ]);
 
-            expect(logs).toEqual([
-                "Invalid config, resetting to default {group_by: Array(0), split_by: Array(0), columns: Array(0), filter: Array(0), sort: Array(0)} `restore()` called before `load()`",
-                "Caught error: `restore()` called before `load()`",
-            ]);
+            expect(logs.length).toBe(2);
+            expect(logs[0]).toMatch(
+                /Invalid config, resetting to default \{[^}]+\} `restore\(\)` called before `load\(\)`/
+            );
+            expect(logs[1]).toEqual(
+                "Caught error: `restore()` called before `load()`"
+            );
         });
     });
 });