Ticket #2956 (closed feature: fixed)

Opened 3 years ago

Last modified 2 years ago

script protocol for reading features cross-origin

Reported by: tschaub Owned by: fredj
Priority: minor Milestone: 2.11 Release
Component: Protocol Version: 2.10
Keywords: Cc:
State: Commit

Description

A script protocol would make it possible to interface with feature providing services that accept a named callback.

Attachments

patch-2956-A0.diff Download (58.5 KB) - added by pgiraud 2 years ago.
patch with examples and unit tests
2956.patch Download (23.9 KB) - added by tschaub 2 years ago.
script protocol
2956_js_and_test_with_3032_applied.patch Download (21.7 KB) - added by vmische 2 years ago.
Patch for Script.js (including test) after #3032 got applied
2956.2.patch Download (26.0 KB) - added by tschaub 2 years ago.
script protocol for cross origin requests

Change History

  Changed 3 years ago by tschaub

I've put up an  example of a OpenLayers.Protocol.Script for cross-origin feature reading.

  Changed 2 years ago by pgiraud

Hi Tim,

I went further with your great work. First of all, I fixed 2 typos (See  11022). Then I put the OL.Protocol.HTTP:filterToParams method into a FilterSerializer mixin so that it can be shared between HTTP and Script protocols (See  11023)

This may be an advantage in two cases :

  • this helps users if they want to create their own custom filter serializer,
  • this helps building a lighter library if we only want the Script protocol, since it removes dependency on the HTTP one.

I also hope you're OK with me committing directly to your sandbox.

Changed 2 years ago by pgiraud

patch with examples and unit tests

  Changed 2 years ago by pgiraud

  • state set to Review

Patch attached. Please review.

  Changed 2 years ago by tschaub

Thanks for adding tests to this. I'll say I'm not that fond of the FilterSerializer. I do like that this patch takes all that specific filter serialization out of the HTTP protocol, but I don't think it necessarily makes it any more configurable. It would be nice if the app dev could provide their own filterToParams method. With this change, if OpenLayers.Protocol.FilterSerializer is loaded, it will clobber any custom filterToParams method.

OpenLayers.Protocol.FilterSerializer is a pretty generic name for a very specific convention for serializing filters. Is this convention unique to MapFish, or did it comes from elsewhere?

At a minimum, the script protocol should call initialize on the super after extending itself with the filterToParams (and regex2value) method from the mixin. I think I'd like it even better if that custom filterToParams method were assigned a more suitable name (e.g. a single OpenLayers.Protocol.mapFishFilterSerializer method would do the trick and you could configure your custom protocols to use it for filterToParams).

In case it is not obvious what I mean by the specific filterToParams method, I can also imagine methods that would serialize as  CQL or  ECQL or following the  GDATA guide for field conditions (though I think their  filter syntax for Analytics Feeds would be more relevant) or  any of  the others who have riffed on WHERE clauses from SQL.

In short, why the custom query syntax baked into OpenLayers?

(PS - Hope this doesn't sound overly harsh. I think it will be great to get this Script protocol in. The specific filterToParams bit has just puzzled me for a while.)

Changed 2 years ago by tschaub

script protocol

  Changed 2 years ago by tschaub

I've pulled out the specific filterToParams bit and think we should address that separately (#3032).

The patch now contains the original code and example with the addition of pgiraud's tests. Example works and tests pass in IE 6, FF 3, and Chrome 9. Thanks for any review.

Changed 2 years ago by vmische

Patch for Script.js (including test) after #3032 got applied

  Changed 2 years ago by vmische

I love this patch and really love to see this one in 2.11.

The serialization got refactored out in #3032. I've attached a patch that uses this serialization as default one, if no other is specified. This makes sense as it now has the same behaviour as the HTTP.js protocol has.

Tests (before and after the patch) pass on Firefox 3.6.13 and Chromium 11.0.678.0 (75511).

Changed 2 years ago by tschaub

script protocol for cross origin requests

  Changed 2 years ago by tschaub

Thanks for the added tests vmische. I've updated the patch to work after r11673. Patch includes tests and  example. Thanks for any review.

follow-up: ↓ 9   Changed 2 years ago by erilem

  • state changed from Review to Commit

Looks good. Tests pass in FireFox3, IE8, and Chrome 9. Please commit.

in reply to: ↑ 8   Changed 2 years ago by erilem

Replying to erilem:

Looks good. Tests pass in FireFox3, IE8, and Chrome 9. Please commit.

One minor thing: @requires OpenLayers/Protocol/HTTP.js should be removed.

  Changed 2 years ago by tschaub

  • status changed from new to closed
  • resolution set to fixed

(In [11691]) Adding a protocol for reading features cross-origin from services that support JSON with a callback. r=erilem (closes #2956)

Note: See TracTickets for help on using tickets.