SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.96k stars 1.24k forks source link

SAPUI5 OData V4 model patch requests from Javascript #2413

Closed cjohn001 closed 5 years ago

cjohn001 commented 5 years ago

@uhlmannm

Hello Mathias, I would like to come back on you regarding our discussion about the use of the odata v4 interface from javascript. Reading and creation of new records now seems to work for me. However, since yesterday I am trying to update the values of certain items from javascript. Again a topic I cannot do via binding the model data to ui controls.

So I tried the following to retrieve the contexts of a list of items I would like to change:

someFun: function(){
              var oMealConfigList = oModel.bindList("/MealConfigs");
              oMealConfigList.filter( new Filter({
                path: "Weekday",
                operator: FilterOperator.NE,
                value1: strSelectedWeekday
              }));

              //setup of callback to process the data once they have been retrieved
             oMealConfigList.attachChange(function(){
                var aMealContexts = oMealConfigList.getContexts();
                this.updateMealConfigs(aMealContexts, aMealConfigs)
              }.bind(this));

              // this trigger the request if the data are not yet there and activates attachChange as 
              // registered above
              var aMealContexts = oMealConfigList.getContexts(); 
              if (aMealContexts.length){
                this.updateMealConfigs(aMealContexts, aMealConfigs);
              }
}

This is working as expected. In the updateMealConfigs function below, I obtain the context array as expected.

