GeoServer

On Wed, Feb 29, 2012 at 8:53 PM, Martin Davis <mdavis@...> wrote:
> The invertQuery method in Rendering transformation processes is very useful
> (in fact, essential).  But I've just realized a Catch-22 with it.
>
> I need to expand the bounding box of the query (to capture more features for
> accurate surface generation).  I want the user to be able to specify the
> size of the expansion.  The obvious place to do this is in the process
> parameters.  But - the invertQuery method is called *before* the parameters
> are supplied to the execute method!  So for the first call at least, the
> query is not expanded by the correct amount.  (The process class saves the
> expandBy parameter in an instance variable, so subsequent calls operate
> correctly).
>
> Is there a better way to do this?

A stateful transformation is dangeours business, as you have just one
instantiated,
ever (styles are cached, and so is whatever is inside of them),
and it serves all concurrent requests for all possible requests, maybe
for different
maps or with different parameters.

Long story short, do whatever you need to in order to make the transformation
stateless.

The rendering transformation framework was setup, proposed and accepted
by the community in this geotools improvement proposal:
http://docs.codehaus.org/display/GEOTOOLS/Rendering+transformations

I see you are struggling with the current design, all you can do to
make it better
is to follow the same road I walked when setting up rendering transformations:
check your needs, propose a design, setup a improvement proposal, discuss it
and get it approved

My main driver were vector to vector and raster to vector transformations,
with a focus on making it possible and making it reusable as WPS processes
and transformations at the same time.
Your setup is different, you are trying to walk the opposite direction, want it
to be easier, and it seems, setup in such a way that you don't need to care
about thread safety. The improvement proposal is the only way to revise an
existing API (or to roll new API), so that's really your only option.

Cheers
Andrea

-- 
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy

phone: +39 0584 962313
fax:      +39 0584 962313
mob:    +39 339 8844549

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-------------------------------------------------------

On Wed, Feb 29, 2012 at 1:56 PM, Andrea Aime
<andrea.aime@...:

> On Wed, Feb 29, 2012 at 8:53 PM, Martin Davis <mdavis@...> wrote:
> > The invertQuery method in Rendering transformation processes is very
> useful
> > (in fact, essential).  But I've just realized a Catch-22 with it.
> >
> > I need to expand the bounding box of the query (to capture more features
> for
> > accurate surface generation).  I want the user to be able to specify the
> > size of the expansion.  The obvious place to do this is in the process
> > parameters.  But - the invertQuery method is called *before* the
> parameters
> > are supplied to the execute method!  So for the first call at least, the
> > query is not expanded by the correct amount.  (The process class saves
> the
> > expandBy parameter in an instance variable, so subsequent calls operate
> > correctly).
> >
> > Is there a better way to do this?
>
> A stateful transformation is dangeours business, as you have just one
> instantiated,
> ever (styles are cached, and so is whatever is inside of them),
> and it serves all concurrent requests for all possible requests, maybe
> for different
> maps or with different parameters.
>

Hmmm...  do you mean there is a single copy of the process object shared
across all styles which reference that process?  Or is there a separate one
created for each style which uses it?  If the former, then yes, I have a
problem!


> Long story short, do whatever you need to in order to make the
> transformation
> stateless.
>
> The rendering transformation framework was setup, proposed and accepted
> by the community in this geotools improvement proposal:
> http://docs.codehaus.org/display/GEOTOOLS/Rendering+transformations
>
> I see you are struggling with the current design, all you can do to
> make it better
> is to follow the same road I walked when setting up rendering
> transformations:
> check your needs, propose a design, setup a improvement proposal, discuss
> it
> and get it approved
>
> My main driver were vector to vector and raster to vector transformations,
> with a focus on making it possible and making it reusable as WPS processes
> and transformations at the same time.
> Your setup is different, you are trying to walk the opposite direction,
> want it
> to be easier, and it seems, setup in such a way that you don't need to care
> about thread safety. The improvement proposal is the only way to revise an
> existing API (or to roll new API), so that's really your only option.
>

I totally agree stateless is best. And I'm certainly not criticizing the
original design - Rendering Transformations are very powerful and extremely
useful!

I am just not seeing how I can take advantage of the inveryQuery method to
alter the query window as needed (and as the original proposal stated as a
design goal).

