Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorClaudio Guglielmo2017-12-18 09:54:02 +0000
committerClaudio Guglielmo2017-12-18 11:31:01 +0000
commitecd78e68082f8f7ccddc4c424a647a3ac6c5e87a (patch)
tree40e7514b8dfa38ac09c8cec9baa38732abf5b820
parentbe41d6d85ab6b7773563689b4816ced19dedf627 (diff)
downloadorg.eclipse.scout.rt-ecd78e68082f8f7ccddc4c424a647a3ac6c5e87a.tar.gz
org.eclipse.scout.rt-ecd78e68082f8f7ccddc4c424a647a3ac6c5e87a.tar.xz
org.eclipse.scout.rt-ecd78e68082f8f7ccddc4c424a647a3ac6c5e87a.zip
TableField.js: fix memory leak
Also changed from lists to maps which should improve performance at least for large tables. Additionally made sure that if a newly inserted row is updated before it will be deleted again won't mark the table field as touched. https://www.eclipse.org/forums/index.php/t/1090629/ 219855
-rw-r--r--org.eclipse.scout.rt.ui.html.test/src/test/js/scout/form/fields/tablefield/TableFieldSpec.js21
-rw-r--r--org.eclipse.scout.rt.ui.html/src/main/js/scout/form/fields/tablefield/TableField.js86
-rw-r--r--org.eclipse.scout.rt.ui.html/src/main/js/scout/util/objects.js9
3 files changed, 74 insertions, 42 deletions
diff --git a/org.eclipse.scout.rt.ui.html.test/src/test/js/scout/form/fields/tablefield/TableFieldSpec.js b/org.eclipse.scout.rt.ui.html.test/src/test/js/scout/form/fields/tablefield/TableFieldSpec.js
index 63899cdb10..44bd1c4536 100644
--- a/org.eclipse.scout.rt.ui.html.test/src/test/js/scout/form/fields/tablefield/TableFieldSpec.js
+++ b/org.eclipse.scout.rt.ui.html.test/src/test/js/scout/form/fields/tablefield/TableFieldSpec.js
@@ -111,6 +111,16 @@ describe("TableField", function() {
expect(tableField.requiresSave).toBe(true);
});
+ it('does not create a memory leak if same row is updated multiple times', function() {
+ tableField.table.updateRow(firstRow);
+ tableField.updateRequiresSave();
+ expect(Object.keys(tableField._updatedRows).length).toBe(1);
+
+ tableField.table.updateRow(firstRow);
+ tableField.updateRequiresSave();
+ expect(Object.keys(tableField._updatedRows).length).toBe(1);
+ });
+
it('should require save when row has been deleted', function() {
tableField.table.deleteRow(firstRow);
tableField.updateRequiresSave();
@@ -133,6 +143,17 @@ describe("TableField", function() {
expect(tableField.requiresSave).toBe(false);
});
+ it('should NOT require save when row has been inserted and deleted again even if it was updated or checked in the meantime', function() {
+ var rowModel = tableHelper.createModelRow('new', ['foo', 'bar']);
+ tableField.table.insertRow(rowModel);
+ var insertedRow = tableField.table.rowsMap['new'];
+ tableField.table.updateRow(insertedRow);
+ tableField.table.checkRow(insertedRow);
+ tableField.table.deleteRow(insertedRow);
+ tableField.updateRequiresSave();
+ expect(tableField.requiresSave).toBe(false);
+ });
+
it('should require save when row has been checked', function() {
tableField.table.setProperty('checkable', true);
tableField.table.checkRow(firstRow);
diff --git a/org.eclipse.scout.rt.ui.html/src/main/js/scout/form/fields/tablefield/TableField.js b/org.eclipse.scout.rt.ui.html/src/main/js/scout/form/fields/tablefield/TableField.js
index 2b29080e1d..7f71703bcc 100644
--- a/org.eclipse.scout.rt.ui.html/src/main/js/scout/form/fields/tablefield/TableField.js
+++ b/org.eclipse.scout.rt.ui.html/src/main/js/scout/form/fields/tablefield/TableField.js
@@ -14,10 +14,10 @@ scout.TableField = function() {
this.gridDataHints.weightY = 1.0;
this.gridDataHints.h = 3;
this._tableChangedHandler;
- this._deletedRows = [];
- this._insertedRows = [];
- this._updatedRows = [];
- this._checkedRows = [];
+ this._deletedRows = scout.objects.createMap();
+ this._insertedRows = scout.objects.createMap();
+ this._updatedRows = scout.objects.createMap();
+ this._checkedRows = scout.objects.createMap();
this._addAdapterProperties(['table']);
};
scout.inherits(scout.TableField, scout.FormField);
@@ -83,46 +83,47 @@ scout.TableField.prototype._removeTable = function() {
};
scout.TableField.prototype.computeRequiresSave = function() {
- return this._deletedRows.length > 0 ||
- this._insertedRows.length > 0 ||
- this._updatedRows.length > 0 ||
- this._checkedRows.length > 0;
+ return Object.keys(this._deletedRows).length > 0 ||
+ Object.keys(this._insertedRows).length > 0 ||
+ Object.keys(this._updatedRows).length > 0 ||
+ Object.keys(this._checkedRows).length > 0;
};
scout.TableField.prototype._onTableChanged = function(event) {
if (scout.isOneOf(event.type, 'rowsDeleted', 'allRowsDeleted')) {
- scout.arrays.pushAll(this._deletedRows, event.rows);
- this._updateInsertedRows();
- return;
- }
- if (event.type === 'rowsInserted') {
- scout.arrays.pushAll(this._insertedRows, event.rows);
- return;
- }
- if (event.type === 'rowsUpdated') {
- scout.arrays.pushAll(this._updatedRows, event.rows);
- return;
- }
- if (event.type === 'rowsChecked') {
- this._toggleCheckedRows(event.rows);
+ this._updateDeletedRows(event.rows);
+ } else if (event.type === 'rowsInserted') {
+ this._updateInsertedRows(event.rows);
+ } else if (event.type === 'rowsUpdated') {
+ this._updateUpdatedRows(event.rows);
+ } else if (event.type === 'rowsChecked') {
+ this._updateCheckedRows(event.rows);
}
};
-/**
- * If a row is contained in both arrays (_deletedRows, _insertedRows) an inserted row has been
- * deleted again. In that case we can remove that row from both arrays.
- */
-scout.TableField.prototype._updateInsertedRows = function () {
- var insertedAndDeletedRows = [];
- this._deletedRows.forEach(function(deletedRow) {
- if (this._insertedRows.indexOf(deletedRow) !== -1) {
- insertedAndDeletedRows.push(deletedRow);
+scout.TableField.prototype._updateDeletedRows = function (rows) {
+ rows.forEach(function(row) {
+ if (row.id in this._insertedRows) {
+ // If a row is contained in _insertedRows an inserted row has been deleted again.
+ // In that case we can remove that row from the maps and don't have to add it to deletedRows as well.
+ delete this._insertedRows[row.id];
+ delete this._updatedRows[row.id];
+ delete this._checkedRows[row.id];
+ return;
}
+ this._deletedRows[row.id] = row;
}, this);
+};
+
+scout.TableField.prototype._updateInsertedRows = function (rows) {
+ rows.forEach(function(row) {
+ this._insertedRows[row.id] = row;
+ }, this);
+};
- insertedAndDeletedRows.forEach(function(row) {
- scout.arrays.remove(this._deletedRows, row);
- scout.arrays.remove(this._insertedRows, row);
+scout.TableField.prototype._updateUpdatedRows = function (rows) {
+ rows.forEach(function(row) {
+ this._updatedRows[row.id] = row;
}, this);
};
@@ -130,21 +131,22 @@ scout.TableField.prototype._updateInsertedRows = function () {
* If a row already exists in the _checkedRows array, remove it (row was checked/unchecked again, which
* means it is no longer changed). Add it to the array otherwise.
*/
-scout.TableField.prototype._toggleCheckedRows = function (rows) {
+scout.TableField.prototype._updateCheckedRows = function (rows) {
rows.forEach(function(row) {
- var removed = scout.arrays.remove(this._checkedRows, row);
- if (!removed) {
- this._checkedRows.push(row);
+ if (row.id in this._checkedRows) {
+ delete this._checkedRows[row.id];
+ } else {
+ this._checkedRows[row.id] = row;
}
}, this);
};
scout.TableField.prototype.markAsSaved = function() {
scout.TableField.parent.prototype.markAsSaved.call(this);
- this._deletedRows = [];
- this._insertedRows = [];
- this._updatedRows = [];
- this._checkedRows = [];
+ this._deletedRows = scout.objects.createMap();
+ this._insertedRows = scout.objects.createMap();
+ this._updatedRows = scout.objects.createMap();
+ this._checkedRows = scout.objects.createMap();
};
scout.TableField.prototype.validate = function() {
diff --git a/org.eclipse.scout.rt.ui.html/src/main/js/scout/util/objects.js b/org.eclipse.scout.rt.ui.html/src/main/js/scout/util/objects.js
index 8cb811b90d..c86c7823d6 100644
--- a/org.eclipse.scout.rt.ui.html/src/main/js/scout/util/objects.js
+++ b/org.eclipse.scout.rt.ui.html/src/main/js/scout/util/objects.js
@@ -11,6 +11,15 @@
scout.objects = {
/**
+ * Uses Object.create(null) to create an object without a prototype. This is different to use the literal {} which links the object to Object.prototype.
+ * <p>
+ * Not using the literal has the advantage that the object does not contain any inherited properties like `toString` so it is not necessary to use `o.hasOwnProperty(p)` instead of `p in o` to check for the existence.
+ */
+ createMap: function() {
+ return Object.create(null);
+ },
+
+ /**
* Copies all the properties (including the ones from the prototype.) from dest to source
* @memberOf scout.objects
* @returns the destination object (the destination parameter will be modified as well)

Back to the top