From 8e338b7b35192b8faf546fefcc8480cd36a21152 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 3 Sep 2021 22:03:25 +0000 Subject: [PATCH] fix: cereal backwards compatibility (#5421) * fix: remove duplicate serialization hook implementations * feat: add backwards compatibility to field serialization * feat: add support for serializing old mutator hooks * fix: build * fix: refactor field changes into helpers * fix: typo * fix: removing xmlns * tests: add tests for serialization and deserialization of mutator hooks * fix: switch to early returns --- core/field.js | 51 +++++ core/field_angle.js | 20 -- core/field_checkbox.js | 18 +- core/field_colour.js | 20 -- core/field_dropdown.js | 13 +- core/field_label_serializable.js | 20 -- core/field_multilineinput.js | 14 +- core/field_number.js | 20 -- core/field_textinput.js | 20 -- core/field_variable.js | 9 +- core/serialization/blocks.js | 13 +- tests/mocha/field_test.js | 266 ++++++++++++++++++++++++ tests/mocha/jso_deserialization_test.js | 27 +++ tests/mocha/jso_serialization_test.js | 12 ++ 14 files changed, 395 insertions(+), 128 deletions(-) diff --git a/core/field.js b/core/field.js index cb6f0e890..a1d93d0af 100644 --- a/core/field.js +++ b/core/field.js @@ -422,6 +422,10 @@ Blockly.Field.prototype.toXml = function(fieldElement) { * @package */ Blockly.Field.prototype.saveState = function() { + var legacyState = this.saveLegacyState(Blockly.Field); + if (legacyState !== null) { + return legacyState; + } return this.getValue(); }; @@ -432,9 +436,56 @@ Blockly.Field.prototype.saveState = function() { * @package */ Blockly.Field.prototype.loadState = function(state) { + if (this.loadLegacyState(Blockly.Field, state)) { + return; + } this.setValue(state); }; +// eslint-disable-next-line valid-jsdoc +/** + * Returns a stringified version of the XML state, if it should be used. + * Otherwise this returns null, to signal the field should use its own + * serialization. + * @param {?} callingClass The class calling this method. + * Used to see if `this` has overridden any relevant hooks. + * @return {?string} The stringified version of the XML state, or null. + * @protected + */ +Blockly.Field.prototype.saveLegacyState = function(callingClass) { + if (callingClass.prototype.saveState === this.saveState && + callingClass.prototype.toXml !== this.toXml) { + var elem = Blockly.utils.xml.createElement("field"); + elem.setAttribute("name", this.name || ''); + var text = Blockly.Xml.domToText(this.toXml(elem)); + return text.replace( + ' xmlns="https://developers.google.com/blockly/xml"', ''); + } + // Either they called this on purpose from their saveState, or they have + // no implementations of either hook. Just do our thing. + return null; +}; + +// eslint-disable-next-line valid-jsdoc +/** + * Loads the given state using either the old XML hoooks, if they should be + * used. Returns true to indicate loading has been handled, false otherwise. + * @param {?} callingClass The class calling this method. + * Used to see if `this` has overridden any relevant hooks. + * @param {*} state The state to apply to the field. + * @return {boolean} Whether the state was applied or not. + */ +Blockly.Field.prototype.loadLegacyState = function(callingClass, state) { + if (callingClass.prototype.loadState === this.loadState && + callingClass.prototype.fromXml !== this.fromXml) { + this.fromXml(Blockly.Xml.textToDom(/** @type {string} */ (state))); + return true; + } + // Either they called this on purpose from their loadState, or they have + // no implementations of either hook. Just do our thing. + return false; +}; + /** * Dispose of all DOM objects and events belonging to this editable field. * @package diff --git a/core/field_angle.js b/core/field_angle.js index c4c292799..ab877589a 100644 --- a/core/field_angle.js +++ b/core/field_angle.js @@ -254,26 +254,6 @@ Blockly.FieldAngle.prototype.initView = function() { this.textElement_.appendChild(this.symbol_); }; -/** - * Saves this field's value. - * @return {number} The angle value held by this field. - * @override - * @package - */ -Blockly.FieldAngle.prototype.saveState = function() { - return /** @type {number} */ (this.getValue()); -}; - -/** - * Sets the field's value based on the given state. - * @param {*} state The state to apply to the angle field. - * @override - * @package - */ -Blockly.FieldAngle.prototype.loadState = function(state) { - this.setValue(state); -}; - /** * Updates the graph when the field rerenders. * @protected diff --git a/core/field_checkbox.js b/core/field_checkbox.js index 3214c97ae..3511cc857 100644 --- a/core/field_checkbox.js +++ b/core/field_checkbox.js @@ -102,22 +102,16 @@ Blockly.FieldCheckbox.prototype.configure_ = function(config) { /** * Saves this field's value. - * @return {boolean} The boolean value held by this field. + * @return {*} The boolean value held by this field. * @override * @package */ Blockly.FieldCheckbox.prototype.saveState = function() { - return /** @type {boolean} */ (this.getValueBoolean()); -}; - -/** - * Sets the field's value based on the given state. - * @param {*} state The state to apply to the checkbox field. - * @override - * @package - */ -Blockly.FieldCheckbox.prototype.loadState = function(state) { - this.setValue(state); + var legacyState = this.saveLegacyState(Blockly.FieldCheckbox); + if (legacyState !== null) { + return legacyState; + } + return this.getValueBoolean(); }; /** diff --git a/core/field_colour.js b/core/field_colour.js index fad70d9ad..17d593f0e 100644 --- a/core/field_colour.js +++ b/core/field_colour.js @@ -185,26 +185,6 @@ Blockly.FieldColour.prototype.initView = function() { } }; -/** - * Saves this field's value. - * @return {string} The colour value held by this field. - * @override - * @package - */ -Blockly.FieldColour.prototype.saveState = function() { - return /** @type {string} */ (this.getValue()); -}; - -/** - * Sets the field's value based on the given state. - * @param {*} state The state to apply to the colour field. - * @override - * @package - */ -Blockly.FieldColour.prototype.loadState = function(state) { - this.setValue(state); -}; - /** * @override */ diff --git a/core/field_dropdown.js b/core/field_dropdown.js index c6e4b1484..70e8cbfe3 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -168,16 +168,6 @@ Blockly.FieldDropdown.prototype.fromXml = function(fieldElement) { this.setValue(fieldElement.textContent); }; -/** - * Saves this field's value. - * @return {*} The dropdown value held by this field. - * @override - * @package - */ -Blockly.FieldDropdown.prototype.saveState = function() { - return this.getValue(); -}; - /** * Sets the field's value based on the given state. * @param {*} state The state to apply to the dropdown field. @@ -185,6 +175,9 @@ Blockly.FieldDropdown.prototype.saveState = function() { * @package */ Blockly.FieldDropdown.prototype.loadState = function(state) { + if (this.loadLegacyState(Blockly.FieldDropdown, state)) { + return; + } if (this.isOptionListDynamic()) { this.getOptions(false); } diff --git a/core/field_label_serializable.js b/core/field_label_serializable.js index 5774ba776..0201db130 100644 --- a/core/field_label_serializable.js +++ b/core/field_label_serializable.js @@ -67,25 +67,5 @@ Blockly.FieldLabelSerializable.prototype.EDITABLE = false; */ Blockly.FieldLabelSerializable.prototype.SERIALIZABLE = true; -/** - * Saves this field's value. - * @return {string} The text value held by this field. - * @override - * @package - */ -Blockly.FieldLabelSerializable.prototype.saveState = function() { - return /** @type {string} */ (this.getValue()); -}; - -/** - * Sets the field's value based on the given state. - * @param {*} state The state to apply to the label field. - * @override - * @package - */ -Blockly.FieldLabelSerializable.prototype.loadState = function(state) { - this.setValue(state); -}; - Blockly.fieldRegistry.register( 'field_label_serializable', Blockly.FieldLabelSerializable); diff --git a/core/field_multilineinput.js b/core/field_multilineinput.js index 14a5640d7..738cbf38d 100644 --- a/core/field_multilineinput.js +++ b/core/field_multilineinput.js @@ -123,21 +123,27 @@ Blockly.FieldMultilineInput.prototype.fromXml = function(fieldElement) { /** * Saves this field's value. - * @return {string} The text value held by this field. - * @override + * @return {*} The state of this field. * @package */ Blockly.FieldMultilineInput.prototype.saveState = function() { - return /** @type {string} */ (this.getValue()); + var legacyState = this.saveLegacyState(Blockly.FieldMultilineInput); + if (legacyState !== null) { + return legacyState; + } + return this.getValue(); }; /** * Sets the field's value based on the given state. - * @param {*} state The state to apply to the multiline input field. + * @param {*} state The state of the variable to assign to this variable field. * @override * @package */ Blockly.FieldMultilineInput.prototype.loadState = function(state) { + if (this.loadLegacyState(Blockly.Field, state)) { + return; + } this.setValue(state); }; diff --git a/core/field_number.js b/core/field_number.js index bb11a5121..369a161f8 100644 --- a/core/field_number.js +++ b/core/field_number.js @@ -117,26 +117,6 @@ Blockly.FieldNumber.prototype.configure_ = function(config) { this.setPrecisionInternal_(config['precision']); }; -/** - * Saves this field's value. - * @return {number} The number value held by this field. - * @override - * @package - */ -Blockly.FieldNumber.prototype.saveState = function() { - return /** @type {number} */ (this.getValue()); -}; - -/** - * Sets the field's value based on the given state. - * @param {*} state The state to apply to the nuber field. - * @override - * @package - */ -Blockly.FieldNumber.prototype.loadState = function(state) { - this.setValue(state); -}; - /** * Set the maximum, minimum and precision constraints on this field. * Any of these properties may be undefined or NaN to be disabled. diff --git a/core/field_textinput.js b/core/field_textinput.js index a2000303e..b2153b65e 100644 --- a/core/field_textinput.js +++ b/core/field_textinput.js @@ -178,26 +178,6 @@ Blockly.FieldTextInput.prototype.initView = function() { this.createTextElement_(); }; -/** - * Saves this field's value. - * @return {*} The text value held by this field. - * @override - * @package - */ -Blockly.FieldTextInput.prototype.saveState = function() { - return this.getValue(); -}; - -/** - * Sets the field's value based on the given state. - * @param {*} state The state to apply to the text input field. - * @override - * @package - */ -Blockly.FieldTextInput.prototype.loadState = function(state) { - this.setValue(state); -}; - /** * Ensure that the input value casts to a valid string. * @param {*=} opt_newValue The input value. diff --git a/core/field_variable.js b/core/field_variable.js index 45b039c42..738d1437e 100644 --- a/core/field_variable.js +++ b/core/field_variable.js @@ -196,11 +196,15 @@ Blockly.FieldVariable.prototype.toXml = function(fieldElement) { /** * Saves this field's value. - * @return {{id: string}} The ID of the variable referenced by this field. + * @return {*} The ID of the variable referenced by this field. * @override * @package */ Blockly.FieldVariable.prototype.saveState = function() { + var legacyState = this.saveLegacyState(Blockly.Field); + if (legacyState !== null) { + return legacyState; + } // Make sure the variable is initialized. this.initModel(); return { @@ -215,6 +219,9 @@ Blockly.FieldVariable.prototype.saveState = function() { * @package */ Blockly.FieldVariable.prototype.loadState = function(state) { + if (this.loadLegacyState(Blockly.Field, state)) { + return; + } // This is necessary so that blocks in the flyout can have custom var names. var variable = Blockly.Variables.getOrCreateVariablePackage( this.sourceBlock_.workspace, diff --git a/core/serialization/blocks.js b/core/serialization/blocks.js index 27ad854b9..b0c4870a0 100644 --- a/core/serialization/blocks.js +++ b/core/serialization/blocks.js @@ -24,6 +24,7 @@ const {ISerializer} = goog.requireType('Blockly.serialization.ISerializer'); const Size = goog.require('Blockly.utils.Size'); // eslint-disable-next-line no-unused-vars const Workspace = goog.requireType('Blockly.Workspace'); +const Xml = goog.require('Blockly.Xml'); const inputTypes = goog.require('Blockly.inputTypes'); const priorities = goog.require('Blockly.serialization.priorities'); const serializationRegistry = goog.require('Blockly.serialization.registry'); @@ -162,6 +163,12 @@ const saveExtraState = function(block, state) { if (extraState !== null) { state['extraState'] = extraState; } + } else if (block.mutationToDom) { + const extraState = block.mutationToDom(); + if (extraState !== null) { + state['extraState'] = Xml.domToText(extraState).replace( + ' xmlns="https://developers.google.com/blockly/xml"', ''); + } } }; @@ -427,7 +434,11 @@ const loadExtraState = function(block, state) { if (!state['extraState']) { return; } - block.loadExtraState(state['extraState']); + if (block.loadExtraState) { + block.loadExtraState(state['extraState']); + } else { + block.domToMutation(Xml.textToDom(state['extraState'])); + } }; /** diff --git a/tests/mocha/field_test.js b/tests/mocha/field_test.js index f89193d4b..b3eb40f55 100644 --- a/tests/mocha/field_test.js +++ b/tests/mocha/field_test.js @@ -12,9 +12,11 @@ suite('Abstract Fields', function() { // console logs. createDeprecationWarningStub(); }); + teardown(function() { sharedTestTeardown.call(this); }); + suite('Is Serializable', function() { // Both EDITABLE and SERIALIZABLE are default. function FieldDefault() { @@ -68,6 +70,269 @@ suite('Abstract Fields', function() { chai.assert.isTrue(field.isSerializable()); }); }); + + suite('Serialization', function() { + class DefaultSerializationField extends Blockly.Field { + constructor(value, validator = undefined, config = undefined) { + super(value, validator, config); + this.SERIALIZABLE = true; + } + } + + class CustomXmlField extends Blockly.Field { + constructor(value, validator = undefined, config = undefined) { + super(value, validator, config); + this.SERIALIZABLE = true; + } + + toXml(fieldElement) { + fieldElement.textContent = 'custom value'; + return fieldElement; + } + + fromXml(fieldElement) { + this.someProperty = fieldElement.textContent; + } + } + + class CustomXmlCallSuperField extends Blockly.Field { + constructor(value, validator = undefined, config = undefined) { + super(value, validator, config); + this.SERIALIZABLE = true; + } + + toXml(fieldElement) { + super.toXml(fieldElement); + fieldElement.setAttribute('attribute', 'custom value'); + return fieldElement; + } + + fromXml(fieldElement) { + super.fromXml(fieldElement); + this.someProperty = fieldElement.getAttribute('attribute'); + } + } + + class CustomJsoField extends Blockly.Field { + constructor(value, validator = undefined, config = undefined) { + super(value, validator, config); + this.SERIALIZABLE = true; + } + + saveState() { + return 'custom value'; + } + + loadState(state) { + this.someProperty = state; + } + } + + class CustomJsoCallSuperField extends Blockly.Field { + constructor(value, validator = undefined, config = undefined) { + super(value, validator, config); + this.SERIALIZABLE = true; + } + + saveState() { + return { + default: super.saveState(), + val: 'custom value' + }; + } + + loadState(state) { + super.loadState(state.default); + this.someProperty = state.val; + } + } + + class CustomXmlAndJsoField extends Blockly.Field { + constructor(value, validator = undefined, config = undefined) { + super(value, validator, config); + this.SERIALIZABLE = true; + } + + toXml(fieldElement) { + fieldElement.textContent = 'custom value'; + return fieldElement; + } + + fromXml(fieldElement) { + this.someProperty = fieldElement.textContent; + } + + saveState() { + return 'custom value'; + } + + loadState(state) { + this.someProperty = state; + } + } + + suite('Save', function() { + suite('JSO', function() { + test('No implementations', function() { + const field = new DefaultSerializationField('test value'); + const value = field.saveState(); + chai.assert.equal(value, 'test value'); + }); + + test('Xml implementations', function() { + const field = new CustomXmlField('test value'); + const value = field.saveState(); + chai.assert.equal(value, 'custom value'); + }); + + test('Xml super implementation', function() { + const field = new CustomXmlCallSuperField('test value'); + const value = field.saveState(); + chai.assert.equal( + value, + 'test value'); + }); + + test('JSO implementations', function() { + const field = new CustomJsoField('test value'); + const value = field.saveState(); + chai.assert.equal(value, 'custom value'); + }); + + test('JSO super implementations', function() { + const field = new CustomJsoCallSuperField('test value'); + const value = field.saveState(); + chai.assert.deepEqual( + value, {default: 'test value', val: 'custom value'}); + }); + + test('Xml and JSO implementations', function() { + const field = new CustomXmlAndJsoField('test value'); + const value = field.saveState(); + chai.assert.equal(value, 'custom value'); + }); + }); + + suite('Xml', function() { + test('No implementations', function() { + const field = new DefaultSerializationField('test value'); + const element = document.createElement('field'); + const value = Blockly.Xml.domToText(field.toXml(element)); + chai.assert.equal( + value, + 'test value'); + }); + + test('Xml implementations', function() { + const field = new CustomXmlField('test value'); + const element = document.createElement('field'); + const value = Blockly.Xml.domToText(field.toXml(element)); + chai.assert.equal( + value, + 'custom value' + ); + }); + + test('Xml super implementation', function() { + const field = new CustomXmlCallSuperField('test value'); + const element = document.createElement('field'); + const value = Blockly.Xml.domToText(field.toXml(element)); + chai.assert.equal( + value, + 'test value'); + }); + + test('Xml and JSO implementations', function() { + const field = new CustomXmlAndJsoField('test value'); + const element = document.createElement('field'); + const value = Blockly.Xml.domToText(field.toXml(element)); + chai.assert.equal( + value, + 'custom value' + ); + }); + }); + }); + + suite('Load', function() { + suite('JSO', function() { + test('No implementations', function() { + const field = new DefaultSerializationField(''); + field.loadState('test value'); + chai.assert.equal(field.getValue(), 'test value'); + }); + + test('Xml implementations', function() { + const field = new CustomXmlField(''); + field.loadState('custom value'); + chai.assert.equal(field.someProperty, 'custom value'); + }); + + test('Xml super implementation', function() { + const field = new CustomXmlCallSuperField(''); + field.loadState( + 'test value'); + chai.assert.equal(field.getValue(), 'test value'); + chai.assert.equal(field.someProperty, 'custom value'); + }); + + test('JSO implementations', function() { + const field = new CustomJsoField(''); + field.loadState('custom value'); + chai.assert.equal(field.someProperty, 'custom value'); + }); + + test('JSO super implementations', function() { + const field = new CustomJsoCallSuperField(''); + field.loadState({default: 'test value', val: 'custom value'}); + chai.assert.equal(field.getValue(), 'test value'); + chai.assert.equal(field.someProperty, 'custom value'); + }); + + test('Xml and JSO implementations', function() { + const field = new CustomXmlAndJsoField(''); + field.loadState('custom value'); + chai.assert.equal(field.someProperty, 'custom value'); + }); + }); + + suite('Xml', function() { + test('No implementations', function() { + const field = new DefaultSerializationField(''); + field.fromXml( + Blockly.Xml.textToDom('test value')); + chai.assert.equal(field.getValue(), 'test value'); + }); + + test('Xml implementations', function() { + const field = new CustomXmlField(''); + field.fromXml( + Blockly.Xml.textToDom('custom value')); + chai.assert.equal(field.someProperty, 'custom value'); + }); + + test('Xml super implementation', function() { + const field = new CustomXmlCallSuperField(''); + field.fromXml( + Blockly.Xml.textToDom( + 'test value' + ) + ); + chai.assert.equal(field.getValue(), 'test value'); + chai.assert.equal(field.someProperty, 'custom value'); + }); + + test('XML andd JSO implementations', function() { + const field = new CustomXmlAndJsoField(''); + field.fromXml( + Blockly.Xml.textToDom('custom value')); + chai.assert.equal(field.someProperty, 'custom value'); + }); + }); + }); + }); + suite('setValue', function() { function addSpies(field, excludeSpies = []) { if (!excludeSpies.includes('doValueInvalid_')) { @@ -316,6 +581,7 @@ suite('Abstract Fields', function() { sinon.assert.calledOnce(this.field.doValueUpdate_); }); }); + suite('Customization', function() { // All this field does is wrap the abstract field. function CustomField(opt_config) { diff --git a/tests/mocha/jso_deserialization_test.js b/tests/mocha/jso_deserialization_test.js index 3b2f0efbc..a5ed16238 100644 --- a/tests/mocha/jso_deserialization_test.js +++ b/tests/mocha/jso_deserialization_test.js @@ -676,4 +676,31 @@ suite('JSO Deserialization', function() { 'third-load' ]); }); + + suite('Extra state', function() { + // Most of this is covered by our round-trip tests. But we need one test + // for old xml hooks. + test('Xml hooks', function() { + Blockly.Blocks['test_block'] = { + init: function() { }, + + mutationToDom: function() { }, + + domToMutation: function(element) { + this.someProperty = element.getAttribute('value'); + } + }; + + const block = Blockly.serialization.blocks.load( + { + 'type': 'test_block', + 'extraState': '', + }, + this.workspace); + + delete Blockly.Blocks['test_block']; + + chai.assert.equal(block.someProperty, 'some value'); + }); + }); }); diff --git a/tests/mocha/jso_serialization_test.js b/tests/mocha/jso_serialization_test.js index 7cf4af353..f9dd6b200 100644 --- a/tests/mocha/jso_serialization_test.js +++ b/tests/mocha/jso_serialization_test.js @@ -203,6 +203,18 @@ suite('JSO Serialization', function() { const jso = Blockly.serialization.blocks.save(block); assertProperty(jso, 'extraState', ['state1', 42, true]); }); + + test('Xml hooks', function() { + const block = this.workspace.newBlock('row_block'); + block.mutationToDom = function() { + var container = Blockly.utils.xml.createElement('mutation'); + container.setAttribute('value', 'some value'); + return container; + }; + const jso = Blockly.serialization.blocks.save(block); + assertProperty( + jso, 'extraState', ''); + }); }); suite('Icons', function() {