Merge pull request #259 from jpmorganchase/computed-update-fix

Fixed bug which caused computed-columns creation to break updates in …
finos · Oct 4, 2018 · 2e7c903 · 2e7c903
2 parents 94e983d + c4b0661
commit 2e7c903
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 11 deletions.
diff --git a/packages/perspective-viewer-hypergrid/src/js/hypergrid.js b/packages/perspective-viewer-hypergrid/src/js/hypergrid.js
@@ -282,7 +282,9 @@ async function grid_update(div, view, task) {
         return;
     }
 
-    this.hypergrid.behavior.dataModel.setDirty(nrows);
+    const dataModel = this.hypergrid.behavior.dataModel;
+    dataModel.setDirty(nrows);
+    dataModel._view = view;
     this.hypergrid.canvas.paintNow();
 }
 
@@ -337,7 +339,7 @@ async function grid_create(div, view, task) {
     dataModel._view = view;
 
     dataModel.pspFetch = async function(range) {
-        let next_page = await view.to_columns(range);
+        let next_page = await dataModel._view.to_columns(range);
         this.data = [];
         const rows = psp2hypergrid(next_page, hidden, schema, tschema, rowPivots).rows;
         const data = this.data;

diff --git a/packages/perspective-viewer-hypergrid/test/html/regressions.html b/packages/perspective-viewer-hypergrid/test/html/regressions.html
@@ -0,0 +1,41 @@
+<!--
+   
+   Copyright (c) 2017, the Perspective Authors.
+   
+   This file is part of the Perspective library, distributed under the terms of
+   the Apache License 2.0.  The full license can be found in the LICENSE file.
+
+-->
+
+<!DOCTYPE html>
+<html>
+    <head>
+
+        <script src="perspective.view.js"></script>
+        <script src="hypergrid.plugin.js"></script>
+
+        <link rel='stylesheet' href="demo.css">
+
+    </head>
+    <body>
+
+        <perspective-viewer>
+
+        </perspective-viewer>
+
+        <script>
+
+            var viewer = document.getElementsByTagName('perspective-viewer')[0];
+            viewer.load([
+                {x: 1, y: "a", z: true},
+                {x: 2, y: "b", z: false},
+                {x: 3, y: "c", z: true},
+                {x: 4, y: "a", z: false},
+                {x: 5, y: "b", z: true},
+                {x: 6, y: "c", z: false}
+            ], {index: 'x'}); 
+
+        </script>
+
+    </body>
+</html>
diff --git a/packages/perspective-viewer-hypergrid/test/js/regressions.spec.js b/packages/perspective-viewer-hypergrid/test/js/regressions.spec.js
@@ -0,0 +1,47 @@
+/******************************************************************************
+ *
+ * Copyright (c) 2017, the Perspective Authors.
+ *
+ * This file is part of the Perspective library, distributed under the terms of
+ * the Apache License 2.0.  The full license can be found in the LICENSE file.
+ *
+ */
+
+const utils = require("@jpmorganchase/perspective-viewer/test/js/utils.js");
+
+async function capture_update(page, viewer, body) {
+    await page.evaluate(element => {
+        element.addEventListener("perspective-view-update", () => {
+            element.setAttribute("test-updated", true);
+        });
+    }, viewer);
+    await body();
+    await page.waitFor(element => element.hasAttribute("test-updated"), {}, viewer);
+    await page.evaluate(element => element.removeAttribute("test-updated"), viewer);
+}
+
+utils.with_server({}, () => {
+    describe.page("regressions.html", () => {
+        describe("Updates", () => {
+            test.capture("regular updates", async page => {
+                await page.click("#config_button");
+                const viewer = await page.$("perspective-viewer");
+                await capture_update(page, viewer, () => page.evaluate(element => element.update([{x: 3, y: "Updated!"}]), viewer));
+                await page.waitFor("perspective-viewer:not([updating])");
+            });
+
+            test.capture("saving a computed column does not interrupt update rendering", async page => {
+                await page.click("#config_button");
+                const viewer = await page.$("perspective-viewer");
+                await page.click("#add-computed-column");
+                await page.$eval("perspective-computed-column", element => {
+                    const columns = [{name: "y", type: "string"}];
+                    element._apply_state(columns, element.computations["length"], "new_cc");
+                });
+                await page.click("#psp-cc-button-save");
+                await page.waitForSelector("perspective-viewer:not([updating])");
+                await capture_update(page, viewer, () => page.evaluate(element => element.update([{x: 3, y: "Updated!"}]), viewer));
+            });
+        });
+    });
+});
diff --git a/packages/perspective-viewer-hypergrid/test/results/results.json b/packages/perspective-viewer-hypergrid/test/results/results.json
@@ -1,4 +1,5 @@
 {
+    "regressions.html/regular updates": "edcc330ff7f45b41329315853db41cdc",
     "superstore.html/shows a grid without any settings applied.": "5c9f7fcce4e9bb1f144409afbc43b0fa",
     "superstore.html/pivots by a row.": "a4a3502c749d8a5f08c3b02e28096bc5",
     "superstore.html/pivots by two rows.": "c562f3ff391ca00b012b3ae234e54403",
@@ -12,5 +13,6 @@
     "superstore.html/sorts by a hidden column.": "93293864b4d0670ba1e46ec5ff397d02",
     "superstore.html/filters by a numeric column.": "f3b7ca04028702596acabfbe46ca074c",
     "superstore.html/pivots by a column.": "d1dce7a5d99e1760954bb99751e0a761",
-    "superstore.html/collapses to depth smaller than viewport": "42f4811a16d4aaaa4a2d7ac5e632d7d9"
+    "superstore.html/collapses to depth smaller than viewport": "42f4811a16d4aaaa4a2d7ac5e632d7d9",
+    "regressions.html/saving a computed column does not interrupt update rendering": "67a91687a0ae568d86c9ddb0fa1fff63"
 }
diff --git a/packages/perspective-viewer/test/js/utils.js b/packages/perspective-viewer/test/js/utils.js
@@ -110,9 +110,9 @@ beforeAll(async () => {
         } else {
             text = msg.text;
         }
-        if (msg.type === "error") {
+        if (msg.type() === "error") {
             errors.push(text);
-            private_console.log(`${__name}: ${text}`);
+            private_console.log(`${__name}: ${text}\n`);
         }
     });
     page.on("pageerror", msg => {
@@ -208,10 +208,7 @@ test.capture = function capture(name, body, timeout = 60000, viewport = null) {
                 }
             }
             if (process.env.WRITE_TESTS) {
-                setTimeout(() => {
-                    results[_url + "/" + name] = hash;
-                    new_results[_url + "/" + name] = hash;
-                });
+                new_results[_url + "/" + name] = hash;
             }
             expect(errors).toEqual([]);
             expect(hash).toBe(results[_url + "/" + name]);

diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js
@@ -1306,6 +1306,7 @@ module.exports = function(Module) {
         let pdata;
         let cols = this._columns();
         let schema = this.gnode.get_tblschema();
+        let names = schema.columns();
         let types = schema.types();
 
         if (data instanceof ArrayBuffer) {
@@ -1314,6 +1315,12 @@ module.exports = function(Module) {
             pdata = parse_data(data, cols, types);
         }
 
+        for (let i = 0; i < names.size(); i++) {
+            if (cols.indexOf(names.get(i)) === -1) {
+                pdata.types.splice(i, 1);
+            }
+        }
+
         let tbl;
         try {
             tbl = __MODULE__.make_table(pdata.row_count || 0, pdata.names, pdata.types, pdata.cdata, this.limit_index, this.limit || 4294967295, this.index || "", pdata.is_arrow, false);
@@ -1335,6 +1342,7 @@ module.exports = function(Module) {
                 tbl.delete();
             }
             schema.delete();
+            names.delete();
             types.delete();
         }
     };
@@ -1407,7 +1415,7 @@ module.exports = function(Module) {
                 computed = this.computed.concat(computed);
             }
 
-            return new table(gnode, pool, this.index, computed);
+            return new table(gnode, pool, this.index, computed, this.limit, this.limit_index);
         } catch (e) {
             if (pool) {
                 pool.delete();
@@ -1425,11 +1433,12 @@ module.exports = function(Module) {
 
     table.prototype._columns = function() {
         let schema = this.gnode.get_tblschema();
+        let computed_schema = this._computed_schema();
         let cols = schema.columns();
         let names = [];
         for (let cidx = 0; cidx < cols.size(); cidx++) {
             let name = cols.get(cidx);
-            if (name !== "psp_okey") {
+            if (name !== "psp_okey" && typeof computed_schema[name] === "undefined") {
                 names.push(name);
             }
         }

diff --git a/packages/perspective/test/js/updates.js b/packages/perspective/test/js/updates.js
@@ -169,6 +169,25 @@ module.exports = perspective => {
         });
     });
 
+    describe("Computed column updates", function() {
+        it("String computed column of arity 1", async function() {
+            var table = perspective.table(data);
+
+            let table2 = table.add_computed([
+                {
+                    column: "yes/no",
+                    type: "string",
+                    func: z => (z === true ? "yes" : "no"),
+                    inputs: ["z"]
+                }
+            ]);
+            table2.update(data);
+            let result = await table2.view({aggregate: [{op: "count", column: "yes/no"}]}).to_json();
+            let expected = [{"yes/no": "yes"}, {"yes/no": "no"}, {"yes/no": "yes"}, {"yes/no": "no"}, {"yes/no": "yes"}, {"yes/no": "no"}, {"yes/no": "yes"}, {"yes/no": "no"}];
+            expect(result).toEqual(expected);
+        });
+    });
+
     describe("Notifications", function() {
         it("`on_update()`", function(done) {
             var table = perspective.table(meta);