At this point I'm not looking for easiness - just a workable solution!  8^)


-- 
Martin Davis
OpenGeo - http://opengeo.org
Expert service straight from the developers.

On Wed, Feb 29, 2012 at 11:33 PM, Martin Davis <mdavis@...> wrote:
>> A stateful transformation is dangeours business, as you have just one
>> instantiated,
>> ever (styles are cached, and so is whatever is inside of them),
>> and it serves all concurrent requests for all possible requests, maybe
>> for different
>> maps or with different parameters.
>
>
> Hmmm...  do you mean there is a single copy of the process object shared
> across all styles which reference that process?  Or is there a separate one
> created for each style which uses it?  If the former, then yes, I have a
> problem!

Hmm... good question... I had to look it up in the code.
So, process factories are free to instantiate a process multiple times,
each time create(Name) is called, but the Spring based one will
end up calling:
return applicationContext.getBean(beanName);

which means Spring will return the actual process bean, and that normally
means it's a singleton, unless you declared otherwise in the application
context.

However rendering transformations are filter functions, wrappers around
processes, created by the ProcessFunctionFactory.. and as far as I can
see there too you get the on the fly creation of the process.

That said, styles are cached and they refer to the filter function wrapping
the process directly, which means each style has its own private
rendering transform, but that one is long lived, that is, all requests hitting
that style will share the same function instance and thus the same process.

> I totally agree stateless is best. And I'm certainly not criticizing the
> original design - Rendering Transformations are very powerful and extremely
> useful!

The original design has limits, it was a first attempt to do transformations on
the fly, and could certainly be improved. Since I'm not able to provide you
with solutions to your case it certainly looks there are holes and
that improvements
could be made, I was ponting at the only tool that can work out improvements.

> I am just not seeing how I can take advantage of the inveryQuery method to
> alter the query window as needed (and as the original proposal stated as a
> design goal).

Mumble... if you directly implement a FilterFunction you should have the
full list of arguments _before_ execute is called (since when the function
the arguemnts are known) and... oh...
Have a look at the RenderingProcess interface, which an annotated
process can implement to get rendering transform support and... voilà, see
the method signatures:

Query invertQuery(Map<String, Object> input, Query targetQuery,
GridGeometry gridGeometry) throws ProcessException;

GridGeometry invertGridGeometry(Map<String, Object> input, Query targetQuery,
            GridGeometry targetGridGeometry) throws ProcessException;

You actually get full access to the parameters in the map form.
I believe that's what you're looking for.
Doh, I added that interface over six months ago and I did not remember
it was there

Cheers
Andrea

-- 
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy

phone: +39 0584 962313
fax:      +39 0584 962313
mob:    +39 339 8844549

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-------------------------------------------------------

On Wed, Feb 29, 2012 at 11:31 PM, Andrea Aime
<andrea.aime@...:

> On Wed, Feb 29, 2012 at 11:33 PM, Martin Davis <mdavis@...> wrote:
> >> A stateful transformation is dangeours business, as you have just one
> >> instantiated,
> >> ever (styles are cached, and so is whatever is inside of them),
> >> and it serves all concurrent requests for all possible requests, maybe
> >> for different
> >> maps or with different parameters.
> >
> >
> > Hmmm...  do you mean there is a single copy of the process object shared
> > across all styles which reference that process?  Or is there a separate
> one
> > created for each style which uses it?  If the former, then yes, I have a
> > problem!
>
> Hmm... good question... I had to look it up in the code.
> So, process factories are free to instantiate a process multiple times,
> each time create(Name) is called, but the Spring based one will
> end up calling:
> return applicationContext.getBean(beanName);
>
> which means Spring will return the actual process bean, and that normally
> means it's a singleton, unless you declared otherwise in the application
> context.
>
> However rendering transformations are filter functions, wrappers around
> processes, created by the ProcessFunctionFactory.. and as far as I can
> see there too you get the on the fly creation of the process.
>
> That said, styles are cached and they refer to the filter function wrapping
> the process directly, which means each style has its own private
> rendering transform, but that one is long lived, that is, all requests
> hitting
> that style will share the same function instance and thus the same process.
>

