[OpenLayers-Dev] Possible bug with Protocol.HTTP

Stephen Woodbridge woodbri at swoodbridge.com
Mon May 3 00:53:29 EDT 2010


Hi Devs,

A follow up on this problem is posted below ...

Stephen Woodbridge wrote:
> Hi Devs,
> 
> I have been working with OL.Protocol.HTTP and I think I have identified 
> a bug in the code after spending considerable time reading and tracing 
> the code via firebug. Basically if you create a Protocol.HTTP with a 
> user callback, it never gets called. Below is a summary of my analysis 
> and I would appreciate it if one of you can confirm or refute these 
> observations.
> 
> In OpenLayers.Protocol.HTTP.commit() near the end the code starts 
> issuing requests with:
> 
>          var queue = types[OpenLayers.State.INSERT];
>          if(queue.length > 0) {
>              resp.push(this.create(
>                  queue, OpenLayers.Util.applyDefaults(
>                      {callback: insertCallback, scope: this}, options.create
>                  )
>              ));
>          }
>          queue = types[OpenLayers.State.UPDATE];
>          for(var i=queue.length-1; i>=0; --i) {
>              resp.push(this.update(
>                  queue[i], OpenLayers.Util.applyDefaults(
>                      {callback: callback, scope: this}, options.update
>                  ))
>              );
>          }
>          queue = types[OpenLayers.State.DELETE];
>          for(var i=queue.length-1; i>=0; --i) {
>              resp.push(this["delete"](
>                  queue[i], OpenLayers.Util.applyDefaults(
>                      {callback: callback, scope: this}, options["delete"]
>                  ))
>              );
>          }
> 
> I think the arguments to OpenLayers.Util.applyDefaults(to, from) are 
> reversed in the above code. I think the intent here was to create a hash 
> like:
> 
>      options[requestType] = {callback: callback, scope: this}
> 
> that would get passed as the options array to to the create, update, 
> delete method respectively because this is what is expected later by the 
> OpenLayers.Protocol.HTTP.callUserCallback. Also not the callback() and 
> insertCallback() are local function to commit() and should not be 
> confused with this.callback.
> 
> And in fact the above code might be more correctly expressed as for example:
> 
>          queue = types[OpenLayers.State.DELETE];
>          for(var i=queue.length-1; i>=0; --i) {
>              var opt = OpenLayers.Util.applyDefaults(options, "delete: 
> {callback: callback, scope: this});
>              resp.push(this["delete"](queue[i], opt)
>              );
>          }
> 
> It would seem, this would more accurately reflect the usage below in 
> callUserCallback():
> 
>      callUserCallback: function(resp, options) {
>          var opt = options[resp.requestType];
>          if(opt && opt.callback) {
>              opt.callback.call(opt.scope, resp);
>          }
>      },
> 
> In current code (OL 2.8 and OL 2.9) opt is currently always undefined so 
> the callback never gets called.
> 
> A simple test of this is to create an OpenLayers.Protocol.HTTP with a
> 
>      callback: function() {alert( "Hello World!"); }
> 
> and issue a create, update, or delete request via the protocol and the 
> callback is not currently called.
> 
> I admit to a fledgling understanding of OpenLayers but I have been 
> trying to get my head around why the callback is not getting called. My 
> next step is to try and patch the "start issuing requests" block of code 
> and see if I can get it to work with a patch which I'll report back on, 
> but it would be nice if a dev can look at this analysis and provide some 
> feedback on it.
> 

I made the patch mentioned above but it still does not call the user 
callback. So it seems that I am still missing something. I think that 
this problem warrants one of the following:

1) if the callback capability does not work it should be removed
2) as Eric had indicated to me, that is it possible to register a 
"success" or "failure" event on the save strategy object so this might 
need example or a note in the docs somewhere
3) may be nothing or something else

I was able to get all but one of my issues resolved with the following 
patch to Protocol.HTTP.js which I will probably just turn into a new 
class Protocol.MyHTTP.js because my server is not implementing exactly 
HTTP.js although I might try to make it conform to that given some 
additional time and effort.

Here is my patch in case it might be helpful. Modifying the order of the 
arguments did not cause the callback function passed to the initializer 
to be called. I was also bothered by the fact that a bunch of places had:

     reqFeatures: feature,

which cause problems because the code tested for reqFeatures.length and 
was not processed, but changing that to:

     reqFeatures: [feature],

resolved that problem. The working (more or less) app is here:
http://imaptools.com:8080/tilecache/test.html?zoom=17&lat=33.89595&lon=35.49935&layers=BT


woodbri at mappy:/u/www/html/ol29/lib/OpenLayers/Protocol$ diff -Naur 
HTTP-orig.js HTTP.js
--- HTTP-orig.js        2010-05-02 12:22:55.000000000 -0400
+++ HTTP.js     2010-05-03 00:17:30.000000000 -0400
@@ -364,7 +364,7 @@
                    this.options.url + "/" + feature.fid;

          var resp = new OpenLayers.Protocol.Response({
-            reqFeatures: feature,
+            reqFeatures: [feature],
              requestType: "update"
          });

@@ -408,20 +408,22 @@
       *     completes.
       */
      "delete": function(feature, options) {
-        options = OpenLayers.Util.applyDefaults(options, this.options);
          var url = options.url ||
                    feature.url ||
                    this.options.url + "/" + feature.fid;

+        options = OpenLayers.Util.applyDefaults(options, this.options);
+
          var resp = new OpenLayers.Protocol.Response({
-            reqFeatures: feature,
+            reqFeatures: [feature],
              requestType: "delete"
          });

          resp.priv = OpenLayers.Request.DELETE({
              url: url,
              callback: this.createCallback(this.handleDelete, resp, 
options),
-            headers: options.headers
+            headers: options.headers,
+            data: this.format.write(feature) // my delete passes the 
object as xml instead of pulling the fid off the path
          });

          return resp;
@@ -572,27 +574,18 @@
          // start issuing requests
          var queue = types[OpenLayers.State.INSERT];
          if(queue.length > 0) {
-            resp.push(this.create(
-                queue, OpenLayers.Util.applyDefaults(
-                    {callback: insertCallback, scope: this}, options.create
-                )
-            ));
+            var opt = OpenLayers.Util.applyDefaults(options, {create: 
{callback: insertCallback, scope: this}});
+            resp.push(this.create(queue, opt));
          }
          queue = types[OpenLayers.State.UPDATE];
          for(var i=queue.length-1; i>=0; --i) {
-            resp.push(this.update(
-                queue[i], OpenLayers.Util.applyDefaults(
-                    {callback: callback, scope: this}, options.update
-                ))
-            );
+            var opt = OpenLayers.Util.applyDefaults(options, {update: 
{callback: callback, scope: this}});
+            resp.push(this.update(queue[i], opt));
          }
          queue = types[OpenLayers.State.DELETE];
          for(var i=queue.length-1; i>=0; --i) {
-            resp.push(this["delete"](
-                queue[i], OpenLayers.Util.applyDefaults(
-                    {callback: callback, scope: this}, options["delete"]
-                ))
-            );
+            var opt = OpenLayers.Util.applyDefaults(options, {update: 
{callback: callback, scope: this}});
+            resp.push(this["delete"](queue[i], opt));
          }
          return resp;
      },



More information about the Dev mailing list