[Geoserver-devel] review of dispatcher lifecycle interface

Hi all,

I have created an interface for the life cycle of a dispatcher request as per the discussion for http://jira.codehaus.org/browse/GEOS-2333. I have attached the interface and dispatcher patch to a new issue:

http://jira.codehaus.org/browse/GEOS-2402

A code review would be nice. Thanks.

-Justin

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Justin Deoliveira ha scritto:

Hi all,

I have created an interface for the life cycle of a dispatcher request as per the discussion for http://jira.codehaus.org/browse/GEOS-2333. I have attached the interface and dispatcher patch to a new issue:

http://jira.codehaus.org/browse/GEOS-2402

A code review would be nice. Thanks.

The DispatcherCallback seems pretty complete. Afaik the callback can
alter everything in the request, but not the object returned from
dispatching. Say that you may want to wrap it, or swap it solid with
something else, what about that?

Also, I see no place for exceptions. One thing that we may want to
do is to add request validation, such as refusing WMS requests whose
size is more than 1000x1000, of WFS validation, say you have a huge
data set and don't want people to download it fully, so you
want to check they added a bbox filter and/or an attribute filter.
Stuff like that seems like a good fit for this callback, and I guess
they would throw an exception. Hmm... service exceptions are runtime
exceptions, so no need to declare them, but making it explicit would
help from a documentation point of view.

Just as a style thought, wouldn't it be nicer to have a separate
class dealing with forwarding all of the messages to listeners
rather than having all the fireXXX methods sprinkled around in
the dispatcher (which is quite big already)? Anyways, not a big deal.

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Andrea Aime wrote:

Justin Deoliveira ha scritto:

Hi all,

I have created an interface for the life cycle of a dispatcher request as per the discussion for http://jira.codehaus.org/browse/GEOS-2333. I have attached the interface and dispatcher patch to a new issue:

http://jira.codehaus.org/browse/GEOS-2402

A code review would be nice. Thanks.

The DispatcherCallback seems pretty complete. Afaik the callback can
alter everything in the request, but not the object returned from
dispatching. Say that you may want to wrap it, or swap it solid with
something else, what about that?

Good point, changing init to return the Request seems like a good idea.

Also, I see no place for exceptions. One thing that we may want to
do is to add request validation, such as refusing WMS requests whose
size is more than 1000x1000, of WFS validation, say you have a huge
data set and don't want people to download it fully, so you
want to check they added a bbox filter and/or an attribute filter.
Stuff like that seems like a good fit for this callback, and I guess
they would throw an exception. Hmm... service exceptions are runtime
exceptions, so no need to declare them, but making it explicit would
help from a documentation point of view.

Yeah good point, I think the interface should state that the callback should throw ServiceException's when things go haywire... or some validation or check fails.

Just as a style thought, wouldn't it be nicer to have a separate
class dealing with forwarding all of the messages to listeners
rather than having all the fireXXX methods sprinkled around in
the dispatcher (which is quite big already)? Anyways, not a big deal.

Can do, a multiplexer of sorts :slight_smile:

Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Justin Deoliveira ha scritto:

Andrea Aime wrote:

Justin Deoliveira ha scritto:

Hi all,

I have created an interface for the life cycle of a dispatcher request as per the discussion for http://jira.codehaus.org/browse/GEOS-2333. I have attached the interface and dispatcher patch to a new issue:

http://jira.codehaus.org/browse/GEOS-2402

A code review would be nice. Thanks.

The DispatcherCallback seems pretty complete. Afaik the callback can
alter everything in the request, but not the object returned from
dispatching. Say that you may want to wrap it, or swap it solid with
something else, what about that?

Good point, changing init to return the Request seems like a good idea.

Actually I was thinking changing
void operationExecuted( Request request, Operation operation, Object result );

to return an override result so that you can for example wrap the
resulting feature collection somehow.

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Actually I was thinking changing
void operationExecuted( Request request, Operation operation, Object result );

to return an override result so that you can for example wrap the
resulting feature collection somehow.

Ah... right, sorry, But yeah, +100 :slight_smile:

Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.