Hi,
I'm looking into the dispatcher callback with a need,
I want to replace the http response with a wrapped
one that counts the bytes returned to the client.
Now, everything in the DispatcherCallback design
and implementation promises that a callback can
replace the arguments it gets with the one it will
return. For the init case:
Request init(Request);
should make it so that the Dispatcher ends up using
the Request provided by the callback, should it decide
to override it.
So far so good, but in practice it does not work for
a couple of reasons:
- the dispatcher does not really replace it Request
with the one coming from the dispatcher callback
chain. Ouch...
- the Request is implemented in a way that makes its
replacement almost impossible
The first item can be fixed easily, the latter requires
a little more work. The main stoppers are that the
Request fields are using default visibility and the
Dispatcher is accessing them directly, without using
the getters.
Now, for my use case I would be content to have
the Request expose setters: in this case the
callbacks could modify the Request contents without
replacing it.
Alternatively, if we want to make the Request replaceable
with "something else" we'd have to:
- make it possible to subclass it
- make it possible to wrap it
This would amount to make all the fields protected and
make the Dispatcher access them via the getters.
At that point a dispatcher callback could roll its
own wrapper that replaces one or more of the Request
fields.
Opinions?
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
Makes sense. When the dispatcher was originally written the internal Request class was not intended to be made public api. But... things evolve 
So it sounds reasonable, make Request subclassable and wrappable. A bit of work but mostly mechanical and low risk right?
Anyways, +1 on the idea.
-Justin
On 1/26/10 10:56 AM, Andrea Aime wrote:
Hi,
I'm looking into the dispatcher callback with a need,
I want to replace the http response with a wrapped
one that counts the bytes returned to the client.
Now, everything in the DispatcherCallback design
and implementation promises that a callback can
replace the arguments it gets with the one it will
return. For the init case:
Request init(Request);
should make it so that the Dispatcher ends up using
the Request provided by the callback, should it decide
to override it.
So far so good, but in practice it does not work for
a couple of reasons:
- the dispatcher does not really replace it Request
with the one coming from the dispatcher callback
chain. Ouch...
- the Request is implemented in a way that makes its
replacement almost impossible
The first item can be fixed easily, the latter requires
a little more work. The main stoppers are that the
Request fields are using default visibility and the
Dispatcher is accessing them directly, without using
the getters.
Now, for my use case I would be content to have
the Request expose setters: in this case the
callbacks could modify the Request contents without
replacing it.
Alternatively, if we want to make the Request replaceable
with "something else" we'd have to:
- make it possible to subclass it
- make it possible to wrap it
This would amount to make all the fields protected and
make the Dispatcher access them via the getters.
At that point a dispatcher callback could roll its
own wrapper that replaces one or more of the Request
fields.
Opinions?
Cheers
Andrea
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
Justin Deoliveira ha scritto:
Makes sense. When the dispatcher was originally written the internal Request class was not intended to be made public api. But... things evolve
So it sounds reasonable, make Request subclassable and wrappable. A bit of work but mostly mechanical and low risk right?
Ok. One thing. The problem could be addressed two ways:
- make all fields in the Request modifiable. Roll out setters. This
avoids the need to change the Dispatcher and still address my
current need. Won't make the Request any more wrappable though
- keep only getters, make fields protected, make Dispatcher access
them via getters. This makes Request easy to wrap and subclass.
Or we could do both.
What do you think?
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
On 1/26/10 1:27 PM, Andrea Aime wrote:
Justin Deoliveira ha scritto:
Makes sense. When the dispatcher was originally written the internal
Request class was not intended to be made public api. But... things
evolve
So it sounds reasonable, make Request subclassable and wrappable. A
bit of work but mostly mechanical and low risk right?
Ok. One thing. The problem could be addressed two ways:
- make all fields in the Request modifiable. Roll out setters. This
avoids the need to change the Dispatcher and still address my
current need. Won't make the Request any more wrappable though
- keep only getters, make fields protected, make Dispatcher access
them via getters. This makes Request easy to wrap and subclass.
Or we could do both.
What do you think?
Well since we are doing the work to change it I say let's go all the way. Let's add both setters and getters for each property and make it a good old plain java bean.
Cheers
Andrea
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
Justin Deoliveira ha scritto:
What do you think?
Well since we are doing the work to change it I say let's go all the way. Let's add both setters and getters for each property and make it a good old plain java bean.
Full monty patch available here: http://jira.codehaus.org/browse/GEOS-3787
Setters, getters, protected fields and javadoc, and changed the Dispatcher to actually allow callbacks to override the Request
object.
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.