Announcement

Collapse
No announcement yet.
X
  • Filter
  • Time
Clear All
new posts

    RestDataSource Queuing cacheAllData bug

    The failure is illustrated in the attached images.

    1. Screen A is opened with reference to 4 datasources which have cacheAllData=true on the datasource. Transaction 7 is triggered to fetch all data, everything is OK.
    2. Screen B is opened with reference to 3 of the 4 previous defined datasources and 1 new datasource.
    Transaction 22 is started to retrieve the data, but instead of sending the JSON formatted request, the following request is send:
    isc_tnum=22&isc_dataFormat=json (See the firebug screenshot).

    When I open screen B directly, without opening screen A (i.e. no datasources loaded), the result is ok.

    I have tested this with 3.0 nightly and 3.1p
    Attached Files
    Last edited by dencel; 29 Nov 2012, 02:00.

    #2
    I've tracked down the failing code:

    RPCManager.js@3105:
    Code:
    if (request.isRestRequest) {
    Only the first request is checked if this is an RestRequest, if this request is cached this condition will fail.

    Suggested solution:
    Code:
    var anyRestRequests = false;
    for (var i = 0; i < transaction.operations.length; i++) {
      if (transaction.operations[i].isRestRequest) {
        anyRestRequests = true;
        break;
      }
    }
    if (anyRestRequests) {

    Comment


      #3
      There is one more part that requires modification:
      If the Restrequest is a JSON request, there could be only one request in the transaction so the markup could be errornous when checking for i to add ','.
      Code:
      var operationCount = 0 // Introduced.. 
      for (var i = 0; i < transaction.operations.length; i++) {
          if (transaction.operations[i].clientOnly) continue;
          if (operationCount > 0) requestData += ", ";
          requestData += transaction.operations[i].data;
          operationCount++; // Introduced.. 
      }

      Comment


        #4
        Another solution would be to set isRestRequest on the request, but because the request is never transformed by RestDataSource this could be a problem.

        I've refactored the code a little bit more (sending single requests directly) and this is the end result working fine on mine environment:
        Code:
        var restRequests = []
        for (var i = 0; i < transaction.operations.length; i++) {
            if (transaction.operations[i].isRestRequest) {
              restRequests.push(transaction.operations[i])
            }
        }
        if (restRequests.length > 1) {
            if (request.contentType == this._$XML_MIME_TYPE) {
                requestData = "<transaction transactionNum=\"" + transaction.transactionNum + "\">";
                requestData += "<operations>"
                for (var i = 0; i < restRequests.length; i++) {
                    requestData += restRequests[i].data;
                }
                requestData += "</operations>";
                requestData += "</transaction>";
            } else {  // Not XML, must be JSON
                requestData = "{ \"transaction\": { \"transactionNum\": " + transaction.transactionNum + ", ";
                requestData += "\"operations\": [";
                for (var i = 0; i < restRequests.length; i++) {
                    if (i > 0) requestData += ", ";
                    requestData += restRequests[i].data;
                }
                requestData += "]}}";
            }
         } 
         else if(restRequests.length==1) {
              requestData = restRequests[0].data;
         }
         else if (request.useSimpleHttp) {
            requestData = request.data
         }

        Comment


          #5
          To clarify, you're saying that a mixed queue of RestDataSource requests and other types of requests (presumably useSimpleHttp) won't work.

          The correct approach here is actually to just not attempt to combine dissimilar request types, which fundamentally cannot be sent in a single HTTP request anyway, so there's no point. If it's tricky to refactor your code to avoid sending a request in the middle of a queue, you can use rpcRequest.sendNoQueue to avoid this problem.

          The code changes you suggest don't appear to work at all as is (they would just result in the useSimpleHttp request not being sent), and won't work anyway for more subtle reasons involving callbacks and post-processing of responses.

          Comment


            #6
            Ok, let me clarify a bit more (it's really a big problem for me).

            First of all I am using RestDataSources only. These have the option cacheAllData=true set.

            Now consider this screen initialization 'metacode':
            Code:
            RPCManager.startQueue()
            builSelectItemWithRestDatasourceA()
            RPCManager.sendQueue()
            The cacheAllData fetch is made for dataSource A.

            Secondly another screen is build:
            Code:
            RPCManager.startQueue()
            builSelectItemWithRestDatasourceA()
            builSelectItemWithRestDatasourceB()
            RPCManager.sendQueue()
            DataSourceA still contains the cached dataset, so according to DataSource.js@12615
            Code:
                    if (this.clientOnly || internalCacheRequest) {
                        rpcRequest.clientOnly = true;
                        rpcRequest.callback = {target:this, methodName:"_handleClientOnlyReply" };
            
                        isc.RPC.sendRequest(rpcRequest);
                        return;
                    }
            A simple RPC requests is added to the transaction list. (this one does not contain the .isRestRequest parameter).
            Now the cacheAllData fetch of DataSourceB is added and you have a mixture of two request types in the transaction list. The first one being the cached request!
            This is being illustrated in the images of my original post, see transaction 7 and afterwards transaction 22 (notice the failure and the raw data send).

            I think my last solution does solve this problem and does not have any subtle problems with it.
            To be more clear I suggest to replace RPCManager.js@3104 - 3131:
            Code:
                        var requestData;
                        if (request.isRestRequest) { // ONLY THE FIRST REQUEST IN THE TRANSACTIONLIST IS CHECKED! 
                            if (transaction.operations.length > 1) {
                                if (request.contentType == this._$XML_MIME_TYPE) {
                                    requestData = "<transaction transactionNum=\"" + transaction.transactionNum + "\">";
                                    requestData += "<operations>"
                                    for (var i = 0; i < transaction.operations.length; i++) {
                                        if (transaction.operations[i].clientOnly) continue;
                                        requestData += transaction.operations[i].data;
                                    }
                                    requestData += "</operations>";
                                    requestData += "</transaction>";
                                } else {  // Not XML, must be JSON
                                    requestData = "{ \"transaction\": { \"transactionNum\": " + transaction.transactionNum + ", ";
                                    requestData += "\"operations\": [";
                                    for (var i = 0; i < transaction.operations.length; i++) {
                                        if (transaction.operations[i].clientOnly) continue;
                                        if (i > 0) requestData += ", ";
                                        requestData += transaction.operations[i].data;
                                    }
                                    requestData += "]}}";
                                }
                            } else {
                                requestData = request.data;
                            }
                        } else if (request.useSimpleHttp) {
                            requestData = request.data
                        }
            with
            Code:
            var requestData;
            var restRequests = []
            for (var i = 0; i < transaction.operations.length; i++) {
                if (transaction.operations[i].isRestRequest) {
                  restRequests.push(transaction.operations[i])
                }
            }
            if (restRequests.length > 1) {
                if (request.contentType == this._$XML_MIME_TYPE) {
                    requestData = "<transaction transactionNum=\"" + transaction.transactionNum + "\">";
                    requestData += "<operations>"
                    for (var i = 0; i < restRequests.length; i++) {
                        requestData += restRequests[i].data;
                    }
                    requestData += "</operations>";
                    requestData += "</transaction>";
                } else {  // Not XML, must be JSON
                    requestData = "{ \"transaction\": { \"transactionNum\": " + transaction.transactionNum + ", ";
                    requestData += "\"operations\": [";
                    for (var i = 0; i < restRequests.length; i++) {
                        if (i > 0) requestData += ", ";
                        requestData += restRequests[i].data;
                    }
                    requestData += "]}}";
                }
             } 
             else if(restRequests.length==1) {
                  requestData = restRequests[0].data;
             }
             else if (request.useSimpleHttp) {
                requestData = request.data
             }
            There are essential no changes except my code makes sure the actual restRequests are send.

            Thanks for looking into it.
            Last edited by dencel; 29 Nov 2012, 14:51.

            Comment


              #7
              Ah, so you're actually talking about a queue of requests, all against RestDataSources, where some requests will be fulfilled by cacheAllData, without server contact.

              In this case the cacheAllData requests in the queue result in some nonsense data going to the server.

              Right?

              Comment


                #8
                Yeah, that is the right conclusion.

                This nonsense data is comming from request.data from the first request on the transaction list.

                These are my debugging observartions:
                1) @see RPCManager:2926,
                request variable is set to first request of transaction list.
                2) @see RPCManager:2956,
                special check for allClientOnly fails because there is one cacheAllData fetch in the transaction list
                3) @see RPCManager:3105,
                The check is made against the request variable (1) which does not have the isRestRequest property.
                4) @see RPCManager:3129,
                The requestData is set to the .data property of the request variable. Which is something like: isc_tnum=22&isc_dataFormat=json

                I hope this will make it clear to you.

                Comment


                  #9
                  I discovered a small error in my patchcode:

                  Code:
                   if (request.contentType == this._$XML_MIME_TYPE) {
                  Should be:
                  Code:
                   if (restRequests[0].contentType == this._$XML_MIME_TYPE) {
                  Is this bug going to be resolved soon? Currently I am patching the code myself.

                  Comment


                    #10
                    Yes, it's assigned to an engineer - we'll update this thread when it the fix makes it into a build in the next day or so

                    Comment


                      #11
                      Hello,

                      This did not make it into the nightly yet, can you give me an update?

                      Comment


                        #12
                        This should now be addressed in 3.1/8.3 and 4.0/9.0 - please retest with a build of 19/12 or later

                        Comment


                          #13
                          It works great now, thanks for updating.

                          Comment

                          Working...
                          X