Whew!  I sort of followed that...  However, my debugger shows that there is
only ONE copy of the process across all styles.  Not sure how this changes
the thinking above..   But I was going to suggest that maybe it would be
best to simply create a new Process object for every rendering request.
 The time for object creation is a fraction of the time for parameter
marshalling - not to mention the time for the process itself.  That way
there's less for everyone to worry about  (OO Rule #42 - Singletons are
evil...)

>
> > I am just not seeing how I can take advantage of the inveryQuery method
> to
> > alter the query window as needed (and as the original proposal stated as
> a
> > design goal).
>
> Mumble... if you directly implement a FilterFunction you should have the
> full list of arguments _before_ execute is called (since when the function
> the arguemnts are known) and... oh...
> Have a look at the RenderingProcess interface, which an annotated
> process can implement to get rendering transform support and... voilà, see
> the method signatures:
>
> Query invertQuery(Map<String, Object> input, Query targetQuery,
> GridGeometry gridGeometry) throws ProcessException;
>
> GridGeometry invertGridGeometry(Map<String, Object> input, Query
> targetQuery,
>            GridGeometry targetGridGeometry) throws ProcessException;
>
> You actually get full access to the parameters in the map form.
> I believe that's what you're looking for.
> Doh, I added that interface over six months ago and I did not remember
> it was there
>

I realized this myself yesterday as well...  (at least, I was assuming that
the map "input" was the input expressions, since it's not yet documented).
 I was also confused by this because my Annotation-driven process is
receiving a null for that parameter.  It turns out (I think) this is a bug
in the annotation-driven wrapper ( InvokeMethodRenderingProcess, line 544 -
the input map should be inserted directly into the args array, rather than
being passed through buildProcessArguments).

In the short term I will try and re-implement as a raw Process, and see if
I get the invertQuery args correctly.  In the long term it would be nice to
get that bug fixed, since anno-driven processes are much nicer to write
(and read!)

BTW, I don't think an anno-driven process can implement the
RenderingProcess interface, since that requires implementing
execute(Map...) as well - which then gives two execute methods?   Might be
nice to make that interface a mixin, rather than an extension of Process?
-- 
Martin Davis
OpenGeo - http://opengeo.org
Expert service straight from the developers.

On Thu, Mar 1, 2012 at 6:47 PM, Martin Davis <mdavis@...> wrote:
> Whew!  I sort of followed that...  However, my debugger shows that there is
> only ONE copy of the process across all styles.  Not sure how this changes
> the thinking above..   But I was going to suggest that maybe it would be
> best to simply create a new Process object for every rendering request.  The
> time for object creation is a fraction of the time for parameter marshalling
> - not to mention the time for the process itself.  That way there's less for
> everyone to worry about  (OO Rule #42 - Singletons are evil...)

Singletons in Spring are the rule for good reason (and they are not implemented
as singletons, but the spring context keeps all beans up as general facilities).

Feel free to find a way to get a process be created anew each time and
make that avaiable either as an option and/or as the default
(some processes might have a very high init setup time, so it cannot be
the only option)

>> > I am just not seeing how I can take advantage of the inveryQuery method
>> > to
>> > alter the query window as needed (and as the original proposal stated as
>> > a
>> > design goal).
>>
>> Mumble... if you directly implement a FilterFunction you should have the
>> full list of arguments _before_ execute is called (since when the function
>> the arguemnts are known) and... oh...
>> Have a look at the RenderingProcess interface, which an annotated
>> process can implement to get rendering transform support and... voilà, see
>> the method signatures:
>>
>> Query invertQuery(Map<String, Object> input, Query targetQuery,
>> GridGeometry gridGeometry) throws ProcessException;
>>
>> GridGeometry invertGridGeometry(Map<String, Object> input, Query
>> targetQuery,
>>            GridGeometry targetGridGeometry) throws ProcessException;
>>
>> You actually get full access to the parameters in the map form.
>> I believe that's what you're looking for.
>> Doh, I added that interface over six months ago and I did not remember
>> it was there
>
>
> I realized this myself yesterday as well...  (at least, I was assuming that
> the map "input" was the input expressions, since it's not yet documented).
>  I was also confused by this because my Annotation-driven process is
> receiving a null for that parameter.  It turns out (I think) this is a bug
> in the annotation-driven wrapper ( InvokeMethodRenderingProcess, line 544 -
> the input map should be inserted directly into the args array, rather than
> being passed through buildProcessArguments).

Patches + tests welcomed

>
> BTW, I don't think an anno-driven process can implement the RenderingProcess
> interface, since that requires implementing execute(Map...) as well - which
> then gives two execute methods?   Might be nice to make that interface a
> mixin, rather than an extension of Process?

Agreed. Have a look at what I did in the process that georeferences rasters
on trunk, it is a rendering transformation. Probably some hack based on a
naming convention.

Again, patches + tests welcomed :-)

Cheers
Andrea

-- 
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy

phone: +39 0584 962313
fax:      +39 0584 962313
mob:    +39 339 8844549

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-------------------------------------------------------

Andrea, to follow this up - I followed your advice about keeping the
process stateless, and using SLD env variables to provide the output image
parameters.  After fixing a bug in RenderingProcessFunction, and fully
understanding how annotation-driven rendering processes work - everything
works perfectly.

Will be committing a patch for this shortly.

On Thu, Mar 1, 2012 at 10:19 AM, Andrea Aime
<andrea.aime@...:

> On Thu, Mar 1, 2012 at 6:47 PM, Martin Davis <mdavis@...> wrote:
> > Whew!  I sort of followed that...  However, my debugger shows that there
> is
> > only ONE copy of the process across all styles.  Not sure how this
> changes
> > the thinking above..   But I was going to suggest that maybe it would be
> > best to simply create a new Process object for every rendering request.
>  The
> > time for object creation is a fraction of the time for parameter
> marshalling
> > - not to mention the time for the process itself.  That way there's less
> for
> > everyone to worry about  (OO Rule #42 - Singletons are evil...)
>
> Singletons in Spring are the rule for good reason (and they are not
> implemented
> as singletons, but the spring context keeps all beans up as general
> facilities).
>
> Feel free to find a way to get a process be created anew each time and
> make that avaiable either as an option and/or as the default
> (some processes might have a very high init setup time, so it cannot be
> the only option)
>
> >> > I am just not seeing how I can take advantage of the inveryQuery
> method
> >> > to
> >> > alter the query window as needed (and as the original proposal stated
> as
> >> > a
> >> > design goal).
> >>
> >> Mumble... if you directly implement a FilterFunction you should have the
> >> full list of arguments _before_ execute is called (since when the
> function
> >> the arguemnts are known) and... oh...
> >> Have a look at the RenderingProcess interface, which an annotated
> >> process can implement to get rendering transform support and... voilà,
> see
> >> the method signatures:
> >>
> >> Query invertQuery(Map<String, Object> input, Query targetQuery,
> >> GridGeometry gridGeometry) throws ProcessException;
> >>
> >> GridGeometry invertGridGeometry(Map<String, Object> input, Query
> >> targetQuery,
> >>            GridGeometry targetGridGeometry) throws ProcessException;
> >>
> >> You actually get full access to the parameters in the map form.
> >> I believe that's what you're looking for.
> >> Doh, I added that interface over six months ago and I did not remember
> >> it was there
> >
> >
> > I realized this myself yesterday as well...  (at least, I was assuming
> that
> > the map "input" was the input expressions, since it's not yet
> documented).
> >  I was also confused by this because my Annotation-driven process is
> > receiving a null for that parameter.  It turns out (I think) this is a
> bug
> > in the annotation-driven wrapper ( InvokeMethodRenderingProcess, line
> 544 -
> > the input map should be inserted directly into the args array, rather
> than
> > being passed through buildProcessArguments).
>
> Patches + tests welcomed
>
> >
> > BTW, I don't think an anno-driven process can implement the
> RenderingProcess
> > interface, since that requires implementing execute(Map...) as well -
> which
> > then gives two execute methods?   Might be nice to make that interface a
> > mixin, rather than an extension of Process?
>
> Agreed. Have a look at what I did in the process that georeferences rasters
> on trunk, it is a rendering transformation. Probably some hack based on a
> naming convention.
>
> Again, patches + tests welcomed :-)
>
> --
Martin Davis
OpenGeo - http://opengeo.org
Expert service straight from the developers.