updateMealConfigs: function(aMealContexts, aMealConfigs){
            var oModel = sap.ui.getCore().getModel();
            aMealContexts.forEach(function(aMealContext){
                  var test1 = aMealContext.getProperty("MealName");

              var MealNameProperty = oModel.bindProperty("MealName", aMealContext);
              var test2 = MealNameProperty.getValue();
              MealNameProperty.setValue("MealTest");
            });

However, it seems that I can only read from the ODataContext binding an it is not possible to update the values directly via the context. Please correct me if this assumption is wrong, I interpreted this from the documentation and I also found no functions to update certain properties of the retrieved context via its binding. The line with var test1 gives me the expected result and retrieves the correct content. As it seems that the ContextBinding is readonly, I tried to use it to create a PropertyBinding for the relevant attribute ("MealName") and tried to update it via a property binding. Unfortunately, this also does not work. var test2 does not get the relevant value assigned. Hence, already reading of the value does not work. The following setValue line than leads to an error in the debugger:

Failed to update path /MealConfigs(fkProfile=2,Weekday='2',MealNumber='1')/MealName - Error: Must not change a property before it has been read
    at constructor.O.setValue (https://morpheus.fritz.box/resources/sap/ui/core/library-preload.js:3610:344)
    at eval (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:160:32)
    at Array.forEach (<anonymous>)
    at f.updateMealConfigs (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:155:27)
    at f.eval (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:140:22)
    at constructor.b.fireEvent (https://morpheus.fritz.box/resources/sap-ui-core.js:317:253)
    at constructor.B._fireChange (https://morpheus.fritz.box/resources/sap-ui-core.js:1353:42)
    at https://morpheus.fritz.box/resources/sap/ui/core/library-preload.js:3444:1795 sap.ui.model.odata.v4.ODataPropertyBinding

So I am now wondering what I am doing wrong. Would be great if you could provide an example how to use the api for patch requests. Please also not. I have to change multiple attributes per context at once. So if there is a possibility to update not just a single property via a property binding but rather multiple via a context binding at once, an example for this would be great as well.

  1. Question loosely related: I tried to create a list binding directly via the constructor with a static rather than a dynamic filter, i.e.:

var oMealConfigList = oModel.bindList("/MealConfigs", undefined, undefined, undefined,{
$filter: "Weekday ne " + strSelectedWeekday });

This does not work for me. If I do not call oMealConfigList.filter(...) is there than something different required, to initialize the list binding? And what would this be?

Thanks a lot for your help!

Best regards, Christoph

uhlmannm commented 5 years ago

Hi Christoph (@cjohn001 ),

the currently available APIs are optimized for the use by controls. We have the requirement logged to provide more convenient methods for accessing and changing data in controller code. Our backlog item is CPOUI5UISERVICESV3-1315.

As to your first question: You currently need a property binding for changing properties of a context. The sketched way using a relative property binding is correct. However, the property binding still needs to be initialized. The method for this is sap.ui.model.Binding.initialize. There are two disadvantages:

  1. sap.ui.model.Binding.initialize is protected.
  2. sap.ui.model.Binding.initialize works asynchronously. A change event is raised when the property binding is ready.

However, at this point in time, there is no other way forward. For practicability reasons, I propose to perform the creation of the property bindings and their initialization after data reading but before the user interaction. This will make handling the asynchronism of sap.ui.model.Binding.initialize much easier.

As to your second question: In my tests it is not required to call v4.ODataListBinding.filter. E.g.

            var oListBinding = oModel.bindList("/People",undefined,undefined,undefined,{
                $select: ["UserName","FirstName","LastName","Age"]
            });

            oListBinding.attachChange(function(oEvent){
                var aContexts = oListBinding.getContexts(0,Infinity),
// further code
            });
            oListBinding.getContexts(0,Infinity);

Are there any error messages in the console?

Best regards Mathias.

cjohn001 commented 5 years ago

Hello Mathias, thanks for the feedback. Maybe my explanation was not that good. I think we spoke about different things. Regarding topic 2:

What I am trying to reach is to apply a static filter rather than a dynamic one (commented out in below code) and the select statement at the same time. Hence, I was expecting, that the following should work:

             var oMealConfigList = oModel.bindList("/MealConfigs", undefined, undefined, undefined,{
                    $filter: "Weekday ne " + strSelectedWeekday,
                    $select: ["fkProfile","Weekday","MealNumber","MealName"]
              });
   /*           oMealConfigList.filter( new Filter({
                path: "Weekday",
                operator: FilterOperator.NE,
                value1: strSelectedWeekday
              }));
*/
              //setup of callback to process the data once they have been retrieved
              // code path 1
             oMealConfigList.attachChange(function(){
                var aMealContexts = oMealConfigList.getContexts();
                this.updateMealConfigs(aMealContexts, aMealConfigs);
              }.bind(this));

              // this trigger the request if the data are not yet there and activates attachChange as 
              //registered above
              // code path 2
              var aMealContexts = oMealConfigList.getContexts(); 
              if (aMealContexts.length){
                this.updateMealConfigs(aMealContexts, aMealConfigs);
              }

However it does not work. When executing the above code, something seems to be wrong with the oMealConfgList binding. Even so the list is not initialized the code runs into // code path 1 as aMealContexts.length == 1. However the list is not initialized. The error which is triggered is

> 019-02-15 20:04:46.874435 Failed to update path /MealConfigs/-9007199254740991/MealName - Error: Must not change a property before it has been read
>     at constructor.O.setValue (https://morpheus.fritz.box/resources/sap/ui/core/library-preload.js:3610:344)
>     at eval (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:169:32)
>     at Array.forEach (<anonymous>)
>     at f.updateMealConfigs (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:164:27)
>     at f.onApplyToEntireWeekButtonPressed (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:155:22)
>     at f.b.fireEvent (https://morpheus.fritz.box/resources/sap-ui-core.js:317:253)
>     at f.b.fireEvent (https://morpheus.fritz.box/resources/sap-ui-core.js:969:234)
>     at f.firePress (https://morpheus.fritz.box/resources/sap-ui-core.js:468:286)
>     at f.d.ontap (https://morpheus.fritz.box/resources/sap/m/library-preload.js:834:179)
>     at f.b._handleEvent (https://morpheus.fritz.box/resources/sap-ui-core.js:952:373) sap.ui.model.odata.v4.ODataPropertyBinding

you can see from the path "-9007199254740991"results from an uninitialized variable. If I use a dynamic filter (the commented filter code) than things are working. However, I do not understand what is wrong with my static filter code. I would expect that it is the correct way to setup the static filter.

Best regards, Christoph

cjohn001 commented 5 years ago

Regarding the first question I actually cannot follow your explanation. Could you maybe give a code example. Maybe I first explain what I am trying to do. The user has the possibility to set certain configuration options for each day of a week. There is a list in the ui which displays config options of a single day. For example for Monday. In the UI there is also a button, which the user can press to set all other days of the week to the same values he has just set for Monday. So the idea for the button pressed method is the following.

       onApplyToEntireWeekButtonPressed: function(oEvent){
            var oSelectControl = this.byId("idWeekdaySelect");
            var strSelectedWeekday = oSelectControl.getSelectedKey();
            if(strSelectedWeekday){
              // we read the config parameters of the currently set weekday
              //  which we use to update the parametrization of the other
              // weekday configurations
              var oTable = this.byId("idMealConfigTable");
              var aMealConfigs = oTable.getItems();

              // create binding for meal configs and extract all items we need to update
            var oModel = sap.ui.getCore().getModel();
             var oMealConfigList = oModel.bindList("/MealConfigs", undefined, undefined, undefined,{
                    $select: ["fkProfile","Weekday","MealNumber","MealName"]
              });
              oMealConfigList.filter( new Filter({
                path: "Weekday",
                operator: FilterOperator.NE,
                value1: strSelectedWeekday
              }));

              //setup of callback to process the data once they have been retrieved
             oMealConfigList.attachChange(function(){
                var aMealContexts = oMealConfigList.getContexts();
                this.updateMealConfigs(aMealContexts, aMealConfigs);
              }.bind(this));

              // this trigger the request if the data are not yet there and activates 
              // attachChange as registered above
              var aMealContexts = oMealConfigList.getContexts(); 
              if (aMealContexts.length){
                this.updateMealConfigs(aMealContexts, aMealConfigs);
              }

            }// end if
        },
        /******************************************************** */
        // updates the meal contexts with the propa
        updateMealConfigs: function(aMealContexts, aMealConfigs){
            var oModel = sap.ui.getCore().getModel();
            aMealContexts.forEach(function(aMealContext){

              var MealNameProperty = oModel.bindProperty("MealName", aMealContext);
             // MealNameProperty.initialize(); // at this point?
             // the documentation says
            // Initialize the binding. The message should be called when creating a binding. The default 
            // implementation calls checkUpdate(true).
            // I have not found a documentation about checkUpdate. Is this an event which is send to 
           // MealNameProperty? Would the approach than be to setup callbacks to the properties from the 
            // callback function here. This seems to look not right. I have to change 3 parameters on each 
            //weekday, hence this would result in this callback setting up another 18 callback functions just 
            // to set the parameters.
              MealNameProperty.setValue("MealTest");
            });

It would be great if you could give a code example which shows your sketched approach. Thanks a lot for your help!

Best regards, Christoph

uhlmannm commented 5 years ago

Hi Christoph,

thanks for the feedback. Maybe my explanation was not that good. I think we spoke about different things. Regarding topic 2:

What I am trying to reach is to apply a static filter rather than a dynamic one (commented out in below code) and the select statement at the same time. Hence, I was expecting, that the following should work:

             var oMealConfigList = oModel.bindList("/MealConfigs", undefined, undefined, undefined,{
                    $filter: "Weekday ne " + strSelectedWeekday,
                    $select: ["fkProfile","Weekday","MealNumber","MealName"]
              });
   /*           oMealConfigList.filter( new Filter({
                path: "Weekday",
                operator: FilterOperator.NE,
                value1: strSelectedWeekday
              }));
*/
              //setup of callback to process the data once they have been retrieved
              // code path 1
             oMealConfigList.attachChange(function(){
                var aMealContexts = oMealConfigList.getContexts();
                this.updateMealConfigs(aMealContexts, aMealConfigs);
              }.bind(this));

              // this trigger the request if the data are not yet there and activates attachChange as 
              //registered above
              // code path 2
              var aMealContexts = oMealConfigList.getContexts(); 
              if (aMealContexts.length){
                this.updateMealConfigs(aMealContexts, aMealConfigs);
              }

However it does not work. When executing the above code, something seems to be wrong with the oMealConfgList binding. Even so the list is not initialized the code runs into // code path 1 as aMealContexts.length == 1. However the list is not initialized. The error which is triggered is

> 019-02-15 20:04:46.874435 Failed to update path /MealConfigs/-9007199254740991/MealName - Error: Must not change a property before it has been read
>     at constructor.O.setValue (https://morpheus.fritz.box/resources/sap/ui/core/library-preload.js:3610:344)
>     at eval (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:169:32)
>     at Array.forEach (<anonymous>)
>     at f.updateMealConfigs (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:164:27)
>     at f.onApplyToEntireWeekButtonPressed (https://morpheus.fritz.box/controller/SubMenu/Options/ConfigureMeals.controller.js?eval:155:22)
>     at f.b.fireEvent (https://morpheus.fritz.box/resources/sap-ui-core.js:317:253)
>     at f.b.fireEvent (https://morpheus.fritz.box/resources/sap-ui-core.js:969:234)
>     at f.firePress (https://morpheus.fritz.box/resources/sap-ui-core.js:468:286)
>     at f.d.ontap (https://morpheus.fritz.box/resources/sap/m/library-preload.js:834:179)
>     at f.b._handleEvent (https://morpheus.fritz.box/resources/sap-ui-core.js:952:373) sap.ui.model.odata.v4.ODataPropertyBinding

you can see from the path "-9007199254740991"results from an uninitialized variable. If I use a dynamic filter (the commented filter code) than things are working. However, I do not understand what is wrong with my static filter code. I would expect that it is the correct way to setup the static filter.

It seems that there is a collision with autoExpandSelect. But let me start at the beginning.

First: Please adapt the static $filter to "Weekday ne " + "\'"+strSelectedWeekday+"\'". The string literal has to be in ' otherwise it has to be interpreted by the server as a property.

Method filter is one of the methods that will reset the binding. It seems that with autoExpandSelect:true such a reset or the initialization of the binding is required before calling getContexts. My proposal is to not investigate this as it does not cause issues when binding against controls. Using v4.ODataListBinding.getContexts is only a workaround until a more convenient access to list binding data is available. Please use either filter to set the dynamic filter or changeParameters to set the static filter string.

EDIT 17.09.19 Starting with SAPUI5 1.70 method v4.ODataListBinding.requestContexts is available. Starting with 1.70, protected method getContexts should not be used.

Best regards Mathias.

uhlmannm commented 5 years ago

Hi Christoph,

Regarding the first question I actually cannot follow your explanation. Could you maybe give a code example.

The issue I face here is that the coding that is required currently is far from recommendable. But maybe a different solution for your problem is possible.

Maybe I first explain what I am trying to do. The user has the possibility to set certain configuration options for each day of a week. There is a list in the ui which displays config options of a single day. For example for Monday. In the UI there is also a button, which the user can press to set all other days of the week to the same values he has just set for Monday.

This sounds like an example for a bound action. With a bound action, you would first make sure to save all the data of the selected weekday, i.e. to submit the changes of the user. Then you would call the bound action (with the selected weekday as binding parameter) to copy the data to the other days. Please see also our documentation on OData operations!

Could you please evaluate this proposal?

Best regards Mathias.

cjohn001 commented 5 years ago

Hello Mathias, thanks for the reply.

Method filter is one of the methods that will reset the binding. It seems that with autoExpandSelect:true such a reset or the initialization of the binding is required before calling getContexts. My proposal is to not investigate this as it does not cause issues when binding against controls. Using v4.ODataListBinding.getContexts is only a workaround until a more convenient access to list binding data is available. Please use either filter to set the dynamic filter or changeParameters to set the static filter string.

Ok, I will not investigate it further. The "\'" alone does not solve the issue as you mentioned. I will therefore go with the dynamic filter as it also does the job at hand.

This sounds like an example for a bound action. With a bound action, you would first make sure to save all the data of the selected weekday, i.e. to submit the changes of the user. Then you would call the bound action (with the selected weekday as binding parameter) to copy the data to the other days.

I already thought about this variant as well. However, I have a problem with my backend which is a Teiid Server Instance that provides the odata interface to my mysql database. I have not found an option yet to generate an odata interface with Teiid for a stored procedure. And even this would solve the task at hand, I would get into the same issue again when I have to do a manual update operation in a different place via javascript. Hence, it would be great if you could show how to do the update correctly. Thanks a lot for your help.

Best regards, Christoph

uhlmannm commented 5 years ago

Hi Christoph,

This sounds like an example for a bound action. With a bound action, you would first make sure to save all the data of the selected weekday, i.e. to submit the changes of the user. Then you would call the bound action (with the selected weekday as binding parameter) to copy the data to the other days.

I already thought about this variant as well. However, I have a problem with my backend which is a Teiid Server Instance that provides the odata interface to my mysql database. I have not found an option yet to generate an odata interface with Teiid for a stored procedure.

I am not a Teeid expert. https://docs.jboss.org/author/display/TEIID/OData+V4+Translator looks to me as if Actions (albeit probably not bound) should be supported. From my perspective it would make sense to explore that option.

And even this would solve the task at hand, I would get into the same issue again when I have to do a manual update operation in a different place via javascript. Hence, it would be great if you could show how to do the update correctly.

Right now there is only one way for changing properties: The one used for controls. I would not call this correct when we talk about controller code. Hence I propose to either explore the option to use actions or to wait for a tailored API for changing data in controller code. (Which I cannot promise though.)

In addition, please consider whether the performance gain of your cache synchronization is good enough to justify the more complicated app code. Please consider also the possibilities of relative bindings as discussed in #2354.

As to the example: Let us start with the context of your v4 list binding oContext, the value you want to set sValue and the V4 model oModel. The following code shows how to create the property binding, initialize it and change the value. I also added a call of submitBatch to indicate the problems the submitBatch would cause.

var oPropertyBinding = oModel.bindProperty("Age",oContext);

oPropertyBinding.initialize();
oPropertyBinding.attachEventOnce("change",undefined,function(){
    oPropertyBinding.setValue(sValue);
    oModel.submitBatch("Test");
}); 

The issues are apparent:

  1. The simple submitBatch call in the event handler is fine if only one property (of one context) should be changed. It becomes critical if you need to change several properties. You might be able to circumvent this problem by using $$updateGroupId : $auto for your list binding.
  2. Method initialize is protected and should not be used in controller code.

Edit (24.06.2019): With SAPUI5 1.67 method Context.setProperty is introduced. This makes the workaround desribed above unnecessary.

Best regards Mathias.

cjohn001 commented 5 years ago

Hello Mathias, thanks for the feedback and sorry for the late reply. I had some other issues to sort out first. Well, if I would like to change multiple parameters for an item at the same time, this indeed does not look like a good option. I am currently investigating the stored procedure approach. Seems like teiid does not support native stored procedures, at least one cannot define user access constraints on them at the same time. This makes the approach infeasible for me. However, it seems Teiid supports virtual stored procedures at the teiid layer. This might be the option I am looking for. If this also does not work I will probably go for a native jquery message to change the data. Codewise this is probably the most feasible fallback approach. Best regards, Christoph

uhlmannm commented 5 years ago

Hi Christoph (@cjohn001 ),

with SAPUI5 1.67 the new method Context.setProperty is available. The workround described above is hence no longer required. See also chapter Accessing Data in Controller Code of the SAPUI5 documentation.

Best regards Mathias.

cjohn001 commented 5 years ago

Hello Mathias, Teiid in the meantime allows for stored procedures via odata and I could successfully solve my issue with it. For my use case it is also the best solution I think. But thanks to let me know. Allowing the change of multiple properties on clientside will allow for more efficient development in the future, great :)

cjohn001 commented 5 years ago

Hello Mathias (@uhlmannm), I just played around with the new function Context.setProperty and I do have a question regarding its interface. Like documented, in case of an error the error handler of the returned Promise of the function gets called. I need to say that I am also using error handling according to the odata v4 example which uses the message manager to intercept errors. Somthing like the following

            var oMessageManager = sap.ui.getCore().getMessageManager();
            var oMessageModel = oMessageManager.getMessageModel();
            this._oMessageModelBinding = oMessageModel.bindList("/", undefined, [], new Filter("technical", FilterOperator.EQ, true));
            this.getView().setModel(oMessageModel, "messages");

I was expecting to get an error from setProperty via the technical errors as well. However, this did not happen. Is this the intended behavior? I am just asking, as this introduces a new interface for error handling in addition to the MessageModel. Thanks for the info.

Best regards, Christoph

uhlmannm commented 5 years ago

Hi Christoph (@cjohn001 ),

could you please provide the error response you get?

As described in Server Messages in OData V4 Model the model will evaluate the error response and provide the respective messages to the message model. This should happen also when you use Context#setProperty.

Best regards Mathias.

cjohn001 commented 5 years ago

Hello guys, I am sorry for the late reply, but I had to re-implement the example first.

Please have a look at the following example.

Go to the options page under the following link:

https://ecubed-solutions.ddnss.de/#/Options user: sap password: sap

If you select a different country preference according to the image below you should get an error response from the server as I have modified the code to send a wrong value via setProperty:

Bildschirmfoto 2019-08-18 um 16 32 30

You will find the relevant code in file controller/SubMenu/Options.controller.js. as you can see, the method onMessageBindingChange: function(oEvent) should be called, but this does never happen.

The code for sending the message is in onUpdatePreferredCountryCodeForSearch.

Note 1: If I register functions on the returned promise of setProperty (line 117) I get the error messages in the success,error functions. However, the message model never receives the error message

Note 2: Please note, I tried two variants:

The error message returned by the server is as follows:

--batch_3d066de0-246b-427e-8126-d5d476bf51a3 Content-Type: application/http Content-Transfer-Encoding: binary

HTTP/1.1 400 Bad Request Content-Type: application/json;ieee754compatible=true;odata.metadata=minimal Content-Length: 95

{"error":{"code":null,"message":"Invalid value for property 'PreferredCountryCodeForSearch'."}} --batch_3d066de0-246b-427e-8126-d5d476bf51a3--

pksinsik commented 5 years ago

Hello Christoph,

for unbound messages like the http 400 above the promise of Context.setProperty rejects with the corresponding error, but the method by intent does not write a corresponding message to the message model. Reason: Contrary to PATCHes created via two-way binding, you have triggered the PATCH via API call and thus also have control on how to react on errors. You may handle the error via a message you create in yourself in the promise's reject handler or handle it directly there.

Best regards, Patric

cjohn001 commented 5 years ago

Hello Patric @uhlmannm and Mathias @pksinsik, so it seems your answers are inconsistent. In case Patric is right, which would be compliant to my observation we have to different interfaces for error handling. This again is inconsistent to Mathias answer which states both variants have the same interface. My question now is, is it a bug to have two interfaces or is it the intended behaviour. In the later case I would now adapt my code in this respect. In the first case I would wait for a fix.