Merge "[INTERNAL][FIX] sap.ui.fl: Rebuild filteredResponse after storageResponse update" into rel-1.120

Author: ArthurTrauter

src/sap.ui.fl/src/sap/ui/fl/apply/_internal/flexState/FlexState.js

@@ -560,12 +560,13 @@ sap.ui.define([
 
 	/**
 	 * Some save operations don't require a complete new data request, so the storage response gets a live update.
-	 * This will not invalidate the DataSelectors
+	 * This will also update the runtime persistence.
 	 *
 	 * @param {string} sReference - Flex reference of the app
 	 * @param {object[]} aUpdates - All new FlexObjects in JSON format
 	 */
 	FlexState.updateStorageResponse = function(sReference, aUpdates) {
+		const aFlexObjectUpdates = [];
 		aUpdates.forEach((oUpdate) => {
 			if (oUpdate.type === "ui2") {
 				_mInstances[sReference].unfilteredStorageResponse.changes.ui2personalization = oUpdate.newData;
@@ -574,23 +575,55 @@ sap.ui.define([
 				const sFileName = oUpdate.flexObject.fileName;
 				const aUnfiltered = ObjectPath.get(vPath, _mInstances[sReference].unfilteredStorageResponse.changes);
 				const aFiltered = ObjectPath.get(vPath, _mInstances[sReference].storageResponse.changes);
+				const iExistingFlexObjectIdx = _mInstances[sReference].runtimePersistence.flexObjects.findIndex(
+					(oFlexObject) => oFlexObject.getId() === sFileName
+				);
+				let iFilteredIdx;
+				let iUnfilteredIdx;
+				const oExistingFlexObject = _mInstances[sReference].runtimePersistence.flexObjects[iExistingFlexObjectIdx];
 				switch (oUpdate.type) {
 					case "add":
 						aUnfiltered.push(oUpdate.flexObject);
 						aFiltered.push(oUpdate.flexObject);
+						if (iExistingFlexObjectIdx < 0) {
+							throw new Error("Flex response includes unknown flex object");
+						}
 						break;
 					case "delete":
-						aFiltered.splice(aFiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName), 1);
-						aUnfiltered.splice(aUnfiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName), 1);
+						iFilteredIdx = aFiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName);
+						if (iFilteredIdx > -1) {
+							aFiltered.splice(iFilteredIdx, 1);
+						}
+						iUnfilteredIdx = aUnfiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName);
+						if (iUnfilteredIdx > -1) {
+							aUnfiltered.splice(iUnfilteredIdx, 1);
+						}
+						if (iExistingFlexObjectIdx >= 0) {
+							_mInstances[sReference].runtimePersistence.flexObjects.splice(iExistingFlexObjectIdx, 1);
+							aFlexObjectUpdates.push({ type: "removeFlexObject", updatedObject: oExistingFlexObject });
+						}
 						break;
 					case "update":
-						aFiltered.splice(aFiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName), 1, oUpdate.flexObject);
-						aUnfiltered.splice(aUnfiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName), 1, oUpdate.flexObject);
+						iFilteredIdx = aFiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName);
+						if (iFilteredIdx > -1) {
+							aFiltered.splice(iFilteredIdx, 1, oUpdate.flexObject);
+						}
+						iUnfilteredIdx = aUnfiltered.findIndex((oFlexObject) => oFlexObject.fileName === sFileName);
+						if (iUnfilteredIdx > -1) {
+							aUnfiltered.splice(iUnfilteredIdx, 1, oUpdate.flexObject);
+						}
+						if (oExistingFlexObject && oExistingFlexObject.getState() !== States.LifecycleState.PERSISTED) {
+							oExistingFlexObject.setResponse(oUpdate.flexObject);
+							aFlexObjectUpdates.push({ type: "updateFlexObject", updatedObject: oExistingFlexObject });
+						}
 						break;
 					default:
 				}
 			}
 		});
