[Geoserver-devel] A couple of development questions

Noticed a couple of things on the wms-merge branch:
-SVG support has been half commited, SVGMapResponse is there but requries SVGEncoder which is not
-build.xml geotools target was out of date, with the help of Chris this has been fixed

CVS tells me I should chat with groldan about SVGMapResponse, but I lack contact info.
The documentatino for the WFS Tester tells me that the code is available via cvs - does anyone know where?

Jody

I have updated to xalan 2.5.1 and xerces 2.5. If is an inconvenient for
some reasson I can roll the changes back.

Also removed the old arcsde-0.1.jar and updated
arcsde-SNAPSHOT.

Note that the current ArcSDE DataStore needs some review in
terms of connection usage. The point is that I want to be sure that
for a calling thread, there are no situations where more than a single
connection instance is needed, so I have setted up MAX_CONNECTIONS to 1
in SdeConnectionPool and started to work on it. So, in the while, it is
possible
that a GetFeature call over a SDE FeatureTypes fails.

Gabriel.

Made some changes in AbstractService:

1) if SPEED, FILE and BUFFER are static instances, so their methods should
be synchronized, ending in a not multiuser server, so I made saftyMode
dynamically instantiated in init() and the stratagy choosed* at web app
config level in web.xml. If I'm wrong, just tell me. If this is correct, may
be it will be better to allow for user customized ServiceStratagy
implementations to be parametrized by a servlet context param.

2) it is really important to ensure the work flow of the service strategy:
    - get a Request reader
    - ask it for the Request object. At this time, request parameters should
be fully checked i.e. the Request objects contains the list of
FeatureTypeConfig's rather than just the type names.
    - get the appropiate ResponseHandler
    - ask it to execute the Request
    - set the response content type
    - write to the http response's output stream

this is because we have to be sure that no exception have been produced
before setting the response's content type, so we can set the exception
specific content type; and that Response.getContentType is called AFTER
Response.execute, since the MIME type can depend on any request parameter or
another kind of desission making during the execute process. (i.e. FORMAT in
WMS GetMap)

So, if we just leave to ServiceStratagy the whole responsablity of dealing
with the service process, an implementation could violate this rules.

To do this, I've moved back this logic to AbstractService.doService and
changed ServiceTratagy to be:

class ServiceStratagy
{
    public OuputStream getDestination() throws IOException;
    public void flush();
}

and in AbstractService.doService:
...

Response.writeTo(stratagy.getDestination(httpServletResponse));
stratagy.flush();

Waiting for comments.

regards,

Gabriel

Gabriel Roldán wrote:

Made some changes in AbstractService:

1) if SPEED, FILE and BUFFER are static instances, so their methods should
be synchronized, ending in a not multiuser server, so I made saftyMode
dynamically instantiated in init() and the stratagy choosed* at web app
config level in web.xml. If I'm wrong, just tell me. If this is correct, may
be it will be better to allow for user customized ServiceStratagy
implementations to be parametrized by a servlet context param.

That sounds perfect, and was my intension. I kept making bad design decisions in FeatureResponse because I was unsure if the result was buffered or not. SPEED, FILE and BUFFER are the three ways of execution that Chris kept talking about, I just need to make them explicit so I could plan for them.

2) it is really important to ensure the work flow of the service strategy:
   - get a Request reader
   - ask it for the Request object. At this time, request parameters should
be fully checked i.e. the Request objects contains the list of
FeatureTypeConfig's rather than just the type names.
   - get the appropiate ResponseHandler
   - ask it to execute the Request
   - set the response content type
   - write to the http response's output stream

Thanks for writing that down, I'll make sure that workflow is documented in AbstractService.

this is because we have to be sure that no exception have been produced
before setting the response's content type, so we can set the exception
specific content type; and that Response.getContentType is called AFTER
Response.execute, since the MIME type can depend on any request parameter or
another kind of desission making during the execute process. (i.e. FORMAT in
WMS GetMap)

So, if we just leave to ServiceStratagy the whole responsablity of dealing
with the service process, an implementation could violate this rules.

I think we should right down those rules as something ServiceStratagy must follow, the only reason it did not work out is that I did not know the workflow.

To do this, I've moved back this logic to AbstractService.doService and
changed ServiceTratagy to be:

class ServiceStratagy
{
   public OuputStream getDestination() throws IOException;
   public void flush();
}

and in AbstractService.doService:
...

Response.writeTo(stratagy.getDestination(httpServletResponse));
stratagy.flush();

