Ticket #1646 (closed feature: fixed)

Opened 5 years ago

Last modified 5 years ago

add in framework for better vector behavior

Reported by: tschaub Owned by: tschaub
Priority: minor Milestone: 2.7 Release
Component: Layer.Vector Version: 2.6
Keywords: Cc:
State: Review

Description

Strategy and protocol base classes can be added from the vector behavior branch. Specific strategies and protocols can be added with other tickets.

Attachments

behavior.patch Download (11.8 KB) - added by tschaub 5 years ago.
base strategy and protocol
patch-1646-r7729-A0.diff Download (0.8 KB) - added by elemoine 5 years ago.

Change History

  Changed 5 years ago by tschaub

  • state set to Review

Changed 5 years ago by tschaub

base strategy and protocol

follow-up: ↓ 4   Changed 5 years ago by crschmidt

  • state changed from Review to Commit

So, I chatted with Tim about this. There's only one chunk of code that seems confusing to me: The response code stuff:

OpenLayers.Protocol.Response.SUCCESS_MASK    = 0x000000ff; 
OpenLayers.Protocol.Response.SUCCESS         = 0x00000001; 
OpenLayers.Protocol.Response.FAILURE_MASK    = 0x0000ff00; 
OpenLayers.Protocol.Response.FAILURE         = 0x00000100; 

I don't understand why we're doing this. Why don't we just make the definition of success any positive number (default 1, can be custom-overridden by the Protocol), and errors the same way, any number less than 0...

However, Tim has convinced me that we can go with this for now, and refactor later: he has promised to bring this up with Eric/the mailing list to discuss how to handle this. This patch is fine, but I expect we'll refactor response.code before we get to 2.7. (At least, I sort of hope so: 'success' being defined as '1-255' where 'failure' is '256-511' is weird to me...)

  Changed 5 years ago by tschaub

  • status changed from new to closed
  • state changed from Commit to Complete
  • resolution set to fixed

(In [7650]) Adding strategy and protocol base classes for better vector behavior. This just sets the common API for strategy and protocol. In general, a vector layer can have many strategies. A setLayer method gets called on each of these layers. When a layer is added to a map, any strategies on that layer get activated. When the layer is destroyed, any strategies get destroyed (this could be changed to deactivate for symmetry). A vector layer may also have a protocol. A protocol is typically constructed with a format. The layer doesn't need to know about the format. The protocol doesn't need to know about the layer. Strategies coordinate feature management for a layer. Specific strategies and protocols to follow. r=crschmidt (closes #1646)

in reply to: ↑ 2   Changed 5 years ago by elemoine

Chris, Tim,

What I was thinking of was abstracting the success and error codes. Examples: a REST protocol like MapFish or FeatureServer would map HTTP status codes to abstract codes, the WFS protocol would map WFS status codes to abstract codes, etc. In this way, the protocol user takes action based on these abstract status codes, it need not know what the actual protocol is, and how to deal with that protocol's status code.

Now I'm thinking this may be too complex, but I don't really how an alternative would look like. What do you guys think?

follow-up: ↓ 6   Changed 5 years ago by crschmidt

Eric,

I think that particular goal is perfect. I totally understand it. What I didn't understand is how you picked the *mechanism* of using bitstrings: Currently, we have a constant for SUCCESS: I assume that in the future we'll have constants for other SUCCESS style codes, like SUCCESS+FEATURES ENTERED... ah, I think I might understand why you were thinking bitstrings now. With bit strings, you can say:

return SUCCESS | ADDED_FEATURES

And then you can check whether features were added along with success? Is this what you were thinking?

My thought is that we don't need these 'combinations' of successes: success is anything greater than 0, and then we can set up constants for each type of success we care about. Failure is anything less than 0, and we can set up constants for each type of failure we care about. (For example, "SQL Failure to Execute" could be -10 or something.) To handle the specific error cases, you'll need to have some application level understanding to process anything beyond "success or failure" as far as I'm concerned.

Does this make sense? We can leave the "SUCCESS"/"FAILURE" constants as far as I'm concerned; I'd just like to change the success method and drop the "MASK" stuff.

in reply to: ↑ 5   Changed 5 years ago by elemoine

Replying to crschmidt:

Eric, I think that particular goal is perfect. I totally understand it. What I didn't understand is how you picked the *mechanism* of using bitstrings: Currently, we have a constant for SUCCESS: I assume that in the future we'll have constants for other SUCCESS style codes, like SUCCESS+FEATURES ENTERED...


Exactly.


ah, I think I might understand why you were thinking bitstrings now. With bit strings, you can say: return SUCCESS | ADDED_FEATURES And then you can check whether features were added along with success? Is this what you were thinking?


I was thinking that one could check success using:

this.code & OpenLayers.Protocol.Response.SUCCESS_MASK

as currently done in OpenLayers.Protocol.Response.success()


My thought is that we don't need these 'combinations' of successes: success is anything greater than 0, and then we can set up constants for each type of success we care about. Failure is anything less than 0, and we can set up constants for each type of failure we care about. (For example, "SQL Failure to Execute" could be -10 or something.) To handle the specific error cases, you'll need to have some application level understanding to process anything beyond "success or failure" as far as I'm concerned.


Yes. Using bitfields would be useful if we had more than 2 code types. Here we have success and failure, so using positive numbers to represent one code type (success) and negative numbers to represent the other code type (failure) is perfectly fine. Let's go with that.

Now I have one question related to your "SQL Failure to Execute" failure. Where would that constant be declared? In SQL.js or in Protocol.js? To prevent collisions we probably need to define all constants at a single place in Protocol.js.


Does this make sense? We can leave the "SUCCESS"/"FAILURE" constants as far as I'm concerned; I'd just like to change the success method and drop the "MASK" stuff.


Perfectly fine with me.

Changed 5 years ago by elemoine

  Changed 5 years ago by elemoine

  • status changed from closed to reopened
  • state changed from Complete to Review
  • resolution fixed deleted

As requested by Christopher, a patch to get rid of the unneeded SUCCESS and FAILURE masks. See patch-1646-r7729-A0.diff. Please review.

  Changed 5 years ago by elemoine

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

See #1677.

Note: See TracTickets for help on using tickets.