+		if (aFlexObjectUpdates.length > 0) {
+			oFlexObjectsDataSelector.checkUpdate({ reference: sReference }, aFlexObjectUpdates);
+		}
 	};
 
 	FlexState.clearState = function(sReference) {

src/sap.ui.fl/test/sap/ui/fl/qunit/ChangePersistence.qunit.js

@@ -46,35 +46,36 @@ sap.ui.define([
 ) {
 	"use strict";
 
-	var sandbox = sinon.createSandbox();
-	var aControls = [];
+	const sandbox = sinon.createSandbox();
+	const aControls = [];
+	const sComponentName = "MyComponent";
 
 	function getInitialChangesMap(mPropertyBag) {
 		return merge(DependencyHandler.createEmptyDependencyMap(), mPropertyBag);
 	}
 
 	QUnit.module("sap.ui.fl.ChangePersistence", {
-		beforeEach() {
-			sandbox.stub(FlexState, "initialize").resolves();
+		async beforeEach() {
 			sandbox.stub(VariantManagementState, "getInitialChanges").returns([]);
 			this._mComponentProperties = {
-				name: "MyComponent"
+				name: sComponentName
 			};
 			this.oChangePersistence = new ChangePersistence(this._mComponentProperties);
 
-			return Component.create({
+			const oComponent = await Component.create({
 				name: "sap.ui.fl.qunit.integration.testComponentComplex",
 				manifest: false
-			}).then(function(oComponent) {
-				this._oComponentInstance = oComponent;
-				this.oControl = new Control("abc123");
-				aControls.push(this.oControl);
-				this.oControlWithComponentId = new Control(oComponent.createId("abc123"));
-				aControls.push(this.oControlWithComponentId);
-			}.bind(this));
+			});
+			this._oComponentInstance = oComponent;
+			this.oControl = new Control("abc123");
+			aControls.push(this.oControl);
+			this.oControlWithComponentId = new Control(oComponent.createId("abc123"));
+			aControls.push(this.oControlWithComponentId);
+			await FlQUnitUtils.initializeFlexStateWithData(sandbox, sComponentName);
 		},
 		afterEach() {
 			sandbox.restore();
+			FlexState.clearState(sComponentName);
 			this._oComponentInstance.destroy();
 			aControls.forEach(function(control) {
 				control.destroy();
@@ -1006,7 +1007,7 @@ sap.ui.define([
 				assert.equal(fnGetCompEntitiesByIdMapStub.callCount, 1, "then getCompEntitiesByIdMap called once");
 				assert.equal(oResetChangesStub.callCount, 1, "Storage.reset is called once");
 				var oResetArgs = oResetChangesStub.getCall(0).args[0];
-				assert.equal(oResetArgs.reference, "MyComponent");
+				assert.equal(oResetArgs.reference, sComponentName);
 				assert.equal(oResetArgs.layer, Layer.CUSTOMER);
 				assert.equal(oResetArgs.changes.length, 3); // oCUSTOMERChange1, oCUSTOMERChange2, oMockCompVariant3
 				assert.equal(oResetArgs.changes[0].getId(), "oCUSTOMERChange1");
@@ -1019,7 +1020,7 @@ sap.ui.define([
 			});
 		});
 
-		QUnit.test("when calling resetChanges with selector and change type (control reset)", function(assert) {
+		QUnit.test("when calling resetChanges with selector and change type (control reset)", async function(assert) {
 			// changes for the component
 			var oVENDORChange1 = FlexObjectFactory.createFromFileContent({
 				fileType: "change",
@@ -1059,27 +1060,32 @@ sap.ui.define([
 			sandbox.stub(this.oChangePersistence, "getChangesForComponent").resolves(aChanges);
 			var aDeletedChangeContentIds = {response: [{fileName: "1"}, {fileName: "2"}]};
 
-			var oResetChangesStub = sandbox.stub(WriteStorage, "reset").resolves(aDeletedChangeContentIds);
-			var oUpdateStorageResponseStub = sandbox.stub(FlexState, "updateStorageResponse");
-			var oGetChangesFromMapByNamesStub = sandbox.stub(this.oChangePersistence, "_getChangesFromMapByNames").returns(aChanges);
-
-			return this.oChangePersistence.resetChanges(Layer.VENDOR, "", ["abc123"], ["labelChange"]).then(function() {
-				assert.equal(oResetChangesStub.callCount, 1, "Storage.reset is called once");
-				var oResetArgs = oResetChangesStub.getCall(0).args[0];
-				assert.equal(oResetArgs.reference, "MyComponent");
-				assert.equal(oResetArgs.layer, Layer.VENDOR);
-				assert.deepEqual(oResetArgs.selectorIds, ["abc123"]);
-				assert.deepEqual(oResetArgs.changeTypes, ["labelChange"]);
-				assert.equal(oUpdateStorageResponseStub.callCount, 1, "the FlexState is called once");
-				assert.deepEqual(oUpdateStorageResponseStub.args[0][1],
-					aChanges.map((oFlexObject) => {
-						return {flexObject: oFlexObject.convertToFileContent(), type: "delete"};
-					}),
-					"and with the correct names"
-				);
-				assert.equal(oGetChangesFromMapByNamesStub.callCount, 1, "the getChangesFromMapByNames is called once");
-				assert.deepEqual(oGetChangesFromMapByNamesStub.args[0][0], ["1", "2"], "and with the correct names");
-			});
+			const oResetChangesStub = sandbox.stub(WriteStorage, "reset").resolves(aDeletedChangeContentIds);
+			const oUpdateStorageResponseSpy = sandbox.spy(FlexState, "updateStorageResponse");
+			const oGetChangesFromMapByNamesStub = sandbox.stub(this.oChangePersistence, "_getChangesFromMapByNames").returns(aChanges);
+
+			await this.oChangePersistence.resetChanges(Layer.VENDOR, "", ["abc123"], ["labelChange"]);
+
+			assert.strictEqual(oResetChangesStub.callCount, 1, "Storage.reset is called once");
+			const oResetArgs = oResetChangesStub.getCall(0).args[0];
+			assert.strictEqual(oResetArgs.reference, sComponentName);
+			assert.strictEqual(oResetArgs.layer, Layer.VENDOR);
+			assert.deepEqual(oResetArgs.selectorIds, ["abc123"]);
+			assert.deepEqual(oResetArgs.changeTypes, ["labelChange"]);
+			assert.strictEqual(oUpdateStorageResponseSpy.callCount, 1, "the FlexState.updateStorageResponse is called once");
+			assert.deepEqual(oUpdateStorageResponseSpy.args[0][1],
+				aChanges.map((oFlexObject) => {
+					return {flexObject: oFlexObject.convertToFileContent(), type: "delete"};
+				}),
+				"and with the correct names"
+			);
+			assert.strictEqual(
+				FlexState.getFlexObjectsDataSelector().get({reference: sComponentName}).length,
+				0,
+				"then the change is also removed from the flex state"
+			);
+			assert.strictEqual(oGetChangesFromMapByNamesStub.callCount, 1, "the getChangesFromMapByNames is called once");
+			assert.deepEqual(oGetChangesFromMapByNamesStub.args[0][0], ["1", "2"], "and with the correct names");
 		});
 	});
 

src/sap.ui.fl/test/sap/ui/fl/qunit/apply/_internal/flexState/FlexState.qunit.js

@@ -1227,21 +1227,29 @@ sap.ui.define([
 			await FlexState.initialize({
 				reference: sReference
 			});
+			const oChangePersistence = ChangePersistenceFactory.getChangePersistenceForComponent(sReference);
 			// initial data
-			FlexState.updateStorageResponse(sReference, [
-				{type: "add", flexObject: FlexObjectFactory.createUIChange({id: "initialUIChange1"}).convertToFileContent()},
-				{type: "add", flexObject: FlexObjectFactory.createUIChange({id: "initialUIChange2", variantReference: "flVariant12"}).convertToFileContent()},
-				{type: "add", flexObject: FlexObjectFactory.createUIChange({id: "initialUIChange3", fileType: "ctrl_variant_change"}).convertToFileContent()},
-				{type: "add", flexObject: FlexObjectFactory.createUIChange({id: "initialUIChange4", fileType: "ctrl_variant_management_change"}).convertToFileContent()},
-				{type: "add", flexObject: FlexObjectFactory.createFlVariant({id: "initialFlVariant1"}).convertToFileContent()},
-				{type: "add", flexObject: FlexObjectFactory.createCompVariant({id: "initialCompVariant1"}).convertToFileContent()},
-				{type: "add", flexObject: FlexObjectFactory.createUIChange({
+			const aInitialChanges = [
+				FlexObjectFactory.createUIChange({id: "initialUIChange1"}),
+				FlexObjectFactory.createUIChange({id: "initialUIChange2", variantReference: "flVariant12"}),
+				FlexObjectFactory.createUIChange({id: "initialUIChange3", fileType: "ctrl_variant_change"}),
+				FlexObjectFactory.createUIChange({id: "initialUIChange4", fileType: "ctrl_variant_management_change"}),
+				FlexObjectFactory.createFlVariant({id: "initialFlVariant1"}),
+				FlexObjectFactory.createCompVariant({id: "initialCompVariant1"}),
+				FlexObjectFactory.createUIChange({
 					id: "initialUIChange5",
 					selector: {
 						persistencyKey: "foo"
 					}
-				}).convertToFileContent()}
-			]);
+				})
+			];
+			aInitialChanges.forEach(function(oFlexObject) {
+				oChangePersistence.addDirtyChange(oFlexObject);
+			});
+			FlexState.updateStorageResponse(sReference, aInitialChanges.map((flexObject) => ({
+				type: "add",
+				flexObject: flexObject.convertToFileContent()
+			})));
 			FlexState.rebuildFilteredResponse(sReference);
 			this.oUIChange = FlexObjectFactory.createUIChange({
 				id: "uiChange1"
@@ -1277,16 +1285,26 @@ sap.ui.define([
 		}
 	}, function() {
 		QUnit.test("with all operations at once", async function(assert) {
-			let aFlexObjects = FlexState.getFlexObjectsDataSelector().get({ reference: sReference });
+			const oFlexObjectsDataSelector = FlexState.getFlexObjectsDataSelector();
+			let aFlexObjects = oFlexObjectsDataSelector.get({ reference: sReference });
 			assert.strictEqual(aFlexObjects.length, 8, "initially there are 8 flexObjects");
+			const aNewChanges = [
+				this.oUIChange,
+				this.oVariantChange1,
+				this.oVariantChange2,
+				this.oVariantDepUIChange,
+				this.oFlVariant,
+				this.oCompVariant,
+				this.oCompChange
+			];
+			aNewChanges.forEach(function(oFlexObject) {
+				FlexState.addDirtyFlexObject(sReference, oFlexObject);
+			});
 			FlexState.updateStorageResponse(sReference, [
-				{type: "add", flexObject: this.oUIChange.convertToFileContent()},
-				{type: "add", flexObject: this.oVariantChange1.convertToFileContent()},
-				{type: "add", flexObject: this.oVariantChange2.convertToFileContent()},
-				{type: "add", flexObject: this.oVariantDepUIChange.convertToFileContent()},
-				{type: "add", flexObject: this.oFlVariant.convertToFileContent()},
-				{type: "add", flexObject: this.oCompVariant.convertToFileContent()},
-				{type: "add", flexObject: this.oCompChange.convertToFileContent()},
+				...aNewChanges.map((flexObject) => ({
+					type: "add",
+					flexObject: flexObject.convertToFileContent()
+				})),
 				{type: "ui2", newData: "ui2"}
 			]);
 			const oStorageResponse = await FlexState.getStorageResponse(sReference);
@@ -1299,25 +1317,25 @@ sap.ui.define([
 			assert.strictEqual(oStorageResponse.changes.variants.length, 2, "fl variant was added");
 			assert.strictEqual(oStorageResponse.changes.ui2personalization, "ui2", "ui2 was set");
 
-			FlexState.rebuildFilteredResponse(sReference);
-			aFlexObjects = FlexState.getFlexObjectsDataSelector().get({ reference: sReference });
+			aFlexObjects = oFlexObjectsDataSelector.get({ reference: sReference });
 			assert.strictEqual(aFlexObjects.length, 15, "all flexObjects are part of the DataSelector");
 
 			this.oFlVariant.setFavorite(true);
 			this.oCompVariant.setFavorite(true);
 			this.oUIChange.setContent("foo");
 			this.oCompChange.setContent("bar");
 
+			const oUpdateSpy = sandbox.spy(oFlexObjectsDataSelector, "checkUpdate");
+			const aUpdates = [this.oUIChange, this.oFlVariant, this.oCompVariant, this.oCompChange];
 			FlexState.updateStorageResponse(sReference, [
-				{type: "update", flexObject: this.oUIChange.convertToFileContent()},
-				{type: "update", flexObject: this.oFlVariant.convertToFileContent()},
-				{type: "update", flexObject: this.oCompVariant.convertToFileContent()},
-				{type: "update", flexObject: this.oCompChange.convertToFileContent()},
+				...aUpdates.map((flexObject) => ({
+					type: "update",
+					flexObject: flexObject.convertToFileContent()
+				})),
 				{type: "ui2", newData: "newUi2"}
 			]);
 			assert.strictEqual(oStorageResponse.changes.ui2personalization, "newUi2", "ui2 was set");
-			FlexState.rebuildFilteredResponse(sReference);
-			aFlexObjects = FlexState.getFlexObjectsDataSelector().get({ reference: sReference });
+			aFlexObjects = oFlexObjectsDataSelector.get({ reference: sReference });
 			assert.strictEqual(aFlexObjects.length, 15, "all flexObjects are part of the DataSelector");
 			assert.strictEqual(
 				aFlexObjects.find((oFlexObject) => oFlexObject.getId() === "uiChange1").getContent(),
@@ -1336,14 +1354,12 @@ sap.ui.define([
 				"bar", "the content was updated"
 			);
 
-			FlexState.updateStorageResponse(sReference, [
-				{type: "delete", flexObject: this.oVariantDepUIChange.convertToFileContent()},
-				{type: "delete", flexObject: this.oFlVariant.convertToFileContent()},
-				{type: "delete", flexObject: this.oCompVariant.convertToFileContent()},
-				{type: "delete", flexObject: this.oCompChange.convertToFileContent()}
-			]);
-			FlexState.rebuildFilteredResponse(sReference);
-			aFlexObjects = FlexState.getFlexObjectsDataSelector().get({ reference: sReference });
+			const aDeletes = [this.oVariantDepUIChange, this.oFlVariant, this.oCompVariant, this.oCompChange];
+			FlexState.updateStorageResponse(sReference, aDeletes.map((flexObject) => ({
+				type: "delete",
+				flexObject: flexObject.convertToFileContent()
+			})));
+			aFlexObjects = oFlexObjectsDataSelector.get({ reference: sReference });
 			assert.strictEqual(aFlexObjects.length, 11, "all remaining flexObjects are part of the DataSelector");
 			assert.notOk(
 				aFlexObjects.find((oFlexObject) => oFlexObject.getId() === this.oVariantDepUIChange.getId()),
@@ -1361,6 +1377,58 @@ sap.ui.define([
 				aFlexObjects.find((oFlexObject) => oFlexObject.getId() === this.oCompChange.getId()),
 				"the flexObject was deleted"
 			);
+			assert.ok(
+				oUpdateSpy.firstCall.args[1].every((oUpdateInfo, iIdx) => {
+					return oUpdateInfo.type === "updateFlexObject" && oUpdateInfo.updatedObject === aUpdates[iIdx];
+				}),
+				"the data selector was updated with the correct operation"
+			);
+			assert.ok(
+				oUpdateSpy.secondCall.args[1].every((oUpdateInfo, iIdx) => {
+					return oUpdateInfo.type === "removeFlexObject" && oUpdateInfo.updatedObject === aDeletes[iIdx];
+				}),
+				"the data selector was updated with the correct operation"
+			);
+		});
+
+		QUnit.test("when only ui2personalization is updated", async function(assert) {
+			const oUpdateFlexObjectsSpy = sandbox.spy(FlexState.getFlexObjectsDataSelector(), "checkUpdate");
+			await FlexState.updateStorageResponse(sReference, [
+				{type: "ui2", newData: "ui2"}
+			]);
+			assert.strictEqual(oUpdateFlexObjectsSpy.callCount, 0, "then the flex objects data selector is not updated");
+		});
+
+		QUnit.test("when adding a flex object that is not part of the runtime pertsistence", function(assert) {
+			assert.throws(
+				function() {
+					FlexState.updateStorageResponse(sReference, [
+						{type: "add", flexObject: {id: "unknownObject"}}
+					]);
+				},
+				"then an error is thrown"
+			);
+		});
+
+		QUnit.test("when updating the storage response", function(assert) {
+			FlexState.addDirtyFlexObject(sReference, this.oUIChange);
+			FlexState.updateStorageResponse(sReference, [
+				{type: "add", flexObject: this.oUIChange.convertToFileContent()}
+			]);
+			FlexState.updateStorageResponse(sReference, [
+				{
+					type: "update",
+					flexObject: {
+						...this.oUIChange.convertToFileContent(),
+						...{content: "bar"}
+					}
+				}
+			]);
+			assert.strictEqual(
+				this.oUIChange.getContent(),
+				"bar",
+				"then the content of the runtime persistence object is also updated"
+			);
 		});
 	});
 

src/sap.ui.fl/test/sap/ui/fl/qunit/write/_internal/flexState/FlexObjectState.qunit.js

@@ -59,6 +59,8 @@ sap.ui.define([
 			changeType: "addGroup",
 			layer: Layer.USER
 		});
+		oChangePersistence.addDirtyChanges([oChangeInPersistence1, oChangeInPersistence2]);
+		// Some tests require persisted changes
 		sandbox.stub(oChangePersistence, "getChangesForComponent").resolves([oChangeInPersistence1, oChangeInPersistence2]);
 	}