Waiting for comments.

I will have to look at this, the other thing we have to worry about is the abort contract, ResponseHandler will need to know when the opperation is aborted in order to clean up Transactions, release or Refresh Locks, and so on.

I'll write again when I have looked at your changes.

Thanks for the code review :slight_smile:

Jody

Still working through AbstractService - I added your workflow information.

In general you did a great job of making things work again, I am sorry
to have so many questions:

1) I was just walking through your doService implementation and found a
few places where serviceResponse.abort() was not called, and where
return; was not called after sendError().

2) We missed removing the temporary File during flush(), we also need to
remove the file when something goes wrong so I added an abort method to
the interface.

3) We have also lost ability to gzip the output stream if the response
supports it.
I am not sure where we should put this back in (it used to be in
BufferedStratagy) - oh wait you recomend moving it to a Filter Servlette :slight_smile:

4) We catch any Throwable from the writeTo and the call sendError(
response, throwable ). I added a check to this method to see if
sendError( response, ServiceException ) should be used instead. It may
be better to explicitly catch ServiceExceptions from this method.
There is a chance that getExceptionHandler().newServiceException( t,
pre, null ) actually just returns ServiceExceptions?
If so it was not documented as something a ExceptionHandler must do, so
I decided not to leave it up to chance.

5) I could not tell if you wanted the Stratagy implementations to close
the response output stream or not? BufferedStratagy does, SpeedStratagy
does not. Further more I could not tell if this should be done in the
abort method or not.
My guess is that doService should close the response output stream, and
abort() should stick with freeing and closing its own temporary output
streams.

6) I would like to throw an exception if flush() is called before
getDestination() rather than siliently not do anything

7) Did we want to consider using a Writer, or is that too XML specific
and should be left for writeTo implementations to decide?

8) Are we expected to supply a Buffered OutputStream to writeTo()?
SpeedStratagy uses a BufferedOutputStream, FileStratagy does not?
FeatureResponse uses a buffered writer internally, When used with
SpeedStratagy we are double buffering.

I have fixed these, or added comments where I have no clue.
Can you please look things over again to make sure I did not miss anything.

Jody

1) if SPEED, FILE and BUFFER are static instances, so their methods should
be synchronized, ending in a not multiuser server, so I made saftyMode
dynamically instantiated in init() and the stratagy choosed* at web app
config level in web.xml. If I'm wrong, just tell me. If this is correct, may
be it will be better to allow for user customized ServiceStratagy
implementations to be parametrized by a servlet context param.

Is there any reason we could not do this configuration in the global
section of services.xml? I'd like to see all configuration in one place,
to not make users have to figure out what a web.xml is and go in and
change the params. Granted only power users will probably be changing
this around, but I'd still like configuration all in one place.

  Chris

> 1) if SPEED, FILE and BUFFER are static instances, so their methods

should

> be synchronized, ending in a not multiuser server, so I made saftyMode
> dynamically instantiated in init() and the stratagy choosed* at web app
> config level in web.xml. If I'm wrong, just tell me. If this is correct,

may

> be it will be better to allow for user customized ServiceStratagy
> implementations to be parametrized by a servlet context param.

Is there any reason we could not do this configuration in the global
section of services.xml? I'd like to see all configuration in one place,
to not make users have to figure out what a web.xml is and go in and
change the params. Granted only power users will probably be changing
this around, but I'd still like configuration all in one place.

Actually no, there are no reason to not include it in the global service
config.
It was just the common case of having such a kind of configuration as web
app
parameter when one have to pass a class name as parameter, just like
defining it as a system property for the VM is.

But since we have keywords to identify it (SPEED, FILE and BUFFER),
I don't see a reason for not putting them in the global config. At least
that
we were so flexible that let users specify their own ServiceStratagy
implementations.
In such case, we can evaluate holding it at webapp config level or at
service global
config level.
for example, in web.xml;

<context-param>
    <param-name>serviceStratagy</param-name>
    <param-name>org.something.MyCustomStratagy</param-name>
</context-param>

should be more familiar for developers wanting to do such a customization.

Gabriel

Chris

-------------------------------------------------------
This SF. Net email is sponsored by: GoToMyPC
GoToMyPC is the fast, easy and secure way to access your computer from
any Web browser or wireless device. Click here to Try it Free!
https://www.gotomypc.com/tr/OSDN/AW/Q4_2003/t/g22lp?Target=mm/g22lp.tmpl
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel